Skip to contentSkip to conversation

trace2: move 'def_param' events into 'cmd_name' and 'cmd_alias'

PATCH Superseded Jeff Hostetler via GitGitGadget 4 participants 13 messages Mar 4, 2024 – Mar 7, 2024

lore
Share & tools
lore

Jeff Hostetler via GitGitGadget

author
#0 lore raw

cover trace2: move 'def_param' events into 'cmd_name' and 'cmd_alias'

Some Git commands do not emit def_param events for interesting config and environment variable settings. Let's fix that.

Builtin commands compiled into git.c have the normal control flow and emit a cmd_name event and then def_param events for each interesting config and environment variable. However, some special "query" commands, like --exec-path, or some forms of alias expansion, emitted a cmd_name but did not emit def_param events.

Also, special commands git-remote-https is built from remote-curl.c and git-http-fetch is built from http-fetch.c and do not use the normal set up in git.c. These emitted a cmd_name but not def_param events.

To minimize the footprint of this commit, move the calls to trace2_cmd_list_config() and trace2_cmd_list_env_vars() into trace2_cmd_name() and trace2_cmd_alias() so that we always get a set def_param events when a cmd_name or cmd_alias event is generated.

Users can define local config settings on a repo to classify/name a repo (e.g. "project-foo" vs "personal") and use the def_param feature to label Trace2 data so that (a third-party) telemetry service does not collect data on personal repos or so that telemetry from one work repo is distinguishable from another work repo.

Jeff Hostetler (4):
t0211: demonstrate missing 'def_param' events for certain commands
trace2: avoid emitting 'def_param' set more than once
trace2: emit 'def_param' set with 'cmd_name' event
trace2: remove unneeded calls to generate 'def_param' set

 git.c                  |   6 --
 t/t0211-trace2-perf.sh | 231 +++++++++++++++++++++++++++++++++++++++++
 trace2.c               |  15 +++
 3 files changed, 246 insertions(+), 6 deletions(-)

base-commit: 0f9d4d28b7e6021b7e6db192b7bf47bd3a0d0d1d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1679%2Fjeffhostetler%2Falways-emit-def-param-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1679/jeffhostetler/always-emit-def-param-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1679

Signature · 1 line
gitgitgadget
author
#1 lore raw

PATCH 1/4 t0211: demonstrate missing 'def_param' events for certain commands

From: Jeff Hostetler <[email protected]>

Some Git commands fail to emit 'def_param' events for interesting config and environment variable settings.

Add unit tests to demonstrate this.

Most commands are considered "builtin" and are based upon git.c. These typically do emit 'def_param' events. Exceptions are some of the "query" commands, the "run-dashed" mechanism, and alias handling.

Commands built from remote-curl.c (instead of git.c), such as "git-remote-https", do not emit 'def_param' events.

Likewise, "git-http-fetch" is built http-fetch.c and does not emit them.

Signed-off-by Jeff Hostetler <[email protected]>
---
 t/t0211-trace2-perf.sh | 231 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 231 insertions(+)
t/t0211-trace2-perf.sh +231 -0
index 290b6eaaab1..588c5bad033 100755
@@ -287,4 +287,235 @@ test_expect_success 'unsafe URLs are redacted by default' '
 	grep "d0|main|def_param|.*|remote.origin.url:https://user:[email protected]" actual
 '
 
