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

[Frediano Ziglio]

> 
> 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). »
> 

Fine with me, CI is happy too, see https://gitlab.freedesktop.org/fziglio/spice-gtk/pipelines/9252

Acked with the change.

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

Yes, I think on spice-server we moved from dynamic to static and I noted
this issue, here you are right, was already static linking.

Frediano