[x264-devel] [PATCH 1/2] Add FFmpeg-style silent build rules

[Stephen Hutchinson]

On Fri, Apr 4, 2014 at 1:32 PM, Diego Biurrun <diego at biurrun.de> wrote:
> This was written by Mans back in the day, so you should not set yourself
> as author.  Also, the idea of silencing the build comes from autotools,
> śo these are autotools-style silent build rules, not FFmpeg-style.
>

Some time ago I'd actually tried searching the git repo to find which commit
introduced them, but I clearly wasn't using the right search term (I found it
just now by looking for 'brief' instead of 'silent').  Will fix.

> On Fri, Apr 04, 2014 at 09:41:53AM -0400, Stephen Hutchinson wrote:
>> Similar to autotools, the V variable can be passed directly to
>> make in order to enable or disable the silent rules. V=0 (the
>> default) turns them on, anything other than zero turns them off.
>>
>> The --verbose-build-rules option has been added for those that
>> want to disable them through configure instead.
>
> I'm skeptical of the usefulness of the configure option.
>

Mostly it was just for convenience, but it also makes sure neither
the main build process nor the post-build recipes get silenced.  I
remember having trouble passing V=1 to 'make install'.

>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2,6 +2,18 @@
>>
>>  include config.mak
>>
>> +# Silent make rules, adapted from FFmpeg's common.mak
>> +ifeq ($(V), 0)
>> +ECHO   = printf "$(1)\t%s\n" $(2)
>> +BRIEF  = CC AS YASM AR LD STRIP CP RC RANLIB
>> +
>> +MSG    = $@
>> +M      = @$(call ECHO,$(TAG),$@);
>> +$(foreach VAR,$(BRIEF), \
>> +    $(eval override $(VAR) = @$$(call ECHO,$(VAR),$$(MSG)); $($(VAR))))
>> +endif
>
> Why did you leave out SILENT?
>

Originally it was because there were problems with INSTALL ().  Having SILENT
was then split off into the second patch.

>> @@ -199,9 +211,17 @@ $(OBJS) $(OBJASM) $(OBJSO) $(OBJCLI) $(OBJCHK): .depend
>>
>> +# Force a newline after generating each .depend, or else we get spammed with
>> +# '@printf: not found' errors when the silent build rules are enabled. The
>> +# define is necessary in order to actually use typical newline \n notation.
>> +define \n
>> +
>> +
>> +endef
>
> This looks suspiciously like a hack to paper over some other problem.
>

The foreach that generates depend ends with a ; which causes the silent rules'
printf to get confused:

$ make
 CC     .depend
/bin/sh: @printf: command not found
 CXX    .depend
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
/bin/sh: @printf: command not found
CC     src/core/matroskaparser.o
 CC     src/core/stdiostream.o
 CXX    src/core/audiosource.o

Forcing a newline into the foreach after .depend; stops the printf
error, but in its place, it'll print a line
for every source file echoed into .depend:

$ make
 CC     .depend
 CC     .depend
 CC     .depend
 CC     .depend
 CC     .depend
 CC     .depend
 CC     .depend
 CC     .depend

due to CC being one of the silenced variables, and apparently that
takes priority over the
foreach being silenced.  CC_SILENT simply gives it an escape hatch and
stops the echoing (it
should probably be named CC_VERBOSE now that I think about it - the
purpose of it is to not
silence the variable, and the variable not being silent allows foreach
to be silent the way it
normally is).

>>  .depend: config.mak
>>       @rm -f .depend
>> -     @$(foreach SRC, $(addprefix $(SRCPATH)/, $(SRCS) $(SRCCLI) $(SRCSO)), $(CC) $(CFLAGS) $(SRC) $(DEPMT) $(SRC:$(SRCPATH)/%.c=%.o) $(DEPMM) 1>> .depend;)
>> +     @$(foreach SRC, $(addprefix $(SRCPATH)/, $(SRCS) $(SRCCLI) $(SRCSO)), $(CC_SILENT) $(CFLAGS) $(SRC) $(DEPMT) $(SRC:$(SRCPATH)/%.c=%.o) $(DEPMM) 1>> .depend;${\n})
>>
>> @@ -259,7 +279,7 @@ install-lib-dev:
>>
>>  install-lib-static: lib-static install-lib-dev
>>       $(INSTALL) -m 644 $(LIBX264) $(DESTDIR)$(libdir)
>> -     $(if $(RANLIB), $(RANLIB) $(DESTDIR)$(libdir)/$(LIBX264))
>> +     $(if $(RANLIB), $(RANLIB_VERBOSE) $(DESTDIR)$(libdir)/$(LIBX264))
>
> Why do you manually silence these two?
>

RANLIB_VERBOSE forcibly stops the silencing, because the recipes
invoked by install-lib-static cause RANLIB to either echo the name of the recipe
or to echo nothing at all, rather than echoing the filename of the lib
(which the call
to RANLIB earlier in the Makefile does echo correctly).  This is
also what happens to the INSTALL variable, and what made me resort to
the tactic used in the second patch of manually printf-ing the install rules.