Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 22 Apr 2025 08:57:28 +0200

On Mon, Apr 21, 2025 at 07:54:16PM +0100, Phillip Wood wrote:
> 
> 
> On 21/04/2025 16:41, Junio C Hamano wrote:
> > Phillip Wood <[email-masked]> writes:
> > 
> > > "git ls-files" is complaining that there isn't a git
> > > repository. Looking at the output of the checkout action (reproduced
> > > below) it appears it is extracting a tarball rather than using "git
> > > clone" because git is not available. I don't know what the best way to
> > > fix that is - I guess we could run "apt-get install git" before
> > > calling the checkout action.
> > 
> > Interesting.  The use of actions/checkout@v4 is nothing new in
> > Karthik's series and we haven't seen this issue come up.  What's so
> > different with this particular series, I have to wonder...
> 
> Good Question. Looking at contrib/coccinelle/meson.build which is where the
> invocation of "git ls-files" has been moved from it starts with
> 
>     coccinelle_opt = get_option('coccinelle').require(
>       fs.exists(meson.project_source_root() / '.git'),
>       error_message: 'coccinelle can only be run from a git checkout',
>     )
> 
> I think it is probably fine to skip checking our headers and running
> coccinelle when we don't have a git repository but we should ensure the
> meson build can still be configured in that case by skipping those targets.

Agreed. We should from my perspective just disable those targets when
we either don't have Git or when the source tree is not a Git directory.

> The Makefile falls back to using "find" if "git ls-files" fails which is
> another option.

We could do that, but I wonder whether it's really worth the additional
complexity this introduces. I would expect that almost all users of
those targets would always have a Git repository available anyway. It is
very likely that for example a distributor of Git would run
"check-headers" or "coccicheck".

Our CI is a bit of an outlier here, but that should be a comparatively
easy fix, I assume.

Patrick