Re: [PATCH v3 3/3] test-lib: make BAIL_OUT() work in tests and prereq

[Junio C Hamano <gitster@pobox.com>]

Æ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.

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.

And any change that involves handling of FD #4 probably wants to
further include the BASH_XTRACEFD setting.

Thanks.