[PATCH 0/8] ci: wire up support for Meson

[PATCH 0/8] ci: wire up support for Meson

[Patrick Steinhardt]

Hi,

this small patch series wires up Meson into our CI systems. The intent
is to ensure that it does not regress in functionality as the Git code
base evolves.

To help with keeping it up-to-date, both Meson and our Makefiles learn
to detect missing or superseded test files in "t/meson.build". This
should give users an early notification in case they have to add a newly
added or removed test to these build instructions. Overall I think that
this shouldn't be too much of a burden given that adding a new test is
trivial.

One gap that still exists is newly added code files. Due to many sources
being added to the build conditionally it's hard to have generic checks
for these. So I refrain from doing so in this series -- the build would
already fail anyway when we're missing code, so at least we will know
that something is up.

The series is built on top of caacdb5dfd (The fifteenth batch,
2024-12-10) with ps/build at 904339edbd (Introduce support for the Meson
build system, 2024-12-06) and cw/worktree-extension at 2037ca85ad
(worktree: refactor `repair_worktree_after_gitdir_move()`, 2024-11-29)
merged into it.

Thanks!

Patrick

---
Patrick Steinhardt (8):
      ci/lib: support custom output directories when creating test artifacts
      Makefile: drop -DSUPPRESS_ANNOTATED_LEAKS
      t/unit-tests: rename clar-based unit tests to have a common prefix
      meson: detect missing tests at configure time
      Makefile: detect missing Meson tests
      t: fix out-of-tree tests for some git-p4 tests
      t: introduce compatibility options to clar-based tests
      ci: wire up Meson builds

 .github/workflows/main.yml                  |  7 ++++
 .gitlab-ci.yml                              |  9 ++++++
 Makefile                                    |  5 ++-
 ci/install-dependencies.sh                  |  7 ++++
 ci/lib.sh                                   | 14 ++++----
 ci/print-test-failures.sh                   |  2 +-
 ci/run-build-and-tests.sh                   | 31 ++++++++++++++----
 meson.build                                 |  1 -
 parse-options.h                             | 12 +++++++
 t/Makefile                                  | 18 ++++++++++-
 t/meson.build                               | 40 +++++++++++++++++++++--
 t/t9835-git-p4-metadata-encoding-python2.sh | 48 ++++++++++++++-------------
 t/t9836-git-p4-metadata-encoding-python3.sh | 50 ++++++++++++++---------------
 t/unit-tests/generate-clar-decls.sh         |  5 ++-
 t/unit-tests/{ctype.c => u-ctype.c}         |  0
 t/unit-tests/{strvec.c => u-strvec.c}       |  0
 t/unit-tests/unit-test.c                    | 19 ++++++++++-
 17 files changed, 196 insertions(+), 72 deletions(-)


---
base-commit: 2fcbf72f13e8ce3bf1cda9a689f392f8f6e5c65d
change-id: 20241129-pks-meson-ci-ba1e40652fbe

[PATCH 1/8] ci/lib: support custom output directories when creating test artifacts

[Patrick Steinhardt]

