[Spice-devel] [PATCH 2/2] do not link spice with libcacard

[Michael Tokarev]

09.10.2015 16:36, Christophe Fergeau wrote:
>> --- a/spice-common/m4/spice-deps.m4
>> +++ b/spice-common/m4/spice-deps.m4
>> @@ -58,7 +58,8 @@ AC_DEFUN([SPICE_CHECK_SMARTCARD], [
>>        fi
>>      fi
>>      AS_VAR_APPEND([$1_CFLAGS], [' $(SMARTCARD_CFLAGS)'])
>> -    AS_VAR_APPEND([$1_LIBS], [' $(SMARTCARD_LIBS)'])
>> +dnl only headers are used, not the lib itself
>> +dnl    AS_VAR_APPEND([$1_LIBS], [' $(SMARTCARD_LIBS)'])
> 
> This one is not going to work, spice-common does not need to link
> with libcacard, but spice-gtk which is using the same macro needs it.
> Now that these SPICE_CHECK_xxx macros are used by
> spice-common/spice/spice-gtk, this AS_VAR_APPEND() work they are doing
> is a bit out of place as only spice-common needs it. I'd suggest
> removing it from the macros, and generating
> SPICE_COMMON_CFLAGS/SPICE_COMMON_LIBS manually from
> spice-common/configure.ac as in the attached patch.

Yeah, this is quite twisted :) but makes some sense.

>       This would also
> remove the issue fixed by your first patch.

Well, not exactly.  Why don't you keep the variables in the final
SPICE_COMMON_{CFLAGS,LIBS} line my first patch does, instead of
expanding the values at configure time?  This way it will be possible
to override the components in a common way at make time.  I'm referring
to the first hunk, below:

> From 93756c00c8a08ee043236eeb3ac58a00ecc10c88 Mon Sep 17 00:00:00 2001
> From: Christophe Fergeau <cfergeau at redhat.com>
> Date: Fri, 9 Oct 2015 15:07:27 +0200
> Subject: [spice-common] build-sys: Rework SPICE_CHECK_* m4 macros
> 
> These macros were automatically appending the needed CFLAGS/LIBS to
> variables passed as arguments. This is how spice-common uses them, but
> now how spice-gtk/spice want to use them, and is making the macros more
> complicated than they could (in particular this makes them use
> AS_VAR_APPEND).
> This  is also not flexible enough as spice-gtk uses libcacard libraries,
> while spice-common does not need them. If SPICE_CHECK_SMARTCARD
> unconditionnally libcacard libraries to the variable spice-common passes
> it as an argument, we'll end up linking with an unneeded library.
> 
> This commit removes this automatic appending from the SPICE_CHECK_*
> macros and moves it to spice-common as it's the only one which needs it.
> ---
>  configure.ac     | 14 +++++-----
>  m4/spice-deps.m4 | 81 ++++++++++++++++++++++----------------------------------
>  2 files changed, 39 insertions(+), 56 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 98311bf..4694fc0 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -34,12 +34,14 @@ AC_SUBST([CODE_GENERATOR_BASEDIR])
>  
>  SPICE_CHECK_PYTHON_MODULES()
>  
> -SPICE_CHECK_PIXMAN(SPICE_COMMON)
> -SPICE_CHECK_SMARTCARD(SPICE_COMMON)
> -SPICE_CHECK_CELT051(SPICE_COMMON)
> -SPICE_CHECK_GLIB2(SPICE_COMMON)
> -SPICE_CHECK_OPUS(SPICE_COMMON)
> -SPICE_CHECK_OPENGL(SPICE_COMMON)
> +SPICE_CHECK_PIXMAN
> +SPICE_CHECK_SMARTCARD
> +SPICE_CHECK_CELT051
> +SPICE_CHECK_GLIB2
> +SPICE_CHECK_OPUS
> +SPICE_CHECK_OPENGL
> +SPICE_COMMON_CFLAGS="$PIXMAN_CFLAGS $SMARTCARD_CFLAGS $CELT051_CFLAGS $GLIB2_CFLAGS $OPUS_CFLAGS $GL_CFLAGS"
> +SPICE_COMMON_LIBS="$PIXMAN_LIBS $CELT051_LIBS $GLIB2_LIBS $OPUS_LIBS $GL_LIBS"

Why not use

 +SPICE_COMMON_CFLAGS='$(PIXMAN_CFLAGS) $(SMARTCARD_CFLAGS) $(CELT051_CFLAGS) $(GLIB2_CFLAGS) $(OPUS_CFLAGS) $(GL_CFLAGS)'
 +SPICE_COMMON_LIBS='$(PIXMAN_LIBS) $(CELT051_LIBS) $(GLIB2_LIBS) $(OPUS_LIBS) $(GL_LIBS)'

?

SPICE_CHECK_PIXMAN et al will use AC_DEFINE(PIXMAN_{CFLAGS,LIBS}), so these
will be included in the final Makefile, and these are used in server/
correctly, server/ allows to override these.  But not spice-common.

The rest looks sane to me, but again, I'm not really familiar with
the thing.. ;)

Thanks,

/mjt