git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Josh Steadmon <steadmon@google.com>
Date
Feb 1, 2022, 21:22 UTC
Message-ID
<YfmkgeoSGdic83j4@google.com>
In-Reply-To
<220127.86r18tdeve.gmgdl@evledraar.gmail.com>
On 2022.01.27 04:23, Ævar Arnfjörð Bjarmason wrote:
Show 52 quoted lines
> 
> 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.

Previous: Junio C Hamano
Message 10 of 10 in “test-lib: unset trace2 parent envvars”
  1. test-lib: unset trace2 parent envvarsJosh Steadmon, Jan 14, 2022
  2. Junio C HamanoJan 14, 2022
  3. Josh SteadmonJan 18, 2022
  4. Junio C HamanoJan 18, 2022
  5. Josh SteadmonJan 26, 2022
  6. Taylor BlauJan 18, 2022
  7. test-lib: unset trace2 parent envvarsJosh Steadmon, Jan 26, 2022
  8. Ævar Arnfjörð BjarmasonJan 27, 2022
  9. Junio C HamanoJan 27, 2022
  10. Josh SteadmonFeb 1, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.