From: Fabian Stelzer <fs@gigacodes.de>
Date: Fri, 26 Nov 2021 09:55:09 GMT
Subject: Re: [PATCH v3 3/3] test-lib: make BAIL_OUT() work in tests and prereq
Message-ID: <20211126095509.weeknmg4p6sx7bdn@fs>
In-Reply-To: <xmqqh7c4i0jh.fsf@gitster.g>

On 22.11.2021 09:05, Junio C Hamano wrote:
>Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> -exec 5>&1
>>> -exec 6<&0
>>> -exec 7>&2
>>
>> This doesn't break (I think) with your change here because you only
>> manipulate >&5, but I think the post-image would be lot clearer if...
>>
>>>  if test "$verbose_log" = "t"
>>>  then
>>>  	exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
>>
>> ...this bit of code were moved up along with the "exec". They're
>> currently intentionally snuggled together as we conditionally set the
>> 3rd and 4th fd depending on verbose/tee settings right after the setup
>> of 5/6/7, keeping them grouped in the post-image makes more sense than
>> splitting them up here.
>
>I actually have to wonder if the handling of 3 and 4 should be moved
>down, not up like you suggest, so that they are grouped together
>with the maybe_teardown_verbose and the maybe_setup_verbose helper
>functions.  The lines we see here are the file descriptors that are
>always redirected, which is a bit different.  Raising all of them up
>to group them together is also fine.

Since there is lots of other things happening in between i'd rather
leave 3 & 4 where they are for now. I'm having a hard time grasping what
test-lib.sh does when / where since there is lots of intermingled
function definition / executed code. It could probably benefit from a
new include to move functions to (and wrap some code into new ones). At
the moment i'm a bit swamped with other work though :/

>
>In any case, the comment in front of the block of exec wants to
>become a bit more detailed than just "# Set up additional fds", with
>an explanation about which FD is used for what.
>

How about:

# Set up additional fds to allow i/o with the surrounding test
# harness when redirecting individual test i/o in test_eval_
# fd 5 -> stdout
# fd 6 <- stdin
# fd 7 -> stderr

exec 5>&1
exec 6<&0
exec 7>&2