On (15/10/13 15:42), Benjamin Franzke wrote: >Hi Lukas, > > >2013/10/15 Lukas Slebodnik <lsleb...@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 <benjaminfran...@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); ahh
I remember this patch, but I still think that we should not fail if sasl is not available. A lot of features are optional in sssd. For example cifs plugin :-) SASL in ad-provider can be the same situation. But it is nice catch and we should also update spec file, to explicitly require sasl in sssd-ad. > >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 checked mock-build with fedora 19 and normal build in ubuntu 13.04. There was not problem with any test. LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel