<div dir="ltr">Hi Lukas,<div class="gmail_extra"><br><br><div class="gmail_quote">2013/10/15 Lukas Slebodnik <span dir="ltr"><<a href="mailto:lslebodn@redhat.com" target="_blank">lslebodn@redhat.com</a>></span><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">On (15/10/13 11:47), Benjamin Franzke wrote:<br>
>Hi,<br>
><br>
>These two patches add missing CFLAGS/LIBS to Makefile.am:<br>
><br>
>[PATCH 1/2] BUILD: Link libsss_ad.so to sasl libs<br>
>[PATCH 2/2] BUILD: Use OPENLDAP_CFLAGS instead of LDAP_CFLAGS<br>
</div>ACK to 2nd patch<br>
<div class="im">><br>
>This underlinking was noticed in make check (dlopen-test).<br>
><br>
>Note:<br>
>It failed for me since my openldap build had no sasl support,<br>
>which would otherwise have pulled in libsasl2.so.<br>
>Of course, that support should be in place, but the linking should still be<br>
>fixed.<br>
><br>
>BTW: It would propably be nice to have a configure check whether<br>
>openldap has sasl support, but it seems that would need a check if<br>
>ldap_sasl_interactive_bind returns LDAP_NOT_SUPPORTED.<br>
><br>
>Regards, Ben<br>
<br>
</div>I have a little problem with the first patch.<br>
<br>
<br>
>From d26a15de098df2b42582cda590e184c69f48bb7f Mon Sep 17 00:00:00 2001<br>
From: Benjamin Franzke <<a href="mailto:benjaminfranzke@googlemail.com">benjaminfranzke@googlemail.com</a>><br>
Date: Tue, 15 Oct 2013 10:27:36 +0200<br>
Subject: [PATCH 1/2] BUILD: Link libsss_ad.so to sasl libs<br>
<br>
This is for the sasl_client_init symbol.<br>
Introducted in commit fb945a2c.<br>
---<br>
Makefile.am | 2 ++<br>
<a href="http://configure.ac" target="_blank">configure.ac</a> | 2 +-<br>
2 files changed, 3 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/Makefile.am b/Makefile.am<br>
index ff1e71e72d90a6eff658a40e2ca24c1929b31aa5..8c919d40481720e4661a42a188cea2fd179282d4 100644<br>
--- a/Makefile.am<br>
+++ b/Makefile.am<br>
@@ -1713,12 +1713,14 @@ libsss_ad_la_CFLAGS = \<br>
$(AM_CFLAGS) \<br>
$(SYSTEMD_LOGIN_CFLAGS) \<br>
$(LDAP_CFLAGS) \<br>
+ $(SASL_CFLAGS) \<br>
$(DHASH_CFLAGS) \<br>
$(KRB5_CFLAGS) \<br>
$(NDR_NBT_CFLAGS)<br>
libsss_ad_la_LIBADD = \<br>
$(SYSTEMD_LOGIN_LIBS) \<br>
$(OPENLDAP_LIBS) \<br>
+ $(SASL_LIBS) \<br>
$(DHASH_LIBS) \<br>
$(KEYUTILS_LIBS) \<br>
$(KRB5_LIBS) \<br>
diff --git a/<a href="http://configure.ac" target="_blank">configure.ac</a> b/<a href="http://configure.ac" target="_blank">configure.ac</a><br>
index d28d55f3b0eb35cc4ecc02ae9b1fafbcb9588dcf..7e3819a86ad94176e95f1ded07b88d25399888de 100644<br>
--- a/<a href="http://configure.ac" target="_blank">configure.ac</a><br>
+++ b/<a href="http://configure.ac" target="_blank">configure.ac</a><br>
@@ -260,7 +260,7 @@ fi<br>
<br>
AM_CHECK_INOTIFY<br>
<br>
-AC_CHECK_HEADERS([sasl/sasl.h],,AC_MSG_ERROR([Could not find SASL headers]))<br>
+PKG_CHECK_MODULES([SASL], [libsasl2], [], [AC_MSG_ERROR([Could not find SASL library])])<br>
<br>
pkg-config file needn't be available one some distribution (platforms).<br>
For example popt-devel on fedora 19 doesn't have pkg-config file.<br>
So it is better to fallback to AC_CHECK_HEADER.<br>
<br>
1. And it does not mean that openldap was build with sasl support if libsasl2<br>
is installed on the machine. Detection should more complex.<br></blockquote><div><br></div><div>Yes, thats why the dea to detect whether ldap has sasl support, which is not easy, as Stephen already said.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
2. According to man ldap_sasl_interactive_bind_s, we should only link with<br>
library "OpenLDAP LDAP (libldap, -lldap)" and we does not directly use<br>
any sasl function. What kind of distribution do you use?<br></blockquote><div><br></div><div>As written in the commit message, since commit fb945a2c sssd uses sasl unconditionally:</div><div><div>git grep sasl_client_init</div>
<div>src/providers/ad/ad_init.c: (void)sasl_client_init(ad_sasl_callbacks);</div></div><div><br></div><div>So if sssd should be build without sasl that'd need to be fixed.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
3. IIRC, sssd can be build with ldap without sasl support, but ipa provider<br>
will not work. And it should be similar with ad provider. I would prefer<br>
to use conditional build instead of failing if ldap does not support sasl.<br>
Because with your patch, sssd could not be build without sasl library and<br>
someone can use sssd only with ldap provider (and he doesn't care about<br>
sasl)<br></blockquote><div><br></div><div>How did it not fail before, when sasl was not installed? I mean there was error too..</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><font color="#888888"><br>
LS<br>
</font></span><div class=""><div class="h5">_______________________________________________<br>
sssd-devel mailing list<br>
<a href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a><br>
<a href="https://lists.fedorahosted.org/mailman/listinfo/sssd-devel" target="_blank">https://lists.fedorahosted.org/mailman/listinfo/sssd-devel</a><br>
</div></div></blockquote></div><br></div></div>