[libvirt] [PATCH] Revert "tests: Don't link mock libraries against libvirt and gnulib"
This reverts commit 6aa90452aa63cb1e1ffa84ff5f93f5873bf810a0. Turns out that not linking against libvirt and gnulib is okay for regular Linux (and FreeBSD) builds, but makes mingw very unhappy. .../virnetserverclientmock_la-virnetserverclientmock.o: In function `virNetSocketGetSELinuxContext': .../virnetserverclientmock.c:61: undefined reference to `rpl_strdup' .../libvirportallocatormock_la-virportallocatortest.o: In function `init_syms': .../virportallocatortest.c:61: undefined reference to `virFileClose' --- Pushed as build breaker. Bunny ears of shame will be deployed in the morning. tests/Makefile.am | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index fb862cd..c5986f0 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -946,6 +946,7 @@ virnetserverclientmock_la_SOURCES = \ virnetserverclientmock.c virnetserverclientmock_la_CFLAGS = $(AM_CFLAGS) virnetserverclientmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +virnetserverclientmock_la_LIBADD = $(GNULIB_LIBS) if WITH_GNUTLS virnettlscontexttest_SOURCES = \ @@ -1024,6 +1025,7 @@ libvirportallocatormock_la_SOURCES = \ virportallocatortest.c libvirportallocatormock_la_CFLAGS = $(AM_CFLAGS) -DMOCK_HELPER=1 libvirportallocatormock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +libvirportallocatormock_la_LIBADD = ../src/libvirt.la vircgrouptest_SOURCES = \ vircgrouptest.c testutils.h testutils.c @@ -1049,6 +1051,8 @@ virpcitest_LDADD = $(LDADDS) virpcimock_la_SOURCES = \ virpcimock.c virpcimock_la_CFLAGS = $(AM_CFLAGS) +virpcimock_la_LIBADD = $(GNULIB_LIBS) \ + ../src/libvirt.la virpcimock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) nodeinfomock_la_SOURCES = \ @@ -1064,6 +1068,8 @@ virnetdevtest_LDADD = $(LDADDS) virnetdevmock_la_SOURCES = \ virnetdevmock.c virnetdevmock_la_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS) +virnetdevmock_la_LIBADD = $(GNULIB_LIBS) \ + ../src/libvirt.la virnetdevmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) virrotatingfiletest_SOURCES = \ -- 2.5.0
On Wed, Feb 10, 2016 at 06:33:21PM +0100, Andrea Bolognani wrote:
This reverts commit 6aa90452aa63cb1e1ffa84ff5f93f5873bf810a0.
Turns out that not linking against libvirt and gnulib is okay for regular Linux (and FreeBSD) builds, but makes mingw very unhappy.
.../virnetserverclientmock_la-virnetserverclientmock.o: In function `virNetSocketGetSELinuxContext': .../virnetserverclientmock.c:61: undefined reference to `rpl_strdup' .../libvirportallocatormock_la-virportallocatortest.o: In function `init_syms': .../virportallocatortest.c:61: undefined reference to `virFileClose' --- Pushed as build breaker.
Bunny ears of shame will be deployed in the morning.
You should still be able to drop linkage of libvirt.la, just keep the gnulib bits. The gnulib addition is harmless, because it is a static library the linker will drop all functions which are not actually used by the mock.o, so it will end up being a no-op on Linux, and will only pull in rpl_strup on mingw32. We should never link libvirt.la to mock libs though. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, 2016-02-10 at 18:09 +0000, Daniel P. Berrange wrote:
This reverts commit 6aa90452aa63cb1e1ffa84ff5f93f5873bf810a0. Turns out that not linking against libvirt and gnulib is okay for regular Linux (and FreeBSD) builds, but makes mingw very unhappy. .../virnetserverclientmock_la-virnetserverclientmock.o: In function `virNetSocketGetSELinuxContext': .../virnetserverclientmock.c:61: undefined reference to `rpl_strdup' .../libvirportallocatormock_la-virportallocatortest.o: In function `init_syms': .../virportallocatortest.c:61: undefined reference to `virFileClose' --- Pushed as build breaker. Bunny ears of shame will be deployed in the morning. You should still be able to drop linkage of libvirt.la, just keep
On Wed, Feb 10, 2016 at 06:33:21PM +0100, Andrea Bolognani wrote: the gnulib bits. The gnulib addition is harmless, because it is a static library the linker will drop all functions which are not actually used by the mock.o, so it will end up being a no-op on Linux, and will only pull in rpl_strup on mingw32. We should never link libvirt.la to mock libs though.
Looks like it is complaining about virFileClose(), plus virFilePrint() in the full output, as well. So maybe linking against libvirt.la is needed after all? If there's no harm in that, how about defining MOCKLIBS_LDADD = \ $(GNULIB_LIBS) \ ../src/libvirt.la and using it for every mock library, like the existing $(MOCKLIB_LDFLAGS)? Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On Thu, Feb 11, 2016 at 09:54:45AM +0100, Andrea Bolognani wrote:
On Wed, 2016-02-10 at 18:09 +0000, Daniel P. Berrange wrote:
This reverts commit 6aa90452aa63cb1e1ffa84ff5f93f5873bf810a0. Turns out that not linking against libvirt and gnulib is okay for regular Linux (and FreeBSD) builds, but makes mingw very unhappy. .../virnetserverclientmock_la-virnetserverclientmock.o: In function `virNetSocketGetSELinuxContext': .../virnetserverclientmock.c:61: undefined reference to `rpl_strdup' .../libvirportallocatormock_la-virportallocatortest.o: In function `init_syms': .../virportallocatortest.c:61: undefined reference to `virFileClose' --- Pushed as build breaker. Bunny ears of shame will be deployed in the morning. You should still be able to drop linkage of libvirt.la, just keep
On Wed, Feb 10, 2016 at 06:33:21PM +0100, Andrea Bolognani wrote: the gnulib bits. The gnulib addition is harmless, because it is a static library the linker will drop all functions which are not actually used by the mock.o, so it will end up being a no-op on Linux, and will only pull in rpl_strup on mingw32. We should never link libvirt.la to mock libs though.
Looks like it is complaining about virFileClose(), plus virFilePrint() in the full output, as well. So maybe linking against libvirt.la is needed after all?
That's a bug in virportallocatortest.c that was introduced in 0ee90812. The init_syms() fnuction should *not* be calling VIR_FORCE_CLOSE - it must use close(), because this is the mock-override and so should not be using libvirt.so symbols.
If there's no harm in that, how about defining
MOCKLIBS_LDADD = \ $(GNULIB_LIBS) \ ../src/libvirt.la
and using it for every mock library, like the existing $(MOCKLIB_LDFLAGS)?
Nope as above, we should not be linking libvirt.la to the mock lib, because the mock lib is supposed to be replacing libvirt.la symbols Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, 2016-02-11 at 09:49 +0000, Daniel P. Berrange wrote:
On Thu, Feb 11, 2016 at 09:54:45AM +0100, Andrea Bolognani wrote:
On Wed, 2016-02-10 at 18:09 +0000, Daniel P. Berrange wrote:
This reverts commit 6aa90452aa63cb1e1ffa84ff5f93f5873bf810a0. Turns out that not linking against libvirt and gnulib is okay for regular Linux (and FreeBSD) builds, but makes mingw very unhappy. .../virnetserverclientmock_la-virnetserverclientmock.o: In function `virNetSocketGetSELinuxContext': .../virnetserverclientmock.c:61: undefined reference to `rpl_strdup' .../libvirportallocatormock_la-virportallocatortest.o: In function `init_syms': .../virportallocatortest.c:61: undefined reference to `virFileClose' --- Pushed as build breaker. Bunny ears of shame will be deployed in the morning. You should still be able to drop linkage of libvirt.la, just keep
On Wed, Feb 10, 2016 at 06:33:21PM +0100, Andrea Bolognani wrote: the gnulib bits. The gnulib addition is harmless, because it is a static library the linker will drop all functions which are not actually used by the mock.o, so it will end up being a no-op on Linux, and will only pull in rpl_strup on mingw32. We should never link libvirt.la to mock libs though.
Looks like it is complaining about virFileClose(), plus virFilePrint() in the full output, as well. So maybe linking against libvirt.la is needed after all?
That's a bug in virportallocatortest.c that was introduced in 0ee90812. The init_syms() fnuction should *not* be calling VIR_FORCE_CLOSE - it must use close(), because this is the mock-override and so should not be using libvirt.so symbols.
Okay, I will change it to use plain close().
If there's no harm in that, how about defining
MOCKLIBS_LDADD = \ $(GNULIB_LIBS) \ ../src/libvirt.la
and using it for every mock library, like the existing $(MOCKLIB_LDFLAGS)?
Nope as above, we should not be linking libvirt.la to the mock lib, because the mock lib is supposed to be replacing libvirt.la symbols
I will define $(MOCKLIBS_LDADD) to contain just $(GNULIB_LIBS) then, and make sure mock libraries link against it. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
participants (2)
-
Andrea Bolognani -
Daniel P. Berrange