# [PATCH v3 1/4] coccinelle: meson: rename variables to be more specific 13 messages from 2025-04-14 to 2025-04-20. Participants: Karthik Nayak, Junio C Hamano, Phillip Wood, phillip.wood123@gmail.com. Thread: https://gitlist.dev/t/63294 ## Karthik Nayak, 2025-04-14 21:15 Subject: [PATCH v3 0/4] meson: add corresponding target for Makefile's hdr-check Message-ID: <20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745@gmail.com> URL: https://gitlist.dev/e/20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745%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 v3: - Some renames: - headers_generated -> generated_headers - meson -> Meson - headers-check -> check-headers - headers_check_exclude -> exclude_from_check_headers - Rewrite 'headers_check_exclude' to also contain dirs so we can skip listing individual header files. - Move 'xdiff/*' to 'third_party_sources' and cleanup 'exclude_from_check_headers'. - Use 'echo' instead of 'echo -n'. - Use `fs.replace_suffix` instead of `str.replace`. - Link to v2: https://lore.kernel.org/r/20250410-505-wire-up-sparse-via-meson-v2-0-acb45cc8a2e5@gmail.com Changes in v2: - Add 'hdr-check' to meson, while introducing 'headers-check' as a replacement alias. Schedule 'hdr-check' to be deprecated in the future. - Link to v1: https://lore.kernel.org/r/20250408-505-wire-up-sparse-via-meson-v1-0-17476e5cea3f@gmail.com --- Makefile | 4 +- ci/run-static-analysis.sh | 2 +- contrib/coccinelle/meson.build | 29 ++++--------- meson.build | 94 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 24 deletions(-) Karthik Nayak (4): coccinelle: meson: rename variables to be more specific meson: move headers definition from 'contrib/coccinelle' meson: add support for 'hdr-check' makefile/meson: add 'check-headers' as alias for 'hdr-check' Range-diff versus v2: 1: ca94103a13 ! 1: 3386caeaf5 coccinelle: meson: rename variables to be more specific @@ Metadata ## Commit message ## coccinelle: meson: rename variables to be more specific - In meson, included subdirs export their variables to top level meson + In Meson, included subdirs export their variables to top level Meson builds. In 'contrib/coccinelle/meson.build', we define two such variables `sources` and `headers`. While these variables are specific to the checks in the 'contrib/coccinelle/' directory, they also pollute the 2: 2549f1a5df ! 2: 35f5a580e4 meson: move headers definition from 'contrib/coccinelle' @@ Metadata ## Commit message ## meson: move headers definition from 'contrib/coccinelle' - The meson build for coccinelle static analysis lists all headers to - analyse. Due to the way meson exports variables between subdirs, this - variable is also available in the root meson build. + The Meson build for coccinelle static analysis lists all headers to + analyse. Due to the way Meson exports variables between subdirs, this + variable is also available in the root Meson build. An upcoming commit, will add a new check complimenting 'hdr-check' in the Makefile. This would require the list of headers. So move the - 'coccinelle_headers' to the root meson build and rename it to 'headers', + 'coccinelle_headers' to the root Meson build and rename it to 'headers', remove the root path being appended to each header and retain that in - the coccinelle meson build since it is specific to the coccinelle build. + the coccinelle Meson build since it is specific to the coccinelle build. - Also move the 'third_party_sources' variable to the root meson build + Also move the 'third_party_sources' variable to the root Meson build since it is also a dependency for the 'headers' variable. This also makes it easier to understand as the variable is now propagated from the top level to the bottom. @@ meson.build: builtin_sources = [ + ':!t/t[0-9][0-9][0-9][0-9]*', +] + -+headers = [] +if git.found() ++ headers = [] + foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split() + headers += header + endforeach 3: a46db4a81c < -: ---------- meson: add support for 'hdr-check' -: ---------- > 3: 0df83087ac meson: add support for 'hdr-check' 4: fe0160b6fe ! 4: b5d10772ff makefile/meson: add 'headers-check' as alias for 'hdr-check' @@ Metadata Author: Karthik Nayak ## Commit message ## - makefile/meson: add 'headers-check' as alias for 'hdr-check' + makefile/meson: add 'check-headers' as alias for 'hdr-check' - The 'hdr-check' target in meson and makefile is used to check if headers + The 'hdr-check' target in Meson and makefile is used to check if headers can be compiled individually. The naming however isn't readable as 'hdr' is not a common shortforme for 'header', neither is it an abbreviation. - Let's introduce 'headers-check' as an alternative target for 'hdr-check' + Let's introduce 'check-headers' as an alternative target for 'hdr-check' and add a `TODO` to deprecate the latter after 2 releases. Since this is an internal tool, we can use a shorter deprecation cycle. Change existing usage of 'hdr-check' in 'ci/run-static-analysis.sh' to - also use 'headers-check'. + also use 'check-headers'. Signed-off-by: Karthik Nayak @@ Makefile: HCC = $(HCO:hco=hcc) $(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $< -.PHONY: hdr-check $(HCO) -+# TODO: deprecate 'hdr-check' in lieu of 'headers-check' in Git 2.51+ -+.PHONY: hdr-check headers-check $(HCO) ++# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+ ++.PHONY: hdr-check check-headers $(HCO) hdr-check: $(HCO) -+headers-check: hdr-check ++check-headers: hdr-check .PHONY: style style: @@ ci/run-static-analysis.sh: then fi -make hdr-check || -+make headers-check || ++make check-headers || exit 1 make check-pot ## meson.build ## -@@ meson.build: if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc' +@@ meson.build: if git.found() and compiler.get_argument_syntax() == 'gcc' hco_targets += hco endforeach -+ # TODO: deprecate 'hdr-check' in lieu of 'headers-check' in Git 2.51+ ++ # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+ alias_target('hdr-check', hco_targets) -+ alias_target('headers-check', hco_targets) ++ alias_target('check-headers', hco_targets) endif foreach key, value : { base-commit: 3a956c5f69873611ae5f8dcb9acd117f66b95ddc change-id: 20250330-505-wire-up-sparse-via-meson-2e32dd31208b Thanks - Karthik ``` ## Karthik Nayak, 2025-04-14 21:15 Subject: [PATCH v3 1/4] coccinelle: meson: rename variables to be more specific Message-ID: <20250414-505-wire-up-sparse-via-meson-v3-1-edc6e7f26745@gmail.com> URL: https://gitlist.dev/e/20250414-505-wire-up-sparse-via-meson-v3-1-edc6e7f26745%40gmail.com In-Reply-To: <20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745@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-14 21:16 Subject: [PATCH v3 2/4] meson: move headers definition from 'contrib/coccinelle' Message-ID: <20250414-505-wire-up-sparse-via-meson-v3-2-edc6e7f26745@gmail.com> URL: https://gitlist.dev/e/20250414-505-wire-up-sparse-via-meson-v3-2-edc6e7f26745%40gmail.com In-Reply-To: <20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745@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..3ca5d01071 100644 --- a/meson.build +++ b/meson.build @@ -633,6 +633,28 @@ builtin_sources = [ 'builtin/write-tree.c', ] +third_party_sources = [ + ':!contrib', + ':!compat/inet_ntop.c', + ':!compat/inet_pton.c', + ':!compat/nedmalloc', + ':!compat/obstack.*', + ':!compat/poll', + ':!compat/regex', + ':!sha1collisiondetection', + ':!sha1dc', + ':!t/unit-tests/clar', + ':!t/unit-tests/clar', + ':!t/t[0-9][0-9][0-9][0-9]*', +] + +if git.found() + headers = [] + foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split() + headers += header + endforeach +endif + if not get_option('breaking_changes') builtin_sources += 'builtin/pack-redundant.c' endif -- 2.48.1 ``` ## Karthik Nayak, 2025-04-14 21:16 Subject: [PATCH v3 3/4] meson: add support for 'hdr-check' Message-ID: <20250414-505-wire-up-sparse-via-meson-v3-3-edc6e7f26745@gmail.com> URL: https://gitlist.dev/e/20250414-505-wire-up-sparse-via-meson-v3-3-edc6e7f26745%40gmail.com In-Reply-To: <20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745@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! Signed-off-by: Karthik Nayak --- meson.build | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/meson.build b/meson.build index 3ca5d01071..b1be2b3cbb 100644 --- a/meson.build +++ b/meson.build @@ -646,6 +646,7 @@ third_party_sources = [ ':!t/unit-tests/clar', ':!t/unit-tests/clar', ':!t/t[0-9][0-9][0-9][0-9]*', + ':!xdiff', ] if git.found() @@ -655,6 +656,10 @@ if git.found() endforeach endif +generated_headers = [ + 'command-list.h', +] + if not get_option('breaking_changes') builtin_sources += 'builtin/pack-redundant.c' endif @@ -669,6 +674,7 @@ builtin_sources += custom_target( ], env: script_environment, ) +generated_headers += 'config-list.h' builtin_sources += custom_target( input: 'Documentation/githooks.adoc', @@ -681,6 +687,7 @@ builtin_sources += custom_target( ], env: script_environment, ) +generated_headers += 'hook-list.h' # This contains the variables for GIT-BUILD-OPTIONS, which we use to propagate # build options to our tests. @@ -1995,6 +2002,69 @@ endif subdir('contrib') +exclude_from_check_headers = generated_headers +exclude_from_check_headers += [ + 'compat/', + 'unicode-width.h', +] + +if sha1_backend != 'openssl' + exclude_from_check_headers += 'sha1/openssl.h' +endif +if sha256_backend != 'openssl' + exclude_from_check_headers += 'sha256/openssl.h' +endif +if sha256_backend != 'nettle' + exclude_from_check_headers += 'sha256/nettle.h' +endif +if sha256_backend != 'gcrpyt' + exclude_from_check_headers += 'sha256/gcrypt.h' +endif + +if git.found() and compiler.get_argument_syntax() == 'gcc' + hco_targets = [] + foreach h : headers + skip_header = false + foreach exclude : exclude_from_check_headers + if h.startswith(exclude) + skip_header = true + break + endif + endforeach + + if skip_header + continue + endif + + hcc = custom_target( + input: h, + output: h.underscorify() + 'cc', + command: [ + shell, + '-c', + 'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@' + ] + ) + + hco = custom_target( + input: hcc, + output: fs.replace_suffix(h.underscorify(), '.hco'), + command: [ + compiler.cmd_array(), + libgit_c_args, + '-I', meson.project_source_root(), + '-I', meson.project_source_root() / 't/unit-tests', + '-o', '/dev/null', + '-c', '-xc', + '@INPUT@' + ] + ) + hco_targets += hco + endforeach + + alias_target('hdr-check', hco_targets) +endif + foreach key, value : { 'DIFF': diff.full_path(), 'GIT_SOURCE_DIR': meson.project_source_root(), -- 2.48.1 ``` ## Karthik Nayak, 2025-04-14 21:16 Subject: [PATCH v3 4/4] makefile/meson: add 'check-headers' as alias for 'hdr-check' Message-ID: <20250414-505-wire-up-sparse-via-meson-v3-4-edc6e7f26745@gmail.com> URL: https://gitlist.dev/e/20250414-505-wire-up-sparse-via-meson-v3-4-edc6e7f26745%40gmail.com In-Reply-To: <20250414-505-wire-up-sparse-via-meson-v3-0-edc6e7f26745@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 | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index ac32d2d0bd..961ee508be 100644 --- a/Makefile +++ b/Makefile @@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc) $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE $(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $< -.PHONY: hdr-check $(HCO) +# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+ +.PHONY: hdr-check check-headers $(HCO) hdr-check: $(HCO) +check-headers: hdr-check .PHONY: style style: diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh index 0d51e5ce0e..60c175a094 100755 --- a/ci/run-static-analysis.sh +++ b/ci/run-static-analysis.sh @@ -26,7 +26,7 @@ then exit 1 fi -make hdr-check || +make check-headers || exit 1 make check-pot diff --git a/meson.build b/meson.build index b1be2b3cbb..745cb30165 100644 --- a/meson.build +++ b/meson.build @@ -2062,7 +2062,9 @@ if git.found() and compiler.get_argument_syntax() == 'gcc' hco_targets += hco endforeach + # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+ alias_target('hdr-check', hco_targets) + alias_target('check-headers', hco_targets) endif foreach key, value : { -- 2.48.1 ``` ## Junio C Hamano, 2025-04-14 22:11 Subject: Re: [PATCH v3 3/4] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/xmqqy0w2o0l0.fsf%40gitster.g In-Reply-To: <20250414-505-wire-up-sparse-via-meson-v3-3-edc6e7f26745@gmail.com> ``` Karthik Nayak writes: > +if sha256_backend != 'gcrpyt' That's a bit unexpected name; relative to this one ... > + exclude_from_check_headers += 'sha256/gcrypt.h' > +endif ... I have to suspect that it is a typo? ``` ## Karthik Nayak, 2025-04-15 06:43 Subject: Re: [PATCH v3 3/4] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZTjXetN3GNyvRSHb7mjxhnVd3VLSstnHxw8bAsO6S4wqA%40mail.gmail.com In-Reply-To: ``` Junio C Hamano writes: > Karthik Nayak writes: > >> +if sha256_backend != 'gcrpyt' > > That's a bit unexpected name; relative to this one ... > >> + exclude_from_check_headers += 'sha256/gcrypt.h' >> +endif > > ... I have to suspect that it is a typo? Indeed, I had to double check cause I couldn't spot it when you mentioned. Will fix. Thanks ``` ## Phillip Wood, 2025-04-15 13:28 Subject: Re: [PATCH v3 2/4] meson: move headers definition from 'contrib/coccinelle' Message-ID: <23493ec4-fd9f-44ae-a81a-8851e495b427@gmail.com> URL: https://gitlist.dev/e/23493ec4-fd9f-44ae-a81a-8851e495b427%40gmail.com In-Reply-To: <20250414-505-wire-up-sparse-via-meson-v3-2-edc6e7f26745@gmail.com> ``` Hi Karthik On 14/04/2025 22:16, Karthik Nayak wrote: > diff --git a/meson.build b/meson.build > index e98cfa4909..3ca5d01071 100644 > --- a/meson.build > +++ b/meson.build > @@ -633,6 +633,28 @@ builtin_sources = [ > 'builtin/write-tree.c', > ] > > +third_party_sources = [ This is not the fault of this patch but I found this name rather confusing as it is not a list of sources but a list of exclude patterns. Calling it "third_party_excludes" would be clearer to me at least. > + ':!contrib', > + ':!compat/inet_ntop.c', > + ':!compat/inet_pton.c', > + ':!compat/nedmalloc', > + ':!compat/obstack.*', > + ':!compat/poll', > + ':!compat/regex', > + ':!sha1collisiondetection', > + ':!sha1dc', > + ':!t/unit-tests/clar', > + ':!t/unit-tests/clar', Again not a new problem but this line is a duplicate > + ':!t/t[0-9][0-9][0-9][0-9]*', > +] > + > +if git.found() > + headers = [] This is called "headers" but it is only really the subset of our headers that we want to run static analysis on. Maybe we could call it "headers_to_check" or something that makes it clearer what the list is for. Best Wishes Phillip > + foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split() > + headers += header > + endforeach > +endif > + > if not get_option('breaking_changes') > builtin_sources += 'builtin/pack-redundant.c' > endif > ``` ## phillip.wood123@gmail.com, 2025-04-15 13:28 Subject: Re: [PATCH v3 3/4] meson: add support for 'hdr-check' Message-ID: <958aee61-23eb-4562-af9d-48108e6fe581@gmail.com> URL: https://gitlist.dev/e/958aee61-23eb-4562-af9d-48108e6fe581%40gmail.com In-Reply-To: <20250414-505-wire-up-sparse-via-meson-v3-3-edc6e7f26745@gmail.com> ``` Hi Karthik On 14/04/2025 22:16, Karthik Nayak wrote: > The Makefile supports a target called 'hdr-check', which checks if > individual header files can be independently compiled. Let's port this > functionality to Meson, our new build system too. The implementation > resembles that of the Makefile and provides the same check. > > Since meson builds are out-of-tree, header dependencies are not > automatically met. So unlike the Makefile version, we also need to add > the required dependencies. > > Also add the 'xdiff/' dir to the list of 'third_party_sources' as those > headers must be skipped from the checks too! Doesn't this mean we'll skip all the xdiff files when running coccinelle as well? If so the commit message should point that out and explain why that is an improvement. > +exclude_from_check_headers = generated_headers I'm not sure this is necessary. The list of headers that we filter is generated with "git ls-files" and so wont contain generated headers in the first place. > +exclude_from_check_headers += [ > + 'compat/', > + 'unicode-width.h', > +] > + > +if sha1_backend != 'openssl' > + exclude_from_check_headers += 'sha1/openssl.h' > +endif > +if sha256_backend != 'openssl' > + exclude_from_check_headers += 'sha256/openssl.h' > +endif > +if sha256_backend != 'nettle' > + exclude_from_check_headers += 'sha256/nettle.h' > +endif > +if sha256_backend != 'gcrpyt' > + exclude_from_check_headers += 'sha256/gcrypt.h' > +endif > + > +if git.found() and compiler.get_argument_syntax() == 'gcc' > + hco_targets = [] > + foreach h : headers > + skip_header = false > + foreach exclude : exclude_from_check_headers > + if h.startswith(exclude) > + skip_header = true > + break > + endif > + endforeach This part looks much more maintainable than than the previous version Thanks Phillip > + if skip_header > + continue > + endif > + > + hcc = custom_target( > + input: h, > + output: h.underscorify() + 'cc', > + command: [ > + shell, > + '-c', > + 'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@' > + ] > + ) > + > + hco = custom_target( > + input: hcc, > + output: fs.replace_suffix(h.underscorify(), '.hco'), > + command: [ > + compiler.cmd_array(), > + libgit_c_args, > + '-I', meson.project_source_root(), > + '-I', meson.project_source_root() / 't/unit-tests', > + '-o', '/dev/null', > + '-c', '-xc', > + '@INPUT@' > + ] > + ) > + hco_targets += hco > + endforeach > + > + alias_target('hdr-check', hco_targets) > +endif > + > foreach key, value : { > 'DIFF': diff.full_path(), > 'GIT_SOURCE_DIR': meson.project_source_root(), ``` ## phillip.wood123@gmail.com, 2025-04-15 13:28 Subject: Re: [PATCH v3 4/4] makefile/meson: add 'check-headers' as alias for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/f7d086eb-e616-4bf6-a679-6894ad6eaa85%40gmail.com In-Reply-To: <20250414-505-wire-up-sparse-via-meson-v3-4-edc6e7f26745@gmail.com> ``` Hi Karthik On 14/04/2025 22:16, Karthik Nayak wrote: > The 'hdr-check' target in Meson and makefile is used to check if headers > can be compiled individually. The naming however isn't readable as 'hdr' > is not a common shortforme for 'header', neither is it an abbreviation. > > Let's introduce 'check-headers' as an alternative target for 'hdr-check' > and add a `TODO` to deprecate the latter after 2 releases. Since this > is an internal tool, we can use a shorter deprecation cycle. Thanks for renaming this. I've left one small question at below but this basically looks good to me. > diff --git a/Makefile b/Makefile > index ac32d2d0bd..961ee508be 100644 > --- a/Makefile > +++ b/Makefile > @@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc) > $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE > $(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $< > > -.PHONY: hdr-check $(HCO) > +# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+ > +.PHONY: hdr-check check-headers $(HCO) > hdr-check: $(HCO) > +check-headers: hdr-check > > .PHONY: style > style: > diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh > index 0d51e5ce0e..60c175a094 100755 > --- a/ci/run-static-analysis.sh > +++ b/ci/run-static-analysis.sh > @@ -26,7 +26,7 @@ then > exit 1 > fi > > -make hdr-check || > +make check-headers || > exit 1 > > make check-pot > diff --git a/meson.build b/meson.build > index b1be2b3cbb..745cb30165 100644 > --- a/meson.build > +++ b/meson.build > @@ -2062,7 +2062,9 @@ if git.found() and compiler.get_argument_syntax() == 'gcc' > hco_targets += hco > endforeach > > + # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+ > alias_target('hdr-check', hco_targets) > + alias_target('check-headers', hco_targets) This looks good. One minor question which isn't worth a re-roll on its own - in the Makefile the check-headers target depends on the hdr-check target which means the two will always use the same dependencies. Here we make check-headers depend on hco_targets instead which means that in theory the two targets could get out of sync if the dependencies of hdr-check changed. Is it possible to have a meson target that depends on hdr-check directly? Best Wishes Phillip > endif > > foreach key, value : { > ``` ## Karthik Nayak, 2025-04-20 10:09 Subject: Re: [PATCH v3 2/4] meson: move headers definition from 'contrib/coccinelle' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZRchwJWQfLDCMASEe9RWMOKDQvTfXaKqO%3DShy%3DjhWMNaQ%40mail.gmail.com In-Reply-To: <23493ec4-fd9f-44ae-a81a-8851e495b427@gmail.com> ``` Phillip Wood writes: > Hi Karthik > > On 14/04/2025 22:16, Karthik Nayak wrote: >> diff --git a/meson.build b/meson.build >> index e98cfa4909..3ca5d01071 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -633,6 +633,28 @@ builtin_sources = [ >> 'builtin/write-tree.c', >> ] >> >> +third_party_sources = [ > > This is not the fault of this patch but I found this name rather > confusing as it is not a list of sources but a list of exclude patterns. > Calling it "third_party_excludes" would be clearer to me at least. > This seems to map what the Makefile has, but I would agree, that this should be renamed. >> + ':!contrib', >> + ':!compat/inet_ntop.c', >> + ':!compat/inet_pton.c', >> + ':!compat/nedmalloc', >> + ':!compat/obstack.*', >> + ':!compat/poll', >> + ':!compat/regex', >> + ':!sha1collisiondetection', >> + ':!sha1dc', >> + ':!t/unit-tests/clar', >> + ':!t/unit-tests/clar', > > Again not a new problem but this line is a duplicate > Fair, considering we're touching these fields, I will add an additional commit to simply cleanup these two issues. >> + ':!t/t[0-9][0-9][0-9][0-9]*', >> +] >> + >> +if git.found() >> + headers = [] > > This is called "headers" but it is only really the subset of our headers > that we want to run static analysis on. Maybe we could call it > "headers_to_check" or something that makes it clearer what the list is for. > Agreed, will rename. Thanks! > Best Wishes > > Phillip > >> + foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split() >> + headers += header >> + endforeach >> +endif >> + >> if not get_option('breaking_changes') >> builtin_sources += 'builtin/pack-redundant.c' >> endif >> ``` ## Karthik Nayak, 2025-04-20 10:57 Subject: Re: [PATCH v3 3/4] meson: add support for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZSYgdY0YnP1Uw83N7oT9R%2BAUt034ecgPt0ak%2BRCD3f_6Q%40mail.gmail.com In-Reply-To: <958aee61-23eb-4562-af9d-48108e6fe581@gmail.com> ``` phillip.wood123@gmail.com writes: > Hi Karthik > > On 14/04/2025 22:16, Karthik Nayak wrote: >> The Makefile supports a target called 'hdr-check', which checks if >> individual header files can be independently compiled. Let's port this >> functionality to Meson, our new build system too. The implementation >> resembles that of the Makefile and provides the same check. >> >> Since meson builds are out-of-tree, header dependencies are not >> automatically met. So unlike the Makefile version, we also need to add >> the required dependencies. >> >> Also add the 'xdiff/' dir to the list of 'third_party_sources' as those >> headers must be skipped from the checks too! > > Doesn't this mean we'll skip all the xdiff files when running coccinelle > as well? If so the commit message should point that out and explain why > that is an improvement. > Yes, let me add it to the commit message. >> +exclude_from_check_headers = generated_headers > > I'm not sure this is necessary. The list of headers that we filter is > generated with "git ls-files" and so wont contain generated headers in > the first place. > Good point, since generated headers are part of the '.gitignore', we can skip the 'generated_headers' variable entirely. >> +exclude_from_check_headers += [ >> + 'compat/', >> + 'unicode-width.h', >> +] >> + >> +if sha1_backend != 'openssl' >> + exclude_from_check_headers += 'sha1/openssl.h' >> +endif >> +if sha256_backend != 'openssl' >> + exclude_from_check_headers += 'sha256/openssl.h' >> +endif >> +if sha256_backend != 'nettle' >> + exclude_from_check_headers += 'sha256/nettle.h' >> +endif >> +if sha256_backend != 'gcrpyt' >> + exclude_from_check_headers += 'sha256/gcrypt.h' >> +endif >> + >> +if git.found() and compiler.get_argument_syntax() == 'gcc' >> + hco_targets = [] >> + foreach h : headers >> + skip_header = false >> + foreach exclude : exclude_from_check_headers >> + if h.startswith(exclude) >> + skip_header = true >> + break >> + endif >> + endforeach > > This part looks much more maintainable than than the previous version > Indeed. Thanks! > > Thanks > > Phillip > >> + if skip_header >> + continue >> + endif >> + >> + hcc = custom_target( >> + input: h, >> + output: h.underscorify() + 'cc', >> + command: [ >> + shell, >> + '-c', >> + 'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo \'#include "' + h + '"\' >> @OUTPUT@' >> + ] >> + ) >> + >> + hco = custom_target( >> + input: hcc, >> + output: fs.replace_suffix(h.underscorify(), '.hco'), >> + command: [ >> + compiler.cmd_array(), >> + libgit_c_args, >> + '-I', meson.project_source_root(), >> + '-I', meson.project_source_root() / 't/unit-tests', >> + '-o', '/dev/null', >> + '-c', '-xc', >> + '@INPUT@' >> + ] >> + ) >> + hco_targets += hco >> + endforeach >> + >> + alias_target('hdr-check', hco_targets) >> +endif >> + >> foreach key, value : { >> 'DIFF': diff.full_path(), >> 'GIT_SOURCE_DIR': meson.project_source_root(), ``` ## Karthik Nayak, 2025-04-20 11:02 Subject: Re: [PATCH v3 4/4] makefile/meson: add 'check-headers' as alias for 'hdr-check' Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZRPjeGP-yF_p-vve22mW8SrtqV_i_4JrjmGt1hRSqrGPg%40mail.gmail.com In-Reply-To: ``` phillip.wood123@gmail.com writes: > Hi Karthik > > On 14/04/2025 22:16, Karthik Nayak wrote: >> The 'hdr-check' target in Meson and makefile is used to check if headers >> can be compiled individually. The naming however isn't readable as 'hdr' >> is not a common shortforme for 'header', neither is it an abbreviation. >> >> Let's introduce 'check-headers' as an alternative target for 'hdr-check' >> and add a `TODO` to deprecate the latter after 2 releases. Since this >> is an internal tool, we can use a shorter deprecation cycle. > > Thanks for renaming this. I've left one small question at below but this > basically looks good to me. > >> diff --git a/Makefile b/Makefile >> index ac32d2d0bd..961ee508be 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -3326,8 +3326,10 @@ HCC = $(HCO:hco=hcc) >> $(HCO): %.hco: %.hcc $(GENERATED_H) FORCE >> $(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $< >> >> -.PHONY: hdr-check $(HCO) >> +# TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+ >> +.PHONY: hdr-check check-headers $(HCO) >> hdr-check: $(HCO) >> +check-headers: hdr-check >> >> .PHONY: style >> style: >> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh >> index 0d51e5ce0e..60c175a094 100755 >> --- a/ci/run-static-analysis.sh >> +++ b/ci/run-static-analysis.sh >> @@ -26,7 +26,7 @@ then >> exit 1 >> fi >> >> -make hdr-check || >> +make check-headers || >> exit 1 >> >> make check-pot >> diff --git a/meson.build b/meson.build >> index b1be2b3cbb..745cb30165 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -2062,7 +2062,9 @@ if git.found() and compiler.get_argument_syntax() == 'gcc' >> hco_targets += hco >> endforeach >> >> + # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+ >> alias_target('hdr-check', hco_targets) >> + alias_target('check-headers', hco_targets) > > This looks good. One minor question which isn't worth a re-roll on its > own - in the Makefile the check-headers target depends on the hdr-check > target which means the two will always use the same dependencies. Here > we make check-headers depend on hco_targets instead which means that in > theory the two targets could get out of sync if the dependencies of > hdr-check changed. Is it possible to have a meson target that depends on > hdr-check directly? > That's a good question, I didn't think much about it then, but looking into it, seems like we can do diff --git a/meson.build b/meson.build index 3bf3d65e8b..39319e2610 100644 --- a/meson.build +++ b/meson.build @@ -2055,8 +2055,8 @@ if git.found() and compiler.get_argument_syntax() == 'gcc' endforeach # TODO: deprecate 'hdr-check' in lieu of 'check-headers' in Git 2.51+ - alias_target('hdr-check', hco_targets) - alias_target('check-headers', hco_targets) + hdr_check = alias_target('hdr-check', hco_targets) + alias_target('check-headers', hdr_check) endif foreach key, value : { Not sure if there is a more idiomatic way, but this works. Will add in the next version. > Best Wishes > > Phillip > >> endif >> >> foreach key, value : { >> ```