Re: [PATCH 2/3] Makefile: avoid multiple -Wall in CFLAGS

2 messages, 2 authors, 2021-09-28 · open the first message on its own page

Re: [PATCH 2/3] Makefile: avoid multiple -Wall in CFLAGS

From: Junio C Hamano <hidden>
Date: 2021-09-28 21:19:55

Carlo Marcelo Arenas Belón  [off-list ref] writes:
6163f3f1a4 (config.mak.dev: add -Wall, primarily for -Wformat, to help
autoconf users, 2018-10-12) adds a second -Wall in config.mak.dev to
workaround the lack of one from config.mak.autogen.

Since 6d5d4b4e93 (Makefile: allow for combining DEVELOPER=1 and
CFLAGS="...", 2019-02-22), that variable is set instead as part of
DEVELOPER_FLAGS which won't be overriden by config.mak.autogen, so
it can be safely removed from config.mak.dev if set instead in the
Makefile.
Hmph, don't this break non-developers, though?

They now do not get -Wall that they used to?  Or am I reading the
patch incorrectly?

Thanks.
quoted hunk
This also has the advantage of separating cleanly CFLAGS which are
used for building with the ones that provide with diagnostics.

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
---
 Makefile       | 3 ++-
 config.mak.dev | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 9df565f27b..963b9e7c6b 100644
--- a/Makefile
+++ b/Makefile
@@ -1200,7 +1200,8 @@ endif
 # Set CFLAGS, LDFLAGS and other *FLAGS variables. These might be
 # tweaked by config.* below as well as the command-line, both of
 # which'll override these defaults.
-CFLAGS = -g -O2 -Wall
+CFLAGS = -g -O2
+DEVELOPER_CFLAGS = -Wall
 LDFLAGS =
 CC_LD_DYNPATH = -Wl,-rpath,
 BASIC_CFLAGS = -I.
diff --git a/config.mak.dev b/config.mak.dev
index c81be62a5c..90c47d2782 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -6,7 +6,6 @@ ifeq ($(filter no-error,$(DEVOPTS)),)
 DEVELOPER_CFLAGS += -Werror
 SPARSE_FLAGS += -Wsparse-error
 endif
-DEVELOPER_CFLAGS += -Wall
 ifeq ($(filter no-pedantic,$(DEVOPTS)),)
 DEVELOPER_CFLAGS += -pedantic
 ifneq ($(filter clang4 gcc5,$(COMPILER_FEATURES)),)

Re: [PATCH 2/3] Makefile: avoid multiple -Wall in CFLAGS

From: Carlo Arenas <hidden>
Date: 2021-09-28 23:23:11

On Tue, Sep 28, 2021 at 2:19 PM Junio C Hamano [off-list ref] wrote:
Carlo Marcelo Arenas Belón  [off-list ref] writes:
quoted
6163f3f1a4 (config.mak.dev: add -Wall, primarily for -Wformat, to help
autoconf users, 2018-10-12) adds a second -Wall in config.mak.dev to
workaround the lack of one from config.mak.autogen.

Since 6d5d4b4e93 (Makefile: allow for combining DEVELOPER=1 and
CFLAGS="...", 2019-02-22), that variable is set instead as part of
DEVELOPER_FLAGS which won't be overriden by config.mak.autogen, so
it can be safely removed from config.mak.dev if set instead in the
Makefile.
Hmph, don't this break non-developers, though?

They now do not get -Wall that they used to?  Or am I reading the
patch incorrectly?
It is subtle and apparently problematic as some users[1] rely on
removing -Wall to support non GNU compilers, but my change kept the
-Wall by default by making sure it got set in DEVELOPER_FLAGS instead
(which as Ævar points out might be a layering violation and hence
might need more planning).

Either way I'll be dropping this series for now, to make sure that the
minimal change is put forward as part of js/win-lazyload-buildfix
instead.

Carlo

[1] https://lore.kernel.org/git/CAPUEspj_hOXRc2d+c+DTvShgWX8NH+fXKD4Pk+_G=nj9Z97VnQ@mail.gmail.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help