{"thread":{"id":"66004","subject":"[PATCH] t7614: avoid hiding git's exit code in a pipe","startedAt":"2026-07-15T11:34:00Z","lastAt":"2026-07-16T07:13:11Z","messageCount":3,"participants":["Shlok Kulshreshtha","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"548274","messageId":"20260715113344.3490-1-diy2903@gmail.com","threadId":"66004","inReplyTo":null,"subject":"[PATCH] t7614: avoid hiding git's exit code in a pipe","fromName":"Shlok Kulshreshtha","fromEmail":"diy2903@gmail.com","sentAt":"2026-07-15T11:33:44Z","receivedAt":"2026-07-15T11:34:00Z","isPatch":true,"body":"The exit code of the upstream command in a pipe is ignored, so in\n\n\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual\n\na crash of \"git cat-file\" would go unnoticed: the exit code of the\npipeline is that of \"sed\", which happily succeeds on empty input. The\ntest would thus pass even though \"git cat-file\" failed.\n\nWrite the output of \"git cat-file\" to a file first and run \"sed\" on\nthat file, so that the exit codes of both commands are checked by the\n&&-chain.\n\nSigned-off-by: Shlok Kulshreshtha <diy2903@gmail.com>\n---\nThis is a microproject (\"Avoid suppressing git's exit code in test\nscripts\"), applying the same fix as c6f44e1da5 (t9813: avoid using\npipes) to another script. A search of the list did not turn up anyone\nworking on t7614; please let me know if it is already taken.\n\n t/t7614-merge-signoff.sh | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh\nindex fee258d4f0..e58bf07b7a 100755\n--- a/t/t7614-merge-signoff.sh\n+++ b/t/t7614-merge-signoff.sh\n@@ -45,7 +45,8 @@ test_expect_success 'git merge --signoff adds a sign-off line' '\n \ttest_commit main-branch-2 file2 2 &&\n \tgit checkout other-branch &&\n \tgit merge main --signoff --no-edit &&\n-\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\tgit cat-file commit HEAD >commit &&\n+\tsed -e \"1,/^\\$/d\" commit >actual &&\n \ttest_cmp expected-signed actual\n '\n \n@@ -55,7 +56,8 @@ test_expect_success 'git merge does not add a sign-off line' '\n \ttest_commit main-branch-3 file3 3 &&\n \tgit checkout other-branch &&\n \tgit merge main --no-edit &&\n-\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\tgit cat-file commit HEAD >commit &&\n+\tsed -e \"1,/^\\$/d\" commit >actual &&\n \ttest_cmp expected-unsigned actual\n '\n \n@@ -65,7 +67,8 @@ test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '\n \ttest_commit main-branch-4 file4 4 &&\n \tgit checkout other-branch &&\n \tgit merge main --no-edit --signoff --no-signoff &&\n-\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\tgit cat-file commit HEAD >commit &&\n+\tsed -e \"1,/^\\$/d\" commit >actual &&\n \ttest_cmp expected-unsigned actual\n '\n \n-- \n2.52.0\n\n"},{"id":"548321","messageId":"xmqq1pd4m4ea.fsf@gitster.g","threadId":"66004","inReplyTo":"20260715113344.3490-1-diy2903@gmail.com","subject":"Re: [PATCH] t7614: avoid hiding git's exit code in a pipe","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-15T19:36:29Z","receivedAt":"2026-07-15T19:36:31Z","isPatch":true,"body":"Shlok Kulshreshtha <diy2903@gmail.com> writes:\n\n> The exit code of the upstream command in a pipe is ignored, so in\n>\n> \tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual\n>\n> a crash of \"git cat-file\" would go unnoticed: the exit code of the\n> pipeline is that of \"sed\", which happily succeeds on empty input. The\n> test would thus pass even though \"git cat-file\" failed.\n>\n> Write the output of \"git cat-file\" to a file first and run \"sed\" on\n> that file, so that the exit codes of both commands are checked by the\n> &&-chain.\n>\n> Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com>\n> ---\n> This is a microproject (\"Avoid suppressing git's exit code in test\n> scripts\"), applying the same fix as c6f44e1da5 (t9813: avoid using\n> pipes) to another script. A search of the list did not turn up anyone\n> working on t7614; please let me know if it is already taken.\n\nAll look trivially correct.\n\nTwo clean-up possibilities that are clearly outside the scope of\nthis patch are\n\n * There is no need to backslash-quote the dollar sign.\n   t7604-merge-custom-message.sh next door uses \"1,/^$/d\" just fine.\n\n * This \"cat-file the commit object, and strip away the object\n   header with sed\" pattern appears quite often throughout the test\n   suite.\n\n   $ git grep -B1 -e 'sed -e \"1,/^\\\\*$/d\"' t/\n\n   shows quite a few hits.  It might make sense to give them an easy\n   to use helper script\n\n\tcommit_body () {\n\t\tgit cat-file commit \"$1\" >.commit &&\n\t\tsed -e \"1,/^$/d\" .commit &&\n\t\trm -f .commit\n\t}\n\n   or something like that.\n\nBut again, these are clearly outside the scope of this patch.\n\n>\n>  t/t7614-merge-signoff.sh | 9 ++++++---\n>  1 file changed, 6 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh\n> index fee258d4f0..e58bf07b7a 100755\n> --- a/t/t7614-merge-signoff.sh\n> +++ b/t/t7614-merge-signoff.sh\n> @@ -45,7 +45,8 @@ test_expect_success 'git merge --signoff adds a sign-off line' '\n>  \ttest_commit main-branch-2 file2 2 &&\n>  \tgit checkout other-branch &&\n>  \tgit merge main --signoff --no-edit &&\n> -\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n> +\tgit cat-file commit HEAD >commit &&\n> +\tsed -e \"1,/^\\$/d\" commit >actual &&\n>  \ttest_cmp expected-signed actual\n>  '\n>  \n> @@ -55,7 +56,8 @@ test_expect_success 'git merge does not add a sign-off line' '\n>  \ttest_commit main-branch-3 file3 3 &&\n>  \tgit checkout other-branch &&\n>  \tgit merge main --no-edit &&\n> -\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n> +\tgit cat-file commit HEAD >commit &&\n> +\tsed -e \"1,/^\\$/d\" commit >actual &&\n>  \ttest_cmp expected-unsigned actual\n>  '\n>  \n> @@ -65,7 +67,8 @@ test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '\n>  \ttest_commit main-branch-4 file4 4 &&\n>  \tgit checkout other-branch &&\n>  \tgit merge main --no-edit --signoff --no-signoff &&\n> -\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n> +\tgit cat-file commit HEAD >commit &&\n> +\tsed -e \"1,/^\\$/d\" commit >actual &&\n>  \ttest_cmp expected-unsigned actual\n>  '\n"},{"id":"548370","messageId":"20260716071254.14953-1-diy2903@gmail.com","threadId":"66004","inReplyTo":"xmqq1pd4m4ea.fsf@gitster.g","subject":"Re: [PATCH] t7614: avoid hiding git's exit code in a pipe","fromName":"Shlok Kulshreshtha","fromEmail":"diy2903@gmail.com","sentAt":"2026-07-16T07:12:54Z","receivedAt":"2026-07-16T07:13:11Z","isPatch":true,"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> All look trivially correct.\n\nThanks for the review.\n\n>  * This \"cat-file the commit object, and strip away the object\n>    header with sed\" pattern appears quite often throughout the test\n>    suite.\n> [...]\n>    or something like that.\n>\n> But again, these are clearly outside the scope of this patch.\n\nAgreed on keeping them out of this patch.  I'd like to take up the\ncommit_body() helper as a separate follow-up once this lands, and\nconvert the existing \"cat-file ... | sed\" call sites (including the\nunnecessary backslash before the dollar sign) over to it.\n\nThanks,\nShlok\n"}]}