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

[Łukasz Stelmach]

It was <2013-11-30 sob 01:42>, when Peter Hutterer wrote:
> On 29/11/2013 19:36 , Ł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
>>
>> 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.
>
> Hans has already answered this, but nonetheless I recommend: never
> ever write patches to accommodate for a local custom setup. That may
> be acceptable if you plan to ship the local setup as a fixed image,
> but otherwise you're just spending time better spent elsewhere
> (specifically, fixing the cause of the bug :)

+1 to my experience points ;)

> Doubly so for a system (pkgconfig) which has the sole purpose of
> making such custom local configurations invisible to the user of
> pkgconfig.
>
> In your case as simple prefix change, make install into somewhere and
> adjusting PKG_CONFIG_PATH should've fixed the issue.


Who should rework the patch?

>>
>> Once again, please make sure you take the v4 patches [2][3][4].
>>
>>>> +	LIBS=$SAVE_LIBS
>>>> +	REQUIRED_LIBS="$REQUIRED_LIBS libsystemd-daemon"
>>>
>>> this should also check for the matching version of libxtrans (which would
>>> have to be released first, of course).
>>>
>>> Cheers,
>>>     Peter
>>>
>>>> +fi
>>>> +
>>>>   if test "x$CONFIG_UDEV" = xyes &&
>>>>    { test "x$CONFIG_DBUS_API" = xyes || test "x$CONFIG_HAL" = xyes; }; then
>>>>   	AC_MSG_ERROR([Hotplugging through both libudev and dbus/hal not allowed])
>>>> --
>>>> 1.8.4.2
>>
>> [1] http://thread.gmane.org/gmane.comp.freedesktop.xorg.devel/36092/focus=37696
>> [2] http://thread.gmane.org/gmane.comp.freedesktop.xorg.devel/36092/focus=37693
>> [3] http://lists.x.org/archives/xorg-devel/2013-October/038276.html
>> [4] curl -s
>> http://lists.x.org/archives/xorg-devel/2013-October.txt.gz | gzip
>> -dc | sed -n -e 23359,23647\ \{ -e 's/?ukasz/Łukasz/' -e p -e \}
>>
>
>

-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20131202/7a8f8c4a/attachment.pgp>