Re: [PATCH 1/3] Makefile: restrict -Wpedantic and -Wno-pedantic-ms-format better

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

On Tue, Sep 28 2021, Carlo Marcelo Arenas Belón wrote:

> 6a8cbc41ba (developer: enable pedantic by default, 2021-09-03)
> enables pedantic mode in as many compilers as possible to help gather
> feedback on future tightening of the net, so lets do so.
>
> -Wpedantic is missing in some really old gcc 4 versions so lets restrict
> it to gcc5 and clang4 (it does work in clang3 AFAIK, but it will be
> unlikely a developer will use such an old compiler anyway).
>
> MinGW gcc is the only one which has -Wno-pedantic-ms-format, and while
> that is available also in older compilers, the Windows SDK provides gcc10
> so lets aim for that.  Note that in order to target the flag to only
> Windows, additional changes were needed in config.mak.uname to propagate
> the OS detection done there.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  config.mak.dev   | 6 +++++-
>  config.mak.uname | 3 +++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/config.mak.dev b/config.mak.dev
> index cdf043c52b..c81be62a5c 100644
> --- a/config.mak.dev
> +++ b/config.mak.dev
> @@ -9,11 +9,15 @@ endif
>  DEVELOPER_CFLAGS += -Wall
>  ifeq ($(filter no-pedantic,$(DEVOPTS)),)
>  DEVELOPER_CFLAGS += -pedantic
> +ifneq ($(filter clang4 gcc5,$(COMPILER_FEATURES)),)
>  DEVELOPER_CFLAGS += -Wpedantic
> -ifneq ($(filter gcc5,$(COMPILER_FEATURES)),)
> +ifneq ($(filter gcc10,$(COMPILER_FEATURES)),)
> +ifeq ($(uname_S),MINGW)
>  DEVELOPER_CFLAGS += -Wno-pedantic-ms-format
>  endif
>  endif
> +endif
> +endif
>  DEVELOPER_CFLAGS += -Wdeclaration-after-statement
>  DEVELOPER_CFLAGS += -Wformat-security
>  DEVELOPER_CFLAGS += -Wold-style-definition
> diff --git a/config.mak.uname b/config.mak.uname
> index 76516aaa9a..aa68bbdec7 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -589,6 +589,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
>  	SHELL_PATH = /usr/coreutils/bin/bash
>  endif
>  ifneq (,$(findstring MINGW,$(uname_S)))
> +	uname_S := MINGW

Just in terms of implementation, this somewhat pre-dates your change,
but every single other uname we check as a constant. I wonder if this
"findstring" is really needed under MINGW. It was added in f4626df51f6
(Add target architecture MinGW., 2007-12-01).

(Goes and seaches stackoverflow etc.)

Ah yes, it seems it'll emit e.g. "MINGW64_NT-10.0", ew!

In any case, I wonder if we should at least be better off with the
diff-at-the-end on top (untested).

And also not necessarily for this series, but IMO this sort of thing
really longer-term belongs in config.mak.uname (or maybe a
config.mak.dev.uname, ew!). Well, maybe. Anyway, looking at potentially
implementing that we get into similar ordering issues as I noted in my
2/3 comment, i.e. we'd have to hoist "COMPILER_FEATURES" over to the
main Makefile before including both.

So nevermind I guess, but aside from which variable we set/override
where (and feel very free to ignore my musings there) this change LGTM.

diff --git a/config.mak.uname b/config.mak.uname
index aa68bbdec72..0028891ac67 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -11,6 +11,10 @@ uname_R := $(shell sh -c 'uname -r 2>/dev/null || echo not')
 uname_P := $(shell sh -c 'uname -p 2>/dev/null || echo not')
 uname_V := $(shell sh -c 'uname -v 2>/dev/null || echo not')
 
+ifneq (,$(findstring MINGW,$(uname_S)))
+	uname_S := MINGW
+endif
+
 ifdef MSVC
 	# avoid the MingW and Cygwin configuration sections
 	uname_S := Windows
@@ -588,8 +592,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
 	SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
 	SHELL_PATH = /usr/coreutils/bin/bash
 endif
-ifneq (,$(findstring MINGW,$(uname_S)))
-	uname_S := MINGW
+ifeq ($(uname_S),MINGW)
 	pathsep = ;
 	HAVE_ALLOCA_H = YesPlease
 	NO_PREAD = YesPlease