Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic

From: Jeff King <peff@peff.net>
Date: Sep 22, 2021, 17:04 UTC
Message-ID: <YUtiNuUiBU4Xg4gw@coredump.intra.peff.net>
In-Reply-To: <xmqqmto48ufz.fsf@gitster.g>

On Wed, Sep 22, 2021 at 09:08:00AM -0700, Junio C Hamano wrote:

Show 17 quoted lines

> While I agree with your analysis of the problem, I cannot shake this
> nagging feeling that the proposed solution is barking up a wrong
> tree.  After all, -pedantic and any other option that lets the
> compiler notice that it is being asked to compile an empty source
> can come directly from the end user (e.g. CC="gcc -pedantic" or as
> part of CFLAGS)---realization of which makes me wonder if it is
> essential to compile /dev/null for this check, or any reasonably
> syntactically correct program would do.
> 
> 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?

I kind of like the simplicity of Ævar's approach. We want to know if the compiler can be invoked with options XYZ, so we do so. That should be largely independent of our cflags, and there's prior art in how we invoke it in the detect-compiler script.

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'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
 
 ifndef GENERATE_COMPILATION_DATABASE

-Peff