# [PATCH 0/3] meson: add corresponding target for Makefile's hdr-check 69 messages from 2025-04-08 to 2025-04-23. Participants: Karthik Nayak, Junio C Hamano, Phillip Wood, Patrick Steinhardt, Toon Claes, Eli Schwartz, phillip.wood123@gmail.com. Thread: https://gitlist.dev/t/63271 ## Karthik Nayak, 2025-04-08 14:55 Subject: [PATCH 0/3] meson: add corresponding target for Makefile's hdr-check Message-ID: <20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com> URL: https://gitlist.dev/e/20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f%40gmail.com ``` 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 ``` ## Karthik Nayak, 2025-04-08 14:55 Subject: [PATCH 1/3] coccinelle: meson: rename variables to be more specific Message-ID: <20250408-505-wire-up-sparse-via-meson-v1-1-17476e5cea3f@gmail.com> URL: https://gitlist.dev/e/20250408-505-wire-up-sparse-via-meson-v1-1-17476e5cea3f%40gmail.com In-Reply-To: <20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-08 14:55 Subject: [PATCH 2/3] meson: move headers definition from 'contrib/coccinelle' Message-ID: <20250408-505-wire-up-sparse-via-meson-v1-2-17476e5cea3f@gmail.com> URL: https://gitlist.dev/e/20250408-505-wire-up-sparse-via-meson-v1-2-17476e5cea3f%40gmail.com In-Reply-To: <20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-08 14:55 Subject: [PATCH 3/3] meson: add support for 'headers-check' Message-ID: <20250408-505-wire-up-sparse-via-meson-v1-3-17476e5cea3f@gmail.com> URL: https://gitlist.dev/e/20250408-505-wire-up-sparse-via-meson-v1-3-17476e5cea3f%40gmail.com In-Reply-To: <20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com> ``` 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 --- 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 ``` ## Junio C Hamano, 2025-04-08 22:19 Subject: Re: [PATCH 3/3] meson: add support for 'headers-check' Message-ID: URL: https://gitlist.dev/e/xmqqsemiuwhx.fsf%40gitster.g In-Reply-To: <20250408-505-wire-up-sparse-via-meson-v1-3-17476e5cea3f@gmail.com> ``` Karthik Nayak 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. ``` ## Karthik Nayak, 2025-04-10 09:13 Subject: Re: [PATCH 3/3] meson: add support for 'headers-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZTRAQJjYU2cvxOw%3DtmCkQH7CLm027B5ZkGSg10qdGQENg%40mail.gmail.com In-Reply-To: ``` Junio C Hamano writes: > Karthik Nayak 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 ` to `meson compile `. So they wouldn't mind 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. ``` ## Karthik Nayak, 2025-04-10 11:30 Subject: [PATCH v2 0/4] meson: add corresponding target for Makefile's hdr-check Message-ID: <20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com> URL: https://gitlist.dev/e/20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5%40gmail.com In-Reply-To: <20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com> ``` 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 ## 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 @@ 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 ``` ## Karthik Nayak, 2025-04-10 11:30 Subject: [PATCH v2 1/4] coccinelle: meson: rename variables to be more specific Message-ID: <20250410-505-wire-up-sparse-via-meson-v2-1-acb45cc8a2e5@gmail.com> URL: https://gitlist.dev/e/20250410-505-wire-up-sparse-via-meson-v2-1-acb45cc8a2e5%40gmail.com In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-10 11:30 Subject: [PATCH v2 2/4] meson: move headers definition from 'contrib/coccinelle' Message-ID: <20250410-505-wire-up-sparse-via-meson-v2-2-acb45cc8a2e5@gmail.com> URL: https://gitlist.dev/e/20250410-505-wire-up-sparse-via-meson-v2-2-acb45cc8a2e5%40gmail.com In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-10 11:30 Subject: [PATCH v2 3/4] meson: add support for 'hdr-check' Message-ID: <20250410-505-wire-up-sparse-via-meson-v2-3-acb45cc8a2e5@gmail.com> URL: https://gitlist.dev/e/20250410-505-wire-up-sparse-via-meson-v2-3-acb45cc8a2e5%40gmail.com In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-10 11:30 Subject: [PATCH v2 4/4] makefile/meson: add 'headers-check' as alias for 'hdr-check' Message-ID: <20250410-505-wire-up-sparse-via-meson-v2-4-acb45cc8a2e5@gmail.com> URL: https://gitlist.dev/e/20250410-505-wire-up-sparse-via-meson-v2-4-acb45cc8a2e5%40gmail.com In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com> ``` 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 --- 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 ``` ## Phillip Wood, 2025-04-10 14:50 Subject: Re: [PATCH v2 3/4] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/dc0d6acb-bcf1-4a7e-bfc8-7c2e7793c72b%40gmail.com In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-3-acb45cc8a2e5@gmail.com> ``` 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 > --- > 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(), > ``` ## Phillip Wood, 2025-04-10 14:50 Subject: Re: [PATCH v2 4/4] makefile/meson: add 'headers-check' as alias for 'hdr-check' Message-ID: <7eaa33ca-0ccc-4789-94b5-de1a068e1599@gmail.com> URL: https://gitlist.dev/e/7eaa33ca-0ccc-4789-94b5-de1a068e1599%40gmail.com In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-4-acb45cc8a2e5@gmail.com> ``` 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 > --- > 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 : { > ``` ## Junio C Hamano, 2025-04-10 18:58 Subject: Re: [PATCH v2 4/4] makefile/meson: add 'headers-check' as alias for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/xmqqtt6voncw.fsf%40gitster.g In-Reply-To: <7eaa33ca-0ccc-4789-94b5-de1a068e1599@gmail.com> ``` Phillip Wood 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. ``` ## Junio C Hamano, 2025-04-10 19:06 Subject: Re: [PATCH v2 3/4] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/xmqqplhjomy6.fsf%40gitster.g In-Reply-To: ``` Phillip Wood 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 >> --- >> 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. ``` ## Patrick Steinhardt, 2025-04-11 10:06 Subject: Re: [PATCH v2 2/4] meson: move headers definition from 'contrib/coccinelle' Message-ID: URL: https://gitlist.dev/e/Z_jptp_xj2gpJ65s%40pks.im In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-2-acb45cc8a2e5@gmail.com> ``` 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 ``` ## Patrick Steinhardt, 2025-04-11 10:06 Subject: Re: [PATCH v2 3/4] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/Z_jpvghOGHvw2Y4x%40pks.im In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-3-acb45cc8a2e5@gmail.com> ``` 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 > --- > 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 ``` ## Karthik Nayak, 2025-04-11 12:13 Subject: Re: [PATCH v2 2/4] meson: move headers definition from 'contrib/coccinelle' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZQvM%2BPPJMMmQ2XQrK5Z%3Dj178rJVEY01X0u6ozfgPLhyFQ%40mail.gmail.com In-Reply-To: ``` Patrick Steinhardt 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 ``` ## Toon Claes, 2025-04-14 08:45 Subject: Re: [PATCH v2 3/4] meson: add support for 'hdr-check' Message-ID: <87mscj5dxe.fsf@iotcl.com> URL: https://gitlist.dev/e/87mscj5dxe.fsf%40iotcl.com In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-3-acb45cc8a2e5@gmail.com> ``` Karthik Nayak 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 > --- > 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 ``` ## Karthik Nayak, 2025-04-14 13:49 Subject: Re: [PATCH v2 3/4] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZQO-7CmeKh13%2BvxgYnv1dpKvLQnVPf6R3dkujLf%2BHoMEQ%40mail.gmail.com In-Reply-To: ``` Phillip Wood 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 >> --- >> 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 ``` ## Karthik Nayak, 2025-04-14 14:03 Subject: Re: [PATCH v2 3/4] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZRy3eCd%2BjGjxVGgiYn2xAP0ugJEfzb0zcGt-Wkv4fNbgQ%40mail.gmail.com In-Reply-To: ``` Patrick Steinhardt 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 >> --- >> 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 ``` ## Karthik Nayak, 2025-04-14 14:25 Subject: Re: [PATCH v2 3/4] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZQTwqyAU2cSFCQL4Ofc%2BdVyT5h6x5Ojyi%3DBo6uYqze3Vg%40mail.gmail.com In-Reply-To: <87mscj5dxe.fsf@iotcl.com> ``` Toon Claes writes: > Karthik Nayak 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 >> --- >> 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 ``` ## Karthik Nayak, 2025-04-14 14:27 Subject: Re: [PATCH v2 4/4] makefile/meson: add 'headers-check' as alias for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZQNrqf%2BcqatpbW7jeX6e7XAmk7NZZBH1inYMCjFj23C-Q%40mail.gmail.com In-Reply-To: ``` Junio C Hamano writes: > Phillip Wood 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 ``` ## Karthik Nayak, 2025-04-20 12:21 Subject: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: <20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822@gmail.com> URL: https://gitlist.dev/e/20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822%40gmail.com In-Reply-To: <20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com> ``` 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 ## 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 ``` ## Karthik Nayak, 2025-04-20 12:21 Subject: [PATCH v4 1/5] coccinelle: meson: rename variables to be more specific Message-ID: <20250420-505-wire-up-sparse-via-meson-v4-1-66e14134e822@gmail.com> URL: https://gitlist.dev/e/20250420-505-wire-up-sparse-via-meson-v4-1-66e14134e822%40gmail.com In-Reply-To: <20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-20 12:21 Subject: [PATCH v4 2/5] meson: move headers definition from 'contrib/coccinelle' Message-ID: <20250420-505-wire-up-sparse-via-meson-v4-2-66e14134e822@gmail.com> URL: https://gitlist.dev/e/20250420-505-wire-up-sparse-via-meson-v4-2-66e14134e822%40gmail.com In-Reply-To: <20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-20 12:21 Subject: [PATCH v4 3/5] meson: rename 'third_party_sources' to 'third_party_excludes' Message-ID: <20250420-505-wire-up-sparse-via-meson-v4-3-66e14134e822@gmail.com> URL: https://gitlist.dev/e/20250420-505-wire-up-sparse-via-meson-v4-3-66e14134e822%40gmail.com In-Reply-To: <20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-20 12:21 Subject: [PATCH v4 5/5] makefile/meson: add 'check-headers' as alias for 'hdr-check' Message-ID: <20250420-505-wire-up-sparse-via-meson-v4-5-66e14134e822@gmail.com> URL: https://gitlist.dev/e/20250420-505-wire-up-sparse-via-meson-v4-5-66e14134e822%40gmail.com In-Reply-To: <20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-20 12:21 Subject: [PATCH v4 4/5] meson: add support for 'hdr-check' Message-ID: <20250420-505-wire-up-sparse-via-meson-v4-4-66e14134e822@gmail.com> URL: https://gitlist.dev/e/20250420-505-wire-up-sparse-via-meson-v4-4-66e14134e822%40gmail.com In-Reply-To: <20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822@gmail.com> ``` 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 --- 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 ``` ## Junio C Hamano, 2025-04-21 07:48 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqqh62i6jli.fsf%40gitster.g In-Reply-To: <20250420-505-wire-up-sparse-via-meson-v4-0-66e14134e822@gmail.com> ``` Karthik Nayak 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. ``` ## Phillip Wood, 2025-04-21 08:45 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: <8b380da4-8d27-4efe-85fd-3bb599188fe9@gmail.com> URL: https://gitlist.dev/e/8b380da4-8d27-4efe-85fd-3bb599188fe9%40gmail.com In-Reply-To: ``` On 21/04/2025 08:48, Junio C Hamano wrote: > Karthik Nayak 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 ``` ## Karthik Nayak, 2025-04-21 15:33 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZSR%3D7TEWLHa-wzBB4x%2B4%2B-BH3UC3G7s24Bc26JH63QKOA%40mail.gmail.com In-Reply-To: <8b380da4-8d27-4efe-85fd-3bb599188fe9@gmail.com> ``` Phillip Wood writes: > On 21/04/2025 08:48, Junio C Hamano wrote: >> Karthik Nayak 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 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 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 ``` ## Junio C Hamano, 2025-04-21 15:41 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqq8qnt7c9w.fsf%40gitster.g In-Reply-To: <8b380da4-8d27-4efe-85fd-3bb599188fe9@gmail.com> ``` Phillip Wood 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. ``` ## Phillip Wood, 2025-04-21 18:54 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: <3389d086-4e6e-4896-94dd-9f62d7c4f2df@gmail.com> URL: https://gitlist.dev/e/3389d086-4e6e-4896-94dd-9f62d7c4f2df%40gmail.com In-Reply-To: ``` On 21/04/2025 16:41, Junio C Hamano wrote: > Phillip Wood 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 ``` ## Karthik Nayak, 2025-04-21 20:08 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZSa-qQzi3iWPF%2BM5a4EsvGiQFX%3D2Ca%3DvzuqwSLWWXSw%2Bg%40mail.gmail.com In-Reply-To: ``` Junio C Hamano writes: > Phillip Wood 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. ``` ## Junio C Hamano, 2025-04-21 20:40 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqqplh55ju3.fsf%40gitster.g In-Reply-To: <3389d086-4e6e-4896-94dd-9f62d7c4f2df@gmail.com> ``` Phillip Wood writes: > On 21/04/2025 16:41, Junio C Hamano wrote: >> Phillip Wood 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". ``` ## Junio C Hamano, 2025-04-21 23:33 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqqldrt5bto.fsf%40gitster.g In-Reply-To: ``` Karthik Nayak 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. ``` ## Patrick Steinhardt, 2025-04-22 06:57 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/aAc92C53K2y-ijxA%40pks.im In-Reply-To: <3389d086-4e6e-4896-94dd-9f62d7c4f2df@gmail.com> ``` 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 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 ``` ## Karthik Nayak, 2025-04-22 09:11 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZSSdp4KBHAviudJm%3DH%2BbP0aqru%3DLN2%3D4hsYsqyM7zLMgw%40mail.gmail.com In-Reply-To: ``` Junio C Hamano writes: > Karthik Nayak 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. ``` ## Phillip Wood, 2025-04-22 13:24 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: <17c2eb4f-e291-4189-9846-0f42bdead01d@gmail.com> URL: https://gitlist.dev/e/17c2eb4f-e291-4189-9846-0f42bdead01d%40gmail.com In-Reply-To: ``` 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 > > 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 ``` ## Karthik Nayak, 2025-04-22 14:10 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZRmwnjytxoqG0fJWXFjaZcGQ9g%3DPFmx9N5Thuq64C24GQ%40mail.gmail.com In-Reply-To: <17c2eb4f-e291-4189-9846-0f42bdead01d@gmail.com> ``` Phillip Wood 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 >> >> 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 ``` ## Junio C Hamano, 2025-04-22 15:55 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqqr01k42cx.fsf%40gitster.g In-Reply-To: <17c2eb4f-e291-4189-9846-0f42bdead01d@gmail.com> ``` Phillip Wood 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. ``` ## Junio C Hamano, 2025-04-22 15:56 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqqmsc842bp.fsf%40gitster.g In-Reply-To: ``` Karthik Nayak 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. ``` ## Karthik Nayak, 2025-04-22 18:56 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZSi5FuhpaAiGn5OydLUXhkWC9VrkOvXKwTeP%3D9S2uhKgQ%40mail.gmail.com In-Reply-To: <17c2eb4f-e291-4189-9846-0f42bdead01d@gmail.com> ``` Phillip Wood 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 >> >> 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 ``` ## Eli Schwartz, 2025-04-23 01:17 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/ddfbd943-73e0-4ca2-a458-26b05750ed9f%40gentoo.org In-Reply-To: ``` On 4/22/25 11:55 AM, Junio C Hamano wrote: > Phillip Wood 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 ``` ## Karthik Nayak, 2025-04-23 08:15 Subject: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check Message-ID: <20250423-505-wire-up-sparse-via-meson-v5-0-d1e2be4b2078@gmail.com> URL: https://gitlist.dev/e/20250423-505-wire-up-sparse-via-meson-v5-0-d1e2be4b2078%40gmail.com In-Reply-To: <20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com> ``` 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 ## 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 ``` ## Karthik Nayak, 2025-04-23 08:15 Subject: [PATCH v5 1/6] ci/github: install git before checking out the repository Message-ID: <20250423-505-wire-up-sparse-via-meson-v5-1-d1e2be4b2078@gmail.com> URL: https://gitlist.dev/e/20250423-505-wire-up-sparse-via-meson-v5-1-d1e2be4b2078%40gmail.com In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-0-d1e2be4b2078@gmail.com> ``` 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 Signed-off-by: Karthik Nayak --- .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 ``` ## Karthik Nayak, 2025-04-23 08:15 Subject: [PATCH v5 2/6] coccinelle: meson: rename variables to be more specific Message-ID: <20250423-505-wire-up-sparse-via-meson-v5-2-d1e2be4b2078@gmail.com> URL: https://gitlist.dev/e/20250423-505-wire-up-sparse-via-meson-v5-2-d1e2be4b2078%40gmail.com In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-0-d1e2be4b2078@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-23 08:15 Subject: [PATCH v5 3/6] meson: move headers definition from 'contrib/coccinelle' Message-ID: <20250423-505-wire-up-sparse-via-meson-v5-3-d1e2be4b2078@gmail.com> URL: https://gitlist.dev/e/20250423-505-wire-up-sparse-via-meson-v5-3-d1e2be4b2078%40gmail.com In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-0-d1e2be4b2078@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-23 08:15 Subject: [PATCH v5 4/6] meson: rename 'third_party_sources' to 'third_party_excludes' Message-ID: <20250423-505-wire-up-sparse-via-meson-v5-4-d1e2be4b2078@gmail.com> URL: https://gitlist.dev/e/20250423-505-wire-up-sparse-via-meson-v5-4-d1e2be4b2078%40gmail.com In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-0-d1e2be4b2078@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-23 08:15 Subject: [PATCH v5 5/6] meson: add support for 'hdr-check' Message-ID: <20250423-505-wire-up-sparse-via-meson-v5-5-d1e2be4b2078@gmail.com> URL: https://gitlist.dev/e/20250423-505-wire-up-sparse-via-meson-v5-5-d1e2be4b2078%40gmail.com In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-0-d1e2be4b2078@gmail.com> ``` 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 --- 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 ``` ## Karthik Nayak, 2025-04-23 08:15 Subject: [PATCH v5 6/6] makefile/meson: add 'check-headers' as alias for 'hdr-check' Message-ID: <20250423-505-wire-up-sparse-via-meson-v5-6-d1e2be4b2078@gmail.com> URL: https://gitlist.dev/e/20250423-505-wire-up-sparse-via-meson-v5-6-d1e2be4b2078%40gmail.com In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-0-d1e2be4b2078@gmail.com> ``` 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 --- 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 ``` ## Phillip Wood, 2025-04-23 10:04 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: <1996b081-2684-49e0-a99d-28f22063b4b6@gmail.com> URL: https://gitlist.dev/e/1996b081-2684-49e0-a99d-28f22063b4b6%40gmail.com In-Reply-To: ``` On 21/04/2025 21:40, Junio C Hamano wrote: > Phillip Wood 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". > ``` ## Phillip Wood, 2025-04-23 10:04 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: <895a796e-86ab-4b9f-b480-ca749312601f@gmail.com> URL: https://gitlist.dev/e/895a796e-86ab-4b9f-b480-ca749312601f%40gmail.com In-Reply-To: ``` On 22/04/2025 16:55, Junio C Hamano wrote: > Phillip Wood 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. ``` ## phillip.wood123@gmail.com, 2025-04-23 10:05 Subject: Re: [PATCH v5 1/6] ci/github: install git before checking out the repository Message-ID: <3607a937-9795-4348-981e-dce92203c6d7@gmail.com> URL: https://gitlist.dev/e/3607a937-9795-4348-981e-dce92203c6d7%40gmail.com In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-1-d1e2be4b2078@gmail.com> ``` 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 > Signed-off-by: Karthik Nayak > --- > .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 > ``` ## Phillip Wood, 2025-04-23 10:05 Subject: Re: [PATCH v4 0/5] meson: add corresponding target for Makefile's hdr-check Message-ID: <970e77bb-5e5d-44f5-abbc-201d9e673a89@gmail.com> URL: https://gitlist.dev/e/970e77bb-5e5d-44f5-abbc-201d9e673a89%40gmail.com In-Reply-To: ``` Hi Karthik On 22/04/2025 19:56, Karthik Nayak wrote: > Phillip Wood 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 ``` ## phillip.wood123@gmail.com, 2025-04-23 10:05 Subject: Re: [PATCH v5 5/6] meson: add support for 'hdr-check' Message-ID: <2a25ca04-c824-4bec-a5e7-95ec32556cf9@gmail.com> URL: https://gitlist.dev/e/2a25ca04-c824-4bec-a5e7-95ec32556cf9%40gmail.com In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-5-d1e2be4b2078@gmail.com> ``` 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 > --- > 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(), > ``` ## phillip.wood123@gmail.com, 2025-04-23 10:05 Subject: Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check Message-ID: <8a907622-a975-4f9c-86b3-54b8f5447709@gmail.com> URL: https://gitlist.dev/e/8a907622-a975-4f9c-86b3-54b8f5447709%40gmail.com In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-0-d1e2be4b2078@gmail.com> ``` 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 > > ## 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 > ``` ## Patrick Steinhardt, 2025-04-23 11:30 Subject: Re: [PATCH v5 6/6] makefile/meson: add 'check-headers' as alias for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/aAjPUGJeOjQHpAKz%40pks.im In-Reply-To: <20250423-505-wire-up-sparse-via-meson-v5-6-d1e2be4b2078@gmail.com> ``` 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 ``` ## Karthik Nayak, 2025-04-23 17:39 Subject: Re: [PATCH v5 1/6] ci/github: install git before checking out the repository Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZQJZ0L2wONYJo_JNKSDHHRWQ-sOTuA34FLLgzzZPnHdqg%40mail.gmail.com In-Reply-To: <3607a937-9795-4348-981e-dce92203c6d7@gmail.com> ``` 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 >> Signed-off-by: Karthik Nayak >> --- >> .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 >> ``` ## Junio C Hamano, 2025-04-23 17:40 Subject: Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqqtt6ezshm.fsf%40gitster.g In-Reply-To: <8a907622-a975-4f9c-86b3-54b8f5447709@gmail.com> ``` 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. ``` ## Karthik Nayak, 2025-04-23 17:40 Subject: Re: [PATCH v5 5/6] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZThKEHa-%3DEb%2BHef8Q2uoWs1xOZBuHL-KUt0Z5K-uLuJsw%40mail.gmail.com In-Reply-To: <2a25ca04-c824-4bec-a5e7-95ec32556cf9@gmail.com> ``` 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 >> --- >> 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(), >> ``` ## Karthik Nayak, 2025-04-23 17:41 Subject: Re: [PATCH v5 6/6] makefile/meson: add 'check-headers' as alias for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZTXjrXgqkQo1S9VgLFrgniQmJTtNwtuVt0%2BisgsCoLFdA%40mail.gmail.com In-Reply-To: ``` Patrick Steinhardt 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 ``` ## Karthik Nayak, 2025-04-23 17:42 Subject: Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZSnvTVwooa-S6AE%2Bu1N0MV2D9TBOK3A0NBmCzOF0bjOTg%40mail.gmail.com In-Reply-To: <8a907622-a975-4f9c-86b3-54b8f5447709@gmail.com> ``` 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 ``` ## Junio C Hamano, 2025-04-23 20:04 Subject: Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqqv7quwsob.fsf%40gitster.g In-Reply-To: ``` Junio C Hamano 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. ``` ## Junio C Hamano, 2025-04-23 20:22 Subject: Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqqr01iwruj.fsf%40gitster.g In-Reply-To: ``` Junio C Hamano 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. ``` ## Karthik Nayak, 2025-04-23 21:22 Subject: Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZT%2BhUuvdMbES_k8Pk%2BdLgrOovZB4fiRjJpS2AUrRrUogg%40mail.gmail.com In-Reply-To: ``` Junio C Hamano writes: > Junio C Hamano 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/ ``` ## Junio C Hamano, 2025-04-23 21:54 Subject: Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/xmqqikmuwnly.fsf%40gitster.g In-Reply-To: ``` Karthik Nayak writes: > Junio C Hamano writes: > >> Junio C Hamano 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. ``` ## Karthik Nayak, 2025-04-23 22:12 Subject: Re: [PATCH v5 0/6] meson: add corresponding target for Makefile's hdr-check Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZTK%2BYFYSMfFwNc%3DYP5SvMaOa_wABv3G7_Kyi6iqTw74SQ%40mail.gmail.com In-Reply-To: ``` Junio C Hamano writes: > Karthik Nayak writes: > >> Junio C Hamano writes: >> >>> Junio C Hamano 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. ```