+# Confirm that the requested command produces a "cmd_name" and a
+# set of "def_param" events.
+#
+try_simple () {
+	test_when_finished "rm prop.perf actual" &&
+
+	cmd=$1 &&
+	cmd_name=$2 &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+			$cmd &&
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+	grep "d0|main|cmd_name|.*|$cmd_name" actual &&
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual
+}
+
+# Representative mainstream builtin Git command dispatched
+# in run_builtin() in git.c
+#
+test_expect_success 'expect def_params for normal builtin command' '
+	try_simple "git version" "version"
+'
+
+# Representative query command dispatched in handle_options()
+# in git.c
+#
+test_expect_failure 'expect def_params for query command' '
+	try_simple "git --man-path" "_query_"
+'
+
+# remote-curl.c does not use the builtin setup in git.c, so confirm
+# that executables built from remote-curl.c emit def_params.
+#
+# Also tests the dashed-command handling where "git foo" silently
+# spawns "git-foo".  Make sure that both commands should emit
+# def_params.
+#
+# Pass bogus arguments to remote-https and allow the command to fail
+# because we don't actually have a remote to fetch from.  We just want
+# to see the run-dashed code run an executable built from
+# remote-curl.c rather than git.c.  Confirm that we get def_param
+# events from both layers.
+#
+test_expect_failure 'expect def_params for remote-curl and _run_dashed_' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_might_fail env \
+		ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+		git remote-http x y &&
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	grep "d0|main|cmd_name|.*|_run_dashed_" actual &&
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	grep "d1|main|cmd_name|.*|remote-curl" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
+# Similarly, `git-http-fetch` is not built from git.c so do a
+# trivial fetch so that the main git.c run-dashed code spawns
+# an executable built from http-fetch.c.  Confirm that we get
+# def_param events from both layers.
+#
+test_expect_failure 'expect def_params for http-fetch and _run_dashed_' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_might_fail env \
+		ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+		git http-fetch --stdin file:/// <<-EOF &&
+	EOF
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	grep "d0|main|cmd_name|.*|_run_dashed_" actual &&
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	grep "d1|main|cmd_name|.*|http-fetch" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
+# Historically, alias expansion explicitly emitted the def_param
+# events (independent of whether the command was a builtin, a Git
+# command or arbitrary shell command) so that it wasn't dependent
+# upon the unpeeling of the alias. Let's make sure that we preserve
+# the net effect.
+#
+test_expect_success 'expect def_params during git alias expansion' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_config_global "alias.xxx" "version" &&
+
+	ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+			git xxx &&
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	# "git xxx" is first mapped to "git-xxx" and the child will fail.
+	grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_)" actual &&
+
+	# We unpeel that and substitute "version" into "xxx" (giving
+	# "git version") and update the cmd_name event.
+	grep "d0|main|cmd_name|.*|_run_git_alias_ (_run_dashed_/_run_git_alias_)" actual &&
+
+	# These def_param events could be associated with either of the
+	# above cmd_name events.  It does not matter.
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	# The "git version" child sees a different cmd_name hierarchy.
+	# Also test the def_param (only for completeness).
+	grep "d1|main|cmd_name|.*|version (_run_dashed_/_run_git_alias_/version)" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
+test_expect_success 'expect def_params during shell alias expansion' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_config_global "alias.xxx" "!git version" &&
+
+	ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+			git xxx &&
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	# "git xxx" is first mapped to "git-xxx" and the child will fail.
+	grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_)" actual &&
+
+	# We unpeel that and substitute "git version" for "git xxx" (as a
+	# shell command.  Another cmd_name event is emitted as we unpeel.
+	grep "d0|main|cmd_name|.*|_run_shell_alias_ (_run_dashed_/_run_shell_alias_)" actual &&
+
+	# These def_param events could be associated with either of the
+	# above cmd_name events.  It does not matter.
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	# We get the following only because we used a git command for the
+	# shell command. In general, it could have been a shell script and
+	# we would see nothing.
+	#
+	# The child knows the cmd_name hierarchy so it includes it.
+	grep "d1|main|cmd_name|.*|version (_run_dashed_/_run_shell_alias_/version)" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
+test_expect_failure 'expect def_params during nested git alias expansion' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_config_global "alias.xxx" "yyy" &&
+	test_config_global "alias.yyy" "version" &&
+
+	ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+			git xxx &&
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	# "git xxx" is first mapped to "git-xxx" and try to spawn "git-xxx"
+	# and the child will fail.
+	grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_)" actual &&
+	grep "d0|main|child_start|.*|.* class:dashed argv:\[git-xxx\]" actual &&
+
+	# We unpeel that and substitute "yyy" into "xxx" (giving "git yyy")
+	# and spawn "git-yyy" and the child will fail.
+	grep "d0|main|alias|.*|alias:xxx argv:\[yyy\]" actual &&
+	grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_/_run_dashed_)" actual &&
+	grep "d0|main|child_start|.*|.* class:dashed argv:\[git-yyy\]" actual &&
+
+	# We unpeel that and substitute "version" into "xxx" (giving
+	# "git version") and update the cmd_name event.
+	grep "d0|main|alias|.*|alias:yyy argv:\[version\]" actual &&
+	grep "d0|main|cmd_name|.*|_run_git_alias_ (_run_dashed_/_run_dashed_/_run_git_alias_)" actual &&
+
+	# These def_param events could be associated with any of the
+	# above cmd_name events.  It does not matter.
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual >actual.matches &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	# However, we do not want them repeated each time we unpeel.
+	test_line_count = 1 actual.matches &&
+
+	# The "git version" child sees a different cmd_name hierarchy.
+	# Also test the def_param (only for completeness).
+	grep "d1|main|cmd_name|.*|version (_run_dashed_/_run_dashed_/_run_git_alias_/version)" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
 test_done
Signature · 1 line
gitgitgadget
author
#2 lore raw

PATCH 2/4 trace2: avoid emitting 'def_param' set more than once

In reply to Jeff Hostetler via GitGitGadget

From: Jeff Hostetler <[email protected]>

During nested alias expansion it is possible for "trace2_cmd_list_config()" and "trace2_cmd_list_env_vars()" to be called more than once. This causes a full set of 'def_param' events to be emitted each time. Let's avoid that.

Add code to those two functions to only emit them once.

Signed-off-by Jeff Hostetler <[email protected]>
---
 t/t0211-trace2-perf.sh |  2 +-
 trace2.c               | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
t/t0211-trace2-perf.sh +1 -1
index 588c5bad033..7b353195396 100755
@@ -470,7 +470,7 @@ test_expect_success 'expect def_params during shell alias expansion' '
 	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
 '
 
-test_expect_failure 'expect def_params during nested git alias expansion' '
+test_expect_success 'expect def_params during nested git alias expansion' '
 	test_when_finished "rm prop.perf actual" &&
 
 	test_config_global "trace2.configParams" "cfg.prop.*" &&
