[02/12] build-sys: silence make by default

Message ID 20171208005825.14587-3-marcandre.lureau@redhat.com
State New
Headers show
Series
  • Untitled series #17389
Related show

Commit Message

Marc-André Lureau Dec. 8, 2017, 12:58 a.m.
In particular, do not print anything when there is nothing to do, in
particular, after a successful build:

$ make
make[1]: '/home/elmarco/src/qemu/build/capstone/libcapstone.a' is up to date.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 rules.mak | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Eric Blake Dec. 8, 2017, 7:19 p.m. | #1
On 12/07/2017 06:58 PM, Marc-André Lureau wrote:
> In particular, do not print anything when there is nothing to do, in
> particular, after a successful build:

Do we need both 'in particular'?

> 
> $ make
> make[1]: '/home/elmarco/src/qemu/build/capstone/libcapstone.a' is up to date.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  rules.mak | 5 +++++
>  1 file changed, 5 insertions(+)

Yay!  I've been bothered by this.

You may want to add that the noise is conditional on whether you have
capstone-devel installed.

However,

> 
> diff --git a/rules.mak b/rules.mak
> index 6e943335f3..b760d54908 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -131,6 +131,11 @@ modules:
>  # If called with only a single argument, will print nothing in quiet mode.
>  quiet-command = $(if $(V),$1,$(if $(2),@printf "  %-7s %s\n" $2 $3 && $1, @$1))
>  
> +makeflags_ = $(makeflags_0)
> +makeflags_0 = --no-print-directory -s

Why the mix of long option and short option? Would it be more legible to
use two long options?

In my testing of your patch, '-s' alone also silenced things
(admittedly, when libcapstone.a is up-to-date).  What is
--no-print-directory adding?

/me goes digging

Hmm, we already have this in Makefile:

SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory) BUILD_DIR=$(BUILD_DIR)

So maybe our problem is that we are forgetting to feed SUBDIR_MAKEFLAGS
to our sub-make that builds libcapstone.a?

Next, why is -s silencing the "is up to date" message?  Per 'make
--help', -s controls whether recipes are echoed - but that doesn't seem
to be a recipe that was echoed.  Furthermore, the make manual mentions
that -s is automatically added to MAKEFLAGS if it was present in the
command line; are we accidentally silencing too much (if the user wants
to do 'make -s V=1' to get JUST command lines but not the make noise)?

> +makeflags_1 =
> +MAKEFLAGS += $(makeflags_$(V))
> +

So I'm not yet convinced that this is the right place, or if it is too
heavy of a hammer.
Marc-André Lureau Dec. 13, 2017, 11:30 a.m. | #2
Hi

On Fri, Dec 8, 2017 at 8:19 PM, Eric Blake <eblake@redhat.com> wrote:
> On 12/07/2017 06:58 PM, Marc-André Lureau wrote:
>> In particular, do not print anything when there is nothing to do, in
>> particular, after a successful build:
>
> Do we need both 'in particular'?
>

nope

>>
>> $ make
>> make[1]: '/home/elmarco/src/qemu/build/capstone/libcapstone.a' is up to date.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  rules.mak | 5 +++++
>>  1 file changed, 5 insertions(+)
>
> Yay!  I've been bothered by this.
>
> You may want to add that the noise is conditional on whether you have
> capstone-devel installed.
>
> However,
>
>>
>> diff --git a/rules.mak b/rules.mak
>> index 6e943335f3..b760d54908 100644
>> --- a/rules.mak
>> +++ b/rules.mak
>> @@ -131,6 +131,11 @@ modules:
>>  # If called with only a single argument, will print nothing in quiet mode.
>>  quiet-command = $(if $(V),$1,$(if $(2),@printf "  %-7s %s\n" $2 $3 && $1, @$1))
>>
>> +makeflags_ = $(makeflags_0)
>> +makeflags_0 = --no-print-directory -s
>
> Why the mix of long option and short option? Would it be more legible to
> use two long options?
>
> In my testing of your patch, '-s' alone also silenced things
> (admittedly, when libcapstone.a is up-to-date).  What is
> --no-print-directory adding?
>
> /me goes digging
>
> Hmm, we already have this in Makefile:
>
> SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory) BUILD_DIR=$(BUILD_DIR)
>

Thanks I didn't notice.

> So maybe our problem is that we are forgetting to feed SUBDIR_MAKEFLAGS
> to our sub-make that builds libcapstone.a?

We do.

Now I wonder if this is really a good idea to merge "generic"
makeflags (silence etc) with qemu build-sys specifics (BUILD_DIR=..).
It seems we are lucky enough that capstone uses BUILDDIR and qemu
BUILD_DIR, so we avoided the conflict...

Perhaps it's a good time to switch and use thje genreic MAKEFLAGS in
rules.mk for V=.

> Next, why is -s silencing the "is up to date" message?  Per 'make
> --help', -s controls whether recipes are echoed - but that doesn't seem
> to be a recipe that was echoed.  Furthermore, the make manual mentions
> that -s is automatically added to MAKEFLAGS if it was present in the
> command line; are we accidentally silencing too much (if the user wants
> to do 'make -s V=1' to get JUST command lines but not the make noise)?

V=1 shouldn't add the flags, so I don't see the conflict in your example.

>
>> +makeflags_1 =
>> +MAKEFLAGS += $(makeflags_$(V))
>> +
>
> So I'm not yet convinced that this is the right place, or if it is too
> heavy of a hammer.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>

Patch

diff --git a/rules.mak b/rules.mak
index 6e943335f3..b760d54908 100644
--- a/rules.mak
+++ b/rules.mak
@@ -131,6 +131,11 @@  modules:
 # If called with only a single argument, will print nothing in quiet mode.
 quiet-command = $(if $(V),$1,$(if $(2),@printf "  %-7s %s\n" $2 $3 && $1, @$1))
 
+makeflags_ = $(makeflags_0)
+makeflags_0 = --no-print-directory -s
+makeflags_1 =
+MAKEFLAGS += $(makeflags_$(V))
+
 # cc-option
 # Usage: CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)