[PATCH] BUILD: Link libsss_krb5_common.so to libkeyutils.so

[Benjamin Franzke]

Hi List,

The symbol add_key is used by
src/providers/krb5/krb5_delayed_online_authentication.c
which is part of libsss_krb5_common.so

Fixes following error:
[sssd[be[default]]] [load_backend_module]
  (0x0010): Unable to load ad module with path
  (/usr/lib64/sssd/libsss_ad.so), error:
  /usr/lib64/sssd/libsss_krb5_common.so: undefined symbol: add_key

-lkeyutils was passed to the libraries libsss_{krb5,ipa,ad}.so,
but when compiling with -Wl,--as-needed this flag will be ignored,
since it is not used directly. So it was unavailable to
libsss_krb5_common.so which actually needs it.

This patch removes $(KEYUTILS_LIBS) from those libraries and adds it to
libsss_krb5_common.so

Maybe libsss_krb5_common.so should be added to dlopen-tests?
But then other libraries and functions are needed as well,
which it currently inherits from libsss_{krb5,ipa,ad}.so.

BTW: are these common libraries (i mean ldap too) convenience build
libraries, or to save disk space?
If they're just for convencience maybe they should not be installed?

Regards, Ben

[Simo Sorce]

On Wed, 2013-10-16 at 14:49 +0200, Benjamin Franzke wrote:
> Hi List,
>
> The symbol add_key is used by
> src/providers/krb5/krb5_delayed_online_authentication.c
> which is part of libsss_krb5_common.so
>
> Fixes following error:
> [sssd[be[default]]] [load_backend_module]
>   (0x0010): Unable to load ad module with path
>   (/usr/lib64/sssd/libsss_ad.so), error:
>   /usr/lib64/sssd/libsss_krb5_common.so: undefined symbol: add_key
>
> -lkeyutils was passed to the libraries libsss_{krb5,ipa,ad}.so,
> but when compiling with -Wl,--as-needed this flag will be ignored,
> since it is not used directly. So it was unavailable to
> libsss_krb5_common.so which actually needs it.
>
> This patch removes $(KEYUTILS_LIBS) from those libraries and adds it
> to
> libsss_krb5_common.so
>
> Maybe libsss_krb5_common.so should be added to dlopen-tests?

Yes please.
> But then other libraries and functions are needed as well,
> which it currently inherits from libsss_{krb5,ipa,ad}.so.

You can just open a chain of libraries.
> BTW: are these common libraries (i mean ldap too) convenience build
> libraries, or to save disk space?

Not just to save disk space but to avoid duplicate symbols in multiple
modules.

Simo.

[Lukas Slebodnik]

On (16/10/13 08:56), Simo Sorce wrote:
> On Wed, 2013-10-16 at 14:49 +0200, Benjamin Franzke wrote:
> > Hi List,
> >
> > The symbol add_key is used by
> > src/providers/krb5/krb5_delayed_online_authentication.c
> > which is part of libsss_krb5_common.so
> >
> > Fixes following error:
> > [sssd[be[default]]] [load_backend_module]
> >   (0x0010): Unable to load ad module with path
> >   (/usr/lib64/sssd/libsss_ad.so), error:
> >   /usr/lib64/sssd/libsss_krb5_common.so: undefined symbol: add_key
> >
> > -lkeyutils was passed to the libraries libsss_{krb5,ipa,ad}.so,
> > but when compiling with -Wl,--as-needed this flag will be ignored,
> > since it is not used directly. So it was unavailable to
> > libsss_krb5_common.so which actually needs it.
> >
> > This patch removes $(KEYUTILS_LIBS) from those libraries and adds it
> > to
> > libsss_krb5_common.so
> >
> > Maybe libsss_krb5_common.so should be added to dlopen-tests?
>
> Yes please.
> > But then other libraries and functions are needed as well,
> > which it currently inherits from libsss_{krb5,ipa,ad}.so.
>
> You can just open a chain of libraries.
> > BTW: are these common libraries (i mean ldap too) convenience build
> > libraries, or to save disk space?
>
> Not just to save disk space but to avoid duplicate symbols in multiple
> modules.
>
> Simo.

I am sending rebased patch and patch where libsss_krb5_common.so was added
into dlopen-tests.

LS

[Jakub Hrozek]

On Tue, Nov 12, 2013 at 08:23:04AM +0100, Lukas Slebodnik wrote:
> On (16/10/13 08:56), Simo Sorce wrote:
> > On Wed, 2013-10-16 at 14:49 +0200, Benjamin Franzke wrote:
> > > Hi List,
> > >
> > > The symbol add_key is used by
> > > src/providers/krb5/krb5_delayed_online_authentication.c
> > > which is part of libsss_krb5_common.so
> > >
> > > Fixes following error:
> > > [sssd[be[default]]] [load_backend_module]
> > >   (0x0010): Unable to load ad module with path
> > >   (/usr/lib64/sssd/libsss_ad.so), error:
> > >   /usr/lib64/sssd/libsss_krb5_common.so: undefined symbol: add_key
> > >
> > > -lkeyutils was passed to the libraries libsss_{krb5,ipa,ad}.so,
> > > but when compiling with -Wl,--as-needed this flag will be ignored,
> > > since it is not used directly. So it was unavailable to
> > > libsss_krb5_common.so which actually needs it.
> > >
> > > This patch removes $(KEYUTILS_LIBS) from those libraries and adds it
> > > to
> > > libsss_krb5_common.so
> > >
> > > Maybe libsss_krb5_common.so should be added to dlopen-tests?
> >
> > Yes please.
> > > But then other libraries and functions are needed as well,
> > > which it currently inherits from libsss_{krb5,ipa,ad}.so.
> >
> > You can just open a chain of libraries.
> > > BTW: are these common libraries (i mean ldap too) convenience build
> > > libraries, or to save disk space?
> >
> > Not just to save disk space but to avoid duplicate symbols in multiple
> > modules.
> >
> > Simo.
>
> I am sending rebased patch and patch where libsss_krb5_common.so was added
> into dlopen-tests.
>
> LS
> From 4318c59a8fc327fbc6013c8b424a35a2717fe838 Mon Sep 17 00:00:00 2001
> From: Benjamin Franzke 
> Date: Wed, 16 Oct 2013 13:23:48 +0200
> Subject: [PATCH 1/3] BUILD: Link libsss_krb5_common.so to libkeyutils.so

