Re: [PATCH 2/2] configure.ac: use GIT_CC_CHECK_FLAG_APPEND for adding --with-gcc-warnings configure option
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:50
Elia Pinto [off-list ref] writes:
quoted hunk
Use the GIT_CC_CHECK_FLAGS_APPEND autoconf macro for add in a portable way the new configure option --enable-gcc-warnings (default off). Signed-off-by: Elia Pinto <redacted> --- Makefile | 12 ++++++-- configure.ac | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 103 insertions(+), 5 deletions(-)diff --git a/Makefile b/Makefile index 23485f1..9db34e2 100644 --- a/Makefile +++ b/Makefile@@ -344,11 +344,9 @@ GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN -include GIT-VERSION-FILE -GIT_CFLAGS = -GIT_LDFLAGS = # CFLAGS and LDFLAGS are for the users to override from the command line. -CFLAGS = -g -O2 -Wall $(GIT_CFLAGS) +CFLAGS = -g -O2 -Wall LDFLAGS = $(GIT_LDFLAGS) ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS)
Reverting what you did just a minute ago? I am debating myself if I should say that the new placement may make more sense. On the one hand, -Wanything is more or less developer's personal taste and should be easily overridable via CFLAGS for each invocation of "make", just like -g and -O2. On the other hand, some class of error checking is so useful and bulletproof not to give us false positives that we may want to encourage people to always use them when available. The placement in 1/2 was in line with the former, but the updated placement makes it very hard to selectively disable GCC crud that barfs unnecessarily without disabling everything by GIT_CFLAGS="", I am afraid. Looking at the way the patch to configure.ac tries to add many things, I am not sure if these two patches are good idea (note: I didn't say "I am sure this is going in a wrong direction"). Is it adding everything under the sun, or after careful consideration on each and every ones to see how false-positive-prone it is?
quoted hunk
@@ -1517,6 +1515,14 @@ ifdef DEFAULT_HELP_FORMAT BASIC_CFLAGS += -DDEFAULT_HELP_FORMAT='"$(DEFAULT_HELP_FORMAT)"' endif +ifdef GIT_CFLAGS +BASIC_CFLAGS += $(GIT_CFLAGS) +endif + +ifdef GIT_LDFLAGS +BASIC_LDFLAGS += $(GIT_LDFLAGS) +endif + ALL_CFLAGS += $(BASIC_CFLAGS) ALL_LDFLAGS += $(BASIC_LDFLAGS)diff --git a/configure.ac b/configure.ac index c67ca60..95d5d10 100644 --- a/configure.ac +++ b/configure.ac@@ -1,4 +1,4 @@ -# -*- Autoconf -*- +# -*- Autoconf -*- \ # Process this file with autoconf to produce a configure script. ## Definitions of private macros.@@ -433,7 +433,99 @@ AS_HELP_STRING([],[ARG is the full path to the Tcl/Tk interpreter.]) AS_HELP_STRING([],[Bare --with-tcltk will make the GUI part only if]) AS_HELP_STRING([],[Tcl/Tk interpreter will be found in a system.]), GIT_PARSE_WITH(tcltk)) -# + + +# Turn gcc warning + +AC_ARG_ENABLE([gcc-warnings], + [AS_HELP_STRING([--enable-gcc-warnings], + [turn on GCC warnings (for developers)@<:@default=no@:>@])], + [case $enableval in + yes|no) ;; + *) AC_MSG_ERROR([bad value $enableval for gcc-warnings option]) ;; + esac + git_gcc_warnings=$enableval], + [git_gcc_warnings=no] +) + +AS_IF([test "x$git_gcc_warnings" = xyes ],[ +# Add/Delete as needed +GIT_CC_CHECK_FLAGS_APPEND([with_cflags], [CFLAGS], [\ + -Wall \ + -Wextra \ + -Wformat-y2k \ + -fdiagnostics-show-option \ + -funit-at-a-time \ + -fstrict-aliasing \ + -Wstrict-overflow \ + -fstrict-overflow \ + -Wpointer-arith \ + -Wundef \ + -Wformat-security \ + -Winit-self \ + -Wmissing-include-dirs \ + -Wunused \ + -Wunknown-pragmas \ + -Wstrict-aliasing \ + -Wshadow \ + -Wbad-function-cast \ + -Wcast-align \ + -Wwrite-strings \ + -Wlogical-op \ + -Waggregate-return \ + -Wstrict-prototypes \ + -Wold-style-definition \ + -Wmissing-prototypes \ + -Wmissing-declarations \ + -Wmissing-noreturn \ + -Wmissing-format-attribute \ + -Wredundant-decls \ + -Wnested-externs \ + -Winline \ + -Winvalid-pch \ + -Wvolatile-register-var \ + -Wdisabled-optimization \ + -Wbuiltin-macro-redefined \ + -Wmudflap \ + -Wpacked-bitfield-compat \ + -Wsync-nand \ + -Wattributes \ + -Wcoverage-mismatch \ + -Wmultichar \ + -Wcpp \ + -Wdeprecated-declarations \ + -Wdiv-by-zero \ + -Wdouble-promotion \ + -Wendif-labels \ + -Wformat-contains-nul \ + -Wformat-extra-args \ + -Wformat-zero-length \ + -Wformat=2 \ + -Wmultichar \ + -Wnormalized=nfc \ + -Woverflow \ + -Wpointer-to-int-cast \ + -Wpragmas \ + -Wsuggest-attribute=const \ + -Wsuggest-attribute=noreturn \ + -Wsuggest-attribute=pure \ + -Wtrampolines \ + -Wno-missing-field-initializers \ + -Wno-sign-compare \ + -Wjump-misses-init \ + -Wno-format-nonliteral \ + -fstack-protector-all \ + -fasynchronous-unwind-tables \ + -fdiagnostics-show-option \ + -funit-at-a-time \ + -fipa-pure-const \ + -Wno-aggregate-return \ + -Wno-redundant-decls \ + -Wdeclaration-after-statement ]) + +GIT_CONF_SUBST([GIT_CFLAGS],[$with_cflags]) +]) + ## Checks for programs.