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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Sep 25, 2026, 09:25 UTC
Message-ID: <arY+JMPe0KWucyja@szeder.dev>
In-Reply-To: <20260924235216.GA837070@coredump.intra.peff.net>

On Thu, Sep 24, 2026 at 07:52:16PM -0400, Jeff King wrote:

Show 26 quoted lines

> On Tue, Sep 15, 2026 at 08:09:52AM +0200, SZEDER Gábor wrote:
> 
> > This patch follows the idea of 671df48df8 (meson: precompile
> > "git-compat-util.h", 2026-03-19) to make it faster to build Git using
> > "make".  The notable differences are the boilerplate needed to wire up
> > the precompiled header with "make", and the selection of object files
> > that are built using the precompiled header:
> 
> I got an interesting error message from this today:
> 
>   $ make imap-send.o
>       * new build flags
>       CC tools/precompiled.h.gch
>       CC imap-send.o
>   cc1: warning: ./tools/precompiled.h.gch: not used because ‘NO_OPENSSL’ is defined [-Winvalid-pch]
> 
> You won't see it with:
> 
>   make NO_OPENSSL=1 imap-send.o
> 
> The culprit is that I have this in my config.mak:
> 
>   imap-send.o: EXTRA_CPPFLAGS += -DNO_OPENSSL
> 
> so the build options for the precompiled header and imap-send.c are not
> the same.

Hrm. I've run into this with 'make git.o' and the other object files for which we set EXTRA_CPPFLAGS in our Makefile, and wrote about it at length in the commit message. I thought omitting EXTRA_CPPFLAGS from the command building the precompiled header solved this issue, and was puzzled at first why your use case still causes problems... The reason for the difference is that none of the EXTRA_CPPFLAGS we set in our Makefile affect 'git-compat-util.h', but -DNO_OPENSSL does.

If you set such a custom EXTRA_CPPFLAGS, then you might as well append '-Wno-invalid-pch' to it to silence that warning. The rule building object files using the precompiled header has '-Winvalid-pch' near the beginning while EXTRA_CPPFLAGS are near the end, so we can override it from EXTRA_CPPFLAGS. I didn't find a way to override '-include precompiled.h'.

(Btw, can you do something like this with Meson? :) Without resorting to creating yet another static library, of course.)

Show 6 quoted lines

> So now of course you are asking why I would have such a weird
> line in my config.mak.
> 
> The answer is that I want to disable openssl for old builds, because I
> am often building historical versions which use openssl constructs that
> are deprecated or removed.

Well, for the same reason I have the following in my config.mak:

  # Build knobs to build older versions:
  # 1ed2c7b115 (imap-send: use HMAC() function provided by OpenSSL, 2016-04-09)
  ifeq ($(shell git merge-base --is-ancestor 1ed2c7b11570f5d16bdc70d151fa78c3dccf6d38 HEAD 2>/dev/null; echo $$?),1)
    $(warning Setting NO_OPENSSL for old revisions)
    NO_OPENSSL = UnfortunatelyYes
  endif

Show 7 quoted lines

> So naturally you are now asking why it does
> not just say:
> 
>   NO_OPENSSL = BrokenOnOldVersions
> 
> or similar. But that breaks _some_ old versions which really do need
> openssl for various things.

I haven't run into any such breakages with disabling OPENSSL for the whole build... but maybe I just haven't built old enough versions?! Anyway, will adapt it to your EXTRA_CPPFLAGS trick, thanks.

Show 5 quoted lines

> The good-ish news is that it's mostly cosmetic for me. I also loosen
> -Werror for old builds, for obvious reasons. So it's not breaking any
> build.
> 
> I don't know if my use case is too crazy to care about

I would say so, yes ;)

> but I thought
> I'd mention it in case there are other less-crazy related cases we might
> run into.

Not sure what those less crazy use cases might be, but I'm inclined to say that "If you deliberately set a custom EXTRA_CPPFLAGS that affects 'git-compat-util.h', then you should also add '-Wno-invalid-pch' as well".

Show 5 quoted lines

> And yes, obviously old versions will not have the precompiled header,
> either, but my logic for "loosen compilation" is mostly "we are not on a
> branch nor rebasing", so a sight-seeing trip to "git checkout
> origin/seen" puts me in the same mode. And eventually it _will_ be old,
> too. ;)

I'm not sure about loosening compilation for 'seen', especially when it comes to DEVELOPER=1, because it's best to catch any issues with DEVELOPER=1 while the commit is still only in 'seen'. My config.mak doesn't set DEVELOPER=1 when bisecting or when building a revision reachable from a tagged release.