| Message ID | 20171208005825.14587-3-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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.
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 >
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)
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(+)