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

Fri Jun 1 09:25:34 UTC 2018

On Fri, 2018-06-01 at 04:56 -0400, Frediano Ziglio wrote:
> > 
> > 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.

I would say if the user specifies the prefix, let him care whether the
udev rules will actually be read by anything. I'm with Uri on not
failing when non-root user installs into a prefix. But I'm not sure I'm
not missing part of the problem so please apply a grain of salt if you
would :)

Lukas

> 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
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel