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

Re: [PATCH 1/2] test-lib.sh: set GIT_TRACE2_EVENT_NESTING

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 29, 2021, 14:12 UTC
Message-ID
<211129.86fsrff36j.gmgdl@evledraar.gmail.com>
In-Reply-To
<3d1bbc91aa3a465ecec2de130456b9ccc08f3032.1638193666.git.gitgitgadget@gmail.com>
On Mon, Nov 29 2021, Derrick Stolee via GitGitGadget wrote:
Show 42 quoted lines
> From: Derrick Stolee <dstolee@microsoft.com>
>
> 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 <avarab@gmail.com>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  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;
Previous: Derrick Stolee via GitGitGadgetNext: Junio C Hamano
Message 3 of 50 in “Set GIT_TRACE2_EVENT_NESTING in test-lib.sh”
  1. 0/2 Set GIT_TRACE2_EVENT_NESTING in test-lib.shDerrick Stolee via GitGitGadget, Nov 29, 2021
  2. 1/2 test-lib.sh: set GIT_TRACE2_EVENT_NESTINGDerrick Stolee via GitGitGadget, Nov 29, 2021
  3. Ævar Arnfjörð BjarmasonNov 29, 2021
  4. Junio C HamanoNov 29, 2021
  5. Junio C HamanoNov 29, 2021
  6. 2/2 t/t*: remove custom GIT_TRACE2_EVENT_NESTINGDerrick Stolee via GitGitGadget, Nov 29, 2021
  7. Jeff KingNov 29, 2021
  8. Jeff KingNov 29, 2021
  9. Taylor BlauNov 30, 2021
  10. Derrick StoleeNov 30, 2021
  11. Jeff HostetlerNov 30, 2021
  12. Jeff KingDec 1, 2021
  13. test-lib.sh: have all tests pass under "-x", remove BASH_XTRACEFDÆvar Arnfjörð Bjarmason, Nov 29, 2021
  14. Eric SunshineNov 29, 2021
  15. Jeff KingNov 30, 2021
  16. Ævar Arnfjörð BjarmasonNov 30, 2021
  17. SZEDER GáborNov 30, 2021
  18. Ævar Arnfjörð BjarmasonDec 1, 2021
  19. Jeff KingDec 1, 2021
  20. Junio C HamanoDec 1, 2021
  21. 0/2 test-lib.sh: have all tests pass under "-x", remove BASH_XTRACEFDÆvar Arnfjörð Bjarmason, Dec 1, 2021
  22. 1/2 t1510: remove need for "test_untraceable", retain coverageÆvar Arnfjörð Bjarmason, Dec 1, 2021
  23. SZEDER GáborDec 2, 2021
  24. SZEDER GáborDec 2, 2021
  25. Jeff KingDec 10, 2021
  26. Ævar Arnfjörð BjarmasonDec 10, 2021
  27. SZEDER GáborFeb 6, 2022
  28. 2/2 test-lib.sh: remove the now-unused "test_untraceable" facilityÆvar Arnfjörð Bjarmason, Dec 1, 2021
  29. 0/2 test-lib.sh: have all tests pass under "-x", remove BASH_XTRACEFDÆvar Arnfjörð Bjarmason, Dec 10, 2021
  30. 1/2 t1510: remove need for "test_untraceable", retain coverageÆvar Arnfjörð Bjarmason, Dec 10, 2021
  31. 2/2 test-lib.sh: remove the now-unused "test_untraceable" facilityÆvar Arnfjörð Bjarmason, Dec 10, 2021
  32. SZEDER GáborDec 12, 2021
  33. Ævar Arnfjörð BjarmasonDec 12, 2021
  34. SZEDER GáborDec 12, 2021
  35. Junio C HamanoDec 13, 2021
  36. Jeff KingDec 14, 2021
  37. Junio C HamanoDec 15, 2021
  38. Jeff KingDec 15, 2021
  39. Junio C HamanoDec 15, 2021
  40. Ævar Arnfjörð BjarmasonDec 16, 2021
  41. 0/3 test-lib.sh: have all tests pass under "-x", remove BASH_XTRACEFDÆvar Arnfjörð Bjarmason, Dec 13, 2021
  42. 1/3 t1510: remove need for "test_untraceable", retain coverageÆvar Arnfjörð Bjarmason, Dec 13, 2021
  43. 2/3 test-lib.sh: remove the now-unused "test_untraceable" facilityÆvar Arnfjörð Bjarmason, Dec 13, 2021
  44. 3/3 test-lib.sh: remove "BASH_XTRACEFD"Ævar Arnfjörð Bjarmason, Dec 13, 2021
  45. SZEDER GáborFeb 21, 2022
  46. Ævar Arnfjörð BjarmasonFeb 22, 2022
  47. SZEDER GáborDec 13, 2021
  48. Ævar Arnfjörð BjarmasonFeb 21, 2022
  49. SZEDER GáborFeb 21, 2022
  50. Ævar Arnfjörð BjarmasonFeb 21, 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.