[sssd PR#502][opened] BUILD: Add missing libs found by -Wl,-z,defs

[lslebodn]

   URL: https://github.com/SSSD/sssd/pull/502
Author: lslebodn
 Title: #502: BUILD: Add missing libs found by -Wl,-z,defs
Action: opened

PR body:
"""
    It is not possible to fully build sssd with -Wl,-z,defs
    because we are using sssd_be as a "library" in some cases
    
    e.g.
    src/providers/krb5/.libs/libsss_krb5_common_la-krb5_init_shared.o: In function
`krb5_child_init':
    src/providers/krb5/krb5_init_shared.c:38: undefined reference to
`_dp_opt_get_bool'
    src/providers/krb5/krb5_init_shared.c:47: undefined reference to
`_dp_opt_get_string'
    src/providers/krb5/krb5_init_shared.c:94: undefined reference to
`_dp_opt_get_cstring'
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/502/head:pr502
git checkout pr502

[sssd PR#502][comment] BUILD: Add missing libs found by -Wl,-z,defs

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/502
Title: #502: BUILD: Add missing libs found by -Wl,-z,defs

jhrozek commented:
"""
How should I test this? I tried to revert (err, comment out) the sed that removes the -z
flag from linker flags, then apply the two patches, but the fedora build on rawhide still
failed with:
```
src/util/.libs/child_common.o: In function `write_pipe_handler':
/builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:389: undefined reference to
`sss_atomic_io_s'
src/util/.libs/child_common.o: In function `read_pipe_handler':
/builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:473: undefined reference to
`sss_atomic_io_s'
src/util/.libs/child_common.o: In function `sss_sigchld_init':
/builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:73: undefined reference to
`sss_hash_create'
/builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:82: undefined reference to
`BlockSignals'
src/util/.libs/child_common.o: In function `child_debug_init':
/builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:818: undefined reference to
`sss_strerror'
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:11039: libsss_child.la] Error
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/502#issuecomment-359913557

[sssd PR#502][comment] BUILD: Add missing libs found by -Wl,-z,defs

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/502
Title: #502: BUILD: Add missing libs found by -Wl,-z,defs

jhrozek commented:
"""
btw here is the specfile diff:
```
@@ -31,7 +31,7 @@
 
 Name: sssd
 Version: 1.16.0
-Release: 7%{?dist}
+Release: 7%{?dist}.2
 Group: Applications/System
 Summary: System Security Services Daemon
 License: GPLv3+
@@ -125,6 +125,9 @@ Patch0504: 0504-KCM-temporary-increase-hardcoded-buffers.patch
 Patch0505: 0505-libnfsidmap-Use-public-plugin-header-file-if-availab.patch
 Patch0506: 0506-dyndns_tests-Fix-unit-test-with-missing-features-in-.patch
 
+Patch1000: 1000-BUILD-Add-missing-libs-found-by-Wl-z-defs.patch
+Patch1001: 1001-BUILD-Fix-using-of-libdlopen_test_providers.so-in-te.patch
+
 ### Dependencies ###
 
 Requires: sssd-common = %{version}-%{release}
@@ -684,7 +687,7 @@ done
 %build
 autoreconf -ivf
 
-export LDFLAGS="$(echo %{__global_ldflags} | sed -e
's/-Wl,-z,defs//')"
+#export LDFLAGS="$(echo %{__global_ldflags} | sed -e
's/-Wl,-z,defs//')"
 
 %configure \
     --with-test-dir=/dev/shm \
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/502#issuecomment-359913881

[sssd PR#502][comment] BUILD: Add missing libs found by -Wl,-z,defs

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/502
Title: #502: BUILD: Add missing libs found by -Wl,-z,defs

lslebodn commented:
"""
On (23/01/18 12:05), Jakub Hrozek wrote:
> How should I test this? I tried to revert (err, comment out) the sed
> that removes the -z flag from linker flags, then apply the two patches, but the fedora
> build on rawhide still failed with:
> ```
> src/util/.libs/child_common.o: In function `write_pipe_handler':
> /builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:389: undefined reference to
> `sss_atomic_io_s'
> src/util/.libs/child_common.o: In function `read_pipe_handler':
> /builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:473: undefined reference to
> `sss_atomic_io_s'
> src/util/.libs/child_common.o: In function `sss_sigchld_init':
> /builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:73: undefined reference to
> `sss_hash_create'
> /builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:82: undefined reference to
> `BlockSignals'
> src/util/.libs/child_common.o: In function `child_debug_init':
> /builddir/build/BUILD/sssd-1.16.0/src/util/child_common.c:818: undefined reference to
> `sss_strerror'
> collect2: error: ld returned 1 exit status
> make[2]: *** [Makefile:11039: libsss_child.la] Error
> ``` 
This patch does not try to fix all problems. Just problems which can be easily
fixed with adding missing libraries. This is just 1st step to reduce count
of warnings.

Full solution will need to remove cyclic dependencies between internal
libraries and somehow reorganise dependencies between symbols in sssd_be
and other plugins.

There is also an issue with winbind_idmap_sss.la. It calls smb_register_idmap
which is provided by same samba component. That's the reason why we need
libdlopen_test_winbind_idmap.la for dlopen test.
We might try to workaround that with finding required symbol on the fly
with dlsym. But better approach would be change approach in winbind.

Anyway,
back to your question: I basically tested with `make -j check` + double
checking whether missing symbols are really required by library `objdump -p ...`
and `objdump -T ...`

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/502#issuecomment-360409977

[sssd PR#502][comment] BUILD: Add missing libs found by -Wl,-z,defs

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/502
Title: #502: BUILD: Add missing libs found by -Wl,-z,defs

lslebodn commented:
"""
BTW there is also downstream bug
https://bugzilla.redhat.com/show_bug.cgi?id=1538997

Should I split this patch to few small patches? because it would be good to fix fix
BZ1538997 in rawhide.
"""

See the full comment at https://github.com/SSSD/sssd/pull/502#issuecomment-363131572

[sssd PR#502][comment] BUILD: Add missing libs found by -Wl,-z,defs

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/502
Title: #502: BUILD: Add missing libs found by -Wl,-z,defs

jhrozek commented:
"""
No, go ahead and push the patches. I don't pretend I checked all additions of all
symbols, but I spot checked a few and the changes look good. And the proxy link with -ldl
is quite clear.
"""

See the full comment at https://github.com/SSSD/sssd/pull/502#issuecomment-363142528

[sssd PR#502][+Accepted] BUILD: Add missing libs found by -Wl,-z,defs

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/502
Title: #502: BUILD: Add missing libs found by -Wl,-z,defs

Label: +Accepted

[sssd PR#502][closed] BUILD: Add missing libs found by -Wl,-z,defs

[lslebodn]

   URL: https://github.com/SSSD/sssd/pull/502
Author: lslebodn
 Title: #502: BUILD: Add missing libs found by -Wl,-z,defs
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/502/head:pr502
git checkout pr502

[sssd PR#502][comment] BUILD: Add missing libs found by -Wl,-z,defs

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/502
Title: #502: BUILD: Add missing libs found by -Wl,-z,defs

lslebodn commented:
"""
master:
* b61304a9f0f6bcc099cb855bc5bde13c4024da1a
* 7ad9f9c7c8b40e74647de455e454894a257bc0de
"""

See the full comment at https://github.com/SSSD/sssd/pull/502#issuecomment-363397754

[sssd PR#502][+Pushed] BUILD: Add missing libs found by -Wl,-z,defs

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/502
Title: #502: BUILD: Add missing libs found by -Wl,-z,defs

Label: +Pushed