git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] meson: Check whether git is new enough to support ls-files --deduplicate
@ 2025-07-31 12:15 Martin Storsjö
  2025-08-01  5:27 ` Patrick Steinhardt
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Storsjö @ 2025-07-31 12:15 UTC (permalink / raw)
  To: git

This fixes Meson errors like this:

    ../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()
+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()
     headers_to_check += header
   endforeach
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] meson: Check whether git is new enough to support ls-files --deduplicate
  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
  2025-08-01  7:55   ` Martin Storsjö
  0 siblings, 1 reply; 13+ messages in thread
From: Patrick Steinhardt @ 2025-08-01  5:27 UTC (permalink / raw)
  To: Martin Storsjö; +Cc: git

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] meson: Check whether git is new enough to support ls-files --deduplicate
  2025-08-01  5:27 ` Patrick Steinhardt
@ 2025-08-01  7:55   ` Martin Storsjö
  2025-08-01  7:56     ` [PATCH v2] meson: Tolerate errors from git " Martin Storsjö
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Storsjö @ 2025-08-01  7:55 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git

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!

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

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

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] meson: Tolerate errors from git ls-files --deduplicate
  2025-08-01  7:55   ` Martin Storsjö
@ 2025-08-01  7:56     ` Martin Storsjö
  2025-08-01  9:23       ` Patrick Steinhardt
  2025-08-01 15:59       ` [PATCH v2] " Junio C Hamano
  0 siblings, 2 replies; 13+ messages in thread
From: Martin Storsjö @ 2025-08-01  7:56 UTC (permalink / raw)
  To: git; +Cc: ps

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 about, and 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 passing `check: false` to `run_command`, which makes it
tolerate failures. Then check `returncode()` manually to decide
whether to inspect the output.

Signed-off-by: Martin Storsjö <martin@martin.st>
---
 meson.build | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index 9bc1826cb6..9b519e6eed 100644
--- a/meson.build
+++ b/meson.build
@@ -694,9 +694,12 @@ third_party_excludes = [
 
 headers_to_check = []
 if git.found() 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()
-    headers_to_check += header
-  endforeach
+  ls_headers = run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_excludes, check: false)
+  if ls_headers.returncode() == 0
+    foreach header : ls_headers.stdout().split()
+      headers_to_check += header
+    endforeach
+  endif
 endif
 
 if not get_option('breaking_changes')
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] meson: Tolerate errors from git ls-files --deduplicate
  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 15:59       ` [PATCH v2] " Junio C Hamano
  1 sibling, 1 reply; 13+ messages in thread
From: Patrick Steinhardt @ 2025-08-01  9:23 UTC (permalink / raw)
  To: Martin Storsjö; +Cc: git

On Fri, Aug 01, 2025 at 10:56:22AM +0300, Martin Storsjö wrote:
> 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 about, and 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 passing `check: false` to `run_command`, which makes it
> tolerate failures. Then check `returncode()` manually to decide
> whether to inspect the output.
> 
> Signed-off-by: Martin Storsjö <martin@martin.st>
> ---
>  meson.build | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 9bc1826cb6..9b519e6eed 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -694,9 +694,12 @@ third_party_excludes = [
>  
>  headers_to_check = []
>  if git.found() 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()
> -    headers_to_check += header
> -  endforeach
> +  ls_headers = run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_excludes, check: false)
> +  if ls_headers.returncode() == 0
> +    foreach header : ls_headers.stdout().split()
> +      headers_to_check += header
> +    endforeach
> +  endif
>  endif

Yup, this looks reasonable to me. We could have an `else` branch that
warns about the command failing, for example like this:

    warning("could not find headers: " + ls_headers.stderr())

But other than that I'm happy.

Thanks!

Patrick

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] meson: Tolerate errors from git ls-files --deduplicate
  2025-08-01  9:23       ` Patrick Steinhardt
@ 2025-08-01  9:42         ` Martin Storsjö
  2025-08-01  9:45           ` Patrick Steinhardt
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Storsjö @ 2025-08-01  9:42 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]

On Fri, 1 Aug 2025, Patrick Steinhardt wrote:

> On Fri, Aug 01, 2025 at 10:56:22AM +0300, Martin Storsjö wrote:
>> index 9bc1826cb6..9b519e6eed 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -694,9 +694,12 @@ third_party_excludes = [
>>
>>  headers_to_check = []
>>  if git.found() 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()
>> -    headers_to_check += header
>> -  endforeach
>> +  ls_headers = run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_excludes, check: false)
>> +  if ls_headers.returncode() == 0
>> +    foreach header : ls_headers.stdout().split()
>> +      headers_to_check += header
>> +    endforeach
>> +  endif
>>  endif
>
> Yup, this looks reasonable to me. We could have an `else` branch that
> warns about the command failing, for example like this:
>
>    warning("could not find headers: " + ls_headers.stderr())

This would work - however the output from ls_headers.stderr() is fairly 
long (if you try running e.g. "git ls-files --foobar", you'll get a 37 
line listing of potential options); it's rather distracting for what's 
otherwise a fairly minor build configuration issue.

Using ls_headers.stderr().split('\n')[0] works and just gets us this:

     ../meson.build:703: WARNING: could not find headers: error: unknown option `deduplicate'

