Re: [PATCH 1/3] Makefile: add verbosity and debug options to Makefile

[Todor Minchev <todor.minchev@linux.intel.com>]

On Mon, 2017-02-06 at 11:06 -0800, Jianxun Zhang wrote:
> Todor,
> Please refer to my 2 inline comments.
> 
> > On Feb 2, 2017, at 2:37 PM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> > 
> > By default Makefile verbosity is disabled (V=0). Verbosity can be enabled by
> > setting the V environment variable to any value not equal to 0 (e.g V=1)
> > 
> > Example:
> > make clean V=1; make V=1
> > 
> > A debug version of the rmc binary can be built by using the debug
> > Makefile target. This will include debug symbols and will disable compiler
> > optimizations when compiling rmc.
> > 
> > Example:
> > 
> > make debug
> > 
> > Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> > ---
> > Makefile | 31 +++++++++++++++++++++----------
> > 1 file changed, 21 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 9ade775..d85d8e9 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1,5 +1,12 @@
> > # Copyright (C) 2016 Jianxun Zhang <jianxun.zhang@intel.com>
> > 
> > +V ?= 0
> > +ifeq ($(V),0)
> > +  VERBOSITY = @
> > +else
> > +  VERBOSITY =
> > +endif
> > +
> I am thinking maybe we should remove ‘@‘ in rules and use -s option of make (in recipe) when we want to mute the output. This should reach the same effect without bothering new variables.
> 
> We still enable/disable output as a whole anyway.
> 
> Let me know if this proposal works for you.

Sounds good. Removing the @ will make it possible to toggle the
verbosity with 'make -s' without introducing any extra variables. 

> 
> > TOPDIR = $(shell if [ -z "$$PWD" ]; then pwd; else echo "$$PWD"; fi)
> > 
> > RMC_TOOL_SRC := $(wildcard src/*.c)
> > @@ -20,28 +27,32 @@ RMC_INSTALL_HEADER_PATH := $(RMC_INSTALL_PREFIX)/include/rmc/
> > 
> > ALL_OBJS := $(RMC_TOOL_OBJ) $(RMC_LIB_OBJ)
> > 
> > +
> > RMC_CFLAGS := -Wall -I$(TOPDIR)/inc
> > 
> > all: rmc
> > +debug: RMC_CFLAGS += -DDEBUG -g -O0
> > +debug: rmc
> I am not sure if this is necessary because we already have CFLAGS. I think you can reach the same effect without adding a new debug target:
> make CFLAGS='-DDEBUG -g -O0’
> 

Yup, the above will have the same effect as 'make debug'.
I thought that 'make debug' might be a convenient way to build a debug
binary? Do you think it makes sense to keep this extra target for
convenience since it doesn't affect the other usages of make?

> also refer to commit e8b48e6 in rmc project.
> 
> > 
> > $(ALL_OBJS): %.o: %.c
> > -	@$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> > +	$(VERBOSITY)$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> > 
> > librmc: $(RMC_LIB_OBJ)
> > -	@$(AR) rcs src/lib/$@.a $^
> > +	$(VERBOSITY)$(AR) rcs src/lib/$@.a $^
> > 
> > rmc: $(RMC_TOOL_OBJ) librmc
> > -	@$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) src/lib/librmc.a -o src/$@
> > +	$(VERBOSITY)$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) \
> > +  src/lib/librmc.a -o src/$@
> > 
> > clean:
> > -	@rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
> > +	$(VERBOSITY)rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
> > 
> > .PHONY: clean rmc librmc
> > 
> > install:
> > -	@mkdir -p $(RMC_INSTALL_BIN_PATH)
> > -	@install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
> > -	@mkdir -p $(RMC_INSTALL_LIB_PATH)
> > -	@install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
> > -	@mkdir -p $(RMC_INSTALL_HEADER_PATH)
> > -	@install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
> > +	$(VERBOSITY)mkdir -p $(RMC_INSTALL_BIN_PATH)
> > +	$(VERBOSITY)install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
> > +	$(VERBOSITY)mkdir -p $(RMC_INSTALL_LIB_PATH)
> > +	$(VERBOSITY)install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
> > +	$(VERBOSITY)mkdir -p $(RMC_INSTALL_HEADER_PATH)
> > +	$(VERBOSITY)install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
> > -- 
> > 2.11.0
> > 
>