[Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Michael S. Tsirkin]

V=1 should show what's going on, it's not nice
to silence things unconditionally.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 6817c6f..84ca8be 100644
--- a/Makefile
+++ b/Makefile
@@ -110,7 +110,7 @@ endif
 
 %/config-devices.mak: default-configs/%.mak
 	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, "  GEN   $@")
-	@if test -f $@; then \
+	$(call quiet-command, if test -f $@; then \
 	  if cmp -s $@.old $@; then \
 	    mv $@.tmp $@; \
 	    cp -p $@ $@.old; \
@@ -126,7 +126,7 @@ endif
 	 else \
 	  mv $@.tmp $@; \
 	  cp -p $@ $@.old; \
-	 fi
+	 fi, "  TEST $@");
 
 defconfig:
 	rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
-- 
MST

[Qemu-devel] [PATCH 2/2] Makefile.target: binary depends on config-devices

[Michael S. Tsirkin]

relink binary whenever config-devices.mak changes:
this makes sense as we are adding/removing devices,
so binary has to be relinked to be up to date.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 Makefile.target | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile.target b/Makefile.target
index 58c6ae1..2262d89 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -175,9 +175,11 @@ all-obj-y += $(common-obj-y)
 all-obj-y += $(target-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
 
+$(QEMU_PROG_BUILD): config-devices.mak
+
 # build either PROG or PROGW
 $(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
-	$(call LINK,$^)
+	$(call LINK, $(filter-out %.mak, $^))
 
 gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh
 	$(call quiet-command,rm -f $@ && $(SHELL) $(SRC_PATH)/scripts/feature_to_c.sh $@ $(TARGET_XML_FILES),"  GEN   $(TARGET_DIR)$@")
-- 
MST

Re: [Qemu-devel] [PATCH 2/2] Makefile.target: binary depends on config-devices

[Fam Zheng]

On Thu, 02/19 08:48, Michael S. Tsirkin wrote:
> relink binary whenever config-devices.mak changes:
> this makes sense as we are adding/removing devices,
> so binary has to be relinked to be up to date.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  Makefile.target | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.target b/Makefile.target
> index 58c6ae1..2262d89 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -175,9 +175,11 @@ all-obj-y += $(common-obj-y)
>  all-obj-y += $(target-obj-y)
>  all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
>  
> +$(QEMU_PROG_BUILD): config-devices.mak
> +
>  # build either PROG or PROGW
>  $(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
> -	$(call LINK,$^)
> +	$(call LINK, $(filter-out %.mak, $^))
>  
>  gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh
>  	$(call quiet-command,rm -f $@ && $(SHELL) $(SRC_PATH)/scripts/feature_to_c.sh $@ $(TARGET_XML_FILES),"  GEN   $(TARGET_DIR)$@")
> -- 
> MST
> 

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

Re: [Qemu-devel] [PATCH 2/2] Makefile.target: binary depends on config-devices

[Paolo Bonzini]

On 19/02/2015 08:48, Michael S. Tsirkin wrote:
> relink binary whenever config-devices.mak changes:
> this makes sense as we are adding/removing devices,
> so binary has to be relinked to be up to date.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  Makefile.target | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.target b/Makefile.target
> index 58c6ae1..2262d89 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -175,9 +175,11 @@ all-obj-y += $(common-obj-y)
>  all-obj-y += $(target-obj-y)
>  all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
>  
> +$(QEMU_PROG_BUILD): config-devices.mak
> +
>  # build either PROG or PROGW
>  $(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
> -	$(call LINK,$^)
> +	$(call LINK, $(filter-out %.mak, $^))
>  
>  gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh
>  	$(call quiet-command,rm -f $@ && $(SHELL) $(SRC_PATH)/scripts/feature_to_c.sh $@ $(TARGET_XML_FILES),"  GEN   $(TARGET_DIR)$@")
> 

Not exactly beautiful, but I don't have any better idea.

Applied, thanks.

Paolo

Re: [Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Fam Zheng]

On Thu, 02/19 08:48, Michael S. Tsirkin wrote:
> V=1 should show what's going on, it's not nice
> to silence things unconditionally.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 6817c6f..84ca8be 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -110,7 +110,7 @@ endif
>  
>  %/config-devices.mak: default-configs/%.mak
>  	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, "  GEN   $@")
> -	@if test -f $@; then \
> +	$(call quiet-command, if test -f $@; then \
>  	  if cmp -s $@.old $@; then \
>  	    mv $@.tmp $@; \
>  	    cp -p $@ $@.old; \
> @@ -126,7 +126,7 @@ endif
>  	 else \
>  	  mv $@.tmp $@; \
>  	  cp -p $@ $@.old; \
> -	 fi
> +	 fi, "  TEST $@");

Probably "  GEN   $@" is better, otherwise:

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

>  
>  defconfig:
>  	rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> -- 
> MST
> 

Re: [Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Paolo Bonzini]

On 19/02/2015 08:48, Michael S. Tsirkin wrote:
> V=1 should show what's going on, it's not nice
> to silence things unconditionally.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 6817c6f..84ca8be 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -110,7 +110,7 @@ endif
>  
>  %/config-devices.mak: default-configs/%.mak
>  	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, "  GEN   $@")
> -	@if test -f $@; then \
> +	$(call quiet-command, if test -f $@; then \
>  	  if cmp -s $@.old $@; then \
>  	    mv $@.tmp $@; \
>  	    cp -p $@ $@.old; \
> @@ -126,7 +126,7 @@ endif
>  	 else \
>  	  mv $@.tmp $@; \
>  	  cp -p $@ $@.old; \
> -	 fi
> +	 fi, "  TEST $@");
>  
>  defconfig:
>  	rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> 

Squashing this to make the non-verbose messages clearer, ok?

diff --git a/Makefile b/Makefile
index 5604209..d92d4cd 100644
--- a/Makefile
+++ b/Makefile
@@ -109,7 +109,7 @@ endif
 -include $(SUBDIR_DEVICES_MAK_DEP)
 
 %/config-devices.mak: default-configs/%.mak
-	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, "  GEN   $@")
+	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@.tmp $<, "  GEN   $@.tmp")
 	$(call quiet-command, if test -f $@; then \
 	  if cmp -s $@.old $@; then \
 	    mv $@.tmp $@; \
@@ -126,7 +126,7 @@ endif
 	 else \
 	  mv $@.tmp $@; \
 	  cp -p $@ $@.old; \
-	 fi, "  TEST $@");
+	 fi, "  GEN  $@");
 
 defconfig:
 	rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh
