Re: [PATCH] Revert "tests: Don't link mock libraries against libvirt and gnulib"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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:
> > > 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.
> > 
> > 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]