ACK
> From e1639ee19549d645d84bf3206b2bd1f20964b555 Mon Sep 17 00:00:00 2001
> From: Lukas Slebodnik 
> Date: Mon, 11 Nov 2013 19:35:13 +0100
> Subject: [PATCH 2/3] BUILD: move file find_uid.c into libsss_util.so
>
> Makefile.am | 30 ++++++++----------------------
>  1 file changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 102797cec02161b0553c2adb3cbf19e83fd50be0..62e31a6fe4a22e8d98f73a6fb3c3e39ae3710555 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -590,9 +590,13 @@ libsss_util_la_SOURCES = \
>      src/util/util_errors.c \
>      src/util/sss_ini.c \
>      src/util/io.c \
> src/util/util_sss_idmap.c
> src/util/util_sss_idmap.c \
> src/util/find_uid.c
>
> +libsss_util_la_CFLAGS = \
> $(SYSTEMD_LOGIN_CFLAGS)
>
> libsss_util_la_LIBADD = \
>      $(SSSD_LIBS) \
> $(SYSTEMD_LOGIN_LIBS) \
> $(UNICODE_LIBS)
>
> if BUILD_SUDO
>      libsss_util_la_SOURCES += src/db/sysdb_sudo.c
> @@ -784,16 +788,13 @@ sss_useradd_LDADD = \
>
> sss_userdel_SOURCES = \
>      src/tools/sss_userdel.c \
> src/util/find_uid.c \
> $(SSSD_LCL_TOOLS_OBJ)
>
> sss_userdel_LDADD = \
>      $(TOOLS_LIBS) \
> $(SYSTEMD_LOGIN_LIBS) \
> $(SSSD_INTERNAL_LTLIBS) \
> $(CLIENT_LIBS)
>
> sss_userdel_CFLAGS = \
> $(AM_CFLAGS) \
> $(SYSTEMD_LOGIN_CFLAGS)
> $(AM_CFLAGS)
>
> sss_groupadd_SOURCES = \
>      src/tools/sss_groupadd.c \
> @@ -987,21 +988,18 @@ krb5_utils_tests_SOURCES = \
>      src/providers/krb5/krb5_become_user.c \
>      src/providers/krb5/krb5_common.c \
>      src/util/sss_krb5.c \
> src/util/find_uid.c \
> src/providers/data_provider_fo.c \
> src/providers/data_provider_opts.c \
> src/providers/data_provider_callbacks.c \
> $(SSSD_FAILOVER_OBJ)
>
> krb5_utils_tests_CFLAGS = \
>      $(AM_CFLAGS) \
> $(CHECK_CFLAGS) \
> $(SYSTEMD_LOGIN_CFLAGS)
> $(CHECK_CFLAGS)
>
> krb5_utils_tests_LDADD = \
>      $(SSSD_LIBS)\
>      $(CARES_LIBS) \
>      $(KRB5_LIBS) \
>      $(CHECK_LIBS) \
> $(SYSTEMD_LOGIN_LIBS) \
> $(SSSD_INTERNAL_LTLIBS) \
> libsss_test_common.la
>
> @@ -1251,7 +1249,6 @@ krb5_child_test_SOURCES = \
>      src/providers/krb5/krb5_become_user.c \
>      src/providers/krb5/krb5_common.c \
>      src/util/sss_krb5.c \
> src/util/find_uid.c \
> src/providers/data_provider_fo.c \
> src/providers/data_provider_opts.c \
> src/providers/data_provider_callbacks.c \
>
> @@ -1259,14 +1256,12 @@ krb5_child_test_SOURCES = \
>  krb5_child_test_CFLAGS = \
>      $(AM_CFLAGS) \
>      -DKRB5_CHILD_DIR="$(builddir)" \
> $(CHECK_CFLAGS) \
> $(SYSTEMD_LOGIN_CFLAGS)
> $(CHECK_CFLAGS)
>
> krb5_child_test_LDADD = \
>      $(SSSD_LIBS) \
>      $(CARES_LIBS) \
>      $(KRB5_LIBS) \
>      $(CHECK_LIBS) \
> $(SYSTEMD_LOGIN_LIBS) \
> $(SSSD_INTERNAL_LTLIBS) \
> libsss_test_common.la
>
> @@ -1392,12 +1387,10 @@ ad_access_filter_tests_SOURCES = \
>      $(sssd_be_SOURCES) \
>      src/util/sss_ldap.c \
>      src/util/sss_krb5.c \
> src/util/find_uid.c \
> src/util/user_info_msg.c \
> src/tests/cmocka/test_ad_access_filter.c
>
> ad_access_filter_tests_CFLAGS = \
>      $(AM_CFLAGS) \
> $(SYSTEMD_LOGIN_CFLAGS) \
> -DUNIT_TESTING
>
> ad_access_filter_tests_LDADD = \
>      $(PAM_LIBS) \
> @@ -1406,7 +1399,6 @@ ad_access_filter_tests_LDADD = \
>      $(CARES_LIBS) \
>      $(KRB5_LIBS) \
>      $(SSSD_INTERNAL_LTLIBS) \
> $(SYSTEMD_LOGIN_LIBS) \
> libsss_ldap_common.la \
> libsss_idmap.la \
> libsss_krb5_common.la \
>
> @@ -1606,14 +1598,12 @@ libsss_ldap_la_SOURCES = \
>      src/util/sss_krb5.c
>  libsss_ldap_la_CFLAGS = \
>      $(AM_CFLAGS) \
> $(SYSTEMD_LOGIN_CFLAGS) \
> $(OPENLDAP_CFLAGS) \
> $(KRB5_CFLAGS)
>
> libsss_ldap_la_LIBADD = \
>      $(OPENLDAP_LIBS) \
>      $(DHASH_LIBS) \
>      $(KRB5_LIBS) \
> $(SYSTEMD_LOGIN_LIBS) \
> libsss_ldap_common.la \
> libsss_idmap.la
>
> libsss_ldap_la_LDFLAGS = \
> @@ -1749,21 +1739,17 @@ libsss_ad_la_SOURCES = \
>      src/providers/ad/ad_subdomains.h \
>      src/providers/ad/ad_domain_info.c \
>      src/providers/ad/ad_domain_info.h \
> src/util/find_uid.c \
> src/util/user_info_msg.c \
> src/util/sss_krb5.c \
> src/util/sss_ldap.c
>
> libsss_ad_la_CFLAGS = \
>      $(AM_CFLAGS) \
> $(SYSTEMD_LOGIN_CFLAGS) \
> $(OPENLDAP_CFLAGS) \
> $(SASL_CFLAGS) \

Why was SASL_CFLAGS removed here?
>  $(DHASH_CFLAGS) \
>  $(KRB5_CFLAGS) \
>  $(NDR_NBT_CFLAGS)
>
> libsss_ad_la_LIBADD = \
> $(SYSTEMD_LOGIN_LIBS) \
> $(OPENLDAP_LIBS) \
> $(SASL_LIBS) \
> $(DHASH_LIBS) \
>
> -- 
> 1.8.3.1
> From b044b85ac073ed068796a9365715155c6edd8fd8 Mon Sep 17 00:00:00 2001
> From: Lukas Slebodnik 
> Date: Mon, 11 Nov 2013 19:39:17 +0100
> Subject: [PATCH 3/3] TEST: Add libsss_krb5_common.so to dlopen test
>
> Makefile.am              | 6 ++++--
>  src/tests/dlopen-tests.c | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 62e31a6fe4a22e8d98f73a6fb3c3e39ae3710555..cb23fe4764d7aa831359e21b59a033c32c3a25fa 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1580,9 +1580,11 @@ libsss_krb5_common_la_SOURCES = \
>      src/providers/krb5/krb5_auth.c \
>      src/providers/krb5/krb5_access.c \
>      src/providers/krb5/krb5_child_handler.c \
> src/providers/krb5/krb5_init_shared.c
> src/providers/krb5/krb5_init_shared.c \
> src/util/sss_krb5.c

