Junio C Hamano <gitster@xxxxxxxxx> writes: > ... > 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. Nah, it seems that t3600-rm's "match signal 13" is already broken without O_PIPEFAIL patch on Windows. For example: https://github.com/git/git/runs/1753231308?check_suite_focus=true#step:7:36912 This was introduced by c15ffae5 (rm tests: actually test for SIGPIPE in SIGPIPE test, 2021-01-16) in the same series. I am not sure "actually testing for SIGPIPE" is more important than "make sure 'git rm' choked should not die with cruft", so without thinking too deeply about the issue, my gut reaction is that reverting is better than using !MINGW as other tests. That is, no matter how "git rm" gets killed, it should not leave .git/index.lock behind, and the original already tests that. The patch tried to make sure it dies with signal #13 (and fails the test on Windows) before it even looks at the leftover index.lock file, which feels a bit backwards.