Update `create_failed_test_artifacts ()` so that it can handle arbitrary
test output directories. This fixes creation of these artifacts for
macOS on GitLab CI, which uses a separate output directory already. This
will also be used by our out-of-tree builds with Meson.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/lib.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index 930f98d7228166c37c236beb062b14675fb68ef3..2e7a5f0540b66f24bd0f5744311c2c48b472d63d 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -180,9 +180,9 @@ handle_failed_tests () {
 }
 
 create_failed_test_artifacts () {
-	mkdir -p t/failed-test-artifacts
+	mkdir -p "${TEST_OUTPUT_DIRECTORY:-t}"/failed-test-artifacts
 
-	for test_exit in t/test-results/*.exit
+	for test_exit in "${TEST_OUTPUT_DIRECTORY:-t}"/test-results/*.exit
 	do
 		test 0 != "$(cat "$test_exit")" || continue
 
@@ -191,11 +191,11 @@ create_failed_test_artifacts () {
 		printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
 		echo "The full logs are in the 'print test failures' step below."
 		echo "See also the 'failed-tests-*' artifacts attached to this run."
-		cat "t/test-results/$test_name.markup"
+		cat "${TEST_OUTPUT_DIRECTORY:-t}/test-results/$test_name.markup"
 
-		trash_dir="t/trash directory.$test_name"
-		cp "t/test-results/$test_name.out" t/failed-test-artifacts/
-		tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
+		trash_dir="${TEST_OUTPUT_DIRECTORY:-t}/trash directory.$test_name"
+		cp "${TEST_OUTPUT_DIRECTORY:-t}/test-results/$test_name.out" "${TEST_OUTPUT_DIRECTORY:-t}"/failed-test-artifacts/
+		tar czf "${TEST_OUTPUT_DIRECTORY:-t}/failed-test-artifacts/$test_name.trash.tar.gz" "$trash_dir"
 	done
 }
 

-- 
2.47.1.447.ga7e8429e30.dirty

Re: [PATCH 1/8] ci/lib: support custom output directories when creating test artifacts

[karthik nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> Update `create_failed_test_artifacts ()` so that it can handle arbitrary
> test output directories. This fixes creation of these artifacts for
> macOS on GitLab CI, which uses a separate output directory already. This
> will also be used by our out-of-tree builds with Meson.
>

So currently in the config: `TEST_OUTPUT_DIRECTORY: "/Volumes/RAMDisk"`.
So this is broken as is?

>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  ci/lib.sh | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 930f98d7228166c37c236beb062b14675fb68ef3..2e7a5f0540b66f24bd0f5744311c2c48b472d63d 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -180,9 +180,9 @@ handle_failed_tests () {
>  }
>
>  create_failed_test_artifacts () {
> -	mkdir -p t/failed-test-artifacts
> +	mkdir -p "${TEST_OUTPUT_DIRECTORY:-t}"/failed-test-artifacts
>
> -	for test_exit in t/test-results/*.exit
> +	for test_exit in "${TEST_OUTPUT_DIRECTORY:-t}"/test-results/*.exit
>  	do
>  		test 0 != "$(cat "$test_exit")" || continue
>
> @@ -191,11 +191,11 @@ create_failed_test_artifacts () {
>  		printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
>  		echo "The full logs are in the 'print test failures' step below."
>  		echo "See also the 'failed-tests-*' artifacts attached to this run."
> -		cat "t/test-results/$test_name.markup"
> +		cat "${TEST_OUTPUT_DIRECTORY:-t}/test-results/$test_name.markup"
>
> -		trash_dir="t/trash directory.$test_name"
> -		cp "t/test-results/$test_name.out" t/failed-test-artifacts/
> -		tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
> +		trash_dir="${TEST_OUTPUT_DIRECTORY:-t}/trash directory.$test_name"
> +		cp "${TEST_OUTPUT_DIRECTORY:-t}/test-results/$test_name.out" "${TEST_OUTPUT_DIRECTORY:-t}"/failed-test-artifacts/
> +		tar czf "${TEST_OUTPUT_DIRECTORY:-t}/failed-test-artifacts/$test_name.trash.tar.gz" "$trash_dir"
>  	done
>  }

The changes look good, if the TEST_OUTPUT_DIRECTORY isn't set we default
to `t` which was the previous directory.

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

Re: [PATCH 1/8] ci/lib: support custom output directories when creating test artifacts

[Patrick Steinhardt]

On Thu, Dec 12, 2024 at 02:16:30AM -0800, karthik nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Update `create_failed_test_artifacts ()` so that it can handle arbitrary
> > test output directories. This fixes creation of these artifacts for
> > macOS on GitLab CI, which uses a separate output directory already. This
> > will also be used by our out-of-tree builds with Meson.
> >
> 
> So currently in the config: `TEST_OUTPUT_DIRECTORY: "/Volumes/RAMDisk"`.
> So this is broken as is?

Yeah. I've noticed multiple times that the test output directory is not
uploaded on failing macOS jobs.

Patrick

[PATCH 2/8] Makefile: drop -DSUPPRESS_ANNOTATED_LEAKS

[Patrick Steinhardt]

The -DSUPPRESS_ANNOTATED_LEAKS preprocessor directive was used to enable
our `UNLEAK()` macro in the past, which marks memory as still-reachable
so that the leak sanitizer does not complain. Starting with 52c7dbd036
(git-compat-util: drop now-unused `UNLEAK()` macro, 2024-11-20) this
macro has been removed, and thus the preprocessor directive is not
required anymore, either.

Drop it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile    | 1 -
 meson.build | 1 -
 2 files changed, 2 deletions(-)

diff --git a/Makefile b/Makefile
index 06f01149ecf399ae4bb1932188a007948d767283..2506f3b7e3377ab1a376338c86a727b2ae92a6e9 100644
--- a/Makefile
+++ b/Makefile
@@ -1490,7 +1490,6 @@ ifneq ($(filter undefined,$(SANITIZERS)),)
 BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS
 endif
 ifneq ($(filter leak,$(SANITIZERS)),)
-BASIC_CFLAGS += -DSUPPRESS_ANNOTATED_LEAKS
 BASIC_CFLAGS += -O0
 SANITIZE_LEAK = YesCompiledWithIt
 endif
diff --git a/meson.build b/meson.build
index 0dccebcdf16b07650d943e53643f0e09e2975cc9..1af25af5cc1e718a4e50fb14274a36f811506219 100644
--- a/meson.build
+++ b/meson.build
@@ -712,7 +712,6 @@ else
   build_options_config.set('SANITIZE_ADDRESS', '')
 endif
 if get_option('b_sanitize').contains('leak')
-  libgit_c_args += '-DSUPPRESS_ANNOTATED_LEAKS'
   build_options_config.set('SANITIZE_LEAK', 'YesCompiledWithIt')
 else
   build_options_config.set('SANITIZE_LEAK', '')

-- 
2.47.1.447.ga7e8429e30.dirty

[PATCH 3/8] t/unit-tests: rename clar-based unit tests to have a common prefix

[Patrick Steinhardt]

All of the code files for unit tests using the self-grown unit testing
framework have have a "t-" prefix to their name. This makes it easy to
identify them and use globbing in our Makefile and in other places. On
the other hand though, our clar-based unit tests have no prefix at all
and thus cannot easily be discerned from other files in the unit test
directory.

Introduce a new "u-" prefix for clar-based unit tests. This prefix will
be used in a subsequent commit to easily identify such tests.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile                              | 4 ++--
 t/meson.build                         | 4 ++--
 t/unit-tests/generate-clar-decls.sh   | 5 ++++-
 t/unit-tests/{ctype.c => u-ctype.c}   | 0
 t/unit-tests/{strvec.c => u-strvec.c} | 0
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 2506f3b7e3377ab1a376338c86a727b2ae92a6e9..6eafaf174aaa380ad8e6a86f75d003eb6c058fb3 100644
--- a/Makefile
+++ b/Makefile
@@ -1344,8 +1344,8 @@ THIRD_PARTY_SOURCES += sha1dc/%
 THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%
 THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%
 
-CLAR_TEST_SUITES += ctype
-CLAR_TEST_SUITES += strvec
+CLAR_TEST_SUITES += u-ctype
+CLAR_TEST_SUITES += u-strvec
 CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)
 CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))
 CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o
diff --git a/t/meson.build b/t/meson.build
index 13fe854ba0a18f9b83dbc48651f581198042ffd3..9e676e69363ed6311426500d98fe281e30d26bcb 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1,6 +1,6 @@
 clar_test_suites = [
-  'unit-tests/ctype.c',
-  'unit-tests/strvec.c',
+  'unit-tests/u-ctype.c',
+  'unit-tests/u-strvec.c',
 ]
 
 clar_sources = [
diff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh
index 688e0885f4f28182c3afe19c067b6d59dcacccfc..3b315c64b3711bfccc5941852a0782e02cee82f0 100755
--- a/t/unit-tests/generate-clar-decls.sh
+++ b/t/unit-tests/generate-clar-decls.sh
@@ -11,6 +11,9 @@ shift
 
 for suite in "$@"
 do
-	sed -ne "s/^\(void test_$(basename "${suite%.c}")__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$/extern \1;/p" "$suite" ||
+	suite_name=$(basename "$suite")
+	suite_name=${suite_name%.c}
+	suite_name=${suite_name#u-}
+	sed -ne "s/^\(void test_${suite_name}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$/extern \1;/p" "$suite" ||
 	exit 1
 done >"$OUTPUT"
diff --git a/t/unit-tests/ctype.c b/t/unit-tests/u-ctype.c
similarity index 100%
rename from t/unit-tests/ctype.c
rename to t/unit-tests/u-ctype.c
diff --git a/t/unit-tests/strvec.c b/t/unit-tests/u-strvec.c
similarity index 100%
rename from t/unit-tests/strvec.c
rename to t/unit-tests/u-strvec.c

-- 
2.47.1.447.ga7e8429e30.dirty

Re: [PATCH 3/8] t/unit-tests: rename clar-based unit tests to have a common prefix

[karthik nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> All of the code files for unit tests using the self-grown unit testing
> framework have have a "t-" prefix to their name. This makes it easy to

s/have have/have

> identify them and use globbing in our Makefile and in other places. On
> the other hand though, our clar-based unit tests have no prefix at all
> and thus cannot easily be discerned from other files in the unit test
> directory.
>
> Introduce a new "u-" prefix for clar-based unit tests. This prefix will
> be used in a subsequent commit to easily identify such tests.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Makefile                              | 4 ++--
>  t/meson.build                         | 4 ++--
>  t/unit-tests/generate-clar-decls.sh   | 5 ++++-
>  t/unit-tests/{ctype.c => u-ctype.c}   | 0
>  t/unit-tests/{strvec.c => u-strvec.c} | 0
>  5 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 2506f3b7e3377ab1a376338c86a727b2ae92a6e9..6eafaf174aaa380ad8e6a86f75d003eb6c058fb3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1344,8 +1344,8 @@ THIRD_PARTY_SOURCES += sha1dc/%
>  THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%
>  THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%
>
> -CLAR_TEST_SUITES += ctype
> -CLAR_TEST_SUITES += strvec
> +CLAR_TEST_SUITES += u-ctype
> +CLAR_TEST_SUITES += u-strvec
>  CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)
>  CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))
>  CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o
> diff --git a/t/meson.build b/t/meson.build
> index 13fe854ba0a18f9b83dbc48651f581198042ffd3..9e676e69363ed6311426500d98fe281e30d26bcb 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -1,6 +1,6 @@
>  clar_test_suites = [
> -  'unit-tests/ctype.c',
> -  'unit-tests/strvec.c',
> +  'unit-tests/u-ctype.c',
> +  'unit-tests/u-strvec.c',
>  ]
>
>  clar_sources = [
> diff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh
> index 688e0885f4f28182c3afe19c067b6d59dcacccfc..3b315c64b3711bfccc5941852a0782e02cee82f0 100755
> --- a/t/unit-tests/generate-clar-decls.sh
> +++ b/t/unit-tests/generate-clar-decls.sh
> @@ -11,6 +11,9 @@ shift
>
>  for suite in "$@"
>  do
> -	sed -ne "s/^\(void test_$(basename "${suite%.c}")__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$/extern \1;/p" "$suite" ||
> +	suite_name=$(basename "$suite")

So this strips away all dir and keeps the file name.
So `t/unit-tests/u-strvec.c` becomes `u-strvec.c`

> +	suite_name=${suite_name%.c}

We strip the .c here, so `u-strvec.c` becomes `u-strvec`

> +	suite_name=${suite_name#u-}

So we finally strip the `u-` prefix here.

> +	sed -ne "s/^\(void test_${suite_name}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$/extern \1;/p" "$suite" ||

Then we find all test functions in those files. Makes sense.

[snip]

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

Re: [PATCH 3/8] t/unit-tests: rename clar-based unit tests to have a common prefix

[Patrick Steinhardt]

On Thu, Dec 12, 2024 at 02:42:41AM -0800, karthik nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > All of the code files for unit tests using the self-grown unit testing
> > framework have have a "t-" prefix to their name. This makes it easy to
> 
> s/have have/have

Indeed, fixed.

Patrick

[PATCH 4/8] meson: detect missing tests at configure time

[Patrick Steinhardt]

It is quite easy for the list of integration tests to go out-of-sync
without anybody noticing. Introduce a new configure-time check that
verifies that all tests are wired up properly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/meson.build | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/t/meson.build b/t/meson.build
index 9e676e69363ed6311426500d98fe281e30d26bcb..f1fbc6ae179079f4d5d86f9a60956fad84d0495c 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1092,6 +1092,42 @@ integration_tests = [
   't9903-bash-prompt.sh',
 ]
 
+# Sanity check that we are not missing any tests present in 't/'. This check
+# only runs once at configure time and is thus best-effort, only. It is
+# sufficient to catch missing test suites in our CI though.
+foreach glob, tests : {
+  't[0-9][0-9][0-9][0-9]-*.sh': integration_tests,
+  'unit-tests/t-*.c': unit_test_programs,
+  'unit-tests/u-*.c': clar_test_suites,
+}
+  actual_tests = run_command(shell, '-c', 'ls ' + glob,
+    check: true,
+    env: script_environment,
+  ).stdout().strip().split('\n')
+
+  if tests != actual_tests
+    missing_tests = [ ]
+    foreach actual_test : actual_tests
+      if actual_test not in tests
+        missing_tests += actual_test
+      endif
+    endforeach
+    if missing_tests.length() > 0
+      error('Missing tests:\n\n - ' + '\n - '.join(missing_tests))
+    endif
+
+    superfluous_tests = [ ]
+    foreach integration_test : tests
+      if integration_test not in actual_tests
+        superfluous_tests += integration_test
+      endif
+    endforeach
+    if superfluous_tests.length() > 0
+      error('Superfluous tests:\n\n - ' + '\n - '.join(superfluous_tests))
+    endif
+  endif
+endforeach
+
 # GIT_BUILD_DIR needs to be Unix-style without drive prefixes as it get added
 # to the PATH variable. And given that drive prefixes contain a colon we'd
 # otherwise end up with a broken PATH if we didn't convert it.

-- 
2.47.1.447.ga7e8429e30.dirty

Re: [PATCH 4/8] meson: detect missing tests at configure time

[Toon Claes]

Patrick Steinhardt <ps@pks.im> writes:

> It is quite easy for the list of integration tests to go out-of-sync
> without anybody noticing. Introduce a new configure-time check that
> verifies that all tests are wired up properly.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/meson.build | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/t/meson.build b/t/meson.build
> index 9e676e69363ed6311426500d98fe281e30d26bcb..f1fbc6ae179079f4d5d86f9a60956fad84d0495c 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -1092,6 +1092,42 @@ integration_tests = [
>    't9903-bash-prompt.sh',
>  ]
>  
> +# Sanity check that we are not missing any tests present in 't/'. This check
> +# only runs once at configure time and is thus best-effort, only. It is
> +# sufficient to catch missing test suites in our CI though.
> +foreach glob, tests : {
> +  't[0-9][0-9][0-9][0-9]-*.sh': integration_tests,
> +  'unit-tests/t-*.c': unit_test_programs,
> +  'unit-tests/u-*.c': clar_test_suites,
> +}
> +  actual_tests = run_command(shell, '-c', 'ls ' + glob,
> +    check: true,
> +    env: script_environment,
> +  ).stdout().strip().split('\n')
> +
> +  if tests != actual_tests
> +    missing_tests = [ ]
> +    foreach actual_test : actual_tests
> +      if actual_test not in tests
> +        missing_tests += actual_test
> +      endif
> +    endforeach
> +    if missing_tests.length() > 0
> +      error('Missing tests:\n\n - ' + '\n - '.join(missing_tests))

This gives nice output:

    $ touch t/unit-tests/u-bar.c t/unit-tests/u-foo.c

    $ meson setup builddir --reconfigure

    The Meson build system
    Version: 1.4.1
    [snip]
    Configuring update.sample using configuration
    Configuring exclude using configuration

    t/meson.build:1116:6: ERROR: Problem encountered: Missing tests:

     - unit-tests/u-bar.c
     - unit-tests/u-foo.c

    A full log can be found at git/builddir/meson-logs/meson-log.txt

But I think the error message is a little bit confusing. It sounds like
the test file is missing, but it's the configuration of the test that is
missing.

Now I've realized it hard to write a good error message here. But I
would suggest something like "Tests files found, but not configured".

> +    endif
> +
> +    superfluous_tests = [ ]
> +    foreach integration_test : tests
> +      if integration_test not in actual_tests
> +        superfluous_tests += integration_test
> +      endif
> +    endforeach
> +    if superfluous_tests.length() > 0
> +      error('Superfluous tests:\n\n - ' + '\n - '.join(superfluous_tests))

Also here I would suggest different error message, maybe something like
"Tests configured, but file not found"

-- 
Toon

Re: [PATCH 4/8] meson: detect missing tests at configure time

[Patrick Steinhardt]

On Fri, Dec 13, 2024 at 10:58:47AM +0100, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/t/meson.build b/t/meson.build
> > index 9e676e69363ed6311426500d98fe281e30d26bcb..f1fbc6ae179079f4d5d86f9a60956fad84d0495c 100644
> > --- a/t/meson.build
> > +++ b/t/meson.build
> > @@ -1092,6 +1092,42 @@ integration_tests = [
> >    't9903-bash-prompt.sh',
> >  ]
> >  
> > +# Sanity check that we are not missing any tests present in 't/'. This check
> > +# only runs once at configure time and is thus best-effort, only. It is
> > +# sufficient to catch missing test suites in our CI though.
> > +foreach glob, tests : {
> > +  't[0-9][0-9][0-9][0-9]-*.sh': integration_tests,
> > +  'unit-tests/t-*.c': unit_test_programs,
> > +  'unit-tests/u-*.c': clar_test_suites,
> > +}
> > +  actual_tests = run_command(shell, '-c', 'ls ' + glob,
> > +    check: true,
> > +    env: script_environment,
> > +  ).stdout().strip().split('\n')
> > +
> > +  if tests != actual_tests
> > +    missing_tests = [ ]
> > +    foreach actual_test : actual_tests
> > +      if actual_test not in tests
> > +        missing_tests += actual_test
> > +      endif
> > +    endforeach
> > +    if missing_tests.length() > 0
> > +      error('Missing tests:\n\n - ' + '\n - '.join(missing_tests))
> 
> This gives nice output:
> 
>     $ touch t/unit-tests/u-bar.c t/unit-tests/u-foo.c
> 
>     $ meson setup builddir --reconfigure
> 
>     The Meson build system
>     Version: 1.4.1
>     [snip]
>     Configuring update.sample using configuration
>     Configuring exclude using configuration
> 
>     t/meson.build:1116:6: ERROR: Problem encountered: Missing tests:
> 
>      - unit-tests/u-bar.c
>      - unit-tests/u-foo.c
> 
>     A full log can be found at git/builddir/meson-logs/meson-log.txt
> 
> But I think the error message is a little bit confusing. It sounds like
> the test file is missing, but it's the configuration of the test that is
> missing.
> 
> Now I've realized it hard to write a good error message here. But I
> would suggest something like "Tests files found, but not configured".
> 
> > +    endif
> > +
> > +    superfluous_tests = [ ]
> > +    foreach integration_test : tests
> > +      if integration_test not in actual_tests
> > +        superfluous_tests += integration_test
> > +      endif
> > +    endforeach
> > +    if superfluous_tests.length() > 0
> > +      error('Superfluous tests:\n\n - ' + '\n - '.join(superfluous_tests))
> 
> Also here I would suggest different error message, maybe something like
> "Tests configured, but file not found"

Both good suggestions, thanks!

Patrick

[PATCH 5/8] Makefile: detect missing Meson tests

[Patrick Steinhardt]

In the preceding commit, we have introduced consistency checks to Meson
to detect any discrepancies with missing or extraneous tests in its
build instructions. These checks only get executed in Meson though, so
any users of our Makefiles wouldn't be alerted of the fact that they
have to modify the Meson build instructions in case they add or remove
any tests.

Add a comparable test target to our Makefile to plug this gap.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/Makefile | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/t/Makefile b/t/Makefile
index 131ffd778fe00864fae1f5750269615556c6cdea..290fb03ff011d39c31c5073c796aa6f4dc966283 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -59,7 +59,7 @@ CHAINLINTSUPPRESS = GIT_TEST_EXT_CHAIN_LINT=0 && export GIT_TEST_EXT_CHAIN_LINT
 
 all:: $(DEFAULT_TEST_TARGET)
 
-test: pre-clean check-chainlint $(TEST_LINT)
+test: pre-clean check-chainlint check-meson $(TEST_LINT)
 	$(CHAINLINTSUPPRESS) $(MAKE) aggregate-results-and-cleanup
 
 failed:
@@ -114,6 +114,22 @@ check-chainlint:
 	{ $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests >'$(CHAINLINTTMP_SQ)'/actual || true; } && \
 	diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
 
+check-meson:
+	@# awk acts up when trying to match single quotes, so we use \047 instead.
+	@printf "%s\n" \
+		"integration_tests t[0-9][0-9][0-9][0-9]-*.sh" \
+		"unit_test_programs unit-tests/t-*.c" \
+		"clar_test_suites unit-tests/u-*.c" | \
+	while read -r variable pattern; do \
+		meson_tests=$$(awk "/^$$variable = \[\$$/ {flag=1 ; next } /^]$$/ { flag=0 } flag { gsub(/^  \047/, \"\"); gsub(/\047,\$$/, \"\"); print }" meson.build) && \
+		actual_tests=$$(ls $$pattern) && \
+		if test "$$meson_tests" != "$$actual_tests"; then \
+			echo "Meson tests differ from actual tests:"; \
+			diff -u <(echo "$$meson_tests") <(echo "$$actual_tests"); \
+			exit 1; \
+		fi; \
+	done
+
 test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
 	test-lint-filenames
 ifneq ($(GIT_TEST_CHAIN_LINT),0)

-- 
2.47.1.447.ga7e8429e30.dirty

[PATCH 6/8] t: fix out-of-tree tests for some git-p4 tests

[Patrick Steinhardt]

Both t9835 and t9836 exercise git-p4, but one exercises Python 2 whereas
the other one uses Python 3. These tests do not exercise "git p4", but
instead they use "git p4.py" so that the unbuilt version of "git-p4.py"
is used that has "#!/usr/bin/env python" as shebang instead of the
replaced shebang.

But "git-p4.py" is not in our PATH during out-of-tree builds, and thus
we cannot locate "git-p4.py". The tests thus break with CMake and Meson.

Fix this by instead manually setting up script wrappers that invoke the
respective Python interpreter directly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t9835-git-p4-metadata-encoding-python2.sh | 48 ++++++++++++++-------------
 t/t9836-git-p4-metadata-encoding-python3.sh | 50 ++++++++++++++---------------
 2 files changed, 50 insertions(+), 48 deletions(-)

diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
index 036bf79c6674f6f1f0d667c7270674168428ffee..02f9ec09053890a4d41b7dc95644066d6481bbb6 100755
--- a/t/t9835-git-p4-metadata-encoding-python2.sh
+++ b/t/t9835-git-p4-metadata-encoding-python2.sh
@@ -14,23 +14,25 @@ python_target_version='2'
 ## SECTION REPEATED IN t9836 ##
 ###############################
 
-# Please note: this test calls "git-p4.py" rather than "git-p4", because the
-# latter references a specific path so we can't easily force it to run under
-# the python version we need to.
-
-python_major_version=$(python -V 2>&1 | cut -c  8)
-python_target_binary=$(which python$python_target_version)
-if ! test "$python_major_version" = "$python_target_version" && test "$python_target_binary"
+# These tests are specific to Python 2. Write a custom script that executes
+# git-p4 directly with the Python 2 interpreter to ensure that we use that
+# version even if Git was compiled with Python 3.
+python_target_binary=$(which python2)
+if test -n "$python_target_binary"
 then
 	mkdir temp_python
-	PATH="$(pwd)/temp_python:$PATH" && export PATH
-	ln -s $python_target_binary temp_python/python
+	PATH="$(pwd)/temp_python:$PATH"
+	export PATH
+
+	write_script temp_python/git-p4-python2 <<-EOF
+	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
+	EOF
 fi
 
-python_major_version=$(python -V 2>&1 | cut -c  8)
-if ! test "$python_major_version" = "$python_target_version"
+git p4-python2 >err
+if ! grep 'valid commands' err
 then
-	skip_all="skipping python$python_target_version-specific git p4 tests; python$python_target_version not available"
+	skip_all="skipping python2 git p4 tests; python2 not available"
 	test_done
 fi
 
@@ -81,14 +83,14 @@ test_expect_success 'init depot' '
 test_expect_success 'clone non-utf8 repo with strict encoding' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	test_must_fail git -c git-p4.metadataDecodingStrategy=strict p4.py clone --dest="$git" //depot@all 2>err &&
+	test_must_fail git -c git-p4.metadataDecodingStrategy=strict p4-python2 clone --dest="$git" //depot@all 2>err &&
 	grep "Decoding perforce metadata failed!" err
 '
 
 test_expect_success 'check utf-8 contents with passthrough strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -100,7 +102,7 @@ test_expect_success 'check utf-8 contents with passthrough strategy' '
 test_expect_success 'check latin-1 contents corrupted in git with passthrough strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -114,7 +116,7 @@ test_expect_success 'check latin-1 contents corrupted in git with passthrough st
 test_expect_success 'check utf-8 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -126,7 +128,7 @@ test_expect_success 'check utf-8 contents with fallback strategy' '
 test_expect_success 'check latin-1 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -138,7 +140,7 @@ test_expect_success 'check latin-1 contents with fallback strategy' '
 test_expect_success 'check cp-1252 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -150,7 +152,7 @@ test_expect_success 'check cp-1252 contents with fallback strategy' '
 test_expect_success 'check cp850 contents parsed with correct fallback' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback -c git-p4.metadataFallbackEncoding=cp850 p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback -c git-p4.metadataFallbackEncoding=cp850 p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -162,7 +164,7 @@ test_expect_success 'check cp850 contents parsed with correct fallback' '
 test_expect_success 'check cp850-only contents escaped when cp1252 is fallback' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -174,7 +176,7 @@ test_expect_success 'check cp850-only contents escaped when cp1252 is fallback'
 test_expect_success 'check cp-1252 contents on later sync after clone with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$cli" &&
 		P4USER=cp1252_author &&
@@ -186,7 +188,7 @@ test_expect_success 'check cp-1252 contents on later sync after clone with fallb
 	(
 		cd "$git" &&
 
-		git p4.py sync --branch=master &&
+		git p4-python2 sync --branch=master &&
 
 		git log p4/master >actual &&
 		grep "sœme more cp-1252 tæxt" actual &&
@@ -201,7 +203,7 @@ test_expect_success 'check cp-1252 contents on later sync after clone with fallb
 test_expect_success 'passthrough (latin-1 contents corrupted in git) is the default with python2' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
diff --git a/t/t9836-git-p4-metadata-encoding-python3.sh b/t/t9836-git-p4-metadata-encoding-python3.sh
index 63350dc4b5c6262480cd0be8fd88fba714c55428..5e5217a66b4fdb3c7fcf073a50952c7e9009e9fe 100755
--- a/t/t9836-git-p4-metadata-encoding-python3.sh
+++ b/t/t9836-git-p4-metadata-encoding-python3.sh
@@ -8,29 +8,29 @@ failing, and produces maximally sane output in git.'
 
 . ./lib-git-p4.sh
 
-python_target_version='3'
-
 ###############################
 ## SECTION REPEATED IN t9835 ##
 ###############################
 
-# Please note: this test calls "git-p4.py" rather than "git-p4", because the
-# latter references a specific path so we can't easily force it to run under
-# the python version we need to.
-
-python_major_version=$(python -V 2>&1 | cut -c  8)
-python_target_binary=$(which python$python_target_version)
-if ! test "$python_major_version" = "$python_target_version" && test "$python_target_binary"
+# These tests are specific to Python 3. Write a custom script that executes
+# git-p4 directly with the Python 3 interpreter to ensure that we use that
+# version even if Git was compiled with Python 2.
+python_target_binary=$(which python3)
+if test -n "$python_target_binary"
 then
 	mkdir temp_python
-	PATH="$(pwd)/temp_python:$PATH" && export PATH
-	ln -s $python_target_binary temp_python/python
+	PATH="$(pwd)/temp_python:$PATH"
+	export PATH
+
+	write_script temp_python/git-p4-python3 <<-EOF
+	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
+	EOF
 fi
 
-python_major_version=$(python -V 2>&1 | cut -c  8)
-if ! test "$python_major_version" = "$python_target_version"
+git p4-python3 >err
+if ! grep 'valid commands' err
 then
-	skip_all="skipping python$python_target_version-specific git p4 tests; python$python_target_version not available"
+	skip_all="skipping python3 git p4 tests; python3 not available"
 	test_done
 fi
 
@@ -81,14 +81,14 @@ test_expect_success 'init depot' '
 test_expect_success 'clone non-utf8 repo with strict encoding' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	test_must_fail git -c git-p4.metadataDecodingStrategy=strict p4.py clone --dest="$git" //depot@all 2>err &&
+	test_must_fail git -c git-p4.metadataDecodingStrategy=strict p4-python3 clone --dest="$git" //depot@all 2>err &&
 	grep "Decoding perforce metadata failed!" err
 '
 
 test_expect_success 'check utf-8 contents with passthrough strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -100,7 +100,7 @@ test_expect_success 'check utf-8 contents with passthrough strategy' '
 test_expect_success 'check latin-1 contents corrupted in git with passthrough strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -114,7 +114,7 @@ test_expect_success 'check latin-1 contents corrupted in git with passthrough st
 test_expect_success 'check utf-8 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -126,7 +126,7 @@ test_expect_success 'check utf-8 contents with fallback strategy' '
 test_expect_success 'check latin-1 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -138,7 +138,7 @@ test_expect_success 'check latin-1 contents with fallback strategy' '
 test_expect_success 'check cp-1252 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -150,7 +150,7 @@ test_expect_success 'check cp-1252 contents with fallback strategy' '
 test_expect_success 'check cp850 contents parsed with correct fallback' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback -c git-p4.metadataFallbackEncoding=cp850 p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback -c git-p4.metadataFallbackEncoding=cp850 p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -162,7 +162,7 @@ test_expect_success 'check cp850 contents parsed with correct fallback' '
 test_expect_success 'check cp850-only contents escaped when cp1252 is fallback' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -174,7 +174,7 @@ test_expect_success 'check cp850-only contents escaped when cp1252 is fallback'
 test_expect_success 'check cp-1252 contents on later sync after clone with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$cli" &&
 		P4USER=cp1252_author &&
@@ -186,7 +186,7 @@ test_expect_success 'check cp-1252 contents on later sync after clone with fallb
 	(
 		cd "$git" &&
 
-		git p4.py sync --branch=master &&
+		git p4-python3 sync --branch=master &&
 
 		git log p4/master >actual &&
 		grep "sœme more cp-1252 tæxt" actual &&
@@ -202,7 +202,7 @@ test_expect_success 'check cp-1252 contents on later sync after clone with fallb
 test_expect_success 'fallback (both utf-8 and cp-1252 contents handled) is the default with python3' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git p4.py clone --dest="$git" //depot@all &&
+	git p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&

-- 
2.47.1.447.ga7e8429e30.dirty

Re: [PATCH 6/8] t: fix out-of-tree tests for some git-p4 tests

[karthik nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> Both t9835 and t9836 exercise git-p4, but one exercises Python 2 whereas
> the other one uses Python 3. These tests do not exercise "git p4", but
> instead they use "git p4.py" so that the unbuilt version of "git-p4.py"
> is used that has "#!/usr/bin/env python" as shebang instead of the
> replaced shebang.
>
> But "git-p4.py" is not in our PATH during out-of-tree builds, and thus
> we cannot locate "git-p4.py". The tests thus break with CMake and Meson.
>
> Fix this by instead manually setting up script wrappers that invoke the
> respective Python interpreter directly.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t9835-git-p4-metadata-encoding-python2.sh | 48 ++++++++++++++-------------
>  t/t9836-git-p4-metadata-encoding-python3.sh | 50 ++++++++++++++---------------
>  2 files changed, 50 insertions(+), 48 deletions(-)
>
> diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
> index 036bf79c6674f6f1f0d667c7270674168428ffee..02f9ec09053890a4d41b7dc95644066d6481bbb6 100755
> --- a/t/t9835-git-p4-metadata-encoding-python2.sh
> +++ b/t/t9835-git-p4-metadata-encoding-python2.sh
> @@ -14,23 +14,25 @@ python_target_version='2'
>  ## SECTION REPEATED IN t9836 ##
>  ###############################
>
> -# Please note: this test calls "git-p4.py" rather than "git-p4", because the
> -# latter references a specific path so we can't easily force it to run under
> -# the python version we need to.
> -
> -python_major_version=$(python -V 2>&1 | cut -c  8)
> -python_target_binary=$(which python$python_target_version)
> -if ! test "$python_major_version" = "$python_target_version" && test "$python_target_binary"
> +# These tests are specific to Python 2. Write a custom script that executes
> +# git-p4 directly with the Python 2 interpreter to ensure that we use that
> +# version even if Git was compiled with Python 3.
> +python_target_binary=$(which python2)
> +if test -n "$python_target_binary"
>  then
>  	mkdir temp_python
> -	PATH="$(pwd)/temp_python:$PATH" && export PATH
> -	ln -s $python_target_binary temp_python/python
> +	PATH="$(pwd)/temp_python:$PATH"
> +	export PATH
> +
> +	write_script temp_python/git-p4-python2 <<-EOF
> +	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
> +	EOF
>  fi
>

So if the python version (2 here), is available, we create a temp script
which will use that version. That script is then used in all the
commands below. Makes sense.

This is similarly replicated in `t9836` but with Python 3.

[snip]

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

Re: [PATCH 6/8] t: fix out-of-tree tests for some git-p4 tests

[Patrick Steinhardt]

On Thu, Dec 12, 2024 at 02:53:47AM -0800, karthik nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
> > index 036bf79c6674f6f1f0d667c7270674168428ffee..02f9ec09053890a4d41b7dc95644066d6481bbb6 100755
> > --- a/t/t9835-git-p4-metadata-encoding-python2.sh
> > +++ b/t/t9835-git-p4-metadata-encoding-python2.sh
> > @@ -14,23 +14,25 @@ python_target_version='2'
> >  ## SECTION REPEATED IN t9836 ##
> >  ###############################
> >
> > -# Please note: this test calls "git-p4.py" rather than "git-p4", because the
> > -# latter references a specific path so we can't easily force it to run under
> > -# the python version we need to.
> > -
> > -python_major_version=$(python -V 2>&1 | cut -c  8)
> > -python_target_binary=$(which python$python_target_version)
> > -if ! test "$python_major_version" = "$python_target_version" && test "$python_target_binary"
> > +# These tests are specific to Python 2. Write a custom script that executes
> > +# git-p4 directly with the Python 2 interpreter to ensure that we use that
> > +# version even if Git was compiled with Python 3.
> > +python_target_binary=$(which python2)
> > +if test -n "$python_target_binary"
> >  then
> >  	mkdir temp_python
> > -	PATH="$(pwd)/temp_python:$PATH" && export PATH
> > -	ln -s $python_target_binary temp_python/python
> > +	PATH="$(pwd)/temp_python:$PATH"
> > +	export PATH
> > +
> > +	write_script temp_python/git-p4-python2 <<-EOF
> > +	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
> > +	EOF
> >  fi
> >
> 
> So if the python version (2 here), is available, we create a temp script
> which will use that version. That script is then used in all the
> commands below. Makes sense.
> 
> This is similarly replicated in `t9836` but with Python 3.

Yeah. This whole setup is just plain awkward from my point of view.
Another option would be to accept reality and stop supporting Python 2
altogether. But doing that as part of this patch series did not feel
like a good idea to me.

Thanks for your review!

Patrick

Re: [PATCH 6/8] t: fix out-of-tree tests for some git-p4 tests

[Toon Claes]

Patrick Steinhardt <ps@pks.im> writes:

> Both t9835 and t9836 exercise git-p4, but one exercises Python 2 whereas
> the other one uses Python 3. These tests do not exercise "git p4", but
> instead they use "git p4.py" so that the unbuilt version of "git-p4.py"
> is used that has "#!/usr/bin/env python" as shebang instead of the
> replaced shebang.

It took me a while to figure out what you mean by "the replaced
shebang"? I think you mean something like:

    These tests do not exercise "git p4", but instead they use "git
    p4.py". This calls the unbuilt version of "git-p4.py", which has the
    "#!/usr/bin/env python" shebang. This allows the test to modify
    which `python` comes first in $PATH, making it possible to force a
    Python version.

> But "git-p4.py" is not in our PATH during out-of-tree builds, and thus
> we cannot locate "git-p4.py". The tests thus break with CMake and Meson.
>
> Fix this by instead manually setting up script wrappers that invoke the
> respective Python interpreter directly.

I like this approach, way more explicit now the Python version is in the
command itself.

>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t9835-git-p4-metadata-encoding-python2.sh | 48 ++++++++++++++-------------
>  t/t9836-git-p4-metadata-encoding-python3.sh | 50 ++++++++++++++---------------
>  2 files changed, 50 insertions(+), 48 deletions(-)
>
> diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
> index 036bf79c6674f6f1f0d667c7270674168428ffee..02f9ec09053890a4d41b7dc95644066d6481bbb6 100755
> --- a/t/t9835-git-p4-metadata-encoding-python2.sh
> +++ b/t/t9835-git-p4-metadata-encoding-python2.sh
> @@ -14,23 +14,25 @@ python_target_version='2'
>  ## SECTION REPEATED IN t9836 ##

To be honest, I don't understand why this section wasn't put in a
function in lib-git-p4.sh in the first place, instead of duplicating?
Anyhow, I think for two test files it's fine to duplicate this code, and
after all you're not changing that.

But I've noticed you are no longer using `python_target_version`. I
would suggest to either remove the variable, or use it again so the code
between the two test files is identical again. Doing the latter would
probably mean you also need to create a variable like
`p4_python=p4-python$python_target_version` and use `$p4_python` instead
of `p4-python2` throughout the script, so I'm not sure that improves
things.

>  ###############################
>  
> -# Please note: this test calls "git-p4.py" rather than "git-p4", because the
> -# latter references a specific path so we can't easily force it to run under
> -# the python version we need to.
> -
> -python_major_version=$(python -V 2>&1 | cut -c  8)
> -python_target_binary=$(which python$python_target_version)
> -if ! test "$python_major_version" = "$python_target_version" && test "$python_target_binary"
> +# These tests are specific to Python 2. Write a custom script that executes
> +# git-p4 directly with the Python 2 interpreter to ensure that we use that
> +# version even if Git was compiled with Python 3.
> +python_target_binary=$(which python2)
> +if test -n "$python_target_binary"
>  then
>  	mkdir temp_python
> -	PATH="$(pwd)/temp_python:$PATH" && export PATH
> -	ln -s $python_target_binary temp_python/python
> +	PATH="$(pwd)/temp_python:$PATH"
> +	export PATH
> +
> +	write_script temp_python/git-p4-python2 <<-EOF
> +	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
> +	EOF
>  fi
>  
> -python_major_version=$(python -V 2>&1 | cut -c  8)
> -if ! test "$python_major_version" = "$python_target_version"
> +git p4-python2 >err
> +if ! grep 'valid commands' err

I like this sanity check, this verifies if the command actually works:

Thus the output when the script is properly created:

    usage: /home/toon/devel/git/git-p4 <command> [options]

    valid commands: branches, clone, sync, submit, unshelve, commit, rebase

    Try /home/toon/devel/git/git-p4 <command> --help for command specific help.


And when the script was not written:

    git: 'p4-python2' is not a git command. See 'git --help'.


I noticed though, the stderr output isn's shallowed into /dev/null,
resulting the output for the test to be the following if Python 2 is not found:

    make[2]: Entering directory '/home/toon/devel/git/t'
    *** t9835-git-p4-metadata-encoding-python2.sh ***
    which: no python2 in (/home/toon/devel/git/bin-wrappers:/home/toon/.local/bin:[snip])
    git: 'p4-python2' is not a git command. See 'git --help'.
    not ok 1 - start p4d


I think that's totally fine though, it's giving the user proper
information about what is wrong.

--
Toon

Re: [PATCH 6/8] t: fix out-of-tree tests for some git-p4 tests

[Patrick Steinhardt]

On Fri, Dec 13, 2024 at 11:00:23AM +0100, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
> > index 036bf79c6674f6f1f0d667c7270674168428ffee..02f9ec09053890a4d41b7dc95644066d6481bbb6 100755
> > --- a/t/t9835-git-p4-metadata-encoding-python2.sh
> > +++ b/t/t9835-git-p4-metadata-encoding-python2.sh
> > @@ -14,23 +14,25 @@ python_target_version='2'
> >  ## SECTION REPEATED IN t9836 ##
> 
> To be honest, I don't understand why this section wasn't put in a
> function in lib-git-p4.sh in the first place, instead of duplicating?
> Anyhow, I think for two test files it's fine to duplicate this code, and
> after all you're not changing that.
> 
> But I've noticed you are no longer using `python_target_version`. I
> would suggest to either remove the variable, or use it again so the code
> between the two test files is identical again. Doing the latter would
> probably mean you also need to create a variable like
> `p4_python=p4-python$python_target_version` and use `$p4_python` instead
> of `p4-python2` throughout the script, so I'm not sure that improves
> things.

Good catch. I did it in the Python 3 test, but forgot to do it here, as
well.

> >  ###############################
> >  
> > -# Please note: this test calls "git-p4.py" rather than "git-p4", because the
> > -# latter references a specific path so we can't easily force it to run under
> > -# the python version we need to.
> > -
> > -python_major_version=$(python -V 2>&1 | cut -c  8)
> > -python_target_binary=$(which python$python_target_version)
> > -if ! test "$python_major_version" = "$python_target_version" && test "$python_target_binary"
> > +# These tests are specific to Python 2. Write a custom script that executes
> > +# git-p4 directly with the Python 2 interpreter to ensure that we use that
> > +# version even if Git was compiled with Python 3.
> > +python_target_binary=$(which python2)
> > +if test -n "$python_target_binary"
> >  then
> >  	mkdir temp_python
> > -	PATH="$(pwd)/temp_python:$PATH" && export PATH
> > -	ln -s $python_target_binary temp_python/python
> > +	PATH="$(pwd)/temp_python:$PATH"
> > +	export PATH
> > +
> > +	write_script temp_python/git-p4-python2 <<-EOF
> > +	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
> > +	EOF
> >  fi
> >  
> > -python_major_version=$(python -V 2>&1 | cut -c  8)
> > -if ! test "$python_major_version" = "$python_target_version"
> > +git p4-python2 >err
> > +if ! grep 'valid commands' err
> 
> I like this sanity check, this verifies if the command actually works:
> 
> Thus the output when the script is properly created:
> 
>     usage: /home/toon/devel/git/git-p4 <command> [options]
> 
>     valid commands: branches, clone, sync, submit, unshelve, commit, rebase
> 
>     Try /home/toon/devel/git/git-p4 <command> --help for command specific help.
> 
> 
> And when the script was not written:
> 
>     git: 'p4-python2' is not a git command. See 'git --help'.
> 
> 
> I noticed though, the stderr output isn's shallowed into /dev/null,
> resulting the output for the test to be the following if Python 2 is not found:
> 
>     make[2]: Entering directory '/home/toon/devel/git/t'
>     *** t9835-git-p4-metadata-encoding-python2.sh ***
>     which: no python2 in (/home/toon/devel/git/bin-wrappers:/home/toon/.local/bin:[snip])
>     git: 'p4-python2' is not a git command. See 'git --help'.
>     not ok 1 - start p4d
> 
> 
> I think that's totally fine though, it's giving the user proper
> information about what is wrong.

Yeah, I actually consider it a win to have it. Not that anybody ever
executes these tests outside of CI anyway.

Patrick

[PATCH 7/8] t: introduce compatibility options to clar-based tests

[Patrick Steinhardt]

Our unit tests that don't yet use the clar unit testing framework ignore
any option that they do not understand. It is thus fine to just pass
test options we set up globally to those unit tests as they are simply
ignored. This makes our life easier because we don't have to special
case those options with Meson, where test options are set up globally
via `meson test --test-args=`.

But our clar-based unit testing framework is way stricter here and will
fail in case it is passed an unknown option. Stub out these options with
no-ops to make our life a bit easier.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 parse-options.h          | 12 ++++++++++++
 t/unit-tests/unit-test.c | 19 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index f0801d4532a175b65783689f2a68fb56da2c8e87..d01361ca97fd7227a0005b5c447d954fea472ca0 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -353,6 +353,18 @@ struct option {
 	.callback = parse_opt_noop_cb, \
 }
 
+static char *parse_options_noop_ignored_value MAYBE_UNUSED;
+#define OPT_NOOP_ARG(s, l) { \
+	.type = OPTION_CALLBACK, \
+	.short_name = (s), \
+	.long_name = (l), \
+	.value = &parse_options_noop_ignored_value, \
+	.argh = "ignored", \
+	.help = N_("no-op (backward compatibility)"), \
+	.flags = PARSE_OPT_HIDDEN, \
+	.callback = parse_opt_noop_cb, \
+}
+
 #define OPT_ALIAS(s, l, source_long_name) { \
 	.type = OPTION_ALIAS, \
 	.short_name = (s), \
diff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c
index a474cdcfd351d9d624178a769329252237f951b7..fa8818842a42478c7a8fa6f6ecbee0777bdf472f 100644
--- a/t/unit-tests/unit-test.c
+++ b/t/unit-tests/unit-test.c
@@ -18,8 +18,25 @@ int cmd_main(int argc, const char **argv)
 			 N_("immediately exit upon the first failed test")),
 		OPT_STRING_LIST('r', "run", &run_args, N_("suite[::test]"),
 				N_("run only test suite or individual test <suite[::test]>")),
-		OPT_STRING_LIST('x', "exclude", &exclude_args, N_("suite"),
+		OPT_STRING_LIST(0, "exclude", &exclude_args, N_("suite"),
 				N_("exclude test suite <suite>")),
+		/*
+		 * Compatibility wrappers so that we don't have to filter
+		 * options understood by integration tests.
+		 */
+		OPT_NOOP_NOARG('d', "debug"),
+		OPT_NOOP_NOARG(0, "github-workflow-markup"),
+		OPT_NOOP_NOARG(0, "no-bin-wrappers"),
+		OPT_NOOP_ARG(0, "root"),
+		OPT_NOOP_ARG(0, "stress"),
+		OPT_NOOP_NOARG(0, "tee"),
+		OPT_NOOP_NOARG(0, "with-dashes"),
+		OPT_NOOP_ARG(0, "valgrind"),
+		OPT_NOOP_ARG(0, "valgrind-only"),
+		OPT_NOOP_NOARG('v', "verbose"),
+		OPT_NOOP_NOARG('V', "verbose-log"),
+		OPT_NOOP_ARG(0, "verbose-only"),
+		OPT_NOOP_NOARG('x', NULL),
 		OPT_END(),
 	};
 	struct strvec args = STRVEC_INIT;