Why was sss_krb5.c and KRB5_LIBS added in this patch?
> libsss_krb5_common_la_LIBADD = \
> $(KEYUTILS_LIBS)
> $(KEYUTILS_LIBS) \
> $(KRB5_LIBS)
>
> libsss_krb5_common_la_LDFLAGS = \
>      -avoid-version
>
> diff --git a/src/tests/dlopen-tests.c b/src/tests/dlopen-tests.c
> index dd4cc754ac37621b953a462b5e73696d4556770c..9c09e3334005fdac03be84489777c0e96e84cf4e 100644
> --- a/src/tests/dlopen-tests.c
> +++ b/src/tests/dlopen-tests.c
> @@ -80,6 +80,8 @@ struct so {
>                           LIBPFX"libsss_ipa.so", NULL } },
>      { "libsss_krb5.so", { LIBPFX"libdlopen_test_providers.so",
>                            LIBPFX"libsss_krb5.so", NULL } },
> { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
>                             LIBPFX"libsss_krb5_common.so", NULL } },
> { "libsss_ldap.so", { LIBPFX"libdlopen_test_providers.so",
>                          LIBPFX"libsss_ldap.so", NULL } },
>    { "libsss_proxy.so", { LIBPFX"libdlopen_test_providers.so",
>
> -- 
> 1.8.3.1

[Lukas Slebodnik]

On (12/11/13 11:15), Jakub Hrozek wrote:
> On Tue, Nov 12, 2013 at 08:23:04AM +0100, Lukas Slebodnik wrote:
> > On (16/10/13 08:56), Simo Sorce wrote:
> > > On Wed, 2013-10-16 at 14:49 +0200, Benjamin Franzke wrote:
> > > > Hi List,
> > > >
> > > > The symbol add_key is used by
> > > > src/providers/krb5/krb5_delayed_online_authentication.c
> > > > which is part of libsss_krb5_common.so
> > > >
> > > > Fixes following error:
> > > > [sssd[be[default]]] [load_backend_module]
> > > >   (0x0010): Unable to load ad module with path
> > > >   (/usr/lib64/sssd/libsss_ad.so), error:
> > > >   /usr/lib64/sssd/libsss_krb5_common.so: undefined symbol: add_key
> > > >
> > > > -lkeyutils was passed to the libraries libsss_{krb5,ipa,ad}.so,
> > > > but when compiling with -Wl,--as-needed this flag will be ignored,
> > > > since it is not used directly. So it was unavailable to
> > > > libsss_krb5_common.so which actually needs it.
> > > >
> > > > This patch removes $(KEYUTILS_LIBS) from those libraries and adds it
> > > > to
> > > > libsss_krb5_common.so
> > > >
> > > > Maybe libsss_krb5_common.so should be added to dlopen-tests?
> > >
> > > Yes please.
> > > > But then other libraries and functions are needed as well,
> > > > which it currently inherits from libsss_{krb5,ipa,ad}.so.
> > >
> > > You can just open a chain of libraries.
> > > > BTW: are these common libraries (i mean ldap too) convenience build
> > > > libraries, or to save disk space?
> > >
> > > Not just to save disk space but to avoid duplicate symbols in multiple
> > > modules.
> > >
> > > Simo.
> >
> > I am sending rebased patch and patch where libsss_krb5_common.so was added
> > into dlopen-tests.
> >
> > LS
> > From 4318c59a8fc327fbc6013c8b424a35a2717fe838 Mon Sep 17 00:00:00 2001
> > From: Benjamin Franzke 
> > Date: Wed, 16 Oct 2013 13:23:48 +0200
> > Subject: [PATCH 1/3] BUILD: Link libsss_krb5_common.so to libkeyutils.so
>
> ACK
> > From e1639ee19549d645d84bf3206b2bd1f20964b555 Mon Sep 17 00:00:00 2001
> > From: Lukas Slebodnik 
> > Date: Mon, 11 Nov 2013 19:35:13 +0100
> > Subject: [PATCH 2/3] BUILD: move file find_uid.c into libsss_util.so
> >
> > Makefile.am | 30 ++++++++----------------------
> >  1 file changed, 8 insertions(+), 22 deletions(-)
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 102797cec02161b0553c2adb3cbf19e83fd50be0..62e31a6fe4a22e8d98f73a6fb3c3e39ae3710555 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -590,9 +590,13 @@ libsss_util_la_SOURCES = \
> >      src/util/util_errors.c \
> >      src/util/sss_ini.c \
> >      src/util/io.c \
> > src/util/util_sss_idmap.c
> > src/util/util_sss_idmap.c \
> > src/util/find_uid.c
> >
> > +libsss_util_la_CFLAGS = \
> > $(SYSTEMD_LOGIN_CFLAGS)
> >
> > libsss_util_la_LIBADD = \
> >      $(SSSD_LIBS) \
> > $(SYSTEMD_LOGIN_LIBS) \
> > $(UNICODE_LIBS)
> >
> > if BUILD_SUDO
> >      libsss_util_la_SOURCES += src/db/sysdb_sudo.c
> > @@ -784,16 +788,13 @@ sss_useradd_LDADD = \
> >
> > sss_userdel_SOURCES = \
> >      src/tools/sss_userdel.c \
> > src/util/find_uid.c \
> > $(SSSD_LCL_TOOLS_OBJ)
> >
> > sss_userdel_LDADD = \
> >      $(TOOLS_LIBS) \
> > $(SYSTEMD_LOGIN_LIBS) \
> > $(SSSD_INTERNAL_LTLIBS) \
> > $(CLIENT_LIBS)
> >
> > sss_userdel_CFLAGS = \
> > $(AM_CFLAGS) \
> > $(SYSTEMD_LOGIN_CFLAGS)
> > $(AM_CFLAGS)
> >
> > sss_groupadd_SOURCES = \
> >      src/tools/sss_groupadd.c \
> > @@ -987,21 +988,18 @@ krb5_utils_tests_SOURCES = \
> >      src/providers/krb5/krb5_become_user.c \
> >      src/providers/krb5/krb5_common.c \
> >      src/util/sss_krb5.c \
> > src/util/find_uid.c \
> > src/providers/data_provider_fo.c \
> > src/providers/data_provider_opts.c \
> > src/providers/data_provider_callbacks.c \
> > $(SSSD_FAILOVER_OBJ)
> >
> > krb5_utils_tests_CFLAGS = \
> >      $(AM_CFLAGS) \
> > $(CHECK_CFLAGS) \
> > $(SYSTEMD_LOGIN_CFLAGS)
> > $(CHECK_CFLAGS)
> >
> > krb5_utils_tests_LDADD = \
> >      $(SSSD_LIBS)\
> >      $(CARES_LIBS) \
> >      $(KRB5_LIBS) \
> >      $(CHECK_LIBS) \
> > $(SYSTEMD_LOGIN_LIBS) \
> > $(SSSD_INTERNAL_LTLIBS) \
> > libsss_test_common.la
> >
> > @@ -1251,7 +1249,6 @@ krb5_child_test_SOURCES = \
> >      src/providers/krb5/krb5_become_user.c \
> >      src/providers/krb5/krb5_common.c \
> >      src/util/sss_krb5.c \
> > src/util/find_uid.c \
> > src/providers/data_provider_fo.c \
> > src/providers/data_provider_opts.c \
> > src/providers/data_provider_callbacks.c \
> >
> > @@ -1259,14 +1256,12 @@ krb5_child_test_SOURCES = \
> >  krb5_child_test_CFLAGS = \
> >      $(AM_CFLAGS) \
> >      -DKRB5_CHILD_DIR="$(builddir)" \
> > $(CHECK_CFLAGS) \
> > $(SYSTEMD_LOGIN_CFLAGS)
> > $(CHECK_CFLAGS)
> >
> > krb5_child_test_LDADD = \
> >      $(SSSD_LIBS) \
> >      $(CARES_LIBS) \
> >      $(KRB5_LIBS) \
> >      $(CHECK_LIBS) \
> > $(SYSTEMD_LOGIN_LIBS) \
> > $(SSSD_INTERNAL_LTLIBS) \
> > libsss_test_common.la
> >
> > @@ -1392,12 +1387,10 @@ ad_access_filter_tests_SOURCES = \
> >      $(sssd_be_SOURCES) \
> >      src/util/sss_ldap.c \
> >      src/util/sss_krb5.c \
> > src/util/find_uid.c \
> > src/util/user_info_msg.c \
> > src/tests/cmocka/test_ad_access_filter.c
> >
> > ad_access_filter_tests_CFLAGS = \
> >      $(AM_CFLAGS) \
> > $(SYSTEMD_LOGIN_CFLAGS) \
> > -DUNIT_TESTING
> >
> > ad_access_filter_tests_LDADD = \
> >      $(PAM_LIBS) \
> > @@ -1406,7 +1399,6 @@ ad_access_filter_tests_LDADD = \
> >      $(CARES_LIBS) \
> >      $(KRB5_LIBS) \
> >      $(SSSD_INTERNAL_LTLIBS) \
> > $(SYSTEMD_LOGIN_LIBS) \
> > libsss_ldap_common.la \
> > libsss_idmap.la \
> > libsss_krb5_common.la \
> >
> > @@ -1606,14 +1598,12 @@ libsss_ldap_la_SOURCES = \
> >      src/util/sss_krb5.c
> >  libsss_ldap_la_CFLAGS = \
> >      $(AM_CFLAGS) \
> > $(SYSTEMD_LOGIN_CFLAGS) \
> > $(OPENLDAP_CFLAGS) \
> > $(KRB5_CFLAGS)
> >
> > libsss_ldap_la_LIBADD = \
> >      $(OPENLDAP_LIBS) \
> >      $(DHASH_LIBS) \
> >      $(KRB5_LIBS) \
> > $(SYSTEMD_LOGIN_LIBS) \
> > libsss_ldap_common.la \
> > libsss_idmap.la
> >
> > libsss_ldap_la_LDFLAGS = \
> > @@ -1749,21 +1739,17 @@ libsss_ad_la_SOURCES = \
> >      src/providers/ad/ad_subdomains.h \
> >      src/providers/ad/ad_domain_info.c \
> >      src/providers/ad/ad_domain_info.h \
> > src/util/find_uid.c \
> > src/util/user_info_msg.c \
> > src/util/sss_krb5.c \
> > src/util/sss_ldap.c
> >
> > libsss_ad_la_CFLAGS = \
> >      $(AM_CFLAGS) \
> > $(SYSTEMD_LOGIN_CFLAGS) \
> > $(OPENLDAP_CFLAGS) \
> > $(SASL_CFLAGS) \
>
> Why was SASL_CFLAGS removed here?

It was a mistake.
> >  $(DHASH_CFLAGS) \
> >  $(KRB5_CFLAGS) \
> >  $(NDR_NBT_CFLAGS)
> >
> > libsss_ad_la_LIBADD = \
> > $(SYSTEMD_LOGIN_LIBS) \
> > $(OPENLDAP_LIBS) \
> > $(SASL_LIBS) \
> > $(DHASH_LIBS) \
> >
> > -- 
> > 1.8.3.1
> > From b044b85ac073ed068796a9365715155c6edd8fd8 Mon Sep 17 00:00:00 2001
> > From: Lukas Slebodnik 
> > Date: Mon, 11 Nov 2013 19:39:17 +0100
> > Subject: [PATCH 3/3] TEST: Add libsss_krb5_common.so to dlopen test
> >
> > Makefile.am              | 6 ++++--
> >  src/tests/dlopen-tests.c | 2 ++
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 62e31a6fe4a22e8d98f73a6fb3c3e39ae3710555..cb23fe4764d7aa831359e21b59a033c32c3a25fa 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -1580,9 +1580,11 @@ libsss_krb5_common_la_SOURCES = \
> >      src/providers/krb5/krb5_auth.c \
> >      src/providers/krb5/krb5_access.c \
> >      src/providers/krb5/krb5_child_handler.c \
> > src/providers/krb5/krb5_init_shared.c
> > src/providers/krb5/krb5_init_shared.c \
> > src/util/sss_krb5.c
>
> Why was sss_krb5.c and KRB5_LIBS added in this patch?

Variable KRB5_LIBS was added because libsss_krb5_common.so directly call krb5
functions and dlopen test failed without it.

bash$ objdump -T .libs/libsss_krb5_common.so  | grep -E "UND.*krb5_" | tail -n5
0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_cc_destroy
0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_set_trace_callback
0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_parse_name_flags
0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_kt_resolve
0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_cc_set_default_name

dlopen test failed without file sss_krb5.c

bash$ ./dlopen-tests
Running suite(s): dlopen
0%: Checks: 1, Failures: 1, Errors: 0
./src/tests/dlopen-tests.c:131:F:dlopen:test_dlopen_base:0:
    Error opening libsss_krb5_common.so: [dlopen() failed: ./sssd_build/.libs/libsss_krb5_common.so: undefined symbol: check_fast]

Updated patches are attached.

LS

[Simo Sorce]

On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> On (12/11/13 11:15), Jakub Hrozek wrote:
> > > From b044b85ac073ed068796a9365715155c6edd8fd8 Mon Sep 17 00:00:00 2001
> > > From: Lukas Slebodnik 
> > > Date: Mon, 11 Nov 2013 19:39:17 +0100
> > > Subject: [PATCH 3/3] TEST: Add libsss_krb5_common.so to dlopen test
> > >
> > > Makefile.am              | 6 ++++--
> > >  src/tests/dlopen-tests.c | 2 ++
> > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 62e31a6fe4a22e8d98f73a6fb3c3e39ae3710555..cb23fe4764d7aa831359e21b59a033c32c3a25fa 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -1580,9 +1580,11 @@ libsss_krb5_common_la_SOURCES = \
> > >      src/providers/krb5/krb5_auth.c \
> > >      src/providers/krb5/krb5_access.c \
> > >      src/providers/krb5/krb5_child_handler.c \
> > > src/providers/krb5/krb5_init_shared.c
> > > src/providers/krb5/krb5_init_shared.c \
> > > src/util/sss_krb5.c
> >
> > Why was sss_krb5.c and KRB5_LIBS added in this patch?
>
> Variable KRB5_LIBS was added because libsss_krb5_common.so directly call krb5
> functions and dlopen test failed without it.
>
> bash$ objdump -T .libs/libsss_krb5_common.so  | grep -E "UND.*krb5_" | tail -n5
> 0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_cc_destroy
> 0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_set_trace_callback
> 0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_parse_name_flags
> 0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_kt_resolve
> 0000000000000000      DF *UND*  0000000000000000  krb5_3_MIT  krb5_cc_set_default_name
>
> dlopen test failed without file sss_krb5.c
>
> bash$ ./dlopen-tests
> Running suite(s): dlopen
> 0%: Checks: 1, Failures: 1, Errors: 0
> ./src/tests/dlopen-tests.c:131:F:dlopen:test_dlopen_base:0:
>     Error opening libsss_krb5_common.so: [dlopen() failed: ./sssd_build/.libs/libsss_krb5_common.so: undefined symbol: check_fast]
>
> Updated patches are attached.

This is not the right way to fix the failure.

That library *knowingly* has missing symbols because it is an internal
library.

You need to build or link to an outer program that have only the
necessary symbols you know are going to be available.

In this case those symbols are in libsss_krb5.so, so all you should nee
to do is to have this in tests:

+    { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
+                                 LIBPFX"libsss_krb5_common.so",
+ 				  LIBPFX"libsss_krb5.so",
+ 				  NULL } },

Adding c files to librraies to "make them pass" the dl-open test is
wrong in 99% of the cases. you are just making the test pass not testing
that the libraries have the right dependencies. In particular for
internal modules you really need to understand the hierarchy of the
modules and link in the test all the necessary dependencies before
thinking of adding any file to the module itself.

In this case what you did was to duplicate symblos in the resulting Text
file, and that can potentially cause very serious consequencies if any
of those symbols is a static symbols that is meant to have a share de
value all code rely on.

Simo.

[Simo Sorce]

On Tue, 2013-11-12 at 11:03 -0500, Simo Sorce wrote:
> On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> > Updated patches are attached.
>
> This is not the right way to fix the failure.
>
> That library *knowingly* has missing symbols because it is an internal
> library.
>
> You need to build or link to an outer program that have only the
> necessary symbols you know are going to be available.
>
> In this case those symbols are in libsss_krb5.so, so all you should nee
> to do is to have this in tests:
> { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
>                             LIBPFX"libsss_krb5_common.so",
> 		  LIBPFX"libsss_krb5.so",
> 		  NULL } },
>
> Adding c files to librraies to "make them pass" the dl-open test is
> wrong in 99% of the cases. you are just making the test pass not testing
> that the libraries have the right dependencies. In particular for
> internal modules you really need to understand the hierarchy of the
> modules and link in the test all the necessary dependencies before
> thinking of adding any file to the module itself.
>
> In this case what you did was to duplicate symblos in the resulting Text
> file, and that can potentially cause very serious consequencies if any
> of those symbols is a static symbols that is meant to have a share de
> value all code rely on.