However I wonder if it's worth it, or if it just makes the meson file 
potentially more brittle? (E.g. what if split() returns an array of 0 
elements? Not sure if that's possible though...)

// Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] meson: Tolerate errors from git ls-files --deduplicate
  2025-08-01  9:42         ` Martin Storsjö
@ 2025-08-01  9:45           ` Patrick Steinhardt
  2025-08-01 10:24             ` Martin Storsjö
  0 siblings, 1 reply; 13+ messages in thread
From: Patrick Steinhardt @ 2025-08-01  9:45 UTC (permalink / raw)
  To: Martin Storsjö; +Cc: git

On Fri, Aug 01, 2025 at 12:42:52PM +0300, Martin Storsjö wrote:
> On Fri, 1 Aug 2025, Patrick Steinhardt wrote:
> 
> > On Fri, Aug 01, 2025 at 10:56:22AM +0300, Martin Storsjö wrote:
> > > index 9bc1826cb6..9b519e6eed 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -694,9 +694,12 @@ third_party_excludes = [
> > > 
> > >  headers_to_check = []
> > >  if git.found() 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()
> > > -    headers_to_check += header
> > > -  endforeach
> > > +  ls_headers = run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_excludes, check: false)
> > > +  if ls_headers.returncode() == 0
> > > +    foreach header : ls_headers.stdout().split()
> > > +      headers_to_check += header
> > > +    endforeach
> > > +  endif
> > >  endif
> > 
> > Yup, this looks reasonable to me. We could have an `else` branch that
> > warns about the command failing, for example like this:
> > 
> >    warning("could not find headers: " + ls_headers.stderr())
> 
> This would work - however the output from ls_headers.stderr() is fairly long
> (if you try running e.g. "git ls-files --foobar", you'll get a 37 line
> listing of potential options); it's rather distracting for what's otherwise
> a fairly minor build configuration issue.
> 
> Using ls_headers.stderr().split('\n')[0] works and just gets us this:
> 
>     ../meson.build:703: WARNING: could not find headers: error: unknown option `deduplicate'
> 
> However I wonder if it's worth it, or if it just makes the meson file
> potentially more brittle? (E.g. what if split() returns an array of 0
> elements? Not sure if that's possible though...)

True. Maybe we can just not include stderr at all but say:

    could not list headers, disabling static analysis targets

That's probably sufficient.

Patrick

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] meson: Tolerate errors from git ls-files --deduplicate
  2025-08-01  9:45           ` Patrick Steinhardt
@ 2025-08-01 10:24             ` Martin Storsjö
  2025-08-01 10:25               ` [PATCH v3] " Martin Storsjö
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Storsjö @ 2025-08-01 10:24 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 2094 bytes --]

On Fri, 1 Aug 2025, Patrick Steinhardt wrote:

> On Fri, Aug 01, 2025 at 12:42:52PM +0300, Martin Storsjö wrote:
>> On Fri, 1 Aug 2025, Patrick Steinhardt wrote:
>>
>>> On Fri, Aug 01, 2025 at 10:56:22AM +0300, Martin Storsjö wrote:
>>>> index 9bc1826cb6..9b519e6eed 100644
>>>> --- a/meson.build
>>>> +++ b/meson.build
>>>> @@ -694,9 +694,12 @@ third_party_excludes = [
>>>>
>>>>  headers_to_check = []
>>>>  if git.found() 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()
>>>> -    headers_to_check += header
>>>> -  endforeach
>>>> +  ls_headers = run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_excludes, check: false)
>>>> +  if ls_headers.returncode() == 0
>>>> +    foreach header : ls_headers.stdout().split()
>>>> +      headers_to_check += header
>>>> +    endforeach
>>>> +  endif
>>>>  endif
>>>
>>> Yup, this looks reasonable to me. We could have an `else` branch that
>>> warns about the command failing, for example like this:
>>>
>>>    warning("could not find headers: " + ls_headers.stderr())
>>
>> This would work - however the output from ls_headers.stderr() is fairly long
>> (if you try running e.g. "git ls-files --foobar", you'll get a 37 line
>> listing of potential options); it's rather distracting for what's otherwise
>> a fairly minor build configuration issue.
>>
>> Using ls_headers.stderr().split('\n')[0] works and just gets us this:
>>
>>     ../meson.build:703: WARNING: could not find headers: error: unknown option `deduplicate'
>>
>> However I wonder if it's worth it, or if it just makes the meson file
>> potentially more brittle? (E.g. what if split() returns an array of 0
>> elements? Not sure if that's possible though...)
>
> True. Maybe we can just not include stderr at all but say:
>
>    could not list headers, disabling static analysis targets
>
> That's probably sufficient.

Thanks, that sounds reasonable!

// Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3] meson: Tolerate errors from git ls-files --deduplicate
  2025-08-01 10:24             ` Martin Storsjö
@ 2025-08-01 10:25               ` Martin Storsjö
  2025-08-01 14:44                 ` Patrick Steinhardt
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Storsjö @ 2025-08-01 10:25 UTC (permalink / raw)
  To: git; +Cc: ps

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 about, and 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 passing `check: false` to `run_command`, which makes it
tolerate failures. Then check `returncode()` manually to decide
whether to inspect the output.

Signed-off-by: Martin Storsjö <martin@martin.st>
---
 meson.build | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index 9bc1826cb6..10a6dbc639 100644
--- a/meson.build
+++ b/meson.build
@@ -694,9 +694,14 @@ third_party_excludes = [
 
 headers_to_check = []
 if git.found() 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()
-    headers_to_check += header
-  endforeach
+  ls_headers = run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_excludes, check: false)
+  if ls_headers.returncode() == 0
+    foreach header : ls_headers.stdout().split()
+      headers_to_check += header
+    endforeach
+  else
+    warning('could not list headers, disabling static analysis targets')
+  endif
 endif
 
 if not get_option('breaking_changes')
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] meson: Tolerate errors from git ls-files --deduplicate
  2025-08-01 10:25               ` [PATCH v3] " Martin Storsjö
@ 2025-08-01 14:44                 ` Patrick Steinhardt
  0 siblings, 0 replies; 13+ messages in thread
From: Patrick Steinhardt @ 2025-08-01 14:44 UTC (permalink / raw)
  To: Martin Storsjö; +Cc: git

On Fri, Aug 01, 2025 at 01:25:41PM +0300, Martin Storsjö wrote:
> 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 about, and 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 passing `check: false` to `run_command`, which makes it
> tolerate failures. Then check `returncode()` manually to decide
> whether to inspect the output.

Thanks, this version looks good to me!

Patrick

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] meson: Tolerate errors from git ls-files --deduplicate
  2025-08-01  7:56     ` [PATCH v2] meson: Tolerate errors from git " Martin Storsjö
  2025-08-01  9:23       ` Patrick Steinhardt
@ 2025-08-01 15:59       ` Junio C Hamano
  2025-08-01 16:27         ` Martin Storsjö
  1 sibling, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2025-08-01 15:59 UTC (permalink / raw)
  To: Martin Storsjö; +Cc: git, ps

Martin Storsjö <martin@martin.st> writes:

> When using the Meson build system with an old-enough Git version

It would be good to be more specific what you mean by old-enough.
93a7d983 (ls-files.c: add --deduplicate option, 2021-01-23) appeared
in 2.31-rc0, so

    with versions of Git before 2.31

perhaps.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] meson: Tolerate errors from git ls-files --deduplicate
  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ö
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Storsjö @ 2025-08-01 16:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, ps

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

On Fri, 1 Aug 2025, Junio C Hamano wrote:

> Martin Storsjö <martin@martin.st> writes:
>
>> When using the Meson build system with an old-enough Git version
>
> It would be good to be more specific what you mean by old-enough.
> 93a7d983 (ls-files.c: add --deduplicate option, 2021-01-23) appeared
> in 2.31-rc0, so
>
>    with versions of Git before 2.31
>
> perhaps.

Right, that's even clearer - will resend with amended commit message.

// Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4] meson: Tolerate errors from git ls-files --deduplicate
  2025-08-01 16:27         ` Martin Storsjö
@ 2025-08-01 16:28           ` Martin Storsjö
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Storsjö @ 2025-08-01 16:28 UTC (permalink / raw)
  To: git; +Cc: gitster, ps

When using the Meson build system with versions of Git before 2.31,
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 about, and 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 passing `check: false` to `run_command`, which makes it
tolerate failures. Then check `returncode()` manually to decide
whether to inspect the output.

Signed-off-by: Martin Storsjö <martin@martin.st>
---
v4: Clarified the affected span of Git versions - no changes to the
patch itself.
---
 meson.build | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index 9bc1826cb6..10a6dbc639 100644
--- a/meson.build
+++ b/meson.build
@@ -694,9 +694,14 @@ third_party_excludes = [
 
 headers_to_check = []
 if git.found() 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()
-    headers_to_check += header
-  endforeach
+  ls_headers = run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_excludes, check: false)
+  if ls_headers.returncode() == 0
+    foreach header : ls_headers.stdout().split()
+      headers_to_check += header
+    endforeach
+  else
+    warning('could not list headers, disabling static analysis targets')
+  endif
 endif
 
 if not get_option('breaking_changes')
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-08-01 16:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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ö

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