[SSSD] Build Fixes

Lukas Slebodnik lslebodn at redhat.com
Thu Oct 31 10:12:28 UTC 2013


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



More information about the sssd-devel mailing list