Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
-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...
quoted
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.
On 22.11.2021 09:05, Junio C Hamano wrote:
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
quoted
-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...
quoted
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