Re: [PATCH 0/2] Set GIT_TRACE2_EVENT_NESTING in test-lib.sh
- From
Jeff King <peff@peff.net>
- Date
- Nov 29, 2021, 18:44 UTC
- Message-ID
- <YaUfq88UwWVcb7KG@coredump.intra.peff.net>
- In-Reply-To
- <YaUegEGxfAf72O9Z@coredump.intra.peff.net>
On Mon, Nov 29, 2021 at 01:40:00PM -0500, Jeff King wrote:
Show 18 quoted lines
> 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.
-Peff