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

[Josh Steadmon <steadmon@google.com>]

On 2022.01.27 04:23, Ævar Arnfjörð Bjarmason wrote:
> 
> On Wed, Jan 26 2022, Josh Steadmon wrote:
> 
> > The trace2 subsystem can inherit certain information from parent
> > processes via environment variables; e.g., the parent command name and
> > session ID. This allows trace2 to note when a command is the child
> > process of another Git process, and to adjust various pieces of output
> > accordingly.
> >
> > 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"`.
> >
> > 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.
> >
> > In t/test-lib.sh, we keep a pattern of permitted GIT_* environment
> > variables. Variables matching /^GIT_TRACE.*/ are currently allowed via
> > this pattern. We want to preserve this behavior, because it can be
> > useful to collect trace output over the entire test suite. Instead of
> > modifying the allow-pattern, we instead fix this issue by unsetting only
> > the GIT_TRACE2_PARENT_NAME and GIT_TRACE2_PARENT_SID in t/test-lib.sh.
> >
> > Reported-by: Emily Shaffer <emilyshaffer@google.com>
> > Signed-off-by: Josh Steadmon <steadmon@google.com>
> > ---
> > Updated commit message and added code comments to explain why we keep
> > "TRACE" in the allow pattern.
> >
> >  t/test-lib.sh | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 0f7a137c7d..faf25ba1b2 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -432,7 +432,7 @@ EDITOR=:
> >  unset VISUAL EMAIL LANGUAGE $("$PERL_PATH" -e '
> >  	my @env = keys %ENV;
> >  	my $ok = join("|", qw(
> > -		TRACE
> > +		TRACE	# Allow tracing in general, but see unsets below.
> 
> I think it would be good to turn that -e into a -we, which would tell
> you why this doesn't work the way you think:
> 
>     Possible attempt to put comments in qw() list at -e line 14.
>     Possible attempt to separate words with commas at -e line 14.
> 
> I.e. you added one string "#" to the array, and a string "Allow" etc.

Whoops, yeah, thanks for the catch. I've forgotten most of the Perl I
ever knew but should have seen that.