Re: [PATCH 0/2] Set GIT_TRACE2_EVENT_NESTING in test-lib.sh

[Taylor Blau <me@ttaylorr.com>]

On Mon, Nov 29, 2021 at 01:44:59PM -0500, Jeff King wrote:
> On Mon, Nov 29, 2021 at 01:40:00PM -0500, Jeff King wrote:
>
> > On Mon, Nov 29, 2021 at 01:47:44PM +0000, Derrick Stolee via GitGitGadget wrote:
> >
> > > As reported by Ævar [1] and diagnosed by Peff in a reply, the default
> > > GIT_TRACE2_EVENT_NESTING is set so low that tests that look for trace2
> > > events can start failing due to an added trace2 region. This can even be
> > > subtle, including the fact that the progress API adds trace2 regions when in
> > > use (which can depend on the verbose output of a test script).
> >
> > I think this is a good change for fixing the immediate problem of the
> > test suite failing.
> >
> > But I have to wonder if this is masking a problem that real users will
> > see. Aren't we silently discarding trace2 output that could be useful to
> > somebody debugging or trying to get performance metrics?
> >
> > I.e., should we be bumping the internal nesting max of 2 to something
> > higher? If that would that cause problems with existing metrics, should
> > we be designing new metrics to avoid nesting?
>
> One thing I should have added to the first paragraph: the patches
> themselves look fine and I'm OK with this as a band-aid in the short
> term.

Yeah, I agree. This test in particular must have bad luck, because I
noticed the same failure in GitHub's fork when merging with 2.33 (but
due to some custom trace2 regions causing us to blow past the limit even
without `-v`).

It might be nice to allow setting 'GIT_TRACE2_EVENT_NESTING=false' or
something similar, but let's deal with that elsewhere.

hanks,
Taylor