| Message ID | 20210928091054.78895-2-carenas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Makefile: tighten pedantic flag | expand |
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
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > In any case, I wonder if we should at least be better off with the > diff-at-the-end on top (untested). > ... > --- 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 > + It does sound like a better organization to "normalize" different spellings early so that later users of the macro can pretend that there is no "MINGW64_BLA-foo" to worry about. > 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 Thanks.
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 pathsep = ; HAVE_ALLOCA_H = YesPlease NO_PREAD = YesPlease @@ -705,6 +706,8 @@ ifeq ($(uname_S),QNX) NO_STRLCPY = YesPlease endif +export uname_S + vcxproj: # Require clean work tree git update-index -q --refresh && \
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(-)