Re: [RCF/PATCH] Makefile: move 'ifdef DEVELOPER' after config.mak* inclusion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Matthieu Moy <Matthieu.Moy@xxxxxxx> writes:

> The DEVELOPER knob was introduced in 658df95 (add DEVELOPER makefile
> knob to check for acknowledged warnings, 2016-02-25), and works well
> when used as "make DEVELOPER=1", and when the configure script was not
> used.
>
> However, the advice given in CodingGuidelines to add DEVELOPER=1 to
> config.mak does not: config.mak is included after testing for
> DEVELOPER in the Makefile, and at least GNU Make's manual specifies
> "Conditional directives are parsed immediately", hence the config.mak
> declaration is not visible at the time the conditional is evaluated.
>
> Also, when using the configure script to generate a
> config.mak.autogen, the later file contained a "CFLAGS = <flags>"
> initialization, which overrode the "CFLAGS += -W..." triggered by
> DEVELOPER.
>
> This patch fixes both issues.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxx>
> ---
> I'm surprised that no one noticed the issue. Probably because the
> Makefile is silent by default. Did I miss anything obvious?

Probably because the overlap of the population that use DEVELOPER=1
and config.mak is miniscule?  

When you work in multiple "git worktrees" (or its equivalent), it is
far more convenient to have a single "make" wrapper that you use
everywhere than to make sure that you copy (or symlink) a config.mak
into each and every one of them.

In any case, this change is a right thing to do.  Thanks for
noticing.

>  Makefile | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 15fcd57..2226319 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -380,18 +380,6 @@ ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
>  ALL_LDFLAGS = $(LDFLAGS)
>  STRIP ?= strip
>  
> -ifdef DEVELOPER
> -CFLAGS += -Werror \
> -	-Wdeclaration-after-statement \
> -	-Wno-format-zero-length \
> -	-Wold-style-definition \
> -	-Woverflow \
> -	-Wpointer-arith \
> -	-Wstrict-prototypes \
> -	-Wunused \
> -	-Wvla
> -endif
> -
>  # Create as necessary, replace existing, make ranlib unneeded.
>  ARFLAGS = rcs
>  
> @@ -952,6 +940,18 @@ include config.mak.uname
>  -include config.mak.autogen
>  -include config.mak
>  
> +ifdef DEVELOPER
> +CFLAGS += -Werror \
> +	-Wdeclaration-after-statement \
> +	-Wno-format-zero-length \
> +	-Wold-style-definition \
> +	-Woverflow \
> +	-Wpointer-arith \
> +	-Wstrict-prototypes \
> +	-Wunused \
> +	-Wvla
> +endif
> +
>  ifndef sysconfdir
>  ifeq ($(prefix),/usr)
>  sysconfdir = /etc
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]