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

[Emil Velikov]

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 ?

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

Thanks
Emil