[PATCH] configure.ac: enable systemd socket activation in libxtrans

[Hans de Goede]

Hi,

On 11/29/2013 10:36 AM, Łukasz Stelmach wrote:
> It was <2013-11-29 pią 06:00>, when Peter Hutterer wrote:
>> On Thu, Nov 28, 2013 at 04:23:07PM +0100, Hans de Goede wrote:
>>> From: Łukasz Stelmach <l.stelmach at samsung.com>
>>>
>>> Signed-off-by: Łukasz Stelmach <l.stelmach at samsung.com>
>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>> ---
>>>   configure.ac | 28 ++++++++++++++++++++++++++++
>>>   1 file changed, 28 insertions(+)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 6c4a609..ab246d6 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -832,6 +832,34 @@ AC_SUBST(SDK_REQUIRED_MODULES)
>>>
>>>   REQUIRED_MODULES="$FIXESPROTO $DAMAGEPROTO $XCMISCPROTO $XTRANS
>>> $BIGREQSPROTO $SDK_REQUIRED_MODULES"
>>>
>>> +#
>>> +# systemd socket activation
>>> +#
>>> +# activate the code in libxtrans that grabs sockets' file-descriptors
>>> +# instead of creating them.
>>> +#
>>> +AC_ARG_WITH([systemd],
>>> +	AS_HELP_STRING([--with-systemd], [support systemd socket activation]),
>>> +	[], [with_systemd=check])
>>> +have_systemd=check
>>> +if test "x$with_systemd" != "xno"; then
>>> +	PKG_CHECK_MODULES([systemd], [libsystemd-daemon],
>>> +		[AC_DEFINE(HAVE_SYSTEMD, 1, [Define if libsystemd-daemon is available])
>>> +		have_systemd=yes;],
>>> +		[have_systemd=no])
>>> +	if test "x$with_systemd" = "xyes" -a "x$have_systemd" = "xno"; then
>>> +		AC_MSG_ERROR([systemd support requested but no library has been found])
>>> +	fi
>>> +fi
>>> +AM_CONDITIONAL([HAVE_SYSTEMD], [test "x$have_systemd" = "xyes"])
>>> +if test "x$have_systemd" = "xyes"; then
>>
>> while the above is fine, I'd prefer if base logic of the check is the same
>> as all the other checks for misc "auto" stuff. which seems to be
>>
>> PKG_CHECK_MODULES(foo, foo, [have_foo=yes], [have_foo=no])
>> if test "x$foo" = xauto; then
>>     foo=$have_foo
>> fi
>> if test "x$foo" = xyes; then
>>       if "xhave_foo" = xno; then
>>          error
>>       fi
>>       AC_DEFINE(...)
>>       other stuff
>> fi
>> AM_CONDITIONAL()
>>
>> there are multiple versions of this floating around in the configure file
>> but the majority seems to go about it this way. it also helps re-using the
>> have_systemd if it's needed elsewhere in the future.
>>
>>
>>> +	SAVE_LIBS=$LIBS
>>> +	LIBS="$systemd_LIBS"
>>> + AC_CHECK_FUNCS([sd_listen_fds], [], [AC_MSG_ERROR([sd_listen_fds()
>>> is missing from libsystemd-daemon])])
>>
>> can this even happen? quick check of the systemd code doesn't seem like it's
>> easily disabled. and the history of it is back long enough that we don't
>> need to require a specific version
>
>
> This is not the latest version of the patch. The latest version[1]
> has the message changed to
>
>      sd_listen_fds() is missing or libsystemd-daemon is not available

Yes I noticed that and deliberately did not pick up that change, because
pkg-config has already found libsystemd-daemon so it is a weird error to
give.

> It may happen if there is something wrong with paths and for some reason
> the library is not available to the linker. Personally I encountered
> this when I was developing the patches with the systemd libraries not
> installed in the system but rather laying in the source directory.

We also rely on pkg-config provided libs info on a lot of other places
without verifying it. If a system is broken to such a degree that pkg-config
and the actual installed libs are a mismatch then that is simply something
which is broken and we should not get out of our way to support broken
systems, or to provide error messages tweaked for broken systems, trying to
support such systems will just be a never-ending stream of problems.

Besides that the new message is less clear for users who do have
libsystemd-daemon, but a too old version, which is why I stuck with
the old version of the error message.

Regards,

Hans