Re: [Qemu-devel] [PATCH 02/12] build-sys: silence make by default

[Marc-André Lureau]

Hi

On Fri, Dec 8, 2017 at 8:19 PM, Eric Blake <address@hidden> 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 <address@hidden>
>> ---
>>  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
>



-- 
Marc-André Lureau