[Spice-devel] [PATCH spice-streaming-agent v3 1/5] Install udev rule

[Frediano Ziglio]

> 
> On 06/01/2018 12:12 AM, Frediano Ziglio wrote:
> >>
> >> On 05/23/2018 12:03 PM, Frediano Ziglio wrote:
> >>> The udev rule is used to do some action when the device is added to the
> >>> system. Current rule change the permission of the special file to allow
> >>> to
> >>> open it by any user.
> >>> Some systems use /lib/udev while others use /usr/lib/udev.
> >>> Allow to specify the full path to support both type of systems.
> >>>
> >>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> >>> ---
> >>>    Makefile.am                   |  4 ++++
> >>>    configure.ac                  | 10 ++++++++++
> >>>    spice-streaming-agent.spec.in |  4 +++-
> >>>    3 files changed, 17 insertions(+), 1 deletion(-)
> >>>
> >>> Changes since v2:
> >>> - use pkg-config to get default udev directory
> >>
> >> Hi,
> >>
> >> This fails make install and make distcheck (regular user can not
> >> install files under /usr/lib/ ... ) .
> >>
> > 
> > Looks like is expected, see
> > https://stackoverflow.com/questions/26980634/can-i-install-a-systemd-file-during-distcheck-using-dc-install-base
> > 
> > 
> > I had to add
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 32fdaff..a2da845 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -15,6 +15,8 @@ gdmautostart_DATA =
> > $(builddir)/data/spice-streaming.desktop
> >   pkgconfigdir = $(libdir)/pkgconfig
> >   pkgconfig_DATA = spice-streaming-agent.pc
> > 
> > +AM_DISTCHECK_CONFIGURE_FLAGS =
> > --with-udevrulesdir='$$(prefix)/$(UDEVRULESDIR)'
> > +
> >   udevrulesdir = $(UDEVRULESDIR)
> >   udevrules_DATA = $(srcdir)/data/90-spice-guest-streaming.rules
> > 
> > 
> > now is working.
> > Looks like make distcheck uses the prefix instead of DESTDIR (used for
> > instance
> > by rpm to avoid these issues).
> > I think in a normal usage "make install" should install udev rule requiring
> > root permissions.
> 
> I prefer that the following will not fail as non-root user:
> ./configure --prefix=/tmp/mydir && make install
> (also not that great when running as root).
> 

It would make sense only if by default you don't install udev rules,
if you do they must be installed where udev will read, unless you
want to change udev to read the rules from your directory but in
this case you need root too so will fail again.
The only sensible option would be to not install udev rules by
default and add a --enable-udev (or similar).

Similar issue happens with spice-gtk and policy kit for the same
reason, fails if I don't specify --disable-polkit.

> Anyway, this patch now works on Fedora 28, but fails on RHEL-7.
> 
> On my RHEL-7, "autoreconf -fi" fails with:
> configure.ac:58: error: possibly undefined macro: PKG_CHECK_VAR
> 
> Searching the internet I found this bug
> https://bugzilla.redhat.com/show_bug.cgi?id=1084372
> 

Sending a new version, looks like is easier to call pkg-config
directly instead of trying to provide PKG_CHECK_VAR (available
since pkg-config 0.28, RHEL7 has 0.27).

Frediano

> Uri.
> 
> 
> > 
> >> I changed this patch as follows (v4):
> >> - do not install the udev rule (remove this part from Makefile.am)
> >> - do not change configure.ac (no need for UDEVRULESDIR anymore)
> >> - spec-file: do not change configure
> >> - spec-file: copy (install) the udev-rule directly
> >>
> >> Uri.
> >>
> >> =====
> >>
> >>   From e5b7287fb7908e2d3ced19b229d61ca4540134f5 Mon Sep 17 00:00:00 2001
> >> From: Frediano Ziglio <fziglio at redhat.com>
> >> Date: Thu, 31 May 2018 22:19:59 +0300
> >> Subject: [PATCH spice-streaming-agent v3 1/5] spec-file: add udev rule
> >>
> >> The udev rule is used to do some action when the device is added to the
> >> system. Current rule changes the permission of the virtio serial port
> >> device
> >> to allow opening it by any user.
> >>
> >> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> >> Signed-off-by: Uri Lublin <uril at redhat.com>
> >> ---
> >>    Makefile.am                   | 1 +
> >>    spice-streaming-agent.spec.in | 5 +++++
> >>    2 files changed, 6 insertions(+)
> >>
> >> diff --git a/Makefile.am b/Makefile.am
> >> index 94ad7aa..291a883 100644
> >> --- a/Makefile.am
> >> +++ b/Makefile.am
> >> @@ -18,5 +18,6 @@ pkgconfig_DATA = spice-streaming-agent.pc
> >>    EXTRA_DIST = \
> >>    	spice-streaming-agent.spec \
> >>    	spice-streaming-agent.pc \
> >> +	data/90-spice-guest-streaming.rules \
> >>    	data/spice-streaming.desktop \
> >>    	$(NULL)
> >> diff --git a/spice-streaming-agent.spec.in b/spice-streaming-agent.spec.in
> >> index 132a851..ec144f6 100644
> >> --- a/spice-streaming-agent.spec.in
> >> +++ b/spice-streaming-agent.spec.in
> >> @@ -10,6 +10,7 @@ BuildRequires:  spice-protocol >=
> >> @SPICE_PROTOCOL_MIN_VER@
> >>    BuildRequires:  libX11-devel libXfixes-devel
> >>    BuildRequires:  libjpeg-turbo-devel
> >>    BuildRequires:  catch-devel
> >> +BuildRequires:  pkgconfig(udev)
> >>    # we need /usr/sbin/semanage program which is available on different
> >>    # packages depending on distribution
> >>    Requires(post): /usr/sbin/semanage
> >> @@ -43,6 +44,9 @@ if test -d "%{buildroot}/%{_libdir}/%{name}/plugins";
> >> then
> >>        find %{buildroot}/%{_libdir}/%{name}/plugins -name '*.la' -delete
> >>    fi
> >>
> >> +mkdir -p %{buildroot}/%{_udevrulesdir}
> >> +install data/90-spice-guest-streaming.rules %{buildroot}/%{_udevrulesdir}
> >> +
> >>    %post
> >>    semanage fcontext -a -t xserver_exec_t
> >> %{_bindir}/spice-streaming-agent 2>/dev/null || :
> >>    restorecon %{_bindir}/spice-streaming-agent || :
> >> @@ -55,6 +59,7 @@ fi
> >>
> >>    %files
> >>    %doc COPYING ChangeLog README
> >> +%{_udevrulesdir}/90-spice-guest-streaming.rules
> >>    %{_bindir}/spice-streaming-agent
> >>    %{_sysconfdir}/xdg/autostart/spice-streaming.desktop
> >>    %{_datadir}/gdm/greeter/autostart/spice-streaming.desktop
> >