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 <[email protected]> >> >>> 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 <[email protected]> >>> 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 <[email protected]> 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 diff --git a/Makefile.am b/Makefile.am index e9bed47e3454867dadf7a2afdf90513c6bd0a796..0d795971f547e92740ca072942c1874d594035bd 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1760,12 +1760,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 9934b50b90591cc46867fbb17a84a9aada2cd25e..93038204fad6f2f52bc741cfcc3aebc9bdecb988 100644 --- a/configure.ac +++ b/configure.ac @@ -158,6 +158,7 @@ m4_include([src/external/cifsidmap.m4]) m4_include([src/external/signal.m4]) m4_include([src/external/inotify.m4]) m4_include([src/external/libndr_nbt.m4]) +m4_include([src/external/sasl.m4]) WITH_UNICODE_LIB if test x$unicode_lib = xlibunistring; then @@ -262,8 +263,6 @@ fi AM_CHECK_INOTIFY -AC_CHECK_HEADERS([sasl/sasl.h],,AC_MSG_ERROR([Could not find SASL headers])) - AC_CACHE_CHECK([whether compiler supports __attribute__((destructor))], sss_client_cv_attribute_destructor, [AC_COMPILE_IFELSE( diff --git a/src/external/sasl.m4 b/src/external/sasl.m4 new file mode 100644 index 0000000000000000000000000000000000000000..5980624af359557575c93032b470653b84c74c55 --- /dev/null +++ b/src/external/sasl.m4 @@ -0,0 +1,17 @@ +SASL_OBJ="" +AC_SUBST(SASL_OBJ) +AC_SUBST(SASL_LIBS) +AC_SUBST(SASL_CFLAGS) + +PKG_CHECK_MODULES([SASL], [libsasl2], [found_sasl=yes], [found_sasl=no]) + +SSS_AC_EXPAND_LIB_DIR() +AS_IF([test x"$found_sasl" != xyes], + [AC_CHECK_HEADERS([sasl/sasl.h], + [AC_CHECK_LIB([sasl2], + [sasl_client_init], + [SASL_LIBS="-L$sss_extra_libdir -lsasl2"], + [AC_MSG_ERROR([SASL library must support sasl_client_init])], + [-L$sss_extra_libdir])], + [AC_MSG_ERROR([SASL header files are not installed])])] +) -- 1.8.3.1
>From 982ff9ce7d2bfb4a2276ff376018bc12120652dd Mon Sep 17 00:00:00 2001 From: Benjamin Franzke <[email protected]> Date: Tue, 15 Oct 2013 10:35:28 +0200 Subject: [PATCH 2/2] BUILD: Use OPENLDAP_CFLAGS instead of LDAP_CFLAGS LDAP_CFLAGS is never defined. OPENLDAP_CFLAGS is set by src/external/ldap.m4. This patch does: sed -i 's/$(LDAP_CFLAGS)/$(OPENLDAP_CFLAGS)/' Makefile.am --- Makefile.am | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index 0d795971f547e92740ca072942c1874d594035bd..b9705118dd0f624aa3c62863474769afdf91cc09 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1607,7 +1607,7 @@ libsss_ldap_la_SOURCES = \ libsss_ldap_la_CFLAGS = \ $(AM_CFLAGS) \ $(SYSTEMD_LOGIN_CFLAGS) \ - $(LDAP_CFLAGS) \ + $(OPENLDAP_CFLAGS) \ $(KRB5_CFLAGS) libsss_ldap_la_LIBADD = \ $(OPENLDAP_LIBS) \ @@ -1703,7 +1703,7 @@ libsss_ipa_la_SOURCES = \ libsss_ipa_la_CFLAGS = \ $(AM_CFLAGS) \ $(SYSTEMD_LOGIN_CFLAGS) \ - $(LDAP_CFLAGS) \ + $(OPENLDAP_CFLAGS) \ $(DHASH_CFLAGS) \ $(NDR_NBT_CFLAGS) \ $(KRB5_CFLAGS) @@ -1759,7 +1759,7 @@ libsss_ad_la_SOURCES = \ libsss_ad_la_CFLAGS = \ $(AM_CFLAGS) \ $(SYSTEMD_LOGIN_CFLAGS) \ - $(LDAP_CFLAGS) \ + $(OPENLDAP_CFLAGS) \ $(SASL_CFLAGS) \ $(DHASH_CFLAGS) \ $(KRB5_CFLAGS) \ -- 1.8.3.1
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