index 7242707..7958086 100644
--- a/scripts/make_device_config.sh
+++ b/scripts/make_device_config.sh
@@ -2,7 +2,7 @@
 # Construct a target device config file from a default, pulling in any
 # files from include directives.
 
-dest=$1.tmp
+dest=$1
 dep=`dirname $1`-`basename $1`.d
 src=$2
 src_dir=`dirname $src`

Re: [Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Fam Zheng]

On Fri, 02/27 19:40, Paolo Bonzini wrote:
> 
> 
> On 19/02/2015 08:48, Michael S. Tsirkin wrote:
> > V=1 should show what's going on, it's not nice
> > to silence things unconditionally.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  Makefile | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 6817c6f..84ca8be 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -110,7 +110,7 @@ endif
> >  
> >  %/config-devices.mak: default-configs/%.mak
> >  	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, "  GEN   $@")
> > -	@if test -f $@; then \
> > +	$(call quiet-command, if test -f $@; then \
> >  	  if cmp -s $@.old $@; then \
> >  	    mv $@.tmp $@; \
> >  	    cp -p $@ $@.old; \
> > @@ -126,7 +126,7 @@ endif
> >  	 else \
> >  	  mv $@.tmp $@; \
> >  	  cp -p $@ $@.old; \
> > -	 fi
> > +	 fi, "  TEST $@");
> >  
> >  defconfig:
> >  	rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> > 
> 
> Squashing this to make the non-verbose messages clearer, ok?
> 
> diff --git a/Makefile b/Makefile
> index 5604209..d92d4cd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -109,7 +109,7 @@ endif
>  -include $(SUBDIR_DEVICES_MAK_DEP)
>  
>  %/config-devices.mak: default-configs/%.mak
> -	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, "  GEN   $@")
> +	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@.tmp $<, "  GEN   $@.tmp")
>  	$(call quiet-command, if test -f $@; then \
>  	  if cmp -s $@.old $@; then \
>  	    mv $@.tmp $@; \
> @@ -126,7 +126,7 @@ endif
>  	 else \
>  	  mv $@.tmp $@; \
>  	  cp -p $@ $@.old; \
> -	 fi, "  TEST $@");
> +	 fi, "  GEN  $@");
>  
>  defconfig:
>  	rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh
> index 7242707..7958086 100644
> --- a/scripts/make_device_config.sh
> +++ b/scripts/make_device_config.sh
> @@ -2,7 +2,7 @@
>  # Construct a target device config file from a default, pulling in any
>  # files from include directives.
>  
> -dest=$1.tmp
> +dest=$1
>  dep=`dirname $1`-`basename $1`.d
>  src=$2
>  src_dir=`dirname $src`

Looks good to me.

Fam

Re: [Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

On 02/27/2015 05:57 PM, Fam Zheng wrote:
> On Fri, 02/27 19:40, Paolo Bonzini wrote:
>>
>>
>> On 19/02/2015 08:48, Michael S. Tsirkin wrote:
>>> V=1 should show what's going on, it's not nice
>>> to silence things unconditionally.

>> Squashing this to make the non-verbose messages clearer, ok?

Also, s/mak/make/ in the subject

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Paolo Bonzini]

