Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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.
