Re: [PATCH v2] add DEVELOPER makefile knob to check for acknowledged warnings

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Feb 25, 2016 at 9:42 AM,  <larsxschneider@gmail.com> wrote:
> From: Lars Schneider <larsxschneider@gmail.com>
>
> We assume Git developers have a reasonably modern compiler and recommend
> them to enable the DEVELOPER makefile knob to ensure their patches are
> clear of all compiler warnings the Git core project cares about.
>
> Enable the DEVELOPER makefile knob in the Travis-CI build.
>
> Suggested-by: Jeff King <peff@peff.net>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>
> This patch is the successor of "[PATCH v1] travis-ci: override CFLAGS properly,
> add -Wdeclaration-after-statement" [1] which enables compiler warnings for the
> Travis-CI build.
>
> Peff suggested to codify the knowledge about the compiler warnings the Git
> project cares about [2] which I have done here.
>
> The only problem is the "-Wold-style-declaration" compiler warning as this is
> only supported by GCC and not Clang. Should we ignore that warning or would you
> prefer to detect the GCC compiler and add the warning? The Linux kernel project
> does a similar thing [3].
>
>
> Thanks,
> Lars
>
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/285752
> [2] http://article.gmane.org/gmane.comp.version-control.git/285761
> [3] https://github.com/torvalds/linux/blob/6dc390ad61ac8dfca5fa9b0823981fb6f7ec17a0/Makefile#L303-L306
>
>
>  .travis.yml                    |  2 +-
>  Documentation/CodingGuidelines |  4 ++++
>  Makefile                       | 12 ++++++++++++
>  3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index c3bf9c6..168ae21 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -15,12 +15,12 @@ addons:
>
>  env:
>    global:
> +    - DEVELOPER=1
>      - P4_VERSION="15.2"
>      - GIT_LFS_VERSION="1.1.0"
>      - DEFAULT_TEST_TARGET=prove
>      - GIT_PROVE_OPTS="--timer --jobs 3"
>      - GIT_TEST_OPTS="--verbose --tee"
> -    - CFLAGS="-g -O2 -Wall -Werror"
>      - GIT_TEST_CLONE_2GB=YesPlease
>      # t9810 occasionally fails on Travis CI OS X
>      # t9816 occasionally fails with "TAP out of sequence errors" on Travis CI OS X
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index c6e536f..1c676c2 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -171,6 +171,10 @@ For C programs:
>
>   - We try to keep to at most 80 characters per line.
>
> + - As a Git developer we assume you have a reasonably modern compiler
> +   and we recommend you to enable the DEVELOPER makefile knob to
> +   ensure your patch is clear of all compiler warnings we care about.
> +
>   - We try to support a wide range of C compilers to compile Git with,
>     including old ones. That means that you should not use C99
>     initializers, even if a lot of compilers grok it.
> diff --git a/Makefile b/Makefile
> index fd19b54..9d4614d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -380,6 +380,18 @@ 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
>
> --
> 2.5.1

[I realize this patch has long since been integrated]

Is there any way with this to both supply CFLAGS & DEVELOPER=1 on the
command-line, to get my custom -O<whatever> & these -W flags? I.e.:

$ make DEVELOPER=1 V=1
[...] -g -O2 -Wall -Werror -Wdeclaration-after-statement
-Wno-format-zero-length -Wold-style-definition -Woverflow
-Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -I. [...]
$ make DEVELOPER=1 CFLAGS="-g -O0 -Wall" V=1
[...] -g -O0 -Wall -I. [...]

I thought the second case would prepend my "-g -O0 -Wall" but then be
followed by the various -W developer flags, but it isn't, am I just
doing something stupid, or is there no way to combine these two?