On 02/03/2015 16:52, Eric Blake wrote:
> On 02/27/2015 05:57 PM, Fam Zheng wrote:
>> On Fri, 02/27 19:40, Paolo Bonzini wrote:
>>> 
>>> 
>>> On 19/02/2015 08:48, Michael S. Tsirkin wrote:
>>>> V=1 should show what's going on, it's not nice to silence
>>>> things unconditionally.
> 
>>> Squashing this to make the non-verbose messages clearer, ok?
> 
> Also, s/mak/make/ in the subject

It would be more like s/mak/.mak/ :)  But I've already sent a pull
request.

Paolo

Re: [Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Michael S. Tsirkin]

On Fri, Feb 27, 2015 at 07:40:26PM +0100, Paolo Bonzini wrote:
> 
> 
> On 19/02/2015 08:48, Michael S. Tsirkin wrote:
> > V=1 should show what's going on, it's not nice
> > to silence things unconditionally.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  Makefile | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 6817c6f..84ca8be 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -110,7 +110,7 @@ endif
> >  
> >  %/config-devices.mak: default-configs/%.mak
> >  	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, "  GEN   $@")
> > -	@if test -f $@; then \
> > +	$(call quiet-command, if test -f $@; then \
> >  	  if cmp -s $@.old $@; then \
> >  	    mv $@.tmp $@; \
> >  	    cp -p $@ $@.old; \
> > @@ -126,7 +126,7 @@ endif
> >  	 else \
> >  	  mv $@.tmp $@; \
> >  	  cp -p $@ $@.old; \
> > -	 fi
> > +	 fi, "  TEST $@");
> >  
> >  defconfig:
> >  	rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> > 
> 
> Squashing this to make the non-verbose messages clearer, ok?

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> diff --git a/Makefile b/Makefile
> index 5604209..d92d4cd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -109,7 +109,7 @@ endif
>  -include $(SUBDIR_DEVICES_MAK_DEP)
>  
>  %/config-devices.mak: default-configs/%.mak
> -	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, "  GEN   $@")
> +	$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@.tmp $<, "  GEN   $@.tmp")
>  	$(call quiet-command, if test -f $@; then \
>  	  if cmp -s $@.old $@; then \
>  	    mv $@.tmp $@; \
> @@ -126,7 +126,7 @@ endif
>  	 else \
>  	  mv $@.tmp $@; \
>  	  cp -p $@ $@.old; \
> -	 fi, "  TEST $@");
> +	 fi, "  GEN  $@");
>  
>  defconfig:
>  	rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh
> index 7242707..7958086 100644
> --- a/scripts/make_device_config.sh
> +++ b/scripts/make_device_config.sh
> @@ -2,7 +2,7 @@
>  # Construct a target device config file from a default, pulling in any
>  # files from include directives.
>  
> -dest=$1.tmp
> +dest=$1
>  dep=`dirname $1`-`basename $1`.d
>  src=$2
>  src_dir=`dirname $src`

Re: [Qemu-devel] [PATCH 1/2] Makefile: don't silence mak file test with V=1

[Peter Maydell]

On 27 February 2015 at 18:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Squashing this to make the non-verbose messages clearer, ok?
>
> diff --git a/Makefile b/Makefile
> index 5604209..d92d4cd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -109,7 +109,7 @@ endif
>  -include $(SUBDIR_DEVICES_MAK_DEP)
>
>  %/config-devices.mak: default-configs/%.mak
> -       $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, "  GEN   $@")
> +       $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@.tmp $<, "  GEN   $@.tmp")
>         $(call quiet-command, if test -f $@; then \
>           if cmp -s $@.old $@; then \
>             mv $@.tmp $@; \
> @@ -126,7 +126,7 @@ endif
>          else \
>           mv $@.tmp $@; \
>           cp -p $@ $@.old; \
> -        fi, "  TEST $@");
> +        fi, "  GEN  $@");
>
>  defconfig:
>         rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK)
> diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh
> index 7242707..7958086 100644
> --- a/scripts/make_device_config.sh
> +++ b/scripts/make_device_config.sh
> @@ -2,7 +2,7 @@
>  # Construct a target device config file from a default, pulling in any
>  # files from include directives.
>
> -dest=$1.tmp
> +dest=$1
>  dep=`dirname $1`-`basename $1`.d
>  src=$2
>  src_dir=`dirname $src`

This squashed-in change breaks automatically rebuilding the
config-devices.mak file when a default-configs file indirectly
included from the architecture's top level config is changed.
(This is most common for AArch64 because of the way it includes
the arm-softmmu config file; so any change to the arm-softmmu
device configs means the aarch64-softmmu qemu isn't rebuilt as
it should be.)

The problem is that we're now passing the script $@.tmp as its
first argument, and so the 'dep' filename it constructs
ends up as aarch64-softmmu-config-devices.mak.tmp.d. But the
Makefile include line defining the dep file to pull in is:
SUBDIR_DEVICES_MAK_DEP=$(patsubst %, %-config-devices.mak.d, $(TARGET_DIRS))
so we never include the .d file at all.

The basic "%/config-devices.mak: default-configs/%.mak" rule
means that this is only a problem for the indirectly included
config files.

-- PMM