{"thread":{"id":"65487","subject":"[PATCH v2 01/12] t: prepare `test_match_signal ()` calls for `set -e`","startedAt":"2026-04-15T13:06:41Z","lastAt":"2026-04-16T10:46:24Z","messageCount":15,"participants":["Patrick Steinhardt","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":12},"messages":[{"id":"541629","messageId":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260413-b4-pks-tests-with-set-e-v1-0-5b83763a0e84@pks.im","subject":"[PATCH v2 00/12] t: detect errors outside of test cases","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:33Z","receivedAt":"2026-04-15T13:06:40Z","isPatch":true,"body":"Hi,\n\nthis is a follow-up to the recent discussion we had around `set -e` to\nmake our tests more robust and basically supersedes Junio's [1].\n\nI've tested the patches with both Bash and Dash, and all tests are\npassing on my machine with both of them. CI seems to be happy, as\nwell. But I would expect that this change probably has some fallout,\neven though I hope that it's generally going to be small and contained.\n\nThis series is based on 8c9303b1ff (Merge branch\n'jc/no-writev-does-not-work', 2026-04-10).\n\nI've created an MR with GitLab [2] and a PR with GitHub [3] to verify\nthat these changes work on both platforms.\n\nChanges in v2:\n  - Use `ret=0; $command || ret=$?` pattern.\n  - Restore `echo 0` in SIGPIPE tests.\n  - Fix \"lib-git-svn.sh\" to gracefully handle the case where SVN Perl\n    modules aren't installed.\n  - Use `|| :` consistently instead of `|| true`.\n  - Fix up a couple of tests that fail on FreeBSD 15. The test suite is\n    now passing on this system, too.\n  - Only enable `set -e` on Bash 5 and newer.\n  - Link to v1: https://patch.msgid.link/20260413-b4-pks-tests-with-set-e-v1-0-5b83763a0e84@pks.im\n\nThanks!\n\nPatrick\n\n[1]: <20260325062114.2067946-1-gitster@pobox.com>\n[2]: https://gitlab.com/gitlab-org/git/-/merge_requests/541\n[3]: https://github.com/git/git/pull/2270\n\n---\nPatrick Steinhardt (12):\n      t: prepare `test_match_signal ()` calls for `set -e`\n      t: prepare `test_must_fail ()` for `set -e`\n      t: prepare `stop_git_daemon ()` for `set -e`\n      t: prepare `git config --unset` calls for `set -e`\n      t: prepare conditional test execution for `set -e`\n      t: prepare execution of potentially failing commands for `set -e`\n      t: prepare `test_when_finished ()`/`test_atexit()` for `set -e`\n      t0008: silence error in subshell when using `grep -v`\n      t1301: don't fail in case setfacl(1) doesn't exist or fails\n      t6002: fix use of `expr` with `set -e`\n      t9902: fix use of `read` with `set -e`\n      t: detect errors outside of test cases\n\n t/lib-git-daemon.sh                |  8 +++++---\n t/lib-git-svn.sh                   |  7 +++----\n t/lib-httpd.sh                     |  3 +--\n t/t0005-signals.sh                 |  4 ++--\n t/t0008-ignores.sh                 |  4 ++--\n t/t1301-shared-repo.sh             |  2 +-\n t/t3600-rm.sh                      |  2 +-\n t/t3901-i18n-patch.sh              |  3 ++-\n t/t4032-diff-inter-hunk-context.sh | 14 ++++++++------\n t/t5000-tar-tree.sh                |  4 ++--\n t/t6002-rev-list-bisect.sh         | 17 ++++++++++-------\n t/t7422-submodule-output.sh        |  2 +-\n t/t7450-bad-git-dotfiles.sh        | 24 +++++++++++++-----------\n t/t7508-status.sh                  |  4 ++--\n t/t9138-git-svn-authors-prog.sh    |  4 ++--\n t/t9200-git-cvsexportcommit.sh     |  3 +--\n t/t9400-git-cvsserver-server.sh    |  5 +++--\n t/t9401-git-cvsserver-crlf.sh      |  4 ++--\n t/t9402-git-cvsserver-refs.sh      |  4 ++--\n t/t9902-completion.sh              |  2 +-\n t/test-lib-functions.sh            | 12 ++++++------\n t/test-lib.sh                      | 19 +++++++++++++++----\n 22 files changed, 85 insertions(+), 66 deletions(-)\n\nRange-diff versus v1:\n\n 1:  210ccb018c !  1:  6e3147dbb1 t: prepare `test_match_signal ()` calls for `set -e`\n    @@ Commit message\n         but as we expect `foo` to fail this will cause the overall subshell to\n         fail once we `set -e`.\n     \n    -    Fix this issue by using `foo || echo $?` instead.\n    +    Fix this issue by using `foo && echo 0 || echo $?` instead.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n    @@ t/t0005-signals.sh: test_expect_success 'create blob' '\n      \n      test_expect_success !MINGW 'a constipated git dies with SIGPIPE' '\n     -\tOUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&\n    -+\tOUT=$( ((large_git || echo $? 1>&3) | :) 3>&1 ) &&\n    ++\tOUT=$( ((large_git && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) &&\n      \ttest_match_signal 13 \"$OUT\"\n      '\n      \n      test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' '\n     -\tOUT=$( ((trap \"\" PIPE && large_git; echo $? 1>&3) | :) 3>&1 ) &&\n    -+\tOUT=$( ((trap \"\" PIPE && large_git || echo $? 1>&3) | :) 3>&1 ) &&\n    ++\tOUT=$( ((trap \"\" PIPE && large_git && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) &&\n      \ttest_match_signal 13 \"$OUT\"\n      '\n      \n    @@ t/t3600-rm.sh: test_expect_success 'choking \"git rm\" should not let it die with\n      test_expect_success !MINGW 'choking \"git rm\" should not let it die with cruft (induce and check SIGPIPE)' '\n      \tchoke_git_rm_setup &&\n     -\tOUT=$( ((trap \"\" PIPE && git rm -n \"some-file-*\"; echo $? 1>&3) | :) 3>&1 ) &&\n    -+\tOUT=$( ((trap \"\" PIPE && git rm -n \"some-file-*\" || echo $? 1>&3) | :) 3>&1 ) &&\n    ++\tOUT=$( ((trap \"\" PIPE && git rm -n \"some-file-*\" && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) &&\n      \ttest_match_signal 13 \"$OUT\" &&\n      \ttest_path_is_missing .git/index.lock\n      '\n 2:  c056357f6d <  -:  ---------- t: prepare `test_must_fail ()` for `set -e`\n -:  ---------- >  2:  393374871a t: prepare `test_must_fail ()` for `set -e`\n 3:  d9076a67ba !  3:  2ff2e3fb7d t: prepare `stop_git_daemon ()` for `set -e`\n    @@ Commit message\n             than not that we have already killed it, and the call to kill will\n             fail.\n     \n    -    Prepare for this change by making the call to `wait` part of a condition\n    -    and by silencing failures of the second call to `kill`.\n    +    Prepare for this change by handling the failure of `wait` with `||` and\n    +    by silencing failures of the second call to `kill`.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n    @@ t/lib-git-daemon.sh: stop_git_daemon() {\n      \tkill \"$GIT_DAEMON_PID\"\n     -\twait \"$GIT_DAEMON_PID\" >&3 2>&4\n     -\tret=$?\n    -+\tif wait \"$GIT_DAEMON_PID\" >&3 2>&4\n    -+\tthen\n    -+\t\tret=0\n    -+\telse\n    -+\t\tret=$?\n    -+\tfi\n    ++\tret=0; wait \"$GIT_DAEMON_PID\" >&3 2>&4 || ret=$?\n     +\n      \tif ! test_match_signal 15 $ret\n      \tthen\n 4:  50be774536 =  4:  2c51b9d9fa t: prepare `git config --unset` calls for `set -e`\n 5:  b1ac21d4dd =  5:  adba2b830f t: prepare conditional test execution for `set -e`\n 6:  19518eeac5 !  6:  61f949e1fb t: prepare execution of potentially failing commands for `set -e`\n    @@ t/lib-git-svn.sh: GIT_SVN_DIR=$GIT_DIR/svn/refs/remotes/git-svn\n      then\n      \tskip_all='skipping git svn tests, svn not found'\n      \ttest_done\n    +@@ t/lib-git-svn.sh: export svnrepo\n    + svnconf=$PWD/svnconf\n    + export svnconf\n    + \n    ++x=0\n    + perl -w -e \"\n    + use SVN::Core;\n    + use SVN::Repos;\n    + \\$SVN::Core::VERSION gt '1.1.0' or exit(42);\n    + system(qw/svnadmin create --fs-type fsfs/, \\$ENV{svnrepo}) == 0 or exit(41);\n    +-\" >&3 2>&4\n    +-x=$?\n    ++\" >&3 2>&4 || x=$?\n    + if test $x -ne 0\n    + then\n    + \tif test $x -eq 42; then\n     \n      ## t/lib-httpd.sh ##\n     @@ t/lib-httpd.sh: start_httpd() {\n    @@ t/lib-httpd.sh: start_httpd() {\n      \t\tcat \"$HTTPD_ROOT_PATH\"/error.log >&4 2>/dev/null\n      \t\ttest_skip_or_die GIT_TEST_HTTPD \"web server setup failed\"\n     \n    + ## t/t3901-i18n-patch.sh ##\n    +@@ t/t3901-i18n-patch.sh: check_encoding () {\n    + \t\t8859)\n    + \t\t\tgrep \"^encoding ISO8859-1\" ;;\n    + \t\t*)\n    +-\t\t\tgrep \"^encoding ISO8859-1\"; test \"$?\" != 0 ;;\n    ++\t\t\tret=0; grep \"^encoding ISO8859-1\" || ret=$?\n    ++\t\t\ttest \"$ret\" != 0 ;;\n    + \t\tesac || return 1\n    + \t\tj=$i\n    + \t\ti=$(($i+1))\n    +\n    + ## t/t5000-tar-tree.sh ##\n    +@@ t/t5000-tar-tree.sh: test_expect_success LONG_IS_64BIT 'set up repository with huge blob' '\n    + # would generate the whole 64GB).\n    + test_expect_success LONG_IS_64BIT 'generate tar with huge size' '\n    + \t{\n    +-\t\tgit archive HEAD\n    +-\t\techo $? >exit-code\n    ++\t\t{ ret=0 && git archive HEAD || ret=$?; } &&\n    ++\t\techo \"$ret\" >exit-code\n    + \t} | test_copy_bytes 4096 >huge.tar &&\n    + \techo 141 >expect &&\n    + \ttest_cmp expect exit-code\n    +\n    + ## t/t7422-submodule-output.sh ##\n    +@@ t/t7422-submodule-output.sh: test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE'\n    + \t(\n    + \t\tcd repo &&\n    + \t\tGIT_ALLOW_PROTOCOL=file git submodule add \"$(pwd)\"/../submodule &&\n    +-\t\t{ git submodule status --recursive 2>err; echo $?>status; } |\n    ++\t\t{ { ret=0 && git submodule status --recursive 2>err || ret=$?; } && echo $ret >status; } |\n    + \t\t\tgrep -q recursive-submodule-path-1 &&\n    + \t\ttest_must_be_empty err &&\n    + \t\ttest_match_signal 13 \"$(cat status)\"\n    +\n      ## t/t9200-git-cvsexportcommit.sh ##\n     @@ t/t9200-git-cvsexportcommit.sh: if ! test_have_prereq PERL; then\n      \ttest_done\n    @@ t/t9402-git-cvsserver-refs.sh: check_diff() {\n      then\n      \tskip_all='skipping git-cvsserver tests, perl not available'\n     \n    + ## t/test-lib-functions.sh ##\n    +@@ t/test-lib-functions.sh: test_might_fail () {\n    + test_expect_code () {\n    + \twant_code=$1\n    + \tshift\n    +-\t\"$@\" 2>&7\n    +-\texit_code=$?\n    ++\texit_code=0; \"$@\" 2>&7 || exit_code=$?\n    + \tif test $exit_code = $want_code\n    + \tthen\n    + \t\treturn 0\n    +\n      ## t/test-lib.sh ##\n     @@ t/test-lib.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n      ################################################################\n    @@ t/test-lib.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n      then\n      \tif test -n \"$GIT_TEST_INSTALLED\"\n      \tthen\n    +@@ t/test-lib.sh: then\n    + \t# from any previous runs.\n    + \t>\"$GIT_TEST_TEE_OUTPUT_FILE\"\n    + \n    +-\t(GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} \"$0\" \"$@\" 2>&1;\n    +-\t echo $? >\"$TEST_RESULTS_BASE.exit\") | tee -a \"$GIT_TEST_TEE_OUTPUT_FILE\"\n    ++\t(\n    ++\t\tret=0 && GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} \"$0\" \"$@\" 2>&1 || ret=$?\n    ++\t\techo \"$ret\" >\"$TEST_RESULTS_BASE.exit\"\n    ++\t) | tee -a \"$GIT_TEST_TEE_OUTPUT_FILE\"\n    + \ttest \"$(cat \"$TEST_RESULTS_BASE.exit\")\" = 0\n    + \texit\n    + fi\n 7:  7d7583d1ea =  7:  697830e576 t: prepare `test_when_finished ()`/`test_atexit()` for `set -e`\n 8:  749a350716 !  8:  d5d1ea03ab t0008: silence error in subshell when using `grep -v`\n    @@ t/t0008-ignores.sh: test_expect_success_multiple () {\n      \n     -\texpect_verbose=$( echo \"$expect_all\" | grep -v '^::\t' )\n     -\texpect=$( echo \"$expect_verbose\" | sed -e 's/.*\t//' )\n    -+\texpect_verbose=$(echo \"$expect_all\" | grep -v '^::\t' || true)\n    ++\texpect_verbose=$(echo \"$expect_all\" | grep -v '^::\t' || :)\n     +\texpect=$(echo \"$expect_verbose\" | sed -e 's/.*\t//')\n      \n      \ttest_expect_success $prereq \"$testname${no_index_opt:+ with $no_index_opt}\" '\n 9:  14c8dd5148 !  9:  75a150e2dd t1301: don't fail in case setfacl(1) doesn't exist or fails\n    @@ t/t1301-shared-repo.sh: TEST_CREATE_REPO_NO_TEMPLATE=1\n      \n      # Remove a default ACL from the test dir if possible.\n     -setfacl -k . 2>/dev/null\n    -+setfacl -k . 2>/dev/null || true\n    ++setfacl -k . 2>/dev/null || :\n      \n      # User must have read permissions to the repo -> failure on --shared=0400\n      test_expect_success 'shared = 0400 (faulty permission u-w)' '\n10:  a81e602616 = 10:  ba22bab22d t6002: fix use of `expr` with `set -e`\n11:  dcf5c849e9 = 11:  5a8e2df836 t9902: fix use of `read` with `set -e`\n12:  691e1c9b58 ! 12:  8266ee6035 t: detect errors outside of test cases\n    @@ Commit message\n         Improve the status quo by enabling the errexit option so that any such\n         unchecked failures will cause us to abort immediately.\n     \n    +    Note that for now, we only enable this option for Bash 5 and newer. This\n    +    is because other shells have wildly different behaviour, and older\n    +    versions of Bash (especially on macOS) are buggy. The list of enabled\n    +    shells may be extended going forward.\n    +\n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n      ## t/test-lib.sh ##\n    @@ t/test-lib.sh\n      # along with this program.  If not, see https://www.gnu.org/licenses/ .\n      \n     +# Enable the use of errexit so that any unexpected failures will cause us to\n    -+# abort tests, even when outside of a specific test case.\n    -+set -e\n    ++# abort tests, even when outside of a specific test case. Note that we only\n    ++# enable this on Bash 5 and newer, as `set -e` has wildly different behaviour\n    ++# across shells. The list of allowed shells may be extended going forward.\n    ++if test \"${BASH_VERSINFO:=0}\" -ge 5\n    ++then\n    ++\tset -e\n    ++fi\n     +\n      # Test the binaries we have just built.  The tests are kept in\n      # t/ subdirectory and are run in 'trash directory' subdirectory.\n\n---\nbase-commit: 8c9303b1ffae5b745d1b0a1f98330cf7944d8db0\nchange-id: 20260410-b4-pks-tests-with-set-e-3ae479b24b51\n\n"},{"id":"541628","messageId":"20260415-b4-pks-tests-with-set-e-v2-1-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 01/12] t: prepare `test_match_signal ()` calls for `set -e`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:34Z","receivedAt":"2026-04-15T13:06:41Z","isPatch":true,"body":"We have a couple of calls to `test_match_signal ()` where we execute a\nGit command and expect it to die with a specific signal. These calls\nwill essentially execute the process in a subshell via `foo; echo $?`,\nbut as we expect `foo` to fail this will cause the overall subshell to\nfail once we `set -e`.\n\nFix this issue by using `foo && echo 0 || echo $?` instead.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t0005-signals.sh | 4 ++--\n t/t3600-rm.sh      | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t0005-signals.sh b/t/t0005-signals.sh\nindex afba0fc3fc..84319cf169 100755\n--- a/t/t0005-signals.sh\n+++ b/t/t0005-signals.sh\n@@ -42,12 +42,12 @@ test_expect_success 'create blob' '\n '\n \n test_expect_success !MINGW 'a constipated git dies with SIGPIPE' '\n-\tOUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&\n+\tOUT=$( ((large_git && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) &&\n \ttest_match_signal 13 \"$OUT\"\n '\n \n test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' '\n-\tOUT=$( ((trap \"\" PIPE && large_git; echo $? 1>&3) | :) 3>&1 ) &&\n+\tOUT=$( ((trap \"\" PIPE && large_git && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) &&\n \ttest_match_signal 13 \"$OUT\"\n '\n \ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 1f16e6b522..a371ea690e 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -260,7 +260,7 @@ test_expect_success 'choking \"git rm\" should not let it die with cruft (induce S\n \n test_expect_success !MINGW 'choking \"git rm\" should not let it die with cruft (induce and check SIGPIPE)' '\n \tchoke_git_rm_setup &&\n-\tOUT=$( ((trap \"\" PIPE && git rm -n \"some-file-*\"; echo $? 1>&3) | :) 3>&1 ) &&\n+\tOUT=$( ((trap \"\" PIPE && git rm -n \"some-file-*\" && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) &&\n \ttest_match_signal 13 \"$OUT\" &&\n \ttest_path_is_missing .git/index.lock\n '\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541630","messageId":"20260415-b4-pks-tests-with-set-e-v2-2-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 02/12] t: prepare `test_must_fail ()` for `set -e`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:35Z","receivedAt":"2026-04-15T13:06:44Z","isPatch":true,"body":"The helper function `test_must_fail ()` executes a specific Git command\nthat may or may not fail in a specific way. This is done by executing\nthe command in question and then comparing its exit code against a set\nof conditions.\n\nThis works, but once we run our test suite with `set -e` we may bail out\nof `test_must_fail ()` early in case the command actually fails, even\nthough we expect it to fail. Prepare for this change by handling the\nfailed case with `||`.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/test-lib-functions.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex f3af10fb7e..5fd5494ef1 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1195,8 +1195,9 @@ test_must_fail () {\n \t\techo >&7 \"test_must_fail: only 'git' is allowed: $*\"\n \t\treturn 1\n \tfi\n-\t\"$@\" 2>&7\n-\texit_code=$?\n+\n+\texit_code=0; \"$@\" 2>&7 || exit_code=$?\n+\n \tif test $exit_code -eq 0 && ! list_contains \"$_test_ok\" success\n \tthen\n \t\techo >&4 \"test_must_fail: command succeeded: $*\"\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541631","messageId":"20260415-b4-pks-tests-with-set-e-v2-3-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 03/12] t: prepare `stop_git_daemon ()` for `set -e`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:36Z","receivedAt":"2026-04-15T13:06:46Z","isPatch":true,"body":"We have a couple of calls to `stop_git_daemon ()` outside of specific\ntest cases that will kill a backgrounded git-daemon(1) process and\nexpect the process with a specific error code. While these function\ncalls do end up killing git-daemon(1), the error handling we have in\nthose contexts is basically ineffective. So while we expect the process\nto exit with a specific error code, we will just continue with any error\nin case it doesn't.\n\nThis will change once we enable `set -e` in a subsequent commit. There's\ntwo issues though that will make this _always_ fail:\n\n  - Our call to `wait` is expected to fail, but because it's not part of\n    a condition it will cause us to bail out immediately with `set -e`.\n\n  - We try to kill git-daemon(1) a second time via the pidfile. We can\n    generally expect that this is the same PID though as we had in the\n    \"GIT_DAEMON_PID\" environment variable, and thus it's more likely\n    than not that we have already killed it, and the call to kill will\n    fail.\n\nPrepare for this change by handling the failure of `wait` with `||` and\nby silencing failures of the second call to `kill`.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/lib-git-daemon.sh | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\nindex e62569222b..d172aa51f0 100644\n--- a/t/lib-git-daemon.sh\n+++ b/t/lib-git-daemon.sh\n@@ -85,14 +85,16 @@ stop_git_daemon() {\n \n \t# kill git-daemon child of git\n \tsay >&3 \"Stopping git daemon ...\"\n+\n \tkill \"$GIT_DAEMON_PID\"\n-\twait \"$GIT_DAEMON_PID\" >&3 2>&4\n-\tret=$?\n+\tret=0; wait \"$GIT_DAEMON_PID\" >&3 2>&4 || ret=$?\n+\n \tif ! test_match_signal 15 $ret\n \tthen\n \t\terror \"git daemon exited with status: $ret\"\n \tfi\n-\tkill \"$(cat \"$GIT_DAEMON_PIDFILE\")\" 2>/dev/null\n+\n+\tkill \"$(cat \"$GIT_DAEMON_PIDFILE\")\" 2>/dev/null || :\n \tGIT_DAEMON_PID=\n \trm -f git_daemon_output \"$GIT_DAEMON_PIDFILE\"\n }\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541632","messageId":"20260415-b4-pks-tests-with-set-e-v2-4-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 04/12] t: prepare `git config --unset` calls for `set -e`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:37Z","receivedAt":"2026-04-15T13:06:49Z","isPatch":true,"body":"We have a couple of calls to `git config --unset` that ultimately end up\nas no-ops as the configuration variables aren't set (anymore) in the\nfirst place. These calls are mostly intended to recover unconditionally\nfrom tests that may have executed only partially, but they'll ultimately\nfail during a normal test run.\n\nThis hasn't been a problem until now as we aren't running tests with\n`set -e`. This is about to change though, so let's silence the case\nwhere we cannot unset the config keys.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t4032-diff-inter-hunk-context.sh | 2 +-\n t/t7508-status.sh                  | 4 ++--\n t/t9138-git-svn-authors-prog.sh    | 4 ++--\n 3 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh\nindex bada0cbd32..c98eb6abb2 100755\n--- a/t/t4032-diff-inter-hunk-context.sh\n+++ b/t/t4032-diff-inter-hunk-context.sh\n@@ -17,7 +17,7 @@ f() {\n \n t() {\n \tuse_config=\n-\tgit config --unset diff.interHunkContext\n+\tgit config --unset diff.interHunkContext || :\n \n \tcase $# in\n \t4) hunks=$4; cmd=\"diff -U$3\";;\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex a5e21bf8bf..1167b835a4 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -773,8 +773,8 @@ test_expect_success TTY 'status --porcelain ignores color.status' '\n '\n \n # recover unconditionally from color tests\n-git config --unset color.status\n-git config --unset color.ui\n+git config --unset color.status || :\n+git config --unset color.ui || :\n \n test_expect_success 'status --porcelain respects -b' '\n \ndiff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh\nindex 784ec7fc2d..5bb38cb23a 100755\n--- a/t/t9138-git-svn-authors-prog.sh\n+++ b/t/t9138-git-svn-authors-prog.sh\n@@ -68,8 +68,8 @@ test_expect_success 'authors-file overrode authors-prog' '\n \t)\n '\n \n-git --git-dir=x/.git config --unset svn.authorsfile\n-git --git-dir=x/.git config --unset svn.authorsprog\n+git --git-dir=x/.git config --unset svn.authorsfile || :\n+git --git-dir=x/.git config --unset svn.authorsprog || :\n \n test_expect_success 'authors-prog imported user without email' '\n \tsvn mkdir -m gg --username gg-hermit \"$svnrepo\"/gg &&\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541633","messageId":"20260415-b4-pks-tests-with-set-e-v2-5-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 05/12] t: prepare conditional test execution for `set -e`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:38Z","receivedAt":"2026-04-15T13:06:52Z","isPatch":true,"body":"We have some test in our test suite where we use the pattern of\n`test ... && test_expect_succeess` to conditionally execute a test. The\nproblem is that when we decide to not execute the test, we'll indeed\nskip the test, but the overall statement will also be unsuccessful. This\nwill become a problem once we enable `set -e`.\n\nPrepare for this future by turning this into a proper conditional, which\nis also a bit easier to read overall.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t4032-diff-inter-hunk-context.sh | 12 +++++++-----\n t/t7450-bad-git-dotfiles.sh        | 24 +++++++++++++-----------\n 2 files changed, 20 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh\nindex c98eb6abb2..2d216fb70f 100755\n--- a/t/t4032-diff-inter-hunk-context.sh\n+++ b/t/t4032-diff-inter-hunk-context.sh\n@@ -40,11 +40,13 @@ t() {\n \t\ttest $(git $cmd $file | grep '^@@ ' | wc -l) = $hunks\n \t\"\n \n-\ttest -f $expected &&\n-\ttest_expect_success \"$label: check output\" \"\n-\t\tgit $cmd $file | grep -v '^index ' >actual &&\n-\t\ttest_cmp $expected actual\n-\t\"\n+\tif test -f $expected\n+\tthen\n+\t\ttest_expect_success \"$label: check output\" \"\n+\t\t\tgit $cmd $file | grep -v '^index ' >actual &&\n+\t\t\ttest_cmp $expected actual\n+\t\t\"\n+\tfi\n }\n \n cat <<EOF >expected.f1.0.1 || exit 1\ndiff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh\nindex f512eed278..8cc86522b2 100755\n--- a/t/t7450-bad-git-dotfiles.sh\n+++ b/t/t7450-bad-git-dotfiles.sh\n@@ -220,17 +220,19 @@ check_dotx_symlink () {\n \t\t)\n \t'\n \n-\ttest -n \"$refuse_index\" &&\n-\ttest_expect_success \"refuse to load symlinked $name into index ($type)\" '\n-\t\ttest_must_fail \\\n-\t\t\tgit -C $dir \\\n-\t\t\t    -c core.protectntfs \\\n-\t\t\t    -c core.protecthfs \\\n-\t\t\t    read-tree $tree 2>err &&\n-\t\tgrep \"invalid path.*$name\" err &&\n-\t\tgit -C $dir ls-files -s >out &&\n-\t\ttest_must_be_empty out\n-\t'\n+\tif test -n \"$refuse_index\"\n+\tthen\n+\t\ttest_expect_success \"refuse to load symlinked $name into index ($type)\" '\n+\t\t\ttest_must_fail \\\n+\t\t\t\tgit -C $dir \\\n+\t\t\t\t    -c core.protectntfs \\\n+\t\t\t\t    -c core.protecthfs \\\n+\t\t\t\t    read-tree $tree 2>err &&\n+\t\t\tgrep \"invalid path.*$name\" err &&\n+\t\t\tgit -C $dir ls-files -s >out &&\n+\t\t\ttest_must_be_empty out\n+\t\t'\n+\tfi\n }\n \n check_dotx_symlink gitmodules vanilla .gitmodules\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541634","messageId":"20260415-b4-pks-tests-with-set-e-v2-6-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 06/12] t: prepare execution of potentially failing commands for `set -e`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:39Z","receivedAt":"2026-04-15T13:06:55Z","isPatch":true,"body":"Several of our tests verify whether a certain binary can be executed,\npotentially skipping tests in case we cannot, for example because the\nbinary doesn't exist. In those cases we often run the binary outside of\nany conditionally.\n\nThis will start to fail once we enable `set -e`, as that will cause us\nto bail out the test immediately. Improve these tests by executing them\ninside of a conditional instead.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/lib-git-svn.sh                |  7 +++----\n t/lib-httpd.sh                  |  3 +--\n t/t3901-i18n-patch.sh           |  3 ++-\n t/t5000-tar-tree.sh             |  4 ++--\n t/t7422-submodule-output.sh     |  2 +-\n t/t9200-git-cvsexportcommit.sh  |  3 +--\n t/t9400-git-cvsserver-server.sh |  5 +++--\n t/t9401-git-cvsserver-crlf.sh   |  4 ++--\n t/t9402-git-cvsserver-refs.sh   |  4 ++--\n t/test-lib-functions.sh         |  3 +--\n t/test-lib.sh                   | 10 ++++++----\n 11 files changed, 24 insertions(+), 24 deletions(-)\n\ndiff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh\nindex 2fde2353fd..52843f667d 100644\n--- a/t/lib-git-svn.sh\n+++ b/t/lib-git-svn.sh\n@@ -15,8 +15,7 @@ GIT_SVN_DIR=$GIT_DIR/svn/refs/remotes/git-svn\n SVN_TREE=$GIT_SVN_DIR/svn-tree\n test_set_port SVNSERVE_PORT\n \n-svn >/dev/null 2>&1\n-if test $? -ne 1\n+if ! svn help >/dev/null 2>&1\n then\n \tskip_all='skipping git svn tests, svn not found'\n \ttest_done\n@@ -27,13 +26,13 @@ export svnrepo\n svnconf=$PWD/svnconf\n export svnconf\n \n+x=0\n perl -w -e \"\n use SVN::Core;\n use SVN::Repos;\n \\$SVN::Core::VERSION gt '1.1.0' or exit(42);\n system(qw/svnadmin create --fs-type fsfs/, \\$ENV{svnrepo}) == 0 or exit(41);\n-\" >&3 2>&4\n-x=$?\n+\" >&3 2>&4 || x=$?\n if test $x -ne 0\n then\n \tif test $x -eq 42; then\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 4c76e813e3..fc646447d5 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -235,11 +235,10 @@ start_httpd() {\n \n \ttest_atexit stop_httpd\n \n-\t\"$LIB_HTTPD_PATH\" -d \"$HTTPD_ROOT_PATH\" \\\n+\tif ! \"$LIB_HTTPD_PATH\" -d \"$HTTPD_ROOT_PATH\" \\\n \t\t-f \"$TEST_PATH/apache.conf\" $HTTPD_PARA \\\n \t\t-c \"Listen 127.0.0.1:$LIB_HTTPD_PORT\" -k start \\\n \t\t>&3 2>&4\n-\tif test $? -ne 0\n \tthen\n \t\tcat \"$HTTPD_ROOT_PATH\"/error.log >&4 2>/dev/null\n \t\ttest_skip_or_die GIT_TEST_HTTPD \"web server setup failed\"\ndiff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh\nindex f03601b49a..ef7d7e1edc 100755\n--- a/t/t3901-i18n-patch.sh\n+++ b/t/t3901-i18n-patch.sh\n@@ -28,7 +28,8 @@ check_encoding () {\n \t\t8859)\n \t\t\tgrep \"^encoding ISO8859-1\" ;;\n \t\t*)\n-\t\t\tgrep \"^encoding ISO8859-1\"; test \"$?\" != 0 ;;\n+\t\t\tret=0; grep \"^encoding ISO8859-1\" || ret=$?\n+\t\t\ttest \"$ret\" != 0 ;;\n \t\tesac || return 1\n \t\tj=$i\n \t\ti=$(($i+1))\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 5465054f17..a8c28533dc 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -503,8 +503,8 @@ test_expect_success LONG_IS_64BIT 'set up repository with huge blob' '\n # would generate the whole 64GB).\n test_expect_success LONG_IS_64BIT 'generate tar with huge size' '\n \t{\n-\t\tgit archive HEAD\n-\t\techo $? >exit-code\n+\t\t{ ret=0 && git archive HEAD || ret=$?; } &&\n+\t\techo \"$ret\" >exit-code\n \t} | test_copy_bytes 4096 >huge.tar &&\n \techo 141 >expect &&\n \ttest_cmp expect exit-code\ndiff --git a/t/t7422-submodule-output.sh b/t/t7422-submodule-output.sh\nindex aea1ddf117..852136fdfd 100755\n--- a/t/t7422-submodule-output.sh\n+++ b/t/t7422-submodule-output.sh\n@@ -198,7 +198,7 @@ test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE'\n \t(\n \t\tcd repo &&\n \t\tGIT_ALLOW_PROTOCOL=file git submodule add \"$(pwd)\"/../submodule &&\n-\t\t{ git submodule status --recursive 2>err; echo $?>status; } |\n+\t\t{ { ret=0 && git submodule status --recursive 2>err || ret=$?; } && echo $ret >status; } |\n \t\t\tgrep -q recursive-submodule-path-1 &&\n \t\ttest_must_be_empty err &&\n \t\ttest_match_signal 13 \"$(cat status)\"\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nindex 14cbe96527..581cf3d28f 100755\n--- a/t/t9200-git-cvsexportcommit.sh\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -11,8 +11,7 @@ if ! test_have_prereq PERL; then\n \ttest_done\n fi\n \n-cvs >/dev/null 2>&1\n-if test $? -ne 1\n+if ! cvs version >/dev/null 2>&1\n then\n     skip_all='skipping git cvsexportcommit tests, cvs not found'\n     test_done\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex e499c7f955..4b45398bab 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -17,12 +17,13 @@ if ! test_have_prereq PERL; then\n \tskip_all='skipping git cvsserver tests, perl not available'\n \ttest_done\n fi\n-cvs >/dev/null 2>&1\n-if test $? -ne 1\n+\n+if ! cvs version >/dev/null 2>&1\n then\n     skip_all='skipping git-cvsserver tests, cvs not found'\n     test_done\n fi\n+\n perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {\n     skip_all='skipping git-cvsserver tests, Perl SQLite interface unavailable'\n     test_done\ndiff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh\nindex a34805acdc..6b4cbb1651 100755\n--- a/t/t9401-git-cvsserver-crlf.sh\n+++ b/t/t9401-git-cvsserver-crlf.sh\n@@ -60,12 +60,12 @@ check_status_options() {\n     return $stat\n }\n \n-cvs >/dev/null 2>&1\n-if test $? -ne 1\n+if ! cvs version >/dev/null 2>&1\n then\n     skip_all='skipping git-cvsserver tests, cvs not found'\n     test_done\n fi\n+\n if ! test_have_prereq PERL\n then\n     skip_all='skipping git-cvsserver tests, perl not available'\ndiff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh\nindex 2ee41f9443..65f2ceedec 100755\n--- a/t/t9402-git-cvsserver-refs.sh\n+++ b/t/t9402-git-cvsserver-refs.sh\n@@ -68,12 +68,12 @@ check_diff() {\n \n #########\n \n-cvs >/dev/null 2>&1\n-if test $? -ne 1\n+if ! cvs version >/dev/null 2>&1\n then\n \tskip_all='skipping git-cvsserver tests, cvs not found'\n \ttest_done\n fi\n+\n if ! test_have_prereq PERL\n then\n \tskip_all='skipping git-cvsserver tests, perl not available'\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 5fd5494ef1..879ee1ee59 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1248,8 +1248,7 @@ test_might_fail () {\n test_expect_code () {\n \twant_code=$1\n \tshift\n-\t\"$@\" 2>&7\n-\texit_code=$?\n+\texit_code=0; \"$@\" 2>&7 || exit_code=$?\n \tif test $exit_code = $want_code\n \tthen\n \t\treturn 0\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 70fd3e9baf..de7d9e7b92 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -143,8 +143,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n ################################################################\n # It appears that people try to run tests without building...\n GIT_BINARY=\"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X\"\n-\"$GIT_BINARY\" >/dev/null\n-if test $? != 1\n+\n+if ! \"$GIT_BINARY\" version >/dev/null\n then\n \tif test -n \"$GIT_TEST_INSTALLED\"\n \tthen\n@@ -454,8 +454,10 @@ then\n \t# from any previous runs.\n \t>\"$GIT_TEST_TEE_OUTPUT_FILE\"\n \n-\t(GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} \"$0\" \"$@\" 2>&1;\n-\t echo $? >\"$TEST_RESULTS_BASE.exit\") | tee -a \"$GIT_TEST_TEE_OUTPUT_FILE\"\n+\t(\n+\t\tret=0 && GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} \"$0\" \"$@\" 2>&1 || ret=$?\n+\t\techo \"$ret\" >\"$TEST_RESULTS_BASE.exit\"\n+\t) | tee -a \"$GIT_TEST_TEE_OUTPUT_FILE\"\n \ttest \"$(cat \"$TEST_RESULTS_BASE.exit\")\" = 0\n \texit\n fi\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541635","messageId":"20260415-b4-pks-tests-with-set-e-v2-7-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 07/12] t: prepare `test_when_finished ()`/`test_atexit()` for `set -e`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:40Z","receivedAt":"2026-04-15T13:06:57Z","isPatch":true,"body":"Both `test_when_finished ()` and `test_atexit ()` build up a chain of\ncleanup commands by prepending each new command to the existing cleanup\nstring. To preserve the exit code of the test body across cleanup\nexecution, we append the following logic:\n\n    } && (exit \"$eval_ret\"); eval_ret=$?; ...\n\nThe intent of this is to run the cleanup block and then unconditionally\nrestore `eval_ret`. The original behaviour of this is is:\n\n   +------------------+---------+------------------------------------+\n   |test body         │ cleanup │ old behaviour                      │\n   +------------------+---------+------------------------------------+\n   │pass (eval_ret=0) | pass    │ && taken -> (exit 0) -> eval_ret=0 |\n   +------------------+---------+------------------------------------+\n   │pass (eval_ret=0) | fail    │ && not taken -> eval_ret=$?        |\n   +------------------+---------+------------------------------------+\n   │fail (eval_ret=1) | pass    │ && taken -> (exit 1) -> eval_ret=1 |\n   +------------------+---------+------------------------------------+\n   │fail (eval_ret=1) | fail    | && not taken -> eval_ret=$?        |\n   +------------------+---------+------------------------------------+\n\nThis logic will start to fail once we enable `set -e`. When `$eval_ret`\nis non-zero, the subshell we create will fail, and with `set -e` we'll\nthus bail out without evaluating the logic after the semicolon.\n\nFix this issue by instead using `|| eval_ret=\\$?; ...`. Besides being\na bit simpler, it also retains the original behaviour:\n\n   +------------------+---------+------------------------------------+\n   |test body         │ cleanup │ old behaviour                      │\n   +------------------+---------+------------------------------------+\n   │pass (eval_ret=0) | pass    │ || not taken -> eval_ret unchanged |\n   +------------------+---------+------------------------------------+\n   │pass (eval_ret=0) | fail    │ || taken -> eval_ret=$?            |\n   +------------------+---------+------------------------------------+\n   │fail (eval_ret=1) | pass    │ || not taken -> eval_ret unchanged |\n   +------------------+---------+------------------------------------+\n   │fail (eval_ret=1) | fail    | || taken -> eval_ret=$?            |\n   +------------------+---------+------------------------------------+\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/test-lib-functions.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 879ee1ee59..502bb0ddcb 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1512,7 +1512,7 @@ test_when_finished () {\n \ttest \"${BASH_SUBSHELL-0}\" = 0 ||\n \tBUG \"test_when_finished does nothing in a subshell\"\n \ttest_cleanup=\"{ $*\n-\t\t} && (exit \\\"\\$eval_ret\\\"); eval_ret=\\$?; $test_cleanup\"\n+\t\t} || eval_ret=\\$?; $test_cleanup\"\n }\n \n # This function can be used to schedule some commands to be run\n@@ -1540,7 +1540,7 @@ test_atexit () {\n \ttest \"${BASH_SUBSHELL-0}\" = 0 ||\n \tBUG \"test_atexit does nothing in a subshell\"\n \ttest_atexit_cleanup=\"{ $*\n-\t\t} && (exit \\\"\\$eval_ret\\\"); eval_ret=\\$?; $test_atexit_cleanup\"\n+\t\t} || eval_ret=\\$?; $test_atexit_cleanup\"\n }\n \n # Deprecated wrapper for \"git init\", use \"git init\" directly instead\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541636","messageId":"20260415-b4-pks-tests-with-set-e-v2-8-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 08/12] t0008: silence error in subshell when using `grep -v`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:41Z","receivedAt":"2026-04-15T13:07:02Z","isPatch":true,"body":"In t0008 we use `grep -v` in a subshell, but expect that this command\nwill sometimes not match anything. This would cause grep(1) to return an\nerror code, but given that we don't run with `set -e` we swallow this\nerror.\n\nWe're about to enable `set -e`. Prepare for this by ignoring any errors.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t0008-ignores.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex e716b5cdfa..d77a179bdd 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -122,8 +122,8 @@ test_expect_success_multiple () {\n \tfi\n \ttestname=\"$1\" expect_all=\"$2\" code=\"$3\"\n \n-\texpect_verbose=$( echo \"$expect_all\" | grep -v '^::\t' )\n-\texpect=$( echo \"$expect_verbose\" | sed -e 's/.*\t//' )\n+\texpect_verbose=$(echo \"$expect_all\" | grep -v '^::\t' || :)\n+\texpect=$(echo \"$expect_verbose\" | sed -e 's/.*\t//')\n \n \ttest_expect_success $prereq \"$testname${no_index_opt:+ with $no_index_opt}\" '\n \t\texpect \"$expect\" &&\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541637","messageId":"20260415-b4-pks-tests-with-set-e-v2-9-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 09/12] t1301: don't fail in case setfacl(1) doesn't exist or fails","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:42Z","receivedAt":"2026-04-15T13:07:03Z","isPatch":true,"body":"In t1301 we're trying to remove any potentially-existing default ACLs\nthat might exist on the transh directory by executing setfacl(1).\nAccording to 8ed0a740dd (t1301-shared-repo.sh: don't let a default ACL\ninterfere with the test, 2008-10-16), this is done because we play\naround with permissions and umasks in this test suite.\n\nThe setfacl(1) binary may not exist on some systems though, even though\ntests ultimately still pass. This doesn't matter currently, but will\ncause the test to fail once we start running with `set -e`. Silence such\nfailures by ignoring failures here.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t1301-shared-repo.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex 630a47af21..0e0d07a1a1 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -12,7 +12,7 @@ TEST_CREATE_REPO_NO_TEMPLATE=1\n . ./test-lib.sh\n \n # Remove a default ACL from the test dir if possible.\n-setfacl -k . 2>/dev/null\n+setfacl -k . 2>/dev/null || :\n \n # User must have read permissions to the repo -> failure on --shared=0400\n test_expect_success 'shared = 0400 (faulty permission u-w)' '\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541638","messageId":"20260415-b4-pks-tests-with-set-e-v2-10-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 10/12] t6002: fix use of `expr` with `set -e`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:43Z","receivedAt":"2026-04-15T13:07:06Z","isPatch":true,"body":"In `test_bisection_diff ()` we use `expr` to perform some math. This\ncommand has some gotchas though in that it will only return success when\nthe result is neither null nor zero. In some of our cases though it\nactually _is_ zero, and that will cause the expressions to fail once we\nenable `set -e`.\n\nPrepare for this change by instead using `$(( ))`, which doesn't have\nthe same issue. While at it, modernize the function a tiny bit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t6002-rev-list-bisect.sh | 17 ++++++++++-------\n 1 file changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\nindex daa009c9a1..f2de40b5ed 100755\n--- a/t/t6002-rev-list-bisect.sh\n+++ b/t/t6002-rev-list-bisect.sh\n@@ -27,13 +27,16 @@ test_bisection_diff()\n \t# Test if bisection size is close to half of list size within\n \t# tolerance.\n \t#\n-\t_bisect_err=$(expr $_list_size - $_bisection_size \\* 2)\n-\ttest \"$_bisect_err\" -lt 0 && _bisect_err=$(expr 0 - $_bisect_err)\n-\t_bisect_err=$(expr $_bisect_err / 2) ; # floor\n-\n-\ttest_expect_success \\\n-\t\"bisection diff $_bisect_option $_head $* <= $_max_diff\" \\\n-\t'test $_bisect_err -le $_max_diff'\n+\t_bisect_err=$(($_list_size - $_bisection_size * 2))\n+\tif test \"$_bisect_err\" -lt 0\n+\tthen\n+\t\t_bisect_err=$((0 - $_bisect_err))\n+\tfi\n+\t_bisect_err=$(($_bisect_err / 2)) ; # floor\n+\n+\ttest_expect_success \"bisection diff $_bisect_option $_head $* <= $_max_diff\" '\n+\t\ttest $_bisect_err -le $_max_diff\n+\t'\n }\n \n date >path0\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541639","messageId":"20260415-b4-pks-tests-with-set-e-v2-11-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 11/12] t9902: fix use of `read` with `set -e`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:44Z","receivedAt":"2026-04-15T13:07:07Z","isPatch":true,"body":"In t9902 we're using the `read` builtin to read some values into a\nvariable. This is done by using `-d \"\"`, which cause us to read until\nthe end of the heredoc. There is a gotcha though: when the delimiter\nisn't found at all, then the read builtin will return an error. This\nhasn't been an issue until now as we didn't run with `set -e`, but\nthat'll change in a subsequent commit.\n\nPrepare for this change by silencing the error.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t9902-completion.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 2f9a597ec7..e3a7df7691 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -590,7 +590,7 @@ test_expect_success '__gitcomp - doesnt fail because of invalid variable name' '\n \t__gitcomp \"$invalid_variable_name\"\n '\n \n-read -r -d \"\" refs <<-\\EOF\n+read -r -d \"\" refs <<-\\EOF || :\n main\n maint\n next\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541640","messageId":"20260415-b4-pks-tests-with-set-e-v2-12-4e4904a96f15@pks.im","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-0-4e4904a96f15@pks.im","subject":"[PATCH v2 12/12] t: detect errors outside of test cases","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-15T13:06:45Z","receivedAt":"2026-04-15T13:07:10Z","isPatch":true,"body":"We have recently merged a patch series that had a simple misspelling of\n`test_expect_success`. Instead of making our tests fail though, this\ntypo went completely undetected and all of our tests passed, which is of\ncourse unfortunate. This is a more general issue with our test suite:\nall commands that run outside of a specific test case can fail, and if\nwe don't explicitly check for such failure then this failure will be\nsilently ignored.\n\nImprove the status quo by enabling the errexit option so that any such\nunchecked failures will cause us to abort immediately.\n\nNote that for now, we only enable this option for Bash 5 and newer. This\nis because other shells have wildly different behaviour, and older\nversions of Bash (especially on macOS) are buggy. The list of enabled\nshells may be extended going forward.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/test-lib.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex de7d9e7b92..1f7868c537 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -15,6 +15,15 @@\n # You should have received a copy of the GNU General Public License\n # along with this program.  If not, see https://www.gnu.org/licenses/ .\n \n+# Enable the use of errexit so that any unexpected failures will cause us to\n+# abort tests, even when outside of a specific test case. Note that we only\n+# enable this on Bash 5 and newer, as `set -e` has wildly different behaviour\n+# across shells. The list of allowed shells may be extended going forward.\n+if test \"${BASH_VERSINFO:=0}\" -ge 5\n+then\n+\tset -e\n+fi\n+\n # Test the binaries we have just built.  The tests are kept in\n # t/ subdirectory and are run in 'trash directory' subdirectory.\n if test -z \"$TEST_DIRECTORY\"\n\n-- \n2.54.0.rc2.529.gd9106f7525.dirty\n\n"},{"id":"541714","messageId":"20260416060059.GC646814@coredump.intra.peff.net","threadId":"65487","inReplyTo":"20260415-b4-pks-tests-with-set-e-v2-12-4e4904a96f15@pks.im","subject":"Re: [PATCH v2 12/12] t: detect errors outside of test cases","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-16T06:00:59Z","receivedAt":"2026-04-16T06:01:00Z","isPatch":true,"body":"On Wed, Apr 15, 2026 at 03:06:45PM +0200, Patrick Steinhardt wrote:\n\n> Improve the status quo by enabling the errexit option so that any such\n> unchecked failures will cause us to abort immediately.\n> \n> Note that for now, we only enable this option for Bash 5 and newer. This\n> is because other shells have wildly different behaviour, and older\n> versions of Bash (especially on macOS) are buggy. The list of enabled\n> shells may be extended going forward.\n\nOK, we know that this does not cause false positives because all of the\ntests should pass. It would be nice if we could verify that it catches\nbugs, too. Doing this:\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex e4d32bb4d2..5521f21e64 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -980,4 +980,6 @@ test_expect_success 're-init reads matching includeIf.onbranch' '\n \ttest_cmp expect err\n '\n \n+test_expect_foobar 'baz'\n+\n test_done\n\nwill fail for me, but only if I specially ask to use bash, either\nmanually or by setting TEST_SHELL_PATH (since /bin/sh is dash on\nDebian). Is there something in both GitHub and GitLab CI that will\nreliably use an acceptable version of bash?\n\nI guess perhaps Windows, though I don't know what version is used there.\nBut should we maybe set TEST_SHELL_PATH in at least one of the linux\nbuilds?\n\n-Peff\n"},{"id":"541721","messageId":"aeC99qYToLuiyZco@pks.im","threadId":"65487","inReplyTo":"20260416060059.GC646814@coredump.intra.peff.net","subject":"Re: [PATCH v2 12/12] t: detect errors outside of test cases","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-16T10:46:14Z","receivedAt":"2026-04-16T10:46:24Z","isPatch":true,"body":"On Thu, Apr 16, 2026 at 02:00:59AM -0400, Jeff King wrote:\n> On Wed, Apr 15, 2026 at 03:06:45PM +0200, Patrick Steinhardt wrote:\n> \n> > Improve the status quo by enabling the errexit option so that any such\n> > unchecked failures will cause us to abort immediately.\n> > \n> > Note that for now, we only enable this option for Bash 5 and newer. This\n> > is because other shells have wildly different behaviour, and older\n> > versions of Bash (especially on macOS) are buggy. The list of enabled\n> > shells may be extended going forward.\n> \n> OK, we know that this does not cause false positives because all of the\n> tests should pass. It would be nice if we could verify that it catches\n> bugs, too. Doing this:\n> \n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index e4d32bb4d2..5521f21e64 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -980,4 +980,6 @@ test_expect_success 're-init reads matching includeIf.onbranch' '\n>  \ttest_cmp expect err\n>  '\n>  \n> +test_expect_foobar 'baz'\n> +\n>  test_done\n> \n> will fail for me, but only if I specially ask to use bash, either\n> manually or by setting TEST_SHELL_PATH (since /bin/sh is dash on\n> Debian). Is there something in both GitHub and GitLab CI that will\n> reliably use an acceptable version of bash?\n> \n> I guess perhaps Windows, though I don't know what version is used there.\n> But should we maybe set TEST_SHELL_PATH in at least one of the linux\n> builds?\n\nOur Fedora-based builds use Bash 5.3.0, so we at least have some test\ncoverage [1]. But I agree that it would make sense to maybe also make\none of our Ubuntu-based builds use Bash explicitly instead of Dash.\n\nPatrick\n\n[1]: https://gitlab.com/gitlab-org/git/-/jobs/13947942805\n"}]}