[Qemu-devel] [PATCH 0/2] build: simplify and fix fix-obj-vars, more libcacard cleanups

[Qemu-devel] [PATCH 0/2] build: simplify and fix fix-obj-vars, more libcacard cleanups

[Paolo Bonzini]

On top of the patch that Michael just sent.

Paolo Bonzini (2):
  build: simplify and fix fix-obj-vars
  build: more libcacard cleanups

 configure            |  2 --
 hw/usb/Makefile.objs |  4 ++--
 rules.mak            | 15 ++++++++-------
 3 files changed, 10 insertions(+), 11 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] build: simplify and fix fix-obj-vars

[Paolo Bonzini]

fix-obj-vars has the undesired side effect of breaking -cflags
-objs and -libs variables in the toplevel Makefile.objs.  The
variables in the toplevel Makefile.objs do not need any fix,
so fix-obj-vars need not do anything.

Since we are touching it, remove the now unnecessary $(if)
in the callers.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rules.mak | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/rules.mak b/rules.mak
index c0c3d86..26014c5 100644
--- a/rules.mak
+++ b/rules.mak
@@ -175,16 +175,17 @@ $(eval save-$2-$1 :=)
 endef
 
 define fix-obj-vars
-$(foreach v,$($1), \
+$(if $2,
+  $(foreach v,$($1), \
 	$(if $($v-cflags), \
-		$(eval $2$v-cflags := $($v-cflags)) \
+		$(eval $2/$v-cflags := $($v-cflags)) \
 		$(eval $v-cflags := )) \
 	$(if $($v-libs), \
-		$(eval $2$v-libs := $($v-libs)) \
+		$(eval $2/$v-libs := $($v-libs)) \
 		$(eval $v-libs := )) \
 	$(if $($v-objs), \
-		$(eval $2$v-objs := $(addprefix $2,$($v-objs))) \
-		$(eval $v-objs := )))
+		$(eval $2/$v-objs := $(addprefix $2/,$($v-objs))) \
+		$(eval $v-objs := ))))
 endef
 
 define unnest-dir
@@ -192,7 +193,7 @@ $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
 $(eval obj-parent-$1 := $(obj))
 $(eval obj := $(if $(obj),$(obj)/$1,$1))
 $(eval include $(SRC_PATH)/$1/Makefile.objs)
-$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(if $(obj),$(obj)/)))
+$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(obj)))
 $(eval obj := $(obj-parent-$1))
 $(eval obj-parent-$1 := )
 $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
@@ -228,7 +229,7 @@ endef
 define unnest-vars
 $(eval obj := $1)
 $(eval nested-vars := $2)
-$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(if $(obj),$(obj)/)))
+$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(obj)))
 $(eval old-nested-dirs := )
 $(call unnest-vars-1)
 $(if $1,$(foreach v,$(nested-vars),$(eval \
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/2] build: simplify and fix fix-obj-vars

[Fam Zheng]

On Thu, 05/08 15:08, Paolo Bonzini wrote:
> fix-obj-vars has the undesired side effect of breaking -cflags
> -objs and -libs variables in the toplevel Makefile.objs.  The
> variables in the toplevel Makefile.objs do not need any fix,
> so fix-obj-vars need not do anything.
> 
> Since we are touching it, remove the now unnecessary $(if)
> in the callers.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rules.mak | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/rules.mak b/rules.mak
> index c0c3d86..26014c5 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -175,16 +175,17 @@ $(eval save-$2-$1 :=)
>  endef
>  
>  define fix-obj-vars
> -$(foreach v,$($1), \
> +$(if $2,
> +  $(foreach v,$($1), \

Not very consistent in indentation. BTW what's the right space/tab style to
use in Makefiles for multi-line macros like this?

>  	$(if $($v-cflags), \
> -		$(eval $2$v-cflags := $($v-cflags)) \
> +		$(eval $2/$v-cflags := $($v-cflags)) \
>  		$(eval $v-cflags := )) \
>  	$(if $($v-libs), \
> -		$(eval $2$v-libs := $($v-libs)) \
> +		$(eval $2/$v-libs := $($v-libs)) \
>  		$(eval $v-libs := )) \
>  	$(if $($v-objs), \
> -		$(eval $2$v-objs := $(addprefix $2,$($v-objs))) \
> -		$(eval $v-objs := )))
> +		$(eval $2/$v-objs := $(addprefix $2/,$($v-objs))) \
> +		$(eval $v-objs := ))))
>  endef
>  
>  define unnest-dir
> @@ -192,7 +193,7 @@ $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
>  $(eval obj-parent-$1 := $(obj))
>  $(eval obj := $(if $(obj),$(obj)/$1,$1))
>  $(eval include $(SRC_PATH)/$1/Makefile.objs)
> -$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(if $(obj),$(obj)/)))
> +$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(obj)))
>  $(eval obj := $(obj-parent-$1))
>  $(eval obj-parent-$1 := )
>  $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
> @@ -228,7 +229,7 @@ endef
>  define unnest-vars
>  $(eval obj := $1)
>  $(eval nested-vars := $2)
> -$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(if $(obj),$(obj)/)))

