Re: [PATCH 6/7] test-lib: make --verbose output valid TAP

[Chris Torek <chris.torek@gmail.com>]

There is a lot of historical stuff I can't really comment on
here, so I'll just point out this one tiny bit of shell oddity:

On Tue, Mar 9, 2021 at 8:04 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Make the --verbose output be valid TAP, making it machine-readable for
> TAP parsers again.
[snippage]

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index aa7068b06b6..0070d05234b 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
[snippage]
> @@ -364,9 +366,22 @@ then
>         (
>                 GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} "$0" "$@" 2>&1
>                 echo $? >"$TEST_RESULTS_BASE.exit"
> -       ) | tee -a "$GIT_TEST_TEE_OUTPUT_FILE"
> +       ) | "$GIT_BUILD_DIR"/t/helper/test-tool tee \
> +               --tap --prefix="GIT_TEST_TEE_STARTED " \
> +               --escape-stdout ${HARNESS_ACTIVE+--escape-file} \
> +               "$GIT_TEST_TEE_OUTPUT_FILE"
>         test "$(cat "$TEST_RESULTS_BASE.exit")" = 0
>         exit
> +elif test -n "$verbose" -a -n "$HARNESS_ACTIVE"
> +then
> +       ret=
> +       (
> +               GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} "$0" "$@" 2>&1
> +               ret=$?
> +       ) | "$GIT_BUILD_DIR"/t/helper/test-tool tee \
> +               --tap --prefix="GIT_TEST_TEE_STARTED " \
> +               --escape-stdout
> +       exit $ret
>  fi
>
>  if test -n "$trace" && test -n "$test_untraceable"
[snippage]

In the block beginning with:

    ret=

we have a subshell:

    ( ... ) | "$GIT_BUILD_DIR"/t/helper/test-tool tee ...

In the subshell itself, we set `ret=$?`.  But this is inside a
subshell, which then exits, so the setting of `ret` will get lost.

Did you perhaps want `{ ...; }` instead here?  Unfortunately the
pipe means that the whole left side may run in a subshell
anyway, so even that doesn't fix the problem.  We need a
temp file, a la the code above that dumps $? into
"$TEST_RESULTS_BASE.exit".

Chris