Re: [PATCH v2 4/4] Makefile: precompile "git-compat-util.h"

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Oct 3, 2026, 19:00 UTC
Message-ID: <asFQxeh+IUGrlu6W@szeder.dev>
In-Reply-To: <20260915060952.569535-5-szeder.dev@gmail.com>

On Tue, Sep 15, 2026 at 08:09:52AM +0200, SZEDER Gábor wrote:

Show 8 quoted lines

>     - The precompiled header should not change what actually gets
>       compiled.  Therefore, use the precompiled header only when
>       compiling source files that start with including
>       "git-compat-util.h" (directly or indirectly, e.g. via
>       "builtin.h"), or its inclusion is only preceeded by #define
>       directives that don't influence "git-compat-util.h" between its
>       include guards [2] (currently DISABLE_SIGN_COMPARE_WARNINGS,
>       USE_THE_REPOSITORY_VARIABLE or GIT_TEST_PROGRESS_ONLY). [3]

Well, it turns out the precompiled header does change what gets compiled, and it causes a visible behavior difference, though I think it's really minor.

The crux of the issue is that without the precompiled header the compiler processes "git-compat-util.h", but with it, for some reason, it processes "./git-compat-util.h".

This is visible when expanding __FILE__ in "git-compat-util.h", e.g. in the assert macro in regexec_buf(). When the assertion is triggered, e.g. with this diff:

diff --git a/common-main.c b/common-main.c
index 6b7ab077b0..dd2a90c849 100644
--- a/common-main.c
+++ b/common-main.c
@@ -5,6 +5,9 @@ int main(int argc, const char **argv)
 {
 	int result;
 
+	/* Intentionally bogus regexec_buf() call to trigger its assert() */
+	regexec_buf(NULL, NULL, 0, 0, NULL, 0);
+
 	init_git(argv);
 	result = cmd_main(argc, argv);
 
Then without the precompiled header we get:

  $ ./git
  git: git-compat-util.h:1002: regexec_buf: Assertion `nmatch > 0 && pmatch' failed.
  Aborted (core dumped)

But with the precompiled header:

  $ ./git
  git: ./git-compat-util.h:1002: regexec_buf: Assertion `nmatch > 0 && pmatch' failed.
  Aborted (core dumped)

Similar could happen when the ALLOC_GROW_BY() macro is invoked with
bogus parameters to trigger a BUG().

(Sidenote: While this assert does prevent us from invoking regexec()
with nonsense, the source file name and line number in the resulting
error message are not as useful as they could be, it would be better
to show the caller's filename and line number.)

Since in "git-compat-util.h" __FILE__ is only expanded in error
messages that should basically never happen (BUG() and assert()), I
think this is acceptable.


BTW, this is also visible in compiler error messages:

diff --git a/git-compat-util.h b/git-compat-util.h
index a0f901ce79..00c1f26911 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1,6 +1,8 @@
 #ifndef GIT_COMPAT_UTIL_H
 #define GIT_COMPAT_UTIL_H
 
+trigger_compiler_error
+
 #if __STDC_VERSION__ - 0 < 199901L
 /*
  * Git is in a testing period for mandatory C99 support in the compiler.  If

Without precompiled header:

      CC daemon.o
  In file included from daemon.c:3:
  git-compat-util.h:4:23: error: expected ‘;’ before ‘typedef’
      4 | trigger_compiler_error
        |                       ^
        |                       ;

With precompiled header:

      CC tools/precompiled.h.gch
  In file included from tools/precompiled.h:1:
  ./git-compat-util.h:4:23: error: expected ‘;’ before ‘typedef’
      4 | trigger_compiler_error
        |                       ^
        |                       ;