Oh let me add.
I did my check very quickly so it is possible that my version is not the
final correct approach, I was just describing how you should pass in
multiple libs for dependency.

It is possible that you need those symbols directly in
libsss_krb5_common.so, but if that is actually the case, then it means
you have to remove those symbols from libsss_krb5.so and add them
libsss_krb5_common.so as dependency for libsss_krb5.so in makefile and
tests. This may even require moving functions from one file to another
in the extreme case.

HTH,
Simo.

[Lukas Slebodnik]

On (12/11/13 11:15), Simo Sorce wrote:
> On Tue, 2013-11-12 at 11:03 -0500, Simo Sorce wrote:
> > On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> > > Updated patches are attached.
> >
> > This is not the right way to fix the failure.
> >
> > That library *knowingly* has missing symbols because it is an internal
> > library.
> >
> > You need to build or link to an outer program that have only the
> > necessary symbols you know are going to be available.
> >
> > In this case those symbols are in libsss_krb5.so, so all you should nee
> > to do is to have this in tests:
> > { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
> >                             LIBPFX"libsss_krb5_common.so",
> > 		  LIBPFX"libsss_krb5.so",
> > 		  NULL } },
> >
> > Adding c files to librraies to "make them pass" the dl-open test is
> > wrong in 99% of the cases. you are just making the test pass not testing
> > that the libraries have the right dependencies. In particular for
> > internal modules you really need to understand the hierarchy of the
> > modules and link in the test all the necessary dependencies before
> > thinking of adding any file to the module itself.
> >
> > In this case what you did was to duplicate symblos in the resulting Text
> > file, and that can potentially cause very serious consequencies if any
> > of those symbols is a static symbols that is meant to have a share de
> > value all code rely on.
>
> Oh let me add.
> I did my check very quickly so it is possible that my version is not the
> final correct approach, I was just describing how you should pass in
> multiple libs for dependency.
>
> It is possible that you need those symbols directly in
> libsss_krb5_common.so, but if that is actually the case, then it means
> you have to remove those symbols from libsss_krb5.so and add them
> libsss_krb5_common.so as dependency for libsss_krb5.so in makefile and
> tests. This may even require moving functions from one file to another
> in the extreme case.

zombie patches are returning :-)

I hope all commit messages are self descripting.

They were lot of changes in Makefile.am
sh-4.2$ git diff --stat HEAD~6..HEAD Makefile.am
 Makefile.am | 99 +++++++++++++++++++------------------------------------------
 1 file changed, 31 insertions(+), 68 deletions(-)

LS

[Lukas Slebodnik]

On (15/03/14 17:57), Lukas Slebodnik wrote:
> On (12/11/13 11:15), Simo Sorce wrote:
> > On Tue, 2013-11-12 at 11:03 -0500, Simo Sorce wrote:
> > > On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> > > > Updated patches are attached.
> > >
> > > This is not the right way to fix the failure.
> > >
> > > That library *knowingly* has missing symbols because it is an internal
> > > library.
> > >
> > > You need to build or link to an outer program that have only the
> > > necessary symbols you know are going to be available.
> > >
> > > In this case those symbols are in libsss_krb5.so, so all you should nee
> > > to do is to have this in tests:
> > > { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
> > >                             LIBPFX"libsss_krb5_common.so",
> > > 		  LIBPFX"libsss_krb5.so",
> > > 		  NULL } },
> > >
> > > Adding c files to librraies to "make them pass" the dl-open test is
> > > wrong in 99% of the cases. you are just making the test pass not testing
> > > that the libraries have the right dependencies. In particular for
> > > internal modules you really need to understand the hierarchy of the
> > > modules and link in the test all the necessary dependencies before
> > > thinking of adding any file to the module itself.
> > >
> > > In this case what you did was to duplicate symblos in the resulting Text
> > > file, and that can potentially cause very serious consequencies if any
> > > of those symbols is a static symbols that is meant to have a share de
> > > value all code rely on.
> >
> > Oh let me add.
> > I did my check very quickly so it is possible that my version is not the
> > final correct approach, I was just describing how you should pass in
> > multiple libs for dependency.
> >
> > It is possible that you need those symbols directly in
> > libsss_krb5_common.so, but if that is actually the case, then it means
> > you have to remove those symbols from libsss_krb5.so and add them
> > libsss_krb5_common.so as dependency for libsss_krb5.so in makefile and
> > tests. This may even require moving functions from one file to another
> > in the extreme case.
>
> zombie patches are returning :-)
>
> I hope all commit messages are self descripting.
>
> They were lot of changes in Makefile.am
> sh-4.2$ git diff --stat HEAD~6..HEAD Makefile.am
> Makefile.am | 99 +++++++++++++++++++------------------------------------------
> 1 file changed, 31 insertions(+), 68 deletions(-)
>
> LS
> From 457eaee99fb5afba5f6dd3752efcf78ce19d723c Mon Sep 17 00:00:00 2001
> From: Lukas Slebodnik 
> Date: Sat, 15 Mar 2014 17:29:25 +0100
> Subject: [PATCH 5/6] BUILD: Move duplicated files from providers to
> libsss_ldap_common.so

self-NACK

make distcheck fails with 5th patch.

LS

[Lukas Slebodnik]

