Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic
From: Junio C Hamano <hidden>
Date: 2021-09-22 18:28:44
Possibly related (same subject, not in this thread)
- 2021-09-23 · Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic · Junio C Hamano <hidden>
- 2021-09-23 · Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic · Jeff King <hidden>
- 2021-09-23 · Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic · Junio C Hamano <hidden>
- 2021-09-22 · Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic · Ævar Arnfjörð Bjarmason <hidden>
- 2021-09-22 · Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic · Jeff King <hidden>
Jeff King [off-list ref] writes:
quoted
I wonder if the attached (with clean-up to remove the tracing cruft) would show us a better direction. It feeds a single line int dummy_for_dep_check; C "program" from the standard input of the compiler to tackle the "you are not supposed to be compiling an empty compilation unit" problem in a more direct way.That feels a bit like we're playing a game of chicken with the compiler in terms of what it may complain about. For example, sparse will complain: foo.c:1:5: warning: symbol 'dummy_for_dep_check' was not declared. Should it be static? Might compilers ever learn to warn of the same thing?
Certainly. That is the reason why I said "direction", not
"solution", and I do not think it is beyond our capability to come
up with a minimal "C program" that would be lint clean to make it
as a part of the "solution".
For example, would sparse or compilers complain about this?
extern int incr(int); int incr(int i) { return i + 1; }
quoted hunk
So I'd argue we should go even simpler, like:diff --git a/Makefile b/Makefile index 3628d14f16..4597a126d0 100644 --- a/Makefile +++ b/Makefile@@ -1277,7 +1277,7 @@ COMPUTE_HEADER_DEPENDENCIES = auto endif ifeq ($(COMPUTE_HEADER_DEPENDENCIES),auto) -dep_check = $(shell $(CC) $(ALL_CFLAGS) \ +dep_check = $(shell $(CC) \ -c -MF /dev/null -MQ /dev/null -MMD -MP \ -x c /dev/null -o /dev/null 2>&1; \ echo $$?)
I am all for this simplification. It takes us back to the state before 1816bf26 (Makefile: Improve compiler header dependency check, 2011-08-30). But I think that is more or less orthogonal to the "you are not supposed to feed an empty compilation unit" issue.
quoted hunk
I'm also tempted by a hunk like this. Then we can set the REQUIRE flag in a CI job (or locally for git devs who know they have gcc) and notice an unexpected breakage in the auto test.@@ -1295,6 +1295,9 @@ ifneq ($(COMPUTE_HEADER_DEPENDENCIES),no) $(error please set COMPUTE_HEADER_DEPENDENCIES to yes, no, or auto \ (not "$(COMPUTE_HEADER_DEPENDENCIES)")) endif +ifdef REQUIRE_COMPUTE_HEADER_DEPENDENCIES +$(error computed header dependencies required, but auto-check did not find them) +endif endif
Yup, I like that, too.