-- 
2.47.1.447.ga7e8429e30.dirty

Re: [PATCH 7/8] t: introduce compatibility options to clar-based tests

[Toon Claes]

Patrick Steinhardt <ps@pks.im> writes:

> Our unit tests that don't yet use the clar unit testing framework ignore
> any option that they do not understand. It is thus fine to just pass
> test options we set up globally to those unit tests as they are simply
> ignored. This makes our life easier because we don't have to special
> case those options with Meson, where test options are set up globally
> via `meson test --test-args=`.
>
> But our clar-based unit testing framework is way stricter here and will
> fail in case it is passed an unknown option. Stub out these options with
> no-ops to make our life a bit easier.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  parse-options.h          | 12 ++++++++++++
>  t/unit-tests/unit-test.c | 19 ++++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options.h b/parse-options.h
> index f0801d4532a175b65783689f2a68fb56da2c8e87..d01361ca97fd7227a0005b5c447d954fea472ca0 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -353,6 +353,18 @@ struct option {
>  	.callback = parse_opt_noop_cb, \
>  }
>  
> +static char *parse_options_noop_ignored_value MAYBE_UNUSED;
> +#define OPT_NOOP_ARG(s, l) { \
> +	.type = OPTION_CALLBACK, \
> +	.short_name = (s), \
> +	.long_name = (l), \
> +	.value = &parse_options_noop_ignored_value, \
> +	.argh = "ignored", \
> +	.help = N_("no-op (backward compatibility)"), \
> +	.flags = PARSE_OPT_HIDDEN, \
> +	.callback = parse_opt_noop_cb, \
> +}
> +
>  #define OPT_ALIAS(s, l, source_long_name) { \
>  	.type = OPTION_ALIAS, \
>  	.short_name = (s), \
> diff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c
> index a474cdcfd351d9d624178a769329252237f951b7..fa8818842a42478c7a8fa6f6ecbee0777bdf472f 100644
> --- a/t/unit-tests/unit-test.c
> +++ b/t/unit-tests/unit-test.c
> @@ -18,8 +18,25 @@ int cmd_main(int argc, const char **argv)
>  			 N_("immediately exit upon the first failed test")),
>  		OPT_STRING_LIST('r', "run", &run_args, N_("suite[::test]"),
>  				N_("run only test suite or individual test <suite[::test]>")),
> -		OPT_STRING_LIST('x', "exclude", &exclude_args, N_("suite"),
> +		OPT_STRING_LIST(0, "exclude", &exclude_args, N_("suite"),

Could you please explain in the commit message why it's safe to unassign
`-x` from `--exclude`?


-- 
Toon

[PATCH 8/8] ci: wire up Meson builds

[Patrick Steinhardt]

Wire up CI builds for both GitLab and GitHub that use the Meson build
system.

While the setup is mostly trivial, one gotcha is the test output
directory used to be in "t/", but now it is contained in the build
directory. To unify the logic across Makefile- and Meson-based builds we
explicitly set up the `TEST_OUTPUT_DIRECTORY` variable so that it is the
same for both build systems.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml |  7 +++++++
 .gitlab-ci.yml             |  9 +++++++++
 ci/install-dependencies.sh |  7 +++++++
 ci/lib.sh                  |  2 +-
 ci/print-test-failures.sh  |  2 +-
 ci/run-build-and-tests.sh  | 31 ++++++++++++++++++++++++-------
 6 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 808ddc19b8a799abc414c6d6ba078a6e5be6bdfb..c231419abc670fb0bd096c2dce63fd1b66ab14b7 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -286,6 +286,9 @@ jobs:
           - jobname: osx-gcc
             cc: gcc-13
             pool: macos-13
+          - jobname: osx-meson
+            cc: clang
+            pool: macos-13
           - jobname: linux-gcc-default
             cc: gcc
             pool: ubuntu-latest
@@ -298,11 +301,15 @@ jobs:
           - jobname: linux-asan-ubsan
             cc: clang
             pool: ubuntu-latest
+          - jobname: linux-meson
+            cc: gcc
+            pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
       CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
       distro: ${{matrix.vector.pool}}
+      TEST_OUTPUT_DIRECTORY: ${{github.workspace}}/t
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v4
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index a1bc92893f27d6dd404133686b71c8061e55618c..8163aacc8c8715d09f19bd1cc7199532fb5141e2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -20,6 +20,7 @@ test:linux:
     - saas-linux-medium-amd64
   variables:
     CUSTOM_PATH: "/custom"
+    TEST_OUTPUT_DIRECTORY: "/tmp/test-output"
   before_script:
     - ./ci/install-dependencies.sh
   script:
@@ -31,6 +32,7 @@ test:linux:
       if test "$CI_JOB_STATUS" != 'success'
       then
         sudo --preserve-env --set-home --user=builder ./ci/print-test-failures.sh
+        mv "$TEST_OUTPUT_DIRECTORY"/failed-test-artifacts t/
       fi
   parallel:
     matrix:
@@ -67,6 +69,10 @@ test:linux:
         image: fedora:latest
       - jobname: linux-musl
         image: alpine:latest
+      - jobname: linux-meson
+        image: ubuntu:latest
+        CC: gcc
+        CC_PACKAGE: gcc
   artifacts:
     paths:
       - t/failed-test-artifacts
@@ -104,6 +110,9 @@ test:osx:
       - jobname: osx-reftable
         image: macos-13-xcode-14
         CC: clang
+      - jobname: osx-meson
+        image: macos-14-xcode-15
+        CC: clang
   artifacts:
     paths:
       - t/failed-test-artifacts
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index d020cb7aa5ef64e8cb9e4c580064a84f4b3d1bfb..d1cb9fa8785388b3674fcea4dd682abc0725c968 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -58,6 +58,7 @@ ubuntu-*|ubuntu32-*|debian-*)
 		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo default-jre \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
 		libemail-valid-perl libio-pty-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
+		libpcre2-dev meson ninja-build pkg-config \
 		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
 
 	case "$distro" in
@@ -90,6 +91,12 @@ macos-*)
 	sudo xattr -d com.apple.quarantine "$CUSTOM_PATH/p4" "$CUSTOM_PATH/p4d" 2>/dev/null || true
 	rm helix-core-server.tgz
 
