Hi Lukas,

2013/10/15 Lukas Slebodnik <lslebodn@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@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..

LS

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel