Re: [PATCH] kbuild: fix make V=1

From: Mike Frysinger
Date: Tue Feb 12 2008 - 10:08:11 EST


On Tuesday 12 February 2008, Oleg Verych wrote:
> On Tue, Feb 12, 2008 at 09:56:05AM +0100, Sam Ravnborg wrote:
> > On Tue, Feb 12, 2008 at 12:38:24AM +0100, Oleg Verych wrote:
> > > * Date: Mon, 11 Feb 2008 17:47:09 +0100
> > > []
> > >
> > > > Mike spotted another missing thing from his initial
> > > > patch so I folded it into the fix and pushed out
> > > > a new kbuild.git tree.
> > > >
> > > > See updated patch below.
> > >
> > > Sam, do you agree my fix was more reliable (yea, not only efficient:)?
> >
> > You more or less just reverted the original patch - so it was obviously
> > more reliable than introducing new stuff as the fix did.
> > But we are at -r1 so I prefer to get the inteded behaviour
> > and not the minmal fix.
>
> Processing below changes arguments, not semantics of generated shell
> code. And IMHO this is more reliable way of doing things. If one really
> wants silence without commonly accepted ">/dev/null 2>&1" practice, then
> choose portable "-n" argument for `echo`.
>
> - quiet_chk_filechk = echo ' CHK $@'
> -silent_chk_filechk = :
> - quiet_upd_filechk = echo ' UPD $@'
> -silent_upd_filechk = :
> +quiet_chk_filechk = ' CHK $@'
> +quiet_upd_filechk = ' UPD $@'
> +
> define filechk
> $(Q)set -e; \
> - $($(quiet)chk_filechk); \
> + echo $($(quiet)chk_filechk); \
> mkdir -p $(dir $@); \
> $(filechk_$(1)) < $< > $@.tmp; \
> if [ -r $@ ] && cmp -s $@ $@.tmp; then \
> rm -f $@.tmp; \
> else \
> - $($(quiet)upd_filechk); \
> + echo $($(quiet)upd_filechk); \
> mv -f $@.tmp $@; \
> fi
> endef

i dont see how yours is more efficient when it always runs echo. nor does it
give the same behavior ... your propposed change will echo blank lines in the
silent mode which is incorrect. it also does not seem to follow the standard
convention of other kconfig commands that have quiet/silent prefixes ... such
commands do not define arguments to an unknown program/function, nor do they
add arbitrary redirection which gets leads to confusion as to final
expansion, they define the entire command.

what Sam posted (and what was merged) makes sense to me.
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.