[libvirt] [PATCH] tests: Avoid double linking some libraries

The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely: CCLD domaincapstest ../src/libvirt_probes.o:(.probes+0x0): multiple definition of `libvirt_event_poll_add_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x0): first defined here ../src/libvirt_probes.o:(.probes+0x2): multiple definition of `libvirt_event_poll_update_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x2): first defined here ../src/libvirt_probes.o:(.probes+0x4): multiple definition of `libvirt_event_poll_remove_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x4): first defined here ../src/libvirt_probes.o:(.probes+0x6): multiple definition of `libvirt_event_poll_dispatch_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x6): first defined here And so on. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- I'd push this as build breaker, but there's another approach to consider, create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS) What's your preference? tests/Makefile.am | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index a262c7b..b9a6345 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -509,12 +509,11 @@ endif WITH_STORAGE if WITH_DTRACE_PROBES qemu_LDADDS += ../src/libvirt_qemu_probes.lo endif WITH_DTRACE_PROBES -qemu_LDADDS += $(LDADDS) qemuxml2argvtest_SOURCES = \ qemuxml2argvtest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LIBXML_LIBS) +qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LDADDS) $(LIBXML_LIBS) qemuxml2argvmock_la_SOURCES = \ qemuxml2argvmock.c @@ -525,62 +524,64 @@ qemuxml2argvmock_la_LDFLAGS = -module -avoid-version \ qemuxml2xmltest_SOURCES = \ qemuxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2xmltest_LDADD = $(qemu_LDADDS) +qemuxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuxmlnstest_SOURCES = \ qemuxmlnstest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxmlnstest_LDADD = $(qemu_LDADDS) +qemuxmlnstest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuargv2xmltest_SOURCES = \ qemuargv2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuargv2xmltest_LDADD = $(qemu_LDADDS) +qemuargv2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuhelptest_SOURCES = qemuhelptest.c testutils.c testutils.h -qemuhelptest_LDADD = $(qemu_LDADDS) +qemuhelptest_LDADD = $(qemu_LDADDS) $(LDADDS) qemumonitortest_SOURCES = qemumonitortest.c testutils.c testutils.h -qemumonitortest_LDADD = $(qemu_LDADDS) +qemumonitortest_LDADD = $(qemu_LDADDS) $(LDADDS) qemumonitorjsontest_SOURCES = \ qemumonitorjsontest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemumonitorjsontest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemumonitorjsontest_LDADD = libqemumonitortestutils.la \ + $(qemu_LDADDS) $(LDADDS) qemucapabilitiestest_SOURCES = \ qemucapabilitiestest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemucapabilitiestest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemucapabilitiestest_LDADD = libqemumonitortestutils.la \ + $(qemu_LDADDS) $(LDADDS) qemucaps2xmltest_SOURCES = \ qemucaps2xmltest.c \ testutils.c testutils.h \ $(NULL) -qemucaps2xmltest_LDADD = $(qemu_LDADDS) +qemucaps2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuagenttest_SOURCES = \ qemuagenttest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) qemuhotplugtest_SOURCES = \ qemuhotplugtest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) domainsnapshotxml2xmltest_SOURCES = \ domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) +domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) else ! WITH_QEMU EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemuxmlnstest.c qemuhelptest.c domainsnapshotxml2xmltest.c \ @@ -837,7 +838,7 @@ domaincapstest_LDADD = $(LDADDS) if WITH_QEMU domaincapstest_SOURCES += testutilsqemu.c testutilsqemu.h -domaincapstest_LDADD += $(qemu_LDADDS) +domaincapstest_LDADD += $(qemu_LDADDS) $(GNULIB_LIBS) endif WITH_QEMU if WITH_LIBVIRTD -- 1.8.5.5