+	case "$jobname" in
+	osx-meson)
+		brew install meson ninja pcre2
+		;;
+	esac
+
 	if test -n "$CC_PACKAGE"
 	then
 		BREW_PACKAGE=${CC_PACKAGE/-/@}
diff --git a/ci/lib.sh b/ci/lib.sh
index 2e7a5f0540b66f24bd0f5744311c2c48b472d63d..b436f855414226df7f27a1b5ce95702f227d0c53 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -236,7 +236,7 @@ then
 	CC="${CC_PACKAGE:-${CC:-gcc}}"
 	DONT_SKIP_TAGS=t
 	handle_failed_tests () {
-		echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
+		echo "FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:-t}/failed-test-artifacts" >>$GITHUB_ENV
 		create_failed_test_artifacts
 		return 1
 	}
diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index b1f80aeac345dd70746b02b6ca1b5282a0c2a4aa..655687dd827e5b3e4d4879803b0d4499e7751380 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -46,7 +46,7 @@ do
 			;;
 		github-actions)
 			mkdir -p failed-test-artifacts
-			echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
+			echo "FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:t}/failed-test-artifacts" >>$GITHUB_ENV
 			cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
 			tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
 			continue
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index 2e28d02b20f2469afddc4e04fdbd18465babb1ef..c4a41bba0b84df57f6e60aeac2de29dbc0e27dc1 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -48,12 +48,29 @@ pedantic)
 	;;
 esac
 