On (17/03/14 07:53), Lukas Slebodnik wrote:
> On (15/03/14 17:57), Lukas Slebodnik wrote:
> > On (12/11/13 11:15), Simo Sorce wrote:
> > > On Tue, 2013-11-12 at 11:03 -0500, Simo Sorce wrote:
> > > > On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> > > > > Updated patches are attached.
> > > >
> > > > This is not the right way to fix the failure.
> > > >
> > > > That library *knowingly* has missing symbols because it is an internal
> > > > library.
> > > >
> > > > You need to build or link to an outer program that have only the
> > > > necessary symbols you know are going to be available.
> > > >
> > > > In this case those symbols are in libsss_krb5.so, so all you should nee
> > > > to do is to have this in tests:
> > > > { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
> > > >                             LIBPFX"libsss_krb5_common.so",
> > > > 		  LIBPFX"libsss_krb5.so",
> > > > 		  NULL } },
> > > >
> > > > Adding c files to librraies to "make them pass" the dl-open test is
> > > > wrong in 99% of the cases. you are just making the test pass not testing
> > > > that the libraries have the right dependencies. In particular for
> > > > internal modules you really need to understand the hierarchy of the
> > > > modules and link in the test all the necessary dependencies before
> > > > thinking of adding any file to the module itself.
> > > >
> > > > In this case what you did was to duplicate symblos in the resulting Text
> > > > file, and that can potentially cause very serious consequencies if any
> > > > of those symbols is a static symbols that is meant to have a share de
> > > > value all code rely on.
> > >
> > > Oh let me add.
> > > I did my check very quickly so it is possible that my version is not the
> > > final correct approach, I was just describing how you should pass in
> > > multiple libs for dependency.
> > >
> > > It is possible that you need those symbols directly in
> > > libsss_krb5_common.so, but if that is actually the case, then it means
> > > you have to remove those symbols from libsss_krb5.so and add them
> > > libsss_krb5_common.so as dependency for libsss_krb5.so in makefile and
> > > tests. This may even require moving functions from one file to another
> > > in the extreme case.
> >
> > zombie patches are returning :-)
> >
> > I hope all commit messages are self descripting.
> >
> > They were lot of changes in Makefile.am
> > sh-4.2$ git diff --stat HEAD~6..HEAD Makefile.am
> > Makefile.am | 99 +++++++++++++++++++------------------------------------------
> > 1 file changed, 31 insertions(+), 68 deletions(-)
> >
> > LS
> > From 457eaee99fb5afba5f6dd3752efcf78ce19d723c Mon Sep 17 00:00:00 2001
> > From: Lukas Slebodnik 
> > Date: Sat, 15 Mar 2014 17:29:25 +0100
> > Subject: [PATCH 5/6] BUILD: Move duplicated files from providers to
> > libsss_ldap_common.so
>
> self-NACK
>
> make distcheck fails with 5th patch.

Interdiff for 5th patch:

diff --git a/Makefile.am b/Makefile.am
index 2d0a295..a65369b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1742,6 +1742,10 @@ endif
 # Plugin Libraries #
 ####################

+# libsss_krb5_common must be installed before libsss_ldap_common
+# because libtool tries to relink libsss_ldap_common when installing
+# libsss_ldap_common and therefore make distcheck fails
+pkglib_LTLIBRARIES += libsss_krb5_common.la
 pkglib_LTLIBRARIES += libsss_ldap_common.la
 libsss_ldap_common_la_SOURCES = \
     src/providers/ldap/ldap_id.c \
@@ -1778,6 +1782,8 @@ libsss_ldap_common_la_SOURCES = \
     src/providers/ldap/sdap.c \
     src/util/user_info_msg.c \
     src/util/sss_ldap.c
 libsss_ldap_common_la_LIBADD = \
     $(KRB5_LIBS) \
     libsss_krb5_common.la \
@@ -1800,11 +1806,7 @@ libsss_ldap_common_la_SOURCES += \
     src/providers/ldap/sdap_async_autofs.c
 endif

libsss_ldap_common_la_CFLAGS = \
    $(KRB5_CFLAGS)

-pkglib_LTLIBRARIES += libsss_krb5_common.la
 libsss_krb5_common_la_SOURCES = \
     src/providers/krb5/krb5_utils.c \
     src/providers/krb5/krb5_become_user.c \

Updated patches are attached

LS

[Jakub Hrozek]

On Mon, Mar 17, 2014 at 09:34:04AM +0100, Lukas Slebodnik wrote:
> On (17/03/14 07:53), Lukas Slebodnik wrote:
> > On (15/03/14 17:57), Lukas Slebodnik wrote:
> > > On (12/11/13 11:15), Simo Sorce wrote:
> > > > On Tue, 2013-11-12 at 11:03 -0500, Simo Sorce wrote:
> > > > > On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> > > > > > Updated patches are attached.
> > > > >
> > > > > This is not the right way to fix the failure.
> > > > >
> > > > > That library *knowingly* has missing symbols because it is an internal
> > > > > library.
> > > > >
> > > > > You need to build or link to an outer program that have only the
> > > > > necessary symbols you know are going to be available.
> > > > >
> > > > > In this case those symbols are in libsss_krb5.so, so all you should nee
> > > > > to do is to have this in tests:
> > > > > { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
> > > > >                             LIBPFX"libsss_krb5_common.so",
> > > > > 		  LIBPFX"libsss_krb5.so",
> > > > > 		  NULL } },
> > > > >
> > > > > Adding c files to librraies to "make them pass" the dl-open test is
> > > > > wrong in 99% of the cases. you are just making the test pass not testing
> > > > > that the libraries have the right dependencies. In particular for
> > > > > internal modules you really need to understand the hierarchy of the
> > > > > modules and link in the test all the necessary dependencies before
> > > > > thinking of adding any file to the module itself.
> > > > >
> > > > > In this case what you did was to duplicate symblos in the resulting Text
> > > > > file, and that can potentially cause very serious consequencies if any
> > > > > of those symbols is a static symbols that is meant to have a share de
> > > > > value all code rely on.
> > > >
> > > > Oh let me add.
> > > > I did my check very quickly so it is possible that my version is not the
> > > > final correct approach, I was just describing how you should pass in
> > > > multiple libs for dependency.
> > > >
> > > > It is possible that you need those symbols directly in
> > > > libsss_krb5_common.so, but if that is actually the case, then it means
> > > > you have to remove those symbols from libsss_krb5.so and add them
> > > > libsss_krb5_common.so as dependency for libsss_krb5.so in makefile and
> > > > tests. This may even require moving functions from one file to another
> > > > in the extreme case.
> > >
> > > zombie patches are returning :-)
> > >
> > > I hope all commit messages are self descripting.
> > >
> > > They were lot of changes in Makefile.am
> > > sh-4.2$ git diff --stat HEAD~6..HEAD Makefile.am
> > > Makefile.am | 99 +++++++++++++++++++------------------------------------------
> > > 1 file changed, 31 insertions(+), 68 deletions(-)
> > >
> > > LS
> > > From 457eaee99fb5afba5f6dd3752efcf78ce19d723c Mon Sep 17 00:00:00 2001
> > > From: Lukas Slebodnik 
> > > Date: Sat, 15 Mar 2014 17:29:25 +0100
> > > Subject: [PATCH 5/6] BUILD: Move duplicated files from providers to
> > > libsss_ldap_common.so
> >
> > self-NACK
> >
> > make distcheck fails with 5th patch.
>
> Interdiff for 5th patch:
>
> diff --git a/Makefile.am b/Makefile.am
> index 2d0a295..a65369b 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1742,6 +1742,10 @@ endif
>  # Plugin Libraries #
>  ####################
>
> +# libsss_krb5_common must be installed before libsss_ldap_common
> +# because libtool tries to relink libsss_ldap_common when installing
> +# libsss_ldap_common and therefore make distcheck fails
> +pkglib_LTLIBRARIES += libsss_krb5_common.la
>  pkglib_LTLIBRARIES += libsss_ldap_common.la
>  libsss_ldap_common_la_SOURCES = \
>      src/providers/ldap/ldap_id.c \
> @@ -1778,6 +1782,8 @@ libsss_ldap_common_la_SOURCES = \
>      src/providers/ldap/sdap.c \
>      src/util/user_info_msg.c \
>      src/util/sss_ldap.c
>  libsss_ldap_common_la_LIBADD = \
>      $(KRB5_LIBS) \
>      libsss_krb5_common.la \
> @@ -1800,11 +1806,7 @@ libsss_ldap_common_la_SOURCES += \
>      src/providers/ldap/sdap_async_autofs.c
>  endif
>
> libsss_ldap_common_la_CFLAGS = \
>     $(KRB5_CFLAGS)
>
> -pkglib_LTLIBRARIES += libsss_krb5_common.la
>  libsss_krb5_common_la_SOURCES = \
>      src/providers/krb5/krb5_utils.c \
>      src/providers/krb5/krb5_become_user.c \
>
> Updated patches are attached
>
> LS

Sorry for the delay in reviewing the packages.

I think these packages are good and I'd like to ACK them.

The testing consisted of:
    * visual inspection of the patches. The commit messages help,
      thanks!
    * applying the dlopen-test patch on master. The dlopen test then
      didn't pass. After aplying the other patches, the dlopen test
      passed as well.
    * starting IPA, AD and LDAP (with and without ID mapping to be sure)
      went fine

[Jakub Hrozek]

