Subject: Re: [PATCH] test-lib: unset trace2 parent envvars

From: Junio C Hamano <[email-masked]>
Date: Thu, 13 Jan 2022 22:01:07 -0800

Josh Steadmon <[email-masked]> writes:

> This behavior breaks certain tests that examine trace2 output when the
> tests run as a child of another git process, such as in `git rebase -x
> "make test"`.

Well explained.  The paragraph makes it clear how easy to trigger
and get bitten by this problem.

> While we could fix this by unsetting the relevant variables in the
> affected tests (currently t0210, t0211, t0212, and t6421), this would
> leave other tests vulnerable to similar breakage if new test cases are
> added which inspect trace2 output. So fix this in general by unsetting
> GIT_TRACE2_PARENT_NAME and GIT_TRACE2_PARENT_SID in test-lib.sh.

This probably makes sense, but I wonder how it interacts with a user
who runs "cd t && GIT_TRACE2=blah ./t0987-test-this.sh" to trace the
entire test script, though.

> Reported-by: Emily Shaffer <[email-masked]>
> Helped-by: Jonathan Tan <[email-masked]>
> Signed-off-by: Josh Steadmon <[email-masked]>
> ---
>  t/test-lib.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 0f7a137c7d..e4716b0b86 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -449,6 +449,8 @@ unset VISUAL EMAIL LANGUAGE $("$PERL_PATH" -e '
>  unset XDG_CACHE_HOME
>  unset XDG_CONFIG_HOME
>  unset GITPERLLIB
> +unset GIT_TRACE2_PARENT_NAME
> +unset GIT_TRACE2_PARENT_SID

Hmph.  Have you noticed the more generic "We want to unset almost
everything that begins with GIT_, other than those selected few that
are designed to be used to affect the tests" above the part you
touched?

I am wondering if we should tweak the list there, instead of special
casing these two and these two only. There is a pattern that allows
anything that match "^GIT_(other|TRACE|things)", and I suspect that
the pattern is way too loose (i.e. it allows any garbage to follow,
and by allowing "TRACE", it also catches "TRACE2" because the former
is a prefix of the latter), which is a problem.