[PATCH 0/3] meson: add corresponding target for Makefile's hdr-check

[PATCH 0/3] meson: add corresponding target for Makefile's hdr-check

[Karthik Nayak]

To bridge the remaining gaps between Makefile and meson, this patch series adds
a new check 'headers-check' which is similar to the Makefile's 'hdr-check'.

The first two commits are small cleanups, where we re-organize existing
variables to make it easier to add the target. The third commit adds the target.

This is based on master 9d22ac5122 (The third batch, 2025-04-07) with
'es/meson-build-skip-coccinelle' merged in.

---
 contrib/coccinelle/meson.build |  29 +++------
 meson.build                    | 129 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 22 deletions(-)

Karthik Nayak (3):
      coccinelle: meson: rename variables to be more specific
      meson: move headers definition from 'contrib/coccinelle'
      meson: add support for 'headers-check'
---



---

base-commit: 3a956c5f69873611ae5f8dcb9acd117f66b95ddc
change-id: 20250330-505-wire-up-sparse-via-meson-2e32dd31208b

Thanks
- Karthik

[PATCH 1/3] coccinelle: meson: rename variables to be more specific

[Karthik Nayak]

In meson, included subdirs export their variables to top level meson
builds. In 'contrib/coccinelle/meson.build', we define two such
variables `sources` and `headers`. While these variables are specific to
the checks in the 'contrib/coccinelle/' directory, they also pollute the
top level 'meson.build'.

Rename them to be more specific, this ensures that they aren't
mistakenly used in the upper levels and avoid variable name collisions.

While here, change the empty list denotation to be consistent with other
places.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index ea054c924f..03ce52d752 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -55,18 +55,18 @@ concatenated_rules = custom_target(
   capture: true,
 )
 
-sources = [ ]
+coccinelle_sources = []
 foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.c', third_party_sources, check: true).stdout().split()
-  sources += source
+  coccinelle_sources += source
 endforeach
 
-headers = [ ]
+coccinelle_headers = []
 foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
-  headers += meson.project_source_root() / header
+  coccinelle_headers += meson.project_source_root() / header
 endforeach
 
 patches = [ ]