trace2.c +12 -0
index f1e268bd159..facce641ef3 100644
@@ -464,17 +464,29 @@ void trace2_cmd_alias_fl(const char *file, int line, const char *alias,
 
 void trace2_cmd_list_config_fl(const char *file, int line)
 {
+	static int emitted = 0;
+
 	if (!trace2_enabled)
 		return;
 
+	if (emitted)
+		return;
+	emitted = 1;
+
 	tr2_cfg_list_config_fl(file, line);
 }
 
 void trace2_cmd_list_env_vars_fl(const char *file, int line)
 {
+	static int emitted = 0;
+
 	if (!trace2_enabled)
 		return;
 
+	if (emitted)
+		return;
+	emitted = 1;
+
 	tr2_list_env_vars_fl(file, line);
 }
Signature · 1 line
gitgitgadget
author
#3 lore raw

PATCH 3/4 trace2: emit 'def_param' set with 'cmd_name' event

In reply to Jeff Hostetler via GitGitGadget

From: Jeff Hostetler <[email protected]>

Some commands do not cause a set of 'def_param' events to be emitted. This includes "git-remote-https", "git-http-fetch", and various "query" commands, like "git --man-path".

Since all of these commands do emit a 'cmd_name' event, add code to the "trace2_cmd_name()" function to generate the set of 'def_param' events.

We can later remove explicit calls to "trace2_cmd_list_config()" and "trace2_cmd_list_env_vars()" in git.c.

Signed-off-by Jeff Hostetler <[email protected]>
---
 t/t0211-trace2-perf.sh | 6 +++---
 trace2.c               | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)
t/t0211-trace2-perf.sh +3 -3
index 7b353195396..13ef69b92f8 100755
@@ -320,7 +320,7 @@ test_expect_success 'expect def_params for normal builtin command' '
 # Representative query command dispatched in handle_options()
 # in git.c
 #
-test_expect_failure 'expect def_params for query command' '
+test_expect_success 'expect def_params for query command' '
 	try_simple "git --man-path" "_query_"
 '
 

@@ -337,7 +337,7 @@ test_expect_failure 'expect def_params for query command' '
 # remote-curl.c rather than git.c.  Confirm that we get def_param
 # events from both layers.
 #
-test_expect_failure 'expect def_params for remote-curl and _run_dashed_' '
+test_expect_success 'expect def_params for remote-curl and _run_dashed_' '
 	test_when_finished "rm prop.perf actual" &&
 
 	test_config_global "trace2.configParams" "cfg.prop.*" &&

@@ -366,7 +366,7 @@ test_expect_failure 'expect def_params for remote-curl and _run_dashed_' '
 # an executable built from http-fetch.c.  Confirm that we get
 # def_param events from both layers.
 #
-test_expect_failure 'expect def_params for http-fetch and _run_dashed_' '
+test_expect_success 'expect def_params for http-fetch and _run_dashed_' '
 	test_when_finished "rm prop.perf actual" &&
 
 	test_config_global "trace2.configParams" "cfg.prop.*" &&
trace2.c +3 -0
index facce641ef3..f894532d053 100644
@@ -433,6 +433,9 @@ void trace2_cmd_name_fl(const char *file, int line, const char *name)
 	for_each_wanted_builtin (j, tgt_j)
 		if (tgt_j->pfn_command_name_fl)
 			tgt_j->pfn_command_name_fl(file, line, name, hierarchy);
+
+	trace2_cmd_list_config();
+	trace2_cmd_list_env_vars();
 }
 
 void trace2_cmd_mode_fl(const char *file, int line, const char *mode)
Signature · 1 line
gitgitgadget
author
#4 lore raw

PATCH 4/4 trace2: remove unneeded calls to generate 'def_param' set

In reply to Jeff Hostetler via GitGitGadget

From: Jeff Hostetler <[email protected]>

Now that "trace2_cmd_name()" implicitly calls "trace2_cmd_list_config()" and "trace2_cmd_list_env_vars()", we don't need to explicitly call them.

Signed-off-by Jeff Hostetler <[email protected]>
---
 git.c | 6 ------
 1 file changed, 6 deletions(-)
git.c +0 -6
index 7068a184b0a..a769d72ab8f 100644
@@ -373,8 +373,6 @@ static int handle_alias(int *argcp, const char ***argv)
 			strvec_pushv(&child.args, (*argv) + 1);
 
 			trace2_cmd_alias(alias_command, child.args.v);
-			trace2_cmd_list_config();
-			trace2_cmd_list_env_vars();
 			trace2_cmd_name("_run_shell_alias_");
 
 			ret = run_command(&child);

@@ -411,8 +409,6 @@ static int handle_alias(int *argcp, const char ***argv)
 		COPY_ARRAY(new_argv + count, *argv + 1, *argcp);
 
 		trace2_cmd_alias(alias_command, new_argv);
-		trace2_cmd_list_config();
-		trace2_cmd_list_env_vars();
 
 		*argv = new_argv;
 		*argcp += count - 1;

@@ -462,8 +458,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	trace_argv_printf(argv, "trace: built-in: git");
 	trace2_cmd_name(p->cmd);
