Re: [PATCH] meson: Check whether git is new enough to support ls-files --deduplicate
- From
Martin Storsjö <martin@martin.st>
- Date
- Aug 1, 2025, 07:55 UTC
- Message-ID
- <69faab5-43d4-812c-90f-c518ff7f618@martin.st>
- In-Reply-To
- <aIxQWSrnMOuEmfHo@pks.im>
Hi Patrick,
On Fri, 1 Aug 2025, Patrick Steinhardt wrote:
> Our commit messages are described so that we first describe the error > and then we describe how this is fixed, and typically they are written > in such a way that they can be read without requiring you to also read > the subject line.
Thanks for the commit message rewrite!
Show 13 quoted lines
> So something like: > > When using the Meson build system with an old-enough Git version > that does not yet know the `git ls-files --deduplicate` option one > can observe the following error: > > ../meson.build:697:19: ERROR: Command `/usr/bin/git -C /home/martin/code/git ls-files --deduplicate '*.h' ':!contrib' ':!compat/inet_ntop.c' ':!compat/inet_pton.c' ':!compat/nedmalloc' ':!compat/obstack.*' ':!compat/poll' ':!compat/regex' ':!sha1collisiondetection' ':!sha1dc' ':!t/unit-tests/clar' ':!t/t[0-9][0-9][0-9][0-9]*' ':!xdiff'` failed with status 129. > > The failing command is used to find all header files in our code > base, which is required for static analysis. > > Static analysis is an entirely optional feature that distributors > typically don't care, and about we already know to skip running the
I presume this is a typo, and this should be "typically don't care about, and we already know to ..." - I've amended your suggestion in this way locally.
Show 12 quoted lines
>> headers_to_check = []
>> -if git.found() and fs.exists(meson.project_source_root() / '.git')
>> +if meson.version().version_compare('>=0.62')
>> + new_enough_git = git.found() and git.version().version_compare('>=2.31')
>> +else
>> + # On Meson 0.61, we can't check git.version(), so we just have to
>> + # assume that the found git is new enough.
>> + new_enough_git = git.found()
>
> I'd rather call this `git_supports_file_deduplication`. It's a bit of a
> mouthful, but `new_enough_git` raises the question of what it is new
> enough for.Fair enough, good suggestion. Although based on your other suggestion below, we don't end up needing this part at all.
Show 7 quoted lines
>> +endif >> +if new_enough_git and fs.exists(meson.project_source_root() / '.git') >> foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_excludes, check: true).stdout().split() > > I wonder whether we could avoid the whole version check machinery and > just change this command to `check: false`. If so we accept in case the > command fails, which we can check by calling `.returncode()`.
Thanks, this sounds like a much neater way of avoiding the whole issue!
// Martin