On 07/03/2014 08:34 AM, Michal Privoznik wrote:
The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely:
CCLD domaincapstest ../src/libvirt_probes.o:(.probes+0x0): multiple definition of `libvirt_event_poll_add_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x0): first defined here ../src/libvirt_probes.o:(.probes+0x2): multiple definition of `libvirt_event_poll_update_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x2): first defined here ../src/libvirt_probes.o:(.probes+0x4): multiple definition of `libvirt_event_poll_remove_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x4): first defined here ../src/libvirt_probes.o:(.probes+0x6): multiple definition of `libvirt_event_poll_dispatch_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x6): first defined here
And so on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
I'd push this as build breaker, but there's another approach to consider, create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS)
What's your preference?
What is below works for me - so to that degree I'd ACK. However, if someone had a preference for whatever it is you're describing as an option, then I suppose that would be fine too. Makefile's are mostly black magic to me :-) John
tests/Makefile.am | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am index a262c7b..b9a6345 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -509,12 +509,11 @@ endif WITH_STORAGE if WITH_DTRACE_PROBES qemu_LDADDS += ../src/libvirt_qemu_probes.lo endif WITH_DTRACE_PROBES -qemu_LDADDS += $(LDADDS)
qemuxml2argvtest_SOURCES = \ qemuxml2argvtest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LIBXML_LIBS) +qemuxml2argvtest_LDADD = $(qemu_LDADDS) $(LDADDS) $(LIBXML_LIBS)
qemuxml2argvmock_la_SOURCES = \ qemuxml2argvmock.c @@ -525,62 +524,64 @@ qemuxml2argvmock_la_LDFLAGS = -module -avoid-version \ qemuxml2xmltest_SOURCES = \ qemuxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2xmltest_LDADD = $(qemu_LDADDS) +qemuxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS)
qemuxmlnstest_SOURCES = \ qemuxmlnstest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxmlnstest_LDADD = $(qemu_LDADDS) +qemuxmlnstest_LDADD = $(qemu_LDADDS) $(LDADDS)
qemuargv2xmltest_SOURCES = \ qemuargv2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuargv2xmltest_LDADD = $(qemu_LDADDS) +qemuargv2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS)
qemuhelptest_SOURCES = qemuhelptest.c testutils.c testutils.h -qemuhelptest_LDADD = $(qemu_LDADDS) +qemuhelptest_LDADD = $(qemu_LDADDS) $(LDADDS)
qemumonitortest_SOURCES = qemumonitortest.c testutils.c testutils.h -qemumonitortest_LDADD = $(qemu_LDADDS) +qemumonitortest_LDADD = $(qemu_LDADDS) $(LDADDS)
qemumonitorjsontest_SOURCES = \ qemumonitorjsontest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemumonitorjsontest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemumonitorjsontest_LDADD = libqemumonitortestutils.la \ + $(qemu_LDADDS) $(LDADDS)
qemucapabilitiestest_SOURCES = \ qemucapabilitiestest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemucapabilitiestest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemucapabilitiestest_LDADD = libqemumonitortestutils.la \ + $(qemu_LDADDS) $(LDADDS)
qemucaps2xmltest_SOURCES = \ qemucaps2xmltest.c \ testutils.c testutils.h \ $(NULL) -qemucaps2xmltest_LDADD = $(qemu_LDADDS) +qemucaps2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS)
qemuagenttest_SOURCES = \ qemuagenttest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemuagenttest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS)
qemuhotplugtest_SOURCES = \ qemuhotplugtest.c \ testutils.c testutils.h \ testutilsqemu.c testutilsqemu.h \ $(NULL) -qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) +qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS)
domainsnapshotxml2xmltest_SOURCES = \ domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) +domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) else ! WITH_QEMU EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemuxmlnstest.c qemuhelptest.c domainsnapshotxml2xmltest.c \ @@ -837,7 +838,7 @@ domaincapstest_LDADD = $(LDADDS)
if WITH_QEMU domaincapstest_SOURCES += testutilsqemu.c testutilsqemu.h -domaincapstest_LDADD += $(qemu_LDADDS) +domaincapstest_LDADD += $(qemu_LDADDS) $(GNULIB_LIBS) endif WITH_QEMU
if WITH_LIBVIRTD