-	trace2_cmd_list_config();
-	trace2_cmd_list_env_vars();
 
 	validate_cache_entries(the_repository->index);
 	status = p->fn(argc, argv, prefix);
Signature · 1 line
gitgitgadget
Reviewed-by on v1 4/4
#5 lore raw

Re: [PATCH 4/4] trace2: remove unneeded calls to generate 'def_param' set

Quoted 44 lines · Jeff Hostetler via GitGitGadget

From: Jeff Hostetler <[email protected]>

Now that "trace2_cmd_name()" implicitly calls "trace2_cmd_list_config()" and "trace2_cmd_list_env_vars()", we don't need to explicitly call them.

Signed-off-by Jeff Hostetler <[email protected]>
---
 git.c | 6 ------
 1 file changed, 6 deletions(-)
git.c +0 -6
index 7068a184b0a..a769d72ab8f 100644
@@ -373,8 +373,6 @@ static int handle_alias(int *argcp, const char ***argv)
 			strvec_pushv(&child.args, (*argv) + 1);
 
 			trace2_cmd_alias(alias_command, child.args.v);
-			trace2_cmd_list_config();
-			trace2_cmd_list_env_vars();
 			trace2_cmd_name("_run_shell_alias_");
 
 			ret = run_command(&child);

@@ -411,8 +409,6 @@ static int handle_alias(int *argcp, const char ***argv)
 		COPY_ARRAY(new_argv + count, *argv + 1, *argcp);
 
 		trace2_cmd_alias(alias_command, new_argv);
-		trace2_cmd_list_config();
-		trace2_cmd_list_env_vars();
 
 		*argv = new_argv;
 		*argcp += count - 1;

@@ -462,8 +458,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	trace_argv_printf(argv, "trace: built-in: git");
 	trace2_cmd_name(p->cmd);
-	trace2_cmd_list_config();
-	trace2_cmd_list_env_vars();
 
 	validate_cache_entries(the_repository->index);
 	status = p->fn(argc, argv, prefix);
Signature · 1 line
gitgitgadget

I'd personally prefer to see this squashed into Patch 3, but I don't feel too strongly about it. Either way, the series LGTM.

Reviewed-by Josh Steadmon <[email protected]>
#6 lore raw

Re: [PATCH 4/4] trace2: remove unneeded calls to generate 'def_param' set

Quoted 49 lines · Josh Steadmon
Quoted 44 lines · Jeff Hostetler via GitGitGadget

From: Jeff Hostetler <[email protected]>

Now that "trace2_cmd_name()" implicitly calls "trace2_cmd_list_config()" and "trace2_cmd_list_env_vars()", we don't need to explicitly call them.

Signed-off-by Jeff Hostetler <[email protected]>
---
 git.c | 6 ------
 1 file changed, 6 deletions(-)
git.c +0 -6
index 7068a184b0a..a769d72ab8f 100644
@@ -373,8 +373,6 @@ static int handle_alias(int *argcp, const char ***argv)
 			strvec_pushv(&child.args, (*argv) + 1);
 
 			trace2_cmd_alias(alias_command, child.args.v);
-			trace2_cmd_list_config();
-			trace2_cmd_list_env_vars();
 			trace2_cmd_name("_run_shell_alias_");
 
 			ret = run_command(&child);

@@ -411,8 +409,6 @@ static int handle_alias(int *argcp, const char ***argv)
 		COPY_ARRAY(new_argv + count, *argv + 1, *argcp);
 
 		trace2_cmd_alias(alias_command, new_argv);
-		trace2_cmd_list_config();
-		trace2_cmd_list_env_vars();
 
 		*argv = new_argv;
 		*argcp += count - 1;

@@ -462,8 +458,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	trace_argv_printf(argv, "trace: built-in: git");
 	trace2_cmd_name(p->cmd);
-	trace2_cmd_list_config();
-	trace2_cmd_list_env_vars();
 
 	validate_cache_entries(the_repository->index);
 	status = p->fn(argc, argv, prefix);
Signature · 1 line
gitgitgadget

I'd personally prefer to see this squashed into Patch 3, but I don't feel too strongly about it. Either way, the series LGTM.

Quoted 1 line
Reviewed-by Josh Steadmon <[email protected]>

Let's see what JeffH says about this. I agree with you that making some stuff redundant in [Patch 3/4] and fixing the redundancy in this step does feel somewhat roundabout way of doing this.

Thanks.

#7 lore raw

Re: [PATCH 4/4] trace2: remove unneeded calls to generate 'def_param' set

Quoted 51 lines · Junio C Hamano
Quoted 49 lines · Josh Steadmon
Quoted 44 lines · Jeff Hostetler via GitGitGadget

From: Jeff Hostetler <[email protected]>

Now that "trace2_cmd_name()" implicitly calls "trace2_cmd_list_config()" and "trace2_cmd_list_env_vars()", we don't need to explicitly call them.

Signed-off-by Jeff Hostetler <[email protected]>
---
  git.c | 6 ------
  1 file changed, 6 deletions(-)
git.c +0 -6
index 7068a184b0a..a769d72ab8f 100644
@@ -373,8 +373,6 @@ static int handle_alias(int *argcp, const char ***argv)
  			strvec_pushv(&child.args, (*argv) + 1);
 
  			trace2_cmd_alias(alias_command, child.args.v);
