Created attachment 15812 [details] Patch to wscript to fix OmniOS linking again A change in the "wscript" from earlier version breaks OmniOS linking. See attached patch that reverts the change (and fixes the problem).
This fix is still needed to build Samba 4.14.0rc4 on latest OmniOS. Output from bin/config.log without it: > ['gcc', '-lreadline', '-lncurses', '-lnsl', '-lsocket', '-Wl,--as-needed', 'test.c.1.o', '-o/Users/peter86/Build/samba/samba-4.14.0rc4/bin/.conf_check_33fa872a3523539c06934d3ea23c1f54/testbuild/default/testprog', '-Wl,-Bstatic', '-Wl,-Bdynamic', '-L/usr/local/lib', '-L/usr/local/lib', '-lc'] > err: ld: fatal: unrecognized option '--as-needed' > ld: fatal: use the -z help option for usage information
this wscript line is also working correctly for you? : conf.ADD_LDFLAGS('-Wl,--as-needed', testflags=True)
Yes, that seems to work also. # conf.add_as_needed() # conf.env.asneeded_ldflags = conf.ADD_LDFLAGS('-Wl,--as-needed', testflags=True) conf.ADD_LDFLAGS('-Wl,--as-needed', testflags=True) (Then compilation fails later on in source3/modules/vfs_solarisacls.c - but that’s a complete other issue.) - Peter > On 2 Mar 2021, at 23:16, samba-bugs@samba.org wrote: > > https://bugzilla.samba.org/show_bug.cgi?id=14288 > > --- Comment #2 from Björn Jacke <bjacke@samba.org> --- > this wscript line is also working correctly for you? : > conf.ADD_LDFLAGS('-Wl,--as-needed', testflags=True) > > -- > You are receiving this mail because: > You reported the bug.
This bug was referenced in samba master: 996560191ac6bd603901dcd6c0de5d239e019ef4
I've reported the waf issue upsteam: https://gitlab.com/ita1024/waf/-/issues/2342
Created attachment 16494 [details] backported fix for 4.14, 4.13, 4.12
(In reply to Björn Jacke from comment #6) Pushed to autobuild-v4-{14,13,12}-test.
This bug was referenced in samba v4-13-test: eebf510fbd8847077c7bec72a1cda674b5a02714
This bug was referenced in samba v4-12-test: c63f00801cae16a855aa5283fa0cc449e84577ce
This bug was referenced in samba v4-14-test: 4d1ed9c319deac5cba1682611dcefdf002cb9d48
Does Samba actually work well with this patch? I'm concerned that Samba might build but then just fail per bug 11355 instead.
Sadly this has caused a regression as --as-needed is now omitted on Linux. The issue here is that conf.ADD_LDFLAGS() modified EXTRA_LDFLAGS while conf.add_as_needed() changes LINKFLAGS. I forgot to check the build output before after.
(In reply to Andrew Bartlett from comment #12) So this is a showstopper for Samba 4.14.0?
(In reply to Andrew Bartlett from comment #12) --as-needed isn't omitted in the case I'm seeing. The problem is with CTDB's reqid_test, which uses this build rule in ctdb/wscript: bld.SAMBA_BINARY('reqid_test', source='tests/src/reqid_test.c', deps='samba-util', install_path='${CTDB_TEST_LIBEXECDIR}') The installation link command looks like: [4694/6851] Linking bin/default/ctdb/reqid_test.inst 20:16:55 runner ['/usr/lib64/ccache/gcc', 'ctdb/tests/src/reqid_test.c.181.o', '-o/root/samba-4.15.0pre1-GIT-b145434/bin/default/ctdb/reqid _test.inst', '-Wl,-rpath,/usr/local/samba/lib', '-Wl,-Bstatic', '-Wl,-Bdynamic', '-L/root/samba-4.15.0pre1-GIT-b145434/bin/default/lib/repl ace', '-L/root/samba-4.15.0pre1-GIT-b145434/bin/default/lib/talloc', '-L/root/samba-4.15.0pre1-GIT-b145434/bin/default/lib/tevent', '-L/roo t/samba-4.15.0pre1-GIT-b145434/bin/default/lib/util', '-L/usr/local/lib', '-L/usr/local/lib', '-lsamba-util', '-lgenrand-samba4', '-lsocket -blocking-samba4', '-lutil-setid-samba4', '-ltevent', '-lsamba-debug-samba4', '-ltime-basic-samba4', '-lsys-rw-samba4', '-ltalloc', '-lrepl ace-samba4', '-liov-buf-samba4', '-licui18n', '-lgnutls', '-licudata', '-licuuc', '-lsystemd', '-lunwind-generic', '-lunwind', '-lbsd', '-l dl', '-lpthread', '-pie', '-Wl,-z,relro,-z,now', '-Wl,-no-undefined', '-Wl,--export-dynamic', '-Wl,--as-needed'] It is missing -Wl,-rpath,/usr/local/samba/lib/private, needed to locate private libraries that are dependencies of samba-util: [root@testtest1 ~]# ldd /usr/local/samba/libexec/ctdb/tests/reqid_test linux-vdso.so.1 (0x00007fff261f6000) libsamba-util.so.0 => /usr/local/samba/lib/libsamba-util.so.0 (0x00007fc9d9970000) libgenrand-samba4.so => not found libsocket-blocking-samba4.so => not found libutil-setid-samba4.so => not found libtevent.so.0 => /lib64/libtevent.so.0 (0x00007fc9d975c000) libsamba-debug-samba4.so => not found libtime-basic-samba4.so => not found libsys-rw-samba4.so => not found libtalloc.so.2 => /lib64/libtalloc.so.2 (0x00007fc9d9547000) libreplace-samba4.so => not found libiov-buf-samba4.so => not found libicui18n.so.60 => /lib64/libicui18n.so.60 (0x00007fc9d9086000) libgnutls.so.30 => /lib64/libgnutls.so.30 (0x00007fc9d8c95000) ... If I add a direct dependency on, say, replace: bld.SAMBA_BINARY('reqid_test', source='tests/src/reqid_test.c', deps='samba-util replace', install_path='${CTDB_TEST_LIBEXECDIR}') then this works around the problem. So, for some reason the changed code still picks up all the dependencies but it stops directories for recursive dependencies being added to rpath. Looking at third_party/waf/waflib/Tools/c_config.py:add_as_needed(), I can't see why using this function causes anything to happen with rpath. It is also unclear why the failure happens on CentOS 7 but it doesn't happen on Debian testing.
(In reply to Karolin Seeger from comment #13) Good question. The short answer is that it causes a test regression when the CTDB tests are installed, on 1 platform that we know about. We're probably the only ones who do that. The real question is whether any other binaries suffer from the same problem is the CTDB reqid_test binary. It appears these binaries are affected: [root@testtbuild1 samba-4.15.0pre1-GIT-b145434]# for i in $(find bin/default/ -name "*.inst") ; do if ldd "$i" | grep -q "not found" ; then echo "$i"; fi; done | sort bin/default/ctdb/reqid_test.inst bin/default/examples/libsmbclient/testacl.inst bin/default/examples/libsmbclient/testacl2.inst bin/default/examples/libsmbclient/testacl3.inst bin/default/examples/libsmbclient/testbrowse.inst bin/default/examples/libsmbclient/testbrowse2.inst bin/default/examples/libsmbclient/testchmod.inst bin/default/examples/libsmbclient/testctx.inst bin/default/examples/libsmbclient/testfstatvfs.inst bin/default/examples/libsmbclient/testnotify.inst bin/default/examples/libsmbclient/testread.inst bin/default/examples/libsmbclient/testsmbc.inst bin/default/examples/libsmbclient/teststat.inst bin/default/examples/libsmbclient/teststat2.inst bin/default/examples/libsmbclient/teststat3.inst bin/default/examples/libsmbclient/teststatvfs.inst bin/default/examples/libsmbclient/testtruncate.inst bin/default/examples/libsmbclient/testutime.inst bin/default/examples/libsmbclient/testwrite.inst bin/default/nsswitch/stress-nss-libwbclient.inst bin/default/source3/lib/netapi/examples/dsgetdc/dsgetdc.inst bin/default/source3/lib/netapi/examples/file/file_close.inst bin/default/source3/lib/netapi/examples/file/file_enum.inst bin/default/source3/lib/netapi/examples/file/file_getinfo.inst bin/default/source3/lib/netapi/examples/getdc/getdc.inst bin/default/source3/lib/netapi/examples/group/group_add.inst bin/default/source3/lib/netapi/examples/group/group_adduser.inst bin/default/source3/lib/netapi/examples/group/group_del.inst bin/default/source3/lib/netapi/examples/group/group_deluser.inst bin/default/source3/lib/netapi/examples/group/group_enum.inst bin/default/source3/lib/netapi/examples/group/group_getinfo.inst bin/default/source3/lib/netapi/examples/group/group_getusers.inst bin/default/source3/lib/netapi/examples/group/group_setinfo.inst bin/default/source3/lib/netapi/examples/group/group_setusers.inst bin/default/source3/lib/netapi/examples/join/getjoinableous.inst bin/default/source3/lib/netapi/examples/join/getjoininformation.inst bin/default/source3/lib/netapi/examples/join/netdomjoin.inst bin/default/source3/lib/netapi/examples/join/rename_machine.inst bin/default/source3/lib/netapi/examples/localgroup/localgroup_add.inst bin/default/source3/lib/netapi/examples/localgroup/localgroup_addmembers.inst bin/default/source3/lib/netapi/examples/localgroup/localgroup_del.inst bin/default/source3/lib/netapi/examples/localgroup/localgroup_delmembers.inst bin/default/source3/lib/netapi/examples/localgroup/localgroup_enum.inst bin/default/source3/lib/netapi/examples/localgroup/localgroup_getinfo.inst bin/default/source3/lib/netapi/examples/localgroup/localgroup_getmembers.inst bin/default/source3/lib/netapi/examples/localgroup/localgroup_setinfo.inst bin/default/source3/lib/netapi/examples/localgroup/localgroup_setmembers.inst bin/default/source3/lib/netapi/examples/netlogon/netlogon_control.inst bin/default/source3/lib/netapi/examples/netlogon/netlogon_control2.inst bin/default/source3/lib/netapi/examples/netlogon/nltest.inst bin/default/source3/lib/netapi/examples/server/remote_tod.inst bin/default/source3/lib/netapi/examples/server/server_getinfo.inst bin/default/source3/lib/netapi/examples/share/share_add.inst bin/default/source3/lib/netapi/examples/share/share_del.inst bin/default/source3/lib/netapi/examples/share/share_enum.inst bin/default/source3/lib/netapi/examples/share/share_getinfo.inst bin/default/source3/lib/netapi/examples/share/share_setinfo.inst bin/default/source3/lib/netapi/examples/shutdown/shutdown_abort.inst bin/default/source3/lib/netapi/examples/shutdown/shutdown_init.inst bin/default/source3/lib/netapi/examples/user/user_add.inst bin/default/source3/lib/netapi/examples/user/user_chgpwd.inst bin/default/source3/lib/netapi/examples/user/user_del.inst bin/default/source3/lib/netapi/examples/user/user_dispinfo.inst bin/default/source3/lib/netapi/examples/user/user_enum.inst bin/default/source3/lib/netapi/examples/user/user_getgroups.inst bin/default/source3/lib/netapi/examples/user/user_getinfo.inst bin/default/source3/lib/netapi/examples/user/user_getlocalgroups.inst bin/default/source3/lib/netapi/examples/user/user_modalsget.inst bin/default/source3/lib/netapi/examples/user/user_modalsset.inst bin/default/source3/lib/netapi/examples/user/user_setgroups.inst bin/default/source3/lib/netapi/examples/user/user_setinfo.inst bin/default/source3/lib/netapi/tests/netapitest.inst bin/default/source3/smbd/notifyd/notifyd-tests.inst bin/default/source3/versiontest.inst All examples and tests? Showstoppers? Not sure...
Thanks, Martin! Still not 100% sure, so delaying the release(s). 4.13.5 is scheduled for tomorrow and also affected, so let's see what will happen until tomorrow...
With all due respect to our normal rules advising against a revert and all due respect to OmniOS users, I think we should *revert the change* so things work as they did previously on Linux until someone has the time and resources to get this working and tested on a multi-platform basis. Multi-platform build and linking bugs tend to be subtle and complex, I wouldn't suggest trying to come to a full fix on a deadline. Sorry,
This bug was referenced in samba v4-13-stable (Release samba-4.13.5): eebf510fbd8847077c7bec72a1cda674b5a02714
This bug was referenced in samba v4-14-stable (Release samba-4.14.0): 4d1ed9c319deac5cba1682611dcefdf002cb9d48
(In reply to Samba QA Contact from comment #18) but was reverted
Re-assigning to Andrew for further investigations.
(In reply to Karolin Seeger from comment #21) Lucky me! But more seriously, just in case it wasn't clear to the reporter and anyone else interested, we have reverted the change because of the releases that were due out that otherwise would have been blocked. Fixing this is clearly non-trivial, and any further patches are going to need clear proof that the Linux case is unchanged, both in terms of relevant parts of config.log or such, and in terms of the linking of the binaries which are output. As confirming this under CI is proving a challenge, we will need to be particularly careful.
(In reply to Andrew Bartlett from comment #22) One "quick-and-dirty" fix could be to only do this for the sys.platform == 'sunos5' case, since that's the only(? - as far as I know) platform (still in some use) where Gnu ld isn't used... (I know you've been reluctant to add OS-specific hacks before but that contains this fix to just this platform atleast :-) --- samba-4.14.0/wscript tors jan. 21 14:20:41 2021 +++ samba-4.14.0-liu/wscript ons mars 10 09:51:23 2021 @@ -340,7 +340,11 @@ # allows us to find problems on our development hosts faster. # It also results in faster load time. - conf.add_as_needed() + # Solaris based systems doesn't use gnu ld, and so no '--as-needed' + if sys.platform == 'sunos5': + conf.ADD_LDFLAGS('-Wl,--as-needed', testflags=True) + else: + conf.add_as_needed() if not conf.CHECK_NEED_LC("-lc not needed"): conf.ADD_LDFLAGS('-lc', testflags=False)
such a OS specific exceptio hack if definetely not what we want to start. The build regression on centos7 looks quite obviously like unclean waf build depencencies. A "proof that the Linux case is unchanged" looks like an impossible proof like the proof of the existence of god. We should find the wrong waf dependencies and fix that and reimplement the correct as-needed check again. I tried also adding the -z defs linker flags to our waf buiuld that we used to have before we switched to waf but that also resulted in various undefined symbol errors, that should actually not pop up.
(In reply to Björn Jacke from comment #24) We have often used this standard (that things that are meant to be unchanged are unchanged) for things like this. Often we compare config.h files or CPP output to ensure that what is suggested to be harmless changes to configure checks are indeed as harmless as they appear.
This bug was referenced in samba v4-12-stable (Release samba-4.12.12): c63f00801cae16a855aa5283fa0cc449e84577ce
(In reply to Björn Jacke from comment #24) > The build regression on centos7 looks quite obviously like unclean > waf build depencencies. [...] We should find the wrong waf dependencies > and fix that and reimplement the correct as-needed check again. I don't think this is true. The breakage involves private libraries that are part of samba-util (or, more precisely, private libraries that samba-util depends on). These recursive dependencies are all linked. However, as I previously commented, with the patch, the rpath magic for those recursive dependencies isn't properly handled.
This bug was referenced in samba master: ff1c3af603b47a7e8f9faad8d1c2e4a489559155
I've backported https://gitlab.com/samba-team/samba/-/merge_requests/1874 to v4-13-test and v4-14-test. CI pipelines are running now. I'll also wait until these branches pass my overnight runs on CentOS 8.
Created attachment 16588 [details] Patch for 4.14, 4.13 Pipelines passed for: * 4.14 @ https://gitlab.com/samba-team/devel/samba/-/pipelines/284810020 * 4.13 @ https://gitlab.com/samba-team/devel/samba/-/pipelines/284809855 Before I add reviewers, waiting for: * My overnight tests to confirm that these patches are OK on CentOS 8 * Peter Eriksson (bug submitter) to confirm that this patch fixes the build on OmniOS Thanks...
My tests on CentOS 8 pass. Peter, can you please confirm that this fixes the build on OmniOS? I'm hoping this can still make it into the next 4.14 release, which due about a week from now. Thanks...
(In reply to Martin Schwenke from comment #31) I can confirm that the patch works fine on OmniOS!
(In reply to Peter Eriksson from comment #32) Thanks!
Pushed to autobuild-v4-{14,13}-test.
This bug was referenced in samba v4-13-test: 56156a8fd5432728b3d0526bb3ac3165ab5ebc90
This bug was referenced in samba v4-14-test: 12bfc43006347ad6f775181528b872c0a968f8cd
Closing out bug report. Thanks!
This bug was referenced in samba v4-14-stable (Release samba-4.14.3): 12bfc43006347ad6f775181528b872c0a968f8cd
This bug was referenced in samba v4-13-stable: 56156a8fd5432728b3d0526bb3ac3165ab5ebc90