On Sat, Jan 23 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> -test_expect_success !MINGW 'a constipated git dies with SIGPIPE' ' >> +test_expect_success !MINGW,!BASH_SET_O_PIPEFAIL 'a constipated git dies with SIGPIPE' ' >> OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) && >> test_match_signal 13 "$OUT" >> ' >> >> -test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' ' >> +test_expect_success !MINGW,!BASH_SET_O_PIPEFAIL 'a constipated git dies with SIGPIPE even if parent ignores it' ' >> OUT=$( ((trap "" PIPE; large_git; echo $? 1>&3) | :) 3>&1 ) && >> test_match_signal 13 "$OUT" >> ' > > The above have already !MINGW > >> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh >> index 4f7e62d05c..7b5d92add5 100755 >> --- a/t/t3600-rm.sh >> +++ b/t/t3600-rm.sh >> @@ -251,7 +251,10 @@ 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 >> ' > > but this one does not. Yet, we've been using test_match_signal on 13 > without issues, it appears. > > And somehow with the lazy prereq on SET_O_PIPEFAIL, this part starts > to break, like so: > > https://github.com/git/git/runs/1752687552?check_suite_focus=true#step:7:37042 > > The output captured in OUT is 0 as we can see on #37032 in the test > log. > > I am tempted to eject 11/11 and probably 10/11 out of the topic, as > the earlier patches before them look more or less uncontroversial > cleanups, and 11/11 seems to be more trouble than it is worth at > this moment. I think that makes sense, once I fix the breakage on 07/11 you noted downthread. > It's not like this would allow us to loosen the rule that we > shouldn't put a "git" invocation of the git subcommand being tested > on the upstream side of a pipe[...] FWIW it seems from my off-list discussion with the bash maintainer that no version of my patch is likely to make it into bash. He views it as a feature that "pipefail" treats all non-zero exit codes equally. But as it pertains to our test suite I mainly wrote this to check if we had any failures that didn't make sense once SIGPIPE was ignored. I think smoking out any potential historical cases (and finding we didn't have any that mattered) was probably an effort worth it in itself. Then we just have to continue not putting git on the LHS on a pipe for new tests. > [...]---not everybody is running bash, and it is unrealistic to tell > our developers "if you want to make sure your tests are good, you must > install and use this patched bash". I think if others think this was worth keeping and bash never accepted the patches we could rather easily get most of the benefit from it by having a CI job that ran with such a patched bash. To test with "set -o pipefail" it's enough that it's done semi-regularly by someone if you want to smoke out bugs in the tests, not everyone has to do it all the time.