On Tue, Apr 15, 2014 at 11:02:15AM +0200, Jakub Hrozek wrote:
> On Mon, Mar 17, 2014 at 09:34:04AM +0100, Lukas Slebodnik wrote:
> > On (17/03/14 07:53), Lukas Slebodnik wrote:
> > > On (15/03/14 17:57), Lukas Slebodnik wrote:
> > > > On (12/11/13 11:15), Simo Sorce wrote:
> > > > > On Tue, 2013-11-12 at 11:03 -0500, Simo Sorce wrote:
> > > > > > On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> > > > > > > Updated patches are attached.
> > > > > >
> > > > > > This is not the right way to fix the failure.
> > > > > >
> > > > > > That library *knowingly* has missing symbols because it is an internal
> > > > > > library.
> > > > > >
> > > > > > You need to build or link to an outer program that have only the
> > > > > > necessary symbols you know are going to be available.
> > > > > >
> > > > > > In this case those symbols are in libsss_krb5.so, so all you should nee
> > > > > > to do is to have this in tests:
> > > > > > { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
> > > > > >                             LIBPFX"libsss_krb5_common.so",
> > > > > > 		  LIBPFX"libsss_krb5.so",
> > > > > > 		  NULL } },
> > > > > >
> > > > > > Adding c files to librraies to "make them pass" the dl-open test is
> > > > > > wrong in 99% of the cases. you are just making the test pass not testing
> > > > > > that the libraries have the right dependencies. In particular for
> > > > > > internal modules you really need to understand the hierarchy of the
> > > > > > modules and link in the test all the necessary dependencies before
> > > > > > thinking of adding any file to the module itself.
> > > > > >
> > > > > > In this case what you did was to duplicate symblos in the resulting Text
> > > > > > file, and that can potentially cause very serious consequencies if any
> > > > > > of those symbols is a static symbols that is meant to have a share de
> > > > > > value all code rely on.
> > > > >
> > > > > Oh let me add.
> > > > > I did my check very quickly so it is possible that my version is not the
> > > > > final correct approach, I was just describing how you should pass in
> > > > > multiple libs for dependency.
> > > > >
> > > > > It is possible that you need those symbols directly in
> > > > > libsss_krb5_common.so, but if that is actually the case, then it means
> > > > > you have to remove those symbols from libsss_krb5.so and add them
> > > > > libsss_krb5_common.so as dependency for libsss_krb5.so in makefile and
> > > > > tests. This may even require moving functions from one file to another
> > > > > in the extreme case.
> > > >
> > > > zombie patches are returning :-)
> > > >
> > > > I hope all commit messages are self descripting.
> > > >
> > > > They were lot of changes in Makefile.am
> > > > sh-4.2$ git diff --stat HEAD~6..HEAD Makefile.am
> > > > Makefile.am | 99 +++++++++++++++++++------------------------------------------
> > > > 1 file changed, 31 insertions(+), 68 deletions(-)
> > > >
> > > > LS
> > > > From 457eaee99fb5afba5f6dd3752efcf78ce19d723c Mon Sep 17 00:00:00 2001
> > > > From: Lukas Slebodnik 
> > > > Date: Sat, 15 Mar 2014 17:29:25 +0100
> > > > Subject: [PATCH 5/6] BUILD: Move duplicated files from providers to
> > > > libsss_ldap_common.so
> > >
> > > self-NACK
> > >
> > > make distcheck fails with 5th patch.
> >
> > Interdiff for 5th patch:
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 2d0a295..a65369b 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -1742,6 +1742,10 @@ endif
> >  # Plugin Libraries #
> >  ####################
> >
> > +# libsss_krb5_common must be installed before libsss_ldap_common
> > +# because libtool tries to relink libsss_ldap_common when installing
> > +# libsss_ldap_common and therefore make distcheck fails
> > +pkglib_LTLIBRARIES += libsss_krb5_common.la
> >  pkglib_LTLIBRARIES += libsss_ldap_common.la
> >  libsss_ldap_common_la_SOURCES = \
> >      src/providers/ldap/ldap_id.c \
> > @@ -1778,6 +1782,8 @@ libsss_ldap_common_la_SOURCES = \
> >      src/providers/ldap/sdap.c \
> >      src/util/user_info_msg.c \
> >      src/util/sss_ldap.c
> >  libsss_ldap_common_la_LIBADD = \
> >      $(KRB5_LIBS) \
> >      libsss_krb5_common.la \
> > @@ -1800,11 +1806,7 @@ libsss_ldap_common_la_SOURCES += \
> >      src/providers/ldap/sdap_async_autofs.c
> >  endif
> >
> > libsss_ldap_common_la_CFLAGS = \
> >     $(KRB5_CFLAGS)
> >
> > -pkglib_LTLIBRARIES += libsss_krb5_common.la
> >  libsss_krb5_common_la_SOURCES = \
> >      src/providers/krb5/krb5_utils.c \
> >      src/providers/krb5/krb5_become_user.c \
> >
> > Updated patches are attached
> >
> > LS
>
> Sorry for the delay in reviewing the packages.
>
> I think these packages are good and I'd like to ACK them.
>
> The testing consisted of:
>     * visual inspection of the patches. The commit messages help,
>       thanks!
>     * applying the dlopen-test patch on master. The dlopen test then
>       didn't pass. After aplying the other patches, the dlopen test
>       passed as well.
>     * starting IPA, AD and LDAP (with and without ID mapping to be sure)
>       went fine

Pushed all patches to master in:
    a25942bc2f6ac9b3b2817ede35fa2b445373c6e4
    cc1c033c34b5f816b633d27a21aefbf811a7cf72
    6261893e00bd14fdd192ffc9a1379cb9c647d326
    12805da52a93c268290cec7b8fbbdbd4ea8abc3e
    7fc27c7a3ccbb6aecb8cf4a4a5f91962028cb897
    717008c8c3f29f3a1a77266cc72a6cfa616bf295

[Lukas Slebodnik]

On (16/04/14 11:12), Jakub Hrozek wrote:
> On Tue, Apr 15, 2014 at 11:02:15AM +0200, Jakub Hrozek wrote:
> > On Mon, Mar 17, 2014 at 09:34:04AM +0100, Lukas Slebodnik wrote:
> > > On (17/03/14 07:53), Lukas Slebodnik wrote:
> > > > On (15/03/14 17:57), Lukas Slebodnik wrote:
> > > > > On (12/11/13 11:15), Simo Sorce wrote:
> > > > > > On Tue, 2013-11-12 at 11:03 -0500, Simo Sorce wrote:
> > > > > > > On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> > > > > >
> > > > > > > > Updated patches are attached.
> > > > > > > 
> > > > > > > This is not the right way to fix the failure.
> > > > > > > 
> > > > > > > That library *knowingly* has missing symbols because it is an internal
> > > > > > > library.
> > > > > > > 
> > > > > > > You need to build or link to an outer program that have only the
> > > > > > > necessary symbols you know are going to be available.
> > > > > > > 
> > > > > > > In this case those symbols are in libsss_krb5.so, so all you should nee
> > > > > > > to do is to have this in tests:
> > > > > > > 
> > > > > > > +    { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
> > > > > > > +                                 LIBPFX"libsss_krb5_common.so",
> > > > > > > + 				  LIBPFX"libsss_krb5.so",
> > > > > > > + 				  NULL } },
> > > > > > > 
> > > > > > > Adding c files to librraies to "make them pass" the dl-open test is
> > > > > > > wrong in 99% of the cases. you are just making the test pass not testing
> > > > > > > that the libraries have the right dependencies. In particular for
> > > > > > > internal modules you really need to understand the hierarchy of the
> > > > > > > modules and link in the test all the necessary dependencies before
> > > > > > > thinking of adding any file to the module itself.
> > > > > > > 
> > > > > > > In this case what you did was to duplicate symblos in the resulting Text
> > > > > > > file, and that can potentially cause very serious consequencies if any
> > > > > > > of those symbols is a static symbols that is meant to have a share de
> > > > > > > value all code rely on.
> > > > > >
> > > > > > Oh let me add.
> > > > > > I did my check very quickly so it is possible that my version is not the
> > > > > > final correct approach, I was just describing how you should pass in
> > > > > > multiple libs for dependency.
> > > > > >
> > > > > > It is possible that you need those symbols directly in
> > > > > > libsss_krb5_common.so, but if that is actually the case, then it means
> > > > > > you have to remove those symbols from libsss_krb5.so and add them
> > > > > > libsss_krb5_common.so as dependency for libsss_krb5.so in makefile and
> > > > > > tests. This may even require moving functions from one file to another
> > > > > > in the extreme case.
> > > > >
> > > > > zombie patches are returning :-)
> > > > >
> > > > > I hope all commit messages are self descripting.
> > > > >
> > > > > They were lot of changes in Makefile.am
> > > > > sh-4.2$ git diff --stat HEAD~6..HEAD Makefile.am
> > > > > Makefile.am | 99 +++++++++++++++++++------------------------------------------
> > > > > 1 file changed, 31 insertions(+), 68 deletions(-)
> > > > >
> > > > > LS
> > > > > From 457eaee99fb5afba5f6dd3752efcf78ce19d723c Mon Sep 17 00:00:00 2001
> > > > > From: Lukas Slebodnik 
> > > > > Date: Sat, 15 Mar 2014 17:29:25 +0100
> > > > > Subject: [PATCH 5/6] BUILD: Move duplicated files from providers to
> > > > > libsss_ldap_common.so
> > > >
> > > > self-NACK
> > > >
> > > > make distcheck fails with 5th patch.
> > >
> > > Interdiff for 5th patch:
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 2d0a295..a65369b 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -1742,6 +1742,10 @@ endif
> > >  # Plugin Libraries #
> > >  ####################
> > >
> > > +# libsss_krb5_common must be installed before libsss_ldap_common
> > > +# because libtool tries to relink libsss_ldap_common when installing
> > > +# libsss_ldap_common and therefore make distcheck fails
> > > +pkglib_LTLIBRARIES += libsss_krb5_common.la
> > >  pkglib_LTLIBRARIES += libsss_ldap_common.la
> > >  libsss_ldap_common_la_SOURCES = \
> > >      src/providers/ldap/ldap_id.c \
> > > @@ -1778,6 +1782,8 @@ libsss_ldap_common_la_SOURCES = \
> > >      src/providers/ldap/sdap.c \
> > >      src/util/user_info_msg.c \
> > >      src/util/sss_ldap.c
> > >  libsss_ldap_common_la_LIBADD = \
> > >      $(KRB5_LIBS) \
> > >      libsss_krb5_common.la \
> > > @@ -1800,11 +1806,7 @@ libsss_ldap_common_la_SOURCES += \
> > >      src/providers/ldap/sdap_async_autofs.c
> > >  endif
> > >
> > > libsss_ldap_common_la_CFLAGS = \
> > >     $(KRB5_CFLAGS)
> > >
> > > -pkglib_LTLIBRARIES += libsss_krb5_common.la
> > >  libsss_krb5_common_la_SOURCES = \
> > >      src/providers/krb5/krb5_utils.c \
> > >      src/providers/krb5/krb5_become_user.c \
> > >
> > > Updated patches are attached
> > >
> > > LS
> >
> > Sorry for the delay in reviewing the packages.
> >
> > I think these packages are good and I'd like to ACK them.
> >
> > The testing consisted of:
> >     * visual inspection of the patches. The commit messages help,
> >       thanks!
> >     * applying the dlopen-test patch on master. The dlopen test then
> >       didn't pass. After aplying the other patches, the dlopen test
> >       passed as well.
> >     * starting IPA, AD and LDAP (with and without ID mapping to be sure)
> >       went fine
>
> Pushed all patches to master in:
>    a25942bc2f6ac9b3b2817ede35fa2b445373c6e4
>    cc1c033c34b5f816b633d27a21aefbf811a7cf72
>    6261893e00bd14fdd192ffc9a1379cb9c647d326
>    12805da52a93c268290cec7b8fbbdbd4ea8abc3e
>    7fc27c7a3ccbb6aecb8cf4a4a5f91962028cb897
>    717008c8c3f29f3a1a77266cc72a6cfa616bf295

