[pulseaudio-discuss] [PATCH 6/8] launch: Add systemd units for launching pulseaudio user instances

[Tanu Kaskinen]

On Tue, 2014-11-18 at 11:10 +0000, Colin Guthrie wrote:
> Tanu Kaskinen wrote on 18/11/14 10:26:
> >> I do think it will be quite hard to support this scheme when you pass
> >> e.g. --prefix /home/colin/builds/pa/ and it somehow still knows to put
> >> the units in /usr/local (obviously it doesn't make sense to use
> >> $prefix/systemd/user/ as a path as this only works for two very specific
> >> values of $prefix (three if you count /etc as prefix but you'd have to
> >> be pretty mad to do that!)).
> > 
> > If I pass --prefix=/home/tanu/foo to configure without any other
> > parameters, I expect that "make install" will NOT try to write to any
> > system location. If the prefix points to a location under the user's
> > home directory, "make install" should not require root access.
> 
> I both agree and disagree! I agree that it should not need root privs
> but I also think that if you're really supporting this properly you'd
> need to intentionally break some features.

I'm not sure what you mean by "properly", but it's not a goal for me to
make --prefix=/home/tanu/foo work perfectly without any additional
options. I just want that it doesn't do anything unexpected, and
installing to a system location is unexpected behaviour IMO.

> e.g. there is *no* directory the user can use without root privileges
> where updated udev rules would be read from. So of the user is hacking
> on that part and uses --prefix=/home/tanu/foo then there should be a big
> fat warning that udev rule installation has been disabled due to
> no-standard prefix or whatever.
> 
> It would be very wrong to install udev rules into
> /home/tanu/foo/udev/rules.d or similar folder as they would have
> absolutely zero effect on the system.

I don't see why that would be "very wrong". It's the least-surprising
place, and can't cause any harm. That said, I agree that a big fat
warning would be a good idea if udevrulesdir is anything else
than /lib/udev/rules.d or if systemduserunitdir is anything else than
the value given by pkg-config.

> > Similarly, the default behaviour (i.e. no parameters to configure)
> > should be a safe install - no overwriting of files managed by the
> > packaging system.
> 
> Again I do somewhat agree here, but only slightly pragmatically due to
> the above problem!
> 
> I think to handle this really nicely we need some sort of intelligence
> and awareness of --prefix where /usr and /usr/local are handled somewhat
> specially.
> 
> If --prefix is /usr then all is well;
> 
> If --prefix is /usr/local then user systemd user unit paths should be
> mangled accordingly and udev rules installation should be completely
> disabled and a big warning produced (udev rules cannot live in
> /usr/local as this may be a separate partition and mounted after udev
> has started).

What do you mean by "user units should be mangled"? Mangled how?

As for handling udev rules installation, I think a warning is enough if
udevrulesdir is anything else than /lib/udev/rules.d. I'm not strongly
opposing disabling the installation altogether, but spending extra
effort on that seems rather pointless.

> If --prefix is begins $HOME then systemd user unit paths could be
> mangled to $XDG_CONFIG_HOME/systemd/user/ or
> $HOME/.local/share/systemd/user/ and a warning produced to inform the
> user. As above udev rule installation should be totally disabled.

I'd prefer to not do this kind of magic by default. We could have a
special --install-to-home option that would do its best to make
everything work for home installations, but I'm not going to work on
that.

> >> Personally I'd rather leave it as it is, and just suggest to people to
> >> use $XDG_CONFIG_HOME/systemd/user/ when doing their own builds as their
> >> own user or use /usr/local/systemd/system/user/ when using a /usr/local
> >> prefix. If logic around this could be made available in e.g. an
> >> appropriate autotools include that lots of packages could use then
> >> that's fine by me. I just don't like the idea of every project doing
> >> something homegrown and different; systemd is providing so much benefit
> >> from standardising things I'd hate to see that undone with small
> >> convenience hacks littered around everywhere (even if they are sensible
> >> enough in principle).
> > 
> > If the "convenience hacks" are sensible enough in principle (not
> > actually hacks at all), wouldn't it be good to make those "hacks" the
> > standard? I don't think it makes sense for any software to overwrite the
> > distro's unit files by default when running "make install".
> 
> I don't fully disagree, and that's why I suggested that if this kind of
> logic is to be accepted it should be done so in an appropriate autotools
> include that can be shared, not in some magic that only lives in PA's
> configure.ac. This autotools stuff should be included upstream in
> systemd if it's going to be done at all.

What kind of autotools logic do you have in mind? As far as I can see,
we only need to add a check that prints a warning if the udev and
systemd directories are something that will not have effect with normal
systemd installations.

> >> If it is fixed for this tho', then it should also be fixed for the udev
> >> rules.
> > 
> > The udev rules case is different, because udev doesn't support rules
> > under /usr/local. I agree that the it would be good to get the udev rule
> > installation fixed, but I don't see why it should be tied to fixing the
> > unit dir case.
> 
> It's the same problem. With certain values of --prefix, certain other
> paths no longer make sense. A --prefix of /usr/local might work with
> systemd user units, but they don't work for systemd system units (these
> are parsed before /usr/local might be mounted), so if we ever ship
> those, we'll have to behave differently there (same as udev rules).

Ah, I didn't realize that problem with system units. I'd prefer handling
user and system units in the same way, so I now agree with you: it's
worth a warning if any units are going to be installed under /usr/local,
even if it might actually work with user units. As I wrote above, I
don't like disabling the installation, however. A warning is enough.

> I'm honestly surprised that it's even supported upstream for user units
> and I'm tempted to try and push for it to be removed upstream unless
> there is a *really* compelling reason to keep it.

Getting clarification about the reason behind supporting units
under /usr/local would definitely be a good thing.

> So yeah, I don't see it as any different. /usr is a special prefix for
> udev, /usr and /usr/local are special prefixes for systemd user units, a
> prefix starting with $HOME could be special for systemd user units, but
> honestly, I really think you're trying to be far to clever here to
> support a use case that maybe five people in the next ten years will
> actually genuinely benefit from.
> 
> Documenting it would be the better approach IMO.
> 
> 
> 
> > As a sidenote, here are my thoughts how the udev rules case should be
> > fixed: either fix udev so that it can load rules from /usr/local or drop
> > the udev rules altogether. I'd prefer the latter.
> 
> The latter is the *only* option. udev rules fundamentally cannot be read
> from /usr/local as it may be mounted way after udev starts.

Ok, good point.

> > The udev rules are
> > only used to attach some information to the devices based on the device
> > properties, but the same device properties are available through libudev
> > too, so we could implement the rules inside PulseAudio. That would have
> > the benefit that you could install updated rules under your home
> > directory, as opposed to the current situation that requires root access
> > to update PulseAudio's udev rules.
> 
> Well, yeah, that's a posibility, but it would seem counter intuitive to
> me to do this internally just to work around a very minor inconvenience
> to a very select few individual who compile up PA from source.
> 
> I honestly think you're spending waaaay to much effort thinking about
> this when some documentation and perhaps a simple ban for prefix != /usr
> for rules and units + a fat configure warning would do effectively the
> same thing while informing devs better "how things work" and not leaving
> left over files littered around the place that could break their systems
> when reverting back to distro provided packages.

It sounds like you think I'm proposing some very complicated thing. I
just want to avoid overwriting system files by default, which means that
if the prefix is /usr/local, everything should be installed
under /usr/local. Some non-critical things don't work in that case, so
your idea of printing a warning during configure is good.

-- 
Tanu