{"thread":{"id":"64958","subject":"[PATCH 0/5] Some assorted fixes for GitLab CI","startedAt":"2026-02-09T16:56:26Z","lastAt":"2026-02-11T06:33:10Z","messageCount":13,"participants":["Patrick Steinhardt","Justin Tobler","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"535564","messageId":"20260209-b4-pks-ci-meson-improvements-v1-0-38444dec4874@pks.im","threadId":"64958","inReplyTo":null,"subject":"[PATCH 0/5] Some assorted fixes for GitLab CI","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-09T16:56:10Z","receivedAt":"2026-02-09T16:56:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nI recently had the pleasure of debugging a couple of failing\nMSVC+Windows jobs in GitLab CI, which hasn't been quite fun because we\ndidn't know to print error logs, and neither did we upload the failed\ntest artifacts. This patch series is the result of this frustration and\nfixes a couple of smaller issues in the context of our CI:\n\n  - I noticed that test slicing is slightly wrong because of a\n    difference between zero- and one-based indices, which causes us to\n    skip the first test on GitLab.\n\n  - I deduplicated how we run Meson tests so that both GitLab and GitHub\n    use the same \"run-test-slice-meson.sh\" script.\n\n  - I add logic to handle failing tests via \"print-test-failures.sh\".\n\nThe result can be found at [1]. Note that tests are failing, but those\nfailures are fixed in a separate patch series via [2]. In any case, I\nguess those test failures also serve as a good demonstration how the\nfailing tests show up now.\n\nThanks!\n\nPatrick\n\n[1]: https://gitlab.com/gitlab-org/git/-/merge_requests/497\n[2]: <20260209-b4-pks-ci-msvc-iconv-fixes-v1-0-1e3167cd8828@pks.im>\n\n---\nPatrick Steinhardt (5):\n      ci: handle failures of test-slice helper\n      ci: don't skip smallest test slice in GitLab\n      ci: make test slicing consistent across Meson/Make\n      gitlab-ci: use \"run-test-slice-meson.sh\"\n      gitlab-ci: handle failed tests on MSVC+Meson job\n\n .github/workflows/main.yml |  4 ++--\n .gitlab-ci.yml             | 17 +++++++++++++++--\n ci/run-test-slice-meson.sh |  2 +-\n ci/run-test-slice.sh       |  6 +++---\n t/helper/test-path-utils.c | 18 ++++++++++++------\n 5 files changed, 33 insertions(+), 14 deletions(-)\n\n\n---\nbase-commit: 3e0db84c88c57e70ac8be8c196dfa92c5d656fbc\nchange-id: 20260209-b4-pks-ci-meson-improvements-93d8a1ffdd27\n\n"},{"id":"535565","messageId":"20260209-b4-pks-ci-meson-improvements-v1-1-38444dec4874@pks.im","threadId":"64958","inReplyTo":"20260209-b4-pks-ci-meson-improvements-v1-0-38444dec4874@pks.im","subject":"[PATCH 1/5] ci: handle failures of test-slice helper","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-09T16:56:11Z","receivedAt":"2026-02-09T16:56:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"run-test-slice.sh\" script executes the test helper to slice up\ntests passed to it. As the execution is part of a pipe though, we end up\nignoring any potential error code returned by the helper.\n\nMake the code more robust by storing the tests in a variable first so\nthat we can split up the pipeline.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/run-test-slice.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/ci/run-test-slice.sh b/ci/run-test-slice.sh\nindex 0444c79c02..ff948e397f 100755\n--- a/ci/run-test-slice.sh\n+++ b/ci/run-test-slice.sh\n@@ -5,9 +5,9 @@\n \n . ${0%/*}/lib.sh\n \n-group \"Run tests\" make --quiet -C t T=\"$(cd t &&\n-\t./helper/test-tool path-utils slice-tests \"$1\" \"$2\" t[0-9]*.sh |\n-\ttr '\\n' ' ')\" ||\n+TESTS=$(cd t && ./helper/test-tool path-utils slice-tests \"$1\" \"$2\" t[0-9]*.sh)\n+\n+group \"Run tests\" make --quiet -C t T=\"$(echo \"$TESTS\" | tr '\\n' ' ')\" ||\n handle_failed_tests\n \n # We only have one unit test at the moment, so run it in the first slice\n\n-- \n2.53.0.295.g64333814d3.dirty\n\n"},{"id":"535566","messageId":"20260209-b4-pks-ci-meson-improvements-v1-2-38444dec4874@pks.im","threadId":"64958","inReplyTo":"20260209-b4-pks-ci-meson-improvements-v1-0-38444dec4874@pks.im","subject":"[PATCH 2/5] ci: don't skip smallest test slice in GitLab","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-09T16:56:12Z","receivedAt":"2026-02-09T16:56:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"ci/run-test-slice.sh\" script can be used to slice up all of our\ntests into N pieces and then run each of them on a separate CI job.\nThis is used by both GitLab and GitHub CI to speed up Windows tests,\nwhich would otherwise be painfully slow.\n\nThe infra itself is fueled by `test-tool path-utils slice-tests`. This\ntool receives as input an \"offset\" and a \"stride\" that can be combined\nto slice up tests. This framing can be misleading though: you are\nexpected to pass a zero-based index as \"offset\", and the complete number\nof slices to the \"stride\". The latter makes sense, but it is somewhat\nsurprising that the offset needs to be zero-based. And this is in fact\nbiting us: while GitHub passes zero-based indices, GitLab passes\n`$CI_NODE_INDEX`, which is a one-based indice.\n\nIdeally, we should have verification that the parameters make sense.\nAnd naturally, one would for example expect that it's an error to call\nthe binary with an offset larger than the stride. But with the current\nframing as \"offset\" it's not even wrong to do so, as it is of course\nwell-defined to start at a larger offset than the stride.\n\nThis means that we get this wrong on GitLab's CI, as we pass a one based\nindex there, and this causes us to skip one of the tests. Interestingly,\nit's not the lexicographically first test that we skip. Instead, as we\nsort tests by size before slicing them, we skip the _smallest_ test.\n\nReframe the problem to instead talk about \"slice number\" and \"total\nnumber of slices\". For all of our use cases this is semantically\nequivalent, but it allows us to perform some verifications:\n\n  - The total number of slices must be greater than 1.\n\n  - The selected slice must be between 1 <= nr <= slices_total.\n\nAs the indices are now one-based it means that GitLab's CI is fixed.\nThe GitHub workflow is updated accordingly.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .github/workflows/main.yml |  2 +-\n t/helper/test-path-utils.c | 18 ++++++++++++------\n 2 files changed, 13 insertions(+), 7 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex f2e93f5461..2b175dc5c6 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -150,7 +150,7 @@ jobs:\n     - uses: git-for-windows/setup-git-for-windows-sdk@v1\n     - name: test\n       shell: bash\n-      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n+      run: . /etc/profile && ci/run-test-slice.sh ${{ matrix.nr + 1 }} 10\n     - name: print test failures\n       if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n       shell: bash\ndiff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c\nindex f5f33751da..874542ec34 100644\n--- a/t/helper/test-path-utils.c\n+++ b/t/helper/test-path-utils.c\n@@ -477,14 +477,20 @@ int cmd__path_utils(int argc, const char **argv)\n \n \tif (argc > 5 && !strcmp(argv[1], \"slice-tests\")) {\n \t\tint res = 0;\n-\t\tlong offset, stride, i;\n+\t\tlong slice, slices_total, i;\n \t\tstruct string_list list = STRING_LIST_INIT_NODUP;\n \t\tstruct stat st;\n \n-\t\toffset = strtol(argv[2], NULL, 10);\n-\t\tstride = strtol(argv[3], NULL, 10);\n-\t\tif (stride < 1)\n-\t\t\tstride = 1;\n+\t\tslices_total = strtol(argv[3], NULL, 10);\n+\t\tif (slices_total < 1)\n+\t\t\tdie(\"there must be at least one slice, got '%s'\",\n+\t\t\t    argv[3]);\n+\n+\t\tslice = strtol(argv[2], NULL, 10);\n+\t\tif (1 > slice || slice > slices_total)\n+\t\t\tdie(\"slice must be in the range 1 <= slice <= %ld, got '%s'\",\n+\t\t\t    slices_total, argv[2]);\n+\n \t\tfor (i = 4; i < argc; i++)\n \t\t\tif (stat(argv[i], &st))\n \t\t\t\tres = error_errno(\"Cannot stat '%s'\", argv[i]);\n@@ -492,7 +498,7 @@ int cmd__path_utils(int argc, const char **argv)\n \t\t\t\tstring_list_append(&list, argv[i])->util =\n \t\t\t\t\t(void *)(intptr_t)st.st_size;\n \t\tQSORT(list.items, list.nr, cmp_by_st_size);\n-\t\tfor (i = offset; i < list.nr; i+= stride)\n+\t\tfor (i = slice - 1; i < list.nr; i+= slices_total)\n \t\t\tprintf(\"%s\\n\", list.items[i].string);\n \n \t\treturn !!res;\n\n-- \n2.53.0.295.g64333814d3.dirty\n\n"},{"id":"535567","messageId":"20260209-b4-pks-ci-meson-improvements-v1-3-38444dec4874@pks.im","threadId":"64958","inReplyTo":"20260209-b4-pks-ci-meson-improvements-v1-0-38444dec4874@pks.im","subject":"[PATCH 3/5] ci: make test slicing consistent across Meson/Make","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-09T16:56:13Z","receivedAt":"2026-02-09T16:56:34Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In the preceding commit we have adjusted test slicing to be one-based\nwhen using the \"ci/run-test-slice.sh\" script. But we also have an\nequivalent script for Meson that is still zero-based, which is of course\ninconsistent.\n\nAdapt the script to be one-based, as well, and adapt the GitHub workflow\naccordingly. Note that GitLab doesn't yet use the script, so it does not\nneed to be adapted. This will change in the next commit though.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .github/workflows/main.yml | 2 +-\n ci/run-test-slice-meson.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 2b175dc5c6..1b7a16e1f1 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -298,7 +298,7 @@ jobs:\n         path: build\n     - name: Test\n       shell: pwsh\n-      run: ci/run-test-slice-meson.sh build ${{matrix.nr}} 10\n+      run: ci/run-test-slice-meson.sh build ${{matrix.nr + 1}} 10\n     - name: print test failures\n       if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n       shell: bash\ndiff --git a/ci/run-test-slice-meson.sh b/ci/run-test-slice-meson.sh\nindex 961c94fba0..a6df927ba5 100755\n--- a/ci/run-test-slice-meson.sh\n+++ b/ci/run-test-slice-meson.sh\n@@ -9,5 +9,5 @@\n \n group \"Run tests\" \\\n \tmeson test -C \"$1\" --no-rebuild --print-errorlogs \\\n-\t\t--test-args=\"$GIT_TEST_OPTS\" --slice \"$((1+$2))/$3\" ||\n+\t\t--test-args=\"$GIT_TEST_OPTS\" --slice \"$(($2))/$3\" ||\n handle_failed_tests\n\n-- \n2.53.0.295.g64333814d3.dirty\n\n"},{"id":"535568","messageId":"20260209-b4-pks-ci-meson-improvements-v1-4-38444dec4874@pks.im","threadId":"64958","inReplyTo":"20260209-b4-pks-ci-meson-improvements-v1-0-38444dec4874@pks.im","subject":"[PATCH 4/5] gitlab-ci: use \"run-test-slice-meson.sh\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-09T16:56:14Z","receivedAt":"2026-02-09T16:56:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While our GitHub workflow already uses \"ci/run-test-slice-meson.sh\",\nGitLab CI open-codes the parameters. Adapt the latter to also use the\nsame script so that we always use the same Meson options across both CI\nsystems.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .gitlab-ci.yml | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex b419a84e2c..04857b479d 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -183,7 +183,8 @@ test:msvc-meson:\n     - job: \"build:msvc-meson\"\n       artifacts: true\n   script:\n-    - meson test -C build --no-rebuild --print-errorlogs --slice $Env:CI_NODE_INDEX/$Env:CI_NODE_TOTAL\n+    - |\n+      & \"C:/Program Files/Git/usr/bin/bash.exe\" -l -c 'ci/run-test-slice-meson.sh build $CI_NODE_INDEX $CI_NODE_TOTAL'\n   parallel: 10\n   artifacts:\n     reports:\n\n-- \n2.53.0.295.g64333814d3.dirty\n\n"},{"id":"535569","messageId":"20260209-b4-pks-ci-meson-improvements-v1-5-38444dec4874@pks.im","threadId":"64958","inReplyTo":"20260209-b4-pks-ci-meson-improvements-v1-0-38444dec4874@pks.im","subject":"[PATCH 5/5] gitlab-ci: handle failed tests on MSVC+Meson job","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-09T16:56:15Z","receivedAt":"2026-02-09T16:56:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The MSVC+Meson job does not currently have any logic to print failing\ntests, nor does it upload the failed test artifacts. Backfill this logic\nto make help debugging efforts in case any of its jobs has failed.\n\nGitHub already knows to do this, so we don't need an equivalent change\nover there.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .gitlab-ci.yml | 14 +++++++++++++-\n 1 file changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex 04857b479d..71b8a6e642 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -157,6 +157,8 @@ test:mingw64:\n   parallel: 10\n \n .msvc-meson:\n+  variables:\n+    TEST_OUTPUT_DIRECTORY: \"C:/Git-Test\"\n   tags:\n     - saas-windows-medium-amd64\n   before_script:\n@@ -164,12 +166,13 @@ test:mingw64:\n     - choco install -y git meson ninja rust-ms\n     - Import-Module $env:ChocolateyInstall\\helpers\\chocolateyProfile.psm1\n     - refreshenv\n+    - New-Item -Path $env:TEST_OUTPUT_DIRECTORY -ItemType Directory\n \n build:msvc-meson:\n   extends: .msvc-meson\n   stage: build\n   script:\n-    - meson setup build --vsenv -Dperl=disabled -Dbackend_max_links=1 -Dcredential_helpers=wincred\n+    - meson setup build --vsenv -Dperl=disabled -Dbackend_max_links=1 -Dcredential_helpers=wincred -Dtest_output_directory=\"$TEST_OUTPUT_DIRECTORY\"\n     - meson compile -C build\n   artifacts:\n     paths:\n@@ -185,10 +188,19 @@ test:msvc-meson:\n   script:\n     - |\n       & \"C:/Program Files/Git/usr/bin/bash.exe\" -l -c 'ci/run-test-slice-meson.sh build $CI_NODE_INDEX $CI_NODE_TOTAL'\n+  after_script:\n+    - |\n+      if ($env:CI_JOB_STATUS -ne \"success\") {\n+        & \"C:/Program Files/Git/usr/bin/bash.exe\" -l -c 'ci/print-test-failures.sh'\n+        Move-Item -Path \"$env:TEST_OUTPUT_DIRECTORY/failed-test-artifacts\" -Destination t/\n+      }\n   parallel: 10\n   artifacts:\n+    paths:\n+      - t/failed-test-artifacts\n     reports:\n       junit: build/meson-logs/testlog.junit.xml\n+    when: on_failure\n \n test:fuzz-smoke-tests:\n   image: ubuntu:latest\n\n-- \n2.53.0.295.g64333814d3.dirty\n\n"},{"id":"535584","messageId":"aYofxzIvnhv3arR8@denethor","threadId":"64958","inReplyTo":"20260209-b4-pks-ci-meson-improvements-v1-2-38444dec4874@pks.im","subject":"Re: [PATCH 2/5] ci: don't skip smallest test slice in GitLab","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-09T18:07:11Z","receivedAt":"2026-02-09T18:07:13Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/02/09 05:56PM, Patrick Steinhardt wrote:\n> The \"ci/run-test-slice.sh\" script can be used to slice up all of our\n> tests into N pieces and then run each of them on a separate CI job.\n> This is used by both GitLab and GitHub CI to speed up Windows tests,\n> which would otherwise be painfully slow.\n> \n> The infra itself is fueled by `test-tool path-utils slice-tests`. This\n> tool receives as input an \"offset\" and a \"stride\" that can be combined\n> to slice up tests. This framing can be misleading though: you are\n> expected to pass a zero-based index as \"offset\", and the complete number\n> of slices to the \"stride\". The latter makes sense, but it is somewhat\n> surprising that the offset needs to be zero-based. And this is in fact\n> biting us: while GitHub passes zero-based indices, GitLab passes\n> `$CI_NODE_INDEX`, which is a one-based indice.\n> \n> Ideally, we should have verification that the parameters make sense.\n> And naturally, one would for example expect that it's an error to call\n> the binary with an offset larger than the stride. But with the current\n> framing as \"offset\" it's not even wrong to do so, as it is of course\n> well-defined to start at a larger offset than the stride.\n\nIt was also suprising for me to see that the \"offset\" could be set to a\nvalue higher than the stride. I can't see any reason that we would want\nthis to be the case.\n\n> This means that we get this wrong on GitLab's CI, as we pass a one based\n> index there, and this causes us to skip one of the tests. Interestingly,\n> it's not the lexicographically first test that we skip. Instead, as we\n> sort tests by size before slicing them, we skip the _smallest_ test.\n> \n> Reframe the problem to instead talk about \"slice number\" and \"total\n> number of slices\". For all of our use cases this is semantically\n> equivalent, but it allows us to perform some verifications:\n> \n>   - The total number of slices must be greater than 1.\n> \n>   - The selected slice must be between 1 <= nr <= slices_total.\n\nThis seems reasonable to me.\n\n> As the indices are now one-based it means that GitLab's CI is fixed.\n> The GitHub workflow is updated accordingly.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  .github/workflows/main.yml |  2 +-\n>  t/helper/test-path-utils.c | 18 ++++++++++++------\n>  2 files changed, 13 insertions(+), 7 deletions(-)\n> \n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index f2e93f5461..2b175dc5c6 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -150,7 +150,7 @@ jobs:\n>      - uses: git-for-windows/setup-git-for-windows-sdk@v1\n>      - name: test\n>        shell: bash\n> -      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10\n> +      run: . /etc/profile && ci/run-test-slice.sh ${{ matrix.nr + 1 }} 10\n\nHere the GitHub CI is updated to be one-based indexed. The GitLab CI is\nalready set up that way.\n\n>      - name: print test failures\n>        if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n>        shell: bash\n> diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c\n> index f5f33751da..874542ec34 100644\n> --- a/t/helper/test-path-utils.c\n> +++ b/t/helper/test-path-utils.c\n> @@ -477,14 +477,20 @@ int cmd__path_utils(int argc, const char **argv)\n>  \n>  \tif (argc > 5 && !strcmp(argv[1], \"slice-tests\")) {\n>  \t\tint res = 0;\n> -\t\tlong offset, stride, i;\n> +\t\tlong slice, slices_total, i;\n>  \t\tstruct string_list list = STRING_LIST_INIT_NODUP;\n>  \t\tstruct stat st;\n>  \n> -\t\toffset = strtol(argv[2], NULL, 10);\n> -\t\tstride = strtol(argv[3], NULL, 10);\n> -\t\tif (stride < 1)\n> -\t\t\tstride = 1;\n> +\t\tslices_total = strtol(argv[3], NULL, 10);\n> +\t\tif (slices_total < 1)\n> +\t\t\tdie(\"there must be at least one slice, got '%s'\",\n> +\t\t\t    argv[3]);\n\nHere we validate the slices count is greater than one.\n\n> +\n> +\t\tslice = strtol(argv[2], NULL, 10);\n> +\t\tif (1 > slice || slice > slices_total)\n> +\t\t\tdie(\"slice must be in the range 1 <= slice <= %ld, got '%s'\",\n> +\t\t\t    slices_total, argv[2]);\n\nHere we validate the provided slice index is in the correct range.\n\n> +\n>  \t\tfor (i = 4; i < argc; i++)\n>  \t\t\tif (stat(argv[i], &st))\n>  \t\t\t\tres = error_errno(\"Cannot stat '%s'\", argv[i]);\n> @@ -492,7 +498,7 @@ int cmd__path_utils(int argc, const char **argv)\n>  \t\t\t\tstring_list_append(&list, argv[i])->util =\n>  \t\t\t\t\t(void *)(intptr_t)st.st_size;\n>  \t\tQSORT(list.items, list.nr, cmp_by_st_size);\n> -\t\tfor (i = offset; i < list.nr; i+= stride)\n> +\t\tfor (i = slice - 1; i < list.nr; i+= slices_total)\n>  \t\t\tprintf(\"%s\\n\", list.items[i].string);\n>  \n>  \t\treturn !!res;\n\nThis patch looks good.\n\n-Justin\n"},{"id":"535588","messageId":"aYojRnqBi8nzZhPD@denethor","threadId":"64958","inReplyTo":"20260209-b4-pks-ci-meson-improvements-v1-3-38444dec4874@pks.im","subject":"Re: [PATCH 3/5] ci: make test slicing consistent across Meson/Make","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-09T18:19:57Z","receivedAt":"2026-02-09T18:20:03Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/02/09 05:56PM, Patrick Steinhardt wrote:\n> In the preceding commit we have adjusted test slicing to be one-based\n> when using the \"ci/run-test-slice.sh\" script. But we also have an\n> equivalent script for Meson that is still zero-based, which is of course\n> inconsistent.\n> \n> Adapt the script to be one-based, as well, and adapt the GitHub workflow\n> accordingly. Note that GitLab doesn't yet use the script, so it does not\n> need to be adapted. This will change in the next commit though.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  .github/workflows/main.yml | 2 +-\n>  ci/run-test-slice-meson.sh | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 2b175dc5c6..1b7a16e1f1 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -298,7 +298,7 @@ jobs:\n>          path: build\n>      - name: Test\n>        shell: pwsh\n> -      run: ci/run-test-slice-meson.sh build ${{matrix.nr}} 10\n> +      run: ci/run-test-slice-meson.sh build ${{matrix.nr + 1}} 10\n\nDue to the changes in the prior patch, GitHub CI passing 0 as the slice\nvalue would cause a failure correct? I wonder if we should combine this\nchange with the previous patch. Otherwise this patch looks good.\n\n-Justin\n"},{"id":"535591","messageId":"aYolvOd4erKFhSUE@denethor","threadId":"64958","inReplyTo":"20260209-b4-pks-ci-meson-improvements-v1-5-38444dec4874@pks.im","subject":"Re: [PATCH 5/5] gitlab-ci: handle failed tests on MSVC+Meson job","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-09T18:33:50Z","receivedAt":"2026-02-09T18:33:54Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/02/09 05:56PM, Patrick Steinhardt wrote:\n> The MSVC+Meson job does not currently have any logic to print failing\n> tests, nor does it upload the failed test artifacts. Backfill this logic\n> to make help debugging efforts in case any of its jobs has failed.\n> \n> GitHub already knows to do this, so we don't need an equivalent change\n> over there.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  .gitlab-ci.yml | 14 +++++++++++++-\n>  1 file changed, 13 insertions(+), 1 deletion(-)\n> \n> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\n> index 04857b479d..71b8a6e642 100644\n> --- a/.gitlab-ci.yml\n> +++ b/.gitlab-ci.yml\n> @@ -157,6 +157,8 @@ test:mingw64:\n>    parallel: 10\n>  \n>  .msvc-meson:\n> +  variables:\n> +    TEST_OUTPUT_DIRECTORY: \"C:/Git-Test\"\n>    tags:\n>      - saas-windows-medium-amd64\n>    before_script:\n> @@ -164,12 +166,13 @@ test:mingw64:\n>      - choco install -y git meson ninja rust-ms\n>      - Import-Module $env:ChocolateyInstall\\helpers\\chocolateyProfile.psm1\n>      - refreshenv\n> +    - New-Item -Path $env:TEST_OUTPUT_DIRECTORY -ItemType Directory\n\nBefore the script starts we create the test output directory.\n\n>  build:msvc-meson:\n>    extends: .msvc-meson\n>    stage: build\n>    script:\n> -    - meson setup build --vsenv -Dperl=disabled -Dbackend_max_links=1 -Dcredential_helpers=wincred\n> +    - meson setup build --vsenv -Dperl=disabled -Dbackend_max_links=1 -Dcredential_helpers=wincred -Dtest_output_directory=\"$TEST_OUTPUT_DIRECTORY\"\n\nNow we set the test output directory build option accordingly.\n\n>      - meson compile -C build\n>    artifacts:\n>      paths:\n> @@ -185,10 +188,19 @@ test:msvc-meson:\n>    script:\n>      - |\n>        & \"C:/Program Files/Git/usr/bin/bash.exe\" -l -c 'ci/run-test-slice-meson.sh build $CI_NODE_INDEX $CI_NODE_TOTAL'\n> +  after_script:\n> +    - |\n> +      if ($env:CI_JOB_STATUS -ne \"success\") {\n> +        & \"C:/Program Files/Git/usr/bin/bash.exe\" -l -c 'ci/print-test-failures.sh'\n> +        Move-Item -Path \"$env:TEST_OUTPUT_DIRECTORY/failed-test-artifacts\" -Destination t/\n> +      }\n\nHere we print any failures and move them so they are stored as a CI\nartifact.\n\n>    parallel: 10\n>    artifacts:\n> +    paths:\n> +      - t/failed-test-artifacts\n>      reports:\n>        junit: build/meson-logs/testlog.junit.xml\n> +    when: on_failure\n\nThis patch also looks good.\n\n-Justin\n"},{"id":"535650","messageId":"aYrDXs9DMfBHi5jk@pks.im","threadId":"64958","inReplyTo":"aYojRnqBi8nzZhPD@denethor","subject":"Re: [PATCH 3/5] ci: make test slicing consistent across Meson/Make","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-10T05:34:22Z","receivedAt":"2026-02-10T05:34:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Feb 09, 2026 at 12:19:57PM -0600, Justin Tobler wrote:\n> On 26/02/09 05:56PM, Patrick Steinhardt wrote:\n> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> > index 2b175dc5c6..1b7a16e1f1 100644\n> > --- a/.github/workflows/main.yml\n> > +++ b/.github/workflows/main.yml\n> > @@ -298,7 +298,7 @@ jobs:\n> >          path: build\n> >      - name: Test\n> >        shell: pwsh\n> > -      run: ci/run-test-slice-meson.sh build ${{matrix.nr}} 10\n> > +      run: ci/run-test-slice-meson.sh build ${{matrix.nr + 1}} 10\n> \n> Due to the changes in the prior patch, GitHub CI passing 0 as the slice\n> value would cause a failure correct? I wonder if we should combine this\n> change with the previous patch. Otherwise this patch looks good.\n\nNote that this is the \"-meson.sh\" variant, so this is a different\nscript. I'm mostly just touching up this variant so that it behaves the\nsame as the non-Meson one.\n\nMeson itself would die though in case it's passed an invalid range.\n\nPatrick\n"},{"id":"535714","messageId":"xmqqa4xgxn2m.fsf@gitster.g","threadId":"64958","inReplyTo":"20260209-b4-pks-ci-meson-improvements-v1-3-38444dec4874@pks.im","subject":"Re: [PATCH 3/5] ci: make test slicing consistent across Meson/Make","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-10T22:15:13Z","receivedAt":"2026-02-10T22:15:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> In the preceding commit we have adjusted test slicing to be one-based\n> when using the \"ci/run-test-slice.sh\" script. But we also have an\n> equivalent script for Meson that is still zero-based, which is of course\n> inconsistent.\n>\n> Adapt the script to be one-based, as well, and adapt the GitHub workflow\n> accordingly. Note that GitLab doesn't yet use the script, so it does not\n> need to be adapted. This will change in the next commit though.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  .github/workflows/main.yml | 2 +-\n>  ci/run-test-slice-meson.sh | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 2b175dc5c6..1b7a16e1f1 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -298,7 +298,7 @@ jobs:\n>          path: build\n>      - name: Test\n>        shell: pwsh\n> -      run: ci/run-test-slice-meson.sh build ${{matrix.nr}} 10\n> +      run: ci/run-test-slice-meson.sh build ${{matrix.nr + 1}} 10\n>      - name: print test failures\n>        if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n>        shell: bash\n\nHave we successfully run this one?\n\nI am getting\n\nInvalid workflow file: .github/workflows/main.yml#L1\n(Line: 153, Col: 12): Unexpected symbol: '+'. Located at position 11\nwithin expression: matrix.nr + 1, (Line: 301, Col: 12): Unexpected\nsymbol: '+'. Located at position 11 within expression: matrix.nr + 1\n\nhttps://github.com/orgs/community/discussions/25386 is a 6-year old\ndiscussion so things may have changed quite a lot, but at least back\nthen the claim was\n\n    Github actions doesn’t support math operations in expressions\n    inside ${{ }}. You could add up these two numbers in bash script and\n    then use set-env command to give its value to an environment\n    variable ...\n\nthough.\n\nIn the meantime I'll revert the topic out of 'next'.  Sorry for not\ncatching it while it was in 'seen',.\n\n"},{"id":"535723","messageId":"20260210225401.GA1837188@coredump.intra.peff.net","threadId":"64958","inReplyTo":"xmqqa4xgxn2m.fsf@gitster.g","subject":"Re: [PATCH 3/5] ci: make test slicing consistent across Meson/Make","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-02-10T22:54:01Z","receivedAt":"2026-02-10T22:54:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2026 at 02:15:13PM -0800, Junio C Hamano wrote:\n\n> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> > index 2b175dc5c6..1b7a16e1f1 100644\n> > --- a/.github/workflows/main.yml\n> > +++ b/.github/workflows/main.yml\n> > @@ -298,7 +298,7 @@ jobs:\n> >          path: build\n> >      - name: Test\n> >        shell: pwsh\n> > -      run: ci/run-test-slice-meson.sh build ${{matrix.nr}} 10\n> > +      run: ci/run-test-slice-meson.sh build ${{matrix.nr + 1}} 10\n> >      - name: print test failures\n> >        if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n> >        shell: bash\n> \n> Have we successfully run this one?\n> \n> I am getting\n> \n> Invalid workflow file: .github/workflows/main.yml#L1\n> (Line: 153, Col: 12): Unexpected symbol: '+'. Located at position 11\n> within expression: matrix.nr + 1, (Line: 301, Col: 12): Unexpected\n> symbol: '+'. Located at position 11 within expression: matrix.nr + 1\n> \n> https://github.com/orgs/community/discussions/25386 is a 6-year old\n> discussion so things may have changed quite a lot, but at least back\n> then the claim was\n> \n>     Github actions doesn’t support math operations in expressions\n>     inside ${{ }}. You could add up these two numbers in bash script and\n>     then use set-env command to give its value to an environment\n>     variable ...\n> \n> though.\n\nRight, that's why we used pwsh syntax to do it before, in d3d6493dcf\n(ci: use Meson's new `--slice` option, 2025-07-09).\n\nThat went away in 17bd1108ea (ci(windows-meson-test): handle options and\noutput like other test jobs, 2025-11-18), because the \"+1\" was added\ninto the script itself there. It looks like the patch under discussion\nremoves the +1 from the script, so we'd need to go back to the pwsh\nsyntax.\n\n-Peff\n"},{"id":"535734","messageId":"aYwin2chSoz1RBFw@pks.im","threadId":"64958","inReplyTo":"20260210225401.GA1837188@coredump.intra.peff.net","subject":"Re: [PATCH 3/5] ci: make test slicing consistent across Meson/Make","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-11T06:33:03Z","receivedAt":"2026-02-11T06:33:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Feb 10, 2026 at 05:54:01PM -0500, Jeff King wrote:\n> On Tue, Feb 10, 2026 at 02:15:13PM -0800, Junio C Hamano wrote:\n> \n> > > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> > > index 2b175dc5c6..1b7a16e1f1 100644\n> > > --- a/.github/workflows/main.yml\n> > > +++ b/.github/workflows/main.yml\n> > > @@ -298,7 +298,7 @@ jobs:\n> > >          path: build\n> > >      - name: Test\n> > >        shell: pwsh\n> > > -      run: ci/run-test-slice-meson.sh build ${{matrix.nr}} 10\n> > > +      run: ci/run-test-slice-meson.sh build ${{matrix.nr + 1}} 10\n> > >      - name: print test failures\n> > >        if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n> > >        shell: bash\n> > \n> > Have we successfully run this one?\n\nI haven't kicked off a GitHub workflow for this patch series. Guess I\nshould've done that. I've created https://github.com/git/git/pull/2195\nnow to give v2 a test run first.\n\n> > I am getting\n> > \n> > Invalid workflow file: .github/workflows/main.yml#L1\n> > (Line: 153, Col: 12): Unexpected symbol: '+'. Located at position 11\n> > within expression: matrix.nr + 1, (Line: 301, Col: 12): Unexpected\n> > symbol: '+'. Located at position 11 within expression: matrix.nr + 1\n> > \n> > https://github.com/orgs/community/discussions/25386 is a 6-year old\n> > discussion so things may have changed quite a lot, but at least back\n> > then the claim was\n> > \n> >     Github actions doesn’t support math operations in expressions\n> >     inside ${{ }}. You could add up these two numbers in bash script and\n> >     then use set-env command to give its value to an environment\n> >     variable ...\n> > \n> > though.\n> \n> Right, that's why we used pwsh syntax to do it before, in d3d6493dcf\n> (ci: use Meson's new `--slice` option, 2025-07-09).\n> \n> That went away in 17bd1108ea (ci(windows-meson-test): handle options and\n> output like other test jobs, 2025-11-18), because the \"+1\" was added\n> into the script itself there. It looks like the patch under discussion\n> removes the +1 from the script, so we'd need to go back to the pwsh\n> syntax.\n\nIndeed, will fix. Thanks!\n\nPatrick\n"}]}