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

[Ladislav Michl]

On Mon, Aug 03, 2020 at 07:34:54AM -0400, Greg Troxel wrote:
> Ladislav Michl <ladis@linux-mips.org> writes:
[snip]
> > 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.

That "not 100% right" is a reason patch is not touching existing things.

> >> 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.

(above sentence continues with "..." bellow)
Now I need to know if you agree patch is safe, so I can prepare v2 with
more detailed commit message. Otherwise I'll resend later after release.

> >> 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.

Please note that @SBINDIR@ is newly introduced, so it cannot break
anything. Anyway, final decision is not up to me :)

Best regards,
        ladis