Re: [PATCH 01/12] t: prepare `test_match_signal ()` calls for `set -e`

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> We have a couple of calls to `test_match_signal ()` where we execute a
> Git command and expect it to die with a specific signal. These calls
> will essentially execute the process in a subshell via `foo; echo $?`,
> but as we expect `foo` to fail this will cause the overall subshell to
> fail once we `set -e`.
>
> Fix this issue by using `foo || echo $?` instead.

Hmph, if the 'foo' command that is expected to fail succeeds by a
bug, we won't see 0 on the standard output anymore.  Comparing 13
with 0 or 13 with an empty string "" would fail either way, so that
may not give us a practical difference, but this somehow leaves a
foul small in my nose.

The technique used in <20260325062114.2067946-2-gitster@pobox.com>
would give us something ugly like

-	OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&
+	OUT=$( ((st=0; large_git || st=$?; echo $st 1>&3) | :) 3>&1 ) &&

and it ensures that we give 0 when we succeed, but I am not sure if
this is worth doing.

Or perhaps

-	OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&
+	OUT=$( ((large_git || echo $? 1>&3) | :) 3>&1 ) &&
+	OUT=${OUT:-0} &&

I dunno.

If "large_git" somehow writes into file descriptor #3, then OUT will
be contaminated with something other than $?, so the additional "if
empty substitute with 0" would not work without doing something like

-	OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&
+	OUT=$( ((large_git 3>/dev/null || echo $? 1>&3) | :) 3>&1 ) &&
+	OUT=${OUT:-0} &&

I guess.  For that matter, if large_git spits out "13" to its file
descriptor #3, I do not know what would have happened ;-)

>  test_expect_success !MINGW 'a constipated git dies with SIGPIPE' '
> -	OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) &&
> +	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' '
> -	OUT=$( ((trap "" PIPE && large_git; echo $? 1>&3) | :) 3>&1 ) &&
> +	OUT=$( ((trap "" PIPE && large_git || echo $? 1>&3) | :) 3>&1 ) &&
>  	test_match_signal 13 "$OUT"
>  '
>  
> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
> index 1f16e6b522..8e68a00dcb 100755
> --- a/t/t3600-rm.sh
> +++ b/t/t3600-rm.sh
> @@ -260,7 +260,7 @@ test_expect_success 'choking "git rm" should not let it die with cruft (induce S
>  
>  test_expect_success !MINGW 'choking "git rm" should not let it die with cruft (induce and check SIGPIPE)' '
>  	choke_git_rm_setup &&
> -	OUT=$( ((trap "" PIPE && git rm -n "some-file-*"; echo $? 1>&3) | :) 3>&1 ) &&
> +	OUT=$( ((trap "" PIPE && git rm -n "some-file-*" || echo $? 1>&3) | :) 3>&1 ) &&
>  	test_match_signal 13 "$OUT" &&
>  	test_path_is_missing .git/index.lock
>  '