Re: [PATCH] systemd/*: Use @SBINDIR@.

[Greg Troxel]

Ladislav Michl <ladis@linux-mips.org> writes:

>> I'm really not sure what is going on here.  It is a core principle that
>> "make install", however spelled, respects destdir and does not try to
>> write outside of it.
>
> Yes, "make install" respects DESTDIR, but here we want to use @SBINDIR@ the
> same way @LIBDIR@ is used - to contain a path on target system.
>
>> The commit needs a clear explanation of the problem, what the solution
>> is, how it works, and why it won't break anybody else.  Yes, I can sort
>> of back that out from the diff, but not quite.
>> 
>> If you are just trying to add a variable SBINDIR so you can use it, then
>> I don't see why the destdir comment applies; destdir is used to install,
>> but isn't part of the directory paths.
>
> And that's exactly why add_destdir is set to False.

This really does not make sense, as it would cause almost every use of a
substituted path variable to be wrong.

> Without second argument add_destdir defaults to True as current SConstruct
> carries this implementation:
> def installdir(idir, add_destdir=True):
>     # use os.path.join to handle absolute paths properly.
>     wrapped = os.path.join(env['prefix'], env[idir])
>     if add_destdir:
>         wrapped = os.path.normpath(DESTDIR + os.path.sep + wrapped)
>     wrapped.replace("/usr/etc", "/etc")
>     wrapped.replace("/usr/lib/systemd", "/lib/systemd")
>     return wrapped
>
> So yes, @SBINDIR@ would end with DESTDIR prepended. Not a bug, it just
> depends how installdir is used.

I believe you but this doesn't make sense.

This seems like a major conceptual problem in our SConstruct.
Basically, when scons does an install it should use destdir (always) and
when a path is substituted it should not (always).  Maybe that's not
100% right, but it seems obvious.

>> I don't use systemd but this seems ok.  I am surprised that it was ever
>> working before as my understanding is that GNU/Linux norms are to put
>> everything in /usr.
>
> Well, gpsd default is /usr/local prefix and...

Sure, default for everything when hand built, using ancient autoconf
norms, is /usr/local.  But on GNU/Linux, packaging systems put things in
/usr.

>> Do systemd-using GNU/Linux packaging systems patch this file, because
>> they put gpsd in /usr vs /usr/local?  That would be nice to have in the
>> commit message too.
>
> ...package maintainers often do things at their own, so it worked.
> I just wanted to make things running out of the box for whatever
> prefix.
>
> In theory, we could also do something like this:
>
> diff --git a/packaging/deb/etc_init.d_gpsd.in 
> b/packaging/deb/etc_init.d_gpsd.in
> index dcba68c93..6d6e27139 100644
> --- a/packaging/deb/etc_init.d_gpsd.in
> +++ b/packaging/deb/etc_init.d_gpsd.in
> @@ -26,7 +26,7 @@
>  PATH=/sbin:/usr/sbin:/bin:/usr/bin
>  DESC="GPS (Global Positioning System) daemon"
>  NAME=gpsd
> -DAEMON=/usr/sbin/$NAME
> +DAEMON=@SBINDIR@/$NAME
>  PIDFILE=@RUNDIR@/$NAME.pid
>  SCRIPTNAME=/etc/init.d/$NAME
>
> But I do not think it is worth doing as distros have paths set in stone.
> Bernd?
>

>> I'm scared of touching SConstruct this close to a release, too.
>
> This is not anything important and can wait easily. I'm also using
> my own service files ;-)

Great to hear.  I am much less worried post release.