-group Build make
-if test -n "$run_tests"
-then
-	group "Run tests" make test ||
-	handle_failed_tests
-fi
-check_unignored_build_artifacts
+case "$jobname" in
+*-meson)
+	group "Configure" meson setup build . \
+		--warnlevel 2 --werror \
+		--wrap-mode nofallback
+	group "Build" meson compile -C build --
+	if test -n "$run_tests"
+	then
+		group "Run tests" meson test -C build --print-errorlogs --test-args="$GIT_TEST_OPTS" || (
+			./t/aggregate-results.sh "${TEST_OUTPUT_DIRECTORY:-t}/test-results"
+			handle_failed_tests
+		)
+	fi
+	;;
+*)
+	group Build make
+	if test -n "$run_tests"
+	then
+		group "Run tests" make test ||
+		handle_failed_tests
+	fi
+	;;
+esac
 
+check_unignored_build_artifacts
 save_good_tree

-- 
2.47.1.447.ga7e8429e30.dirty

Re: [PATCH 8/8] ci: wire up Meson builds

[Toon Claes]

Patrick Steinhardt <ps@pks.im> writes:

[snip]

> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index a1bc92893f27d6dd404133686b71c8061e55618c..8163aacc8c8715d09f19bd1cc7199532fb5141e2 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -20,6 +20,7 @@ test:linux:
>      - saas-linux-medium-amd64
>    variables:
>      CUSTOM_PATH: "/custom"
> +    TEST_OUTPUT_DIRECTORY: "/tmp/test-output"
>    before_script:
>      - ./ci/install-dependencies.sh
>    script:
> @@ -31,6 +32,7 @@ test:linux:
>        if test "$CI_JOB_STATUS" != 'success'
>        then
>          sudo --preserve-env --set-home --user=builder ./ci/print-test-failures.sh
> +        mv "$TEST_OUTPUT_DIRECTORY"/failed-test-artifacts t/
>        fi
>    parallel:
>      matrix:
> @@ -67,6 +69,10 @@ test:linux:
>          image: fedora:latest
>        - jobname: linux-musl
>          image: alpine:latest
> +      - jobname: linux-meson
> +        image: ubuntu:latest
> +        CC: gcc
> +        CC_PACKAGE: gcc

Is it needed to explicitly set the CC_PACKAGE? I see in other places we
only set it when it's set to `gcc-8`?

-- 
Toon

Re: [PATCH 8/8] ci: wire up Meson builds

[Patrick Steinhardt]

On Fri, Dec 13, 2024 at 11:01:15AM +0100, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> [snip]
> 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index a1bc92893f27d6dd404133686b71c8061e55618c..8163aacc8c8715d09f19bd1cc7199532fb5141e2 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -20,6 +20,7 @@ test:linux:
> >      - saas-linux-medium-amd64
> >    variables:
> >      CUSTOM_PATH: "/custom"
> > +    TEST_OUTPUT_DIRECTORY: "/tmp/test-output"
> >    before_script:
> >      - ./ci/install-dependencies.sh
> >    script:
> > @@ -31,6 +32,7 @@ test:linux:
> >        if test "$CI_JOB_STATUS" != 'success'
> >        then
> >          sudo --preserve-env --set-home --user=builder ./ci/print-test-failures.sh
> > +        mv "$TEST_OUTPUT_DIRECTORY"/failed-test-artifacts t/
> >        fi
> >    parallel:
> >      matrix:
> > @@ -67,6 +69,10 @@ test:linux:
> >          image: fedora:latest
> >        - jobname: linux-musl
> >          image: alpine:latest
> > +      - jobname: linux-meson
> > +        image: ubuntu:latest
> > +        CC: gcc
> > +        CC_PACKAGE: gcc
> 
> Is it needed to explicitly set the CC_PACKAGE? I see in other places we
> only set it when it's set to `gcc-8`?

Good point, it's not necessary.

Thanks for your review!

Patrick

[PATCH v2 0/8] ci: wire up support for Meson

[Patrick Steinhardt]

Hi,

this small patch series wires up Meson into our CI systems. The intent
is to ensure that it does not regress in functionality as the Git code
base evolves.

To help with keeping it up-to-date, both Meson and our Makefiles learn
to detect missing or superseded test files in "t/meson.build". This
should give users an early notification in case they have to add a newly
added or removed test to these build instructions. Overall I think that
this shouldn't be too much of a burden given that adding a new test is
trivial.

One gap that still exists is newly added code files. Due to many sources
being added to the build conditionally it's hard to have generic checks
for these. So I refrain from doing so in this series -- the build would
already fail anyway when we're missing code, so at least we will know
that something is up.

Changes in v2:

  - Improve a couple of commit messages.
  - Explain why we remove `-x` from the unit-test driver.
  - Remove unneeded `CC_PACKAGE` variable.
  - Improve error messages when tests weren't found.
  - Link to v1: https://lore.kernel.org/r/20241211-pks-meson-ci-v1-0-28d18b494374@pks.im

The series is built on top of caacdb5dfd (The fifteenth batch,
2024-12-10) with ps/build at 904339edbd (Introduce support for the Meson
build system, 2024-12-06) and cw/worktree-extension at 2037ca85ad
(worktree: refactor `repair_worktree_after_gitdir_move()`, 2024-11-29)
merged into it.

Thanks!

Patrick

---
Patrick Steinhardt (8):
      ci/lib: support custom output directories when creating test artifacts
      Makefile: drop -DSUPPRESS_ANNOTATED_LEAKS
      t/unit-tests: rename clar-based unit tests to have a common prefix
      meson: detect missing tests at configure time
      Makefile: detect missing Meson tests
      t: fix out-of-tree tests for some git-p4 tests
      t: introduce compatibility options to clar-based tests
      ci: wire up Meson builds

 .github/workflows/main.yml                  |  7 ++++
 .gitlab-ci.yml                              |  8 +++++
 Makefile                                    |  5 ++-
 ci/install-dependencies.sh                  |  7 ++++
 ci/lib.sh                                   | 14 ++++----
 ci/print-test-failures.sh                   |  2 +-
 ci/run-build-and-tests.sh                   | 31 ++++++++++++++----
 meson.build                                 |  1 -
 parse-options.h                             | 12 +++++++
 t/Makefile                                  | 18 ++++++++++-
 t/meson.build                               | 40 +++++++++++++++++++++--
 t/t9835-git-p4-metadata-encoding-python2.sh | 50 ++++++++++++++---------------
 t/t9836-git-p4-metadata-encoding-python3.sh | 50 ++++++++++++++---------------
 t/unit-tests/generate-clar-decls.sh         |  5 ++-
 t/unit-tests/{ctype.c => u-ctype.c}         |  0
 t/unit-tests/{strvec.c => u-strvec.c}       |  0
 t/unit-tests/unit-test.c                    | 19 ++++++++++-
 17 files changed, 195 insertions(+), 74 deletions(-)

Range-diff versus v1:

1:  329404a926 = 1:  43562781da ci/lib: support custom output directories when creating test artifacts
2:  71a8cbc131 = 2:  f1655e9338 Makefile: drop -DSUPPRESS_ANNOTATED_LEAKS
3:  29e75b59e5 ! 3:  f5403a7ce0 t/unit-tests: rename clar-based unit tests to have a common prefix
    @@ Commit message
         t/unit-tests: rename clar-based unit tests to have a common prefix
     
         All of the code files for unit tests using the self-grown unit testing
    -    framework have have a "t-" prefix to their name. This makes it easy to
    +    framework have a "t-" prefix to their name. This makes it easy to
         identify them and use globbing in our Makefile and in other places. On
         the other hand though, our clar-based unit tests have no prefix at all
         and thus cannot easily be discerned from other files in the unit test
4:  644903d148 ! 4:  99a4ffdc2a meson: detect missing tests at configure time
    @@ t/meson.build: integration_tests = [
     +      endif
     +    endforeach
     +    if missing_tests.length() > 0
    -+      error('Missing tests:\n\n - ' + '\n - '.join(missing_tests))
    ++      error('Test files found, but not configured:\n\n - ' + '\n - '.join(missing_tests))
     +    endif
     +
     +    superfluous_tests = [ ]
    @@ t/meson.build: integration_tests = [
     +      endif
     +    endforeach
     +    if superfluous_tests.length() > 0
    -+      error('Superfluous tests:\n\n - ' + '\n - '.join(superfluous_tests))
    ++      error('Test files configured, but not found:\n\n - ' + '\n - '.join(superfluous_tests))
     +    endif
     +  endif
     +endforeach
5:  afad056353 = 5:  ce27e9c1a6 Makefile: detect missing Meson tests
6:  e398d80f77 ! 6:  8c46997ba4 t: fix out-of-tree tests for some git-p4 tests
    @@ Commit message
     
         Both t9835 and t9836 exercise git-p4, but one exercises Python 2 whereas
         the other one uses Python 3. These tests do not exercise "git p4", but
    -    instead they use "git p4.py" so that the unbuilt version of "git-p4.py"
    -    is used that has "#!/usr/bin/env python" as shebang instead of the
    -    replaced shebang.
    +    instead they use "git p4.py". This calls the unbuilt version of
    +    "git-p4.py" that still has the "#!/usr/bin/env python" shebang, which
    +    allows the test to modify which Python version comes first in $PATH,
    +    making it possible to force a Python version.
     
         But "git-p4.py" is not in our PATH during out-of-tree builds, and thus
         we cannot locate "git-p4.py". The tests thus break with CMake and Meson.
    @@ Commit message
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## t/t9835-git-p4-metadata-encoding-python2.sh ##
    -@@ t/t9835-git-p4-metadata-encoding-python2.sh: python_target_version='2'
    +@@ t/t9835-git-p4-metadata-encoding-python2.sh: failing, and produces maximally sane output in git.'
    + 
    + . ./lib-git-p4.sh
    + 
    +-python_target_version='2'
    +-
    + ###############################
      ## SECTION REPEATED IN t9836 ##
      ###############################
      
7:  efa4698fee ! 7:  357f8e7e67 t: introduce compatibility options to clar-based tests
    @@ Commit message
         fail in case it is passed an unknown option. Stub out these options with
         no-ops to make our life a bit easier.
     
    +    Note that this also requires us to remove the `-x` short option for
    +    `--exclude`. This is because `-x` has another meaning in our integration
    +    tests, as it enables shell tracing. I doubt there are a lot of people
    +    out there using it as we only got a small hand full of clar tests in the
    +    first place. So better change it now so that we can in the long run
    +    improve compatibility between the two different test drivers.
    +
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## parse-options.h ##
8:  d6f7685eca ! 8:  c1c27341eb ci: wire up Meson builds
    @@ .gitlab-ci.yml: test:linux:
     +      - jobname: linux-meson
     +        image: ubuntu:latest
     +        CC: gcc
    -+        CC_PACKAGE: gcc
        artifacts:
          paths:
            - t/failed-test-artifacts

---
base-commit: 2fcbf72f13e8ce3bf1cda9a689f392f8f6e5c65d
change-id: 20241129-pks-meson-ci-ba1e40652fbe

[PATCH v2 1/8] ci/lib: support custom output directories when creating test artifacts

[Patrick Steinhardt]

Update `create_failed_test_artifacts ()` so that it can handle arbitrary
test output directories. This fixes creation of these artifacts for
macOS on GitLab CI, which uses a separate output directory already. This
will also be used by our out-of-tree builds with Meson.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/lib.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index 930f98d7228166c37c236beb062b14675fb68ef3..2e7a5f0540b66f24bd0f5744311c2c48b472d63d 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -180,9 +180,9 @@ handle_failed_tests () {
 }
 
 create_failed_test_artifacts () {
-	mkdir -p t/failed-test-artifacts
+	mkdir -p "${TEST_OUTPUT_DIRECTORY:-t}"/failed-test-artifacts
 
-	for test_exit in t/test-results/*.exit
+	for test_exit in "${TEST_OUTPUT_DIRECTORY:-t}"/test-results/*.exit
 	do
 		test 0 != "$(cat "$test_exit")" || continue
 
@@ -191,11 +191,11 @@ create_failed_test_artifacts () {
 		printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
 		echo "The full logs are in the 'print test failures' step below."
 		echo "See also the 'failed-tests-*' artifacts attached to this run."
-		cat "t/test-results/$test_name.markup"
+		cat "${TEST_OUTPUT_DIRECTORY:-t}/test-results/$test_name.markup"
 
-		trash_dir="t/trash directory.$test_name"
-		cp "t/test-results/$test_name.out" t/failed-test-artifacts/
-		tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
+		trash_dir="${TEST_OUTPUT_DIRECTORY:-t}/trash directory.$test_name"
+		cp "${TEST_OUTPUT_DIRECTORY:-t}/test-results/$test_name.out" "${TEST_OUTPUT_DIRECTORY:-t}"/failed-test-artifacts/
+		tar czf "${TEST_OUTPUT_DIRECTORY:-t}/failed-test-artifacts/$test_name.trash.tar.gz" "$trash_dir"
 	done
 }
 

-- 
2.47.1.668.gf74b3f243a.dirty

[PATCH v2 2/8] Makefile: drop -DSUPPRESS_ANNOTATED_LEAKS

[Patrick Steinhardt]

The -DSUPPRESS_ANNOTATED_LEAKS preprocessor directive was used to enable
our `UNLEAK()` macro in the past, which marks memory as still-reachable
so that the leak sanitizer does not complain. Starting with 52c7dbd036
(git-compat-util: drop now-unused `UNLEAK()` macro, 2024-11-20) this
macro has been removed, and thus the preprocessor directive is not
required anymore, either.

Drop it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile    | 1 -
 meson.build | 1 -
 2 files changed, 2 deletions(-)

diff --git a/Makefile b/Makefile
index 06f01149ecf399ae4bb1932188a007948d767283..2506f3b7e3377ab1a376338c86a727b2ae92a6e9 100644
--- a/Makefile
+++ b/Makefile
@@ -1490,7 +1490,6 @@ ifneq ($(filter undefined,$(SANITIZERS)),)
 BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS
 endif
 ifneq ($(filter leak,$(SANITIZERS)),)
-BASIC_CFLAGS += -DSUPPRESS_ANNOTATED_LEAKS
 BASIC_CFLAGS += -O0
 SANITIZE_LEAK = YesCompiledWithIt
 endif
diff --git a/meson.build b/meson.build
index 0dccebcdf16b07650d943e53643f0e09e2975cc9..1af25af5cc1e718a4e50fb14274a36f811506219 100644
--- a/meson.build
+++ b/meson.build
@@ -712,7 +712,6 @@ else
   build_options_config.set('SANITIZE_ADDRESS', '')
 endif
 if get_option('b_sanitize').contains('leak')
-  libgit_c_args += '-DSUPPRESS_ANNOTATED_LEAKS'
   build_options_config.set('SANITIZE_LEAK', 'YesCompiledWithIt')
 else
   build_options_config.set('SANITIZE_LEAK', '')

-- 
2.47.1.668.gf74b3f243a.dirty

[PATCH v2 3/8] t/unit-tests: rename clar-based unit tests to have a common prefix

[Patrick Steinhardt]

All of the code files for unit tests using the self-grown unit testing
framework have a "t-" prefix to their name. This makes it easy to
identify them and use globbing in our Makefile and in other places. On
the other hand though, our clar-based unit tests have no prefix at all
and thus cannot easily be discerned from other files in the unit test
directory.

Introduce a new "u-" prefix for clar-based unit tests. This prefix will
be used in a subsequent commit to easily identify such tests.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile                              | 4 ++--
 t/meson.build                         | 4 ++--
 t/unit-tests/generate-clar-decls.sh   | 5 ++++-
 t/unit-tests/{ctype.c => u-ctype.c}   | 0
 t/unit-tests/{strvec.c => u-strvec.c} | 0
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 2506f3b7e3377ab1a376338c86a727b2ae92a6e9..6eafaf174aaa380ad8e6a86f75d003eb6c058fb3 100644
--- a/Makefile
+++ b/Makefile
@@ -1344,8 +1344,8 @@ THIRD_PARTY_SOURCES += sha1dc/%
 THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%
 THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%
 
-CLAR_TEST_SUITES += ctype
-CLAR_TEST_SUITES += strvec
+CLAR_TEST_SUITES += u-ctype
+CLAR_TEST_SUITES += u-strvec
 CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)
 CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))
 CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o
diff --git a/t/meson.build b/t/meson.build
index 13fe854ba0a18f9b83dbc48651f581198042ffd3..9e676e69363ed6311426500d98fe281e30d26bcb 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1,6 +1,6 @@
 clar_test_suites = [
-  'unit-tests/ctype.c',
-  'unit-tests/strvec.c',
+  'unit-tests/u-ctype.c',
+  'unit-tests/u-strvec.c',
 ]
 
 clar_sources = [
diff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh
index 688e0885f4f28182c3afe19c067b6d59dcacccfc..3b315c64b3711bfccc5941852a0782e02cee82f0 100755
--- a/t/unit-tests/generate-clar-decls.sh
+++ b/t/unit-tests/generate-clar-decls.sh
@@ -11,6 +11,9 @@ shift
 
 for suite in "$@"
 do
-	sed -ne "s/^\(void test_$(basename "${suite%.c}")__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$/extern \1;/p" "$suite" ||
+	suite_name=$(basename "$suite")
+	suite_name=${suite_name%.c}
+	suite_name=${suite_name#u-}
+	sed -ne "s/^\(void test_${suite_name}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$/extern \1;/p" "$suite" ||
 	exit 1
 done >"$OUTPUT"
diff --git a/t/unit-tests/ctype.c b/t/unit-tests/u-ctype.c
similarity index 100%
rename from t/unit-tests/ctype.c
rename to t/unit-tests/u-ctype.c
diff --git a/t/unit-tests/strvec.c b/t/unit-tests/u-strvec.c
similarity index 100%
rename from t/unit-tests/strvec.c
rename to t/unit-tests/u-strvec.c

-- 
2.47.1.668.gf74b3f243a.dirty

[PATCH v2 4/8] meson: detect missing tests at configure time

[Patrick Steinhardt]

It is quite easy for the list of integration tests to go out-of-sync
without anybody noticing. Introduce a new configure-time check that
verifies that all tests are wired up properly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/meson.build | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/t/meson.build b/t/meson.build
index 9e676e69363ed6311426500d98fe281e30d26bcb..602ebfe6a260211eb2083e5808058852ca86e7a0 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1092,6 +1092,42 @@ integration_tests = [
   't9903-bash-prompt.sh',
 ]
 
+# Sanity check that we are not missing any tests present in 't/'. This check
+# only runs once at configure time and is thus best-effort, only. It is
+# sufficient to catch missing test suites in our CI though.
+foreach glob, tests : {
+  't[0-9][0-9][0-9][0-9]-*.sh': integration_tests,
+  'unit-tests/t-*.c': unit_test_programs,
+  'unit-tests/u-*.c': clar_test_suites,
+}
+  actual_tests = run_command(shell, '-c', 'ls ' + glob,
+    check: true,
+    env: script_environment,
+  ).stdout().strip().split('\n')
+
+  if tests != actual_tests
+    missing_tests = [ ]
+    foreach actual_test : actual_tests
+      if actual_test not in tests
+        missing_tests += actual_test
+      endif
+    endforeach
+    if missing_tests.length() > 0
+      error('Test files found, but not configured:\n\n - ' + '\n - '.join(missing_tests))
+    endif
+
+    superfluous_tests = [ ]
+    foreach integration_test : tests
+      if integration_test not in actual_tests
+        superfluous_tests += integration_test
+      endif
+    endforeach
+    if superfluous_tests.length() > 0
+      error('Test files configured, but not found:\n\n - ' + '\n - '.join(superfluous_tests))
+    endif
+  endif
+endforeach
+
 # GIT_BUILD_DIR needs to be Unix-style without drive prefixes as it get added
 # to the PATH variable. And given that drive prefixes contain a colon we'd
 # otherwise end up with a broken PATH if we didn't convert it.

-- 
2.47.1.668.gf74b3f243a.dirty

[PATCH v2 5/8] Makefile: detect missing Meson tests

[Patrick Steinhardt]

In the preceding commit, we have introduced consistency checks to Meson
to detect any discrepancies with missing or extraneous tests in its
build instructions. These checks only get executed in Meson though, so
any users of our Makefiles wouldn't be alerted of the fact that they
have to modify the Meson build instructions in case they add or remove
any tests.

Add a comparable test target to our Makefile to plug this gap.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/Makefile | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/t/Makefile b/t/Makefile
index 131ffd778fe00864fae1f5750269615556c6cdea..290fb03ff011d39c31c5073c796aa6f4dc966283 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -59,7 +59,7 @@ CHAINLINTSUPPRESS = GIT_TEST_EXT_CHAIN_LINT=0 && export GIT_TEST_EXT_CHAIN_LINT
 
 all:: $(DEFAULT_TEST_TARGET)
 
-test: pre-clean check-chainlint $(TEST_LINT)
+test: pre-clean check-chainlint check-meson $(TEST_LINT)
 	$(CHAINLINTSUPPRESS) $(MAKE) aggregate-results-and-cleanup
 
 failed:
@@ -114,6 +114,22 @@ check-chainlint:
 	{ $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests >'$(CHAINLINTTMP_SQ)'/actual || true; } && \
 	diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
 
+check-meson:
+	@# awk acts up when trying to match single quotes, so we use \047 instead.
+	@printf "%s\n" \
+		"integration_tests t[0-9][0-9][0-9][0-9]-*.sh" \
+		"unit_test_programs unit-tests/t-*.c" \
+		"clar_test_suites unit-tests/u-*.c" | \
+	while read -r variable pattern; do \
+		meson_tests=$$(awk "/^$$variable = \[\$$/ {flag=1 ; next } /^]$$/ { flag=0 } flag { gsub(/^  \047/, \"\"); gsub(/\047,\$$/, \"\"); print }" meson.build) && \
+		actual_tests=$$(ls $$pattern) && \
+		if test "$$meson_tests" != "$$actual_tests"; then \
+			echo "Meson tests differ from actual tests:"; \
+			diff -u <(echo "$$meson_tests") <(echo "$$actual_tests"); \
+			exit 1; \
+		fi; \
+	done
+
 test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
 	test-lint-filenames
 ifneq ($(GIT_TEST_CHAIN_LINT),0)

-- 
2.47.1.668.gf74b3f243a.dirty

[PATCH v2 6/8] t: fix out-of-tree tests for some git-p4 tests

[Patrick Steinhardt]

Both t9835 and t9836 exercise git-p4, but one exercises Python 2 whereas
the other one uses Python 3. These tests do not exercise "git p4", but
instead they use "git p4.py". This calls the unbuilt version of
"git-p4.py" that still has the "#!/usr/bin/env python" shebang, which
allows the test to modify which Python version comes first in $PATH,
making it possible to force a Python version.

But "git-p4.py" is not in our PATH during out-of-tree builds, and thus
we cannot locate "git-p4.py". The tests thus break with CMake and Meson.

Fix this by instead manually setting up script wrappers that invoke the
respective Python interpreter directly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t9835-git-p4-metadata-encoding-python2.sh | 50 ++++++++++++++---------------
 t/t9836-git-p4-metadata-encoding-python3.sh | 50 ++++++++++++++---------------
 2 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/t/t9835-git-p4-metadata-encoding-python2.sh b/t/t9835-git-p4-metadata-encoding-python2.sh
index 036bf79c6674f6f1f0d667c7270674168428ffee..6116f806f631157aca7de6dbd33dcd94bbefb8aa 100755
--- a/t/t9835-git-p4-metadata-encoding-python2.sh
+++ b/t/t9835-git-p4-metadata-encoding-python2.sh
@@ -8,29 +8,29 @@ failing, and produces maximally sane output in git.'
 
 . ./lib-git-p4.sh
 
-python_target_version='2'
-
 ###############################
 ## SECTION REPEATED IN t9836 ##
 ###############################
 
-# Please note: this test calls "git-p4.py" rather than "git-p4", because the
-# latter references a specific path so we can't easily force it to run under
-# the python version we need to.
-
-python_major_version=$(python -V 2>&1 | cut -c  8)
-python_target_binary=$(which python$python_target_version)
-if ! test "$python_major_version" = "$python_target_version" && test "$python_target_binary"
+# These tests are specific to Python 2. Write a custom script that executes
+# git-p4 directly with the Python 2 interpreter to ensure that we use that
+# version even if Git was compiled with Python 3.
+python_target_binary=$(which python2)
+if test -n "$python_target_binary"
 then
 	mkdir temp_python
-	PATH="$(pwd)/temp_python:$PATH" && export PATH
-	ln -s $python_target_binary temp_python/python
+	PATH="$(pwd)/temp_python:$PATH"
+	export PATH
+
+	write_script temp_python/git-p4-python2 <<-EOF
+	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
+	EOF
 fi
 
-python_major_version=$(python -V 2>&1 | cut -c  8)
-if ! test "$python_major_version" = "$python_target_version"
+git p4-python2 >err
+if ! grep 'valid commands' err
 then
-	skip_all="skipping python$python_target_version-specific git p4 tests; python$python_target_version not available"
+	skip_all="skipping python2 git p4 tests; python2 not available"
 	test_done
 fi
 
@@ -81,14 +81,14 @@ test_expect_success 'init depot' '
 test_expect_success 'clone non-utf8 repo with strict encoding' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	test_must_fail git -c git-p4.metadataDecodingStrategy=strict p4.py clone --dest="$git" //depot@all 2>err &&
+	test_must_fail git -c git-p4.metadataDecodingStrategy=strict p4-python2 clone --dest="$git" //depot@all 2>err &&
 	grep "Decoding perforce metadata failed!" err
 '
 
 test_expect_success 'check utf-8 contents with passthrough strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -100,7 +100,7 @@ test_expect_success 'check utf-8 contents with passthrough strategy' '
 test_expect_success 'check latin-1 contents corrupted in git with passthrough strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -114,7 +114,7 @@ test_expect_success 'check latin-1 contents corrupted in git with passthrough st
 test_expect_success 'check utf-8 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -126,7 +126,7 @@ test_expect_success 'check utf-8 contents with fallback strategy' '
 test_expect_success 'check latin-1 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -138,7 +138,7 @@ test_expect_success 'check latin-1 contents with fallback strategy' '
 test_expect_success 'check cp-1252 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -150,7 +150,7 @@ test_expect_success 'check cp-1252 contents with fallback strategy' '
 test_expect_success 'check cp850 contents parsed with correct fallback' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback -c git-p4.metadataFallbackEncoding=cp850 p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback -c git-p4.metadataFallbackEncoding=cp850 p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -162,7 +162,7 @@ test_expect_success 'check cp850 contents parsed with correct fallback' '
 test_expect_success 'check cp850-only contents escaped when cp1252 is fallback' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -174,7 +174,7 @@ test_expect_success 'check cp850-only contents escaped when cp1252 is fallback'
 test_expect_success 'check cp-1252 contents on later sync after clone with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$cli" &&
 		P4USER=cp1252_author &&
@@ -186,7 +186,7 @@ test_expect_success 'check cp-1252 contents on later sync after clone with fallb
 	(
 		cd "$git" &&
 
-		git p4.py sync --branch=master &&
+		git p4-python2 sync --branch=master &&
 
 		git log p4/master >actual &&
 		grep "sœme more cp-1252 tæxt" actual &&
@@ -201,7 +201,7 @@ test_expect_success 'check cp-1252 contents on later sync after clone with fallb
 test_expect_success 'passthrough (latin-1 contents corrupted in git) is the default with python2' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python2 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
diff --git a/t/t9836-git-p4-metadata-encoding-python3.sh b/t/t9836-git-p4-metadata-encoding-python3.sh
index 63350dc4b5c6262480cd0be8fd88fba714c55428..5e5217a66b4fdb3c7fcf073a50952c7e9009e9fe 100755
--- a/t/t9836-git-p4-metadata-encoding-python3.sh
+++ b/t/t9836-git-p4-metadata-encoding-python3.sh
@@ -8,29 +8,29 @@ failing, and produces maximally sane output in git.'
 
 . ./lib-git-p4.sh
 
-python_target_version='3'
-
 ###############################
 ## SECTION REPEATED IN t9835 ##
 ###############################
 
-# Please note: this test calls "git-p4.py" rather than "git-p4", because the
-# latter references a specific path so we can't easily force it to run under
-# the python version we need to.
-
-python_major_version=$(python -V 2>&1 | cut -c  8)
-python_target_binary=$(which python$python_target_version)
-if ! test "$python_major_version" = "$python_target_version" && test "$python_target_binary"
+# These tests are specific to Python 3. Write a custom script that executes
+# git-p4 directly with the Python 3 interpreter to ensure that we use that
+# version even if Git was compiled with Python 2.
+python_target_binary=$(which python3)
+if test -n "$python_target_binary"
 then
 	mkdir temp_python
-	PATH="$(pwd)/temp_python:$PATH" && export PATH
-	ln -s $python_target_binary temp_python/python
+	PATH="$(pwd)/temp_python:$PATH"
+	export PATH
+
+	write_script temp_python/git-p4-python3 <<-EOF
+	exec "$python_target_binary" "$(git --exec-path)/git-p4" "\$@"
+	EOF
 fi
 
-python_major_version=$(python -V 2>&1 | cut -c  8)
-if ! test "$python_major_version" = "$python_target_version"
+git p4-python3 >err
+if ! grep 'valid commands' err
 then
-	skip_all="skipping python$python_target_version-specific git p4 tests; python$python_target_version not available"
+	skip_all="skipping python3 git p4 tests; python3 not available"
 	test_done
 fi
 
@@ -81,14 +81,14 @@ test_expect_success 'init depot' '
 test_expect_success 'clone non-utf8 repo with strict encoding' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	test_must_fail git -c git-p4.metadataDecodingStrategy=strict p4.py clone --dest="$git" //depot@all 2>err &&
+	test_must_fail git -c git-p4.metadataDecodingStrategy=strict p4-python3 clone --dest="$git" //depot@all 2>err &&
 	grep "Decoding perforce metadata failed!" err
 '
 
 test_expect_success 'check utf-8 contents with passthrough strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -100,7 +100,7 @@ test_expect_success 'check utf-8 contents with passthrough strategy' '
 test_expect_success 'check latin-1 contents corrupted in git with passthrough strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=passthrough p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=passthrough p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -114,7 +114,7 @@ test_expect_success 'check latin-1 contents corrupted in git with passthrough st
 test_expect_success 'check utf-8 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -126,7 +126,7 @@ test_expect_success 'check utf-8 contents with fallback strategy' '
 test_expect_success 'check latin-1 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -138,7 +138,7 @@ test_expect_success 'check latin-1 contents with fallback strategy' '
 test_expect_success 'check cp-1252 contents with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -150,7 +150,7 @@ test_expect_success 'check cp-1252 contents with fallback strategy' '
 test_expect_success 'check cp850 contents parsed with correct fallback' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback -c git-p4.metadataFallbackEncoding=cp850 p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback -c git-p4.metadataFallbackEncoding=cp850 p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -162,7 +162,7 @@ test_expect_success 'check cp850 contents parsed with correct fallback' '
 test_expect_success 'check cp850-only contents escaped when cp1252 is fallback' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&
@@ -174,7 +174,7 @@ test_expect_success 'check cp850-only contents escaped when cp1252 is fallback'
 test_expect_success 'check cp-1252 contents on later sync after clone with fallback strategy' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git -c git-p4.metadataDecodingStrategy=fallback p4.py clone --dest="$git" //depot@all &&
+	git -c git-p4.metadataDecodingStrategy=fallback p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$cli" &&
 		P4USER=cp1252_author &&
@@ -186,7 +186,7 @@ test_expect_success 'check cp-1252 contents on later sync after clone with fallb
 	(
 		cd "$git" &&
 
-		git p4.py sync --branch=master &&
+		git p4-python3 sync --branch=master &&
 
 		git log p4/master >actual &&
 		grep "sœme more cp-1252 tæxt" actual &&
@@ -202,7 +202,7 @@ test_expect_success 'check cp-1252 contents on later sync after clone with fallb
 test_expect_success 'fallback (both utf-8 and cp-1252 contents handled) is the default with python3' '
 	test_when_finished cleanup_git &&
 	test_when_finished remove_user_cache &&
-	git p4.py clone --dest="$git" //depot@all &&
+	git p4-python3 clone --dest="$git" //depot@all &&
 	(
 		cd "$git" &&
 		git log >actual &&

-- 
2.47.1.668.gf74b3f243a.dirty

[PATCH v2 7/8] t: introduce compatibility options to clar-based tests

[Patrick Steinhardt]

Our unit tests that don't yet use the clar unit testing framework ignore
any option that they do not understand. It is thus fine to just pass
test options we set up globally to those unit tests as they are simply
ignored. This makes our life easier because we don't have to special
case those options with Meson, where test options are set up globally
via `meson test --test-args=`.

But our clar-based unit testing framework is way stricter here and will
fail in case it is passed an unknown option. Stub out these options with
no-ops to make our life a bit easier.

Note that this also requires us to remove the `-x` short option for
`--exclude`. This is because `-x` has another meaning in our integration
tests, as it enables shell tracing. I doubt there are a lot of people
out there using it as we only got a small hand full of clar tests in the
first place. So better change it now so that we can in the long run
improve compatibility between the two different test drivers.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 parse-options.h          | 12 ++++++++++++
 t/unit-tests/unit-test.c | 19 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index f0801d4532a175b65783689f2a68fb56da2c8e87..d01361ca97fd7227a0005b5c447d954fea472ca0 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -353,6 +353,18 @@ struct option {
 	.callback = parse_opt_noop_cb, \
 }
 
+static char *parse_options_noop_ignored_value MAYBE_UNUSED;
+#define OPT_NOOP_ARG(s, l) { \
+	.type = OPTION_CALLBACK, \
+	.short_name = (s), \
+	.long_name = (l), \
+	.value = &parse_options_noop_ignored_value, \
+	.argh = "ignored", \
+	.help = N_("no-op (backward compatibility)"), \
+	.flags = PARSE_OPT_HIDDEN, \
+	.callback = parse_opt_noop_cb, \
+}
+
 #define OPT_ALIAS(s, l, source_long_name) { \
 	.type = OPTION_ALIAS, \
 	.short_name = (s), \
diff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c
index a474cdcfd351d9d624178a769329252237f951b7..fa8818842a42478c7a8fa6f6ecbee0777bdf472f 100644
--- a/t/unit-tests/unit-test.c
+++ b/t/unit-tests/unit-test.c
@@ -18,8 +18,25 @@ int cmd_main(int argc, const char **argv)
 			 N_("immediately exit upon the first failed test")),
 		OPT_STRING_LIST('r', "run", &run_args, N_("suite[::test]"),
 				N_("run only test suite or individual test <suite[::test]>")),
-		OPT_STRING_LIST('x', "exclude", &exclude_args, N_("suite"),
+		OPT_STRING_LIST(0, "exclude", &exclude_args, N_("suite"),
 				N_("exclude test suite <suite>")),
+		/*
+		 * Compatibility wrappers so that we don't have to filter
+		 * options understood by integration tests.
+		 */
+		OPT_NOOP_NOARG('d', "debug"),
+		OPT_NOOP_NOARG(0, "github-workflow-markup"),
+		OPT_NOOP_NOARG(0, "no-bin-wrappers"),
+		OPT_NOOP_ARG(0, "root"),
+		OPT_NOOP_ARG(0, "stress"),
+		OPT_NOOP_NOARG(0, "tee"),
+		OPT_NOOP_NOARG(0, "with-dashes"),
+		OPT_NOOP_ARG(0, "valgrind"),
+		OPT_NOOP_ARG(0, "valgrind-only"),
+		OPT_NOOP_NOARG('v', "verbose"),
+		OPT_NOOP_NOARG('V', "verbose-log"),
+		OPT_NOOP_ARG(0, "verbose-only"),
+		OPT_NOOP_NOARG('x', NULL),
 		OPT_END(),
 	};
 	struct strvec args = STRVEC_INIT;

-- 
2.47.1.668.gf74b3f243a.dirty

Re: [PATCH v2 7/8] t: introduce compatibility options to clar-based tests

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> Note that this also requires us to remove the `-x` short option for
> `--exclude`. This is because `-x` has another meaning in our integration
> tests, as it enables shell tracing. I doubt there are a lot of people
> out there using it as we only got a small hand full of clar tests in the
> first place. So better change it now so that we can in the long run
> improve compatibility between the two different test drivers.

Yup, even though we always hate to see a change that forces people
to adjust to, this is a developer thing, and it certainly is better
to do such a change, if we must, earlier.

Thanks.

[PATCH v2 8/8] ci: wire up Meson builds

[Patrick Steinhardt]

Wire up CI builds for both GitLab and GitHub that use the Meson build
system.

While the setup is mostly trivial, one gotcha is the test output
directory used to be in "t/", but now it is contained in the build
directory. To unify the logic across Makefile- and Meson-based builds we
explicitly set up the `TEST_OUTPUT_DIRECTORY` variable so that it is the
same for both build systems.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml |  7 +++++++
 .gitlab-ci.yml             |  8 ++++++++
 ci/install-dependencies.sh |  7 +++++++
 ci/lib.sh                  |  2 +-
 ci/print-test-failures.sh  |  2 +-
 ci/run-build-and-tests.sh  | 31 ++++++++++++++++++++++++-------
 6 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 808ddc19b8a799abc414c6d6ba078a6e5be6bdfb..c231419abc670fb0bd096c2dce63fd1b66ab14b7 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -286,6 +286,9 @@ jobs:
           - jobname: osx-gcc
             cc: gcc-13
             pool: macos-13
+          - jobname: osx-meson
+            cc: clang
+            pool: macos-13
           - jobname: linux-gcc-default
             cc: gcc
             pool: ubuntu-latest
@@ -298,11 +301,15 @@ jobs:
           - jobname: linux-asan-ubsan
             cc: clang
             pool: ubuntu-latest
+          - jobname: linux-meson
+            cc: gcc
+            pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
       CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
       distro: ${{matrix.vector.pool}}
+      TEST_OUTPUT_DIRECTORY: ${{github.workspace}}/t
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v4
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index a1bc92893f27d6dd404133686b71c8061e55618c..3eec72ddc666f593521058b5f38eb6220f42ce0f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -20,6 +20,7 @@ test:linux:
     - saas-linux-medium-amd64
   variables:
     CUSTOM_PATH: "/custom"
+    TEST_OUTPUT_DIRECTORY: "/tmp/test-output"
   before_script:
     - ./ci/install-dependencies.sh
   script:
@@ -31,6 +32,7 @@ test:linux:
       if test "$CI_JOB_STATUS" != 'success'
       then
         sudo --preserve-env --set-home --user=builder ./ci/print-test-failures.sh
+        mv "$TEST_OUTPUT_DIRECTORY"/failed-test-artifacts t/
       fi
   parallel:
     matrix:
@@ -67,6 +69,9 @@ test:linux:
         image: fedora:latest
       - jobname: linux-musl
         image: alpine:latest
+      - jobname: linux-meson
+        image: ubuntu:latest
+        CC: gcc
   artifacts:
     paths:
       - t/failed-test-artifacts
@@ -104,6 +109,9 @@ test:osx:
       - jobname: osx-reftable
         image: macos-13-xcode-14
         CC: clang
+      - jobname: osx-meson
+        image: macos-14-xcode-15
+        CC: clang
   artifacts:
     paths:
       - t/failed-test-artifacts
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index d020cb7aa5ef64e8cb9e4c580064a84f4b3d1bfb..d1cb9fa8785388b3674fcea4dd682abc0725c968 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -58,6 +58,7 @@ ubuntu-*|ubuntu32-*|debian-*)
 		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo default-jre \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
 		libemail-valid-perl libio-pty-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
+		libpcre2-dev meson ninja-build pkg-config \
 		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
 
 	case "$distro" in
@@ -90,6 +91,12 @@ macos-*)
 	sudo xattr -d com.apple.quarantine "$CUSTOM_PATH/p4" "$CUSTOM_PATH/p4d" 2>/dev/null || true
 	rm helix-core-server.tgz
 
