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

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

 



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

> 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@xxxxxxxxxx>
> ---
>  Makefile.am       |  6 +-----
>  configure.ac      |  2 --
>  src/Makefile.am   | 34 +++++++++++++++++++---------------
>  tests/Makefile.am |  4 ++--
>  4 files changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index e9dfbe4d..68810075 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1,11 +1,7 @@
>  ACLOCAL_AMFLAGS = -I m4
>  NULL =
>  
> -SUBDIRS = subprojects/spice-common src man po doc data tools
> -
> -if BUILD_TESTS
> -SUBDIRS += tests
> -endif
> +SUBDIRS = subprojects/spice-common src man po doc data tools tests
>  
>  if HAVE_INTROSPECTION
>  if WITH_VALA
> diff --git a/configure.ac b/configure.ac
> index e686d76a..69c03dab 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -571,8 +571,6 @@ AC_SUBST(SPICE_GTK_REQUIRES)
>  
>  m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES([yes])])
>  
> -AM_CONDITIONAL([BUILD_TESTS], [test x"$enable_static" = xyes])
> -
>  AC_OUTPUT([
>  Makefile
>  spice-client-glib-2.0.pc
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 1bb6f9bf..b876530c 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -44,6 +44,7 @@ acldir = $(ACL_HELPER_DIR)
>  acl_PROGRAMS = spice-client-glib-usb-acl-helper
>  endif
>  
> +noinst_LTLIBRARIES = libspice-client-glib-impl.la
>  lib_LTLIBRARIES = libspice-client-glib-2.0.la
>  
>  if WITH_GTK
> @@ -175,7 +176,9 @@ libspice_client_glib_2_0_la_LDFLAGS =	\
>  	$(GLIB_SYMBOLS_LDFLAGS)		\
>  	$(NULL)
>  
> -libspice_client_glib_2_0_la_LIBADD =					\
> +libspice_client_glib_2_0_la_LIBADD = libspice-client-glib-impl.la
> +
> +libspice_client_glib_impl_la_LIBADD =					\
>  	$(SPICE_COMMON_DIR)/common/libspice-common.la			\
>  	$(SPICE_COMMON_DIR)/common/libspice-common-client.la		\
>  	$(GLIB2_LIBS)							\
> @@ -208,7 +211,8 @@ else
>  USB_ACL_HELPER_SRCS =
>  endif
>  
> -libspice_client_glib_2_0_la_SOURCES =			\
> +libspice_client_glib_2_0_la_SOURCES =
> +libspice_client_glib_impl_la_SOURCES =			\
>  	bio-gio.c					\
>  	bio-gio.h					\
>  	spice-audio.c					\
> @@ -270,7 +274,7 @@ libspice_client_glib_2_0_la_SOURCES =			\
>  	spice-glib-main.c				\
>  	$(NULL)
>  
> -nodist_libspice_client_glib_2_0_la_SOURCES =	\
> +nodist_libspice_client_glib_impl_la_SOURCES =	\
>  	spice-glib-enums.c			\
>  	spice-marshal.c				\
>  	spice-marshal.h				\
> @@ -308,49 +312,49 @@ nodist_libspice_client_glibinclude_HEADERS =	\
>  	$(NULL)
>  
>  if HAVE_PULSE
> -libspice_client_glib_2_0_la_SOURCES +=	\
> +libspice_client_glib_impl_la_SOURCES +=	\
>  	spice-pulse.c			\
>  	spice-pulse.h			\
>  	$(NULL)
>  endif
>  
>  if HAVE_GSTAUDIO
> -libspice_client_glib_2_0_la_SOURCES +=	\
> +libspice_client_glib_impl_la_SOURCES +=	\
>  	spice-gstaudio.c		\
>  	spice-gstaudio.h		\
>  	$(NULL)
>  endif
>  
>  if HAVE_BUILTIN_MJPEG
> -libspice_client_glib_2_0_la_SOURCES +=	\
> +libspice_client_glib_impl_la_SOURCES +=	\
>  	channel-display-mjpeg.c		\
>  	$(NULL)
>  endif
>  
>  if HAVE_GSTVIDEO
> -libspice_client_glib_2_0_la_SOURCES +=	\
> +libspice_client_glib_impl_la_SOURCES +=	\
>  	channel-display-gst.c		\
>  	$(NULL)
>  endif
>  
>  if WITH_PHODAV
> -libspice_client_glib_2_0_la_SOURCES +=	\
> +libspice_client_glib_impl_la_SOURCES +=	\
>  	giopipe.c			\
>  	giopipe.h			\
>  	$(NULL)
>  endif
>  
>  if WITH_UCONTEXT
> -libspice_client_glib_2_0_la_SOURCES += continuation.h continuation.c
> coroutine_ucontext.c
> +libspice_client_glib_impl_la_SOURCES += continuation.h continuation.c
> coroutine_ucontext.c
>  endif
>  
>  if WITH_WINFIBER
> -libspice_client_glib_2_0_la_SOURCES += coroutine_winfibers.c
> +libspice_client_glib_impl_la_SOURCES += coroutine_winfibers.c
>  endif
>  
>  if WITH_GTHREAD
> -libspice_client_glib_2_0_la_SOURCES += coroutine_gthread.c
> -libspice_client_glib_2_0_la_LIBADD += $(GTHREAD_LIBS)
> +libspice_client_glib_impl_la_SOURCES += coroutine_gthread.c
> +libspice_client_glib_impl_la_LIBADD += $(GTHREAD_LIBS)
>  endif
>  
>  
> @@ -363,10 +367,10 @@ WIN_USB_FILES= \
>  
>  if OS_WIN32
>  if WITH_USBREDIR
> -libspice_client_glib_2_0_la_SOURCES += \
> +libspice_client_glib_impl_la_SOURCES += \
>  	$(WIN_USB_FILES)
>  endif
> -libspice_client_glib_2_0_la_LIBADD += -lws2_32 -lgdi32
> +libspice_client_glib_impl_la_LIBADD += -lws2_32 -lgdi32
>  endif
>  
>  if WITH_POLKIT
> @@ -397,7 +401,7 @@ install-data-hook:
>  endif
>  
>  
> -$(libspice_client_glib_2_0_la_SOURCES): spice-glib-enums.h spice-marshal.h
> +$(libspice_client_glib_impl_la_SOURCES): spice-glib-enums.h spice-marshal.h
>  
>  if WITH_GTK
>  $(libspice_client_gtk_3_0_la_SOURCES): spice-glib-enums.h
>  spice-widget-enums.h
> 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.
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. 

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]