Could we push Benjamin's patch to 1.11 branch?

LS

[Jakub Hrozek]

On Wed, Apr 23, 2014 at 11:04:54PM +0200, Lukas Slebodnik wrote:
> On (16/04/14 11:12), Jakub Hrozek wrote:
> > On Tue, Apr 15, 2014 at 11:02:15AM +0200, Jakub Hrozek wrote:
> > > On Mon, Mar 17, 2014 at 09:34:04AM +0100, Lukas Slebodnik wrote:
> > > > On (17/03/14 07:53), Lukas Slebodnik wrote:
> > > > > On (15/03/14 17:57), Lukas Slebodnik wrote:
> > > > > > On (12/11/13 11:15), Simo Sorce wrote:
> > > > > > >On Tue, 2013-11-12 at 11:03 -0500, Simo Sorce wrote:
> > > > > > >> On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> > > > > > >
> > > > > > >> > Updated patches are attached.
> > > > > > >> 
> > > > > > >> This is not the right way to fix the failure.
> > > > > > >> 
> > > > > > >> That library *knowingly* has missing symbols because it is an internal
> > > > > > >> library.
> > > > > > >> 
> > > > > > >> You need to build or link to an outer program that have only the
> > > > > > >> necessary symbols you know are going to be available.
> > > > > > >> 
> > > > > > >> In this case those symbols are in libsss_krb5.so, so all you should nee
> > > > > > >> to do is to have this in tests:
> > > > > > >> 
> > > > > > >> +    { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
> > > > > > >> +                                 LIBPFX"libsss_krb5_common.so",
> > > > > > >> + 				  LIBPFX"libsss_krb5.so",
> > > > > > >> + 				  NULL } },
> > > > > > >> 
> > > > > > >> Adding c files to librraies to "make them pass" the dl-open test is
> > > > > > >> wrong in 99% of the cases. you are just making the test pass not testing
> > > > > > >> that the libraries have the right dependencies. In particular for
> > > > > > >> internal modules you really need to understand the hierarchy of the
> > > > > > >> modules and link in the test all the necessary dependencies before
> > > > > > >> thinking of adding any file to the module itself.
> > > > > > >> 
> > > > > > >> In this case what you did was to duplicate symblos in the resulting Text
> > > > > > >> file, and that can potentially cause very serious consequencies if any
> > > > > > >> of those symbols is a static symbols that is meant to have a share de
> > > > > > >> value all code rely on.
> > > > > > >
> > > > > > >Oh let me add.
> > > > > > >I did my check very quickly so it is possible that my version is not the
> > > > > > >final correct approach, I was just describing how you should pass in
> > > > > > >multiple libs for dependency.
> > > > > > >
> > > > > > >It is possible that you need those symbols directly in
> > > > > > >libsss_krb5_common.so, but if that is actually the case, then it means
> > > > > > >you have to remove those symbols from libsss_krb5.so and add them
> > > > > > >libsss_krb5_common.so as dependency for libsss_krb5.so in makefile and
> > > > > > >tests. This may even require moving functions from one file to another
> > > > > > >in the extreme case.
> > > > > > >
> > > > > >
> > > > > > zombie patches are returning :-)
> > > > > >
> > > > > > I hope all commit messages are self descripting.
> > > > > >
> > > > > > They were lot of changes in Makefile.am
> > > > > > sh-4.2$ git diff --stat HEAD~6..HEAD Makefile.am
> > > > > > Makefile.am | 99 +++++++++++++++++++------------------------------------------
> > > > > > 1 file changed, 31 insertions(+), 68 deletions(-)
> > > > > >
> > > > > > LS
> > > > > > From 457eaee99fb5afba5f6dd3752efcf78ce19d723c Mon Sep 17 00:00:00 2001
> > > > > > From: Lukas Slebodnik 
> > > > > > Date: Sat, 15 Mar 2014 17:29:25 +0100
> > > > > > Subject: [PATCH 5/6] BUILD: Move duplicated files from providers to
> > > > > > libsss_ldap_common.so
> > > > >
> > > > > self-NACK
> > > > >
> > > > > make distcheck fails with 5th patch.
> > > >
> > > > Interdiff for 5th patch:
> > > >
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index 2d0a295..a65369b 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -1742,6 +1742,10 @@ endif
> > > >  # Plugin Libraries #
> > > >  ####################
> > > >
> > > > +# libsss_krb5_common must be installed before libsss_ldap_common
> > > > +# because libtool tries to relink libsss_ldap_common when installing
> > > > +# libsss_ldap_common and therefore make distcheck fails
> > > > +pkglib_LTLIBRARIES += libsss_krb5_common.la
> > > >  pkglib_LTLIBRARIES += libsss_ldap_common.la
> > > >  libsss_ldap_common_la_SOURCES = \
> > > >      src/providers/ldap/ldap_id.c \
> > > > @@ -1778,6 +1782,8 @@ libsss_ldap_common_la_SOURCES = \
> > > >      src/providers/ldap/sdap.c \
> > > >      src/util/user_info_msg.c \
> > > >      src/util/sss_ldap.c
> > > >  libsss_ldap_common_la_LIBADD = \
> > > >      $(KRB5_LIBS) \
> > > >      libsss_krb5_common.la \
> > > > @@ -1800,11 +1806,7 @@ libsss_ldap_common_la_SOURCES += \
> > > >      src/providers/ldap/sdap_async_autofs.c
> > > >  endif
> > > >
> > > > libsss_ldap_common_la_CFLAGS = \
> > > >     $(KRB5_CFLAGS)
> > > >
> > > > -pkglib_LTLIBRARIES += libsss_krb5_common.la
> > > >  libsss_krb5_common_la_SOURCES = \
> > > >      src/providers/krb5/krb5_utils.c \
> > > >      src/providers/krb5/krb5_become_user.c \
> > > >
> > > > Updated patches are attached
> > > >
> > > > LS
> > >
> > > Sorry for the delay in reviewing the packages.
> > >
> > > I think these packages are good and I'd like to ACK them.
> > >
> > > The testing consisted of:
> > >     * visual inspection of the patches. The commit messages help,
> > >       thanks!
> > >     * applying the dlopen-test patch on master. The dlopen test then
> > >       didn't pass. After aplying the other patches, the dlopen test
> > >       passed as well.
> > >     * starting IPA, AD and LDAP (with and without ID mapping to be sure)
> > >       went fine
> >
> > Pushed all patches to master in:
> >    a25942bc2f6ac9b3b2817ede35fa2b445373c6e4
> >    cc1c033c34b5f816b633d27a21aefbf811a7cf72
> >    6261893e00bd14fdd192ffc9a1379cb9c647d326
> >    12805da52a93c268290cec7b8fbbdbd4ea8abc3e
> >    7fc27c7a3ccbb6aecb8cf4a4a5f91962028cb897
> >    717008c8c3f29f3a1a77266cc72a6cfa616bf295
>
> Could we push Benjamin's patch to 1.11 branch?
>
> LS

Sure, did you only want to push Benjamin's patch
(717008c8c3f29f3a1a77266cc72a6cfa616bf295 in master) or the whole set?

