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

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Thu, Sep 24, 2026 at 07:52:16PM -0400, Jeff King wrote:
> 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.)

> 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

> 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.

> 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".

> 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.