+	case "$jobname" in
+	osx-meson)
+		brew install meson ninja pcre2
+		;;
+	esac
+
 	if test -n "$CC_PACKAGE"
 	then
 		BREW_PACKAGE=${CC_PACKAGE/-/@}
diff --git a/ci/lib.sh b/ci/lib.sh
index 2e7a5f0540b66f24bd0f5744311c2c48b472d63d..b436f855414226df7f27a1b5ce95702f227d0c53 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -236,7 +236,7 @@ then
 	CC="${CC_PACKAGE:-${CC:-gcc}}"
 	DONT_SKIP_TAGS=t
 	handle_failed_tests () {
-		echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
+		echo "FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:-t}/failed-test-artifacts" >>$GITHUB_ENV
 		create_failed_test_artifacts
 		return 1
 	}
diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index b1f80aeac345dd70746b02b6ca1b5282a0c2a4aa..655687dd827e5b3e4d4879803b0d4499e7751380 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -46,7 +46,7 @@ do
 			;;
 		github-actions)
 			mkdir -p failed-test-artifacts
-			echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
+			echo "FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:t}/failed-test-artifacts" >>$GITHUB_ENV
 			cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
 			tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
 			continue
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index 2e28d02b20f2469afddc4e04fdbd18465babb1ef..c4a41bba0b84df57f6e60aeac2de29dbc0e27dc1 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -48,12 +48,29 @@ pedantic)
 	;;
 esac
 
