diff mbox

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

Message ID 54F0BA1A.5080509@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Feb. 27, 2015, 6:40 p.m. UTC
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?

Comments

Fam Zheng Feb. 28, 2015, 12:57 a.m. UTC | #1
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
Michael S. Tsirkin Feb. 28, 2015, 6:50 p.m. UTC | #2
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`
Eric Blake March 2, 2015, 3:52 p.m. UTC | #3
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
Paolo Bonzini March 2, 2015, 4:22 p.m. UTC | #4
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
Peter Maydell March 11, 2015, 5:24 p.m. UTC | #5
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
diff mbox

Patch

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`