[Lukas Slebodnik]

On (25/04/14 11:43), Jakub Hrozek wrote:
> On Wed, Apr 23, 2014 at 11:04:54PM +0200, Lukas Slebodnik wrote:
> > On (16/04/14 11:12), Jakub Hrozek wrote:
> > > On Tue, Apr 15, 2014 at 11:02:15AM +0200, Jakub Hrozek wrote:
> > > > On Mon, Mar 17, 2014 at 09:34:04AM +0100, Lukas Slebodnik wrote:
> > > > > On (17/03/14 07:53), Lukas Slebodnik wrote:
> > > > > > On (15/03/14 17:57), Lukas Slebodnik wrote:
> > > > > > >On (12/11/13 11:15), Simo Sorce wrote:
> > > > > > >>On Tue, 2013-11-12 at 11:03 -0500, Simo Sorce wrote:
> > > > > > >>> On Tue, 2013-11-12 at 16:23 +0100, Lukas Slebodnik wrote:
> > > > > > >>
> > > > > > >>> > Updated patches are attached.
> > > > > > >>> 
> > > > > > >>> This is not the right way to fix the failure.
> > > > > > >>> 
> > > > > > >>> That library *knowingly* has missing symbols because it is an internal
> > > > > > >>> library.
> > > > > > >>> 
> > > > > > >>> You need to build or link to an outer program that have only the
> > > > > > >>> necessary symbols you know are going to be available.
> > > > > > >>> 
> > > > > > >>> In this case those symbols are in libsss_krb5.so, so all you should nee
> > > > > > >>> to do is to have this in tests:
> > > > > > >>> 
> > > > > > >>> +    { "libsss_krb5_common.so", { LIBPFX"libdlopen_test_providers.so",
> > > > > > >>> +                                 LIBPFX"libsss_krb5_common.so",
> > > > > > >>> + 				  LIBPFX"libsss_krb5.so",
> > > > > > >>> + 				  NULL } },
> > > > > > >>> 
> > > > > > >>> Adding c files to librraies to "make them pass" the dl-open test is
> > > > > > >>> wrong in 99% of the cases. you are just making the test pass not testing
> > > > > > >>> that the libraries have the right dependencies. In particular for
> > > > > > >>> internal modules you really need to understand the hierarchy of the
> > > > > > >>> modules and link in the test all the necessary dependencies before
> > > > > > >>> thinking of adding any file to the module itself.
> > > > > > >>> 
> > > > > > >>> In this case what you did was to duplicate symblos in the resulting Text
> > > > > > >>> file, and that can potentially cause very serious consequencies if any
> > > > > > >>> of those symbols is a static symbols that is meant to have a share de
> > > > > > >>> value all code rely on.
> > > > > > >>
> > > > > > >>Oh let me add.
> > > > > > >>I did my check very quickly so it is possible that my version is not the
> > > > > > >>final correct approach, I was just describing how you should pass in
> > > > > > >>multiple libs for dependency.
> > > > > > >>
> > > > > > >>It is possible that you need those symbols directly in
> > > > > > >>libsss_krb5_common.so, but if that is actually the case, then it means
> > > > > > >>you have to remove those symbols from libsss_krb5.so and add them
> > > > > > >>libsss_krb5_common.so as dependency for libsss_krb5.so in makefile and
> > > > > > >>tests. This may even require moving functions from one file to another
> > > > > > >>in the extreme case.
> > > > > > >>
> > > > > > >
> > > > > > >zombie patches are returning :-)
> > > > > > >
> > > > > > >I hope all commit messages are self descripting.
> > > > > > >
> > > > > > >They were lot of changes in Makefile.am
> > > > > > >sh-4.2$ git diff --stat HEAD~6..HEAD Makefile.am
> > > > > > > Makefile.am | 99 +++++++++++++++++++------------------------------------------
> > > > > > > 1 file changed, 31 insertions(+), 68 deletions(-)
> > > > > > >
> > > > > > >LS
> > > > > >
> > > > > > >From 457eaee99fb5afba5f6dd3752efcf78ce19d723c Mon Sep 17 00:00:00 2001
> > > > > > >From: Lukas Slebodnik 
> > > > > > >Date: Sat, 15 Mar 2014 17:29:25 +0100
> > > > > > >Subject: [PATCH 5/6] BUILD: Move duplicated files from providers to
> > > > > > > libsss_ldap_common.so
> > > > > > >
> > > > > >
> > > > > > self-NACK
> > > > > >
> > > > > > make distcheck fails with 5th patch.
> > > > >
> > > > > Interdiff for 5th patch:
> > > > >
> > > > > diff --git a/Makefile.am b/Makefile.am
> > > > > index 2d0a295..a65369b 100644
> > > > > --- a/Makefile.am
> > > > > +++ b/Makefile.am
> > > > > @@ -1742,6 +1742,10 @@ endif
> > > > >  # Plugin Libraries #
> > > > >  ####################
> > > > >
> > > > > +# libsss_krb5_common must be installed before libsss_ldap_common
> > > > > +# because libtool tries to relink libsss_ldap_common when installing
> > > > > +# libsss_ldap_common and therefore make distcheck fails
> > > > > +pkglib_LTLIBRARIES += libsss_krb5_common.la
> > > > >  pkglib_LTLIBRARIES += libsss_ldap_common.la
> > > > >  libsss_ldap_common_la_SOURCES = \
> > > > >      src/providers/ldap/ldap_id.c \
> > > > > @@ -1778,6 +1782,8 @@ libsss_ldap_common_la_SOURCES = \
> > > > >      src/providers/ldap/sdap.c \
> > > > >      src/util/user_info_msg.c \
> > > > >      src/util/sss_ldap.c
> > > > >  libsss_ldap_common_la_LIBADD = \
> > > > >      $(KRB5_LIBS) \
> > > > >      libsss_krb5_common.la \
> > > > > @@ -1800,11 +1806,7 @@ libsss_ldap_common_la_SOURCES += \
> > > > >      src/providers/ldap/sdap_async_autofs.c
> > > > >  endif
> > > > >
> > > > > libsss_ldap_common_la_CFLAGS = \
> > > > >     $(KRB5_CFLAGS)
> > > > >
> > > > > -pkglib_LTLIBRARIES += libsss_krb5_common.la
> > > > >  libsss_krb5_common_la_SOURCES = \
> > > > >      src/providers/krb5/krb5_utils.c \
> > > > >      src/providers/krb5/krb5_become_user.c \
> > > > >
> > > > > Updated patches are attached
> > > > >
> > > > > LS
> > > >
> > > > Sorry for the delay in reviewing the packages.
> > > >
> > > > I think these packages are good and I'd like to ACK them.
> > > >
> > > > The testing consisted of:
> > > >     * visual inspection of the patches. The commit messages help,
> > > >       thanks!
> > > >     * applying the dlopen-test patch on master. The dlopen test then
> > > >       didn't pass. After aplying the other patches, the dlopen test
> > > >       passed as well.
> > > >     * starting IPA, AD and LDAP (with and without ID mapping to be sure)
> > > >       went fine
> > >
> > > Pushed all patches to master in:
> > >    a25942bc2f6ac9b3b2817ede35fa2b445373c6e4
> > >    cc1c033c34b5f816b633d27a21aefbf811a7cf72
> > >    6261893e00bd14fdd192ffc9a1379cb9c647d326
> > >    12805da52a93c268290cec7b8fbbdbd4ea8abc3e
> > >    7fc27c7a3ccbb6aecb8cf4a4a5f91962028cb897
> > >    717008c8c3f29f3a1a77266cc72a6cfa616bf295
> >
> > Could we push Benjamin's patch to 1.11 branch?
> >
> > LS
>
> Sure, did you only want to push Benjamin's patch
> (717008c8c3f29f3a1a77266cc72a6cfa616bf295 in master) or the whole set?

just Benjamin's patch. Other patches will probably not apply to 1.11 branch
and they were necessary mostly for dlopen test.

LS

[Jakub Hrozek]

On Fri, Apr 25, 2014 at 11:56:38AM +0200, Lukas Slebodnik wrote:
> > > Could we push Benjamin's patch to 1.11 branch?
> > >
> > > LS
> >
> > Sure, did you only want to push Benjamin's patch
> > (717008c8c3f29f3a1a77266cc72a6cfa616bf295 in master) or the whole set?
>
> just Benjamin's patch. Other patches will probably not apply to 1.11 branch
> and they were necessary mostly for dlopen test.

OK, no problem. Pushed to sssd-1-11:
    917ff0aa43d3858df1b82f75608601622c12ba57

[Lukas Slebodnik]

On (28/04/14 22:31), Jakub Hrozek wrote:
> On Fri, Apr 25, 2014 at 11:56:38AM +0200, Lukas Slebodnik wrote:
> > > > Could we push Benjamin's patch to 1.11 branch?
> > > >
> > > > LS
> > >
> > > Sure, did you only want to push Benjamin's patch
> > > (717008c8c3f29f3a1a77266cc72a6cfa616bf295 in master) or the whole set?
> >
> > just Benjamin's patch. Other patches will probably not apply to 1.11 branch
> > and they were necessary mostly for dlopen test.
>
> OK, no problem. Pushed to sssd-1-11:
>    917ff0aa43d3858df1b82f75608601622c12ba57

Thank you

LS