patcht7614: avoid hiding git's exit code in a pipe
3 messages between Jul 15, 2026 and Jul 16, 2026, from Shlok Kulshreshtha, Junio C Hamano.
Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.
Shlok KulshreshthaJul 15, 2026, 11:33 UTC on loreThe exit code of the upstream command in a pipe is ignored, so in
git cat-file commit HEAD | sed -e "1,/^\$/d" >actual
a crash of "git cat-file" would go unnoticed: the exit code of the pipeline is that of "sed", which happily succeeds on empty input. The test would thus pass even though "git cat-file" failed.
Write the output of "git cat-file" to a file first and run "sed" on that file, so that the exit codes of both commands are checked by the &&-chain.
Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com>
---
This is a microproject ("Avoid suppressing git's exit code in test
scripts"), applying the same fix as c6f44e1da5 (t9813: avoid using
pipes) to another script. A search of the list did not turn up anyone
working on t7614; please let me know if it is already taken. t/t7614-merge-signoff.sh | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
Show changes to t/t7614-merge-signoff.sh +6 −3
diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh
index fee258d4f0..e58bf07b7a 100755
--- a/t/t7614-merge-signoff.sh
+++ b/t/t7614-merge-signoff.sh
@@ -45,7 +45,8 @@ test_expect_success 'git merge --signoff adds a sign-off line' '
test_commit main-branch-2 file2 2 &&
git checkout other-branch &&
git merge main --signoff --no-edit &&
- git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+ git cat-file commit HEAD >commit &&
+ sed -e "1,/^\$/d" commit >actual &&
test_cmp expected-signed actual
'
@@ -55,7 +56,8 @@ test_expect_success 'git merge does not add a sign-off line' '
test_commit main-branch-3 file3 3 &&
git checkout other-branch &&
git merge main --no-edit &&
- git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+ git cat-file commit HEAD >commit &&
+ sed -e "1,/^\$/d" commit >actual &&
test_cmp expected-unsigned actual
'
@@ -65,7 +67,8 @@ test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
test_commit main-branch-4 file4 4 &&
git checkout other-branch &&
git merge main --no-edit --signoff --no-signoff &&
- git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+ git cat-file commit HEAD >commit &&
+ sed -e "1,/^\$/d" commit >actual &&
test_cmp expected-unsigned actual
'
--
2.52.0
Re: [PATCH] t7614: avoid hiding git's exit code in a pipe
Shlok Kulshreshtha <diy2903@gmail.com> writes:
Show 18 quoted lines
> The exit code of the upstream command in a pipe is ignored, so in
>
> git cat-file commit HEAD | sed -e "1,/^\$/d" >actual
>
> a crash of "git cat-file" would go unnoticed: the exit code of the
> pipeline is that of "sed", which happily succeeds on empty input. The
> test would thus pass even though "git cat-file" failed.
>
> Write the output of "git cat-file" to a file first and run "sed" on
> that file, so that the exit codes of both commands are checked by the
> &&-chain.
>
> Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com>
> ---
> This is a microproject ("Avoid suppressing git's exit code in test
> scripts"), applying the same fix as c6f44e1da5 (t9813: avoid using
> pipes) to another script. A search of the list did not turn up anyone
> working on t7614; please let me know if it is already taken.All look trivially correct.
Two clean-up possibilities that are clearly outside the scope of this patch are
* There is no need to backslash-quote the dollar sign.
t7604-merge-custom-message.sh next door uses "1,/^$/d" just fine.
* This "cat-file the commit object, and strip away the object
header with sed" pattern appears quite often throughout the test
suite.
$ git grep -B1 -e 'sed -e "1,/^\\*$/d"' t/
shows quite a few hits. It might make sense to give them an easy
to use helper script
commit_body () {
git cat-file commit "$1" >.commit &&
sed -e "1,/^$/d" .commit &&
rm -f .commit
} or something like that.
But again, these are clearly outside the scope of this patch.
Show 37 quoted lines
>
> t/t7614-merge-signoff.sh | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh
> index fee258d4f0..e58bf07b7a 100755
> --- a/t/t7614-merge-signoff.sh
> +++ b/t/t7614-merge-signoff.sh
> @@ -45,7 +45,8 @@ test_expect_success 'git merge --signoff adds a sign-off line' '
> test_commit main-branch-2 file2 2 &&
> git checkout other-branch &&
> git merge main --signoff --no-edit &&
> - git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + git cat-file commit HEAD >commit &&
> + sed -e "1,/^\$/d" commit >actual &&
> test_cmp expected-signed actual
> '
>
> @@ -55,7 +56,8 @@ test_expect_success 'git merge does not add a sign-off line' '
> test_commit main-branch-3 file3 3 &&
> git checkout other-branch &&
> git merge main --no-edit &&
> - git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + git cat-file commit HEAD >commit &&
> + sed -e "1,/^\$/d" commit >actual &&
> test_cmp expected-unsigned actual
> '
>
> @@ -65,7 +67,8 @@ test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
> test_commit main-branch-4 file4 4 &&
> git checkout other-branch &&
> git merge main --no-edit --signoff --no-signoff &&
> - git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + git cat-file commit HEAD >commit &&
> + sed -e "1,/^\$/d" commit >actual &&
> test_cmp expected-unsigned actual
> '
Re: [PATCH] t7614: avoid hiding git's exit code in a pipe
Junio C Hamano <gitster@pobox.com> writes:
> All look trivially correct.
Show 7 quoted lines
> * This "cat-file the commit object, and strip away the object
> header with sed" pattern appears quite often throughout the test
> suite.
> [...]
> or something like that.
>
> But again, these are clearly outside the scope of this patch.
Agreed on keeping them out of this patch. I'd like to take up the commit_body() helper as a separate follow-up once this lands, and convert the existing "cat-file ... | sed" call sites (including the unnecessary backslash before the dollar sign) over to it.
Thanks, Shlok