-foreach source : sources
+foreach source : coccinelle_sources
   patches += custom_target(
     command: [
       spatch,
@@ -78,7 +78,7 @@ foreach source : sources
     input: meson.project_source_root() / source,
     output: source.underscorify() + '.patch',
     capture: true,
-    depend_files: headers,
+    depend_files: coccinelle_headers,
   )
 endforeach
 

-- 
2.48.1

[PATCH 2/3] meson: move headers definition from 'contrib/coccinelle'

[Karthik Nayak]

The meson build for coccinelle static analysis lists all headers to
analyse. Due to the way meson exports variables between subdirs, this
variable is also available in the root meson build.

An upcoming commit, will add a new check complimenting 'hdr-check' in
the Makefile. This would require the list of headers. So move the
'coccinelle_headers' to the root meson build and rename it to 'headers',
remove the root path being appended to each header and retain that in
the coccinelle meson build since it is specific to the coccinelle build.

Also move the 'third_party_sources' variable to the root meson build
since it is also a dependency for the 'headers' variable. This also
makes it easier to understand as the variable is now propagated from the
top level to the bottom.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 17 +----------------
 meson.build                    | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index 03ce52d752..32568c5103 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -8,21 +8,6 @@ if not spatch.found()
   subdir_done()
 endif
 
-third_party_sources = [
-  ':!contrib',
-  ':!compat/inet_ntop.c',
-  ':!compat/inet_pton.c',
-  ':!compat/nedmalloc',
-  ':!compat/obstack.*',
-  ':!compat/poll',
-  ':!compat/regex',
-  ':!sha1collisiondetection',
-  ':!sha1dc',
-  ':!t/unit-tests/clar',
-  ':!t/unit-tests/clar',
-  ':!t/t[0-9][0-9][0-9][0-9]*',
-]
-
 rules = [
   'array.cocci',
   'commit.cocci',
@@ -61,7 +46,7 @@ foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files',
 endforeach
 
 coccinelle_headers = []
-foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+foreach header : headers
   coccinelle_headers += meson.project_source_root() / header
 endforeach
 
diff --git a/meson.build b/meson.build
index e98cfa4909..790d178007 100644
--- a/meson.build
+++ b/meson.build
@@ -633,6 +633,28 @@ builtin_sources = [
   'builtin/write-tree.c',
 ]
 
+third_party_sources = [
+  ':!contrib',
+  ':!compat/inet_ntop.c',
+  ':!compat/inet_pton.c',
+  ':!compat/nedmalloc',
+  ':!compat/obstack.*',
+  ':!compat/poll',
+  ':!compat/regex',
+  ':!sha1collisiondetection',
+  ':!sha1dc',
+  ':!t/unit-tests/clar',
+  ':!t/unit-tests/clar',
+  ':!t/t[0-9][0-9][0-9][0-9]*',
+]
+
+headers = []
+if git.found()
+  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+    headers += header
+  endforeach
+endif
+
 if not get_option('breaking_changes')
   builtin_sources += 'builtin/pack-redundant.c'
 endif

-- 
2.48.1

[PATCH 3/3] meson: add support for 'headers-check'

[Karthik Nayak]

The Makefile supports a target called 'hdr-check', which checks if
individual header files can be independently compiled. Let's port this
functionality to meson, our new build system too.

Let's avoid the abbreviation and name the target 'headers-check', which
is easier to read.

The implementation resembles that of the Makefile and provides the same
check.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/meson.build b/meson.build
index 790d178007..1e0ecd37d3 100644
--- a/meson.build
+++ b/meson.build
@@ -655,6 +655,12 @@ if git.found()
   endforeach
 endif
 
+headers_generated = [
+  'command-list.h',
+  'config-list.h',
+  'hook-list.h'
+]
+
 if not get_option('breaking_changes')
   builtin_sources += 'builtin/pack-redundant.c'
 endif
@@ -1995,6 +2001,107 @@ endif
 
 subdir('contrib')
 
+headers_check_exclude = headers_generated
+headers_check_exclude += [
+  'compat/apple-common-crypto.h',
+  'compat/bswap.h',
+  'compat/compiler.h',
+  'compat/disk.h',
+  'compat/fsmonitor/fsm-darwin-gcc.h',
+  'compat/fsmonitor/fsm-health.h',
+  'compat/fsmonitor/fsm-listen.h',
+  'compat/mingw.h',
+  'compat/msvc.h',
+  'compat/nedmalloc/malloc.c.h',
+  'compat/nedmalloc/nedmalloc.h',
+  'compat/nonblock.h',
+  'compat/obstack.h',
+  'compat/poll/poll.h',
+  'compat/precompose_utf8.h',
+  'compat/regex/regex.h',
+  'compat/regex/regex_internal.h',
+  'compat/sha1-chunked.h',
+  'compat/terminal.h',
+  'compat/vcbuild/include/sys/param.h',
+  'compat/vcbuild/include/sys/time.h',
+  'compat/vcbuild/include/sys/utime.h',
+  'compat/vcbuild/include/unistd.h',
+  'compat/vcbuild/include/utime.h',
+  'compat/win32.h',
+  'compat/win32/alloca.h',
+  'compat/win32/dirent.h',
+  'compat/win32/lazyload.h',
+  'compat/win32/path-utils.h',
+  'compat/win32/pthread.h',
+  'compat/win32/syslog.h',
+  'compat/zlib-compat.h',
+  't/unit-tests/clar/clar.h',
+  't/unit-tests/clar/clar/fixtures.h',
+  't/unit-tests/clar/clar/fs.h',
+  't/unit-tests/clar/clar/print.h',
+  't/unit-tests/clar/clar/sandbox.h',
+  't/unit-tests/clar/clar/summary.h',
+  't/unit-tests/clar/test/clar_test.h',
+  'unicode-width.h',
+  'xdiff/xdiff.h',
+  'xdiff/xdiffi.h',
+  'xdiff/xemit.h',
+  'xdiff/xinclude.h',
+  'xdiff/xmacros.h',
+  'xdiff/xprepare.h',
+  'xdiff/xtypes.h',
+  'xdiff/xutils.h',
+]
+
+if sha1_backend != 'openssl'
+  headers_check_exclude += 'sha1/openssl.h'
+endif
+if sha256_backend != 'openssl'
+  headers_check_exclude += 'sha256/openssl.h'
+endif
+if sha256_backend != 'nettle'
+  headers_check_exclude += 'sha256/nettle.h'
+endif
+if sha256_backend != 'gcrpyt'
+  headers_check_exclude += 'sha256/gcrypt.h'
+endif
+
+if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
+  hco_targets = []
+  foreach h : headers
+    if headers_check_exclude.contains(h)
+      continue
+    endif
+
+    hcc = custom_target(
+      input: h,
+      output: h.underscorify() + 'cc',
+      command: [
+        shell,
+        '-c',
+        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo -n \'#include "' + h + '"\' >> @OUTPUT@'
+      ]
+    )
+
+    hco = custom_target(
+      input: hcc,
+      output: h.underscorify().replace('.h', '.hco'),
+      command: [
+        compiler.cmd_array(),
+        libgit_c_args,
+        '-I', meson.project_source_root(),
+        '-I', meson.project_source_root() / 't/unit-tests',
+        '-o', '/dev/null',
+        '-c', '-xc',
+        '@INPUT@'
+      ]
+    )
+    hco_targets += hco
+  endforeach
+
+  alias_target('headers-check', hco_targets)
+endif
+
 foreach key, value : {
   'DIFF': diff.full_path(),
   'GIT_SOURCE_DIR': meson.project_source_root(),

-- 
2.48.1

Re: [PATCH 3/3] meson: add support for 'headers-check'

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> The Makefile supports a target called 'hdr-check', which checks if
> individual header files can be independently compiled. Let's port this
> functionality to meson, our new build system too.

Good goal.

> Let's avoid the abbreviation and name the target 'headers-check', which
> is easier to read.

This is a bit dubious.  Are developers supposed to keep track of
correspondence between the long establish name and the new name this
patch just came up with?  For how long?

If we make it one of our goals to name the build target in
pronounceable ways, that is perfectly fine, and it would be a good
task to allow "make headers-check" be a synonym for "make hdr-check"
(and do the same on the meson side), and deprecate hdr-check in a
cycle or two (this is not end-user facing, so the transition period
can legitimately be much shorter than usual.

But we still need to have some transition period to help those who
build from the source adjust their set-ups that have called "make
hdr-check" for a long time.

Re: [PATCH 3/3] meson: add support for 'headers-check'

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> The Makefile supports a target called 'hdr-check', which checks if
>> individual header files can be independently compiled. Let's port this
>> functionality to meson, our new build system too.
>
> Good goal.
>
>> Let's avoid the abbreviation and name the target 'headers-check', which
>> is easier to read.
>
> This is a bit dubious.  Are developers supposed to keep track of
> correspondence between the long establish name and the new name this
> patch just came up with?  For how long?
>

Fair enough.

> If we make it one of our goals to name the build target in
> pronounceable ways, that is perfectly fine, and it would be a good
> task to allow "make headers-check" be a synonym for "make hdr-check"
> (and do the same on the meson side), and deprecate hdr-check in a
> cycle or two (this is not end-user facing, so the transition period
> can legitimately be much shorter than usual.
>
> But we still need to have some transition period to help those who
> build from the source adjust their set-ups that have called "make
> hdr-check" for a long time.

Well put. My intention was that anyone using meson will anyways have to
change from `make <target>` to `meson compile <target>`. So they
wouldn't mind <target> changing too.

But it would be nicer to keep both the old target and the new
replacement and eventually remove the old target. Will do that and send
in a new version.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v2 0/4] meson: add corresponding target for Makefile's hdr-check

[Karthik Nayak]

To bridge the remaining gaps between Makefile and meson, this patch series adds
'hdr-check' to meson to compliment the Makefile's 'hdr-check'.

We also introduce 'headers-check' as an alias to 'hdr-check' as a better named 
replacement in both meson and make and add a note to deprecate 'hdr-check' in
the future.

The first two commits are small cleanups, where we re-organize existing
variables to make it easier to add the target. The third commit adds the
'hdr-check' target to meson. The last commit introduces the 'headers-check'
alias to both meson and the makefile and marks 'hdr-check' to be deprecated.

This is based on master 9d22ac5122 (The third batch, 2025-04-07) with
'es/meson-build-skip-coccinelle' merged in.

---
Changes in v2:
- Add 'hdr-check' to meson, while introducing 'headers-check' as 
  a replacement alias. Schedule 'hdr-check' to be deprecated in the future.
- Link to v1: https://lore.kernel.org/r/20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com

---
 Makefile                       |   4 +-
 ci/run-static-analysis.sh      |   2 +-
 contrib/coccinelle/meson.build |  29 +++------
 meson.build                    | 131 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 142 insertions(+), 24 deletions(-)

Karthik Nayak (4):
      coccinelle: meson: rename variables to be more specific
      meson: move headers definition from 'contrib/coccinelle'
      meson: add support for 'hdr-check'
      makefile/meson: add 'headers-check' as alias for 'hdr-check'

Range-diff versus v1:

1:  aed80c7868 = 1:  c6493671b5 coccinelle: meson: rename variables to be more specific
2:  cf2a8c50c2 = 2:  33e9b21bae meson: move headers definition from 'contrib/coccinelle'
3:  938aac6573 ! 3:  b6be631165 meson: add support for 'headers-check'
    @@ Metadata
     Author: Karthik Nayak <karthik.188@gmail.com>
     
      ## Commit message ##
    -    meson: add support for 'headers-check'
    +    meson: add support for 'hdr-check'
     
         The Makefile supports a target called 'hdr-check', which checks if
         individual header files can be independently compiled. Let's port this
    -    functionality to meson, our new build system too.
    -
    -    Let's avoid the abbreviation and name the target 'headers-check', which
    -    is easier to read.
    -
    -    The implementation resembles that of the Makefile and provides the same
    -    check.
    +    functionality to meson, our new build system too. The implementation
    +    resembles that of the Makefile and provides the same check.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
    @@ meson.build: endif
     +    hco_targets += hco
     +  endforeach
     +
    -+  alias_target('headers-check', hco_targets)
    ++  alias_target('hdr-check', hco_targets)
     +endif
     +
      foreach key, value : {
-:  ---------- > 4:  d265cd97df makefile/meson: add 'headers-check' as alias for 'hdr-check'


base-commit: 3a956c5f69873611ae5f8dcb9acd117f66b95ddc
change-id: 20250330-505-wire-up-sparse-via-meson-2e32dd31208b

Thanks
- Karthik

[PATCH v2 1/4] coccinelle: meson: rename variables to be more specific

[Karthik Nayak]

In meson, included subdirs export their variables to top level meson
builds. In 'contrib/coccinelle/meson.build', we define two such
variables `sources` and `headers`. While these variables are specific to
the checks in the 'contrib/coccinelle/' directory, they also pollute the
top level 'meson.build'.

Rename them to be more specific, this ensures that they aren't
mistakenly used in the upper levels and avoid variable name collisions.

While here, change the empty list denotation to be consistent with other
places.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index ea054c924f..03ce52d752 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -55,18 +55,18 @@ concatenated_rules = custom_target(
   capture: true,
 )
 
-sources = [ ]
+coccinelle_sources = []
 foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.c', third_party_sources, check: true).stdout().split()
-  sources += source
+  coccinelle_sources += source
 endforeach
 
-headers = [ ]
+coccinelle_headers = []
 foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
-  headers += meson.project_source_root() / header
+  coccinelle_headers += meson.project_source_root() / header
 endforeach
 
 patches = [ ]
-foreach source : sources
+foreach source : coccinelle_sources
   patches += custom_target(
     command: [
       spatch,
@@ -78,7 +78,7 @@ foreach source : sources
     input: meson.project_source_root() / source,
     output: source.underscorify() + '.patch',
     capture: true,
-    depend_files: headers,
+    depend_files: coccinelle_headers,
   )
 endforeach
 

-- 
2.48.1

[PATCH v2 2/4] meson: move headers definition from 'contrib/coccinelle'

[Karthik Nayak]

The meson build for coccinelle static analysis lists all headers to
analyse. Due to the way meson exports variables between subdirs, this
variable is also available in the root meson build.

An upcoming commit, will add a new check complimenting 'hdr-check' in
the Makefile. This would require the list of headers. So move the
'coccinelle_headers' to the root meson build and rename it to 'headers',
remove the root path being appended to each header and retain that in
the coccinelle meson build since it is specific to the coccinelle build.

Also move the 'third_party_sources' variable to the root meson build
since it is also a dependency for the 'headers' variable. This also
makes it easier to understand as the variable is now propagated from the
top level to the bottom.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 17 +----------------
 meson.build                    | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index 03ce52d752..32568c5103 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -8,21 +8,6 @@ if not spatch.found()
   subdir_done()
 endif
 
-third_party_sources = [
-  ':!contrib',
-  ':!compat/inet_ntop.c',
-  ':!compat/inet_pton.c',
-  ':!compat/nedmalloc',
-  ':!compat/obstack.*',
-  ':!compat/poll',
-  ':!compat/regex',
-  ':!sha1collisiondetection',
-  ':!sha1dc',
-  ':!t/unit-tests/clar',
-  ':!t/unit-tests/clar',
-  ':!t/t[0-9][0-9][0-9][0-9]*',
-]
-
 rules = [
   'array.cocci',
   'commit.cocci',
@@ -61,7 +46,7 @@ foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files',
 endforeach
 
 coccinelle_headers = []
-foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+foreach header : headers
   coccinelle_headers += meson.project_source_root() / header
 endforeach
 
diff --git a/meson.build b/meson.build
index e98cfa4909..790d178007 100644
--- a/meson.build
+++ b/meson.build
@@ -633,6 +633,28 @@ builtin_sources = [
   'builtin/write-tree.c',
 ]
 
+third_party_sources = [
+  ':!contrib',
+  ':!compat/inet_ntop.c',
+  ':!compat/inet_pton.c',
+  ':!compat/nedmalloc',
+  ':!compat/obstack.*',
+  ':!compat/poll',
+  ':!compat/regex',
+  ':!sha1collisiondetection',
+  ':!sha1dc',
+  ':!t/unit-tests/clar',
+  ':!t/unit-tests/clar',
+  ':!t/t[0-9][0-9][0-9][0-9]*',
+]
+
+headers = []
+if git.found()
+  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+    headers += header
+  endforeach
+endif
+
 if not get_option('breaking_changes')
   builtin_sources += 'builtin/pack-redundant.c'
 endif

-- 
2.48.1

Re: [PATCH v2 2/4] meson: move headers definition from 'contrib/coccinelle'

[Patrick Steinhardt]

On Thu, Apr 10, 2025 at 01:30:32PM +0200, Karthik Nayak wrote:
> diff --git a/meson.build b/meson.build
> index e98cfa4909..790d178007 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -633,6 +633,28 @@ builtin_sources = [
>    'builtin/write-tree.c',
>  ]
>  
> +third_party_sources = [
> +  ':!contrib',
> +  ':!compat/inet_ntop.c',
> +  ':!compat/inet_pton.c',
> +  ':!compat/nedmalloc',
> +  ':!compat/obstack.*',
> +  ':!compat/poll',
> +  ':!compat/regex',
> +  ':!sha1collisiondetection',
> +  ':!sha1dc',
> +  ':!t/unit-tests/clar',
> +  ':!t/unit-tests/clar',
> +  ':!t/t[0-9][0-9][0-9][0-9]*',
> +]
> +
> +headers = []

I think we should make sure that this variable isn't declared at all
unless `git.found()`. Otherwise, we may accidentally it it even though
it does not contain anything sensible.

Patrick

Re: [PATCH v2 2/4] meson: move headers definition from 'contrib/coccinelle'

[Karthik Nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Apr 10, 2025 at 01:30:32PM +0200, Karthik Nayak wrote:
>> diff --git a/meson.build b/meson.build
>> index e98cfa4909..790d178007 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -633,6 +633,28 @@ builtin_sources = [
>>    'builtin/write-tree.c',
>>  ]
>>
>> +third_party_sources = [
>> +  ':!contrib',
>> +  ':!compat/inet_ntop.c',
>> +  ':!compat/inet_pton.c',
>> +  ':!compat/nedmalloc',
>> +  ':!compat/obstack.*',
>> +  ':!compat/poll',
>> +  ':!compat/regex',
>> +  ':!sha1collisiondetection',
>> +  ':!sha1dc',
>> +  ':!t/unit-tests/clar',
>> +  ':!t/unit-tests/clar',
>> +  ':!t/t[0-9][0-9][0-9][0-9]*',
>> +]
>> +
>> +headers = []
>
> I think we should make sure that this variable isn't declared at all
> unless `git.found()`. Otherwise, we may accidentally it it even though
> it does not contain anything sensible.
>

Fair, I wonder if we should treat 'third_party_sources' similarly, but
since it is static, it should be fine.

> Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v2 3/4] meson: add support for 'hdr-check'

[Karthik Nayak]

The Makefile supports a target called 'hdr-check', which checks if
individual header files can be independently compiled. Let's port this
functionality to meson, our new build system too. The implementation
resembles that of the Makefile and provides the same check.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/meson.build b/meson.build
index 790d178007..6fce1aa618 100644
--- a/meson.build
+++ b/meson.build
@@ -655,6 +655,12 @@ if git.found()
   endforeach
 endif
 
+headers_generated = [
+  'command-list.h',
+  'config-list.h',
+  'hook-list.h'
+]
+
 if not get_option('breaking_changes')
   builtin_sources += 'builtin/pack-redundant.c'
 endif
@@ -1995,6 +2001,107 @@ endif
 
 subdir('contrib')
 
+headers_check_exclude = headers_generated
+headers_check_exclude += [
+  'compat/apple-common-crypto.h',
+  'compat/bswap.h',
+  'compat/compiler.h',
+  'compat/disk.h',
+  'compat/fsmonitor/fsm-darwin-gcc.h',
+  'compat/fsmonitor/fsm-health.h',
+  'compat/fsmonitor/fsm-listen.h',
+  'compat/mingw.h',
+  'compat/msvc.h',
+  'compat/nedmalloc/malloc.c.h',
+  'compat/nedmalloc/nedmalloc.h',
+  'compat/nonblock.h',
+  'compat/obstack.h',
+  'compat/poll/poll.h',
+  'compat/precompose_utf8.h',
+  'compat/regex/regex.h',
+  'compat/regex/regex_internal.h',
+  'compat/sha1-chunked.h',
+  'compat/terminal.h',
+  'compat/vcbuild/include/sys/param.h',
+  'compat/vcbuild/include/sys/time.h',
+  'compat/vcbuild/include/sys/utime.h',
+  'compat/vcbuild/include/unistd.h',
+  'compat/vcbuild/include/utime.h',
+  'compat/win32.h',
+  'compat/win32/alloca.h',
+  'compat/win32/dirent.h',
+  'compat/win32/lazyload.h',
+  'compat/win32/path-utils.h',
+  'compat/win32/pthread.h',
+  'compat/win32/syslog.h',
+  'compat/zlib-compat.h',
+  't/unit-tests/clar/clar.h',
+  't/unit-tests/clar/clar/fixtures.h',
+  't/unit-tests/clar/clar/fs.h',
+  't/unit-tests/clar/clar/print.h',
+  't/unit-tests/clar/clar/sandbox.h',
+  't/unit-tests/clar/clar/summary.h',
+  't/unit-tests/clar/test/clar_test.h',
+  'unicode-width.h',
+  'xdiff/xdiff.h',
+  'xdiff/xdiffi.h',
+  'xdiff/xemit.h',
+  'xdiff/xinclude.h',
+  'xdiff/xmacros.h',
+  'xdiff/xprepare.h',
+  'xdiff/xtypes.h',
+  'xdiff/xutils.h',
+]
+
+if sha1_backend != 'openssl'
+  headers_check_exclude += 'sha1/openssl.h'
+endif
+if sha256_backend != 'openssl'
+  headers_check_exclude += 'sha256/openssl.h'
+endif
+if sha256_backend != 'nettle'
+  headers_check_exclude += 'sha256/nettle.h'
+endif
+if sha256_backend != 'gcrpyt'
+  headers_check_exclude += 'sha256/gcrypt.h'
+endif
+
+if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
+  hco_targets = []
+  foreach h : headers
+    if headers_check_exclude.contains(h)
+      continue
+    endif
+
+    hcc = custom_target(
+      input: h,
+      output: h.underscorify() + 'cc',
+      command: [
+        shell,
+        '-c',
+        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo -n \'#include "' + h + '"\' >> @OUTPUT@'
+      ]
+    )
+
+    hco = custom_target(
+      input: hcc,
+      output: h.underscorify().replace('.h', '.hco'),
+      command: [
+        compiler.cmd_array(),
+        libgit_c_args,
+        '-I', meson.project_source_root(),
+        '-I', meson.project_source_root() / 't/unit-tests',
+        '-o', '/dev/null',
+        '-c', '-xc',
+        '@INPUT@'
+      ]
+    )
+    hco_targets += hco
+  endforeach
+
+  alias_target('hdr-check', hco_targets)
+endif
+
 foreach key, value : {
   'DIFF': diff.full_path(),
   'GIT_SOURCE_DIR': meson.project_source_root(),

-- 
2.48.1

Re: [PATCH v2 3/4] meson: add support for 'hdr-check'

[Phillip Wood]

Hi Karthik

On 10/04/2025 12:30, Karthik Nayak wrote:
> The Makefile supports a target called 'hdr-check', which checks if
> individual header files can be independently compiled. Let's port this
> functionality to meson, our new build system too. The implementation
> resembles that of the Makefile and provides the same check.

Thanks for adding this, I've left a few comments below

> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>   meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 107 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 790d178007..6fce1aa618 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -655,6 +655,12 @@ if git.found()
>     endforeach
>   endif
>   
> +headers_generated = [

To me "generated_headers" would be a more natural name and would match 
the style of "coccinelle_headers" from patch 1 as well as the equivalent 
in the Makefile.

> +  'command-list.h',
> +  'config-list.h',
> +  'hook-list.h'
> +]
> +
>   if not get_option('breaking_changes')
>     builtin_sources += 'builtin/pack-redundant.c'
>   endif
> @@ -1995,6 +2001,107 @@ endif
>   
>   subdir('contrib')
>   
> +headers_check_exclude = headers_generated
> +headers_check_exclude += [
> +  'compat/apple-common-crypto.h',
> +  'compat/bswap.h',
> +  'compat/compiler.h',
> +  'compat/disk.h',
> +  'compat/fsmonitor/fsm-darwin-gcc.h',
> +  'compat/fsmonitor/fsm-health.h',
> +  'compat/fsmonitor/fsm-listen.h',
> +  'compat/mingw.h',
> +  'compat/msvc.h',
> +  'compat/nedmalloc/malloc.c.h',
> +  'compat/nedmalloc/nedmalloc.h',
> +  'compat/nonblock.h',
> +  'compat/obstack.h',
> +  'compat/poll/poll.h',
> +  'compat/precompose_utf8.h',
> +  'compat/regex/regex.h',
> +  'compat/regex/regex_internal.h',
> +  'compat/sha1-chunked.h',
> +  'compat/terminal.h',
> +  'compat/vcbuild/include/sys/param.h',
> +  'compat/vcbuild/include/sys/time.h',
> +  'compat/vcbuild/include/sys/utime.h',
> +  'compat/vcbuild/include/unistd.h',
> +  'compat/vcbuild/include/utime.h',
> +  'compat/win32.h',
> +  'compat/win32/alloca.h',
> +  'compat/win32/dirent.h',
> +  'compat/win32/lazyload.h',
> +  'compat/win32/path-utils.h',
> +  'compat/win32/pthread.h',
> +  'compat/win32/syslog.h',
> +  'compat/zlib-compat.h',
> +  't/unit-tests/clar/clar.h',
> +  't/unit-tests/clar/clar/fixtures.h',
> +  't/unit-tests/clar/clar/fs.h',
> +  't/unit-tests/clar/clar/print.h',
> +  't/unit-tests/clar/clar/sandbox.h',
> +  't/unit-tests/clar/clar/summary.h',
> +  't/unit-tests/clar/test/clar_test.h',
> +  'unicode-width.h',
> +  'xdiff/xdiff.h',
> +  'xdiff/xdiffi.h',
> +  'xdiff/xemit.h',
> +  'xdiff/xinclude.h',
> +  'xdiff/xmacros.h',
> +  'xdiff/xprepare.h',
> +  'xdiff/xtypes.h',
> +  'xdiff/xutils.h',
> +]

Having to manually maintain this list is a bit of a shame as every time 
a new file is added to compat we need to add it to meson.build twice. 
The Makefile avoids this by filtering the list of headers used when 
building git based on their path - can we do the same here?

> +if sha1_backend != 'openssl'
> +  headers_check_exclude += 'sha1/openssl.h'
> +endif
> +if sha256_backend != 'openssl'
> +  headers_check_exclude += 'sha256/openssl.h'
> +endif
> +if sha256_backend != 'nettle'
> +  headers_check_exclude += 'sha256/nettle.h'
> +endif
> +if sha256_backend != 'gcrpyt'
> +  headers_check_exclude += 'sha256/gcrypt.h'
> +endif
> +
> +if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
> +  hco_targets = []
> +  foreach h : headers
> +    if headers_check_exclude.contains(h)
> +      continue
> +    endif
> +
> +    hcc = custom_target(
> +      input: h,
> +      output: h.underscorify() + 'cc',
> +      command: [
> +        shell,
> +        '-c',
> +        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo -n \'#include "' + h + '"\' >> @OUTPUT@'

This line is rather long. Also do we really need "echo -n" here, the 
Makefile does not use it.

> +      ]
> +    )
> +
> +    hco = custom_target(
> +      input: hcc,
> +      output: h.underscorify().replace('.h', '.hco'),
> +      command: [
> +        compiler.cmd_array(),
> +        libgit_c_args,
> +        '-I', meson.project_source_root(),
> +        '-I', meson.project_source_root() / 't/unit-tests',

Do you know why we don't need to add these include paths for the 
equivalent rule in the Makefile?

Thanks

Phillip

> +        '-o', '/dev/null',
> +        '-c', '-xc',
> +        '@INPUT@'
> +      ]
> +    )
> +    hco_targets += hco
> +  endforeach
> +
> +  alias_target('hdr-check', hco_targets)
> +endif
> +
>   foreach key, value : {
>     'DIFF': diff.full_path(),
>     'GIT_SOURCE_DIR': meson.project_source_root(),
> 

Re: [PATCH v2 3/4] meson: add support for 'hdr-check'

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Karthik
>
> On 10/04/2025 12:30, Karthik Nayak wrote:
>> The Makefile supports a target called 'hdr-check', which checks if
>> individual header files can be independently compiled. Let's port this
>> functionality to meson, our new build system too. The implementation
>> resembles that of the Makefile and provides the same check.
>
> Thanks for adding this, I've left a few comments below
>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>   meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 107 insertions(+)
>> diff --git a/meson.build b/meson.build
>> index 790d178007..6fce1aa618 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -655,6 +655,12 @@ if git.found()
>>     endforeach
>>   endif
>>   +headers_generated = [
>
> To me "generated_headers" would be a more natural name and would match
> the style of "coccinelle_headers" from patch 1 as well as the
> equivalent in the Makefile.

To me too.

>> ...
>> +  'xdiff/xtypes.h',
>> +  'xdiff/xutils.h',
>> +]
>
> Having to manually maintain this list is a bit of a shame as every
> time a new file is added to compat we need to add it to meson.build
> twice. The Makefile avoids this by filtering the list of headers used
> when building git based on their path - can we do the same here?

Yup.  It cuts both ways, but generally I would prefer that.  Your
creating a new file, and saying "git add" on it, should be a sign
enough that you intend to make it a part of the project, and the
file having a name that follows the project convention (e.g., "C
header files are in certain directories and ends with .h") should be
a sign enough that you want it to be part of the build.  Forcing
people to list them explicitly is a pain.

Yes, the approach to "glob" the list of files used would not help
you catch mistakes like forgetting to "git add" new files.  You test
locally and you commit (without the new file), and you'll be
notified by whoever cloned from you about a "missing file".  But
forcing to list the files in Makefile or meson.build would not help
you catch that kind of mistake.  While it may give you one more
chance to remind yourself that you need to "git add" by ignoring a
new file until you add it to meson.build, if you add it to the file
but not to the commit, the build procedure would not complain.

So, yes, I would pretty much agree with you and prefer to see this
kind of list go, if possible.

Thanks.

Re: [PATCH v2 3/4] meson: add support for 'hdr-check'

[Karthik Nayak]

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Karthik
>
> On 10/04/2025 12:30, Karthik Nayak wrote:
>> The Makefile supports a target called 'hdr-check', which checks if
>> individual header files can be independently compiled. Let's port this
>> functionality to meson, our new build system too. The implementation
>> resembles that of the Makefile and provides the same check.
>
> Thanks for adding this, I've left a few comments below
>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>   meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 107 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 790d178007..6fce1aa618 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -655,6 +655,12 @@ if git.found()
>>     endforeach
>>   endif
>>
>> +headers_generated = [
>
> To me "generated_headers" would be a more natural name and would match
> the style of "coccinelle_headers" from patch 1 as well as the equivalent
> in the Makefile.
>

Yeah, makes sense, I will swap out the name.

>> +  'command-list.h',
>> +  'config-list.h',
>> +  'hook-list.h'
>> +]
>> +
>>   if not get_option('breaking_changes')
>>     builtin_sources += 'builtin/pack-redundant.c'
>>   endif
>> @@ -1995,6 +2001,107 @@ endif
>>
>>   subdir('contrib')
>>
>> +headers_check_exclude = headers_generated
>> +headers_check_exclude += [
>> +  'compat/apple-common-crypto.h',
>> +  'compat/bswap.h',
>> +  'compat/compiler.h',
>> +  'compat/disk.h',
>> +  'compat/fsmonitor/fsm-darwin-gcc.h',
>> +  'compat/fsmonitor/fsm-health.h',
>> +  'compat/fsmonitor/fsm-listen.h',
>> +  'compat/mingw.h',
>> +  'compat/msvc.h',
>> +  'compat/nedmalloc/malloc.c.h',
>> +  'compat/nedmalloc/nedmalloc.h',
>> +  'compat/nonblock.h',
>> +  'compat/obstack.h',
>> +  'compat/poll/poll.h',
>> +  'compat/precompose_utf8.h',
>> +  'compat/regex/regex.h',
>> +  'compat/regex/regex_internal.h',
>> +  'compat/sha1-chunked.h',
>> +  'compat/terminal.h',
>> +  'compat/vcbuild/include/sys/param.h',
>> +  'compat/vcbuild/include/sys/time.h',
>> +  'compat/vcbuild/include/sys/utime.h',
>> +  'compat/vcbuild/include/unistd.h',
>> +  'compat/vcbuild/include/utime.h',
>> +  'compat/win32.h',
>> +  'compat/win32/alloca.h',
>> +  'compat/win32/dirent.h',
>> +  'compat/win32/lazyload.h',
>> +  'compat/win32/path-utils.h',
>> +  'compat/win32/pthread.h',
>> +  'compat/win32/syslog.h',
>> +  'compat/zlib-compat.h',
>> +  't/unit-tests/clar/clar.h',
>> +  't/unit-tests/clar/clar/fixtures.h',
>> +  't/unit-tests/clar/clar/fs.h',
>> +  't/unit-tests/clar/clar/print.h',
>> +  't/unit-tests/clar/clar/sandbox.h',
>> +  't/unit-tests/clar/clar/summary.h',
>> +  't/unit-tests/clar/test/clar_test.h',
>> +  'unicode-width.h',
>> +  'xdiff/xdiff.h',
>> +  'xdiff/xdiffi.h',
>> +  'xdiff/xemit.h',
>> +  'xdiff/xinclude.h',
>> +  'xdiff/xmacros.h',
>> +  'xdiff/xprepare.h',
>> +  'xdiff/xtypes.h',
>> +  'xdiff/xutils.h',
>> +]
>
> Having to manually maintain this list is a bit of a shame as every time
> a new file is added to compat we need to add it to meson.build twice.
> The Makefile avoids this by filtering the list of headers used when
> building git based on their path - can we do the same here?
>

The ideology in Meson is to list all files explicitly [1] and that was
what I wanted to follow. That said I understand the reasoning, and will
implement a glob match in the next version

>> +if sha1_backend != 'openssl'
>> +  headers_check_exclude += 'sha1/openssl.h'
>> +endif
>> +if sha256_backend != 'openssl'
>> +  headers_check_exclude += 'sha256/openssl.h'
>> +endif
>> +if sha256_backend != 'nettle'
>> +  headers_check_exclude += 'sha256/nettle.h'
>> +endif
>> +if sha256_backend != 'gcrpyt'
>> +  headers_check_exclude += 'sha256/gcrypt.h'
>> +endif
>> +
>> +if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>> +  hco_targets = []
>> +  foreach h : headers
>> +    if headers_check_exclude.contains(h)
>> +      continue
>> +    endif
>> +
>> +    hcc = custom_target(
>> +      input: h,
>> +      output: h.underscorify() + 'cc',
>> +      command: [
>> +        shell,
>> +        '-c',
>> +        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo -n \'#include "' + h + '"\' >> @OUTPUT@'
>
> This line is rather long. Also do we really need "echo -n" here, the
> Makefile does not use it.
>

We can remove it, I was a bit stuck here, because of how meson converts
back-slash in 'command' [2], I eventually got this working version. But
the 'echo -n' can be changed to 'echo'.

Regarding the long line, now sure if there is a better way!

>> +      ]
>> +    )
>> +
>> +    hco = custom_target(
>> +      input: hcc,
>> +      output: h.underscorify().replace('.h', '.hco'),
>> +      command: [
>> +        compiler.cmd_array(),
>> +        libgit_c_args,
>> +        '-I', meson.project_source_root(),
>> +        '-I', meson.project_source_root() / 't/unit-tests',
>
> Do you know why we don't need to add these include paths for the
> equivalent rule in the Makefile?
>

Because the Makefile builds in-tree, so the dependencies are met. Since
meson builds are out of tree, we need to explicitly add missing
dependencies. I'll add this in the commit message.

> Thanks
>
> Phillip
>

Thanks for the review!

>> +        '-o', '/dev/null',
>> +        '-c', '-xc',
>> +        '@INPUT@'
>> +      ]
>> +    )
>> +    hco_targets += hco
>> +  endforeach
>> +
>> +  alias_target('hdr-check', hco_targets)
>> +endif
>> +
>>   foreach key, value : {
>>     'DIFF': diff.full_path(),
>>     'GIT_SOURCE_DIR': meson.project_source_root(),
>>

[1]: https://mesonbuild.com/FAQ.html#why-cant-i-specify-target-files-with-a-wildcard
[2]: https://github.com/mesonbuild/meson/issues/1564

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2 3/4] meson: add support for 'hdr-check'

[Patrick Steinhardt]

On Thu, Apr 10, 2025 at 01:30:33PM +0200, Karthik Nayak wrote:
> The Makefile supports a target called 'hdr-check', which checks if
> individual header files can be independently compiled. Let's port this
> functionality to meson, our new build system too. The implementation

Nit: Meson is typically spelt with an upper-case 'M'.

> resembles that of the Makefile and provides the same check.
> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 790d178007..6fce1aa618 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -655,6 +655,12 @@ if git.found()
>    endforeach
>  endif
>  
> +headers_generated = [
> +  'command-list.h',
> +  'config-list.h',
> +  'hook-list.h'
> +]
> +
>  if not get_option('breaking_changes')
>    builtin_sources += 'builtin/pack-redundant.c'
>  endif
> @@ -1995,6 +2001,107 @@ endif
>  
>  subdir('contrib')
>  
> +headers_check_exclude = headers_generated
> +headers_check_exclude += [
> +  'compat/apple-common-crypto.h',
> +  'compat/bswap.h',
> +  'compat/compiler.h',
> +  'compat/disk.h',
> +  'compat/fsmonitor/fsm-darwin-gcc.h',
> +  'compat/fsmonitor/fsm-health.h',
> +  'compat/fsmonitor/fsm-listen.h',
> +  'compat/mingw.h',
> +  'compat/msvc.h',
> +  'compat/nedmalloc/malloc.c.h',
> +  'compat/nedmalloc/nedmalloc.h',
> +  'compat/nonblock.h',
> +  'compat/obstack.h',
> +  'compat/poll/poll.h',
> +  'compat/precompose_utf8.h',
> +  'compat/regex/regex.h',
> +  'compat/regex/regex_internal.h',
> +  'compat/sha1-chunked.h',
> +  'compat/terminal.h',
> +  'compat/vcbuild/include/sys/param.h',
> +  'compat/vcbuild/include/sys/time.h',
> +  'compat/vcbuild/include/sys/utime.h',
> +  'compat/vcbuild/include/unistd.h',
> +  'compat/vcbuild/include/utime.h',
> +  'compat/win32.h',
> +  'compat/win32/alloca.h',
> +  'compat/win32/dirent.h',
> +  'compat/win32/lazyload.h',
> +  'compat/win32/path-utils.h',
> +  'compat/win32/pthread.h',
> +  'compat/win32/syslog.h',
> +  'compat/zlib-compat.h',
> +  't/unit-tests/clar/clar.h',
> +  't/unit-tests/clar/clar/fixtures.h',
> +  't/unit-tests/clar/clar/fs.h',
> +  't/unit-tests/clar/clar/print.h',
> +  't/unit-tests/clar/clar/sandbox.h',
> +  't/unit-tests/clar/clar/summary.h',
> +  't/unit-tests/clar/test/clar_test.h',
> +  'unicode-width.h',
> +  'xdiff/xdiff.h',
> +  'xdiff/xdiffi.h',
> +  'xdiff/xemit.h',
> +  'xdiff/xinclude.h',
> +  'xdiff/xmacros.h',
> +  'xdiff/xprepare.h',
> +  'xdiff/xtypes.h',
> +  'xdiff/xutils.h',
> +]

Many of these feel as if they should've been part of
`third_party_sources`.

> +if sha1_backend != 'openssl'
> +  headers_check_exclude += 'sha1/openssl.h'
> +endif
> +if sha256_backend != 'openssl'
> +  headers_check_exclude += 'sha256/openssl.h'
> +endif
> +if sha256_backend != 'nettle'
> +  headers_check_exclude += 'sha256/nettle.h'
> +endif
> +if sha256_backend != 'gcrpyt'
> +  headers_check_exclude += 'sha256/gcrypt.h'
> +endif
> +
> +if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
> +  hco_targets = []
> +  foreach h : headers
> +    if headers_check_exclude.contains(h)
> +      continue
> +    endif
> +
> +    hcc = custom_target(
> +      input: h,
> +      output: h.underscorify() + 'cc',
> +      command: [
> +        shell,
> +        '-c',
> +        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo -n \'#include "' + h + '"\' >> @OUTPUT@'
> +      ]
> +    )
> +
> +    hco = custom_target(
> +      input: hcc,
> +      output: h.underscorify().replace('.h', '.hco'),

You can use `fs.replace_suffix()` instead of `.replace()`.

> +      command: [
> +        compiler.cmd_array(),
> +        libgit_c_args,
> +        '-I', meson.project_source_root(),
> +        '-I', meson.project_source_root() / 't/unit-tests',
> +        '-o', '/dev/null',
> +        '-c', '-xc',
> +        '@INPUT@'
> +      ]
> +    )
> +    hco_targets += hco
> +  endforeach
> +
> +  alias_target('hdr-check', hco_targets)
> +endif
> +
>  foreach key, value : {
>    'DIFF': diff.full_path(),
>    'GIT_SOURCE_DIR': meson.project_source_root(),

Patrick

Re: [PATCH v2 3/4] meson: add support for 'hdr-check'

[Karthik Nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Apr 10, 2025 at 01:30:33PM +0200, Karthik Nayak wrote:
>> The Makefile supports a target called 'hdr-check', which checks if
>> individual header files can be independently compiled. Let's port this
>> functionality to meson, our new build system too. The implementation
>
> Nit: Meson is typically spelt with an upper-case 'M'.
>

Will modify throughout the series, thanks!

>> resembles that of the Makefile and provides the same check.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 107 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 790d178007..6fce1aa618 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -655,6 +655,12 @@ if git.found()
>>    endforeach
>>  endif
>>
>> +headers_generated = [
>> +  'command-list.h',
>> +  'config-list.h',
>> +  'hook-list.h'
>> +]
>> +
>>  if not get_option('breaking_changes')
>>    builtin_sources += 'builtin/pack-redundant.c'
>>  endif
>> @@ -1995,6 +2001,107 @@ endif
>>
>>  subdir('contrib')
>>
>> +headers_check_exclude = headers_generated
>> +headers_check_exclude += [
>> +  'compat/apple-common-crypto.h',
>> +  'compat/bswap.h',
>> +  'compat/compiler.h',
>> +  'compat/disk.h',
>> +  'compat/fsmonitor/fsm-darwin-gcc.h',
>> +  'compat/fsmonitor/fsm-health.h',
>> +  'compat/fsmonitor/fsm-listen.h',
>> +  'compat/mingw.h',
>> +  'compat/msvc.h',
>> +  'compat/nedmalloc/malloc.c.h',
>> +  'compat/nedmalloc/nedmalloc.h',
>> +  'compat/nonblock.h',
>> +  'compat/obstack.h',
>> +  'compat/poll/poll.h',
>> +  'compat/precompose_utf8.h',
>> +  'compat/regex/regex.h',
>> +  'compat/regex/regex_internal.h',
>> +  'compat/sha1-chunked.h',
>> +  'compat/terminal.h',
>> +  'compat/vcbuild/include/sys/param.h',
>> +  'compat/vcbuild/include/sys/time.h',
>> +  'compat/vcbuild/include/sys/utime.h',
>> +  'compat/vcbuild/include/unistd.h',
>> +  'compat/vcbuild/include/utime.h',
>> +  'compat/win32.h',
>> +  'compat/win32/alloca.h',
>> +  'compat/win32/dirent.h',
>> +  'compat/win32/lazyload.h',
>> +  'compat/win32/path-utils.h',
>> +  'compat/win32/pthread.h',
>> +  'compat/win32/syslog.h',
>> +  'compat/zlib-compat.h',
>> +  't/unit-tests/clar/clar.h',
>> +  't/unit-tests/clar/clar/fixtures.h',
>> +  't/unit-tests/clar/clar/fs.h',
>> +  't/unit-tests/clar/clar/print.h',
>> +  't/unit-tests/clar/clar/sandbox.h',
>> +  't/unit-tests/clar/clar/summary.h',
>> +  't/unit-tests/clar/test/clar_test.h',
>> +  'unicode-width.h',
>> +  'xdiff/xdiff.h',
>> +  'xdiff/xdiffi.h',
>> +  'xdiff/xemit.h',
>> +  'xdiff/xinclude.h',
>> +  'xdiff/xmacros.h',
>> +  'xdiff/xprepare.h',
>> +  'xdiff/xtypes.h',
>> +  'xdiff/xutils.h',
>> +]
>
> Many of these feel as if they should've been part of
> `third_party_sources`.
>

The condensed list is now:

exclude_from_check_headers += [
  'compat/',
  't/unit-tests/clar/',
  'unicode-width.h',
  'xdiff/',
]

I think from this:
- compat/: This captures all compat headers, I'm not sure adding all of
  compat under 'third_party_sources' makes sense though.
- t/unit-tests/clar/: can be removed, since this is already captured in
  'third_party_sources'.
- unicode-width.h: isn't a fit for 'third_party_sources'.
- xdiff/: will move to 'third_party_sources'.


>> +if sha1_backend != 'openssl'
>> +  headers_check_exclude += 'sha1/openssl.h'
>> +endif
>> +if sha256_backend != 'openssl'
>> +  headers_check_exclude += 'sha256/openssl.h'
>> +endif
>> +if sha256_backend != 'nettle'
>> +  headers_check_exclude += 'sha256/nettle.h'
>> +endif
>> +if sha256_backend != 'gcrpyt'
>> +  headers_check_exclude += 'sha256/gcrypt.h'
>> +endif
>> +
>> +if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>> +  hco_targets = []
>> +  foreach h : headers
>> +    if headers_check_exclude.contains(h)
>> +      continue
>> +    endif
>> +
>> +    hcc = custom_target(
>> +      input: h,
>> +      output: h.underscorify() + 'cc',
>> +      command: [
>> +        shell,
>> +        '-c',
>> +        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo -n \'#include "' + h + '"\' >> @OUTPUT@'
>> +      ]
>> +    )
>> +
>> +    hco = custom_target(
>> +      input: hcc,
>> +      output: h.underscorify().replace('.h', '.hco'),
>
> You can use `fs.replace_suffix()` instead of `.replace()`.
>

Nice, will do.

>> +      command: [
>> +        compiler.cmd_array(),
>> +        libgit_c_args,
>> +        '-I', meson.project_source_root(),
>> +        '-I', meson.project_source_root() / 't/unit-tests',
>> +        '-o', '/dev/null',
>> +        '-c', '-xc',
>> +        '@INPUT@'
>> +      ]
>> +    )
>> +    hco_targets += hco
>> +  endforeach
>> +
>> +  alias_target('hdr-check', hco_targets)
>> +endif
>> +
>>  foreach key, value : {
>>    'DIFF': diff.full_path(),
>>    'GIT_SOURCE_DIR': meson.project_source_root(),
>
> Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2 3/4] meson: add support for 'hdr-check'

[Toon Claes]

Karthik Nayak <karthik.188@gmail.com> writes:

> The Makefile supports a target called 'hdr-check', which checks if
> individual header files can be independently compiled. Let's port this
> functionality to meson, our new build system too. The implementation
> resembles that of the Makefile and provides the same check.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 790d178007..6fce1aa618 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -655,6 +655,12 @@ if git.found()
>    endforeach
>  endif
>  
> +headers_generated = [
> +  'command-list.h',
> +  'config-list.h',
> +  'hook-list.h'
> +]

Can we maybe compose this list by instead doing:

    generated_headers = []
    foreach f : builtin_sources + libgit_sources + third_party_sources
      if f.endswith(".h")
        generated_headers += f
      endif
    endforeach

(This would take `third_party_sources` into account as suggested by
Patrick[1]).

If we consider that too much magic, I would suggest:

    generated_headers = []
    builtin_sources += custom_target(
      output: 'config-list.h',
      command: [
        shell,
        meson.current_source_dir() + '/generate-configlist.sh',
        meson.current_source_dir(),
        '@OUTPUT@',
      ],
      env: script_environment,
    )
    generated_headers += 'config-list.h'

I hope this would reduce the chance to forget to add more headers to
this list (assuming people copy the code blurb from another location).

--
Toon

Re: [PATCH v2 3/4] meson: add support for 'hdr-check'

[Karthik Nayak]

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

Toon Claes <toon@iotcl.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> The Makefile supports a target called 'hdr-check', which checks if
>> individual header files can be independently compiled. Let's port this
>> functionality to meson, our new build system too. The implementation
>> resembles that of the Makefile and provides the same check.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 107 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 790d178007..6fce1aa618 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -655,6 +655,12 @@ if git.found()
>>    endforeach
>>  endif
>>
>> +headers_generated = [
>> +  'command-list.h',
>> +  'config-list.h',
>> +  'hook-list.h'
>> +]
>
> Can we maybe compose this list by instead doing:
>
>     generated_headers = []
>     foreach f : builtin_sources + libgit_sources + third_party_sources
>       if f.endswith(".h")
>         generated_headers += f
>       endif
>     endforeach
>
> (This would take `third_party_sources` into account as suggested by
> Patrick[1]).
>

This does make sense, but this also feels like an overkill to only get
three headers.

> If we consider that too much magic, I would suggest:
>
>     generated_headers = []
>     builtin_sources += custom_target(
>       output: 'config-list.h',
>       command: [
>         shell,
>         meson.current_source_dir() + '/generate-configlist.sh',
>         meson.current_source_dir(),
>         '@OUTPUT@',
>       ],
>       env: script_environment,
>     )
>     generated_headers += 'config-list.h'
>
> I hope this would reduce the chance to forget to add more headers to
> this list (assuming people copy the code blurb from another location).
>

This looks nice, Let me modify accordingly!

> --
> Toon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v2 4/4] makefile/meson: add 'headers-check' as alias for 'hdr-check'

[Karthik Nayak]

The 'hdr-check' target in meson and makefile is used to check if headers
can be compiled individually. The naming however isn't readable as 'hdr'
is not a common shortforme for 'header', neither is it an abbreviation.

Let's introduce 'headers-check' as an alternative target for 'hdr-check'
and add a `TODO` to deprecate the latter after 2 releases. Since this
is an internal tool, we can use a shorter deprecation cycle.

Change existing usage of 'hdr-check' in 'ci/run-static-analysis.sh' to
also use 'headers-check'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Makefile                  | 4 +++-
 ci/run-static-analysis.sh | 2 +-
 meson.build               | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index ac32d2d0bd..0ac91e0af1 100644
--- a/Makefile
+++ b/Makefile
@@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc)
 $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE
 	$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<
 
-.PHONY: hdr-check $(HCO)
+# TODO: deprecate 'hdr-check' in lieu of 'headers-check' in Git 2.51+
+.PHONY: hdr-check headers-check $(HCO)
 hdr-check: $(HCO)
+headers-check: hdr-check
 
 .PHONY: style
 style:
diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
index 0d51e5ce0e..2e51411d6e 100755
--- a/ci/run-static-analysis.sh
+++ b/ci/run-static-analysis.sh
@@ -26,7 +26,7 @@ then
 	exit 1
 fi
 
-make hdr-check ||
+make headers-check ||
 exit 1
 
 make check-pot
diff --git a/meson.build b/meson.build
index 6fce1aa618..74597283b9 100644
--- a/meson.build
+++ b/meson.build
@@ -2099,7 +2099,9 @@ if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
     hco_targets += hco
   endforeach
 
+  # TODO: deprecate 'hdr-check' in lieu of 'headers-check' in Git 2.51+
   alias_target('hdr-check', hco_targets)
+  alias_target('headers-check', hco_targets)
 endif
 
 foreach key, value : {

-- 
2.48.1

Re: [PATCH v2 4/4] makefile/meson: add 'headers-check' as alias for 'hdr-check'

[Phillip Wood]

Hi Karthik

On 10/04/2025 12:30, Karthik Nayak wrote:
> The 'hdr-check' target in meson and makefile is used to check if headers
> can be compiled individually. The naming however isn't readable as 'hdr'
> is not a common shortforme for 'header', neither is it an abbreviation.
> 
> Let's introduce 'headers-check' as an alternative target for 'hdr-check'
> and add a `TODO` to deprecate the latter after 2 releases. Since this
> is an internal tool, we can use a shorter deprecation cycle.

Can we call this "check-headers" to match the other "check-" targets in 
the Makefile please

Thanks

Phillip

> Change existing usage of 'hdr-check' in 'ci/run-static-analysis.sh' to
> also use 'headers-check'.
> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>   Makefile                  | 4 +++-
>   ci/run-static-analysis.sh | 2 +-
>   meson.build               | 2 ++
>   3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index ac32d2d0bd..0ac91e0af1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc)
>   $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE
>   	$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<
>   
> -.PHONY: hdr-check $(HCO)
> +# TODO: deprecate 'hdr-check' in lieu of 'headers-check' in Git 2.51+
> +.PHONY: hdr-check headers-check $(HCO)
>   hdr-check: $(HCO)
> +headers-check: hdr-check
>   
>   .PHONY: style
>   style:
> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
> index 0d51e5ce0e..2e51411d6e 100755
> --- a/ci/run-static-analysis.sh
> +++ b/ci/run-static-analysis.sh
> @@ -26,7 +26,7 @@ then
>   	exit 1
>   fi
>   
> -make hdr-check ||
> +make headers-check ||
>   exit 1
>   
>   make check-pot
> diff --git a/meson.build b/meson.build
> index 6fce1aa618..74597283b9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2099,7 +2099,9 @@ if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>       hco_targets += hco
>     endforeach
>   
> +  # TODO: deprecate 'hdr-check' in lieu of 'headers-check' in Git 2.51+
>     alias_target('hdr-check', hco_targets)
> +  alias_target('headers-check', hco_targets)
>   endif
>   
>   foreach key, value : {
> 

Re: [PATCH v2 4/4] makefile/meson: add 'headers-check' as alias for 'hdr-check'

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 10/04/2025 12:30, Karthik Nayak wrote:
>> The 'hdr-check' target in meson and makefile is used to check if headers
>> can be compiled individually. The naming however isn't readable as 'hdr'
>> is not a common shortforme for 'header', neither is it an abbreviation.
>> Let's introduce 'headers-check' as an alternative target for
>> 'hdr-check'
>> and add a `TODO` to deprecate the latter after 2 releases. Since this
>> is an internal tool, we can use a shorter deprecation cycle.
>
> Can we call this "check-headers" to match the other "check-" targets
> in the Makefile please

Excellent suggestion.  If we were to change things, we should get it
right just once.

Thanks.

Re: [PATCH v2 4/4] makefile/meson: add 'headers-check' as alias for 'hdr-check'

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> On 10/04/2025 12:30, Karthik Nayak wrote:
>>> The 'hdr-check' target in meson and makefile is used to check if headers
>>> can be compiled individually. The naming however isn't readable as 'hdr'
>>> is not a common shortforme for 'header', neither is it an abbreviation.
>>> Let's introduce 'headers-check' as an alternative target for
>>> 'hdr-check'
>>> and add a `TODO` to deprecate the latter after 2 releases. Since this
>>> is an internal tool, we can use a shorter deprecation cycle.
>>
>> Can we call this "check-headers" to match the other "check-" targets
>> in the Makefile please
>
> Excellent suggestion.  If we were to change things, we should get it
> right just once.
>
> Thanks.

Yeah I agree too, I based it off 'coccicheck', but seems like 'check-*'
is more consistent.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3 0/4] meson: add corresponding target for Makefile's hdr-check

[Karthik Nayak]

To bridge the remaining gaps between Makefile and Meson, this patch
series adds 'hdr-check' to Meson to compliment the Makefile's
'hdr-check'.

We also introduce 'headers-check' as an alias to 'hdr-check' as a better
named replacement in both Meson and make and add a note to deprecate
'hdr-check' in the future.

The first two commits are small cleanups, where we re-organize existing
variables to make it easier to add the target. The third commit adds the
'hdr-check' target to Meson. The last commit introduces the
'headers-check' alias to both Meson and the makefile and marks
'hdr-check' to be deprecated.

This is based on master 9d22ac5122 (The third batch, 2025-04-07) with
'es/meson-build-skip-coccinelle' merged in.

---
Changes in v3:
- Some renames:
  - headers_generated -> generated_headers
  - meson -> Meson
  - headers-check -> check-headers
  - headers_check_exclude -> exclude_from_check_headers
- Rewrite 'headers_check_exclude' to also contain dirs so we can skip
  listing individual header files.
- Move 'xdiff/*' to 'third_party_sources' and cleanup
  'exclude_from_check_headers'.
- Use 'echo' instead of 'echo -n'.
- Use `fs.replace_suffix` instead of `str.replace`.
- Link to v2: https://lore.kernel.org/r/20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com

Changes in v2:
- Add 'hdr-check' to meson, while introducing 'headers-check' as
  a replacement alias. Schedule 'hdr-check' to be deprecated in the future.
- Link to v1: https://lore.kernel.org/r/20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com

---
 Makefile                       |  4 +-
 ci/run-static-analysis.sh      |  2 +-
 contrib/coccinelle/meson.build | 29 ++++---------
 meson.build                    | 94 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 105 insertions(+), 24 deletions(-)

Karthik Nayak (4):
      coccinelle: meson: rename variables to be more specific
      meson: move headers definition from 'contrib/coccinelle'
      meson: add support for 'hdr-check'
      makefile/meson: add 'check-headers' as alias for 'hdr-check'

Range-diff versus v2:

1:  ca94103a13 ! 1:  3386caeaf5 coccinelle: meson: rename variables to be more specific
    @@ Metadata
      ## Commit message ##
         coccinelle: meson: rename variables to be more specific
     
    -    In meson, included subdirs export their variables to top level meson
    +    In Meson, included subdirs export their variables to top level Meson
         builds. In 'contrib/coccinelle/meson.build', we define two such
         variables `sources` and `headers`. While these variables are specific to
         the checks in the 'contrib/coccinelle/' directory, they also pollute the
2:  2549f1a5df ! 2:  35f5a580e4 meson: move headers definition from 'contrib/coccinelle'
    @@ Metadata
      ## Commit message ##
         meson: move headers definition from 'contrib/coccinelle'
     
    -    The meson build for coccinelle static analysis lists all headers to
    -    analyse. Due to the way meson exports variables between subdirs, this
    -    variable is also available in the root meson build.
    +    The Meson build for coccinelle static analysis lists all headers to
    +    analyse. Due to the way Meson exports variables between subdirs, this
    +    variable is also available in the root Meson build.
     
         An upcoming commit, will add a new check complimenting 'hdr-check' in
         the Makefile. This would require the list of headers. So move the
    -    'coccinelle_headers' to the root meson build and rename it to 'headers',
    +    'coccinelle_headers' to the root Meson build and rename it to 'headers',
         remove the root path being appended to each header and retain that in
    -    the coccinelle meson build since it is specific to the coccinelle build.
    +    the coccinelle Meson build since it is specific to the coccinelle build.
     
    -    Also move the 'third_party_sources' variable to the root meson build
    +    Also move the 'third_party_sources' variable to the root Meson build
         since it is also a dependency for the 'headers' variable. This also
         makes it easier to understand as the variable is now propagated from the
         top level to the bottom.
    @@ meson.build: builtin_sources = [
     +  ':!t/t[0-9][0-9][0-9][0-9]*',
     +]
     +
    -+headers = []
     +if git.found()
    ++  headers = []
     +  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
     +    headers += header
     +  endforeach
3:  a46db4a81c < -:  ---------- meson: add support for 'hdr-check'
-:  ---------- > 3:  0df83087ac meson: add support for 'hdr-check'
4:  fe0160b6fe ! 4:  b5d10772ff makefile/meson: add 'headers-check' as alias for 'hdr-check'
    @@ Metadata
     Author: Karthik Nayak <karthik.188@gmail.com>
     
      ## Commit message ##
    -    makefile/meson: add 'headers-check' as alias for 'hdr-check'
    +    makefile/meson: add 'check-headers' as alias for 'hdr-check'
     
    -    The 'hdr-check' target in meson and makefile is used to check if headers
    +    The 'hdr-check' target in Meson and makefile is used to check if headers
         can be compiled individually. The naming however isn't readable as 'hdr'
         is not a common shortforme for 'header', neither is it an abbreviation.
     
    -    Let's introduce 'headers-check' as an alternative target for 'hdr-check'
    +    Let's introduce 'check-headers' as an alternative target for 'hdr-check'
         and add a `TODO` to deprecate the latter after 2 releases. Since this
         is an internal tool, we can use a shorter deprecation cycle.
     
         Change existing usage of 'hdr-check' in 'ci/run-static-analysis.sh' to
    -    also use 'headers-check'.
    +    also use 'check-headers'.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
    @@ Makefile: HCC = $(HCO:hco=hcc)
      	$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<
      
     -.PHONY: hdr-check $(HCO)
    -+# TODO: deprecate 'hdr-check' in lieu of 'headers-check' in Git 2.51+
    -+.PHONY: hdr-check headers-check $(HCO)
    ++# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
    ++.PHONY: hdr-check check-headers $(HCO)
      hdr-check: $(HCO)
    -+headers-check: hdr-check
    ++check-headers: hdr-check
      
      .PHONY: style
      style:
    @@ ci/run-static-analysis.sh: then
      fi
      
     -make hdr-check ||
    -+make headers-check ||
    ++make check-headers ||
      exit 1
      
      make check-pot
     
      ## meson.build ##
    -@@ meson.build: if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
    +@@ meson.build: if git.found() and compiler.get_argument_syntax() == 'gcc'
          hco_targets += hco
        endforeach
      
    -+  # TODO: deprecate 'hdr-check' in lieu of 'headers-check' in Git 2.51+
    ++  # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
        alias_target('hdr-check', hco_targets)
    -+  alias_target('headers-check', hco_targets)
    ++  alias_target('check-headers', hco_targets)
      endif
      
      foreach key, value : {


base-commit: 3a956c5f69873611ae5f8dcb9acd117f66b95ddc
change-id: 20250330-505-wire-up-sparse-via-meson-2e32dd31208b

Thanks
- Karthik

[PATCH v3 1/4] coccinelle: meson: rename variables to be more specific

[Karthik Nayak]

In Meson, included subdirs export their variables to top level Meson
builds. In 'contrib/coccinelle/meson.build', we define two such
variables `sources` and `headers`. While these variables are specific to
the checks in the 'contrib/coccinelle/' directory, they also pollute the
top level 'meson.build'.

Rename them to be more specific, this ensures that they aren't
mistakenly used in the upper levels and avoid variable name collisions.

While here, change the empty list denotation to be consistent with other
places.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index ea054c924f..03ce52d752 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -55,18 +55,18 @@ concatenated_rules = custom_target(
   capture: true,
 )
 
-sources = [ ]
+coccinelle_sources = []
 foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.c', third_party_sources, check: true).stdout().split()
-  sources += source
+  coccinelle_sources += source
 endforeach
 
-headers = [ ]
+coccinelle_headers = []
 foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
-  headers += meson.project_source_root() / header
+  coccinelle_headers += meson.project_source_root() / header
 endforeach
 
 patches = [ ]
-foreach source : sources
+foreach source : coccinelle_sources
   patches += custom_target(
     command: [
       spatch,
@@ -78,7 +78,7 @@ foreach source : sources
     input: meson.project_source_root() / source,
     output: source.underscorify() + '.patch',
     capture: true,
-    depend_files: headers,
+    depend_files: coccinelle_headers,
   )
 endforeach
 

-- 
2.48.1

[PATCH v3 2/4] meson: move headers definition from 'contrib/coccinelle'

[Karthik Nayak]

The Meson build for coccinelle static analysis lists all headers to
analyse. Due to the way Meson exports variables between subdirs, this
variable is also available in the root Meson build.

An upcoming commit, will add a new check complimenting 'hdr-check' in
the Makefile. This would require the list of headers. So move the
'coccinelle_headers' to the root Meson build and rename it to 'headers',
remove the root path being appended to each header and retain that in
the coccinelle Meson build since it is specific to the coccinelle build.

Also move the 'third_party_sources' variable to the root Meson build
since it is also a dependency for the 'headers' variable. This also
makes it easier to understand as the variable is now propagated from the
top level to the bottom.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 17 +----------------
 meson.build                    | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index 03ce52d752..32568c5103 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -8,21 +8,6 @@ if not spatch.found()
   subdir_done()
 endif
 
-third_party_sources = [
-  ':!contrib',
-  ':!compat/inet_ntop.c',
-  ':!compat/inet_pton.c',
-  ':!compat/nedmalloc',
-  ':!compat/obstack.*',
-  ':!compat/poll',
-  ':!compat/regex',
-  ':!sha1collisiondetection',
-  ':!sha1dc',
-  ':!t/unit-tests/clar',
-  ':!t/unit-tests/clar',
-  ':!t/t[0-9][0-9][0-9][0-9]*',
-]
-
 rules = [
   'array.cocci',
   'commit.cocci',
@@ -61,7 +46,7 @@ foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files',
 endforeach
 
 coccinelle_headers = []
-foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+foreach header : headers
   coccinelle_headers += meson.project_source_root() / header
 endforeach
 
diff --git a/meson.build b/meson.build
index e98cfa4909..3ca5d01071 100644
--- a/meson.build
+++ b/meson.build
@@ -633,6 +633,28 @@ builtin_sources = [
   'builtin/write-tree.c',
 ]
 
+third_party_sources = [
+  ':!contrib',
+  ':!compat/inet_ntop.c',
+  ':!compat/inet_pton.c',
+  ':!compat/nedmalloc',
+  ':!compat/obstack.*',
+  ':!compat/poll',
+  ':!compat/regex',
+  ':!sha1collisiondetection',
+  ':!sha1dc',
+  ':!t/unit-tests/clar',
+  ':!t/unit-tests/clar',
+  ':!t/t[0-9][0-9][0-9][0-9]*',
+]
+
+if git.found()
+  headers = []
+  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+    headers += header
+  endforeach
+endif
+
 if not get_option('breaking_changes')
   builtin_sources += 'builtin/pack-redundant.c'
 endif

-- 
2.48.1

Re: [PATCH v3 2/4] meson: move headers definition from 'contrib/coccinelle'

[Phillip Wood]

Hi Karthik

On 14/04/2025 22:16, Karthik Nayak wrote:
> diff --git a/meson.build b/meson.build
> index e98cfa4909..3ca5d01071 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -633,6 +633,28 @@ builtin_sources = [
>     'builtin/write-tree.c',
>   ]
>   
> +third_party_sources = [

This is not the fault of this patch but I found this name rather 
confusing as it is not a list of sources but a list of exclude patterns. 
Calling it "third_party_excludes" would be clearer to me at least.

> +  ':!contrib',
> +  ':!compat/inet_ntop.c',
> +  ':!compat/inet_pton.c',
> +  ':!compat/nedmalloc',
> +  ':!compat/obstack.*',
> +  ':!compat/poll',
> +  ':!compat/regex',
> +  ':!sha1collisiondetection',
> +  ':!sha1dc',
> +  ':!t/unit-tests/clar',
> +  ':!t/unit-tests/clar',

Again not a new problem but this line is a duplicate

> +  ':!t/t[0-9][0-9][0-9][0-9]*',
> +]
> +
> +if git.found()
> +  headers = []

This is called "headers" but it is only really the subset of our headers 
that we want to run static analysis on. Maybe we could call it 
"headers_to_check" or something that makes it clearer what the list is for.

Best Wishes

Phillip

> +  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
> +    headers += header
> +  endforeach
> +endif
> +
>   if not get_option('breaking_changes')
>     builtin_sources += 'builtin/pack-redundant.c'
>   endif
> 

Re: [PATCH v3 2/4] meson: move headers definition from 'contrib/coccinelle'

[Karthik Nayak]

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Karthik
>
> On 14/04/2025 22:16, Karthik Nayak wrote:
>> diff --git a/meson.build b/meson.build
>> index e98cfa4909..3ca5d01071 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -633,6 +633,28 @@ builtin_sources = [
>>     'builtin/write-tree.c',
>>   ]
>>
>> +third_party_sources = [
>
> This is not the fault of this patch but I found this name rather
> confusing as it is not a list of sources but a list of exclude patterns.
> Calling it "third_party_excludes" would be clearer to me at least.
>

This seems to map what the Makefile has, but I would agree, that this
should be renamed.

>> +  ':!contrib',
>> +  ':!compat/inet_ntop.c',
>> +  ':!compat/inet_pton.c',
>> +  ':!compat/nedmalloc',
>> +  ':!compat/obstack.*',
>> +  ':!compat/poll',
>> +  ':!compat/regex',
>> +  ':!sha1collisiondetection',
>> +  ':!sha1dc',
>> +  ':!t/unit-tests/clar',
>> +  ':!t/unit-tests/clar',
>
> Again not a new problem but this line is a duplicate
>

Fair, considering we're touching these fields, I will add an additional
commit to simply cleanup these two issues.

>> +  ':!t/t[0-9][0-9][0-9][0-9]*',
>> +]
>> +
>> +if git.found()
>> +  headers = []
>
> This is called "headers" but it is only really the subset of our headers
> that we want to run static analysis on. Maybe we could call it
> "headers_to_check" or something that makes it clearer what the list is for.
>

Agreed, will rename. Thanks!

> Best Wishes
>
> Phillip
>
>> +  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
>> +    headers += header
>> +  endforeach
>> +endif
>> +
>>   if not get_option('breaking_changes')
>>     builtin_sources += 'builtin/pack-redundant.c'
>>   endif
>>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3 3/4] meson: add support for 'hdr-check'

[Karthik Nayak]

The Makefile supports a target called 'hdr-check', which checks if
individual header files can be independently compiled. Let's port this
functionality to Meson, our new build system too. The implementation
resembles that of the Makefile and provides the same check.

Since meson builds are out-of-tree, header dependencies are not
automatically met. So unlike the Makefile version, we also need to add
the required dependencies.

Also add the 'xdiff/' dir to the list of 'third_party_sources' as those
headers must be skipped from the checks too!

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 meson.build | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/meson.build b/meson.build
index 3ca5d01071..b1be2b3cbb 100644
--- a/meson.build
+++ b/meson.build
@@ -646,6 +646,7 @@ third_party_sources = [
   ':!t/unit-tests/clar',
   ':!t/unit-tests/clar',
   ':!t/t[0-9][0-9][0-9][0-9]*',
+  ':!xdiff',
 ]
 
 if git.found()
@@ -655,6 +656,10 @@ if git.found()
   endforeach
 endif
 
+generated_headers = [
+  'command-list.h',
+]
+
 if not get_option('breaking_changes')
   builtin_sources += 'builtin/pack-redundant.c'
 endif
@@ -669,6 +674,7 @@ builtin_sources += custom_target(
   ],
   env: script_environment,
 )
+generated_headers += 'config-list.h'
 
 builtin_sources += custom_target(
   input: 'Documentation/githooks.adoc',
@@ -681,6 +687,7 @@ builtin_sources += custom_target(
   ],
   env: script_environment,
 )
+generated_headers += 'hook-list.h'
 
 # This contains the variables for GIT-BUILD-OPTIONS, which we use to propagate
 # build options to our tests.
@@ -1995,6 +2002,69 @@ endif
 
 subdir('contrib')
 
+exclude_from_check_headers = generated_headers
+exclude_from_check_headers += [
+  'compat/',
+  'unicode-width.h',
+]
+
+if sha1_backend != 'openssl'
+  exclude_from_check_headers += 'sha1/openssl.h'
+endif
+if sha256_backend != 'openssl'
+  exclude_from_check_headers += 'sha256/openssl.h'
+endif
+if sha256_backend != 'nettle'
+  exclude_from_check_headers += 'sha256/nettle.h'
+endif
+if sha256_backend != 'gcrpyt'
+  exclude_from_check_headers += 'sha256/gcrypt.h'
+endif
+
+if git.found() and compiler.get_argument_syntax() == 'gcc'
+  hco_targets = []
+  foreach h : headers
+    skip_header = false
+    foreach exclude : exclude_from_check_headers
+      if h.startswith(exclude)
+        skip_header = true
+        break
+      endif
+    endforeach
+
+    if skip_header
+      continue
+    endif
+
+    hcc = custom_target(
+      input: h,
+      output: h.underscorify() + 'cc',
+      command: [
+        shell,
+        '-c',
+        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@'
+      ]
+    )
+
+    hco = custom_target(
+      input: hcc,
+      output: fs.replace_suffix(h.underscorify(), '.hco'),
+      command: [
+        compiler.cmd_array(),
+        libgit_c_args,
+        '-I', meson.project_source_root(),
+        '-I', meson.project_source_root() / 't/unit-tests',
+        '-o', '/dev/null',
+        '-c', '-xc',
+        '@INPUT@'
+      ]
+    )
+    hco_targets += hco
+  endforeach
+
+  alias_target('hdr-check', hco_targets)
+endif
+
 foreach key, value : {
   'DIFF': diff.full_path(),
   'GIT_SOURCE_DIR': meson.project_source_root(),

-- 
2.48.1

Re: [PATCH v3 3/4] meson: add support for 'hdr-check'

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> +if sha256_backend != 'gcrpyt'

That's a bit unexpected name; relative to this one ...

> +  exclude_from_check_headers += 'sha256/gcrypt.h'
> +endif

... I have to suspect that it is a typo?

Re: [PATCH v3 3/4] meson: add support for 'hdr-check'

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +if sha256_backend != 'gcrpyt'
>
> That's a bit unexpected name; relative to this one ...
>
>> +  exclude_from_check_headers += 'sha256/gcrypt.h'
>> +endif
>
> ... I have to suspect that it is a typo?

Indeed, I had to double check cause I couldn't spot it when you
mentioned. Will fix. Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 3/4] meson: add support for 'hdr-check'

[phillip.wood123]

Hi Karthik

On 14/04/2025 22:16, Karthik Nayak wrote:
> The Makefile supports a target called 'hdr-check', which checks if
> individual header files can be independently compiled. Let's port this
> functionality to Meson, our new build system too. The implementation
> resembles that of the Makefile and provides the same check.
> 
> Since meson builds are out-of-tree, header dependencies are not
> automatically met. So unlike the Makefile version, we also need to add
> the required dependencies.
> 
> Also add the 'xdiff/' dir to the list of 'third_party_sources' as those
> headers must be skipped from the checks too!

Doesn't this mean we'll skip all the xdiff files when running coccinelle 
as well? If so the commit message should point that out and explain why 
that is an improvement.

> +exclude_from_check_headers = generated_headers

I'm not sure this is necessary. The list of headers that we filter is 
generated with "git ls-files" and so wont contain generated headers in 
the first place.

> +exclude_from_check_headers += [
> +  'compat/',
> +  'unicode-width.h',
> +]
> +
> +if sha1_backend != 'openssl'
> +  exclude_from_check_headers += 'sha1/openssl.h'
> +endif
> +if sha256_backend != 'openssl'
> +  exclude_from_check_headers += 'sha256/openssl.h'
> +endif
> +if sha256_backend != 'nettle'
> +  exclude_from_check_headers += 'sha256/nettle.h'
> +endif
> +if sha256_backend != 'gcrpyt'
> +  exclude_from_check_headers += 'sha256/gcrypt.h'
> +endif
> +
> +if git.found() and compiler.get_argument_syntax() == 'gcc'
> +  hco_targets = []
> +  foreach h : headers
> +    skip_header = false
> +    foreach exclude : exclude_from_check_headers
> +      if h.startswith(exclude)
> +        skip_header = true
> +        break
> +      endif
> +    endforeach

This part looks much more maintainable than than the previous version

Thanks

Phillip

> +    if skip_header
> +      continue
> +    endif
> +
> +    hcc = custom_target(
> +      input: h,
> +      output: h.underscorify() + 'cc',
> +      command: [
> +        shell,
> +        '-c',
> +        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@'
> +      ]
> +    )
> +
> +    hco = custom_target(
> +      input: hcc,
> +      output: fs.replace_suffix(h.underscorify(), '.hco'),
> +      command: [
> +        compiler.cmd_array(),
> +        libgit_c_args,
> +        '-I', meson.project_source_root(),
> +        '-I', meson.project_source_root() / 't/unit-tests',
> +        '-o', '/dev/null',
> +        '-c', '-xc',
> +        '@INPUT@'
> +      ]
> +    )
> +    hco_targets += hco
> +  endforeach
> +
> +  alias_target('hdr-check', hco_targets)
> +endif
> +
>   foreach key, value : {
>     'DIFF': diff.full_path(),
>     'GIT_SOURCE_DIR': meson.project_source_root(),

Re: [PATCH v3 3/4] meson: add support for 'hdr-check'

[Karthik Nayak]

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

phillip.wood123@gmail.com writes:

> Hi Karthik
>
> On 14/04/2025 22:16, Karthik Nayak wrote:
>> The Makefile supports a target called 'hdr-check', which checks if
>> individual header files can be independently compiled. Let's port this
>> functionality to Meson, our new build system too. The implementation
>> resembles that of the Makefile and provides the same check.
>>
>> Since meson builds are out-of-tree, header dependencies are not
>> automatically met. So unlike the Makefile version, we also need to add
>> the required dependencies.
>>
>> Also add the 'xdiff/' dir to the list of 'third_party_sources' as those
>> headers must be skipped from the checks too!
>
> Doesn't this mean we'll skip all the xdiff files when running coccinelle
> as well? If so the commit message should point that out and explain why
> that is an improvement.
>

Yes, let me add it to the commit message.

>> +exclude_from_check_headers = generated_headers
>
> I'm not sure this is necessary. The list of headers that we filter is
> generated with "git ls-files" and so wont contain generated headers in
> the first place.
>

Good point, since generated headers are part of the '.gitignore', we can
skip the 'generated_headers' variable entirely.

>> +exclude_from_check_headers += [
>> +  'compat/',
>> +  'unicode-width.h',
>> +]
>> +
>> +if sha1_backend != 'openssl'
>> +  exclude_from_check_headers += 'sha1/openssl.h'
>> +endif
>> +if sha256_backend != 'openssl'
>> +  exclude_from_check_headers += 'sha256/openssl.h'
>> +endif
>> +if sha256_backend != 'nettle'
>> +  exclude_from_check_headers += 'sha256/nettle.h'
>> +endif
>> +if sha256_backend != 'gcrpyt'
>> +  exclude_from_check_headers += 'sha256/gcrypt.h'
>> +endif
>> +
>> +if git.found() and compiler.get_argument_syntax() == 'gcc'
>> +  hco_targets = []
>> +  foreach h : headers
>> +    skip_header = false
>> +    foreach exclude : exclude_from_check_headers
>> +      if h.startswith(exclude)
>> +        skip_header = true
>> +        break
>> +      endif
>> +    endforeach
>
> This part looks much more maintainable than than the previous version
>

Indeed. Thanks!

>
> Thanks
>
> Phillip
>
>> +    if skip_header
>> +      continue
>> +    endif
>> +
>> +    hcc = custom_target(
>> +      input: h,
>> +      output: h.underscorify() + 'cc',
>> +      command: [
>> +        shell,
>> +        '-c',
>> +        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@'
>> +      ]
>> +    )
>> +
>> +    hco = custom_target(
>> +      input: hcc,
>> +      output: fs.replace_suffix(h.underscorify(), '.hco'),
>> +      command: [
>> +        compiler.cmd_array(),
>> +        libgit_c_args,
>> +        '-I', meson.project_source_root(),
>> +        '-I', meson.project_source_root() / 't/unit-tests',
>> +        '-o', '/dev/null',
>> +        '-c', '-xc',
>> +        '@INPUT@'
>> +      ]
>> +    )
>> +    hco_targets += hco
>> +  endforeach
>> +
>> +  alias_target('hdr-check', hco_targets)
>> +endif
>> +
>>   foreach key, value : {
>>     'DIFF': diff.full_path(),
>>     'GIT_SOURCE_DIR': meson.project_source_root(),

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3 4/4] makefile/meson: add 'check-headers' as alias for 'hdr-check'

[Karthik Nayak]

The 'hdr-check' target in Meson and makefile is used to check if headers
can be compiled individually. The naming however isn't readable as 'hdr'
is not a common shortforme for 'header', neither is it an abbreviation.

Let's introduce 'check-headers' as an alternative target for 'hdr-check'
and add a `TODO` to deprecate the latter after 2 releases. Since this
is an internal tool, we can use a shorter deprecation cycle.

Change existing usage of 'hdr-check' in 'ci/run-static-analysis.sh' to
also use 'check-headers'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Makefile                  | 4 +++-
 ci/run-static-analysis.sh | 2 +-
 meson.build               | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index ac32d2d0bd..961ee508be 100644
--- a/Makefile
+++ b/Makefile
@@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc)
 $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE
 	$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<
 
-.PHONY: hdr-check $(HCO)
+# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
+.PHONY: hdr-check check-headers $(HCO)
 hdr-check: $(HCO)
+check-headers: hdr-check
 
 .PHONY: style
 style:
diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
index 0d51e5ce0e..60c175a094 100755
--- a/ci/run-static-analysis.sh
+++ b/ci/run-static-analysis.sh
@@ -26,7 +26,7 @@ then
 	exit 1
 fi
 
-make hdr-check ||
+make check-headers ||
 exit 1
 
 make check-pot
diff --git a/meson.build b/meson.build
index b1be2b3cbb..745cb30165 100644
--- a/meson.build
+++ b/meson.build
@@ -2062,7 +2062,9 @@ if git.found() and compiler.get_argument_syntax() == 'gcc'
     hco_targets += hco
   endforeach
 
+  # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
   alias_target('hdr-check', hco_targets)
+  alias_target('check-headers', hco_targets)
 endif
 
 foreach key, value : {

-- 
2.48.1

Re: [PATCH v3 4/4] makefile/meson: add 'check-headers' as alias for 'hdr-check'

[phillip.wood123]

Hi Karthik

On 14/04/2025 22:16, Karthik Nayak wrote:
> The 'hdr-check' target in Meson and makefile is used to check if headers
> can be compiled individually. The naming however isn't readable as 'hdr'
> is not a common shortforme for 'header', neither is it an abbreviation.
> 
> Let's introduce 'check-headers' as an alternative target for 'hdr-check'
> and add a `TODO` to deprecate the latter after 2 releases. Since this
> is an internal tool, we can use a shorter deprecation cycle.

Thanks for renaming this. I've left one small question at below but this 
basically looks good to me.

> diff --git a/Makefile b/Makefile
> index ac32d2d0bd..961ee508be 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc)
>   $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE
>   	$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<
>   
> -.PHONY: hdr-check $(HCO)
> +# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
> +.PHONY: hdr-check check-headers $(HCO)
>   hdr-check: $(HCO)
> +check-headers: hdr-check
>   
>   .PHONY: style
>   style:
> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
> index 0d51e5ce0e..60c175a094 100755
> --- a/ci/run-static-analysis.sh
> +++ b/ci/run-static-analysis.sh
> @@ -26,7 +26,7 @@ then
>   	exit 1
>   fi
>   
> -make hdr-check ||
> +make check-headers ||
>   exit 1
>   
>   make check-pot
> diff --git a/meson.build b/meson.build
> index b1be2b3cbb..745cb30165 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2062,7 +2062,9 @@ if git.found() and compiler.get_argument_syntax() == 'gcc'
>       hco_targets += hco
>     endforeach
>   
> +  # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
>     alias_target('hdr-check', hco_targets)
> +  alias_target('check-headers', hco_targets)

This looks good. One minor question which isn't worth a re-roll on its 
own - in the Makefile the check-headers target depends on the hdr-check 
target which means the two will always use the same dependencies. Here 
we make check-headers depend on hco_targets instead which means that in 
theory the two targets could get out of sync if the dependencies of 
hdr-check changed. Is it possible to have a meson target that depends on 
hdr-check directly?

Best Wishes

Phillip

>   endif
>   
>   foreach key, value : {
> 

Re: [PATCH v3 4/4] makefile/meson: add 'check-headers' as alias for 'hdr-check'

[Karthik Nayak]

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

phillip.wood123@gmail.com writes:

> Hi Karthik
>
> On 14/04/2025 22:16, Karthik Nayak wrote:
>> The 'hdr-check' target in Meson and makefile is used to check if headers
>> can be compiled individually. The naming however isn't readable as 'hdr'
>> is not a common shortforme for 'header', neither is it an abbreviation.
>>
>> Let's introduce 'check-headers' as an alternative target for 'hdr-check'
>> and add a `TODO` to deprecate the latter after 2 releases. Since this
>> is an internal tool, we can use a shorter deprecation cycle.
>
> Thanks for renaming this. I've left one small question at below but this
> basically looks good to me.
>
>> diff --git a/Makefile b/Makefile
>> index ac32d2d0bd..961ee508be 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc)
>>   $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE
>>   	$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<
>>
>> -.PHONY: hdr-check $(HCO)
>> +# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
>> +.PHONY: hdr-check check-headers $(HCO)
>>   hdr-check: $(HCO)
>> +check-headers: hdr-check
>>
>>   .PHONY: style
>>   style:
>> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
>> index 0d51e5ce0e..60c175a094 100755
>> --- a/ci/run-static-analysis.sh
>> +++ b/ci/run-static-analysis.sh
>> @@ -26,7 +26,7 @@ then
>>   	exit 1
>>   fi
>>
>> -make hdr-check ||
>> +make check-headers ||
>>   exit 1
>>
>>   make check-pot
>> diff --git a/meson.build b/meson.build
>> index b1be2b3cbb..745cb30165 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2062,7 +2062,9 @@ if git.found() and compiler.get_argument_syntax() == 'gcc'
>>       hco_targets += hco
>>     endforeach
>>
>> +  # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
>>     alias_target('hdr-check', hco_targets)
>> +  alias_target('check-headers', hco_targets)
>
> This looks good. One minor question which isn't worth a re-roll on its
> own - in the Makefile the check-headers target depends on the hdr-check
> target which means the two will always use the same dependencies. Here
> we make check-headers depend on hco_targets instead which means that in
> theory the two targets could get out of sync if the dependencies of
> hdr-check changed. Is it possible to have a meson target that depends on
> hdr-check directly?
>

That's a good question, I didn't think much about it then, but looking
into it, seems like we can do

  diff --git a/meson.build b/meson.build
  index 3bf3d65e8b..39319e2610 100644
  --- a/meson.build
  +++ b/meson.build
  @@ -2055,8 +2055,8 @@ if git.found() and
compiler.get_argument_syntax() == 'gcc'
     endforeach

     # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
  -  alias_target('hdr-check', hco_targets)
  -  alias_target('check-headers', hco_targets)
  +  hdr_check = alias_target('hdr-check', hco_targets)
  +  alias_target('check-headers', hdr_check)
   endif

   foreach key, value : {

Not sure if there is a more idiomatic way, but this works. Will add in
the next version.

> Best Wishes
>
> Phillip
>
>>   endif
>>
>>   foreach key, value : {
>>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

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

[Karthik Nayak]

To bridge the remaining gaps between Makefile and Meson, this patch
series adds 'hdr-check' to Meson to compliment the Makefile's
'hdr-check'.

We also introduce 'headers-check' as an alias to 'hdr-check' as a better
named replacement in both Meson and make and add a note to deprecate
'hdr-check' in the future.

The first two commits are small cleanups, where we re-organize existing
variables to make it easier to add the target. The third commit adds the
'hdr-check' target to Meson. The last commit introduces the
'headers-check' alias to both Meson and the makefile and marks
'hdr-check' to be deprecated.

This is based on master 9d22ac5122 (The third batch, 2025-04-07) with
'es/meson-build-skip-coccinelle' merged in.

---
Changes in v4:
- Rename headers to headers_to_check, since these headers are only used
  for static analysis.
- Added a commit to rename third_party_sources -> third_party_excludes
  and remove a duplicate.
- Fix a typo 'gcrpyt' -> 'gcrypt'
- Remove 'generated_headers', since we use 'git ls-files' and that would
  already ignore files within '.gitignore'.
- Link to v3: https://lore.kernel.org/r/20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745@gmail.com

Changes in v3:
- Some renames:
  - headers_generated -> generated_headers
  - meson -> Meson
  - headers-check -> check-headers
  - headers_check_exclude -> exclude_from_check_headers
- Rewrite 'headers_check_exclude' to also contain dirs so we can skip
  listing individual header files.
- Move 'xdiff/*' to 'third_party_sources' and cleanup
  'exclude_from_check_headers'.
- Use 'echo' instead of 'echo -n'.
- Use `fs.replace_suffix` instead of `str.replace`.
- Link to v2: https://lore.kernel.org/r/20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com

Changes in v2:
- Add 'hdr-check' to meson, while introducing 'headers-check' as
  a replacement alias. Schedule 'hdr-check' to be deprecated in the future.
- Link to v1: https://lore.kernel.org/r/20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com

---
 Makefile                       |  4 +-
 ci/run-static-analysis.sh      |  2 +-
 contrib/coccinelle/meson.build | 31 ++++-----------
 meson.build                    | 86 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 98 insertions(+), 25 deletions(-)

Karthik Nayak (5):
      coccinelle: meson: rename variables to be more specific
      meson: move headers definition from 'contrib/coccinelle'
      meson: rename 'third_party_sources' to 'third_party_excludes'
      meson: add support for 'hdr-check'
      makefile/meson: add 'check-headers' as alias for 'hdr-check'

Range-diff versus v3:

1:  bc69638da5 = 1:  337431cf0f coccinelle: meson: rename variables to be more specific
2:  de50343e66 ! 2:  2b5bde0cb0 meson: move headers definition from 'contrib/coccinelle'
    @@ contrib/coccinelle/meson.build: foreach source : run_command(git, '-C', meson.pr
      
      coccinelle_headers = []
     -foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
    -+foreach header : headers
    ++foreach header : headers_to_check
        coccinelle_headers += meson.project_source_root() / header
      endforeach
      
    @@ meson.build: builtin_sources = [
     +]
     +
     +if git.found()
    -+  headers = []
    ++  headers_to_check = []
     +  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
    -+    headers += header
    ++    headers_to_check += header
     +  endforeach
     +endif
     +
-:  ---------- > 3:  2c5c9a9dc8 meson: rename 'third_party_sources' to 'third_party_excludes'
3:  fadfc7ef07 ! 4:  5b166c8bcd meson: add support for 'hdr-check'
    @@ Commit message
         the required dependencies.
     
         Also add the 'xdiff/' dir to the list of 'third_party_sources' as those
    -    headers must be skipped from the checks too!
    +    headers must be skipped from the checks too. This also skips the folder
    +    from the 'coccinelle' checks, this is okay, since this code is an
    +    external dependency.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## meson.build ##
    -@@ meson.build: third_party_sources = [
    -   ':!t/unit-tests/clar',
    +@@ meson.build: third_party_excludes = [
    +   ':!sha1dc',
        ':!t/unit-tests/clar',
        ':!t/t[0-9][0-9][0-9][0-9]*',
     +  ':!xdiff',
      ]
      
      if git.found()
    -@@ meson.build: if git.found()
    -   endforeach
    - endif
    - 
    -+generated_headers = [
    -+  'command-list.h',
    -+]
    -+
    - if not get_option('breaking_changes')
    -   builtin_sources += 'builtin/pack-redundant.c'
    - endif
    -@@ meson.build: builtin_sources += custom_target(
    -   ],
    -   env: script_environment,
    - )
    -+generated_headers += 'config-list.h'
    - 
    - builtin_sources += custom_target(
    -   input: 'Documentation/githooks.adoc',
    -@@ meson.build: builtin_sources += custom_target(
    -   ],
    -   env: script_environment,
    - )
    -+generated_headers += 'hook-list.h'
    - 
    - # This contains the variables for GIT-BUILD-OPTIONS, which we use to propagate
    - # build options to our tests.
     @@ meson.build: endif
      
      subdir('contrib')
      
    -+exclude_from_check_headers = generated_headers
    -+exclude_from_check_headers += [
    ++exclude_from_check_headers = [
     +  'compat/',
     +  'unicode-width.h',
     +]
    @@ meson.build: endif
     +if sha256_backend != 'nettle'
     +  exclude_from_check_headers += 'sha256/nettle.h'
     +endif
    -+if sha256_backend != 'gcrpyt'
    ++if sha256_backend != 'gcrypt'
     +  exclude_from_check_headers += 'sha256/gcrypt.h'
     +endif
     +
     +if git.found() and compiler.get_argument_syntax() == 'gcc'
     +  hco_targets = []
    -+  foreach h : headers
    ++  foreach h : headers_to_check
     +    skip_header = false
     +    foreach exclude : exclude_from_check_headers
     +      if h.startswith(exclude)
4:  66c5798df7 ! 5:  97e2e46dc1 makefile/meson: add 'check-headers' as alias for 'hdr-check'
    @@ meson.build: if git.found() and compiler.get_argument_syntax() == 'gcc'
          hco_targets += hco
        endforeach
      
    +-  alias_target('hdr-check', hco_targets)
     +  # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
    -   alias_target('hdr-check', hco_targets)
    -+  alias_target('check-headers', hco_targets)
    ++  hdr_check = alias_target('hdr-check', hco_targets)
    ++  alias_target('check-headers', hdr_check)
      endif
      
      foreach key, value : {


base-commit: 3a956c5f69873611ae5f8dcb9acd117f66b95ddc
change-id: 20250330-505-wire-up-sparse-via-meson-2e32dd31208b

Thanks
- Karthik

[PATCH v4 1/5] coccinelle: meson: rename variables to be more specific

[Karthik Nayak]

In Meson, included subdirs export their variables to top level Meson
builds. In 'contrib/coccinelle/meson.build', we define two such
variables `sources` and `headers`. While these variables are specific to
the checks in the 'contrib/coccinelle/' directory, they also pollute the
top level 'meson.build'.

Rename them to be more specific, this ensures that they aren't
mistakenly used in the upper levels and avoid variable name collisions.

While here, change the empty list denotation to be consistent with other
places.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index ea054c924f..03ce52d752 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -55,18 +55,18 @@ concatenated_rules = custom_target(
   capture: true,
 )
 
-sources = [ ]
+coccinelle_sources = []
 foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.c', third_party_sources, check: true).stdout().split()
-  sources += source
+  coccinelle_sources += source
 endforeach
 
-headers = [ ]
+coccinelle_headers = []
 foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
-  headers += meson.project_source_root() / header
+  coccinelle_headers += meson.project_source_root() / header
 endforeach
 
 patches = [ ]
-foreach source : sources
+foreach source : coccinelle_sources
   patches += custom_target(
     command: [
       spatch,
@@ -78,7 +78,7 @@ foreach source : sources
     input: meson.project_source_root() / source,
     output: source.underscorify() + '.patch',
     capture: true,
-    depend_files: headers,
+    depend_files: coccinelle_headers,
   )
 endforeach
 

-- 
2.48.1

[PATCH v4 2/5] meson: move headers definition from 'contrib/coccinelle'

[Karthik Nayak]

The Meson build for coccinelle static analysis lists all headers to
analyse. Due to the way Meson exports variables between subdirs, this
variable is also available in the root Meson build.

An upcoming commit, will add a new check complimenting 'hdr-check' in
the Makefile. This would require the list of headers. So move the
'coccinelle_headers' to the root Meson build and rename it to 'headers',
remove the root path being appended to each header and retain that in
the coccinelle Meson build since it is specific to the coccinelle build.

Also move the 'third_party_sources' variable to the root Meson build
since it is also a dependency for the 'headers' variable. This also
makes it easier to understand as the variable is now propagated from the
top level to the bottom.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 17 +----------------
 meson.build                    | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index 03ce52d752..4f07824402 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -8,21 +8,6 @@ if not spatch.found()
   subdir_done()
 endif
 
-third_party_sources = [
-  ':!contrib',
-  ':!compat/inet_ntop.c',
-  ':!compat/inet_pton.c',
-  ':!compat/nedmalloc',
-  ':!compat/obstack.*',
-  ':!compat/poll',
-  ':!compat/regex',
-  ':!sha1collisiondetection',
-  ':!sha1dc',
-  ':!t/unit-tests/clar',
-  ':!t/unit-tests/clar',
-  ':!t/t[0-9][0-9][0-9][0-9]*',
-]
-
 rules = [
   'array.cocci',
   'commit.cocci',
@@ -61,7 +46,7 @@ foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files',
 endforeach
 
 coccinelle_headers = []
-foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+foreach header : headers_to_check
   coccinelle_headers += meson.project_source_root() / header
 endforeach
 
diff --git a/meson.build b/meson.build
index e98cfa4909..66ee6fb096 100644
--- a/meson.build
+++ b/meson.build
@@ -633,6 +633,28 @@ builtin_sources = [
   'builtin/write-tree.c',
 ]
 
+third_party_sources = [
+  ':!contrib',
+  ':!compat/inet_ntop.c',
+  ':!compat/inet_pton.c',
+  ':!compat/nedmalloc',
+  ':!compat/obstack.*',
+  ':!compat/poll',
+  ':!compat/regex',
+  ':!sha1collisiondetection',
+  ':!sha1dc',
+  ':!t/unit-tests/clar',
+  ':!t/unit-tests/clar',
+  ':!t/t[0-9][0-9][0-9][0-9]*',
+]
+
+if git.found()
+  headers_to_check = []
+  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+    headers_to_check += header
+  endforeach
+endif
+
 if not get_option('breaking_changes')
   builtin_sources += 'builtin/pack-redundant.c'
 endif

-- 
2.48.1

[PATCH v4 3/5] meson: rename 'third_party_sources' to 'third_party_excludes'

[Karthik Nayak]

The 'third_party_sources' variable was moved to the root 'meson.build'
file in the previous commit. The variable is actually used to exclude
third party sources, so rename it accordingly to 'third_party_excludes'
to avoid confusion. While here, remove a duplicate from the list.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 2 +-
 meson.build                    | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index 4f07824402..dc3f73c2e7 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -41,7 +41,7 @@ concatenated_rules = custom_target(
 )
 
 coccinelle_sources = []
-foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.c', third_party_sources, check: true).stdout().split()
+foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.c', third_party_excludes, check: true).stdout().split()
   coccinelle_sources += source
 endforeach
 
diff --git a/meson.build b/meson.build
index 66ee6fb096..c4dcf756b3 100644
--- a/meson.build
+++ b/meson.build
@@ -633,7 +633,7 @@ builtin_sources = [
   'builtin/write-tree.c',
 ]
 
-third_party_sources = [
+third_party_excludes = [
   ':!contrib',
   ':!compat/inet_ntop.c',
   ':!compat/inet_pton.c',
@@ -644,13 +644,12 @@ third_party_sources = [
   ':!sha1collisiondetection',
   ':!sha1dc',
   ':!t/unit-tests/clar',
-  ':!t/unit-tests/clar',
   ':!t/t[0-9][0-9][0-9][0-9]*',
 ]
 
 if git.found()
   headers_to_check = []
-  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+  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
 endif

-- 
2.48.1

[PATCH v4 4/5] meson: add support for 'hdr-check'

[Karthik Nayak]

The Makefile supports a target called 'hdr-check', which checks if
individual header files can be independently compiled. Let's port this
functionality to Meson, our new build system too. The implementation
resembles that of the Makefile and provides the same check.

Since meson builds are out-of-tree, header dependencies are not
automatically met. So unlike the Makefile version, we also need to add
the required dependencies.

Also add the 'xdiff/' dir to the list of 'third_party_sources' as those
headers must be skipped from the checks too. This also skips the folder
from the 'coccinelle' checks, this is okay, since this code is an
external dependency.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 meson.build | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/meson.build b/meson.build
index c4dcf756b3..afbdc97fb6 100644
--- a/meson.build
+++ b/meson.build
@@ -645,6 +645,7 @@ third_party_excludes = [
   ':!sha1dc',
   ':!t/unit-tests/clar',
   ':!t/t[0-9][0-9][0-9][0-9]*',
+  ':!xdiff',
 ]
 
 if git.found()
@@ -1994,6 +1995,68 @@ endif
 
 subdir('contrib')
 
+exclude_from_check_headers = [
+  'compat/',
+  'unicode-width.h',
+]
+
+if sha1_backend != 'openssl'
+  exclude_from_check_headers += 'sha1/openssl.h'
+endif
+if sha256_backend != 'openssl'
+  exclude_from_check_headers += 'sha256/openssl.h'
+endif
+if sha256_backend != 'nettle'
+  exclude_from_check_headers += 'sha256/nettle.h'
+endif
+if sha256_backend != 'gcrypt'
+  exclude_from_check_headers += 'sha256/gcrypt.h'
+endif
+
+if git.found() and compiler.get_argument_syntax() == 'gcc'
+  hco_targets = []
+  foreach h : headers_to_check
+    skip_header = false
+    foreach exclude : exclude_from_check_headers
+      if h.startswith(exclude)
+        skip_header = true
+        break
+      endif
+    endforeach
+
+    if skip_header
+      continue
+    endif
+
+    hcc = custom_target(
+      input: h,
+      output: h.underscorify() + 'cc',
+      command: [
+        shell,
+        '-c',
+        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@'
+      ]
+    )
+
+    hco = custom_target(
+      input: hcc,
+      output: fs.replace_suffix(h.underscorify(), '.hco'),
+      command: [
+        compiler.cmd_array(),
+        libgit_c_args,
+        '-I', meson.project_source_root(),
+        '-I', meson.project_source_root() / 't/unit-tests',
+        '-o', '/dev/null',
+        '-c', '-xc',
+        '@INPUT@'
+      ]
+    )
+    hco_targets += hco
+  endforeach
+
+  alias_target('hdr-check', hco_targets)
+endif
+
 foreach key, value : {
   'DIFF': diff.full_path(),
   'GIT_SOURCE_DIR': meson.project_source_root(),

-- 
2.48.1

[PATCH v4 5/5] makefile/meson: add 'check-headers' as alias for 'hdr-check'

[Karthik Nayak]

The 'hdr-check' target in Meson and makefile is used to check if headers
can be compiled individually. The naming however isn't readable as 'hdr'
is not a common shortforme for 'header', neither is it an abbreviation.

Let's introduce 'check-headers' as an alternative target for 'hdr-check'
and add a `TODO` to deprecate the latter after 2 releases. Since this
is an internal tool, we can use a shorter deprecation cycle.

Change existing usage of 'hdr-check' in 'ci/run-static-analysis.sh' to
also use 'check-headers'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Makefile                  | 4 +++-
 ci/run-static-analysis.sh | 2 +-
 meson.build               | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index ac32d2d0bd..961ee508be 100644
--- a/Makefile
+++ b/Makefile
@@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc)
 $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE
 	$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<
 
-.PHONY: hdr-check $(HCO)
+# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
+.PHONY: hdr-check check-headers $(HCO)
 hdr-check: $(HCO)
+check-headers: hdr-check
 
 .PHONY: style
 style:
diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
index 0d51e5ce0e..60c175a094 100755
--- a/ci/run-static-analysis.sh
+++ b/ci/run-static-analysis.sh
@@ -26,7 +26,7 @@ then
 	exit 1
 fi
 
-make hdr-check ||
+make check-headers ||
 exit 1
 
 make check-pot
diff --git a/meson.build b/meson.build
index afbdc97fb6..39319e2610 100644
--- a/meson.build
+++ b/meson.build
@@ -2054,7 +2054,9 @@ if git.found() and compiler.get_argument_syntax() == 'gcc'
     hco_targets += hco
   endforeach
 
-  alias_target('hdr-check', hco_targets)
+  # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
+  hdr_check = alias_target('hdr-check', hco_targets)
+  alias_target('check-headers', hdr_check)
 endif
 
 foreach key, value : {

-- 
2.48.1

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

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> This is based on master 9d22ac5122 (The third batch, 2025-04-07) with
> 'es/meson-build-skip-coccinelle' merged in.
>
> ---
> Changes in v4:
> - Rename headers to headers_to_check, since these headers are only used
>   for static analysis.
> - Added a commit to rename third_party_sources -> third_party_excludes
>   and remove a duplicate.
> - Fix a typo 'gcrpyt' -> 'gcrypt'
> - Remove 'generated_headers', since we use 'git ls-files' and that would
>   already ignore files within '.gitignore'.
> - Link to v3: https://lore.kernel.org/r/20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745@gmail.com
>
> Changes in v3:
> - Some renames:
>   - headers_generated -> generated_headers
>   - meson -> Meson
>   - headers-check -> check-headers
>   - headers_check_exclude -> exclude_from_check_headers
> - Rewrite 'headers_check_exclude' to also contain dirs so we can skip
>   listing individual header files.
> - Move 'xdiff/*' to 'third_party_sources' and cleanup
>   'exclude_from_check_headers'.
> - Use 'echo' instead of 'echo -n'.
> - Use `fs.replace_suffix` instead of `str.replace`.
> - Link to v2: https://lore.kernel.org/r/20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com

Like the previous round, this round also seems to break linux.meson
job at GitHub Actions CI when merged to 'seen'.  It may be quite
possible that it is caused by some semantic conflicts, and help to
find where the merged result is wrong is very much appreciated.

For now, I've ejected the topic out of 'seen' again.

Failing CI run:

https://github.com/git/git/actions/runs/14563669225/job/40850047961

The same 'seen' without the merge of this topic:

https://github.com/git/git/actions/runs/14564707024/job/40852298435

Note that I expect win+Meson test (3) job to fail/time-out even
without this topic.  I do not know how the numbered test jobs for
win+Meson part of the CI are sharded, but one of them never finishes
in 'next' so this topic is unlikely to be the cause of the breakage.

Addendum.

The topic alone, rebuilt as specified in your cover letter, seems to
be sufficient to make linux-meson job fail, without any interaction
with other topics in 'seen'.

https://github.com/git/git/actions/runs/14566676782/job/40856998621

Note that we have unrelated breakages like pedantic (due to update to
the fedora image) and sparse (due to deprecation of the ubuntu
image) we fixed since the base commit you picked, so failures of
these jobs are not all that interesting in this run.  But the
breakage of linux-meson job is unique to this topic.

Interestingly, either this topic alone or 'seen' with this topic I
haven't seen the "meson setup + compile + test" sequence in my local
testing, so the breakage seems to be isolated to the CI job.

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

[Phillip Wood]

On 21/04/2025 08:48, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
> 
>> This is based on master 9d22ac5122 (The third batch, 2025-04-07) with
>> 'es/meson-build-skip-coccinelle' merged in.
>>
> 
> Like the previous round, this round also seems to break linux.meson
> job at GitHub Actions CI when merged to 'seen'.  It may be quite
> possible that it is caused by some semantic conflicts, and help to
> find where the merged result is wrong is very much appreciated.
> 
> For now, I've ejected the topic out of 'seen' again.
> 
> Failing CI run:
> 
> https://github.com/git/git/actions/runs/14563669225/job/40850047961
> 

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

Best Wishes

Phillip


Run actions/checkout@v4
/usr/bin/docker exec 
6334961fdc01ddadb7a7af1fadd8ae33a6fce79b7428255d2231145f5e09f51d sh -c 
"cat /etc/*release | grep ^ID"
Syncing repository: git/git
Getting Git version info
   Working directory is '/__w/git/git'
Deleting the contents of '/__w/git/git'
The repository will be downloaded using the GitHub REST API
To create a local Git repository instead, add Git 2.18 or higher to the PATH
Downloading the archive
Writing archive to disk
Extracting the archive
/usr/bin/tar xz --warning=no-unknown-keyword --overwrite -C 
/__w/git/git/23521f8f-82bd-4a9b-994a-cacfd7101756 -f 
/__w/git/git/23521f8f-82bd-4a9b-994a-cacfd7101756.tar.gz
Resolved version git-git-c9e21a0

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

[Karthik Nayak]

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 21/04/2025 08:48, Junio C Hamano wrote:
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>>> This is based on master 9d22ac5122 (The third batch, 2025-04-07) with
>>> 'es/meson-build-skip-coccinelle' merged in.
>>>
>>
>> Like the previous round, this round also seems to break linux.meson
>> job at GitHub Actions CI when merged to 'seen'.  It may be quite
>> possible that it is caused by some semantic conflicts, and help to
>> find where the merged result is wrong is very much appreciated.
>>
>> For now, I've ejected the topic out of 'seen' again.
>>
>> Failing CI run:
>>
>> https://github.com/git/git/actions/runs/14563669225/job/40850047961
>>
>
> "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.
>
> Best Wishes
>
> Phillip
>
>
> Run actions/checkout@v4
> /usr/bin/docker exec
> 6334961fdc01ddadb7a7af1fadd8ae33a6fce79b7428255d2231145f5e09f51d sh -c
> "cat /etc/*release | grep ^ID"
> Syncing repository: git/git
> Getting Git version info
>    Working directory is '/__w/git/git'
> Deleting the contents of '/__w/git/git'
> The repository will be downloaded using the GitHub REST API
> To create a local Git repository instead, add Git 2.18 or higher to the PATH
> Downloading the archive
> Writing archive to disk
> Extracting the archive
> /usr/bin/tar xz --warning=no-unknown-keyword --overwrite -C
> /__w/git/git/23521f8f-82bd-4a9b-994a-cacfd7101756 -f
> /__w/git/git/23521f8f-82bd-4a9b-994a-cacfd7101756.tar.gz
> Resolved version git-git-c9e21a0

Yup, this seems to be the reason, I was primarily using GitLab CI for
ensuring everything was working, apart from locally testing it. Seems
like there is no issues on GitLab CI:

  https://gitlab.com/gitlab-org/git/-/pipelines/1777164771

So the only difference, is that, like you mentioned, GitHub strips the
repository information by downloading a tar file using the REST API. The
reason seems to be due to missing the 'git' executable:

  https://github.com/actions/checkout/issues/363

I tested this with the following patch, which seems to fix the issue at
hand. Will send in a new version with this patch, once I validate all
the tests.

-- >8 --

commit b4567cac2b5eb5e9b50b468a85854bad66a9f445
Author: Karthik Nayak <karthik.188@gmail.com>
Date:   Mon Apr 21 13:06:45 2025 +0200

    ci/github: install git before checking out the repository

    The GitHub CI workflow uses 'actions/checkout@v4' to checkout the
    repository. This action defaults to using the GitHub REST API to obtain
    the repository if Git isn't present. The REST API downloads a tar of the
    repository sans the Git information. Since we don't install Git before
    this step, using the REST API is the current behavior.

    The following commits will add the 'hdr-check' static check to meson.
    The check will use 'git ls-files' to obtain the set of header files.
    This will fail if the repository doesn't contain the Git directory. So
    install Git before running the 'actions/checkout@v4' action.

    Signed-off-by: Karthik Nayak <karthik.188@gmail.com>

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 37541f3d10..a09fcf4d72 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -414,6 +414,16 @@ jobs:
     - name: prepare libc6 for actions
       if: matrix.vector.jobname == 'linux32'
       run: apt -q update && apt -q -y install libc6-amd64 lib64stdc++6
+    - name: install git in container
+      run: |
+        if [ -f /etc/alpine-release ]; then
+          apk update && apk add --no-cache git
+        elif [ -f /etc/almalinux-release ] || [ -f /etc/redhat-release ]; then
+           dnf -y install git
+        else
+          apt -q update && apt -q -y install git
+        fi
+        git config --global --add safe.directory "$GITHUB_WORKSPACE"
     - uses: actions/checkout@v4
     - run: ci/install-dependencies.sh
     - run: useradd builder --create-home

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

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

[Phillip Wood]

Hi Karthik

On 21/04/2025 16:33, Karthik Nayak wrote:

Thanks for putting this together, I've left a couple of code comments below.

>      Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 37541f3d10..a09fcf4d72 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -414,6 +414,16 @@ jobs:
>       - name: prepare libc6 for actions
>         if: matrix.vector.jobname == 'linux32'
>         run: apt -q update && apt -q -y install libc6-amd64 lib64stdc++6
> +    - name: install git in container
> +      run: |
> +        if [ -f /etc/alpine-release ]; then
> +          apk update && apk add --no-cache git
> +        elif [ -f /etc/almalinux-release ] || [ -f /etc/redhat-release ]; then
> +           dnf -y install git
> +        else
> +          apt -q update && apt -q -y install git
> +        fi
> +        git config --global --add safe.directory "$GITHUB_WORKSPACE"

I'd be tempted to check for which package manager to use by using 
`command -v`. That way the only distribution specific knowledge we need 
is the package manager and we don't have to worry about the names of the 
various release files in /etc.

	if command -v git
	then
		: nothing to do
	elif command -v apk
	then
		apk add git
	elif command -v dnf
	then
		dnf -y install git
	else
		apt-get -q -y install git
	fi

The commands above omit anything that updates the package cache as we do 
that anyway in install-dependencies.sh and we only really care about 
getting some version of git installed here. It also uses apt-get to 
match what we do in install-dependencies.sh

I also wonder if we should ditch the checkout action and use something like

     git clone --depth=1 --single-branch ${GITHUB_REF_NAME} \
	${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}.git

so that we know we will be building from a git repository.

Best Wishes

Phillip

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

[Karthik Nayak]

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Karthik
>
> On 21/04/2025 16:33, Karthik Nayak wrote:
>
> Thanks for putting this together, I've left a couple of code comments below.
>
>>      Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>>
>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
>> index 37541f3d10..a09fcf4d72 100644
>> --- a/.github/workflows/main.yml
>> +++ b/.github/workflows/main.yml
>> @@ -414,6 +414,16 @@ jobs:
>>       - name: prepare libc6 for actions
>>         if: matrix.vector.jobname == 'linux32'
>>         run: apt -q update && apt -q -y install libc6-amd64 lib64stdc++6
>> +    - name: install git in container
>> +      run: |
>> +        if [ -f /etc/alpine-release ]; then
>> +          apk update && apk add --no-cache git
>> +        elif [ -f /etc/almalinux-release ] || [ -f /etc/redhat-release ]; then
>> +           dnf -y install git
>> +        else
>> +          apt -q update && apt -q -y install git
>> +        fi
>> +        git config --global --add safe.directory "$GITHUB_WORKSPACE"
>
> I'd be tempted to check for which package manager to use by using
> `command -v`. That way the only distribution specific knowledge we need
> is the package manager and we don't have to worry about the names of the
> various release files in /etc.
>
> 	if command -v git
> 	then
> 		: nothing to do
> 	elif command -v apk
> 	then
> 		apk add git
> 	elif command -v dnf
> 	then
> 		dnf -y install git
> 	else
> 		apt-get -q -y install git
> 	fi
>
> The commands above omit anything that updates the package cache as we do
> that anyway in install-dependencies.sh and we only really care about
> getting some version of git installed here. It also uses apt-get to
> match what we do in install-dependencies.sh
>

Yeah this makes sense, we don't have to worry about specific
distributions.

> I also wonder if we should ditch the checkout action and use something like
>
>      git clone --depth=1 --single-branch ${GITHUB_REF_NAME} \
> 	${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}.git
>
> so that we know we will be building from a git repository.
>

Possibly, but I'll leave this out as I feel we're already straying from
the topic now. Let me know if you feel strongly about it.

> Best Wishes
>
> Phillip

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

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

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> I'd be tempted to check for which package manager to use by using
> `command -v`. That way the only distribution specific knowledge we
> need is the package manager and we don't have to worry about the names
> of the various release files in /etc.
>
> 	if command -v git
> 	then
> 		: nothing to do
> 	elif command -v apk
> 	then
> 		apk add git
> 	elif command -v dnf
> 	then
> 		dnf -y install git
> 	else
> 		apt-get -q -y install git
> 	fi

OK.  "command -v" should be portable enough these days (in the past
people used "type" and yelled at by portability sherriff).  And
having one command line per package manager should be simpler than
having one command line per distro, provided if two distros that
share the same package manager name the "git" package the same way.
We had trouble with "awk" recently ;-)

Curious that we do not check the availability of apt-get here (or
just "apt").

> The commands above omit anything that updates the package cache as we
> do that anyway in install-dependencies.sh and we only really care
> about getting some version of git installed here. It also uses apt-get
> to match what we do in install-dependencies.sh

OK.

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

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 1189 bytes --]

On 4/22/25 11:55 AM, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> I'd be tempted to check for which package manager to use by using
>> `command -v`. That way the only distribution specific knowledge we
>> need is the package manager and we don't have to worry about the names
>> of the various release files in /etc.
>>
>> 	if command -v git
>> 	then
>> 		: nothing to do
>> 	elif command -v apk
>> 	then
>> 		apk add git
>> 	elif command -v dnf
>> 	then
>> 		dnf -y install git
>> 	else
>> 		apt-get -q -y install git
>> 	fi
> 
> OK.  "command -v" should be portable enough these days (in the past
> people used "type" and yelled at by portability sherriff). 


It (command -v) is available since POSIX 2008, yes. Optional before
that, and thus available in many (but not all) contexts.

Debian upgraded from POSIX 2001 to POSIX 2008, as of July 2018, see
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=864615

Quotable quote:

"""
This version of the standard is so outdated that it isn't even any
longer available on the opengroup web site. [...] Please consider
updating the policy.
"""


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Phillip Wood]

On 22/04/2025 16:55, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> I'd be tempted to check for which package manager to use by using
>> `command -v`. That way the only distribution specific knowledge we
>> need is the package manager and we don't have to worry about the names
>> of the various release files in /etc.
>>
>> 	if command -v git
>> 	then
>> 		: nothing to do
>> 	elif command -v apk
>> 	then
>> 		apk add git
>> 	elif command -v dnf
>> 	then
>> 		dnf -y install git
>> 	else
>> 		apt-get -q -y install git
>> 	fi
> 
> OK.  "command -v" should be portable enough these days (in the past
> people used "type" and yelled at by portability sherriff).  And
> having one command line per package manager should be simpler than
> having one command line per distro, provided if two distros that
> share the same package manager name the "git" package the same way.
> We had trouble with "awk" recently ;-)

Oh, I'd not thought that different distributions might have different 
package names for git. We already have a few uses of "command -v" in our 
tests and ci scripts so I don't think using it here should be an issue.

> Curious that we do not check the availability of apt-get here (or
> just "apt").
It is a lazy way of erroring out if we add a new image that uses a 
different package manager to the matrix and forget to update the list 
here. We could instead check for apt-get and add an else clause that 
prints a proper message.

Best Wishes

Phillip

>> The commands above omit anything that updates the package cache as we
>> do that anyway in install-dependencies.sh and we only really care
>> about getting some version of git installed here. It also uses apt-get
>> to match what we do in install-dependencies.sh
> 
> OK.

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

[Karthik Nayak]

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Karthik
>
> On 21/04/2025 16:33, Karthik Nayak wrote:
>
> Thanks for putting this together, I've left a couple of code comments below.
>
>>      Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>>
>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
>> index 37541f3d10..a09fcf4d72 100644
>> --- a/.github/workflows/main.yml
>> +++ b/.github/workflows/main.yml
>> @@ -414,6 +414,16 @@ jobs:
>>       - name: prepare libc6 for actions
>>         if: matrix.vector.jobname == 'linux32'
>>         run: apt -q update && apt -q -y install libc6-amd64 lib64stdc++6
>> +    - name: install git in container
>> +      run: |
>> +        if [ -f /etc/alpine-release ]; then
>> +          apk update && apk add --no-cache git
>> +        elif [ -f /etc/almalinux-release ] || [ -f /etc/redhat-release ]; then
>> +           dnf -y install git
>> +        else
>> +          apt -q update && apt -q -y install git
>> +        fi
>> +        git config --global --add safe.directory "$GITHUB_WORKSPACE"
>
> I'd be tempted to check for which package manager to use by using
> `command -v`. That way the only distribution specific knowledge we need
> is the package manager and we don't have to worry about the names of the
> various release files in /etc.
>
> 	if command -v git
> 	then
> 		: nothing to do
> 	elif command -v apk
> 	then
> 		apk add git
> 	elif command -v dnf
> 	then
> 		dnf -y install git
> 	else
> 		apt-get -q -y install git
> 	fi
>
> The commands above omit anything that updates the package cache as we do
> that anyway in install-dependencies.sh and we only really care about
> getting some version of git installed here. It also uses apt-get to
> match what we do in install-dependencies.sh
>

Seems like this is a no-go, since apt-get fails [1] without first
updating the package cache. So I'm going to do that for all the
commands, which should also ensure that the package cache update in
'install-dependencies.sh' is mostly a no-op.

[1]: https://github.com/gitgitgadget/git/actions/runs/14598683520/job/40951070359?pr=1905

> I also wonder if we should ditch the checkout action and use something like
>
>      git clone --depth=1 --single-branch ${GITHUB_REF_NAME} \
> 	${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}.git
>
> so that we know we will be building from a git repository.
>
> Best Wishes
>
> Phillip

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

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

[Phillip Wood]

Hi Karthik

On 22/04/2025 19:56, Karthik Nayak wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>> Hi Karthik
>> The commands above omit anything that updates the package cache as we do
>> that anyway in install-dependencies.sh and we only really care about
>> getting some version of git installed here. It also uses apt-get to
>> match what we do in install-dependencies.sh
>>
> 
> Seems like this is a no-go, since apt-get fails [1] without first
> updating the package cache.

That's a shame, thanks for trying it

> So I'm going to do that for all the
> commands, which should also ensure that the package cache update in
> 'install-dependencies.sh' is mostly a no-op.

Fair enough

Thanks

Phillip
> [1]: https://github.com/gitgitgadget/git/actions/runs/14598683520/job/40951070359?pr=1905
> 
>> I also wonder if we should ditch the checkout action and use something like
>>
>>       git clone --depth=1 --single-branch ${GITHUB_REF_NAME} \
>> 	${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}.git
>>
>> so that we know we will be building from a git repository.
>>
>> Best Wishes
>>
>> Phillip

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

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> 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...

Thanks.

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

[Phillip Wood]

On 21/04/2025 16:41, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> 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. The Makefile falls back to using "find" if "git ls-files" fails 
which is another option.

Best Wishes

Phillip

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

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 21/04/2025 16:41, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> 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. The Makefile falls back to using "find" if "git ls-files"
> fails which is another option.

Yuck.  I somehow thought that CI jobs are always using a git
checkout, not tarball extract (after all, that is what
actions/checkout implies to me X-<).  Of course it is good to
automatically ensure that our tarball extracts are buildable, but
the way tarballs are built upon release is probably different from
how these tarballs are made automatically (*), so in that sense not
building from a repository but building from "git archive" extract
is not doing anybody a service.


[Footnote]

 * "make dist" is how a release tarball is built for this project,
   not "git archive dist.tar HEAD".

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

[Phillip Wood]

On 21/04/2025 21:40, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
> Yuck.  I somehow thought that CI jobs are always using a git
> checkout, not tarball extract (after all, that is what
> actions/checkout implies to me X-<). 

That's what I'd assumed as well, I was quite surprised when I realized 
the "checkout" action was actually unpacking a tarball

Best Wishes

Phillip

> Of course it is good to
> automatically ensure that our tarball extracts are buildable, but
> the way tarballs are built upon release is probably different from
> how these tarballs are made automatically (*), so in that sense not
> building from a repository but building from "git archive" extract
> is not doing anybody a service.
> 
> 
> [Footnote]
> 
>   * "make dist" is how a release tarball is built for this project,
>     not "git archive dist.tar HEAD".
> 

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

[Patrick Steinhardt]

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 <phillip.wood123@gmail.com> 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

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

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Phillip Wood <phillip.wood123@gmail.com> 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...
>

So the steps in the GitHub CI are:

  ...
  - uses: actions/checkout@v4 #1
  - run: ci/install-dependencies.sh #2
  ...
  - run: sudo --preserve-env --set-home --user=builder
ci/run-build-and-tests.sh #3
  ...

Step #1, clones the repository, since the `git` executable isn't present
at this step, it uses GitHub's REST API to obtain a tar of the
repository.

Step #2, installs all dependencies, which includes the `git` executable.

Step #3, sets up the build, which includes setting up meson in the meson
job. At this point the `git` executable is present, so within meson
`git.found()` would be true. As such we run 'git ls-files' as part of my
patch series, but since the repository doesn't contain the `.git`
folder, the command fails.

So like Phillip mentioned, we need to ensure that the `git` executable
is present before step #1.

I hope that makes sense.

> Thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

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

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> Step #1, clones the repository, since the `git` executable isn't present
> at this step, it uses GitHub's REST API to obtain a tar of the
> repository.
>
> Step #2, installs all dependencies, which includes the `git` executable.
>
> Step #3, sets up the build, which includes setting up meson in the meson
> job. At this point the `git` executable is present, so within meson
> `git.found()` would be true. As such we run 'git ls-files' as part of my
> patch series, but since the repository doesn't contain the `.git`
> folder, the command fails.
>
> So like Phillip mentioned, we need to ensure that the `git` executable
> is present before step #1.
>
> I hope that makes sense.

Please roll that into the appropriate commit log message for the fix
you'd send out, so the next person who wonders why this topic broke
the CI does not have to ask the same question.

Would it make sense to just swap the order, then?  Our sources are
meant to be buildable from either release tarballs (which is created
by "make dist") or a repository (with .git), but from the analysis
of Phillip and you, it sounds like the CI environment has been
building and testing from a "git archive HEAD" output extracted as a
tarball, which is *not* something any real users build from.  Making
sure that building from release tarballs works is a good thing to
ensure in CI, because all our developers are testing in their own
repository (with .git) so we wouldn't easily notice ourselves if we
broke the build procedure in such a way that it would somehow
require say "git describe" or "git ls-files" to work.

Thanks.

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

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Step #1, clones the repository, since the `git` executable isn't present
>> at this step, it uses GitHub's REST API to obtain a tar of the
>> repository.
>>
>> Step #2, installs all dependencies, which includes the `git` executable.
>>
>> Step #3, sets up the build, which includes setting up meson in the meson
>> job. At this point the `git` executable is present, so within meson
>> `git.found()` would be true. As such we run 'git ls-files' as part of my
>> patch series, but since the repository doesn't contain the `.git`
>> folder, the command fails.
>>
>> So like Phillip mentioned, we need to ensure that the `git` executable
>> is present before step #1.
>>
>> I hope that makes sense.
>
> Please roll that into the appropriate commit log message for the fix
> you'd send out, so the next person who wonders why this topic broke
> the CI does not have to ask the same question.
>

Yeah, will do!

> Would it make sense to just swap the order, then?

Unfortunately not, this is a chicken-egg problem. The dependencies are
installed by 'ci/install-dependencies.sh', which are not present until
the source is available.

> Our sources are
> meant to be buildable from either release tarballs (which is created
> by "make dist") or a repository (with .git), but from the analysis
> of Phillip and you, it sounds like the CI environment has been
> building and testing from a "git archive HEAD" output extracted as a
> tarball, which is *not* something any real users build from.  Making
> sure that building from release tarballs works is a good thing to
> ensure in CI, because all our developers are testing in their own
> repository (with .git) so we wouldn't easily notice ourselves if we
> broke the build procedure in such a way that it would somehow
> require say "git describe" or "git ls-files" to work.
>

Another additional point is that this also means that the two CIs
(GitLab and GitHub) now run builds differently, while it would be nice
to have a separate job to test building on tarballs. I would say
currently we should fix this to build on a Git repository.

> Thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

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

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

>> Would it make sense to just swap the order, then?
>
> Unfortunately not, this is a chicken-egg problem. The dependencies are
> installed by 'ci/install-dependencies.sh', which are not present until
> the source is available.

Ah, of course.  Thanks.

[PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check

[Karthik Nayak]

To bridge the remaining gaps between Makefile and Meson, this patch
series adds 'hdr-check' to Meson to compliment the Makefile's
'hdr-check'.

We also introduce 'headers-check' as an alias to 'hdr-check' as a better
named replacement in both Meson and make and add a note to deprecate
'hdr-check' in the future.

The first two commits are small cleanups, where we re-organize existing
variables to make it easier to add the target. The third commit adds the
'hdr-check' target to Meson. The last commit introduces the
'headers-check' alias to both Meson and the makefile and marks
'hdr-check' to be deprecated.

This is based on master 9d22ac5122 (The third batch, 2025-04-07) with
'es/meson-build-skip-coccinelle' merged in.

---
Changes in v5:
- Add a commit to install 'git' in GitHub's CI before the repository is
  checked out. Without the presence of the 'git' executable, GitHub
  downloads a tar of the repo instead of cloning it. This causes the
  patch series to fail in CI.
- Expose the 'headers_to_check' variable even if 'git' executable is not
  found or not a repository. This ensures dependencies of
  'headers_to_check' can simply rely on its length. We also check that
  the '.git' folder is setup before populating 'headers_to_check', this
  ensures Meson doesn't fail in tarball of the Git source code.
- Link to v4: https://lore.kernel.org/r/20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822@gmail.com

Changes in v4:
- Rename headers to headers_to_check, since these headers are only used
  for static analysis.
- Added a commit to rename third_party_sources -> third_party_excludes
  and remove a duplicate.
- Fix a typo 'gcrpyt' -> 'gcrypt'
- Remove 'generated_headers', since we use 'git ls-files' and that would
  already ignore files within '.gitignore'.
- Link to v3: https://lore.kernel.org/r/20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745@gmail.com

Changes in v3:
- Some renames:
  - headers_generated -> generated_headers
  - meson -> Meson
  - headers-check -> check-headers
  - headers_check_exclude -> exclude_from_check_headers
- Rewrite 'headers_check_exclude' to also contain dirs so we can skip
  listing individual header files.
- Move 'xdiff/*' to 'third_party_sources' and cleanup
  'exclude_from_check_headers'.
- Use 'echo' instead of 'echo -n'.
- Use `fs.replace_suffix` instead of `str.replace`.
- Link to v2: https://lore.kernel.org/r/20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com

Changes in v2:
- Add 'hdr-check' to meson, while introducing 'headers-check' as
  a replacement alias. Schedule 'hdr-check' to be deprecated in the future.
- Link to v1: https://lore.kernel.org/r/20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com

---
 .github/workflows/main.yml     | 14 +++++++
 Makefile                       |  4 +-
 ci/run-static-analysis.sh      |  2 +-
 contrib/coccinelle/meson.build | 31 ++++-----------
 meson.build                    | 86 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 112 insertions(+), 25 deletions(-)

Karthik Nayak (6):
      ci/github: install git before checking out the repository
      coccinelle: meson: rename variables to be more specific
      meson: move headers definition from 'contrib/coccinelle'
      meson: rename 'third_party_sources' to 'third_party_excludes'
      meson: add support for 'hdr-check'
      makefile/meson: add 'check-headers' as alias for 'hdr-check'

Range-diff versus v4:

-:  ---------- > 1:  d20993a59e ci/github: install git before checking out the repository
1:  0f1160fa78 = 2:  6eb10dd3b1 coccinelle: meson: rename variables to be more specific
2:  933b199def ! 3:  7950b44c6f meson: move headers definition from 'contrib/coccinelle'
    @@ Commit message
         makes it easier to understand as the variable is now propagated from the
         top level to the bottom.
     
    +    While 'headers_to_check' is only computed when we have a repository and
    +    the 'git' executable is present, the variable itself is exposed as an
    +    empty array. This allows dependencies in upcoming commits to simply
    +    check for length of the array and not worry about dependencies required
    +    to actually populate the array.
    +
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## contrib/coccinelle/meson.build ##
    @@ meson.build: builtin_sources = [
     +  ':!t/t[0-9][0-9][0-9][0-9]*',
     +]
     +
    -+if git.found()
    -+  headers_to_check = []
    ++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_sources, check: true).stdout().split()
     +    headers_to_check += header
     +  endforeach
3:  ab04192864 ! 4:  7fd64b788b meson: rename 'third_party_sources' to 'third_party_excludes'
    @@ meson.build: third_party_sources = [
        ':!t/t[0-9][0-9][0-9][0-9]*',
      ]
      
    - if git.found()
    -   headers_to_check = []
    + 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_sources, check: true).stdout().split()
     +  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
4:  69a4b5abc9 ! 5:  df0472be57 meson: add support for 'hdr-check'
    @@ meson.build: third_party_excludes = [
     +  ':!xdiff',
      ]
      
    - if git.found()
    + headers_to_check = []
     @@ meson.build: endif
      
      subdir('contrib')
    @@ meson.build: endif
     +  exclude_from_check_headers += 'sha256/gcrypt.h'
     +endif
     +
    -+if git.found() and compiler.get_argument_syntax() == 'gcc'
    ++if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
     +  hco_targets = []
     +  foreach h : headers_to_check
     +    skip_header = false
5:  a0c799cf8c ! 6:  7d9a33767b makefile/meson: add 'check-headers' as alias for 'hdr-check'
    @@ ci/run-static-analysis.sh: then
      make check-pot
     
      ## meson.build ##
    -@@ meson.build: if git.found() and compiler.get_argument_syntax() == 'gcc'
    +@@ meson.build: if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
          hco_targets += hco
        endforeach
      


base-commit: 3a956c5f69873611ae5f8dcb9acd117f66b95ddc
change-id: 20250330-505-wire-up-sparse-via-meson-2e32dd31208b

Thanks
- Karthik

[PATCH v5 1/6] ci/github: install git before checking out the repository

[Karthik Nayak]

The GitHub's CI workflow uses 'actions/checkout@v4' to checkout the
repository. This action defaults to using the GitHub REST API to obtain
the repository if the `git` executable isn't available.

The step to build Git in the GitHub workflow can be summarized as:

  ...
  - uses: actions/checkout@v4 #1
  - run: ci/install-dependencies.sh #2
  ...
  - run: sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh #3
  ...

Step #1, clones the repository, since the `git` executable isn't present
at this step, it uses GitHub's REST API to obtain a tar of the
repository.

Step #2, installs all dependencies, which includes the `git` executable.

Step #3, sets up the build, which includes setting up meson in the meson
job. At this point the `git` executable is present.

This means while the `git` executable is present, the repository doesn't
contain the '.git' folder. To keep both the CI's (GitLab and GitHub)
behavior consistent and to ensure that the build is performed on a
real-world scenario, install `git` before the repository is checked out.
This ensures that 'actions/checkout@v4' will clone the repository
instead of using a tarball. We also update the package cache while
installing `git`, this is because some distros will fail to locate the
package without updating the cache.

Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 .github/workflows/main.yml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 37541f3d10..e9112b3a64 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -414,6 +414,20 @@ jobs:
     - name: prepare libc6 for actions
       if: matrix.vector.jobname == 'linux32'
       run: apt -q update && apt -q -y install libc6-amd64 lib64stdc++6
+    - name: install git in container
+      run: |
+        if command -v git
+        then
+          : # nothing to do
+        elif command -v apk
+        then
+          apk add --update git
+        elif command -v dnf
+        then
+          dnf -yq update && dnf -yq install git
+        else
+          apt-get -q update && apt-get -q -y install git
+        fi
     - uses: actions/checkout@v4
     - run: ci/install-dependencies.sh
     - run: useradd builder --create-home

-- 
2.48.1

Re: [PATCH v5 1/6] ci/github: install git before checking out the repository

[phillip.wood123]

Hi Karthik

This looks good, I've left a few comments about the wording of the 
commit message but I wouldn't worry too much unless you end up 
re-rolling for some other reason.

On 23/04/2025 09:15, Karthik Nayak wrote:
> The GitHub's CI workflow uses 'actions/checkout@v4' to checkout the

We don't need "The" here

> repository. This action defaults to using the GitHub REST API to obtain

I'd maybe say "falls back" rather than "defaults"

> the repository if the `git` executable isn't available.
> 
> The step to build Git in the GitHub workflow can be summarized as:
> 
>    ...
>    - uses: actions/checkout@v4 #1
>    - run: ci/install-dependencies.sh #2
>    ...
>    - run: sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh #3
>    ...
> 
> Step #1, clones the repository, since the `git` executable isn't present

It would be more accurate to say that it tries to clone the repository - 
if we fall back to extracting a tarball then we're not cloning.

> at this step, it uses GitHub's REST API to obtain a tar of the
> repository.
> 
> Step #2, installs all dependencies, which includes the `git` executable.
> 
> Step #3, sets up the build, which includes setting up meson in the meson
> job. At this point the `git` executable is present.
> 
> This means while the `git` executable is present, the repository doesn't
> contain the '.git' folder.

I'd maybe say "source tree" instead of "repository" as it isn't a 
repository without a ".git" directory.

> To keep both the CI's (GitLab and GitHub)
> behavior consistent and to ensure that the build is performed on a
> real-world scenario, install `git` before the repository is checked out.
> This ensures that 'actions/checkout@v4' will clone the repository
> instead of using a tarball. We also update the package cache while
> installing `git`, this is because some distros will fail to locate the
> package without updating the cache.

Nice explanation, the code changes look good

Thanks

Phillip

> Helped-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>   .github/workflows/main.yml | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 37541f3d10..e9112b3a64 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -414,6 +414,20 @@ jobs:
>       - name: prepare libc6 for actions
>         if: matrix.vector.jobname == 'linux32'
>         run: apt -q update && apt -q -y install libc6-amd64 lib64stdc++6
> +    - name: install git in container
> +      run: |
> +        if command -v git
> +        then
> +          : # nothing to do
> +        elif command -v apk
> +        then
> +          apk add --update git
> +        elif command -v dnf
> +        then
> +          dnf -yq update && dnf -yq install git
> +        else
> +          apt-get -q update && apt-get -q -y install git
> +        fi
>       - uses: actions/checkout@v4
>       - run: ci/install-dependencies.sh
>       - run: useradd builder --create-home
> 

Re: [PATCH v5 1/6] ci/github: install git before checking out the repository

[Karthik Nayak]

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

phillip.wood123@gmail.com writes:

> Hi Karthik
>
> This looks good, I've left a few comments about the wording of the
> commit message but I wouldn't worry too much unless you end up
> re-rolling for some other reason.
>
> On 23/04/2025 09:15, Karthik Nayak wrote:
>> The GitHub's CI workflow uses 'actions/checkout@v4' to checkout the
>
> We don't need "The" here
>

Yeah, I think we can remove it.

>> repository. This action defaults to using the GitHub REST API to obtain
>
> I'd maybe say "falls back" rather than "defaults"
>

Right!

>> the repository if the `git` executable isn't available.
>>
>> The step to build Git in the GitHub workflow can be summarized as:
>>
>>    ...
>>    - uses: actions/checkout@v4 #1
>>    - run: ci/install-dependencies.sh #2
>>    ...
>>    - run: sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh #3
>>    ...
>>
>> Step #1, clones the repository, since the `git` executable isn't present
>
> It would be more accurate to say that it tries to clone the repository -
> if we fall back to extracting a tarball then we're not cloning.
>

Yes indeed.

>> at this step, it uses GitHub's REST API to obtain a tar of the
>> repository.
>>
>> Step #2, installs all dependencies, which includes the `git` executable.
>>
>> Step #3, sets up the build, which includes setting up meson in the meson
>> job. At this point the `git` executable is present.
>>
>> This means while the `git` executable is present, the repository doesn't
>> contain the '.git' folder.
>
> I'd maybe say "source tree" instead of "repository" as it isn't a
> repository without a ".git" directory.
>

Good point.

>> To keep both the CI's (GitLab and GitHub)
>> behavior consistent and to ensure that the build is performed on a
>> real-world scenario, install `git` before the repository is checked out.
>> This ensures that 'actions/checkout@v4' will clone the repository
>> instead of using a tarball. We also update the package cache while
>> installing `git`, this is because some distros will fail to locate the
>> package without updating the cache.
>
> Nice explanation, the code changes look good
>

Thanks for the review. I'll add it locally to my tree. That way if I end
up with a new version, It'll incorporate these changes.

> Thanks
>
> Phillip
>
>> Helped-by: Phillip Wood <phillip.wood123@gmail.com>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>   .github/workflows/main.yml | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
>> index 37541f3d10..e9112b3a64 100644
>> --- a/.github/workflows/main.yml
>> +++ b/.github/workflows/main.yml
>> @@ -414,6 +414,20 @@ jobs:
>>       - name: prepare libc6 for actions
>>         if: matrix.vector.jobname == 'linux32'
>>         run: apt -q update && apt -q -y install libc6-amd64 lib64stdc++6
>> +    - name: install git in container
>> +      run: |
>> +        if command -v git
>> +        then
>> +          : # nothing to do
>> +        elif command -v apk
>> +        then
>> +          apk add --update git
>> +        elif command -v dnf
>> +        then
>> +          dnf -yq update && dnf -yq install git
>> +        else
>> +          apt-get -q update && apt-get -q -y install git
>> +        fi
>>       - uses: actions/checkout@v4
>>       - run: ci/install-dependencies.sh
>>       - run: useradd builder --create-home
>>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v5 2/6] coccinelle: meson: rename variables to be more specific

[Karthik Nayak]

In Meson, included subdirs export their variables to top level Meson
builds. In 'contrib/coccinelle/meson.build', we define two such
variables `sources` and `headers`. While these variables are specific to
the checks in the 'contrib/coccinelle/' directory, they also pollute the
top level 'meson.build'.

Rename them to be more specific, this ensures that they aren't
mistakenly used in the upper levels and avoid variable name collisions.

While here, change the empty list denotation to be consistent with other
places.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index ea054c924f..03ce52d752 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -55,18 +55,18 @@ concatenated_rules = custom_target(
   capture: true,
 )
 
-sources = [ ]
+coccinelle_sources = []
 foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.c', third_party_sources, check: true).stdout().split()
-  sources += source
+  coccinelle_sources += source
 endforeach
 
-headers = [ ]
+coccinelle_headers = []
 foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
-  headers += meson.project_source_root() / header
+  coccinelle_headers += meson.project_source_root() / header
 endforeach
 
 patches = [ ]
-foreach source : sources
+foreach source : coccinelle_sources
   patches += custom_target(
     command: [
       spatch,
@@ -78,7 +78,7 @@ foreach source : sources
     input: meson.project_source_root() / source,
     output: source.underscorify() + '.patch',
     capture: true,
-    depend_files: headers,
+    depend_files: coccinelle_headers,
   )
 endforeach
 

-- 
2.48.1

[PATCH v5 3/6] meson: move headers definition from 'contrib/coccinelle'

[Karthik Nayak]

The Meson build for coccinelle static analysis lists all headers to
analyse. Due to the way Meson exports variables between subdirs, this
variable is also available in the root Meson build.

An upcoming commit, will add a new check complimenting 'hdr-check' in
the Makefile. This would require the list of headers. So move the
'coccinelle_headers' to the root Meson build and rename it to 'headers',
remove the root path being appended to each header and retain that in
the coccinelle Meson build since it is specific to the coccinelle build.

Also move the 'third_party_sources' variable to the root Meson build
since it is also a dependency for the 'headers' variable. This also
makes it easier to understand as the variable is now propagated from the
top level to the bottom.

While 'headers_to_check' is only computed when we have a repository and
the 'git' executable is present, the variable itself is exposed as an
empty array. This allows dependencies in upcoming commits to simply
check for length of the array and not worry about dependencies required
to actually populate the array.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 17 +----------------
 meson.build                    | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index 03ce52d752..4f07824402 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -8,21 +8,6 @@ if not spatch.found()
   subdir_done()
 endif
 
-third_party_sources = [
-  ':!contrib',
-  ':!compat/inet_ntop.c',
-  ':!compat/inet_pton.c',
-  ':!compat/nedmalloc',
-  ':!compat/obstack.*',
-  ':!compat/poll',
-  ':!compat/regex',
-  ':!sha1collisiondetection',
-  ':!sha1dc',
-  ':!t/unit-tests/clar',
-  ':!t/unit-tests/clar',
-  ':!t/t[0-9][0-9][0-9][0-9]*',
-]
-
 rules = [
   'array.cocci',
   'commit.cocci',
@@ -61,7 +46,7 @@ foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files',
 endforeach
 
 coccinelle_headers = []
-foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
+foreach header : headers_to_check
   coccinelle_headers += meson.project_source_root() / header
 endforeach
 
diff --git a/meson.build b/meson.build
index e98cfa4909..e147ddff28 100644
--- a/meson.build
+++ b/meson.build
@@ -633,6 +633,28 @@ builtin_sources = [
   'builtin/write-tree.c',
 ]
 
+third_party_sources = [
+  ':!contrib',
+  ':!compat/inet_ntop.c',
+  ':!compat/inet_pton.c',
+  ':!compat/nedmalloc',
+  ':!compat/obstack.*',
+  ':!compat/poll',
+  ':!compat/regex',
+  ':!sha1collisiondetection',
+  ':!sha1dc',
+  ':!t/unit-tests/clar',
+  ':!t/unit-tests/clar',
+  ':!t/t[0-9][0-9][0-9][0-9]*',
+]
+
+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_sources, check: true).stdout().split()
+    headers_to_check += header
+  endforeach
+endif
+
 if not get_option('breaking_changes')
   builtin_sources += 'builtin/pack-redundant.c'
 endif

-- 
2.48.1

[PATCH v5 4/6] meson: rename 'third_party_sources' to 'third_party_excludes'

[Karthik Nayak]

The 'third_party_sources' variable was moved to the root 'meson.build'
file in the previous commit. The variable is actually used to exclude
third party sources, so rename it accordingly to 'third_party_excludes'
to avoid confusion. While here, remove a duplicate from the list.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 contrib/coccinelle/meson.build | 2 +-
 meson.build                    | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/contrib/coccinelle/meson.build b/contrib/coccinelle/meson.build
index 4f07824402..dc3f73c2e7 100644
--- a/contrib/coccinelle/meson.build
+++ b/contrib/coccinelle/meson.build
@@ -41,7 +41,7 @@ concatenated_rules = custom_target(
 )
 
 coccinelle_sources = []
-foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.c', third_party_sources, check: true).stdout().split()
+foreach source : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.c', third_party_excludes, check: true).stdout().split()
   coccinelle_sources += source
 endforeach
 
diff --git a/meson.build b/meson.build
index e147ddff28..4618804c7a 100644
--- a/meson.build
+++ b/meson.build
@@ -633,7 +633,7 @@ builtin_sources = [
   'builtin/write-tree.c',
 ]
 
-third_party_sources = [
+third_party_excludes = [
   ':!contrib',
   ':!compat/inet_ntop.c',
   ':!compat/inet_pton.c',
@@ -644,13 +644,12 @@ third_party_sources = [
   ':!sha1collisiondetection',
   ':!sha1dc',
   ':!t/unit-tests/clar',
-  ':!t/unit-tests/clar',
   ':!t/t[0-9][0-9][0-9][0-9]*',
 ]
 
 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_sources, check: true).stdout().split()
+  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
 endif

-- 
2.48.1

[PATCH v5 5/6] meson: add support for 'hdr-check'

[Karthik Nayak]

The Makefile supports a target called 'hdr-check', which checks if
individual header files can be independently compiled. Let's port this
functionality to Meson, our new build system too. The implementation
resembles that of the Makefile and provides the same check.

Since meson builds are out-of-tree, header dependencies are not
automatically met. So unlike the Makefile version, we also need to add
the required dependencies.

Also add the 'xdiff/' dir to the list of 'third_party_sources' as those
headers must be skipped from the checks too. This also skips the folder
from the 'coccinelle' checks, this is okay, since this code is an
external dependency.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 meson.build | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/meson.build b/meson.build
index 4618804c7a..22fc65ec80 100644
--- a/meson.build
+++ b/meson.build
@@ -645,6 +645,7 @@ third_party_excludes = [
   ':!sha1dc',
   ':!t/unit-tests/clar',
   ':!t/t[0-9][0-9][0-9][0-9]*',
+  ':!xdiff',
 ]
 
 headers_to_check = []
@@ -1994,6 +1995,68 @@ endif
 
 subdir('contrib')
 
+exclude_from_check_headers = [
+  'compat/',
+  'unicode-width.h',
+]
+
+if sha1_backend != 'openssl'
+  exclude_from_check_headers += 'sha1/openssl.h'
+endif
+if sha256_backend != 'openssl'
+  exclude_from_check_headers += 'sha256/openssl.h'
+endif
+if sha256_backend != 'nettle'
+  exclude_from_check_headers += 'sha256/nettle.h'
+endif
+if sha256_backend != 'gcrypt'
+  exclude_from_check_headers += 'sha256/gcrypt.h'
+endif
+
+if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
+  hco_targets = []
+  foreach h : headers_to_check
+    skip_header = false
+    foreach exclude : exclude_from_check_headers
+      if h.startswith(exclude)
+        skip_header = true
+        break
+      endif
+    endforeach
+
+    if skip_header
+      continue
+    endif
+
+    hcc = custom_target(
+      input: h,
+      output: h.underscorify() + 'cc',
+      command: [
+        shell,
+        '-c',
+        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@'
+      ]
+    )
+
+    hco = custom_target(
+      input: hcc,
+      output: fs.replace_suffix(h.underscorify(), '.hco'),
+      command: [
+        compiler.cmd_array(),
+        libgit_c_args,
+        '-I', meson.project_source_root(),
+        '-I', meson.project_source_root() / 't/unit-tests',
+        '-o', '/dev/null',
+        '-c', '-xc',
+        '@INPUT@'
+      ]
+    )
+    hco_targets += hco
+  endforeach
+
+  alias_target('hdr-check', hco_targets)
+endif
+
 foreach key, value : {
   'DIFF': diff.full_path(),
   'GIT_SOURCE_DIR': meson.project_source_root(),

-- 
2.48.1

Re: [PATCH v5 5/6] meson: add support for 'hdr-check'

[phillip.wood123]

Hi Karthik

On 23/04/2025 09:15, Karthik Nayak wrote:
> The Makefile supports a target called 'hdr-check', which checks if
> individual header files can be independently compiled. Let's port this
> functionality to Meson, our new build system too. The implementation
> resembles that of the Makefile and provides the same check.
> 
> Since meson builds are out-of-tree, header dependencies are not
> automatically met. So unlike the Makefile version, we also need to add
> the required dependencies.
> 
> Also add the 'xdiff/' dir to the list of 'third_party_sources' as those
> headers must be skipped from the checks too. This also skips the folder
> from the 'coccinelle' checks, this is okay, since this code is an
> external dependency.

The xdiff code is in a kind of limbo as the upstream project is dead and 
so people who want a standalone copy take the code from our repository. 
The Makefile does run coccinelle on it but not hdr-check as that fails. 
Looking at the filenames in contrib/coccinelle they are almost all quite 
git specific so it probably makes sense to skip the xdiff files.

Best Wishes

Phillip

> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>   meson.build | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 4618804c7a..22fc65ec80 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -645,6 +645,7 @@ third_party_excludes = [
>     ':!sha1dc',
>     ':!t/unit-tests/clar',
>     ':!t/t[0-9][0-9][0-9][0-9]*',
> +  ':!xdiff',
>   ]
>   
>   headers_to_check = []
> @@ -1994,6 +1995,68 @@ endif
>   
>   subdir('contrib')
>   
> +exclude_from_check_headers = [
> +  'compat/',
> +  'unicode-width.h',
> +]
> +
> +if sha1_backend != 'openssl'
> +  exclude_from_check_headers += 'sha1/openssl.h'
> +endif
> +if sha256_backend != 'openssl'
> +  exclude_from_check_headers += 'sha256/openssl.h'
> +endif
> +if sha256_backend != 'nettle'
> +  exclude_from_check_headers += 'sha256/nettle.h'
> +endif
> +if sha256_backend != 'gcrypt'
> +  exclude_from_check_headers += 'sha256/gcrypt.h'
> +endif
> +
> +if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
> +  hco_targets = []
> +  foreach h : headers_to_check
> +    skip_header = false
> +    foreach exclude : exclude_from_check_headers
> +      if h.startswith(exclude)
> +        skip_header = true
> +        break
> +      endif
> +    endforeach
> +
> +    if skip_header
> +      continue
> +    endif
> +
> +    hcc = custom_target(
> +      input: h,
> +      output: h.underscorify() + 'cc',
> +      command: [
> +        shell,
> +        '-c',
> +        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@'
> +      ]
> +    )
> +
> +    hco = custom_target(
> +      input: hcc,
> +      output: fs.replace_suffix(h.underscorify(), '.hco'),
> +      command: [
> +        compiler.cmd_array(),
> +        libgit_c_args,
> +        '-I', meson.project_source_root(),
> +        '-I', meson.project_source_root() / 't/unit-tests',
> +        '-o', '/dev/null',
> +        '-c', '-xc',
> +        '@INPUT@'
> +      ]
> +    )
> +    hco_targets += hco
> +  endforeach
> +
> +  alias_target('hdr-check', hco_targets)
> +endif
> +
>   foreach key, value : {
>     'DIFF': diff.full_path(),
>     'GIT_SOURCE_DIR': meson.project_source_root(),
> 

Re: [PATCH v5 5/6] meson: add support for 'hdr-check'

[Karthik Nayak]

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

phillip.wood123@gmail.com writes:

> Hi Karthik
>
> On 23/04/2025 09:15, Karthik Nayak wrote:
>> The Makefile supports a target called 'hdr-check', which checks if
>> individual header files can be independently compiled. Let's port this
>> functionality to Meson, our new build system too. The implementation
>> resembles that of the Makefile and provides the same check.
>>
>> Since meson builds are out-of-tree, header dependencies are not
>> automatically met. So unlike the Makefile version, we also need to add
>> the required dependencies.
>>
>> Also add the 'xdiff/' dir to the list of 'third_party_sources' as those
>> headers must be skipped from the checks too. This also skips the folder
>> from the 'coccinelle' checks, this is okay, since this code is an
>> external dependency.
>
> The xdiff code is in a kind of limbo as the upstream project is dead and
> so people who want a standalone copy take the code from our repository.
> The Makefile does run coccinelle on it but not hdr-check as that fails.
> Looking at the filenames in contrib/coccinelle they are almost all quite
> git specific so it probably makes sense to skip the xdiff files.
>

That was my conclusion indeed. I thought of also removing it from the
Makefile, but I think that it would be better done outside of this
series.

> Best Wishes
>
> Phillip
>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>   meson.build | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 63 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 4618804c7a..22fc65ec80 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -645,6 +645,7 @@ third_party_excludes = [
>>     ':!sha1dc',
>>     ':!t/unit-tests/clar',
>>     ':!t/t[0-9][0-9][0-9][0-9]*',
>> +  ':!xdiff',
>>   ]
>>
>>   headers_to_check = []
>> @@ -1994,6 +1995,68 @@ endif
>>
>>   subdir('contrib')
>>
>> +exclude_from_check_headers = [
>> +  'compat/',
>> +  'unicode-width.h',
>> +]
>> +
>> +if sha1_backend != 'openssl'
>> +  exclude_from_check_headers += 'sha1/openssl.h'
>> +endif
>> +if sha256_backend != 'openssl'
>> +  exclude_from_check_headers += 'sha256/openssl.h'
>> +endif
>> +if sha256_backend != 'nettle'
>> +  exclude_from_check_headers += 'sha256/nettle.h'
>> +endif
>> +if sha256_backend != 'gcrypt'
>> +  exclude_from_check_headers += 'sha256/gcrypt.h'
>> +endif
>> +
>> +if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>> +  hco_targets = []
>> +  foreach h : headers_to_check
>> +    skip_header = false
>> +    foreach exclude : exclude_from_check_headers
>> +      if h.startswith(exclude)
>> +        skip_header = true
>> +        break
>> +      endif
>> +    endforeach
>> +
>> +    if skip_header
>> +      continue
>> +    endif
>> +
>> +    hcc = custom_target(
>> +      input: h,
>> +      output: h.underscorify() + 'cc',
>> +      command: [
>> +        shell,
>> +        '-c',
>> +        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@'
>> +      ]
>> +    )
>> +
>> +    hco = custom_target(
>> +      input: hcc,
>> +      output: fs.replace_suffix(h.underscorify(), '.hco'),
>> +      command: [
>> +        compiler.cmd_array(),
>> +        libgit_c_args,
>> +        '-I', meson.project_source_root(),
>> +        '-I', meson.project_source_root() / 't/unit-tests',
>> +        '-o', '/dev/null',
>> +        '-c', '-xc',
>> +        '@INPUT@'
>> +      ]
>> +    )
>> +    hco_targets += hco
>> +  endforeach
>> +
>> +  alias_target('hdr-check', hco_targets)
>> +endif
>> +
>>   foreach key, value : {
>>     'DIFF': diff.full_path(),
>>     'GIT_SOURCE_DIR': meson.project_source_root(),
>>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v5 6/6] makefile/meson: add 'check-headers' as alias for 'hdr-check'

[Karthik Nayak]

The 'hdr-check' target in Meson and makefile is used to check if headers
can be compiled individually. The naming however isn't readable as 'hdr'
is not a common shortforme for 'header', neither is it an abbreviation.

Let's introduce 'check-headers' as an alternative target for 'hdr-check'
and add a `TODO` to deprecate the latter after 2 releases. Since this
is an internal tool, we can use a shorter deprecation cycle.

Change existing usage of 'hdr-check' in 'ci/run-static-analysis.sh' to
also use 'check-headers'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Makefile                  | 4 +++-
 ci/run-static-analysis.sh | 2 +-
 meson.build               | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index ac32d2d0bd..961ee508be 100644
--- a/Makefile
+++ b/Makefile
@@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc)
 $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE
 	$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<
 
-.PHONY: hdr-check $(HCO)
+# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
+.PHONY: hdr-check check-headers $(HCO)
 hdr-check: $(HCO)
+check-headers: hdr-check
 
 .PHONY: style
 style:
diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
index 0d51e5ce0e..60c175a094 100755
--- a/ci/run-static-analysis.sh
+++ b/ci/run-static-analysis.sh
@@ -26,7 +26,7 @@ then
 	exit 1
 fi
 
-make hdr-check ||
+make check-headers ||
 exit 1
 
 make check-pot
diff --git a/meson.build b/meson.build
index 22fc65ec80..569e3888fb 100644
--- a/meson.build
+++ b/meson.build
@@ -2054,7 +2054,9 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
     hco_targets += hco
   endforeach
 
-  alias_target('hdr-check', hco_targets)
+  # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+
+  hdr_check = alias_target('hdr-check', hco_targets)
+  alias_target('check-headers', hdr_check)
 endif
 
 foreach key, value : {

-- 
2.48.1

Re: [PATCH v5 6/6] makefile/meson: add 'check-headers' as alias for 'hdr-check'

[Patrick Steinhardt]

On Wed, Apr 23, 2025 at 10:15:39AM +0200, Karthik Nayak wrote:
> The 'hdr-check' target in Meson and makefile is used to check if headers
> can be compiled individually. The naming however isn't readable as 'hdr'
> is not a common shortforme for 'header', neither is it an abbreviation.
> 
> Let's introduce 'check-headers' as an alternative target for 'hdr-check'
> and add a `TODO` to deprecate the latter after 2 releases. Since this
> is an internal tool, we can use a shorter deprecation cycle.
> 
> Change existing usage of 'hdr-check' in 'ci/run-static-analysis.sh' to
> also use 'check-headers'.

I wondered whether we also want to rename `coccicheck` to
`check-coccinelle` to match. But even if the answer would be "yes" I
don't think that this change would need to be part of this patch series.

Thanks!

Patrick

Re: [PATCH v5 6/6] makefile/meson: add 'check-headers' as alias for 'hdr-check'

[Karthik Nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Apr 23, 2025 at 10:15:39AM +0200, Karthik Nayak wrote:
>> The 'hdr-check' target in Meson and makefile is used to check if headers
>> can be compiled individually. The naming however isn't readable as 'hdr'
>> is not a common shortforme for 'header', neither is it an abbreviation.
>>
>> Let's introduce 'check-headers' as an alternative target for 'hdr-check'
>> and add a `TODO` to deprecate the latter after 2 releases. Since this
>> is an internal tool, we can use a shorter deprecation cycle.
>>
>> Change existing usage of 'hdr-check' in 'ci/run-static-analysis.sh' to
>> also use 'check-headers'.
>
> I wondered whether we also want to rename `coccicheck` to
> `check-coccinelle` to match. But even if the answer would be "yes" I
> don't think that this change would need to be part of this patch series.
>

I would say 'yes' to both those your statements!

> Thanks!
>
> Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check

[phillip.wood123]

Hi Karthik

This looks good, I've left a couple of comments but I don't think there 
is anything that necessitates a re-roll.

Thanks

Phillip

On 23/04/2025 09:15, Karthik Nayak wrote:
> To bridge the remaining gaps between Makefile and Meson, this patch
> series adds 'hdr-check' to Meson to compliment the Makefile's
> 'hdr-check'.
> 
> We also introduce 'headers-check' as an alias to 'hdr-check' as a better
> named replacement in both Meson and make and add a note to deprecate
> 'hdr-check' in the future.
> 
> The first two commits are small cleanups, where we re-organize existing
> variables to make it easier to add the target. The third commit adds the
> 'hdr-check' target to Meson. The last commit introduces the
> 'headers-check' alias to both Meson and the makefile and marks
> 'hdr-check' to be deprecated.
> 
> This is based on master 9d22ac5122 (The third batch, 2025-04-07) with
> 'es/meson-build-skip-coccinelle' merged in.
> 
> ---
> Changes in v5:
> - Add a commit to install 'git' in GitHub's CI before the repository is
>    checked out. Without the presence of the 'git' executable, GitHub
>    downloads a tar of the repo instead of cloning it. This causes the
>    patch series to fail in CI.
> - Expose the 'headers_to_check' variable even if 'git' executable is not
>    found or not a repository. This ensures dependencies of
>    'headers_to_check' can simply rely on its length. We also check that
>    the '.git' folder is setup before populating 'headers_to_check', this
>    ensures Meson doesn't fail in tarball of the Git source code.
> - Link to v4: https://lore.kernel.org/r/20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822@gmail.com
> 
> Changes in v4:
> - Rename headers to headers_to_check, since these headers are only used
>    for static analysis.
> - Added a commit to rename third_party_sources -> third_party_excludes
>    and remove a duplicate.
> - Fix a typo 'gcrpyt' -> 'gcrypt'
> - Remove 'generated_headers', since we use 'git ls-files' and that would
>    already ignore files within '.gitignore'.
> - Link to v3: https://lore.kernel.org/r/20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745@gmail.com
> 
> Changes in v3:
> - Some renames:
>    - headers_generated -> generated_headers
>    - meson -> Meson
>    - headers-check -> check-headers
>    - headers_check_exclude -> exclude_from_check_headers
> - Rewrite 'headers_check_exclude' to also contain dirs so we can skip
>    listing individual header files.
> - Move 'xdiff/*' to 'third_party_sources' and cleanup
>    'exclude_from_check_headers'.
> - Use 'echo' instead of 'echo -n'.
> - Use `fs.replace_suffix` instead of `str.replace`.
> - Link to v2: https://lore.kernel.org/r/20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com
> 
> Changes in v2:
> - Add 'hdr-check' to meson, while introducing 'headers-check' as
>    a replacement alias. Schedule 'hdr-check' to be deprecated in the future.
> - Link to v1: https://lore.kernel.org/r/20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com
> 
> ---
>   .github/workflows/main.yml     | 14 +++++++
>   Makefile                       |  4 +-
>   ci/run-static-analysis.sh      |  2 +-
>   contrib/coccinelle/meson.build | 31 ++++-----------
>   meson.build                    | 86 ++++++++++++++++++++++++++++++++++++++++++
>   5 files changed, 112 insertions(+), 25 deletions(-)
> 
> Karthik Nayak (6):
>        ci/github: install git before checking out the repository
>        coccinelle: meson: rename variables to be more specific
>        meson: move headers definition from 'contrib/coccinelle'
>        meson: rename 'third_party_sources' to 'third_party_excludes'
>        meson: add support for 'hdr-check'
>        makefile/meson: add 'check-headers' as alias for 'hdr-check'
> 
> Range-diff versus v4:
> 
> -:  ---------- > 1:  d20993a59e ci/github: install git before checking out the repository
> 1:  0f1160fa78 = 2:  6eb10dd3b1 coccinelle: meson: rename variables to be more specific
> 2:  933b199def ! 3:  7950b44c6f meson: move headers definition from 'contrib/coccinelle'
>      @@ Commit message
>           makes it easier to understand as the variable is now propagated from the
>           top level to the bottom.
>       
>      +    While 'headers_to_check' is only computed when we have a repository and
>      +    the 'git' executable is present, the variable itself is exposed as an
>      +    empty array. This allows dependencies in upcoming commits to simply
>      +    check for length of the array and not worry about dependencies required
>      +    to actually populate the array.
>      +
>           Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>       
>        ## contrib/coccinelle/meson.build ##
>      @@ meson.build: builtin_sources = [
>       +  ':!t/t[0-9][0-9][0-9][0-9]*',
>       +]
>       +
>      -+if git.found()
>      -+  headers_to_check = []
>      ++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_sources, check: true).stdout().split()
>       +    headers_to_check += header
>       +  endforeach
> 3:  ab04192864 ! 4:  7fd64b788b meson: rename 'third_party_sources' to 'third_party_excludes'
>      @@ meson.build: third_party_sources = [
>          ':!t/t[0-9][0-9][0-9][0-9]*',
>        ]
>        
>      - if git.found()
>      -   headers_to_check = []
>      + 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_sources, check: true).stdout().split()
>       +  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
> 4:  69a4b5abc9 ! 5:  df0472be57 meson: add support for 'hdr-check'
>      @@ meson.build: third_party_excludes = [
>       +  ':!xdiff',
>        ]
>        
>      - if git.found()
>      + headers_to_check = []
>       @@ meson.build: endif
>        
>        subdir('contrib')
>      @@ meson.build: endif
>       +  exclude_from_check_headers += 'sha256/gcrypt.h'
>       +endif
>       +
>      -+if git.found() and compiler.get_argument_syntax() == 'gcc'
>      ++if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>       +  hco_targets = []
>       +  foreach h : headers_to_check
>       +    skip_header = false
> 5:  a0c799cf8c ! 6:  7d9a33767b makefile/meson: add 'check-headers' as alias for 'hdr-check'
>      @@ ci/run-static-analysis.sh: then
>        make check-pot
>       
>        ## meson.build ##
>      -@@ meson.build: if git.found() and compiler.get_argument_syntax() == 'gcc'
>      +@@ meson.build: if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>            hco_targets += hco
>          endforeach
>        
> 
> 
> base-commit: 3a956c5f69873611ae5f8dcb9acd117f66b95ddc
> change-id: 20250330-505-wire-up-sparse-via-meson-2e32dd31208b
> 
> Thanks
> - Karthik
> 

Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check

[Junio C Hamano]

phillip.wood123@gmail.com writes:

> Hi Karthik
>
> This looks good, I've left a couple of comments but I don't think
> there is anything that necessitates a re-roll.
>
> Thanks
>
> Phillip

Thanks, I think the first one that stops us from using tarball
extract may have the biggest impact on the CI, and might reveal
some other bugs (like "this test used to be skipped because it did
not run in a tarball extract, but now this is run and fails"), which
may cause us to scramble to fix them, but I think that would be a
good thing in the longer term.

WIll queue.

Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> phillip.wood123@gmail.com writes:
>
>> Hi Karthik
>>
>> This looks good, I've left a couple of comments but I don't think
>> there is anything that necessitates a re-roll.
>>
>> Thanks
>>
>> Phillip
>
> Thanks, I think the first one that stops us from using tarball
> extract may have the biggest impact on the CI, and might reveal
> some other bugs (like "this test used to be skipped because it did
> not run in a tarball extract, but now this is run and fails"), which
> may cause us to scramble to fix them, but I think that would be a
> good thing in the longer term.

As it takes quite a lot of time to do full integration of the day,
during which time GitHub CI is idle, I pushed this branch alone as
if it were the tip of 'seen', and it seems that quite a lot of CI
jobs are now broken,

https://github.com/git/git/actions/runs/14624509129/

with "Process completed with exit code 8." at the end of
ci/install-dependencies.sh step.

Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> As it takes quite a lot of time to do full integration of the day,
> during which time GitHub CI is idle, I pushed this branch alone as
> if it were the tip of 'seen', and it seems that quite a lot of CI
> jobs are now broken,
>
> https://github.com/git/git/actions/runs/14624509129/
>
> with "Process completed with exit code 8." at the end of
> ci/install-dependencies.sh step.

Yuck.  It's JGit download that is failing.

Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> As it takes quite a lot of time to do full integration of the day,
>> during which time GitHub CI is idle, I pushed this branch alone as
>> if it were the tip of 'seen', and it seems that quite a lot of CI
>> jobs are now broken,
>>
>> https://github.com/git/git/actions/runs/14624509129/
>>
>> with "Process completed with exit code 8." at the end of
>> ci/install-dependencies.sh step.
>
> Yuck.  It's JGit download that is failing.

Sigh! I did test on GitHub [1], before pushing the patches to the
mailing list, so I was really sad to see your first mail. Now I'm not
sure if I have good luck or bad luck!

[1]: https://github.com/gitgitgadget/git/actions/runs/14604710114/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> As it takes quite a lot of time to do full integration of the day,
>>> during which time GitHub CI is idle, I pushed this branch alone as
>>> if it were the tip of 'seen', and it seems that quite a lot of CI
>>> jobs are now broken,
>>>
>>> https://github.com/git/git/actions/runs/14624509129/
>>>
>>> with "Process completed with exit code 8." at the end of
>>> ci/install-dependencies.sh step.
>>
>> Yuck.  It's JGit download that is failing.
>
> Sigh! I did test on GitHub [1], before pushing the patches to the
> mailing list, so I was really sad to see your first mail. Now I'm not
> sure if I have good luck or bad luck!
>
> [1]: https://github.com/gitgitgadget/git/actions/runs/14604710114/

Yeah, sorry for a false alarm.  

With jgit download temporarily disabled, and with all the recent CI
fixes (like Ubuntu 20.04 updated to newer version, explicitly
installing gawk to fedora environment), most of the tests are
passing again, except for linux-musl-meson:

  https://github.com/git/git/actions/runs/14627488066

I would expect win+Meson test (2) would time out again; I do not
know what is wrong with Windows and have no time to dig further
there.

The tree getting tested in the above run is 'master' with these 6
patches merged in, nothing else.

Thanks.

Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> As it takes quite a lot of time to do full integration of the day,
>>>> during which time GitHub CI is idle, I pushed this branch alone as
>>>> if it were the tip of 'seen', and it seems that quite a lot of CI
>>>> jobs are now broken,
>>>>
>>>> https://github.com/git/git/actions/runs/14624509129/
>>>>
>>>> with "Process completed with exit code 8." at the end of
>>>> ci/install-dependencies.sh step.
>>>
>>> Yuck.  It's JGit download that is failing.
>>
>> Sigh! I did test on GitHub [1], before pushing the patches to the
>> mailing list, so I was really sad to see your first mail. Now I'm not
>> sure if I have good luck or bad luck!
>>
>> [1]: https://github.com/gitgitgadget/git/actions/runs/14604710114/
>
> Yeah, sorry for a false alarm.
>

It's all good, CI reports should _mostly_ be trustable in such regards.
jGit downloads failing is not something to expect.

> With jgit download temporarily disabled, and with all the recent CI
> fixes (like Ubuntu 20.04 updated to newer version, explicitly
> installing gawk to fedora environment), most of the tests are
> passing again, except for linux-musl-meson:
>
>   https://github.com/git/git/actions/runs/14627488066
>

I'm really not sure why this is failing, I see that one of the tests
failed

  2025-04-23T20:37:25.0141268Z  764/1021 t7003-filter-branch
                 FAIL            9.00s   exit status 1

and I also see

  2025-04-23T20:37:25.0161574Z ./t/aggregate-results.sh: line 13:
can't open t/test-results/t*-*.counts: no such file

doesn't seem related to my patches, but again, not sure why this failed,
so possibly could be?

> I would expect win+Meson test (2) would time out again; I do not
> know what is wrong with Windows and have no time to dig further
> there.
>
> The tree getting tested in the above run is 'master' with these 6
> patches merged in, nothing else.
>

Thanks will try and replicate this on gitgitgadget:

  https://github.com/gitgitgadget/git/actions/runs/14629042372

> Thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check

[Karthik Nayak]

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

phillip.wood123@gmail.com writes:

> Hi Karthik
>
> This looks good, I've left a couple of comments but I don't think there
> is anything that necessitates a re-roll.
>

Hello,

Thanks for your review and guidance, appreciate it. I agree, will hold
off on re-rolling. Let's see if there are other comments :)

> Thanks
>
> Phillip
>

Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]