On 07/03/2014 08:44 AM, John Ferlan wrote:
On 07/03/2014 08:34 AM, Michal Privoznik wrote:
The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely:
CCLD domaincapstest ../src/libvirt_probes.o:(.probes+0x0): multiple definition of `libvirt_event_poll_add_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x0): first defined here ../src/libvirt_probes.o:(.probes+0x2): multiple definition of `libvirt_event_poll_update_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x2): first defined here ../src/libvirt_probes.o:(.probes+0x4): multiple definition of `libvirt_event_poll_remove_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x4): first defined here ../src/libvirt_probes.o:(.probes+0x6): multiple definition of `libvirt_event_poll_dispatch_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x6): first defined here
And so on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
I'd push this as build breaker, but there's another approach to consider, create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS)
What's your preference?
What is below works for me - so to that degree I'd ACK. However, if someone had a preference for whatever it is you're describing as an option, then I suppose that would be fine too. Makefile's are mostly black magic to me :-)
John
Let me amend my last statement - something is not quite working... I tried to apply this patch to other work I'm doing and things fell apart with missing symbols. If I try to reset my environment back to the top of the tree, then the build seems to work fine... If I apply this patch, then things fall apart in the same way. It's kind of difficult to describe, but there seems to be some dependency that isn't quite right or doesn't get reset if this patch is removed from the environment. I think the following steps make things reproducible. 1. Start with a "clean" top... 2. Build, will get the multiple definition failure... 3. Apply this patch (git am) 4. Build - get lots of failures 5. git reset HEAD^ 6. git co tests/Makefile.am 7. Build cleanly 8. git am the first change from my series 9. Build, results from step2 10. Repeat steps 3->7 for each/all of the patches from my series. I'm still wondering why steps 3 & 4 worked for me before - perhaps because I was testing something intermediary. Like I said before these Makefiles are black magic to me! John

On 07/03/2014 12:47 PM, John Ferlan wrote:
On 07/03/2014 08:44 AM, John Ferlan wrote:
On 07/03/2014 08:34 AM, Michal Privoznik wrote:
The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely:
CCLD domaincapstest ../src/libvirt_probes.o:(.probes+0x0): multiple definition of `libvirt_event_poll_add_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x0): first defined here ../src/libvirt_probes.o:(.probes+0x2): multiple definition of `libvirt_event_poll_update_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x2): first defined here ../src/libvirt_probes.o:(.probes+0x4): multiple definition of `libvirt_event_poll_remove_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x4): first defined here ../src/libvirt_probes.o:(.probes+0x6): multiple definition of `libvirt_event_poll_dispatch_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x6): first defined here
And so on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
I'd push this as build breaker, but there's another approach to consider, create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS)
What's your preference?
What is below works for me - so to that degree I'd ACK. However, if someone had a preference for whatever it is you're describing as an option, then I suppose that would be fine too. Makefile's are mostly black magic to me :-)
John
Let me amend my last statement - something is not quite working...
I tried to apply this patch to other work I'm doing and things fell apart with missing symbols. If I try to reset my environment back to the top of the tree, then the build seems to work fine... If I apply this patch, then things fall apart in the same way.
It's kind of difficult to describe, but there seems to be some dependency that isn't quite right or doesn't get reset if this patch is removed from the environment.
I think the following steps make things reproducible.
1. Start with a "clean" top... 2. Build, will get the multiple definition failure... 3. Apply this patch (git am) 4. Build - get lots of failures 5. git reset HEAD^ 6. git co tests/Makefile.am 7. Build cleanly 8. git am the first change from my series 9. Build, results from step2 10. Repeat steps 3->7 for each/all of the patches from my series.
I'm still wondering why steps 3 & 4 worked for me before - perhaps because I was testing something intermediary.
Like I said before these Makefiles are black magic to me!
John
The following works for me: $ git diff tests/Makefile.am diff --git a/tests/Makefile.am b/tests/Makefile.am index a262c7b..2441742 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -833,12 +833,12 @@ vircaps2xmltest_LDADD = $(LDADDS) domaincapstest_SOURCES = \ domaincapstest.c testutils.h testutils.c -domaincapstest_LDADD = $(LDADDS) - if WITH_QEMU domaincapstest_SOURCES += testutilsqemu.c testutilsqemu.h -domaincapstest_LDADD += $(qemu_LDADDS) -endif WITH_QEMU +domaincapstest_LDADD = $(qemu_LDADDS) +else ! WITH_QEMU +domaincapstest_LDADD = $(LDADDS) +endif ! WITH_QEMU if WITH_LIBVIRTD libvirtdconftest_SOURCES = \ $ Not sure if it's the "right" solution, but I'm at least able to avoid the cycle of apply, build, remove, build. John

