<div dir="ltr">Hi Lukas,<div class="gmail_extra"><br><br><div class="gmail_quote">2013/10/15 Lukas Slebodnik <span dir="ltr">&lt;<a href="mailto:lslebodn@redhat.com" target="_blank">lslebodn@redhat.com</a>&gt;</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>
&gt;Hi,<br>
&gt;<br>
&gt;These two patches add missing CFLAGS/LIBS to Makefile.am:<br>
&gt;<br>
&gt;[PATCH 1/2] BUILD: Link libsss_ad.so to sasl libs<br>
&gt;[PATCH 2/2] BUILD: Use OPENLDAP_CFLAGS instead of LDAP_CFLAGS<br>
</div>ACK to 2nd patch<br>
<div class="im">&gt;<br>
&gt;This underlinking was noticed in make check (dlopen-test).<br>
&gt;<br>
&gt;Note:<br>
&gt;It failed for me since my openldap build had no sasl support,<br>
&gt;which would otherwise have pulled in libsasl2.so.<br>
&gt;Of course, that support should be in place, but the linking should still be<br>
&gt;fixed.<br>
&gt;<br>
&gt;BTW: It would propably be nice to have a configure check whether<br>
&gt;openldap has sasl support, but it seems that would need a check if<br>
&gt;ldap_sasl_interactive_bind returns LDAP_NOT_SUPPORTED.<br>
&gt;<br>
&gt;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 &lt;<a href="mailto:benjaminfranzke@googlemail.com">benjaminfranzke@googlemail.com</a>&gt;<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&#39;t be available one some distribution (platforms).<br>
For example popt-devel on fedora 19 doesn&#39;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 &quot;OpenLDAP LDAP (libldap, -lldap)&quot; 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&#39;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&#39;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>