From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 05 Oct 2026 10:24:58 GMT
Subject: Re: [PATCH 4/7] meson: use precompiled headers for unit tests
Message-ID: <asN6-vOYWYtbiBFr@pks.im>
In-Reply-To: <eb0432fa-a595-4690-bf13-baffb306cc3a@gmail.com>

On Mon, Oct 05, 2026 at 03:04:16PM +0530, Kaartic Sivaraam wrote:
> On 9/24/26 19:39, Patrick Steinhardt wrote:
> > 
> > diff --git a/t/meson.build b/t/meson.build
> > index 3ca7b27104..9f1ee9ad59 100644
> > --- a/t/meson.build
> > +++ b/t/meson.build
> > @@ -66,6 +65,13 @@ clar_unit_tests = executable('unit-tests',
> >     c_args: [
> >       '-DGIT_CLAR_DECLS_H="' + clar_decls_h.full_path() + '"',
> >     ],
> > +  c_pch: '../tools/precompiled.h',
> > +  link_with: static_library('clar',
> > +    sources: [
> > +      'unit-tests/clar/clar.c',
> > +      clar_suite_h,
> > +    ],
> > +  ),
> 
> Compiling this separately as a static library is cool but now clar.c does
> not get the libgit_c_args it was getting through the dependencies of  the
> unit-tests executable. Is this something that we need to correct?

That's true. But I wonder whether that is maybe even an improvement.
After all, the libgit_c_args contain stuff that is relevant to Git,
only. And given that "clar.c" is a vendored dependency, it does not
really make sense to expose e.g. "-DWITH_BREAKING_CHANGES".

Now there are a small handful of arguments that _might_ be relevant, but
these are only -W-style warning flags. I don't think we really care
about those either, as again, this is a vendored dependency.

So overall I think that this is fine, but I should've maybe called this
out in the commit mesage.

Patrick