Re: [PATCH v2 0/3] diff: build parseopts array on demand

From: "René Scharfe" <l.s.r@web.de>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Git List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>
Subject: Re: [PATCH v2 0/3] diff: build parseopts array on demand
Date: Thu, 1 Dec 2022 20:43:04 +0100	[thread overview]
Message-ID: <3d9ccbda-201b-fdaf-5fa3-224ba10b8c40@web.de> (raw)
In-Reply-To: <CAPig+cTdSNfWwN-c7R3-MF6ipAD6-7aVjxRK9f9XaKzg4yhhyg@mail.gmail.com>

Am 01.12.2022 um 20:19 schrieb Eric Sunshine:
> On Thu, Dec 1, 2022 at 2:11 PM René Scharfe <l.s.r@web.de> wrote:
>> t4205-log-pretty-formats.sh                      (Wstat: 256 Tests: 21 Failed: 0)
>>   Non-zero exit status: 1
>>   Parse errors: No plan found in TAP output
>>
>> The TAP error in t4205-log-pretty-formats.sh is fixed by the following
>> patch, but I can't explain it:
>>
>> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
>> @@ -156,7 +156,7 @@ test_expect_success 'NUL termination with --reflog --pretty=oneline' '
>>         for r in $revs
>>         do
>>                 git show -s --pretty=oneline "$r" >raw &&
>> -               cat raw | lf_to_nul || exit 1
>> +               cat raw | lf_to_nul || return 1
>>         done >expect &&
>
> Makes sense. The `exit 1` undesirably causes the entire script to
> abort, which means test_done() is never invoked, whereas `return 1`
> makes only the test fail. `exit 1` would be appropriate inside a
> subshell but there is no subshell here.

Right.  Here's how I got confused:

What's failing?  "cat raw" works in the trash directory.  lf_to_nul is
not found.  Of course, it's defined in test-lib-functions.sh.  So source
that.  Then "cat raw | lf_to_nul" can be started and fails as expected.
And it reports: "-bash: 4: Bad file descriptor".  What?!

lf_to_nul calls perl, and test-lib-functions.sh defines "perl" as a
function that redirects to &4.  Which is not open in my shell.

But the confusion was at the beginning: Of course it's "git show" that's
failing and triggering the exit from the test, not cat or lf_to_nul.
Didn't even get to that point because I somehow thought the redirection
magic was somehow broken.

So the lesson is to start searching at the beginning, I guess..

René