[PATCHv3] build: Fix systemd-daemon compile/linker flags

[Peter Hutterer]

On Mon, Nov 23, 2015 at 12:07:33AM +0000, Emil Velikov wrote:
> On 22 November 2015 at 23:37, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > On Thu, Nov 19, 2015 at 04:04:30PM +0200, Jussi Kukkonen wrote:
> >> * Dont add systemd to REQUIRED_LIBS (this lead to build failure when
> >>   libsystemd is available but libsystemd-daemon is not)
> >> * Only use systemd compile and linker flags where needed (libxtrans)
> >> * Try libsystemd (>= 210) first so it's used instead of the
> >>   libsystemd-daemon wrapper that might also exist
> >>
> >> Signed-off-by: Jussi Kukkonen <jussi.kukkonen at intel.com>
> >> ---
> >>
> >> Let's try once more. Tried to address Peters comments by limiting
> >> systemd flag use to libos.la. Also fixed the issue Emil pointed out
> >> by checking only for libsystemd versions that actually provide the
> >> required api.
> >>
> >> Note that the actual systemd-daemon api use is not in xserver, but
> >> in the xtrans "library" (at least that's the only one I could find).
> >
> > unfortunately, this one doesn't work when dtrace is enabled (it works fine
> > otherwise). libos.la is the only one where appending a library isn't easy
> > because we convert libos.la to a dtrace object os.O and link that in - and
> > that makes us lose the dependency. So maybe it's better to go down the
> > original path with the REQUIRED_LIBS after all, anything else is going to be
> > messier than a local hack. How about something like this:
> >
> > diff --git a/configure.ac b/configure.ac
> > index 14a5bb8..ecb4263 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -836,21 +836,21 @@ AC_ARG_WITH([systemd-daemon],
> >         AS_HELP_STRING([--with-systemd-daemon],
> >                 [support systemd socket activation (default: auto)]),
> >         [WITH_SYSTEMD_DAEMON=$withval], [WITH_SYSTEMD_DAEMON=auto])
> > -PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
> > -                  [HAVE_SYSTEMD_DAEMON=yes],
> > -                  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd],
> > -                                     [HAVE_SYSTEMD_DAEMON=yes], [HAVE_SYSTEMD_DAEMON=no])])
> > +PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd >= 210],
> > +                  [HAVE_SYSTEMD_DAEMON="libsystemd"],
> > +                  [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd-daemon],
> > +                                     [HAVE_SYSTEMD_DAEMON="libsystemd-daemon"], [HAVE_SYSTEMD_DAEMON=no])])
> >  if test "x$WITH_SYSTEMD_DAEMON" = xauto; then
> >         WITH_SYSTEMD_DAEMON="$HAVE_SYSTEMD_DAEMON"
> >  fi
> > -if test "x$WITH_SYSTEMD_DAEMON" = xyes; then
> > +if test "x$WITH_SYSTEMD_DAEMON" != xno; then
> >         if test "x$HAVE_SYSTEMD_DAEMON" = xno; then
> >                 AC_MSG_ERROR([systemd support requested but no library has been found])
> >         fi
> >         AC_DEFINE(HAVE_SYSTEMD_DAEMON, 1, [Define to 1 if libsystemd-daemon is available])
> Nitpick: Use the correct name in above comment ?

sounds good

> 
> > -       REQUIRED_LIBS="$REQUIRED_LIBS libsystemd-daemon"
> > +       REQUIRED_LIBS="$REQUIRED_LIBS $HAVE_SYSTEMD_DAEMON"
> >  fi
> > -AM_CONDITIONAL([HAVE_SYSTEMD_DAEMON], [test "x$HAVE_SYSTEMD_DAEMON" = "xyes"])
> > +AM_CONDITIONAL([HAVE_SYSTEMD_DAEMON], [test "x$HAVE_SYSTEMD_DAEMON" != "xno"])
> >
> >  if test "x$CONFIG_UDEV" = xyes && test "x$CONFIG_HAL" = xyes; then
> >         AC_MSG_ERROR([Hotplugging through both libudev and hal not allowed])
> >
> > best to rename it to "SYSTEMD_PKG" or so instead of HAVE_SYSTEMD_DAEMON, a
> > bit more testing and it should be fine?
> >
> Why are we using a single variable for "have" and "package name"
> (LIBSYSTEMD_DAEMON or alike in this case) ? Won't things be
> clearer/more consistent if they are kept separate ?

I think we can just use one variable as long as it's not actually called
HAVE_FOO and it may even be clearer than setting two variables. But give
either a try, this was just a quick hack to check if this works, I'll leave
the fine detail work to you ;)

Cheers,
   Peter