-group Build make
-if test -n "$run_tests"
-then
-	group "Run tests" make test ||
-	handle_failed_tests
-fi
-check_unignored_build_artifacts
+case "$jobname" in
+*-meson)
+	group "Configure" meson setup build . \
+		--warnlevel 2 --werror \
+		--wrap-mode nofallback
+	group "Build" meson compile -C build --
+	if test -n "$run_tests"
+	then
+		group "Run tests" meson test -C build --print-errorlogs --test-args="$GIT_TEST_OPTS" || (
+			./t/aggregate-results.sh "${TEST_OUTPUT_DIRECTORY:-t}/test-results"
+			handle_failed_tests
+		)
+	fi
+	;;
+*)
+	group Build make
+	if test -n "$run_tests"
+	then
+		group "Run tests" make test ||
+		handle_failed_tests
+	fi
+	;;
+esac
 
+check_unignored_build_artifacts
 save_good_tree

-- 
2.47.1.668.gf74b3f243a.dirty

[PATCH] ci: allow github-actions print test failures again

[Junio C Hamano]

eab5dbab (ci: wire up Meson builds, 2024-12-13) added two instances
of a very similar construct

    FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:-t}/failed-test-artifacts

one to ci/lib.sh and the other to ci/print-test-failures.sh
Unfortunately, the latter had a typo causing shell to emit "Bad
substitution".  Fix it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ci/print-test-failures.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index dc910e5160..5545e77c13 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -41,7 +41,7 @@ do
 		case "$CI_TYPE" in
 		github-actions)
 			mkdir -p failed-test-artifacts
-			echo "FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:t}/failed-test-artifacts" >>$GITHUB_ENV
+			echo "FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:-t}/failed-test-artifacts" >>$GITHUB_ENV
 			cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
 			tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
 			continue
-- 
2.50.1-447-g6271d3ee0b

Re: [PATCH] ci: allow github-actions print test failures again

[Patrick Steinhardt]

On Wed, Jul 16, 2025 at 02:07:54PM -0700, Junio C Hamano wrote:
> eab5dbab (ci: wire up Meson builds, 2024-12-13) added two instances
> of a very similar construct
> 
>     FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:-t}/failed-test-artifacts
> 
> one to ci/lib.sh and the other to ci/print-test-failures.sh
> Unfortunately, the latter had a typo causing shell to emit "Bad
> substitution".  Fix it.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  ci/print-test-failures.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
> index dc910e5160..5545e77c13 100755
> --- a/ci/print-test-failures.sh
> +++ b/ci/print-test-failures.sh
> @@ -41,7 +41,7 @@ do
>  		case "$CI_TYPE" in
>  		github-actions)
>  			mkdir -p failed-test-artifacts
> -			echo "FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:t}/failed-test-artifacts" >>$GITHUB_ENV
> +			echo "FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:-t}/failed-test-artifacts" >>$GITHUB_ENV
>  			cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
>  			tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
>  			continue

Oh, indeed. Thanks for catching the mistake, the patch looks obviously
good to me.

Patrick

Re: [PATCH v2 0/8] ci: wire up support for Meson

[Toon Claes]

Patrick Steinhardt <ps@pks.im> writes:

> Changes in v2:
>
>   - Improve a couple of commit messages.
>   - Explain why we remove `-x` from the unit-test driver.
>   - Remove unneeded `CC_PACKAGE` variable.
>   - Improve error messages when tests weren't found.
>   - Link to v1: https://lore.kernel.org/r/20241211-pks-meson-ci-v1-0-28d18b494374@pks.im
>
> The series is built on top of caacdb5dfd (The fifteenth batch,
> 2024-12-10) with ps/build at 904339edbd (Introduce support for the Meson
> build system, 2024-12-06) and cw/worktree-extension at 2037ca85ad
> (worktree: refactor `repair_worktree_after_gitdir_move()`, 2024-11-29)
> merged into it.
>
> Thanks!
>
> Patrick

I went through the range-diff and looked at the individual patches, and
I've got no more comments.

-- 
Toon

Re: [PATCH v2 0/8] ci: wire up support for Meson

[Junio C Hamano]

Toon Claes <toon@iotcl.com> writes:

> Patrick Steinhardt <ps@pks.im> writes:
>
>> Changes in v2:
>>
>>   - Improve a couple of commit messages.
>>   - Explain why we remove `-x` from the unit-test driver.
>>   - Remove unneeded `CC_PACKAGE` variable.
>>   - Improve error messages when tests weren't found.
>>   - Link to v1: https://lore.kernel.org/r/20241211-pks-meson-ci-v1-0-28d18b494374@pks.im
>>
>> The series is built on top of caacdb5dfd (The fifteenth batch,
>> 2024-12-10) with ps/build at 904339edbd (Introduce support for the Meson
>> build system, 2024-12-06) and cw/worktree-extension at 2037ca85ad
>> (worktree: refactor `repair_worktree_after_gitdir_move()`, 2024-11-29)
>> merged into it.
>>
>> Thanks!
>>
>> Patrick
>
> I went through the range-diff and looked at the individual patches, and
> I've got no more comments.

Thanks, both.  Let's mark the topic for 'next'.