[Spice-devel] [spice-gtk] build: Remove need for static linking for tests

[Christophe Fergeau]

Hey,

On Wed, Nov 21, 2018 at 06:21:40AM -0500, Frediano Ziglio wrote:
> > 
> > Currently, spice-gtk tests are disabled if build of static libraries is
> > disabled. This commit introduces an intermediate
> > spice-client-glib-impl.la convenience library which does not have its
> > internal symbols hidden, and can thus be used instead of static linking
> > when building the tests.
> 
> Well, symbols are still hidden, but in this case they are exported as
> hidden by the convenience library which is a static library so you
> are using static linking, just that you are statically linking the
> convenience library.
> 
> > The installed spice-client-glib-2.0.la library is just this library with
> > the version script applied to hide non exported symbols.
> > 
> 
> Is just that hidden symbols are not exported by shared objects so
> when you link the convenience library (which is a static library of
> shared objects meant to be used in shared libraries!) into the shared
> object these symbols are not exported.
> spice-client-glib-2.0.la library is not the convenience library, just
> has the same content, a shared library has a completely different
> format.
> 
> Not against this patch, just the comment is really confusing.

Yep, in short I totally forgot spice-gtk uses G_GNUC_INTERNAL, if it did
not, having a shared or static convenience library would not matter.
I'll improve the log:

« Currently, spice-gtk tests are disabled if build of static libraries is
disabled. This commit introduces an intermediate
spice-client-glib-impl.la convenience library which will be used both
for generating the spice-client-glib-2.0 shared library (after applying
the version script), and to link the tests with.
This is better than --enable-static as we don't need to rebuild each
object twice (static and shared). »

> 
> > This was inspired by similar work Fabiano Fidencio did in libosinfo:
> > https://www.redhat.com/archives/libosinfo/2018-November/msg00231.html
> > 
> > Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> > ---
> >  Makefile.am       |  6 +-----
> >  configure.ac      |  2 --
> >  src/Makefile.am   | 34 +++++++++++++++++++---------------
> >  tests/Makefile.am |  4 ++--
> >  4 files changed, 22 insertions(+), 24 deletions(-)
[snip]
> > 
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index bfa43a37..1bb0a259 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -30,10 +30,10 @@ AM_CPPFLAGS =					\
> >  	-DG_LOG_DOMAIN=\"GSpice\"		\
> >  	$(NULL)
> >  
> > -AM_LDFLAGS = $(GIO_LIBS) -static
> > +AM_LDFLAGS = $(GIO_LIBS)
> >  
> >  LDADD =							\
> > -	$(top_builddir)/src/libspice-client-glib-2.0.la	\
> > +	$(top_builddir)/src/libspice-client-glib-impl.la\
> >  	$(NULL)
> >  
> >  test_util_SOURCES = util.c
> 
> I think we use the same "trick" also for spice-server.

I'd have to check...

> A minor issue of this method is that linking tests is much longer
> and resulting executables are really huge. This as each tests will
> include all the library code and data. You probably don't notice
> much are the number of tests is pretty low.
> To this issue I have no much to suggest. Probably tests that could
> be linked to the shared library should be linked to the shared
> library instead of the convenience one. 

I checked a build with/without the patch, and I only see negligible size
increases (measured to 0.1% on one of the binaries). We were already
doing static builds before the patch (see the AM_LDFLAGS change which
removes -static).

Christophe

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20181121/1d96d3bc/attachment.sig>