[PATCH] configure.ac: Use libsystemd in REQUIRED_LIBS check

[Emil Velikov]

On 16 November 2015 at 16:20, Jussi Kukkonen <jussi.kukkonen at intel.com> wrote:
> On 16 November 2015 at 17:26, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>
>> Hi Jussi,
>>
>> I'm not as experienced as other devs here so take all of the following
>> with a pinch of salt.
>>
>> On 16 November 2015 at 14:46, Jussi Kukkonen <jussi.kukkonen at intel.com>
>> wrote:
>> > REQUIRED_LIBS needs to be set to the correct systemd library,
>> > otherwise the later check will either fail or use the wrong
>> > pc file.
>> >
>> > Signed-off-by: Jussi Kukkonen <jussi.kukkonen at intel.com>
>> > ---
>> >  configure.ac | 13 ++++++++++---
>> >  1 file changed, 10 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index 96c0242..f63eca1 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -837,9 +837,16 @@ AC_ARG_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],
>> > +                  [REQUIRED_SYSTEMD_DAEMON=libsystemd-daemon],
>> >                    [PKG_CHECK_MODULES([SYSTEMD_DAEMON], [libsystemd],
>> > -                                     [HAVE_SYSTEMD_DAEMON=yes],
>> > [HAVE_SYSTEMD_DAEMON=no])])
>> > +
>> > [REQUIRED_SYSTEMD_DAEMON=libsystemd],
>> > +                                     [REQUIRED_SYSTEMD_DAEMON=])])
>> > +if test "x$REQUIRED_SYSTEMD_DAEMON" = x; then
>> > +        HAVE_SYSTEMD_DAEMON=no
>> > +else
>> > +        HAVE_SYSTEMD_DAEMON=yes
>> > +fi
>> > +
>> >  if test "x$WITH_SYSTEMD_DAEMON" = xauto; then
>> >         WITH_SYSTEMD_DAEMON="$HAVE_SYSTEMD_DAEMON"
>> >  fi
>> > @@ -848,7 +855,7 @@ if test "x$WITH_SYSTEMD_DAEMON" = xyes; 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])
>> > -       REQUIRED_LIBS="$REQUIRED_LIBS libsystemd-daemon"
>> > +       REQUIRED_LIBS="$REQUIRED_LIBS $REQUIRED_SYSTEMD_DAEMON"
>> Rather than flipping things into a different (and somewhat obscure)
>> way, why don't you just use SYSTEMD_DAEMON_LIBS instead of the
>> hardcoded libsystemd-daemon ?
>>
>> Shorter diff, consistent checks and it should work (although I haven't
>> checked it).
>
>
> The way it's done is indeed a bit obscure (might just be my limited
> experience with xorg build system though). My impression is this:
> Individual dependencies are checked one by one, but libs/cflags are not set
> at this point. Later in the file there's two PKG_CHECK_MODULES calls that
> set $XSERVERCFLAGS and $XSERVERLIBS for all $REQUIRED_LIBS.
>
> I didn't want to change that for systemd alone as I don't know the reasons
> for this design -- but I can do that if that's preferred.
>
Actually the whole thing is a bit different than what I originally
thought. Despite the "_LIBS" name, the variable does not refer to the
soname, but the pkg-config module name - heh wording :-P

So the correct fix here is to create another variable for the module
name and use it. Something vaguely like
PKG_CHECK_MODULES(foo, systemd1,
  [have_foo=yes,LIBSYSTEMD_DAEMON=systemd1],
  PKG_CHECK_MODULES(foo, systemd2,
    [have_foo=yes,LIBSYSTEMD_DAEMON=systemd2])
)

Ignore the weird formatting :-)

We might also even fold XSERVERCFLAGS and XSERVERLIBS, as both should
yield the same _CFLAGS/_LIBS variables.

-Emil