-			trace2_cmd_list_config();
-			trace2_cmd_list_env_vars();
  			trace2_cmd_name("_run_shell_alias_");
 
  			ret = run_command(&child);

@@ -411,8 +409,6 @@ static int handle_alias(int *argcp, const char ***argv)
  		COPY_ARRAY(new_argv + count, *argv + 1, *argcp);
 
  		trace2_cmd_alias(alias_command, new_argv);
-		trace2_cmd_list_config();
-		trace2_cmd_list_env_vars();
 
  		*argv = new_argv;
  		*argcp += count - 1;

@@ -462,8 +458,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
  	trace_argv_printf(argv, "trace: built-in: git");
  	trace2_cmd_name(p->cmd);
-	trace2_cmd_list_config();
-	trace2_cmd_list_env_vars();
 
  	validate_cache_entries(the_repository->index);
  	status = p->fn(argc, argv, prefix);
Signature · 1 line
gitgitgadget

I'd personally prefer to see this squashed into Patch 3, but I don't feel too strongly about it. Either way, the series LGTM.

Quoted 8 lines
Quoted 1 line
Reviewed-by Josh Steadmon <[email protected]>

Let's see what JeffH says about this. I agree with you that making some stuff redundant in [Patch 3/4] and fixing the redundancy in this step does feel somewhat roundabout way of doing this.

Thanks.

Sure we can merge them. That's fine. I can send a V4 or if you want to just squash them together that's fine.

Jeff

#8 lore raw

Re: [PATCH 4/4] trace2: remove unneeded calls to generate 'def_param' set

Quoted 9 lines · Jeff Hostetler
Quoted 6 lines
Quoted 1 line
Reviewed-by Josh Steadmon <[email protected]>

Let's see what JeffH says about this. I agree with you that making some stuff redundant in [Patch 3/4] and fixing the redundancy in this step does feel somewhat roundabout way of doing this. Thanks.

Sure we can merge them. That's fine. I can send a V4 or if you want to just squash them together that's fine.

Let's have a v4 describing the change for combined 3&4 in your words, with Josh's Reviewed-by: already added to the trailers.

Thanks, both of you.

author
#9 lore raw

v2 cover trace2: move generation of 'def_param' events into code for 'cmd_name'

In reply to Jeff Hostetler via GitGitGadget

Here is version 2 of this series. The only change from V1 is to combine the last two commits as discussed.

Thanks Jeff

----------------------------------------------------------------------------

Some Git commands do not emit def_param events for interesting config and environment variable settings. Let's fix that.

Builtin commands compiled into git.c have the normal control flow and emit a cmd_name event and then def_param events for each interesting config and environment variable. However, some special "query" commands, like --exec-path, or some forms of alias expansion, emitted a cmd_name but did not emit def_param events.

Also, special commands git-remote-https is built from remote-curl.c and git-http-fetch is built from http-fetch.c and do not use the normal set up in git.c. These emitted a cmd_name but not def_param events.

To minimize the footprint of this commit, move the calls to trace2_cmd_list_config() and trace2_cmd_list_env_vars() into trace2_cmd_name() so that we always get a set of def_param events when a cmd_name event is generated.

Users can define local config settings on a repo to classify/name a repo (e.g. "project-foo" vs "personal") and use the def_param feature to label Trace2 data so that (a third-party) telemetry service does not collect data on personal repos or so that telemetry from one work repo is distinguishable from another work repo in database queries.

Jeff Hostetler (3):
t0211: demonstrate missing 'def_param' events for certain commands
trace2: avoid emitting 'def_param' set more than once
trace2: emit 'def_param' set with 'cmd_name' event

 git.c                  |   6 --
 t/t0211-trace2-perf.sh | 231 +++++++++++++++++++++++++++++++++++++++++
 trace2.c               |  15 +++
 3 files changed, 246 insertions(+), 6 deletions(-)

base-commit: 0f9d4d28b7e6021b7e6db192b7bf47bd3a0d0d1d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1679%2Fjeffhostetler%2Falways-emit-def-param-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1679/jeffhostetler/always-emit-def-param-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1679

Range-diff vs v1:

 1:  b378b93242a = 1:  b378b93242a t0211: demonstrate missing 'def_param' events for certain commands
 2:  65068e97597 = 2:  65068e97597 trace2: avoid emitting 'def_param' set more than once
 3:  9507184b4f1 ! 3:  178721cd4f0 trace2: emit 'def_param' set with 'cmd_name' event

@@ Commit message
the "trace2_cmd_name()" function to generate the set of 'def_param' events.

- We can later remove explicit calls to "trace2_cmd_list_config()" and
- "trace2_cmd_list_env_vars()" in git.c.
+ Remove explicit calls to "trace2_cmd_list_config()" and
+ "trace2_cmd_list_env_vars()" in git.c since they are no longer needed.

+ Reviewed-by: Josh Steadmon <[email protected]>

Signed-off-by Jeff Hostetler <[email protected]>

