git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: "Martin Storsjö" <martin@martin.st>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] meson: Check whether git is new enough to support ls-files --deduplicate
Date: Fri, 1 Aug 2025 07:27:53 +0200	[thread overview]
Message-ID: <aIxQWSrnMOuEmfHo@pks.im> (raw)
In-Reply-To: <20250731121533.178747-1-martin@martin.st>

On Thu, Jul 31, 2025 at 03:15:30PM +0300, Martin Storsjö wrote:
> This fixes Meson errors like this:

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.

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
    command when we are not in a Git repository. But we do not handle
    the above failure gracefully, even though we could.

    Fix this by detecting whether the Git version is new enough to
    support the `--deduplicate` option. Unfortunately, Meson only
    supports the external_program.version() method since Meson 0.62. So
    with older versions of Meson, we have to just assume that it exists
    (or maybe assume that it doesn't).

>     ../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.
> 
> Unfortunately, Meson only supports the external_program.version()
> method since Meson 0.62. So with older versions of Meson, we have
> to just assume that it exists (or maybe assume that it doesn't).
> 
> Signed-off-by: Martin Storsjö <martin@martin.st>
> ---
>  meson.build | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index 9bc1826cb6..717365baec 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -693,7 +693,14 @@ third_party_excludes = [
>  ]
>  
>  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.

> +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!

Patrick

  reply	other threads:[~2025-08-01  5:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-31 12:15 [PATCH] meson: Check whether git is new enough to support ls-files --deduplicate Martin Storsjö
2025-08-01  5:27 ` Patrick Steinhardt [this message]
2025-08-01  7:55   ` Martin Storsjö
2025-08-01  7:56     ` [PATCH v2] meson: Tolerate errors from git " Martin Storsjö
2025-08-01  9:23       ` Patrick Steinhardt
2025-08-01  9:42         ` Martin Storsjö
2025-08-01  9:45           ` Patrick Steinhardt
2025-08-01 10:24             ` Martin Storsjö
2025-08-01 10:25               ` [PATCH v3] " Martin Storsjö
2025-08-01 14:44                 ` Patrick Steinhardt
2025-08-01 15:59       ` [PATCH v2] " Junio C Hamano
2025-08-01 16:27         ` Martin Storsjö
2025-08-01 16:28           ` [PATCH v4] " Martin Storsjö

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aIxQWSrnMOuEmfHo@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=martin@martin.st \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).