The "COMPUTE_HEADER_DEPENDENCIES" feature added in [1] was extended to
use auto-detection in [2]. Then when -Wpedantic support was added to
DEVOPTS in [3] we started passing -Wpedantic in combination with
-Werror to the compiler here.
This broke the auto-detection, but since we'd quieted it in [4] we
didn't find out. It was emitting all of this on STDERR under GCC:
/dev/null:1: error: ISO C forbids an empty translation unit
[-Werror=pedantic]
cc1: note: unrecognized command-line option
‘-Wno-pedantic-ms-format’ may have been intended to silence
earlier diagnostics
cc1: all warnings being treated as errors
Let's fix that bug by maintaining a NON_DEVELOPER_CFLAGS, it's like
ALL_CFLAGS but without anything we add in config.mak.dev, and
furthermore stop redirecting STDERR to /dev/null, this means that
someone whose compiler doesn't support this will see this output, but
also this new message:
Non-zero 1 exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect
It's also possible that some compilers will emit warnings but still
give a zero exit code, anyone using a compiler like that will
potentially get more verbose output from the Makefile until they set
COMPUTE_HEADER_DEPENDENCIES=no. E.g. on AIX's xlc we'll now emit:
/opt/IBM/xlc/13.1.3/bin/.orig/xlc: 1501-208 (S) command option D is missing a subargument
Non-zero 40 exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect
And on Solaris with SunCC:
cc: Warning: Option -x passed to ld, if ld is invoked, ignored otherwise
cc: refused to overwrite input file by output file: /dev/null
cc: Warning: Option -x passed to ld, if ld is invoked, ignored otherwise
cc: refused to overwrite input file by output file: /dev/null
Non-zero 1 exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect
Both are quieted by setting COMPUTE_HEADER_DEPENDENCIES=no as
suggested.
I considered piping the output and the exit code to a variable
instead, but e.g. under GCC that would lose the coloring in the error
messages.
1. f2fabbf76e4 (Teach Makefile to check header dependencies,
2010-01-26)
2. 111ee18c31f (Makefile: Use computed header dependencies if the
compiler supports it, 2011-08-18)
3. 729b3925ed9 (Makefile: add a DEVOPTS flag to get pedantic
compilation, 2018-07-24)
4. 6a8cbc41bac (developer: enable pedantic by default, 2021-09-03)
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
Makefile | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
From: Carlo Arenas <hidden> Date: 2021-09-22 10:55:53
On Wed, Sep 22, 2021 at 3:38 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:
The "COMPUTE_HEADER_DEPENDENCIES" feature added in [1] was extended to
use auto-detection in [2]. Then when -Wpedantic support was added to
DEVOPTS in [3] we started passing -Wpedantic in combination with
-Werror to the compiler here.
It is my impression though that header dependencies computation is
something that might only work in gcc and clang (because of its gcc
compatibility), so shouldn't this be restricted to those compilers
instead of forcing all others to error?
Also, why is this process run with DEVELOPER=1 to begin with, if we
obviously don't need/want any compilations warnings?
Carlo
On Wed, Sep 22, 2021 at 3:38 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:
quoted
The "COMPUTE_HEADER_DEPENDENCIES" feature added in [1] was extended to
use auto-detection in [2]. Then when -Wpedantic support was added to
DEVOPTS in [3] we started passing -Wpedantic in combination with
-Werror to the compiler here.
It is my impression though that header dependencies computation is
something that might only work in gcc and clang (because of its gcc
compatibility), so shouldn't this be restricted to those compilers
instead of forcing all others to error?
I think it's better to just have those not supporting
COMPUTE_HEADER_DEPENDENCIES define that, wanting to auto-detect things
is what led to the current breakage.
I.e. our whitelisting of GCC and Clang will be out of date if some other
compiler grows support for this, and even if we whitelist gcc and clang
we'll need some script like detect-compiler etc.
Also, why is this process run with DEVELOPER=1 to begin with, if we
obviously don't need/want any compilations warnings?
Hrm? We run the COMPUTE_HEADER_DEPENDENCIES=yes process with and without
DEVELOPER=1, it's just that DEVELOPER=1 breaks it since it turns on
pedantic compilation flags now.
We want -Werror for the actual compilation under DEVELOPER=1, but a
one-off command to see if a compiler has support for something is
entirely different.
The "COMPUTE_HEADER_DEPENDENCIES" feature added in [1] was extended to
use auto-detection in [2], that "auto" detection has always piped
STDERR to /dev/null, so any failures on compilers that didn't support
these GCC flags would silently fall back to
"COMPUTE_HEADER_DEPENDENCIES=no".
Later when -Wpedantic support was added to DEVOPTS in [3] we started
passing -Wpedantic in combination with -Werror to the compiler
here. Note (to the pedantic): [3] actually passed "-pedantic", but it
and "-Wpedantic" are synonyms.
Turning on -Wpedantic in [3] broke the auto-detection, since this
relies on compiling an empty program. GCC would loudly complain on
STDERR:
/dev/null:1: error: ISO C forbids an empty translation unit
[-Werror=pedantic]
cc1: note: unrecognized command-line option
‘-Wno-pedantic-ms-format’ may have been intended to silence
earlier diagnostics
cc1: all warnings being treated as errors
But as that ended up in the "$(dep_check)" variable due to the "2>&1"
in [2] we didn't see it.
Then when [4] made DEVOPTS=pedantic the default specifying
"DEVELOPER=1" would effectively set "COMPUTE_HEADER_DEPENDENCIES=no".
To fix these issues let's unconditionally pass -Wno-pedantic after
$(ALL_CFLAGS), we might get a -Wpedantic via config.mak.dev after, or
the builder might specify it via CFLAGS. In either case this will undo
current and future problems with -Wpedantic.
I think it would make sense to simply remove the "2>&1", it would mean
that anyone using a non-GCC-like compiler would get warnings under
COMPUTE_HEADER_DEPENDENCIES=auto, e.g on AIX's xlc would emit:
/opt/IBM/xlc/13.1.3/bin/.orig/xlc: 1501-208 (S) command option D is missing a subargument
Non-zero 40 exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect
And on Solaris with SunCC:
cc: Warning: Option -x passed to ld, if ld is invoked, ignored otherwise
cc: refused to overwrite input file by output file: /dev/null
cc: Warning: Option -x passed to ld, if ld is invoked, ignored otherwise
cc: refused to overwrite input file by output file: /dev/null
Non-zero 1 exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect
Both could be quieted by setting COMPUTE_HEADER_DEPENDENCIES=no
explicitly, as suggested, but let's see if this'll fix it without
emitting too much noise at those that aren't using "gcc" or "clang".
1. f2fabbf76e4 (Teach Makefile to check header dependencies,
2010-01-26)
2. 111ee18c31f (Makefile: Use computed header dependencies if the
compiler supports it, 2011-08-18)
3. 729b3925ed9 (Makefile: add a DEVOPTS flag to get pedantic
compilation, 2018-07-24)
4. 6a8cbc41bac (developer: enable pedantic by default, 2021-09-03)
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
Makefile | 1 +
1 file changed, 1 insertion(+)
In this v2 I just added an unconditional -Wno-pedantic and omitted the
change to spew error on STDERR. This more narrowly fixes the immediate
issue and doesn't get into whether we should use /dev/null or whatever
as input.
I then noticed that the same bug was present in
"GENERATE_COMPILATION_DATABASE=yes", so there's now a 2nd patch to fix
that.
Ævar Arnfjörð Bjarmason (2):
Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with
DEVOPTS=pedantic
Makefile: pass -Wno-pendantic under GENERATE_COMPILATION_DATABASE=yes
Makefile | 2 ++
1 file changed, 2 insertions(+)
Range-diff against v1:
1: 3ff8ea12bf3 ! 1: 31c871e9bf6 Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic
@@ Commit message
Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic
The "COMPUTE_HEADER_DEPENDENCIES" feature added in [1] was extended to
- use auto-detection in [2]. Then when -Wpedantic support was added to
- DEVOPTS in [3] we started passing -Wpedantic in combination with
- -Werror to the compiler here.
+ use auto-detection in [2], that "auto" detection has always piped
+ STDERR to /dev/null, so any failures on compilers that didn't support
+ these GCC flags would silently fall back to
+ "COMPUTE_HEADER_DEPENDENCIES=no".
- This broke the auto-detection, but since we'd quieted it in [4] we
- didn't find out. It was emitting all of this on STDERR under GCC:
+ Later when -Wpedantic support was added to DEVOPTS in [3] we started
+ passing -Wpedantic in combination with -Werror to the compiler
+ here. Note (to the pedantic): [3] actually passed "-pedantic", but it
+ and "-Wpedantic" are synonyms.
+
+ Turning on -Wpedantic in [3] broke the auto-detection, since this
+ relies on compiling an empty program. GCC would loudly complain on
+ STDERR:
/dev/null:1: error: ISO C forbids an empty translation unit
[-Werror=pedantic]
@@ Commit message
earlier diagnostics
cc1: all warnings being treated as errors
- Let's fix that bug by maintaining a NON_DEVELOPER_CFLAGS, it's like
- ALL_CFLAGS but without anything we add in config.mak.dev, and
- furthermore stop redirecting STDERR to /dev/null, this means that
- someone whose compiler doesn't support this will see this output, but
- also this new message:
+ But as that ended up in the "$(dep_check)" variable due to the "2>&1"
+ in [2] we didn't see it.
- Non-zero 1 exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect
+ Then when [4] made DEVOPTS=pedantic the default specifying
+ "DEVELOPER=1" would effectively set "COMPUTE_HEADER_DEPENDENCIES=no".
+
+ To fix these issues let's unconditionally pass -Wno-pedantic after
+ $(ALL_CFLAGS), we might get a -Wpedantic via config.mak.dev after, or
+ the builder might specify it via CFLAGS. In either case this will undo
+ current and future problems with -Wpedantic.
- It's also possible that some compilers will emit warnings but still
- give a zero exit code, anyone using a compiler like that will
- potentially get more verbose output from the Makefile until they set
- COMPUTE_HEADER_DEPENDENCIES=no. E.g. on AIX's xlc we'll now emit:
+ I think it would make sense to simply remove the "2>&1", it would mean
+ that anyone using a non-GCC-like compiler would get warnings under
+ COMPUTE_HEADER_DEPENDENCIES=auto, e.g on AIX's xlc would emit:
/opt/IBM/xlc/13.1.3/bin/.orig/xlc: 1501-208 (S) command option D is missing a subargument
Non-zero 40 exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect
@@ Commit message
cc: refused to overwrite input file by output file: /dev/null
Non-zero 1 exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect
- Both are quieted by setting COMPUTE_HEADER_DEPENDENCIES=no as
- suggested.
-
- I considered piping the output and the exit code to a variable
- instead, but e.g. under GCC that would lose the coloring in the error
- messages.
+ Both could be quieted by setting COMPUTE_HEADER_DEPENDENCIES=no
+ explicitly, as suggested, but let's see if this'll fix it without
+ emitting too much noise at those that aren't using "gcc" or "clang".
1. f2fabbf76e4 (Teach Makefile to check header dependencies,
2010-01-26)
@@ Commit message
Signed-off-by: Ævar Arnfjörð Bjarmason [off-list ref]
## Makefile ##
-@@ Makefile: ALL_COMMANDS_TO_INSTALL += git-upload-archive$(X)
- ALL_COMMANDS_TO_INSTALL += git-upload-pack$(X)
- endif
-
--ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
-+NON_DEVELOPER_CFLAGS = $(CPPFLAGS) $(CFLAGS)
-+ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(NON_DEVELOPER_CFLAGS)
- ALL_LDFLAGS = $(LDFLAGS)
-
- comma := ,
-@@ Makefile: COMPUTE_HEADER_DEPENDENCIES = auto
- endif
+@@ Makefile: endif
ifeq ($(COMPUTE_HEADER_DEPENDENCIES),auto)
--dep_check = $(shell $(CC) $(ALL_CFLAGS) \
-+dep_check = $(shell $(CC) $(NON_DEVELOPER_CFLAGS) \
+ dep_check = $(shell $(CC) $(ALL_CFLAGS) \
++ -Wno-pedantic \
-c -MF /dev/null -MQ /dev/null -MMD -MP \
-- -x c /dev/null -o /dev/null 2>&1; \
-+ -x c /dev/null -o /dev/null; \
+ -x c /dev/null -o /dev/null 2>&1; \
echo $$?)
- ifeq ($(dep_check),0)
- override COMPUTE_HEADER_DEPENDENCIES = yes
- else
-+$(info Non-zero $(dep_check) exit with COMPUTE_HEADER_DEPENDENCIES=auto, set it to "yes" or "no" to quiet auto-detect)
- override COMPUTE_HEADER_DEPENDENCIES = no
- endif
- endif
-: ----------- > 2: 6b18bd08894 Makefile: pass -Wno-pendantic under GENERATE_COMPILATION_DATABASE=yes
--
2.33.0.1225.g9f062250122
The same bug fixed in the "COMPUTE_HEADER_DEPENDENCIES=auto" mode in
the preceding commit was also present with
"GENERATE_COMPILATION_DATABASE=yes". Let's fix it so it works again
with "DEVOPTS=1".
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
Makefile | 1 +
1 file changed, 1 insertion(+)
From: Jeff King <hidden> Date: 2021-09-23 17:38:30
On Thu, Sep 23, 2021 at 12:08:00AM +0200, Ævar Arnfjörð Bjarmason wrote:
In this v2 I just added an unconditional -Wno-pedantic and omitted the
change to spew error on STDERR. This more narrowly fixes the immediate
issue and doesn't get into whether we should use /dev/null or whatever
as input.
FWIW, this seems perfectly fine to me. Removing $(ALL_CFLAGS) entirely
seems OK to me, too, but this is a smaller change, and would help any
cases where those flags are somehow important to getting the compiler to
function at all.
It's possible some compiler _does_ understand -MF, etc, but not
-Wno-pedantic, but that seems unlikely to me. (And of course there is
always the fallback of setting COMPUTE_HEADER_DEPENDENCIES yourself on
such a system; this is really just about doing the right thing on most
people's setups).
Another related alternative is to use -Wno-error, which would fix the
pedantic problem, along with any other warnings a compiler chooses to
bring up for an empty file. I don't have a strong opinion on one versus
the other.
-Peff