Ian Wienand <iwienand@xxxxxxxxxx> writes: > This adds a trace point in prepare_cmd, so we can see the actual > command being run without having to resort to strace/code inspection. > > e.g. "test = !echo" when run under GIT_TRACE will show: > > $ GIT_TRACE=1 git test hello > 10:58:56.877234 git.c:755 trace: exec: git-test hello > 10:58:56.877382 run-command.c:657 trace: run_command: git-test hello > 10:58:56.878655 run-command.c:657 trace: run_command: echo hello > 10:58:56.878747 run-command.c:437 trace: prepare_cmd: /usr/bin/echo hello > hello Nice. > A "split" alias, e.g. test = "!echo $*" will show the shell wrapping > and appending of "$@". > > $ GIT_TRACE=1 git test hello > 11:00:45.959420 git.c:755 trace: exec: git-test hello > 11:00:45.959737 run-command.c:657 trace: run_command: git-test hello > 11:00:45.961424 run-command.c:657 trace: run_command: 'echo $*' hello > 11:00:45.961528 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'echo $* "$@"' 'echo $*' hello > hello hello Nice again. But ... > For example, this can be very helpful when an alias is giving you an > unexpected syntax error that is very difficult figure out from only > the run_command trace point, e.g. > > test = "!for i in 1 2 3; do echo $i; done" > > will fail if there is an argument given, we can see why from the > output. ... if the reader truly understands "the alias gives the command and its leading arguments, to which the invocation can supply even more arguments", the reader wouldn't be writing such a command line to begin with, no? So I find the example a bit suboptimal. Hopefully additional explanation in patch 2/3 stressed on that point well enough with much more stress than it talks about the implementation detail of using "sh -c" and "$@", so that readers who read it would not even dream of writing such an alias in the first place. > diff --git a/run-command.c b/run-command.c > index 1b821042b4..36b2b2f194 100644 > --- a/run-command.c > +++ b/run-command.c > @@ -435,6 +435,7 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd) > } > } > > + trace_argv_printf(&out->v[1], "trace: prepare_cmd:"); > return 0; > } Nice addition that is obviously correct. > diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh > index 95568342be..75c8763a6c 100755 > --- a/t/t0014-alias.sh > +++ b/t/t0014-alias.sh > @@ -44,4 +44,11 @@ test_expect_success 'run-command formats empty args properly' ' > test_cmp expect actual > ' > > +test_expect_success 'tracing a shell alias with arguments shows full prepared command' ' > + git config alias.echo "!echo \$*" && > + env GIT_TRACE=1 git echo argument 2>output && > + cp output /tmp/output && > + test_grep "^trace: prepare_cmd: /bin/sh -c '\''echo \$\* \"\$@\"" output > +' This is probably too specific search string, I suspect, given that runcommand.c:prepare_shell_cmd() uses SHELL_PATH or "sh" so if your SHELL_PATH is anything but /bin/sh (or if you are unlucky enough to be running this test on Windows), the pattern would not match. You'd want to loosen it a bit, perhaps with "/bin/sh" -> ".*", as the rest of the output are expected to stay constant. By the way, common to this step and also to the previous step, you'd want to use echo "$@" instead of echo $* to encourage better variable reference hygiene. It makes difference when any arguments given to "e" has whitespace, i.e. $ sh -c 'echo $*' - a 'b c' a b c $ sh -c 'echo "$@"' - a 'b c' a b c Thanks.