Re: [PATCH 1/2] test-lib.sh: set GIT_TRACE2_EVENT_NESTING
flat view
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-11-29 16:36:42
Subsystem:
documentation, the rest · Maintainers:
Jonathan Corbet, Linus Torvalds
On Mon, Nov 29 2021, Derrick Stolee via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
From: Derrick Stolee <redacted> The GIT_TRACE2_EVENT feed has a limited nesting depth to avoid overloading the feed when recursing into deep paths while adding more nested regions. Some tests use the GIT_TRACE2_EVENT feed to look for internal events, ensuring that intended behavior is happening. One such example is in t4216-log-bloom.sh which looks for a statistic given as a trace2_data_intmax() call. This test started failing under '-x' with 2ca245f8be5 (csum-file.h: increase hashfile buffer size, 2021-05-18) because the change in stderr triggered the progress API to create an extra trace2 region, ejecting the statistic. This change increases the value of GIT_TRACE2_EVENT_NESTING across the entire test suite to avoid errors like this. Future changes will remove custom assignments of GIT_TRACE2_EVENT_NESTING from some test scripts that were aware of this limitation. Reported-by: Ævar Arnfjörð Bjarmason <redacted> Helped-by: Jeff King [off-list ref] Signed-off-by: Derrick Stolee <redacted> --- t/test-lib.sh | 7 +++++++ 1 file changed, 7 insertions(+)diff --git a/t/test-lib.sh b/t/test-lib.sh index 2679a7596a6..2e815c41c8f 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh@@ -476,6 +476,13 @@ export GIT_TEST_MERGE_ALGORITHM GIT_TRACE_BARE=1 export GIT_TRACE_BARE +# Some tests scan the GIT_TRACE2_EVENT feed for events, but the +# default depth is 2, which frequently causes issues when the +# events are wrapped in new regions. Set it to a sufficiently +# large depth to avoid custom changes in the test suite. +GIT_TRACE2_EVENT_NESTING=100 +export GIT_TRACE2_EVENT_NESTING +
Thanks for following up on this. Rather than hardcoding 100 wouldn't it make sense to have something like the below (which I barely checked for whether it compiled or not, just to check how hard it was to change), so that we can just set this to a "false" value to disable the nesting guard entirely? Needing to dance around the value beint an integer or true/false is an unfortunate side-effect of there not being two separate "enable nesting guard?" and "nest level?" config knob, but there's not much to do about that at this point, so I just mapped "false" to "-1" internally. Maybe values of 0 and 1 should be an error in any case? I didn't check, that would make this approach simpler.
diff --git a/Documentation/config/trace2.txt b/Documentation/config/trace2.txt
index fe1642f0d40..2be5474fdcd 100644
--- a/Documentation/config/trace2.txt
+++ b/Documentation/config/trace2.txt@@ -35,10 +35,14 @@ trace2.eventBrief:: `GIT_TRACE2_EVENT_BRIEF` environment variable. Defaults to false. trace2.eventNesting:: - Integer. Specifies desired depth of nested regions in the + Integer or "false" boolean. Specifies desired depth of nested regions in the event output. Regions deeper than this value will be omitted. May be overridden by the `GIT_TRACE2_EVENT_NESTING` environment variable. Defaults to 2. ++ +Set this to a "false" value (see linkgit:git-config[1] for accepted +values, i.e. "false", "off" etc.) to remove the limitation on nesting +entirely. trace2.configParams:: A comma-separated list of patterns of "important" config
diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c
index 3a0014417cc..135d3fbd8c3 100644
--- a/trace2/tr2_tgt_event.c
+++ b/trace2/tr2_tgt_event.c@@ -53,8 +53,22 @@ static int fn_init(void) return want; nesting = tr2_sysenv_get(TR2_SYSENV_EVENT_NESTING); - if (nesting && *nesting && ((max_nesting = atoi(nesting)) > 0)) - tr2env_event_max_nesting_levels = max_nesting; + if (nesting) { + /* + * TODO: Maybe expose the "off" part of + * git_parse_maybe_bool_text() as + * git_parse_maybe_bool_text_false() and use it here? + * Note that we accept "GIT_TRACE2_EVENT_NESTING=" and + * "trace2.eventNesting=" as "false", as with the rest + * of the config mechanism and git_parse_maybe_bool() + * users. + */ + if (!*nesting || !strcmp(nesting, "false") || + !strcmp(nesting, "no") || !strcmp(nesting, "off")) + tr2env_event_max_nesting_levels = -1; + else if (*nesting && ((max_nesting = atoi(nesting)) > 0)) + tr2env_event_max_nesting_levels = max_nesting; + } brief = tr2_sysenv_get(TR2_SYSENV_EVENT_BRIEF); if (brief && *brief &&
@@ -503,6 +517,15 @@ static void fn_repo_fl(const char *file, int line, jw_release(&jw); } +static int nesting_ok(int nr_open_regions) +{ + if (tr2env_event_max_nesting_levels == -1) + return 1; + if (nr_open_regions <= tr2env_event_max_nesting_levels) + return 1; + return 0; +} + static void fn_region_enter_printf_va_fl(const char *file, int line, uint64_t us_elapsed_absolute, const char *category,
@@ -512,7 +535,7 @@ static void fn_region_enter_printf_va_fl(const char *file, int line, { const char *event_name = "region_enter"; struct tr2tls_thread_ctx *ctx = tr2tls_get_self(); - if (ctx->nr_open_regions <= tr2env_event_max_nesting_levels) { + if (nesting_ok(ctx->nr_open_regions)) { struct json_writer jw = JSON_WRITER_INIT; jw_object_begin(&jw, 0);
@@ -537,7 +560,7 @@ static void fn_region_leave_printf_va_fl( { const char *event_name = "region_leave"; struct tr2tls_thread_ctx *ctx = tr2tls_get_self(); - if (ctx->nr_open_regions <= tr2env_event_max_nesting_levels) { + if (nesting_ok(ctx->nr_open_regions)) { struct json_writer jw = JSON_WRITER_INIT; double t_rel = (double)us_elapsed_region / 1000000.0;
@@ -564,7 +587,7 @@ static void fn_data_fl(const char *file, int line, uint64_t us_elapsed_absolute, { const char *event_name = "data"; struct tr2tls_thread_ctx *ctx = tr2tls_get_self(); - if (ctx->nr_open_regions <= tr2env_event_max_nesting_levels) { + if (nesting_ok(ctx->nr_open_regions)) { struct json_writer jw = JSON_WRITER_INIT; double t_abs = (double)us_elapsed_absolute / 1000000.0; double t_rel = (double)us_elapsed_region / 1000000.0;
@@ -592,7 +615,7 @@ static void fn_data_json_fl(const char *file, int line, { const char *event_name = "data_json"; struct tr2tls_thread_ctx *ctx = tr2tls_get_self(); - if (ctx->nr_open_regions <= tr2env_event_max_nesting_levels) { + if (nesting_ok(ctx->nr_open_regions)) { struct json_writer jw = JSON_WRITER_INIT; double t_abs = (double)us_elapsed_absolute / 1000000.0; double t_rel = (double)us_elapsed_region / 1000000.0;