+ ## git.c ##
+@@ git.c: static int handle_alias(int *argcp, const char ***argv)
+ strvec_pushv(&child.args, (*argv) + 1);
+
+ trace2_cmd_alias(alias_command, child.args.v);
+- trace2_cmd_list_config();
+- trace2_cmd_list_env_vars();
+ trace2_cmd_name("_run_shell_alias_");
+
+ ret = run_command(&child);
+@@ git.c: static int handle_alias(int *argcp, const char ***argv)
+ COPY_ARRAY(new_argv + count, *argv + 1, *argcp);
+
+ trace2_cmd_alias(alias_command, new_argv);
+- trace2_cmd_list_config();
+- trace2_cmd_list_env_vars();
+
+ *argv = new_argv;
+ *argcp += count - 1;
+@@ git.c: static int run_builtin(struct cmd_struct *p, int argc, const char **argv) +
+ trace_argv_printf(argv, "trace: built-in: git");
+ trace2_cmd_name(p->cmd);
+- trace2_cmd_list_config();
+- trace2_cmd_list_env_vars();
+
+ validate_cache_entries(the_repository->index);
+ status = p->fn(argc, argv, prefix);
+
## t/t0211-trace2-perf.sh ##
@@ t/t0211-trace2-perf.sh: test_expect_success 'expect def_params for normal builtin command' '
# Representative query command dispatched in handle_options()
4: e8528715ebf < -: ----------- trace2: remove unneeded calls to generate 'def_param' set

Signature · 1 line
gitgitgadget
author
#10 lore raw

PATCH v2 1/3 t0211: demonstrate missing 'def_param' events for certain commands

From: Jeff Hostetler <[email protected]>

Some Git commands fail to emit 'def_param' events for interesting config and environment variable settings.

Add unit tests to demonstrate this.

Most commands are considered "builtin" and are based upon git.c. These typically do emit 'def_param' events. Exceptions are some of the "query" commands, the "run-dashed" mechanism, and alias handling.

Commands built from remote-curl.c (instead of git.c), such as "git-remote-https", do not emit 'def_param' events.

Likewise, "git-http-fetch" is built http-fetch.c and does not emit them.

Signed-off-by Jeff Hostetler <[email protected]>
---
 t/t0211-trace2-perf.sh | 231 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 231 insertions(+)
t/t0211-trace2-perf.sh +231 -0
index 290b6eaaab1..588c5bad033 100755
@@ -287,4 +287,235 @@ test_expect_success 'unsafe URLs are redacted by default' '
 	grep "d0|main|def_param|.*|remote.origin.url:https://user:[email protected]" actual
 '
 
