Re: [PATCH] t3920: don't ignore errors of more than one command with `|| true`

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 22, 2022, 22:24 UTC
Message-ID: <221122.86mt8iaamz.gmgdl@evledraar.gmail.com>
In-Reply-To: <febcfb0a-c410-fb71-cff9-92acfcb269e2@kdbg.org>

On Mon, Nov 21 2022, Johannes Sixt wrote:

Show 5 quoted lines

> It is customary to write `A || true` to ignore a potential error exit of
> command A. But when we have a sequence `A && B && C || true && D`, then
> a failure of any of A, B, or C skips to D right away. This is not
> intended here. Turn the command whose failure is to be ignored into a
> compound command to ensure it is the only one that is allowed to fail.

Show 18 quoted lines

> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  t/t3920-crlf-messages.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t3920-crlf-messages.sh b/t/t3920-crlf-messages.sh
> index 4c661d4d54..a58522c163 100755
> --- a/t/t3920-crlf-messages.sh
> +++ b/t/t3920-crlf-messages.sh
> @@ -12,7 +12,7 @@ create_crlf_ref () {
>  	cat >.crlf-orig-$branch.txt &&
>  	cat .crlf-orig-$branch.txt | append_cr >.crlf-message-$branch.txt &&
>  	grep 'Subject' .crlf-orig-$branch.txt | tr '\n' ' ' | sed 's/[ ]*$//' | tr -d '\n' >.crlf-subject-$branch.txt &&
> -	grep 'Body' .crlf-message-$branch.txt >.crlf-body-$branch.txt || true &&
> +	{ grep 'Body' .crlf-message-$branch.txt >.crlf-body-$branch.txt || true; } &&
>  	LIB_CRLF_BRANCHES="${LIB_CRLF_BRANCHES} ${branch}" &&
>  	test_tick &&
>  	hash=$(git commit-tree HEAD^{tree} -p HEAD -F .crlf-message-${branch}.txt) &&

Any reason not to make this:

	-       grep 'Body' .crlf-message-$branch.txt >.crlf-body-$branch.txt || true &&
	+       sed -ne '/Body/p' <.crlf-message-$branch.txt >.crlf-body-$branch.txt &&

We usually use that for "grep when we don't care about the exit code". But maybe some CRLF concerns in this code don't allow it (I only tested this on *nix).