On 03.07.2014 20:34, John Ferlan wrote:
On 07/03/2014 12:47 PM, John Ferlan wrote:
On 07/03/2014 08:44 AM, John Ferlan wrote:
On 07/03/2014 08:34 AM, Michal Privoznik wrote:
The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely:
CCLD domaincapstest ../src/libvirt_probes.o:(.probes+0x0): multiple definition of `libvirt_event_poll_add_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x0): first defined here ../src/libvirt_probes.o:(.probes+0x2): multiple definition of `libvirt_event_poll_update_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x2): first defined here ../src/libvirt_probes.o:(.probes+0x4): multiple definition of `libvirt_event_poll_remove_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x4): first defined here ../src/libvirt_probes.o:(.probes+0x6): multiple definition of `libvirt_event_poll_dispatch_handle_semaphore' ../src/libvirt_probes.o:(.probes+0x6): first defined here
And so on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
I'd push this as build breaker, but there's another approach to consider, create qemu_BARE_LDADDS and fill it with the qemu_impl, etc. Then, let: qemu_LDADDS = $(qemu_BARE_LDADDS) $(LDADDS)
What's your preference?
What is below works for me - so to that degree I'd ACK. However, if someone had a preference for whatever it is you're describing as an option, then I suppose that would be fine too. Makefile's are mostly black magic to me :-)
John
Let me amend my last statement - something is not quite working...
I tried to apply this patch to other work I'm doing and things fell apart with missing symbols. If I try to reset my environment back to the top of the tree, then the build seems to work fine... If I apply this patch, then things fall apart in the same way.
It's kind of difficult to describe, but there seems to be some dependency that isn't quite right or doesn't get reset if this patch is removed from the environment.
I think the following steps make things reproducible.
1. Start with a "clean" top... 2. Build, will get the multiple definition failure... 3. Apply this patch (git am) 4. Build - get lots of failures 5. git reset HEAD^ 6. git co tests/Makefile.am 7. Build cleanly 8. git am the first change from my series 9. Build, results from step2 10. Repeat steps 3->7 for each/all of the patches from my series.
I'm still wondering why steps 3 & 4 worked for me before - perhaps because I was testing something intermediary.
Like I said before these Makefiles are black magic to me!
John
The following works for me:
$ git diff tests/Makefile.am diff --git a/tests/Makefile.am b/tests/Makefile.am index a262c7b..2441742 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -833,12 +833,12 @@ vircaps2xmltest_LDADD = $(LDADDS)
domaincapstest_SOURCES = \ domaincapstest.c testutils.h testutils.c -domaincapstest_LDADD = $(LDADDS) - if WITH_QEMU domaincapstest_SOURCES += testutilsqemu.c testutilsqemu.h -domaincapstest_LDADD += $(qemu_LDADDS) -endif WITH_QEMU +domaincapstest_LDADD = $(qemu_LDADDS) +else ! WITH_QEMU +domaincapstest_LDADD = $(LDADDS) +endif ! WITH_QEMU
if WITH_LIBVIRTD libvirtdconftest_SOURCES = \ $
Not sure if it's the "right" solution, but I'm at least able to avoid the cycle of apply, build, remove, build.
John
The problem is, I've unintentionally omitted one qemu_LDADDS change: @@ -1055,7 +1056,7 @@ if WITH_QEMU securityselinuxlabeltest_SOURCES = \ securityselinuxlabeltest.c testutils.h testutils.c \ testutilsqemu.h testutilsqemu.c -securityselinuxlabeltest_LDADD = $(qemu_LDADDS) $(SELINUX_LIBS) +securityselinuxlabeltest_LDADD = $(qemu_LDADDS) $(LDADDS) $(SELINUX_LIBS) securityselinuxlabeltest_DEPENDENCIES = libsecurityselinuxhelper.la \ ../src/libvirt.la endif WITH_QEMU With this everything should be working as expected. I'll rather post it as v2. Michal

On 07/03/2014 06:34 AM, Michal Privoznik wrote:
The problem is, since 614581f32b domaincapstest is linked with $(LDADDS) by default. Then, since 94e3f23e8a7 the test may be conditionally linked with $(qemu_LDADDS) which already contains $(LDADDS). And some linkers doesn't cope with this nicely:
I haven't reviewed this yet, but I'd like a shot at it. Give me 24 hours to respond (I'm first trying to push some of my acked patches as well as maint branch backports) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake
-
John Ferlan
-
Michal Privoznik