+# Confirm that the requested command produces a "cmd_name" and a
+# set of "def_param" events.
+#
+try_simple () {
+	test_when_finished "rm prop.perf actual" &&
+
+	cmd=$1 &&
+	cmd_name=$2 &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+			$cmd &&
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+	grep "d0|main|cmd_name|.*|$cmd_name" actual &&
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual
+}
+
+# Representative mainstream builtin Git command dispatched
+# in run_builtin() in git.c
+#
+test_expect_success 'expect def_params for normal builtin command' '
+	try_simple "git version" "version"
+'
+
+# Representative query command dispatched in handle_options()
+# in git.c
+#
+test_expect_failure 'expect def_params for query command' '
+	try_simple "git --man-path" "_query_"
+'
+
+# remote-curl.c does not use the builtin setup in git.c, so confirm
+# that executables built from remote-curl.c emit def_params.
+#
+# Also tests the dashed-command handling where "git foo" silently
+# spawns "git-foo".  Make sure that both commands should emit
+# def_params.
+#
+# Pass bogus arguments to remote-https and allow the command to fail
+# because we don't actually have a remote to fetch from.  We just want
+# to see the run-dashed code run an executable built from
+# remote-curl.c rather than git.c.  Confirm that we get def_param
+# events from both layers.
+#
+test_expect_failure 'expect def_params for remote-curl and _run_dashed_' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_might_fail env \
+		ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+		git remote-http x y &&
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	grep "d0|main|cmd_name|.*|_run_dashed_" actual &&
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	grep "d1|main|cmd_name|.*|remote-curl" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
+# Similarly, `git-http-fetch` is not built from git.c so do a
+# trivial fetch so that the main git.c run-dashed code spawns
+# an executable built from http-fetch.c.  Confirm that we get
+# def_param events from both layers.
+#
+test_expect_failure 'expect def_params for http-fetch and _run_dashed_' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_might_fail env \
+		ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+		git http-fetch --stdin file:/// <<-EOF &&
+	EOF
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	grep "d0|main|cmd_name|.*|_run_dashed_" actual &&
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	grep "d1|main|cmd_name|.*|http-fetch" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
+# Historically, alias expansion explicitly emitted the def_param
+# events (independent of whether the command was a builtin, a Git
+# command or arbitrary shell command) so that it wasn't dependent
+# upon the unpeeling of the alias. Let's make sure that we preserve
+# the net effect.
+#
+test_expect_success 'expect def_params during git alias expansion' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_config_global "alias.xxx" "version" &&
+
+	ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+			git xxx &&
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	# "git xxx" is first mapped to "git-xxx" and the child will fail.
+	grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_)" actual &&
+
+	# We unpeel that and substitute "version" into "xxx" (giving
+	# "git version") and update the cmd_name event.
+	grep "d0|main|cmd_name|.*|_run_git_alias_ (_run_dashed_/_run_git_alias_)" actual &&
+
+	# These def_param events could be associated with either of the
+	# above cmd_name events.  It does not matter.
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	# The "git version" child sees a different cmd_name hierarchy.
+	# Also test the def_param (only for completeness).
+	grep "d1|main|cmd_name|.*|version (_run_dashed_/_run_git_alias_/version)" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
+test_expect_success 'expect def_params during shell alias expansion' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_config_global "alias.xxx" "!git version" &&
+
+	ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+			git xxx &&
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	# "git xxx" is first mapped to "git-xxx" and the child will fail.
+	grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_)" actual &&
+
+	# We unpeel that and substitute "git version" for "git xxx" (as a
+	# shell command.  Another cmd_name event is emitted as we unpeel.
+	grep "d0|main|cmd_name|.*|_run_shell_alias_ (_run_dashed_/_run_shell_alias_)" actual &&
+
+	# These def_param events could be associated with either of the
+	# above cmd_name events.  It does not matter.
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	# We get the following only because we used a git command for the
+	# shell command. In general, it could have been a shell script and
+	# we would see nothing.
+	#
+	# The child knows the cmd_name hierarchy so it includes it.
+	grep "d1|main|cmd_name|.*|version (_run_dashed_/_run_shell_alias_/version)" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
+test_expect_failure 'expect def_params during nested git alias expansion' '
+	test_when_finished "rm prop.perf actual" &&
+
+	test_config_global "trace2.configParams" "cfg.prop.*" &&
+	test_config_global "trace2.envvars" "ENV_PROP_FOO,ENV_PROP_BAR" &&
+
+	test_config_global "cfg.prop.foo" "red" &&
+
+	test_config_global "alias.xxx" "yyy" &&
+	test_config_global "alias.yyy" "version" &&
+
+	ENV_PROP_FOO=blue \
+		GIT_TRACE2_PERF="$(pwd)/prop.perf" \
+			git xxx &&
+
+	perl "$TEST_DIRECTORY/t0211/scrub_perf.perl" <prop.perf >actual &&
+
+	# "git xxx" is first mapped to "git-xxx" and try to spawn "git-xxx"
+	# and the child will fail.
+	grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_)" actual &&
+	grep "d0|main|child_start|.*|.* class:dashed argv:\[git-xxx\]" actual &&
+
+	# We unpeel that and substitute "yyy" into "xxx" (giving "git yyy")
+	# and spawn "git-yyy" and the child will fail.
+	grep "d0|main|alias|.*|alias:xxx argv:\[yyy\]" actual &&
+	grep "d0|main|cmd_name|.*|_run_dashed_ (_run_dashed_/_run_dashed_)" actual &&
+	grep "d0|main|child_start|.*|.* class:dashed argv:\[git-yyy\]" actual &&
+
+	# We unpeel that and substitute "version" into "xxx" (giving
+	# "git version") and update the cmd_name event.
+	grep "d0|main|alias|.*|alias:yyy argv:\[version\]" actual &&
+	grep "d0|main|cmd_name|.*|_run_git_alias_ (_run_dashed_/_run_dashed_/_run_git_alias_)" actual &&
+
+	# These def_param events could be associated with any of the
+	# above cmd_name events.  It does not matter.
+	grep "d0|main|def_param|.*|cfg.prop.foo:red" actual >actual.matches &&
+	grep "d0|main|def_param|.*|ENV_PROP_FOO:blue" actual &&
+
+	# However, we do not want them repeated each time we unpeel.
+	test_line_count = 1 actual.matches &&
+
+	# The "git version" child sees a different cmd_name hierarchy.
+	# Also test the def_param (only for completeness).
+	grep "d1|main|cmd_name|.*|version (_run_dashed_/_run_dashed_/_run_git_alias_/version)" actual &&
+	grep "d1|main|def_param|.*|cfg.prop.foo:red" actual &&
+	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
+'
+
 test_done
Signature · 1 line
gitgitgadget
author
#11 lore raw

PATCH v2 2/3 trace2: avoid emitting 'def_param' set more than once

In reply to Jeff Hostetler via GitGitGadget

From: Jeff Hostetler <[email protected]>

During nested alias expansion it is possible for "trace2_cmd_list_config()" and "trace2_cmd_list_env_vars()" to be called more than once. This causes a full set of 'def_param' events to be emitted each time. Let's avoid that.

Add code to those two functions to only emit them once.

Signed-off-by Jeff Hostetler <[email protected]>
---
 t/t0211-trace2-perf.sh |  2 +-
 trace2.c               | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
t/t0211-trace2-perf.sh +1 -1
index 588c5bad033..7b353195396 100755
@@ -470,7 +470,7 @@ test_expect_success 'expect def_params during shell alias expansion' '
 	grep "d1|main|def_param|.*|ENV_PROP_FOO:blue" actual
 '
 
