# [PATCH v3 02/10] t7422: fix flaky test caused by buffered stdout 12 messages from 2025-01-07 to 2025-01-09. Participants: Patrick Steinhardt, Jeff King. Thread: https://gitlist.dev/t/62756 ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 00/10] A couple of CI improvements Message-ID: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481%40pks.im In-Reply-To: <20250103-b4-pks-ci-fixes-v1-0-a9bb95dff833@pks.im> ``` Hi, this patch series addresses a couple of issues I've found while investigating flaky CI jobs. Besides two more fixes for flaky jobs it also removes some stale code and simplifies the setup on GitHub Actions to always use containerized jobs on Linux. Test runs can be found for GitLab [1] and GitHub [2]. Changes in v2: - Expand a bit on the reasoning behind the conversion to use containerized jobs. - Fix commit message typo. - Properly fix the race in t7422 via pipe stuffing, as proposed by Peff. - Link to v1: https://lore.kernel.org/r/20250103-b4-pks-ci-fixes-v1-0-a9bb95dff833@pks.im Changes in v3: - Another iteration on the SIGPIPE test, which should now finally plug the race. - Link to v2: https://lore.kernel.org/r/20250106-b4-pks-ci-fixes-v2-0-06ae540771b7@pks.im Thanks! Patrick [1]: https://gitlab.com/gitlab-org/git/-/merge_requests/277 [2]: https://github.com/git/git/pull/1865 --- Patrick Steinhardt (10): t0060: fix EBUSY in MinGW when setting up runtime prefix t7422: fix flaky test caused by buffered stdout github: adapt containerized jobs to be rootless github: convert all Linux jobs to be containerized github: simplify computation of the job's distro gitlab-ci: remove the "linux-old" job gitlab-ci: add linux32 job testing against i386 ci: stop special-casing for Ubuntu 16.04 ci: use latest Ubuntu release ci: remove stale code for Azure Pipelines .github/workflows/main.yml | 78 ++++++++++++++++++++++----------------------- .gitlab-ci.yml | 19 ++++++----- ci/install-dependencies.sh | 6 ++-- ci/lib.sh | 34 +++----------------- ci/print-test-failures.sh | 5 --- t/t0060-path-utils.sh | 10 +++--- t/t7422-submodule-output.sh | 43 ++++++++++++++++++++++--- 7 files changed, 100 insertions(+), 95 deletions(-) Range-diff versus v2: 1: 924cc137a7 = 1: 2d20c22e1c t0060: fix EBUSY in MinGW when setting up runtime prefix 2: 85f732d57f < -: ---------- t7422: fix flaky test caused by buffered stdout -: ---------- > 2: 97e94a22d0 t7422: fix flaky test caused by buffered stdout 3: 96ffed8ad9 = 3: 5f105f2d04 github: adapt containerized jobs to be rootless 4: a276e87563 = 4: ffcb18fe34 github: convert all Linux jobs to be containerized 5: 736d660c31 = 5: e7a9dc276c github: simplify computation of the job's distro 6: 0c2e227cb8 = 6: 7e1f6b651a gitlab-ci: remove the "linux-old" job 7: 60c3cb0d76 = 7: 03b4a82fc0 gitlab-ci: add linux32 job testing against i386 8: 216c043aac = 8: df57d16eb9 ci: stop special-casing for Ubuntu 16.04 9: 234b741805 = 9: 7c63294ace ci: use latest Ubuntu release 10: 421852878a = 10: d47387d596 ci: remove stale code for Azure Pipelines --- base-commit: 1b4e9a5f8b5f048972c21fe8acafe0404096f694 change-id: 20250103-b4-pks-ci-fixes-2d0a23fb5c78 ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 02/10] t7422: fix flaky test caused by buffered stdout Message-ID: <20250107-b4-pks-ci-fixes-v3-2-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-2-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` One test in t7422 asserts that `git submodule status --recursive` properly handles SIGPIPE. This test is flaky though and may sometimes not see a SIGPIPE at all: expecting success of 7422.18 'git submodule status --recursive propagates SIGPIPE': { git submodule status --recursive 2>err; echo $?>status; } | grep -q X/S && test_must_be_empty err && test_match_signal 13 "$(cat status)" ++ git submodule status --recursive ++ grep -q X/S ++ echo 0 ++ test_must_be_empty err ++ test 1 -ne 1 ++ test_path_is_file err ++ test 1 -ne 1 ++ test -f err ++ test -s err +++ cat status ++ test_match_signal 13 0 ++ test 0 = 141 ++ test 0 = 269 ++ return 1 error: last command exited with $?=1 not ok 18 - git submodule status --recursive propagates SIGPIPE The issue is caused by us using grep(1) to terminate the pipe on the first matching line in the recursing git-submodule(1) process. Standard streams are typically buffered though, so this condition is racy and may cause us to terminate the pipe after git-submodule(1) has already exited, and in that case we wouldn't see the expected signal. Fix the issue by generating a couple thousand nested submodules and matching on the first nested submodule. This ensures that the recursive git-submodule(1) process completely fills its stdout buffer, which makes subsequent writes block until the downstream consumer of the pipe either fully drains it or closes it. To verify that this works as expected one can apply the following patch to the preimage of this commit, which used to reliably trigger the race: diff --git a/t/t7422-submodule-output.sh b/t/t7422-submodule-output.sh index 3c5177cc30..df6001f8a0 100755 --- a/t/t7422-submodule-output.sh +++ b/t/t7422-submodule-output.sh @@ -202,7 +202,7 @@ test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE' cd repo && GIT_ALLOW_PROTOCOL=file git submodule add "$(pwd)"/../submodule && { git submodule status --recursive 2>err; echo $?>status; } | - grep -q recursive-submodule-path-1 && + { sleep 1 && grep -q recursive-submodule-path-1 && sleep 1; } && test_must_be_empty err && test_match_signal 13 "$(cat status)" ) With the pipe-stuffing workaround the test runs successfully. Helped-by: Jeff King Signed-off-by: Patrick Steinhardt --- t/t7422-submodule-output.sh | 43 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/t/t7422-submodule-output.sh b/t/t7422-submodule-output.sh index f21e9203678b94701281d5339ae8bfe53d5de0ed..023a5cbdc44bac2389fca45cf7017750627c4ce9 100755 --- a/t/t7422-submodule-output.sh +++ b/t/t7422-submodule-output.sh @@ -167,10 +167,45 @@ do done test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE' ' - { git submodule status --recursive 2>err; echo $?>status; } | - grep -q X/S && - test_must_be_empty err && - test_match_signal 13 "$(cat status)" + # The test setup is somewhat involved because triggering a SIGPIPE is + # racy with buffered pipes. To avoid the raciness we thus need to make + # sure that the subprocess in question fills the buffers completely, + # which requires a couple thousand submodules in total. + test_when_finished "rm -rf submodule repo" && + git init submodule && + ( + cd submodule && + test_commit initial && + + COMMIT=$(git rev-parse HEAD) && + for i in $(test_seq 2000) + do + printf "[submodule \"sm-$i\"]\npath = recursive-submodule-path-$i\n" "$i" || + return 1 + done >gitmodules && + BLOB=$(git hash-object -w --stdin tree && + for i in $(test_seq 2000) + do + printf "160000 commit $COMMIT\trecursive-submodule-path-%d\n" "$i" || + return 1 + done >>tree && + TREE=$(git mktree err; echo $?>status; } | + grep -q recursive-submodule-path-1 && + test_must_be_empty err && + test_match_signal 13 "$(cat status)" + ) ' test_done -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 01/10] t0060: fix EBUSY in MinGW when setting up runtime prefix Message-ID: <20250107-b4-pks-ci-fixes-v3-1-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-1-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` Two of our tests in t0060 verify that the runtime prefix functionality works as expected by creating a separate directory hierarchy, copying the Git executable in there and then creating scripts relative to that executable. These tests fail quite regularly in GitLab CI with the following error: expecting success of 0060.218 '%(prefix)/ works': mkdir -p pretend/bin && cp "$GIT_EXEC_PATH"/git$X pretend/bin/ && git config yes.path "%(prefix)/yes" && GIT_EXEC_PATH= ./pretend/bin/git config --path yes.path >actual && echo "$(pwd)/pretend/yes" >expect && test_cmp expect actual ++ mkdir -p pretend/bin ++ cp /c/GitLab-Runner/builds/gitlab-org/git/git.exe pretend/bin/ cp: cannot create regular file 'pretend/bin/git.exe': Device or resource busy error: last command exited with $?=1 not ok 218 - %(prefix)/ works Seemingly, the "git.exe" binary we are trying to overwrite is still being held open. It is somewhat puzzling why exactly that is: while the preceding test _does_ write to and execute the same path, it should have exited and shouldn't keep any backgrounded processes around. So it must be held open by something else, either in MinGW or in Windows itself. While the root cause is puzzling, the workaround is trivial enough: instead of writing the file twice we simply pull the common setup into a separate test case so that we won't observe EBUSY in the first place. Signed-off-by: Patrick Steinhardt --- t/t0060-path-utils.sh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index dbb2e73bcd912ae6a804603ff54e4c609966fa5d..8545cdfab559b4e247cb2699965e637529fd930a 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -592,17 +592,19 @@ test_lazy_prereq CAN_EXEC_IN_PWD ' ./git rev-parse ' +test_expect_success !VALGRIND,RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'setup runtime prefix' ' + mkdir -p pretend/bin && + cp "$GIT_EXEC_PATH"/git$X pretend/bin/ +' + test_expect_success !VALGRIND,RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' - mkdir -p pretend/bin pretend/libexec/git-core && + mkdir -p pretend/libexec/git-core && echo "echo HERE" | write_script pretend/libexec/git-core/git-here && - cp "$GIT_EXEC_PATH"/git$X pretend/bin/ && GIT_EXEC_PATH= ./pretend/bin/git here >actual && echo HERE >expect && test_cmp expect actual' test_expect_success !VALGRIND,RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' ' - mkdir -p pretend/bin && - cp "$GIT_EXEC_PATH"/git$X pretend/bin/ && git config yes.path "%(prefix)/yes" && GIT_EXEC_PATH= ./pretend/bin/git config --path yes.path >actual && echo "$(pwd)/pretend/yes" >expect && -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 03/10] github: adapt containerized jobs to be rootless Message-ID: <20250107-b4-pks-ci-fixes-v3-3-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-3-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` The containerized jobs in GitHub Actions run as root, giving them special permissions to for example delete files even when the user shouldn't be able to due to file permissions. This limitation keeps us from using containerized jobs for most of our Ubuntu-based jobs as it causes a number of tests to fail. Adapt the jobs to create a separate user that executes the test suite. This follows similar infrastructure that we already have in GitLab CI. Signed-off-by: Patrick Steinhardt --- .github/workflows/main.yml | 6 ++++-- ci/install-dependencies.sh | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 900be9957a23fcaa64e1aefd0c8638c5f84b7997..b02f5873a540b458d38e7951b4ee3d5ca598ae23 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -371,10 +371,12 @@ jobs: run: apt -q update && apt -q -y install libc6-amd64 lib64stdc++6 - uses: actions/checkout@v4 - run: ci/install-dependencies.sh - - run: ci/run-build-and-tests.sh + - run: useradd builder --create-home + - run: chown -R builder . + - run: sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh - name: print test failures if: failure() && env.FAILED_TEST_ARTIFACTS != '' - run: ci/print-test-failures.sh + run: sudo --preserve-env --set-home --user=builder ci/print-test-failures.sh - name: Upload failed tests' directories if: failure() && env.FAILED_TEST_ARTIFACTS != '' uses: actions/upload-artifact@v4 diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index d1cb9fa8785388b3674fcea4dd682abc0725c968..ecb5b9d36c20d3e7e96148ac628a96c62642c308 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -31,7 +31,7 @@ alpine-*) ;; fedora-*|almalinux-*) dnf -yq update >/dev/null && - dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null + dnf -yq install shadow-utils sudo make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null ;; ubuntu-*|ubuntu32-*|debian-*) # Required so that apt doesn't wait for user input on certain packages. -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 04/10] github: convert all Linux jobs to be containerized Message-ID: <20250107-b4-pks-ci-fixes-v3-4-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-4-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` We have split the CI jobs in GitHub Workflows into two categories: - Those running on a machine pool directly. - Those running in a container on the machine pool. The latter is more flexible because it allows us to freely pick whatever container image we want to use for a specific job, while the former only allows us to pick from a handful of different distros. The containerized jobs do not have any significant downsides to the best of my knowledge: - They aren't significantly slower to start up. A quick comparison by Peff shows that the difference is mostly lost in the noise: job | old | new --------------------|------|------ linux-TEST-vars 11m30s 10m54s linux-asan-ubsan 30m26s 31m14s linux-gcc 9m47s 10m6s linux-gcc-default 9m47s 9m41s linux-leaks 25m50s 25m21s linux-meson 10m36s 10m41s linux-reftable 10m25s 10m23s linux-reftable-leaks 27m18s 27m28s linux-sha256 9m54s 10m31s Some jobs are a bit faster, some are a bit slower, but there does not seem to be any significant change. - Containerized jobs run as root, which keeps a couple of tests from running. This has been addressed in the preceding commit though, where we now use setpriv(1) to run tests as a separate user. - GitHub injects a Node binary into containerized jobs, which is dynamically linked. This has led to some issues in the past [1], but only for our 32 bit jobs. The issues have since been resolved. Overall there seem to be no downsides, but the upside is that we have more control over the exact image that these jobs use. Convert the Linux jobs accordingly. [1]: https://lore.kernel.org/git/20240912094841.GD589828@coredump.intra.peff.net/ Signed-off-by: Patrick Steinhardt --- .github/workflows/main.yml | 68 ++++++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b02f5873a540b458d38e7951b4ee3d5ca598ae23..8e5847da4fab009ad699c18e1a5a336a8b45c3ed 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -259,20 +259,6 @@ jobs: fail-fast: false matrix: vector: - - jobname: linux-sha256 - cc: clang - pool: ubuntu-latest - - jobname: linux-reftable - cc: clang - pool: ubuntu-latest - - jobname: linux-gcc - cc: gcc - cc_package: gcc-8 - pool: ubuntu-20.04 - - jobname: linux-TEST-vars - cc: gcc - cc_package: gcc-8 - pool: ubuntu-20.04 - jobname: osx-clang cc: clang pool: macos-13 @@ -285,21 +271,6 @@ jobs: - jobname: osx-meson cc: clang pool: macos-13 - - jobname: linux-gcc-default - cc: gcc - pool: ubuntu-latest - - jobname: linux-leaks - cc: gcc - pool: ubuntu-latest - - jobname: linux-reftable-leaks - cc: gcc - pool: ubuntu-latest - - 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}} @@ -342,6 +313,44 @@ jobs: fail-fast: false matrix: vector: + - jobname: linux-sha256 + image: ubuntu:latest + cc: clang + distro: ubuntu-latest + - jobname: linux-reftable + image: ubuntu:latest + cc: clang + distro: ubuntu-latest + - jobname: linux-gcc + image: ubuntu:20.04 + cc: gcc + cc_package: gcc-8 + distro: ubuntu-20.04 + - jobname: linux-TEST-vars + image: ubuntu:20.04 + cc: gcc + cc_package: gcc-8 + distro: ubuntu-20.04 + - jobname: linux-gcc-default + image: ubuntu:latest + cc: gcc + distro: ubuntu-latest + - jobname: linux-leaks + image: ubuntu:latest + cc: gcc + distro: ubuntu-latest + - jobname: linux-reftable-leaks + image: ubuntu:latest + cc: gcc + distro: ubuntu-latest + - jobname: linux-asan-ubsan + image: ubuntu:latest + cc: clang + distro: ubuntu-latest + - jobname: linux-meson + image: ubuntu:latest + cc: gcc + distro: ubuntu-latest - jobname: linux-musl image: alpine distro: alpine-latest @@ -363,6 +372,7 @@ jobs: env: jobname: ${{matrix.vector.jobname}} distro: ${{matrix.vector.distro}} + CC: ${{matrix.vector.cc}} runs-on: ubuntu-latest container: ${{matrix.vector.image}} steps: -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 06/10] gitlab-ci: remove the "linux-old" job Message-ID: <20250107-b4-pks-ci-fixes-v3-6-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-6-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` The "linux-old" job was historically testing against the oldest supported LTS release of Ubuntu. But with c85bcb5de1 (gitlab-ci: switch from Ubuntu 16.04 to 20.04, 2024-10-31) it has been converted to test against Ubuntu 20.04, which already gets exercised in a couple of other CI jobs. It's thus not adding any significant test coverage. Drop the job. Signed-off-by: Patrick Steinhardt --- .gitlab-ci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9254e01583306e67dc12b6b9e0015183e1108655..00bc727865031620752771af4a9030c7de1b73df 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -36,9 +36,6 @@ test:linux: fi parallel: matrix: - - jobname: linux-old - image: ubuntu:20.04 - CC: gcc - jobname: linux-sha256 image: ubuntu:latest CC: clang -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 05/10] github: simplify computation of the job's distro Message-ID: <20250107-b4-pks-ci-fixes-v3-5-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-5-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` We explicitly list the distro of Linux-based jobs, but it is equivalent to the name of the image in almost all cases, except that colons are replaced with dashes. Drop the redundant information and massage it in our CI scripts, which is equivalent to how we do it in GitLab CI. There are a couple of exceptions: - The "linux32" job, whose distro name is different than the image name. This is handled by adapting all sites to use the new name. - The "alpine" and "fedora" jobs, neither of which specify a tag for their image. This is handled by adding the "latest" tag. Signed-off-by: Patrick Steinhardt --- .github/workflows/main.yml | 22 ++++------------------ ci/install-dependencies.sh | 4 ++-- ci/lib.sh | 2 ++ 3 files changed, 8 insertions(+), 20 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8e5847da4fab009ad699c18e1a5a336a8b45c3ed..b54da639a650682495994e3c7b137eab4e6cb3bf 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -275,7 +275,7 @@ jobs: CC: ${{matrix.vector.cc}} CC_PACKAGE: ${{matrix.vector.cc_package}} jobname: ${{matrix.vector.jobname}} - distro: ${{matrix.vector.pool}} + CI_JOB_IMAGE: ${{matrix.vector.pool}} TEST_OUTPUT_DIRECTORY: ${{github.workspace}}/t runs-on: ${{matrix.vector.pool}} steps: @@ -316,63 +316,49 @@ jobs: - jobname: linux-sha256 image: ubuntu:latest cc: clang - distro: ubuntu-latest - jobname: linux-reftable image: ubuntu:latest cc: clang - distro: ubuntu-latest - jobname: linux-gcc image: ubuntu:20.04 cc: gcc cc_package: gcc-8 - distro: ubuntu-20.04 - jobname: linux-TEST-vars image: ubuntu:20.04 cc: gcc cc_package: gcc-8 - distro: ubuntu-20.04 - jobname: linux-gcc-default image: ubuntu:latest cc: gcc - distro: ubuntu-latest - jobname: linux-leaks image: ubuntu:latest cc: gcc - distro: ubuntu-latest - jobname: linux-reftable-leaks image: ubuntu:latest cc: gcc - distro: ubuntu-latest - jobname: linux-asan-ubsan image: ubuntu:latest cc: clang - distro: ubuntu-latest - jobname: linux-meson image: ubuntu:latest cc: gcc - distro: ubuntu-latest - jobname: linux-musl - image: alpine - distro: alpine-latest + image: alpine:latest # Supported until 2025-04-02. - jobname: linux32 image: i386/ubuntu:focal - distro: ubuntu32-20.04 - jobname: pedantic - image: fedora - distro: fedora-latest + image: fedora:latest # A RHEL 8 compatible distro. Supported until 2029-05-31. - jobname: almalinux-8 image: almalinux:8 - distro: almalinux-8 # Supported until 2026-08-31. - jobname: debian-11 image: debian:11 - distro: debian-11 env: jobname: ${{matrix.vector.jobname}} - distro: ${{matrix.vector.distro}} CC: ${{matrix.vector.cc}} + CI_JOB_IMAGE: ${{matrix.vector.image}} runs-on: ubuntu-latest container: ${{matrix.vector.image}} steps: diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index ecb5b9d36c20d3e7e96148ac628a96c62642c308..d5a959e25ff3236656ff3416b81732ec5c2107c1 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -33,7 +33,7 @@ fedora-*|almalinux-*) dnf -yq update >/dev/null && dnf -yq install shadow-utils sudo make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null ;; -ubuntu-*|ubuntu32-*|debian-*) +ubuntu-*|i386/ubuntu-*|debian-*) # Required so that apt doesn't wait for user input on certain packages. export DEBIAN_FRONTEND=noninteractive @@ -42,7 +42,7 @@ ubuntu-*|ubuntu32-*|debian-*) SVN='libsvn-perl subversion' LANGUAGES='language-pack-is' ;; - ubuntu32-*) + i386/ubuntu-*) SVN= LANGUAGES='language-pack-is' ;; diff --git a/ci/lib.sh b/ci/lib.sh index 8885ee3c3f86c62e8783d27756b8779bd491e7e6..f8b68ab8a6546802756fd516ca15a2c97223da5f 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -246,6 +246,8 @@ then GIT_TEST_OPTS="--github-workflow-markup" JOBS=10 + + distro=$(echo "$CI_JOB_IMAGE" | tr : -) elif test true = "$GITLAB_CI" then CI_TYPE=gitlab-ci -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 07/10] gitlab-ci: add linux32 job testing against i386 Message-ID: <20250107-b4-pks-ci-fixes-v3-7-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-7-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` Add another job to GitLab CI that tests against the i386 architecture. This job is equivalent to the same job in GitHub Workflows. Signed-off-by: Patrick Steinhardt --- .gitlab-ci.yml | 2 ++ ci/lib.sh | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 00bc727865031620752771af4a9030c7de1b73df..29e9056dd5010f8843e42aeae8410973c825de54 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -66,6 +66,8 @@ test:linux: image: fedora:latest - jobname: linux-musl image: alpine:latest + - jobname: linux32 + image: i386/ubuntu:20.04 - jobname: linux-meson image: ubuntu:latest CC: gcc diff --git a/ci/lib.sh b/ci/lib.sh index f8b68ab8a6546802756fd516ca15a2c97223da5f..2293849ada3b45873f80e4392ab93c65657d0f13 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -269,7 +269,7 @@ then CI_OS_NAME=osx JOBS=$(nproc) ;; - *,alpine:*|*,fedora:*|*,ubuntu:*) + *,alpine:*|*,fedora:*|*,ubuntu:*|*,i386/ubuntu:*) CI_OS_NAME=linux JOBS=$(nproc) ;; -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 08/10] ci: stop special-casing for Ubuntu 16.04 Message-ID: <20250107-b4-pks-ci-fixes-v3-8-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-8-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` With c85bcb5de1 (gitlab-ci: switch from Ubuntu 16.04 to 20.04, 2024-10-31) we have adapted the last CI job to stop using Ubuntu 16.04 in favor of Ubuntu 20.04. Remove the special-casing we still have in our CI scripts. Signed-off-by: Patrick Steinhardt --- ci/lib.sh | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/ci/lib.sh b/ci/lib.sh index 2293849ada3b45873f80e4392ab93c65657d0f13..77a4aabdb8fb416c1733f02d02145b6bc0849998 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -347,14 +347,7 @@ ubuntu-*) fi MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE" - case "$distro" in - ubuntu-16.04) - # Apache is too old for HTTP/2. - ;; - *) - export GIT_TEST_HTTPD=true - ;; - esac + export GIT_TEST_HTTPD=true # The Linux build installs the defined dependency versions below. # The OS X build installs much more recent versions, whichever -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 09/10] ci: use latest Ubuntu release Message-ID: <20250107-b4-pks-ci-fixes-v3-9-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-9-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` Both GitHub Actions and GitLab CI use the "ubuntu:latest" tag as the default image for most jobs. This tag is somewhat misleading though, as it does not refer to the latest release of Ubuntu, but to the latest LTS release thereof. But as we already have a couple of jobs exercising the oldest LTS release of Ubuntu that Git still supports, it would make more sense to test the oldest and youngest versions of Ubuntu. Adapt these jobs to instead use the "ubuntu:rolling" tag, which refers to the actual latest release, which currently is Ubuntu 24.10. Signed-off-by: Patrick Steinhardt --- .github/workflows/main.yml | 14 +++++++------- .gitlab-ci.yml | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b54da639a650682495994e3c7b137eab4e6cb3bf..b90381ae015edf9db5aa4b8c0ace9bb5c549c37b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -314,10 +314,10 @@ jobs: matrix: vector: - jobname: linux-sha256 - image: ubuntu:latest + image: ubuntu:rolling cc: clang - jobname: linux-reftable - image: ubuntu:latest + image: ubuntu:rolling cc: clang - jobname: linux-gcc image: ubuntu:20.04 @@ -328,19 +328,19 @@ jobs: cc: gcc cc_package: gcc-8 - jobname: linux-gcc-default - image: ubuntu:latest + image: ubuntu:rolling cc: gcc - jobname: linux-leaks - image: ubuntu:latest + image: ubuntu:rolling cc: gcc - jobname: linux-reftable-leaks - image: ubuntu:latest + image: ubuntu:rolling cc: gcc - jobname: linux-asan-ubsan - image: ubuntu:latest + image: ubuntu:rolling cc: clang - jobname: linux-meson - image: ubuntu:latest + image: ubuntu:rolling cc: gcc - jobname: linux-musl image: alpine:latest diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 29e9056dd5010f8843e42aeae8410973c825de54..8ed3ff5f0373d70b6f609dc5292dda2dd7fd8f88 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -37,10 +37,10 @@ test:linux: parallel: matrix: - jobname: linux-sha256 - image: ubuntu:latest + image: ubuntu:rolling CC: clang - jobname: linux-reftable - image: ubuntu:latest + image: ubuntu:rolling CC: clang - jobname: linux-gcc image: ubuntu:20.04 @@ -51,16 +51,16 @@ test:linux: CC: gcc CC_PACKAGE: gcc-8 - jobname: linux-gcc-default - image: ubuntu:latest + image: ubuntu:rolling CC: gcc - jobname: linux-leaks - image: ubuntu:latest + image: ubuntu:rolling CC: gcc - jobname: linux-reftable-leaks - image: ubuntu:latest + image: ubuntu:rolling CC: gcc - jobname: linux-asan-ubsan - image: ubuntu:latest + image: ubuntu:rolling CC: clang - jobname: pedantic image: fedora:latest @@ -69,7 +69,7 @@ test:linux: - jobname: linux32 image: i386/ubuntu:20.04 - jobname: linux-meson - image: ubuntu:latest + image: ubuntu:rolling CC: gcc artifacts: paths: -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Patrick Steinhardt, 2025-01-07 12:30 Subject: [PATCH v3 10/10] ci: remove stale code for Azure Pipelines Message-ID: <20250107-b4-pks-ci-fixes-v3-10-546a0ebc8481@pks.im> URL: https://gitlist.dev/e/20250107-b4-pks-ci-fixes-v3-10-546a0ebc8481%40pks.im In-Reply-To: <20250107-b4-pks-ci-fixes-v3-0-546a0ebc8481@pks.im> ``` Support for Azure Pipelines has been retired in 6081d3898f (ci: retire the Azure Pipelines definition, 2020-04-11) in favor of GitHub Actions. Our CI library still has some infrastructure left for Azure though that is now unused. Remove it. Signed-off-by: Patrick Steinhardt --- ci/lib.sh | 21 +-------------------- ci/print-test-failures.sh | 5 ----- 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/ci/lib.sh b/ci/lib.sh index 77a4aabdb8fb416c1733f02d02145b6bc0849998..4003354f16c048b969c0bb4340d2ee2777767300 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -206,26 +206,7 @@ export TERM=${TERM:-dumb} # Clear MAKEFLAGS that may come from the outside world. export MAKEFLAGS= -if test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI" -then - CI_TYPE=azure-pipelines - # We are running in Azure Pipelines - CI_BRANCH="$BUILD_SOURCEBRANCH" - CI_COMMIT="$BUILD_SOURCEVERSION" - CI_JOB_ID="$BUILD_BUILDID" - CI_JOB_NUMBER="$BUILD_BUILDNUMBER" - CI_OS_NAME="$(echo "$AGENT_OS" | tr A-Z a-z)" - test darwin != "$CI_OS_NAME" || CI_OS_NAME=osx - CI_REPO_SLUG="$(expr "$BUILD_REPOSITORY_URI" : '.*/\([^/]*/[^/]*\)$')" - CC="${CC:-gcc}" - - # use a subdirectory of the cache dir (because the file share is shared - # among *all* phases) - cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME" - - GIT_TEST_OPTS="--write-junit-xml" - JOBS=10 -elif test true = "$GITHUB_ACTIONS" +if test true = "$GITHUB_ACTIONS" then CI_TYPE=github-actions CI_BRANCH="$GITHUB_REF" diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh index 655687dd827e5b3e4d4879803b0d4499e7751380..dc910e51609cd7344b1ad03fdb4e820e47ad3a88 100755 --- a/ci/print-test-failures.sh +++ b/ci/print-test-failures.sh @@ -39,11 +39,6 @@ do test_name="${test_name##*/}" trash_dir="trash directory.$test_name" case "$CI_TYPE" in - azure-pipelines) - mkdir -p failed-test-artifacts - mv "$trash_dir" failed-test-artifacts - continue - ;; github-actions) mkdir -p failed-test-artifacts echo "FAILED_TEST_ARTIFACTS=${TEST_OUTPUT_DIRECTORY:t}/failed-test-artifacts" >>$GITHUB_ENV -- 2.48.0.rc1.245.gb3e6e7acbc.dirty ``` ## Jeff King, 2025-01-09 07:33 Subject: Re: [PATCH v3 02/10] t7422: fix flaky test caused by buffered stdout Message-ID: <20250109073354.GB2735258@coredump.intra.peff.net> URL: https://gitlist.dev/e/20250109073354.GB2735258%40coredump.intra.peff.net In-Reply-To: <20250107-b4-pks-ci-fixes-v3-2-546a0ebc8481@pks.im> ``` On Tue, Jan 07, 2025 at 01:30:44PM +0100, Patrick Steinhardt wrote: > The issue is caused by us using grep(1) to terminate the pipe on the > first matching line in the recursing git-submodule(1) process. Standard > streams are typically buffered though, so this condition is racy and may > cause us to terminate the pipe after git-submodule(1) has already > exited, and in that case we wouldn't see the expected signal. This patch looks good to me overall, but I think there are a few small inaccuracies in the commit message. I don't think buffering is relevant here. Especially since in the original test there isn't any buffering (the output comes from separate recursive status processes, so each line gets its own write() call). The race you're seeing is: 1. git-submodule (or its child process) writes the first X/S line we're trying to match 2. grep matches the line 3a. grep exits, closing the pipe 3b. git-submodule (or its children) writes the rest of its lines. Steps 3a and 3b happen at the same time without any guarantees. If 3a happens first, we get SIGPIPE. Otherwise, we don't (and the test fails). And when git-submodule exits is not important. The critical timing is when it does its final write(). If the pipe closes after that, even if it is still running, it will not notice. (I admit that one is a micro-nit, though). > Fix the issue by generating a couple thousand nested submodules and > matching on the first nested submodule. This ensures that the recursive > git-submodule(1) process completely fills its stdout buffer, which makes > subsequent writes block until the downstream consumer of the pipe either > fully drains it or closes it. One more micro-nit: "fully drains" is not accurate. If grep reads another 4096 bytes, then that opens up 4096 more bytes in the pipe buffer. And git-submodule can then write to it. Not important for our case, since we "know" grep will not read more after matching, but will just close. So it is really more like "block until the downstream consumer either reads more or closes it". That "know" is a little bit of an assumption. In particular, there is no reason grep could not read into a 1MB buffer, consuming the whole input, match within it, and only then exit. And then we'd be racy again. In practice I'm not too worried about this. I'd be surprised by a buffer bigger than 8k (which is what I saw when I strace'd it on my Linux system), and your generated input is something like 160k. That should fill even a generous pipe buffer plus grep input buffer. > + git init submodule && > + ( > + cd submodule && > + test_commit initial && > + > + COMMIT=$(git rev-parse HEAD) && > + for i in $(test_seq 2000) > + do > + printf "[submodule \"sm-$i\"]\npath = recursive-submodule-path-$i\n" "$i" || > + return 1 > + done >gitmodules && > + BLOB=$(git hash-object -w --stdin + > + printf "100644 blob $BLOB\t.gitmodules\n" >tree && > + for i in $(test_seq 2000) > + do > + printf "160000 commit $COMMIT\trecursive-submodule-path-%d\n" "$i" || > + return 1 > + done >>tree && > + TREE=$(git mktree + > + COMMIT=$(git commit-tree "$TREE") && > + git reset --hard "$COMMIT" > + ) && OK, so the submodule has a huge number of missing children. But that's enough for us, because the "-" lines generated by "submodule status" are fine. So you're able to generate the whole thing with only printf processes running in the loops. Very clever. > + git init repo && > + ( > + cd repo && > + GIT_ALLOW_PROTOCOL=file git submodule add "$(pwd)"/../submodule && > + { git submodule status --recursive 2>err; echo $?>status; } | > + grep -q recursive-submodule-path-1 && > + test_must_be_empty err && > + test_match_signal 13 "$(cat status)" > + ) And then adding another repo wrapping it is what makes it recursive. Nice. -Peff ```