I'm not sure about this removed line since it's not in master, maybe you missed
a patch to squash?

> +$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(obj)))

But the new line looks good.

Thanks,
Reviewed-by: Fam Zheng <famz@redhat.com>

>  $(eval old-nested-dirs := )
>  $(call unnest-vars-1)
>  $(if $1,$(foreach v,$(nested-vars),$(eval \
> -- 
> 1.8.3.1
> 
> 
> 

Re: [Qemu-devel] [PATCH 1/2] build: simplify and fix fix-obj-vars

[Paolo Bonzini]

Il 08/05/2014 15:40, Fam Zheng ha scritto:
>> >  define fix-obj-vars
>> > -$(foreach v,$($1), \
>> > +$(if $2,
>> > +  $(foreach v,$($1), \
> Not very consistent in indentation. BTW what's the right space/tab style to
> use in Makefiles for multi-line macros like this?

No idea... I'll just join the two lines.

>> >  	$(if $($v-cflags), \
>> > -		$(eval $2$v-cflags := $($v-cflags)) \
>> > +		$(eval $2/$v-cflags := $($v-cflags)) \
>> >  		$(eval $v-cflags := )) \
>> >  	$(if $($v-libs), \
>> > -		$(eval $2$v-libs := $($v-libs)) \
>> > +		$(eval $2/$v-libs := $($v-libs)) \
>> >  		$(eval $v-libs := )) \
>> >  	$(if $($v-objs), \
>> > -		$(eval $2$v-objs := $(addprefix $2,$($v-objs))) \
>> > -		$(eval $v-objs := )))
>> > +		$(eval $2/$v-objs := $(addprefix $2/,$($v-objs))) \
>> > +		$(eval $v-objs := ))))
>> >  endef
>> >
>> >  define unnest-dir
>> > @@ -192,7 +193,7 @@ $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
>> >  $(eval obj-parent-$1 := $(obj))
>> >  $(eval obj := $(if $(obj),$(obj)/$1,$1))
>> >  $(eval include $(SRC_PATH)/$1/Makefile.objs)
>> > -$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(if $(obj),$(obj)/)))
>> > +$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(obj)))
>> >  $(eval obj := $(obj-parent-$1))
>> >  $(eval obj-parent-$1 := )
>> >  $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
>> > @@ -228,7 +229,7 @@ endef
>> >  define unnest-vars
>> >  $(eval obj := $1)
>> >  $(eval nested-vars := $2)
>> > -$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(if $(obj),$(obj)/)))
> I'm not sure about this removed line since it's not in master, maybe you missed
> a patch to squash?

It's the original fix-obj-vars fix.

Paolo

>> > +$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(obj)))

[Qemu-devel] [PATCH 2/2] build: more libcacard cleanups

[Paolo Bonzini]

To be squashed in Michael's "libcacard: remove libcacard-specific CFLAGS
and LIBS from global vars".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure            | 2 --
 hw/usb/Makefile.objs | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index b0c0c9c..25d59f9 100755
--- a/configure
+++ b/configure
@@ -3470,7 +3470,6 @@ if test "$smartcard_nss" != "no"; then
 #include <pk11pub.h>
 int main(void) { PK11_FreeSlot(0); return 0; }
 EOF
-    smartcard_includes="-I\$(SRC_PATH)/libcacard"
     # FIXME: do not include $glib_* in here
     nss_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
     nss_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
@@ -4497,7 +4496,6 @@ fi
 
 if test "$smartcard_nss" = "yes" ; then
   echo "CONFIG_SMARTCARD_NSS=y" >> $config_host_mak
-  echo "SMARTCARD_INCLUDES=$smartcard_includes" >> $config_host_mak
   echo "NSS_LIBS=$nss_libs" >> $config_host_mak
   echo "NSS_CFLAGS=$nss_cflags" >> $config_host_mak
 fi
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 54d3982..a32b38b 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -24,8 +24,8 @@ ifeq ($(CONFIG_USB_SMARTCARD),y)
 common-obj-y                          += dev-smartcard-reader.o
 common-obj-y                          += ccid-card-passthru.o
 common-obj-$(CONFIG_SMARTCARD_NSS)    += ccid-card-emulated.o
