An unstated aim of v1 of this series was to fix up the tests on
vanilla bash's "set -o pipefail" enough that the test suite would have
some failures, but wouldn't look like a complete dumpster fire.
But this was confusing and relied on a side-quest to change the
test_{must,might}_fail helpers. See
https://lore.kernel.org/git/YAFntgQE3NZ3yQx5@xxxxxxxxxxxxxxxxxxxxxxx/
I've now ejected all of that, in favor of just fixing some of the
tests instead as Jeff suggested. Jeff, I added your Signed-off-by to
06/11 which you're mostly the author of. Please Ack that you're OK
with that (the original diff-for-discussin didn't have a SOB).
Jeff King (1):
git-svn tests: rewrite brittle tests to use "--[no-]merges".
Ævar Arnfjörð Bjarmason (10):
cache-tree tests: remove unused $2 parameter
cache-tree tests: use a sub-shell with less indirection
cache-tree tests: refactor overly complex function
git svn mergeinfo tests: modernize redirection & quoting style
git svn mergeinfo tests: refactor "test -z" to use test_must_be_empty
rm tests: actually test for SIGPIPE in SIGPIPE test
upload-pack tests: avoid a non-zero "grep" exit status
archive tests: use a cheaper "zipinfo -h" invocation to get header
tests: split up bash detection library
tests: add a "set -o pipefail" for a patched bash
t/README | 5 ++++
t/lib-bash-detection.sh | 8 ++++++
t/lib-bash.sh | 4 ++-
t/t0000-basic.sh | 4 +++
t/t0005-signals.sh | 4 +--
t/t0090-cache-tree.sh | 31 +++++++--------------
t/t3600-rm.sh | 7 +++--
t/t5000-tar-tree.sh | 2 +-
t/t5004-archive-corner-cases.sh | 3 ++-
t/t5703-upload-pack-ref-in-want.sh | 6 ++++-
t/t9151-svn-mergeinfo.sh | 43 ++++++++++++++----------------
t/t9902-completion.sh | 5 ++++
t/test-lib.sh | 29 ++++++++++++++++++++
13 files changed, 99 insertions(+), 52 deletions(-)
create mode 100644 t/lib-bash-detection.sh
Range-diff:
1: d950fbb967 < -: ---------- test-lib: add tests for test_might_fail
2: 1a0ffb1159 < -: ---------- test-lib: add ok=* support to test_might_fail
3: f7eaceeb3e < -: ---------- test_lib: allow test_{must,might}_fail to accept non-git on "sigpipe"
4: 0e77779947 < -: ---------- tests: use "test_might_fail ok=sigpipe grep" when appropriate
-: ---------- > 1: 8e8e03fa3d cache-tree tests: remove unused $2 parameter
-: ---------- > 2: 828d25533c cache-tree tests: use a sub-shell with less indirection
-: ---------- > 3: fefdc570a5 cache-tree tests: refactor overly complex function
-: ---------- > 4: a16938e58d git svn mergeinfo tests: modernize redirection & quoting style
-: ---------- > 5: b520656240 git svn mergeinfo tests: refactor "test -z" to use test_must_be_empty
-: ---------- > 6: f2e70ac911 git-svn tests: rewrite brittle tests to use "--[no-]merges".
-: ---------- > 7: dcf001e165 rm tests: actually test for SIGPIPE in SIGPIPE test
-: ---------- > 8: 2212fa65eb upload-pack tests: avoid a non-zero "grep" exit status
-: ---------- > 9: 8167c2e346 archive tests: use a cheaper "zipinfo -h" invocation to get header
5: c3916b8e7b = 10: 30c454ae7c tests: split up bash detection library
6: 4a988d1c73 ! 11: 6f290f850c tests: add a "set -o pipefail" for a patched bash
@@ Commit message
wind with current bash semantics of failing on SIGPIPE.
This series relies on a patch of mine to bash, which I'm submitting
- upstream. Vanilla bash ignores SIGPIPE under "set -e" since version
- 3.1. It's only under "set -o pipefail" (added in 3.2) that it doesn't
- take account of SIGPIPE, in a seeming omission nobody bothered to fix
- yet.
+ upstream, while not breaking anything for vanilla bash users. They
+ won't have GIT_TEST_PIPEFAIL turned on for them, and will only get
+ breakages if they turn it on explicitly with "GIT_TEST_PIPEFAIL=true".
+
+ Vanilla bash ignores SIGPIPE under "set -e" since version 3.1. It's
+ only under "set -o pipefail" (added in 3.2) that it doesn't take
+ account of SIGPIPE, in a seeming omission nobody bothered to fix yet.
Patching bash[4] with:
@@ Commit message
}
while (p != jobs[job]->pipe);
- Makes it useful for something like the git test suite. With vanilla
- bash and GIT_TEST_PIPEFAIL=true we'll fail 4 tests in my one-off test.
+ Makes it useful for something like the git test suite.
- With my patched bash the only tests we need to skip are those that are
- explicitly testing that a piped command returned SIGPIPE.
+ Under this test mode we only tests we need to skip those tests which
+ are explicitly testing that a piped command returned SIGPIPE. Those
+ tests will now return 0 instead of an exit code indicating SIGPIPE.
- As Jeff noted in [3] that count isn't reliable, as more will fail in a
- way that's hard to reproduce due to the racy nature of vanilla "set -o
- pipefail"
+ Forcing the mode to run under vanilla bash with
+ "GIT_TEST_PIPEFAIL=true" doesn't fail any tests for me, except the
+ test in t0000-basic.sh which explicitly checks for the desired
+ pipefail semantics. However, as Jeff noted in [3] that absence of
+ failure isn't reliable. I might not see some of the failures due to
+ the racy nature of how vanilla "set -o pipefail" interacts with *nix
+ pipe semantics.
1. a378fee5b0 (Documentation: add shell guidelines, 2018-10-05)
2. https://lore.kernel.org/git/cover.1573779465.git.liu.denton@xxxxxxxxx/
@@ t/README: GIT_TEST_DEFAULT_HASH=<hash-algo> specifies which hash algorithm to
+GIT_TEST_PIPEFAIL=<boolean>, when true, run 'set -o pipefail' to catch
+failures in commands that aren't the last in a pipe. Defaults to true
+on bash versions which know how to ignore SIGPIPE failures under the
-+'set -o pipefail' mode (as of 2021-01-14 only in an out-of-tree patch
-+to bash).
++'set -o pipefail' mode.
+
Naming Tests
------------
## t/t0000-basic.sh ##
-@@ t/t0000-basic.sh: test_expect_success 'test_{must,might}_fail accept non-git on "sigpipe"' '
- test_cmp badobjects out
+@@ t/t0000-basic.sh: test_expect_success 'test_must_fail rejects a non-git command with env' '
+ grep -F "test_must_fail: only '"'"'git'"'"' is allowed" err
'
-+test_expect_failure BASH_SET_O_PIPEFAIL 'test_{must,might}_fail ok=sigpipe under bash "set -o pipefail"' '
-+ grep string </dev/null | true
-+'
-+
-+test_expect_failure BASH_SET_O_PIPEFAIL 'test_{must,might}_fail ok=sigpipe under bash "set -o pipefail"' '
-+ test_must_fail grep string </dev/null | true &&
-+ test_might_fail grep string </dev/null | true
-+'
-+
-+test_expect_success BASH_SET_O_PIPEFAIL 'test_{must,might}_fail ok=sigpipe under bash "set -o pipefail"' '
-+ test_must_fail ok=sigpipe grep string </dev/null | true &&
-+ test_might_fail ok=sigpipe grep string </dev/null | true
++test_expect_success BASH_SET_O_PIPEFAIL 'our bash under "set -o pipefail" mode ignores SIGPIPE failures' '
++ yes | head -n 1 | true
+'
+
test_done
@@ t/t0005-signals.sh: test_expect_success 'create blob' '
test_match_signal 13 "$OUT"
'
+ ## t/t3600-rm.sh ##
+@@ t/t3600-rm.sh: test_expect_success 'choking "git rm" should not let it die with cruft' '
+ i=$(( $i + 1 ))
+ done | git update-index --index-info &&
+ OUT=$( ((trap "" PIPE; git rm -n "some-file-*"; echo $? 1>&3) | :) 3>&1 ) &&
+- test_match_signal 13 "$OUT" &&
++ if ! test_have_prereq BASH_SET_O_PIPEFAIL
++ then
++ test_match_signal 13 "$OUT"
++ fi &&
+ test_path_is_missing .git/index.lock
+ '
+
+
## t/t5000-tar-tree.sh ##
@@ t/t5000-tar-tree.sh: test_expect_success LONG_IS_64BIT 'set up repository with huge blob' '
--
2.29.2.222.g5d2a92d10f8
