Have `protocol_test` depend explicitly on libsystemd
On Debian, when I configure with argument --with-systemd-service=no, the unit test protocol_test fails to build with a link-time error:
/usr/bin/ld: cf-serverd-functions.o: undefined reference to symbol 'sd_notifyf@@LIBSYSTEMD_209'
/usr/bin/ld: /lib/x86_64-linux-gnu/libsystemd.so.0: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
This change makes protocol_test depend unconditionally on libsystemd. TBH I'm not sure this is the proper way to fix this; submitting my local fix for posterity's sake as much as anything else.
I think the issue is that you have libsystemd-dev installed and so configure.ac detects that availability and sets HAVE_SD_NOTIFY_BARRIER but... the Makefile.am files that need -lsystemd don't have the proper inclusion as I have done below.
Our github action "unit_test" is failing because we don't install libsystemd-dev and so it is not available.
Can you try out this diff instead of your fix?
diff --git a/cf-serverd/Makefile.am b/cf-serverd/Makefile.am
index b2b268d..9739ff8 100644
--- a/cf-serverd/Makefile.am
+++ b/cf-serverd/Makefile.am
@@ -35,6 +35,8 @@ AM_CFLAGS = \
$(PTHREAD_CFLAGS) \
$(ENTERPRISE_CFLAGS)
+LIBS += $(SYSTEMD_SOCKET_LIBS)
+
libcf_serverd_la_LIBADD = ../libpromises/libpromises.la
libcf_serverd_la_SOURCES = \
diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index 6a72dce..44ba5a1 100644
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -45,6 +45,8 @@ LDADD = ../../libpromises/libpromises.la libtest.la
# implementation, then we are pretty sure that they will be overriden by
# our local implementation. So we include *everything*...
LIBS = $(CORE_LIBS)
+# protocol_test relies on cf-serverd-functions which can rely on libsystemd so need SYSTEMD_SOCKET_LIBS here
+LIBS += $(SYSTEMD_SOCKET_LIBS)
AM_LDFLAGS = $(CORE_LDFLAGS)
AM_CFLAGS = $(PTHREAD_CFLAGS)
Yes-- that works for me, @craigcomstock; want me to cut a new PR/update this one?
Yes-- that works for me, @craigcomstock; want me to cut a new PR/update this one?
Yes, please do update this PR. I expect the checks will pass and we can merge. Thanks!
Updated, @craigcomstock -- thanks for the help.
Goodness @sp1ff looks like we let this one get rather stale. How's it going? I will try and re-run the github checks and see where we are at by looking at the current change set.
@cf-bottom jenkins please :) @sp1ff this will cause a pretty thorough run on our internal CI system and should give us an idea where we are at. I suppose you could push an empty commit here to bump github actions into re-running.
Sure, I triggered a build:
Jenkins: https://ci.cfengine.com/job/pr-pipeline/9633/
Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-9633/
looks like our canaries: rhel7 hub and agent and ubuntu 20 all failed with the current PR
11:20:45 CCLD mon_load_test
11:20:45 /usr/bin/ld: cannot find -lsystemd
11:20:45 collect2: error: ld returned 1 exit status
11:20:45 gmake[4]: *** [protocol_test] Error 1
I'll take a look when I can and see if I can fix this up.
Apologies, @craigcomstock ... life got busy. I rebased this PR & pushed it up-- let's see what happens now.
Apologies, @craigcomstock ... life got busy. I rebased this PR & pushed it up-- let's see what happens now.
No worries! Thanks so much for coming back and trying some more.
@sp1ff looks like the push 5 days ago still doesn't pass tests. I have created a ticket: https://northerntech.atlassian.net/browse/CFE-4274 and assigned it to myself so we can get this done. Thanks for the continued attention here. Appreciate it.
@sp1ff I looked at this for a bit today. I started at the beginning and it seems like the "easiest" workaround is to use
./autogen.sh --with-systemd-service=no --with-systemd-socket=no
It seems our configure.ac doesn't coordinate things quite enough. I will research wether it makes sense to assume --with-systemd-socket=no if --with-systemd-service=no. Seems reasonable but want to make sure.
Thanks for your patience and hope you are well.
Looking at this a bit more I think we have two separate items, from configure --help:
--with-systemd-socket[=PATH]
support systemd socket activation
and
--with-systemd-service=PATH
Install systemd service file in given path. The
default is no, but if specified, the default path is
/usr/lib/systemd/system.
I could see wanting systemd service and not the systemd socket. I can't think of a reason we would want systemd socket support without systemd services. I will make this dependency in configure.ac and hopefully this will make it easier to handle.
if --with-systemd-socket=yes then force --with-systemd-service=yes if --with-systemd-service=no then force --with-systemd-socket=no
Of course this also assumes that libsystemd-dev package or similar is available if you choose systemd stuff.
The problem for you was that the --with-systemd-socket defaults to yes regardless of presence of libsystemd-dev or --with-system-service=no.
didn't want to squash your work here or our conversation so created a new PR with my idea. Will see if it sticks. :)
https://github.com/cfengine/core/pull/5425
@craigcomstock thanks for taking a look!
@sp1ff can you try out the changes I made in https://github.com/cfengine/core/pull/5425 ? They are passing CI and I think they should "work" in your case. If so we can merge that one and close this one. :) Thanks again for the contribution.
Sure-- will give it a try this evening.
Craig Comstock @.***> writes:
@sp1ff can you try out the changes I made in #5425 ? They are passing CI and I think they should "work" in your case. If so we can merge that one and close this one. :) Thanks again for the contribution.
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.
-- Michael @.***>
@craigcomstock wfm! Thank you very much.
@craigcomstock wfm! Thank you very much.
Awesome. The fix is merged in master and our maintenance branches: 3.18.x and 3.21.x.
Be well and thanks again for the notice and contribution.