[SSSD] Build Fixes

Lukas Slebodnik lslebodn at redhat.com
Sat May 17 17:49:50 UTC 2014


On (07/11/13 20:43), Lukas Slebodnik wrote:
>On (31/10/13 11:12), Lukas Slebodnik wrote:
>>On (15/10/13 15:42), Benjamin Franzke wrote:
>>>Hi Lukas,
>>>
>>>
>>>2013/10/15 Lukas Slebodnik <lslebodn at redhat.com>
>>>
>>>> On (15/10/13 11:47), Benjamin Franzke wrote:
>>>> >Hi,
>>>> >
>>>> >These two patches add missing CFLAGS/LIBS to Makefile.am:
>>>> >
>>>> >[PATCH 1/2] BUILD: Link libsss_ad.so to sasl libs
>>>> >[PATCH 2/2] BUILD: Use OPENLDAP_CFLAGS instead of LDAP_CFLAGS
>>>> ACK to 2nd patch
>>>> >
>>>> >This underlinking was noticed in make check (dlopen-test).
>>>> >
>>>> >Note:
>>>> >It failed for me since my openldap build had no sasl support,
>>>> >which would otherwise have pulled in libsasl2.so.
>>>> >Of course, that support should be in place, but the linking should still
>>>> be
>>>> >fixed.
>>>> >
>>>> >BTW: It would propably be nice to have a configure check whether
>>>> >openldap has sasl support, but it seems that would need a check if
>>>> >ldap_sasl_interactive_bind returns LDAP_NOT_SUPPORTED.
>>>> >
>>>> >Regards, Ben
>>>>
>>>> I have a little problem with the first patch.
>>>>
>>>>
>>>> From d26a15de098df2b42582cda590e184c69f48bb7f Mon Sep 17 00:00:00 2001
>>>> From: Benjamin Franzke <benjaminfranzke at googlemail.com>
>>>> Date: Tue, 15 Oct 2013 10:27:36 +0200
>>>> Subject: [PATCH 1/2] BUILD: Link libsss_ad.so to sasl libs
>>>>
>>>> This is for the sasl_client_init symbol.
>>>> Introducted in commit fb945a2c.
>>>> ---
>>>>  Makefile.am  | 2 ++
>>>>  configure.ac | 2 +-
>>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Makefile.am b/Makefile.am
>>>> index
>>>> ff1e71e72d90a6eff658a40e2ca24c1929b31aa5..8c919d40481720e4661a42a188cea2fd179282d4
>>>> 100644
>>>> --- a/Makefile.am
>>>> +++ b/Makefile.am
>>>> @@ -1713,12 +1713,14 @@ libsss_ad_la_CFLAGS = \
>>>>      $(AM_CFLAGS) \
>>>>      $(SYSTEMD_LOGIN_CFLAGS) \
>>>>      $(LDAP_CFLAGS) \
>>>> +    $(SASL_CFLAGS) \
>>>>      $(DHASH_CFLAGS) \
>>>>      $(KRB5_CFLAGS) \
>>>>      $(NDR_NBT_CFLAGS)
>>>>  libsss_ad_la_LIBADD = \
>>>>      $(SYSTEMD_LOGIN_LIBS) \
>>>>      $(OPENLDAP_LIBS) \
>>>> +    $(SASL_LIBS) \
>>>>      $(DHASH_LIBS) \
>>>>      $(KEYUTILS_LIBS) \
>>>>      $(KRB5_LIBS) \
>>>> diff --git a/configure.ac b/configure.ac
>>>> index
>>>> d28d55f3b0eb35cc4ecc02ae9b1fafbcb9588dcf..7e3819a86ad94176e95f1ded07b88d25399888de
>>>> 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -260,7 +260,7 @@ fi
>>>>
>>>>  AM_CHECK_INOTIFY
>>>>
>>>> -AC_CHECK_HEADERS([sasl/sasl.h],,AC_MSG_ERROR([Could not find SASL
>>>> headers]))
>>>> +PKG_CHECK_MODULES([SASL], [libsasl2], [], [AC_MSG_ERROR([Could not find
>>>> SASL library])])
>>>>
>>>> pkg-config file needn't be available one some distribution (platforms).
>>>> For example popt-devel on fedora 19 doesn't have pkg-config file.
>>>> So it is better to fallback to AC_CHECK_HEADER.
>>>>
>>>> 1. And it does not mean that openldap was build with sasl support if
>>>> libsasl2
>>>>    is installed on the machine. Detection should more complex.
>>>>
>>>
>>>Yes,  thats why the dea to detect whether ldap has sasl support, which is
>>>not easy, as Stephen already said.
>>>
>>>>
>>>> 2. According to man ldap_sasl_interactive_bind_s, we should only link with
>>>>    library "OpenLDAP LDAP (libldap, -lldap)" and we does not directly use
>>>>    any sasl function. What kind of distribution do you use?
>>>>
>>>
>>>As written in the commit message, since commit  fb945a2c sssd uses sasl
>>>unconditionally:
>>>git grep sasl_client_init
>>>src/providers/ad/ad_init.c:    (void)sasl_client_init(ad_sasl_callbacks);
>>>
>>>So if sssd should be build without sasl that'd need to be fixed.
>>>
>>>>
>>>> 3. IIRC, sssd can be build with ldap without sasl support, but ipa provider
>>>>    will not work. And it should be similar with ad provider. I would prefer
>>>>    to use conditional build instead of failing if ldap does not support
>>>> sasl.
>>>>    Because with your patch, sssd could not be build without sasl library
>>>> and
>>>>    someone can use sssd only with ldap provider (and he doesn't care about
>>>>    sasl)
>>>>
>>>
>>>How did it not fail before, when sasl was not installed? I mean there was
>>>error too..
>>>
>>I realized sssd cannot be built without header file sasl.h, because
>>we include header file in sdap_async_connection.c
>>   src/providers/ldap/sdap_async_connection.c:#include <sasl/sasl.h>
>>But we needn't link ldap_common with sasl library, because sasl functions
>>are not used in module sdap_async_connection.c
>>
>>The last problem is in your patch, that you replaced
>>macro AC_CHECK_HEADERS with pkg-config detection. Sasl pkg-config file
>>needn't be available on some distributions.
>>For example, the latest ubuntu does not have sasl.pc in the package
>>libsasl2-dev.
>>
>>http://packages.ubuntu.com/saucy/amd64/libsasl2-dev/filelist
>>
>>LS
>
>I am attaching patches with more robust detection of SASL,
>because pkg-config file is not available in older versions of
>cysrus sasl <= 2.1.25. (Fedora<=18, RHEL<=6, all versions of ubuntu)
>
>Configure failed with Benjamin's version on RHEL6.
>
>LS

>From 8b4746d7e2d5c53cbe5d29f1ce984a16aefb5a54 Mon Sep 17 00:00:00 2001
>From: Lukas Slebodnik <lslebodn at redhat.com>
>Date: Tue, 15 Oct 2013 10:27:36 +0200
>Subject: [PATCH 1/2] BUILD: Explicitly link libsss_ad.so with sasl libs
>
>If openldap is not built with sasl support
>libsss_ad.so will not be linked with libsasl2 although
>sasl_client_init is called by function ad_sasl_initialize.
>---
> Makefile.am          |  2 ++
> configure.ac         |  3 +--
> src/external/sasl.m4 | 17 +++++++++++++++++
> 3 files changed, 20 insertions(+), 2 deletions(-)
> create mode 100644 src/external/sasl.m4
>

Could someone push 1st patch also to the 1-11 branch?

LS



More information about the sssd-devel mailing list