-ccid-card-emulated.o-cflags := $(SMARTCARD_INCLUDES)
-libs_softmmu += $(NSS_LIBS)
+ccid-card-emulated.o-cflags := -I$(SRC_PATH)/libcacard
+ccid-card-emulated.o-libs += $(NSS_LIBS)
 endif
 
 ifeq ($(CONFIG_POSIX),y)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/2] build: more libcacard cleanups

[Michael Tokarev]

08.05.2014 17:08, Paolo Bonzini wrote:
> To be squashed in Michael's "libcacard: remove libcacard-specific CFLAGS
> and LIBS from global vars".

This should be squashed into that commit, no need to have two halves touching
the same place.  I thought about removing the indirection for SMARTCARD_INCLUDES
but didn't bother really, yes it is nicer this way.

However, I still (in combined result of the 2 patches) miss one little detail
about libcacard and this stuff, which this patch also does a bit wrong.

$(NSS_LIBS) is not a dependency of hw/usb/ccid-card-emulated.o, it is a
dependency of libcacard/vcard_emul_nss.o - the only single file in whole
qemu tree which actually uses nss stuff.  Hopefully, it works after the
per-object -libs fixup patch.. ;)

(To my taste, qemu should link with libcacard.so, not with individual objects
of libcacard.  Or should be able to do that, anyway.)

Thanks,

/mjt

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  configure            | 2 --
>  hw/usb/Makefile.objs | 4 ++--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/configure b/configure
> index b0c0c9c..25d59f9 100755
> --- a/configure
> +++ b/configure
> @@ -3470,7 +3470,6 @@ if test "$smartcard_nss" != "no"; then
>  #include <pk11pub.h>
>  int main(void) { PK11_FreeSlot(0); return 0; }
>  EOF
> -    smartcard_includes="-I\$(SRC_PATH)/libcacard"
>      # FIXME: do not include $glib_* in here
>      nss_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
>      nss_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
> @@ -4497,7 +4496,6 @@ fi
>  
>  if test "$smartcard_nss" = "yes" ; then
>    echo "CONFIG_SMARTCARD_NSS=y" >> $config_host_mak
> -  echo "SMARTCARD_INCLUDES=$smartcard_includes" >> $config_host_mak
>    echo "NSS_LIBS=$nss_libs" >> $config_host_mak
>    echo "NSS_CFLAGS=$nss_cflags" >> $config_host_mak
>  fi
> diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> index 54d3982..a32b38b 100644
> --- a/hw/usb/Makefile.objs
> +++ b/hw/usb/Makefile.objs
> @@ -24,8 +24,8 @@ ifeq ($(CONFIG_USB_SMARTCARD),y)
>  common-obj-y                          += dev-smartcard-reader.o
>  common-obj-y                          += ccid-card-passthru.o
>  common-obj-$(CONFIG_SMARTCARD_NSS)    += ccid-card-emulated.o
> -ccid-card-emulated.o-cflags := $(SMARTCARD_INCLUDES)
> -libs_softmmu += $(NSS_LIBS)
> +ccid-card-emulated.o-cflags := -I$(SRC_PATH)/libcacard
> +ccid-card-emulated.o-libs += $(NSS_LIBS)
>  endif
>  
>  ifeq ($(CONFIG_POSIX),y)
> 

Re: [Qemu-devel] [PATCH 2/2] build: more libcacard cleanups

[Paolo Bonzini]

Il 08/05/2014 15:16, Michael Tokarev ha scritto:
> However, I still (in combined result of the 2 patches) miss one little detail
> about libcacard and this stuff, which this patch also does a bit wrong.
> 
> $(NSS_LIBS) is not a dependency of hw/usb/ccid-card-emulated.o, it is a
> dependency of libcacard/vcard_emul_nss.o - the only single file in whole
> qemu tree which actually uses nss stuff.  Hopefully, it works after the
> per-object -libs fixup patch.. ;)

You're obviously right and yes, it works giving this overall patch:

diff --git a/Makefile.objs b/Makefile.objs
index f0069ba..b897e1d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -31,6 +31,8 @@ libcacard-y += libcacard/vcard_emul_nss.o
 libcacard-y += libcacard/vcard_emul_type.o
 libcacard-y += libcacard/card_7816.o
 libcacard-y += libcacard/vcardt.o