-test_expect_failure 'expect def_params during nested git alias expansion' '
+test_expect_success 'expect def_params during nested git alias expansion' '
 	test_when_finished "rm prop.perf actual" &&
 
 	test_config_global "trace2.configParams" "cfg.prop.*" &&
trace2.c +12 -0
index f1e268bd159..facce641ef3 100644
@@ -464,17 +464,29 @@ void trace2_cmd_alias_fl(const char *file, int line, const char *alias,
 
 void trace2_cmd_list_config_fl(const char *file, int line)
 {
+	static int emitted = 0;
+
 	if (!trace2_enabled)
 		return;
 
+	if (emitted)
+		return;
+	emitted = 1;
+
 	tr2_cfg_list_config_fl(file, line);
 }
 
 void trace2_cmd_list_env_vars_fl(const char *file, int line)
 {
+	static int emitted = 0;
+
 	if (!trace2_enabled)
 		return;
 
+	if (emitted)
+		return;
+	emitted = 1;
+
 	tr2_list_env_vars_fl(file, line);
 }
Signature · 1 line
gitgitgadget
author
#12 lore raw

PATCH v2 3/3 trace2: emit 'def_param' set with 'cmd_name' event

In reply to Jeff Hostetler via GitGitGadget

From: Jeff Hostetler <[email protected]>

Some commands do not cause a set of 'def_param' events to be emitted. This includes "git-remote-https", "git-http-fetch", and various "query" commands, like "git --man-path".

Since all of these commands do emit a 'cmd_name' event, add code to the "trace2_cmd_name()" function to generate the set of 'def_param' events.

Remove explicit calls to "trace2_cmd_list_config()" and "trace2_cmd_list_env_vars()" in git.c since they are no longer needed.

Reviewed-by Josh Steadmon <[email protected]>
Signed-off-by Jeff Hostetler <[email protected]>
---
 git.c                  | 6 ------
 t/t0211-trace2-perf.sh | 6 +++---
 trace2.c               | 3 +++
 3 files changed, 6 insertions(+), 9 deletions(-)
git.c +0 -6
index 7068a184b0a..a769d72ab8f 100644
@@ -373,8 +373,6 @@ static int handle_alias(int *argcp, const char ***argv)
 			strvec_pushv(&child.args, (*argv) + 1);
 
 			trace2_cmd_alias(alias_command, child.args.v);
-			trace2_cmd_list_config();
-			trace2_cmd_list_env_vars();
 			trace2_cmd_name("_run_shell_alias_");
 
 			ret = run_command(&child);

@@ -411,8 +409,6 @@ static int handle_alias(int *argcp, const char ***argv)
 		COPY_ARRAY(new_argv + count, *argv + 1, *argcp);
 
 		trace2_cmd_alias(alias_command, new_argv);
-		trace2_cmd_list_config();
-		trace2_cmd_list_env_vars();
 
 		*argv = new_argv;
 		*argcp += count - 1;

@@ -462,8 +458,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	trace_argv_printf(argv, "trace: built-in: git");
 	trace2_cmd_name(p->cmd);
-	trace2_cmd_list_config();
-	trace2_cmd_list_env_vars();
 
 	validate_cache_entries(the_repository->index);
 	status = p->fn(argc, argv, prefix);
t/t0211-trace2-perf.sh +3 -3
index 7b353195396..13ef69b92f8 100755
@@ -320,7 +320,7 @@ test_expect_success 'expect def_params for normal builtin command' '
 # Representative query command dispatched in handle_options()
 # in git.c
 #
-test_expect_failure 'expect def_params for query command' '
+test_expect_success 'expect def_params for query command' '
 	try_simple "git --man-path" "_query_"
 '
 

@@ -337,7 +337,7 @@ test_expect_failure 'expect def_params for query command' '
 # remote-curl.c rather than git.c.  Confirm that we get def_param
 # events from both layers.
 #
-test_expect_failure 'expect def_params for remote-curl and _run_dashed_' '
+test_expect_success 'expect def_params for remote-curl and _run_dashed_' '
 	test_when_finished "rm prop.perf actual" &&
 
 	test_config_global "trace2.configParams" "cfg.prop.*" &&

@@ -366,7 +366,7 @@ test_expect_failure 'expect def_params for remote-curl and _run_dashed_' '
 # an executable built from http-fetch.c.  Confirm that we get
 # def_param events from both layers.
 #
-test_expect_failure 'expect def_params for http-fetch and _run_dashed_' '
+test_expect_success 'expect def_params for http-fetch and _run_dashed_' '
 	test_when_finished "rm prop.perf actual" &&
 
 	test_config_global "trace2.configParams" "cfg.prop.*" &&
trace2.c +3 -0
index facce641ef3..f894532d053 100644
@@ -433,6 +433,9 @@ void trace2_cmd_name_fl(const char *file, int line, const char *name)
 	for_each_wanted_builtin (j, tgt_j)
 		if (tgt_j->pfn_command_name_fl)
 			tgt_j->pfn_command_name_fl(file, line, name, hierarchy);
+
+	trace2_cmd_list_config();
+	trace2_cmd_list_env_vars();
 }
 
 void trace2_cmd_mode_fl(const char *file, int line, const char *mode)
Signature · 1 line
gitgitgadget