+libcacard/vcard_emul_nss.o-cflags := $(NSS_CFLAGS)
+libcacard/vcard_emul_nss.o-libs := $(NSS_LIBS)
 
 ######################################################################
 # Target independent part of system emulation. The long term path is to
diff --git a/configure b/configure
index ac2fa15..25d59f9 100755
--- a/configure
+++ b/configure
@@ -3470,10 +3470,10 @@ if test "$smartcard_nss" != "no"; then
 #include <pk11pub.h>
 int main(void) { PK11_FreeSlot(0); return 0; }
 EOF
-    smartcard_includes="-I\$(SRC_PATH)/libcacard"
-    libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
-    libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
-    test_cflags="$libcacard_cflags"
+    # FIXME: do not include $glib_* in here
+    nss_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
+    nss_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
+    test_cflags="$nss_cflags"
     # The header files in nss < 3.13.3 have a bug which causes them to
     # emit a warning. If we're going to compile QEMU with -Werror, then
     # test that the headers don't have this bug. Otherwise we would pass
@@ -3483,11 +3483,8 @@ EOF
     fi
     if test -n "$libtool" &&
        $pkg_config --atleast-version=3.12.8 nss && \
-      compile_prog "$test_cflags" "$libcacard_libs"; then
+      compile_prog "$test_cflags" "$nss_libs"; then
         smartcard_nss="yes"
-        QEMU_CFLAGS="$QEMU_CFLAGS $libcacard_cflags"
-        QEMU_INCLUDES="$QEMU_INCLUDES $smartcard_includes"
-        libs_softmmu="$libcacard_libs $libs_softmmu"
     else
         if test "$smartcard_nss" = "yes"; then
             feature_not_found "nss"
@@ -4499,8 +4496,8 @@ fi
 
 if test "$smartcard_nss" = "yes" ; then
   echo "CONFIG_SMARTCARD_NSS=y" >> $config_host_mak
-  echo "libcacard_libs=$libcacard_libs" >> $config_host_mak
-  echo "libcacard_cflags=$libcacard_cflags" >> $config_host_mak
+  echo "NSS_LIBS=$nss_libs" >> $config_host_mak
+  echo "NSS_CFLAGS=$nss_cflags" >> $config_host_mak
 fi
 
 if test "$libusb" = "yes" ; then
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 17d460c..3fe4dff 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -24,6 +24,7 @@ ifeq ($(CONFIG_USB_SMARTCARD),y)
 common-obj-y                          += dev-smartcard-reader.o
 common-obj-y                          += ccid-card-passthru.o
 common-obj-$(CONFIG_SMARTCARD_NSS)    += ccid-card-emulated.o
+ccid-card-emulated.o-cflags := -I$(SRC_PATH)/libcacard
 endif
 
 ifeq ($(CONFIG_POSIX),y)
diff --git a/libcacard/Makefile b/libcacard/Makefile
index 6b06448..4a76d03 100644
--- a/libcacard/Makefile
+++ b/libcacard/Makefile
@@ -25,7 +25,6 @@ vscclient$(EXESUF): libcacard/vscclient.o libcacard.la
 
 libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \
 	-export-syms $(SRC_PATH)/libcacard/libcacard.syms
-libcacard.la: LIBS = $(libcacard_libs)
 libcacard.la: $(libcacard-lobj-y)
 	$(call LINK,$^)
 
Since this is getting tricky, I'll post the whole series today and send
a pull request tomorrow or Monday.

Paolo

Re: [Qemu-devel] [PATCH 2/2] build: more libcacard cleanups

[Michael Tokarev]

08.05.2014 17:28, Paolo Bonzini wrote:
> Il 08/05/2014 15:16, Michael Tokarev ha scritto:
>> However, I still (in combined result of the 2 patches) miss one little detail
>> about libcacard and this stuff, which this patch also does a bit wrong.
>>
>> $(NSS_LIBS) is not a dependency of hw/usb/ccid-card-emulated.o, it is a
>> dependency of libcacard/vcard_emul_nss.o - the only single file in whole
>> qemu tree which actually uses nss stuff.  Hopefully, it works after the
>> per-object -libs fixup patch.. ;)
> 
> You're obviously right and yes, it works giving this overall patch:

The same as I commited a few mins ago.  But with a fixme for now
in vcard_emul_nss.o-libs place (as I need to keep it working in
my tree too).

http://git.corpit.ru/?p=qemu.git;a=shortlog;h=refs/heads/mjt-buildsys-libs

Thanks,

/mjt