{"thread":{"id":"61526","subject":"[PATCH] alias: document caveats and add trace of prepared command","startedAt":"2024-05-22T02:42:09Z","lastAt":"2024-05-27T17:48:39Z","messageCount":43,"participants":["Ian Wienand","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"495261","messageId":"20240522024133.1108005-1-iwienand@redhat.com","threadId":"61526","inReplyTo":null,"subject":"[PATCH] alias: document caveats and add trace of prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-22T02:41:33Z","receivedAt":"2024-05-22T02:42:09Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"There are a number of hidden caveats when using command/shell\nexpansion (\"!\") in an alias. The command is exec'd, unless it can be\nsplit, when it is then run as an argument to \"sh -c\". Split commands\nhave \"$@\" appended.\n\nFirstly this updates the documentation to explain this a little\nclearer.\n\nSecondly, this adds a trace point in prepare_cmd where this\nsubstitution is done, so we can see the command being run without\nhaving to resort to strace/code inspection. e.g. \"test = !echo\" you\nwill show:\n\n $ GIT_TRACE=1 git test hello\n 10:58:56.877234 git.c:755 trace: exec: git-test hello\n 10:58:56.877382 run-command.c:657 trace: run_command: git-test hello\n 10:58:56.878655 run-command.c:657 trace: run_command: echo hello\n 10:58:56.878747 run-command.c:437 trace: prepare_cmd: /usr/bin/echo hello\n hello\n\nFor a \"split\" alias, e.g. test = \"!echo $*\" you will see\n\n $ GIT_TRACE=1 git test hello\n 11:00:45.959420 git.c:755 trace: exec: git-test hello\n 11:00:45.959737 run-command.c:657 trace: run_command: git-test hello\n 11:00:45.961424 run-command.c:657 trace: run_command: 'echo $*' hello\n 11:00:45.961528 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'echo $* \"$@\"' 'echo $*' hello\n hello hello\n\nwhich clearly shows you the appended \"$@\". This can be very helpful\nwhen an alias is giving you an unexpected synatx error that is very\ndifficult figure out from only the run_command trace point,\ne.g. test = \"!for i in 1 2 3; do echo $i; done\"\n\n $ GIT_TRACE=1 test hello\n 11:02:39.813030 git.c:755 trace: exec: git-test hello\n 11:02:39.813233 run-command.c:657 trace: run_command: git-test hello\n 11:02:39.814384 run-command.c:657 trace: run_command: 'for i in 1 2 3; do echo $i; done' hello\n 11:02:39.814468 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'for i in 1 2 3; do echo $i; done \"$@\"' 'for i in 1 2 3; do echo $i; done' hello\n for i in 1 2 3; do echo $i; done: -c: line 1: syntax error near unexpected token `\"$@\"'\n for i in 1 2 3; do echo $i; done: -c: line 1: `for i in 1 2 3; do echo $i; done \"$@\"'\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 26 +++++++++++++++++++++-----\n run-command.c | 1 +\n 2 files changed, 22 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 01df96fab3..a3f090d79d 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -21,8 +21,24 @@ If the alias expansion is prefixed with an exclamation point,\n it will be treated as a shell command. For example, defining\n `alias.new = !gitk --all --not ORIG_HEAD`, the invocation\n `git new` is equivalent to running the shell command\n-`gitk --all --not ORIG_HEAD`. Note that shell commands will be\n-executed from the top-level directory of a repository, which may\n-not necessarily be the current directory.\n-`GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n-from the original current directory. See linkgit:git-rev-parse[1].\n+`gitk --all --not ORIG_HEAD`. Note:\n++\n+* Shell commands will be executed from the top-level directory of a\n+ repository, which may not necessarily be the current directory.\n+* `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n+ from the original current directory. See linkgit:git-rev-parse[1].\n+* Single commands will be executed directly. Commands that can be \"split\"\n+ (contain whitespace or shell-reserved characters) will be run as shell\n+ commands via an argument to `sh -c`.\n+* When arguments are present to a \"split\" command running in a shell,\n+ the shell command will have `\"$@\"` appended. The first non-command\n+ argument to `sh -c` (i.e. `$0` to the command) is always the alias\n+ string, and other user specified arguments will follow.\n+** This may initially be confusing if your command references argument\n+ variables or is not expecting the presence of `\"$@\"`. For example:\n+ `alias.echo = \"!echo $1\"` when run as `git echo arg` will execute\n+ `sh -c \"echo $1 $@\" \"echo $1\" \"1\"` resulting in output `1 1`.\n+ An alias `alias.for = \"!for i in 1 2 3; do echo $i; done\"` will fail\n+ if any arguments are specified to `git for` as the appended `\"$@\"` will\n+ create invalid shell syntax. Setting `GIT_TRACE=1` can help debug\n+ the command being run.\ndiff --git a/run-command.c b/run-command.c\nindex 1b821042b4..36b2b2f194 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -435,6 +435,7 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd)\n \t\t}\n \t}\n \n+\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n \treturn 0;\n }\n \n-- \n2.45.1\n\n"},{"id":"495262","messageId":"CAPig+cSDZzwH5RLYXbUXvdePHUtrPDDEdapQakB22zBkO25kuw@mail.gmail.com","threadId":"61526","inReplyTo":"20240522024133.1108005-1-iwienand@redhat.com","subject":"Re: [PATCH] alias: document caveats and add trace of prepared command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-05-22T03:29:43Z","receivedAt":"2024-05-22T03:29:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 21, 2024 at 10:42 PM Ian Wienand wrote:\n> [...]\n> For a \"split\" alias, e.g. test = \"!echo $*\" you will see\n>\n> $ GIT_TRACE=1 git test hello\n> 11:00:45.959420 git.c:755 trace: exec: git-test hello\n> 11:00:45.959737 run-command.c:657 trace: run_command: git-test hello\n> 11:00:45.961424 run-command.c:657 trace: run_command: 'echo $*' hello\n> 11:00:45.961528 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'echo $* \"$@\"' 'echo $*' hello\n> hello hello\n>\n> which clearly shows you the appended \"$@\". This can be very helpful\n> when an alias is giving you an unexpected synatx error that is very\n\ns/synatx/syntax/\n\n> difficult figure out from only the run_command trace point,\n> e.g. test = \"!for i in 1 2 3; do echo $i; done\"\n> [...]\n> Signed-off-by: Ian Wienand \n"},{"id":"495302","messageId":"xmqq8r017rtw.fsf@gitster.g","threadId":"61526","inReplyTo":"20240522024133.1108005-1-iwienand@redhat.com","subject":"Re: [PATCH] alias: document caveats and add trace of prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-22T16:07:55Z","receivedAt":"2024-05-22T16:08:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> For a \"split\" alias, e.g. test = \"!echo $*\" you will see\n>\n> $ GIT_TRACE=1 git test hello\n> 11:00:45.959420 git.c:755 trace: exec: git-test hello\n> 11:00:45.959737 run-command.c:657 trace: run_command: git-test hello\n> 11:00:45.961424 run-command.c:657 trace: run_command: 'echo $*' hello\n> 11:00:45.961528 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'echo $* \"$@\"' 'echo $*' hello\n> hello hello\n>\n> which clearly shows you the appended \"$@\". \n\nA question and a comment on this part.\n\nWho are \"you\" in this context?\n\nIt is somewhat surprising if we do not consistently add \"$@\" (or do\nan equivalent), as the point of adding \"$@\" is to allow an alias to\nspecify \"the initial part of a command line\", e.g.\n\n\t[alias] lg = log --oneline\n\nto let you say \"git lg\" to mean \"git log --oneline\" and also you can\nsay \"git lg --graph\" to mean \"git log --oneline --graph\". In other\nwords, what you type after \"git lg\" makes \"the rest of the command\nline\", while alias gives \"the initial part\".\n\nAre you sure that with\n\n\t[alias] lg = log\n\nyou get the rest of the command line ignored, in other words, if you\nsay \"git lg --oneline\", it does not do \"git log --oneline\"?\n\n> Documentation/config/alias.txt | 26 +++++++++++++++++++++-----\n> run-command.c | 1 +\n> 2 files changed, 22 insertions(+), 5 deletions(-)\n>\n> diff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\n> index 01df96fab3..a3f090d79d 100644\n> --- a/Documentation/config/alias.txt\n> +++ b/Documentation/config/alias.txt\n> @@ -21,8 +21,24 @@ If the alias expansion is prefixed with an exclamation point,\n> it will be treated as a shell command. For example, defining\n> `alias.new = !gitk --all --not ORIG_HEAD`, the invocation\n> `git new` is equivalent to running the shell command\n> -`gitk --all --not ORIG_HEAD`. Note that shell commands will be\n> -executed from the top-level directory of a repository, which may\n> -not necessarily be the current directory.\n> -`GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n> -from the original current directory. See linkgit:git-rev-parse[1].\n> +`gitk --all --not ORIG_HEAD`. Note:\n> ++\n> +* Shell commands will be executed from the top-level directory of a\n> + repository, which may not necessarily be the current directory.\n> +* `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n> + from the original current directory. See linkgit:git-rev-parse[1].\n> +* Single commands will be executed directly. Commands that can be \"split\"\n> + (contain whitespace or shell-reserved characters) will be run as shell\n> + commands via an argument to `sh -c`.\n> +* When arguments are present to a \"split\" command running in a shell,\n> + the shell command will have `\"$@\"` appended. The first non-command\n> + argument to `sh -c` (i.e. `$0` to the command) is always the alias\n> + string, and other user specified arguments will follow.\n> +** This may initially be confusing if your command references argument\n> + variables or is not expecting the presence of `\"$@\"`. For example:\n> + `alias.echo = \"!echo $1\"` when run as `git echo arg` will execute\n> + `sh -c \"echo $1 $@\" \"echo $1\" \"1\"` resulting in output `1 1`.\n> + An alias `alias.for = \"!for i in 1 2 3; do echo $i; done\"` will fail\n> + if any arguments are specified to `git for` as the appended `\"$@\"` will\n> + create invalid shell syntax. Setting `GIT_TRACE=1` can help debug\n> + the command being run.\n\nThe above does a bit too many things in a single patch to be\nreviewable. Perhaps make the above change in two patches?\n\n (1) Reformulate the existing test into \"Note:\" followed by a\n bulletted list.\n\n (2) Add new items to the updated and easier-to-read form prepared\n by the first patch.\n\nYou have a lengthy new text in the documentation to help confused\nusers, but it looks to me that it places too much stress on the\nmechanics (i.e. '$@' is added to the command line) without saying\nmuch about the intent (i.e. you need to use '$@' to allow the\ncommand line invocation to supply \"the rest of the command line\"\nwhen you are running the alias via the shell). I've learned over\nthe course of this project that readers learn better when the intent\nbehind the mechanics is given in an understandable form.\n\nI think the idea is \"we want the 'concatenation of what the alias\nsupplies and what the user gives when invoking the alias from the\ncommand line' to work sensibly\". The most generic way to do so when\nprocessing \"git lg --graph -3\" with \"[alias] lg = !git log --oneline\" is\n\n sh -c 'git log --oneline \"$@\"' - '--graph' '-3'\n\n(readers can try this at home by adding 'echo ' before 'log'). We\nmay try to \"optimize\" the most generic pattern when there is no need\nto use \"sh -c\" go a more direct route. For example, if you have\n\n [alias] !foo = bar\n\nthen there is no need to use \"sh -c\" in the first place. \"git foo\nbaz quux\" should behave just like when you said \"sh bar baz quux\" on\nthe command line, i.e. execute your shell script \"bar\" and give\n\"baz\" and \"quux\" as two arguments to that script. We can prepare\nargv[] = (\"sh\", \"bar\", \"baz\", \"quux\") and fork-exec that, so the\ncommand line the underlying exec(2) call sees may not have \"$@\" (and\nit will not have \"-c\", either).\n\nYou can undo the \"optimization\" and rewrite the command line to\n\n sh -c 'bar \"$@\"' - 'baz' 'quux'\n\nand it should mean the same thing.\n\n> diff --git a/run-command.c b/run-command.c\n> index 1b821042b4..36b2b2f194 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -435,6 +435,7 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd)\n> \t\t}\n> \t}\n> \n> +\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n> \treturn 0;\n> }\n\nHmph, we do not have much tests that look for 'trace: foo' in our\ntest suite, but t0014 seems to use it. Don't we need to cover this\nnew trace output in the test in the same patch (probably becomes\npatch 3 after the two documentation changes)?\n\nThanks.\n"},{"id":"495339","messageId":"Zk6P8AMIBUt5X1A1@fedora19.localdomain","threadId":"61526","inReplyTo":"xmqq8r017rtw.fsf@gitster.g","subject":"Re: [PATCH] alias: document caveats and add trace of prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-23T00:38:08Z","receivedAt":"2024-05-23T00:38:18Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"On Wed, May 22, 2024 at 09:07:55AM -0700, Junio C Hamano wrote:\n> Ian Wienand writes:\n> \n> > For a \"split\" alias, e.g. test = \"!echo $*\" you will see\n> >\n> > $ GIT_TRACE=1 git test hello\n> > 11:00:45.959420 git.c:755 trace: exec: git-test hello\n> > 11:00:45.959737 run-command.c:657 trace: run_command: git-test hello\n> > 11:00:45.961424 run-command.c:657 trace: run_command: 'echo $*' hello\n> > 11:00:45.961528 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'echo $* \"$@\"' 'echo $*' hello\n> > hello hello\n> >\n> > which clearly shows you the appended \"$@\". \n> \n> A question and a comment on this part.\n> \n> Who are \"you\" in this context?\n\nHeh, \"you\" is the person who has been given a large git alias that\nprobably should have been written some other way (but likely due to\nthe fact that stuffing all the workflow logic into a git config-file\nmeans that nothing extra like shell/python scripts needed to be\ndistributed) that \"you\" have to now debug :) I feel the extra info in\nGIT_TRACE output showing what is _actually_ happening after\nprepare_cmd would really help this person (who was me :).\n\n> It is somewhat surprising if we do not consistently add \"$@\" (or do\n> an equivalent), as the point of adding \"$@\" is to allow an alias to\n> specify \"the initial part of a command line\", e.g.\n> \n> \t[alias] lg = log --oneline\n> \n> to let you say \"git lg\" to mean \"git log --oneline\" and also you can\n> say \"git lg --graph\" to mean \"git log --oneline --graph\". In other\n> words, what you type after \"git lg\" makes \"the rest of the command\n> line\", while alias gives \"the initial part\".\n> \n> Are you sure that with\n> \n> \t[alias] lg = log\n> \n> you get the rest of the command line ignored, in other words, if you\n> say \"git lg --oneline\", it does not do \"git log --oneline\"?\n\nI'm not quite sure I'm following here, sorry. The behaviour is\ndifferent for \"shell\" aliases prefixed with !. There the stragtegy is\nto append \"$@\", but only if there are arguments. Whatever the merits\nor not of that, I feel like it's impossible to change because it's out\nthere now.\n\nNow I've been looking, I've seen a number of interesting ways people\nhave been dealing with this. Some people have ignored it, for example\nI've seen credential helpers use \":\" to ignore it, like\n\n \"!echo password=\\$${TOKEN_NAME}; :\"\n\nThe git documentation for credential helpers gives another way,\ndefining an inline function, so the \"$@\" appended becomes function\narguments to \"f()\" like\n\n \"!f() { echo $1 $2 ; }; f\"\n\nThen I've seen other people nest in a \"sh -c\" call, and \"fake\" $0 so\nthat the appended \"$@\" becomes the $1 arguments to the interior shell\ncall (cue quoting nightmares!) like\n\n \"!sh -c 'echo $1 $2' foo\"\n\nOnce you realise what is going on, these are somewhat understandable,\nbut the documentation as written doesn't guide you in the right\ndirection.\n\n> The above does a bit too many things in a single patch to be\n> reviewable. Perhaps make the above change in two patches?\n\nSure, I am happy to split this up\n\n> You have a lengthy new text in the documentation to help confused\n> users, but it looks to me that it places too much stress on the\n> mechanics (i.e. '$@' is added to the command line) without saying\n> much about the intent (i.e. you need to use '$@' to allow the\n> command line invocation to supply \"the rest of the command line\"\n> when you are running the alias via the shell). I've learned over\n> the course of this project that readers learn better when the intent\n> behind the mechanics is given in an understandable form.\n\nLet me take your feedback here and work it into the split up change,\nand we can discuss further. Thanks.\n\n> > diff --git a/run-command.c b/run-command.c\n> > index 1b821042b4..36b2b2f194 100644\n> > --- a/run-command.c\n> > +++ b/run-command.c\n> > @@ -435,6 +435,7 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd)\n> > \t\t}\n> > \t}\n> > \n> > +\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n> > \treturn 0;\n> > }\n> \n> Hmph, we do not have much tests that look for 'trace: foo' in our\n> test suite, but t0014 seems to use it. Don't we need to cover this\n> new trace output in the test in the same patch (probably becomes\n> patch 3 after the two documentation changes)?\n\nOk, thanks for the pointer, I'll look at expanding this one.\n\n-i\n\n"},{"id":"495343","messageId":"20240523042143.1220862-1-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240522024133.1108005-1-iwienand@redhat.com","subject":"[PATCH v2 1/3] Documentation: alias: rework notes into points","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-23T04:20:57Z","receivedAt":"2024-05-23T04:21:59Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"There are a number of caveats when using aliases. Rather than\nstuffing them all together in a paragraph, let's separate them out\ninto individual points to make it clearer what's going on.\n---\n Documentation/config/alias.txt | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 01df96fab3..40851ef429 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -21,8 +21,9 @@ If the alias expansion is prefixed with an exclamation point,\n it will be treated as a shell command. For example, defining\n `alias.new = !gitk --all --not ORIG_HEAD`, the invocation\n `git new` is equivalent to running the shell command\n-`gitk --all --not ORIG_HEAD`. Note that shell commands will be\n-executed from the top-level directory of a repository, which may\n-not necessarily be the current directory.\n-`GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n-from the original current directory. See linkgit:git-rev-parse[1].\n+`gitk --all --not ORIG_HEAD`. Note:\n++\n+* Shell commands will be executed from the top-level directory of a\n+ repository, which may not necessarily be the current directory.\n+* `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n+ from the original current directory. See linkgit:git-rev-parse[1].\n-- \n2.45.1\n\n"},{"id":"495344","messageId":"20240523042143.1220862-2-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240523042143.1220862-1-iwienand@redhat.com","subject":"[PATCH v2 2/3] Documentation: alias: add notes on shell expansion","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-23T04:20:58Z","receivedAt":"2024-05-23T04:22:03Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"When writing inline shell for shell-expansion aliases (i.e. prefixed\nwith \"!\"), there are some caveats around argument parsing to be aware\nof. This series of notes attempts to explain what is happening more\nclearly.\n---\n Documentation/config/alias.txt | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 40851ef429..51fa876c91 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -27,3 +27,24 @@ it will be treated as a shell command. For example, defining\n repository, which may not necessarily be the current directory.\n * `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n from the original current directory. See linkgit:git-rev-parse[1].\n+* If the shell alias is the full path to a binary, it will be executed\n+ directly with any arguments.\n+* If the alias contains any whitespace or reserved characters, it will\n+ be considered an inline script and run as an argument to `sh -c`.\n+* When running as a script, Git appends \"$@\" to the alias shell\n+ command when arguments are present. If there are no arguments,\n+ `\"$@\"` will not be appended.\n+** This may initially be confusing if your alias script references\n+ argument variables, or is otherwise not expecting the presence of\n+ `\"$@\"`. For example: `alias.echo = \"!echo $1\"` when run as `git\n+ echo arg` will actually execute `sh -c \"echo $1 $@\" \"echo $1\"\n+ \"arg\"` resulting in output `arg arg`. An alias `alias.for = \"!for\n+ i in 1 2 3; do echo $i; done\"` will fail if any arguments are\n+ specified to `git for` as the appended `\"$@\"` will create invalid\n+ shell syntax.\n+** A convenient way to deal with this is to write your script\n+ operations in an inline function that is then called with any\n+ arguments from the command-line. For example `alias.echo = \"!e() {\n+ echo $* ; }; e\" will work as expected, with the function `e`\n+ receiving any arugments from the command-line.\n+** Setting `GIT_TRACE=1` can help debug the command being run.\n-- \n2.45.1\n\n"},{"id":"495345","messageId":"20240523042143.1220862-3-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240523042143.1220862-1-iwienand@redhat.com","subject":"[PATCH v2 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-23T04:20:59Z","receivedAt":"2024-05-23T04:22:06Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"This adds a trace point in prepare_cmd, so we can see the actual\ncommand being run without having to resort to strace/code inspection.\n\ne.g. \"test = !echo\" when run under GIT_TRACE will show:\n\n $ GIT_TRACE=1 git test hello\n 10:58:56.877234 git.c:755 trace: exec: git-test hello\n 10:58:56.877382 run-command.c:657 trace: run_command: git-test hello\n 10:58:56.878655 run-command.c:657 trace: run_command: echo hello\n 10:58:56.878747 run-command.c:437 trace: prepare_cmd: /usr/bin/echo hello\n hello\n\nA \"split\" alias, e.g. test = \"!echo $*\" will show the shell wrapping\nand appending of \"$@\".\n\n $ GIT_TRACE=1 git test hello\n 11:00:45.959420 git.c:755 trace: exec: git-test hello\n 11:00:45.959737 run-command.c:657 trace: run_command: git-test hello\n 11:00:45.961424 run-command.c:657 trace: run_command: 'echo $*' hello\n 11:00:45.961528 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'echo $* \"$@\"' 'echo $*' hello\n hello hello\n\nFor example, this can be very helpful when an alias is giving you an\nunexpected syntax error that is very difficult figure out from only\nthe run_command trace point, e.g.\n\ntest = \"!for i in 1 2 3; do echo $i; done\"\n\nwill fail if there is an argument given, we can see why from the\noutput.\n\n $ GIT_TRACE=1 test hello\n 11:02:39.813030 git.c:755 trace: exec: git-test hello\n 11:02:39.813233 run-command.c:657 trace: run_command: git-test hello\n 11:02:39.814384 run-command.c:657 trace: run_command: 'for i in 1 2 3; do echo $i; done' hello\n 11:02:39.814468 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'for i in 1 2 3; do echo $i; done \"$@\"' 'for i in 1 2 3; do echo $i; done' hello\n for i in 1 2 3; do echo $i; done: -c: line 1: syntax error near unexpected token `\"$@\"'\n for i in 1 2 3; do echo $i; done: -c: line 1: `for i in 1 2 3; do echo $i; done \"$@\"'\n\nA test case is added.\n---\n run-command.c | 1 +\n t/t0014-alias.sh | 7 +++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 1b821042b4..36b2b2f194 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -435,6 +435,7 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd)\n \t\t}\n \t}\n \n+\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n \treturn 0;\n }\n \ndiff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\nindex 95568342be..75c8763a6c 100755\n--- a/t/t0014-alias.sh\n+++ b/t/t0014-alias.sh\n@@ -44,4 +44,11 @@ test_expect_success 'run-command formats empty args properly' '\n test_cmp expect actual\n '\n \n+test_expect_success 'tracing a shell alias with arguments shows full prepared command' '\n+\tgit config alias.echo \"!echo \\$*\" &&\n+\tenv GIT_TRACE=1 git echo argument 2>output &&\n+\tcp output /tmp/output &&\n+\ttest_grep \"^trace: prepare_cmd: /bin/sh -c '\\''echo \\$\\* \\\"\\$@\\\"\" output\n+'\n+\n test_done\n-- \n2.45.1\n\n"},{"id":"495346","messageId":"CAPig+cTPmiF_f-fsdNPF4A5NoSWrmp6sMPSNY1JFoxipyr7qgg@mail.gmail.com","threadId":"61526","inReplyTo":"20240523042143.1220862-1-iwienand@redhat.com","subject":"Re: [PATCH v2 1/3] Documentation: alias: rework notes into points","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-05-23T04:27:17Z","receivedAt":"2024-05-23T04:27:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 23, 2024 at 12:22 AM Ian Wienand wrote:\n> There are a number of caveats when using aliases. Rather than\n> stuffing them all together in a paragraph, let's separate them out\n> into individual points to make it clearer what's going on.\n> ---\n\nMissing sign-off.\n"},{"id":"495347","messageId":"20240523043806.1223032-1-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240523042143.1220862-1-iwienand@redhat.com","subject":"[PATCH v3 1/3] Documentation: alias: rework notes into points","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-23T04:37:54Z","receivedAt":"2024-05-23T04:38:27Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"There are a number of caveats when using aliases. Rather than\nstuffing them all together in a paragraph, let's separate them out\ninto individual points to make it clearer what's going on.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 01df96fab3..40851ef429 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -21,8 +21,9 @@ If the alias expansion is prefixed with an exclamation point,\n it will be treated as a shell command. For example, defining\n `alias.new = !gitk --all --not ORIG_HEAD`, the invocation\n `git new` is equivalent to running the shell command\n-`gitk --all --not ORIG_HEAD`. Note that shell commands will be\n-executed from the top-level directory of a repository, which may\n-not necessarily be the current directory.\n-`GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n-from the original current directory. See linkgit:git-rev-parse[1].\n+`gitk --all --not ORIG_HEAD`. Note:\n++\n+* Shell commands will be executed from the top-level directory of a\n+ repository, which may not necessarily be the current directory.\n+* `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n+ from the original current directory. See linkgit:git-rev-parse[1].\n-- \n2.45.1\n\n"},{"id":"495348","messageId":"20240523043806.1223032-2-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240523043806.1223032-1-iwienand@redhat.com","subject":"[PATCH v3 2/3] Documentation: alias: add notes on shell expansion","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-23T04:37:55Z","receivedAt":"2024-05-23T04:38:29Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"When writing inline shell for shell-expansion aliases (i.e. prefixed\nwith \"!\"), there are some caveats around argument parsing to be aware\nof. This series of notes attempts to explain what is happening more\nclearly.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 40851ef429..51fa876c91 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -27,3 +27,24 @@ it will be treated as a shell command. For example, defining\n repository, which may not necessarily be the current directory.\n * `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n from the original current directory. See linkgit:git-rev-parse[1].\n+* If the shell alias is the full path to a binary, it will be executed\n+ directly with any arguments.\n+* If the alias contains any whitespace or reserved characters, it will\n+ be considered an inline script and run as an argument to `sh -c`.\n+* When running as a script, Git appends \"$@\" to the alias shell\n+ command when arguments are present. If there are no arguments,\n+ `\"$@\"` will not be appended.\n+** This may initially be confusing if your alias script references\n+ argument variables, or is otherwise not expecting the presence of\n+ `\"$@\"`. For example: `alias.echo = \"!echo $1\"` when run as `git\n+ echo arg` will actually execute `sh -c \"echo $1 $@\" \"echo $1\"\n+ \"arg\"` resulting in output `arg arg`. An alias `alias.for = \"!for\n+ i in 1 2 3; do echo $i; done\"` will fail if any arguments are\n+ specified to `git for` as the appended `\"$@\"` will create invalid\n+ shell syntax.\n+** A convenient way to deal with this is to write your script\n+ operations in an inline function that is then called with any\n+ arguments from the command-line. For example `alias.echo = \"!e() {\n+ echo $* ; }; e\" will work as expected, with the function `e`\n+ receiving any arugments from the command-line.\n+** Setting `GIT_TRACE=1` can help debug the command being run.\n-- \n2.45.1\n\n"},{"id":"495349","messageId":"20240523043806.1223032-3-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240523043806.1223032-1-iwienand@redhat.com","subject":"[PATCH v3 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-23T04:37:56Z","receivedAt":"2024-05-23T04:38:32Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"This adds a trace point in prepare_cmd, so we can see the actual\ncommand being run without having to resort to strace/code inspection.\n\ne.g. \"test = !echo\" when run under GIT_TRACE will show:\n\n $ GIT_TRACE=1 git test hello\n 10:58:56.877234 git.c:755 trace: exec: git-test hello\n 10:58:56.877382 run-command.c:657 trace: run_command: git-test hello\n 10:58:56.878655 run-command.c:657 trace: run_command: echo hello\n 10:58:56.878747 run-command.c:437 trace: prepare_cmd: /usr/bin/echo hello\n hello\n\nA \"split\" alias, e.g. test = \"!echo $*\" will show the shell wrapping\nand appending of \"$@\".\n\n $ GIT_TRACE=1 git test hello\n 11:00:45.959420 git.c:755 trace: exec: git-test hello\n 11:00:45.959737 run-command.c:657 trace: run_command: git-test hello\n 11:00:45.961424 run-command.c:657 trace: run_command: 'echo $*' hello\n 11:00:45.961528 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'echo $* \"$@\"' 'echo $*' hello\n hello hello\n\nFor example, this can be very helpful when an alias is giving you an\nunexpected syntax error that is very difficult figure out from only\nthe run_command trace point, e.g.\n\ntest = \"!for i in 1 2 3; do echo $i; done\"\n\nwill fail if there is an argument given, we can see why from the\noutput.\n\n $ GIT_TRACE=1 test hello\n 11:02:39.813030 git.c:755 trace: exec: git-test hello\n 11:02:39.813233 run-command.c:657 trace: run_command: git-test hello\n 11:02:39.814384 run-command.c:657 trace: run_command: 'for i in 1 2 3; do echo $i; done' hello\n 11:02:39.814468 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'for i in 1 2 3; do echo $i; done \"$@\"' 'for i in 1 2 3; do echo $i; done' hello\n for i in 1 2 3; do echo $i; done: -c: line 1: syntax error near unexpected token `\"$@\"'\n for i in 1 2 3; do echo $i; done: -c: line 1: `for i in 1 2 3; do echo $i; done \"$@\"'\n\nA test case is added.\n\nSigned-off-by: Ian Wienand \n---\n run-command.c | 1 +\n t/t0014-alias.sh | 7 +++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 1b821042b4..36b2b2f194 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -435,6 +435,7 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd)\n \t\t}\n \t}\n \n+\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n \treturn 0;\n }\n \ndiff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\nindex 95568342be..75c8763a6c 100755\n--- a/t/t0014-alias.sh\n+++ b/t/t0014-alias.sh\n@@ -44,4 +44,11 @@ test_expect_success 'run-command formats empty args properly' '\n test_cmp expect actual\n '\n \n+test_expect_success 'tracing a shell alias with arguments shows full prepared command' '\n+\tgit config alias.echo \"!echo \\$*\" &&\n+\tenv GIT_TRACE=1 git echo argument 2>output &&\n+\tcp output /tmp/output &&\n+\ttest_grep \"^trace: prepare_cmd: /bin/sh -c '\\''echo \\$\\* \\\"\\$@\\\"\" output\n+'\n+\n test_done\n-- \n2.45.1\n\n"},{"id":"495350","messageId":"Zk7Ikeo_bc9AIHgm@fedora19.localdomain","threadId":"61526","inReplyTo":"CAPig+cTPmiF_f-fsdNPF4A5NoSWrmp6sMPSNY1JFoxipyr7qgg@mail.gmail.com","subject":"Re: [PATCH v2 1/3] Documentation: alias: rework notes into points","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-23T04:39:45Z","receivedAt":"2024-05-23T04:39:55Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"On Thu, May 23, 2024 at 12:27:17AM -0400, Eric Sunshine wrote:\n> On Thu, May 23, 2024 at 12:22 AM Ian Wienand wrote:\n> > There are a number of caveats when using aliases. Rather than\n> > stuffing them all together in a paragraph, let's separate them out\n> > into individual points to make it clearer what's going on.\n> > ---\n> \n> Missing sign-off.\n\nSigh .. thanks sent v3\n\n-i\n\n"},{"id":"495409","messageId":"xmqq7cfky30j.fsf@gitster.g","threadId":"61526","inReplyTo":"20240523043806.1223032-1-iwienand@redhat.com","subject":"Re: [PATCH v3 1/3] Documentation: alias: rework notes into points","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-23T15:14:04Z","receivedAt":"2024-05-23T15:14:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> There are a number of caveats when using aliases. Rather than\n> stuffing them all together in a paragraph, let's separate them out\n> into individual points to make it clearer what's going on.\n\nNicely explained.\n\n> diff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\n> index 01df96fab3..40851ef429 100644\n> --- a/Documentation/config/alias.txt\n> +++ b/Documentation/config/alias.txt\n> @@ -21,8 +21,9 @@ If the alias expansion is prefixed with an exclamation point,\n> it will be treated as a shell command. For example, defining\n> `alias.new = !gitk --all --not ORIG_HEAD`, the invocation\n> `git new` is equivalent to running the shell command\n> +`gitk --all --not ORIG_HEAD`. Note:\n> ++\n> +* Shell commands will be executed from the top-level directory of a\n> + repository, which may not necessarily be the current directory.\n> +* `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n> + from the original current directory. See linkgit:git-rev-parse[1].\n\nLooking for '[NOTE]' in the Documentation/ files finds sections\nmarked up like this (this one is from Documentation/git-blame.txt):\n\n ...\n parser (which should be quite natural for most scripting languages).\n +\n [NOTE]\n For people who do parsing: to make it more robust, just ignore any\n lines between the first and last one (\"\" and \"filename\" lines)\n ...\n\nand its rendition looks like this:\n\nhttps://git.github.io/htmldocs/git-blame.html#:~:text=most%20scripting%20languages).-,Note,-For%20people%20who\n\nI am undecided if it gives a better presentation, especially when we\nare giving bulletted list, so let's take the patch as-is and leave\nit to interested folks to explore the use of [NOTE] _after_ this set\nof patches lands as #leftoverbit follow-up topic.\n\nThanks.\n"},{"id":"495410","messageId":"xmqqpltcwnqm.fsf@gitster.g","threadId":"61526","inReplyTo":"20240523043806.1223032-3-iwienand@redhat.com","subject":"Re: [PATCH v3 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-23T15:29:21Z","receivedAt":"2024-05-23T15:29:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> This adds a trace point in prepare_cmd, so we can see the actual\n> command being run without having to resort to strace/code inspection.\n>\n> e.g. \"test = !echo\" when run under GIT_TRACE will show:\n>\n> $ GIT_TRACE=1 git test hello\n> 10:58:56.877234 git.c:755 trace: exec: git-test hello\n> 10:58:56.877382 run-command.c:657 trace: run_command: git-test hello\n> 10:58:56.878655 run-command.c:657 trace: run_command: echo hello\n> 10:58:56.878747 run-command.c:437 trace: prepare_cmd: /usr/bin/echo hello\n> hello\n\nNice.\n\n> A \"split\" alias, e.g. test = \"!echo $*\" will show the shell wrapping\n> and appending of \"$@\".\n>\n> $ GIT_TRACE=1 git test hello\n> 11:00:45.959420 git.c:755 trace: exec: git-test hello\n> 11:00:45.959737 run-command.c:657 trace: run_command: git-test hello\n> 11:00:45.961424 run-command.c:657 trace: run_command: 'echo $*' hello\n> 11:00:45.961528 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'echo $* \"$@\"' 'echo $*' hello\n> hello hello\n\nNice again.\n\nBut ...\n\n> For example, this can be very helpful when an alias is giving you an\n> unexpected syntax error that is very difficult figure out from only\n> the run_command trace point, e.g.\n>\n> test = \"!for i in 1 2 3; do echo $i; done\"\n>\n> will fail if there is an argument given, we can see why from the\n> output.\n\n... if the reader truly understands \"the alias gives the command and\nits leading arguments, to which the invocation can supply even more\narguments\", the reader wouldn't be writing such a command line to\nbegin with, no?\n\nSo I find the example a bit suboptimal. Hopefully additional\nexplanation in patch 2/3 stressed on that point well enough with\nmuch more stress than it talks about the implementation detail of\nusing \"sh -c\" and \"$@\", so that readers who read it would not even\ndream of writing such an alias in the first place.\n\n> diff --git a/run-command.c b/run-command.c\n> index 1b821042b4..36b2b2f194 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -435,6 +435,7 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd)\n> \t\t}\n> \t}\n> \n> +\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n> \treturn 0;\n> }\n\nNice addition that is obviously correct.\n\n> diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\n> index 95568342be..75c8763a6c 100755\n> --- a/t/t0014-alias.sh\n> +++ b/t/t0014-alias.sh\n> @@ -44,4 +44,11 @@ test_expect_success 'run-command formats empty args properly' '\n> test_cmp expect actual\n> '\n> \n> +test_expect_success 'tracing a shell alias with arguments shows full prepared command' '\n> +\tgit config alias.echo \"!echo \\$*\" &&\n> +\tenv GIT_TRACE=1 git echo argument 2>output &&\n> +\tcp output /tmp/output &&\n> +\ttest_grep \"^trace: prepare_cmd: /bin/sh -c '\\''echo \\$\\* \\\"\\$@\\\"\" output\n> +'\n\nThis is probably too specific search string, I suspect, given that\nruncommand.c:prepare_shell_cmd() uses SHELL_PATH or \"sh\" so if your\nSHELL_PATH is anything but /bin/sh (or if you are unlucky enough to\nbe running this test on Windows), the pattern would not match.\nYou'd want to loosen it a bit, perhaps with \"/bin/sh\" -> \".*\", as\nthe rest of the output are expected to stay constant.\n\nBy the way, common to this step and also to the previous step,\nyou'd want to use\n\n\techo \"$@\"\n\ninstead of\n\n\techo $*\n\nto encourage better variable reference hygiene. It makes difference\nwhen any arguments given to \"e\" has whitespace, i.e.\n\n $ sh -c 'echo $*' - a 'b c'\n a b c\n $ sh -c 'echo \"$@\"' - a 'b c'\n a b c\n\nThanks.\n"},{"id":"495500","messageId":"xmqqttiop05y.fsf@gitster.g","threadId":"61526","inReplyTo":"xmqqpltcwnqm.fsf@gitster.g","subject":"Re: [PATCH v3 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-23T23:40:25Z","receivedAt":"2024-05-23T23:40:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> +\tgit config alias.echo \"!echo \\$*\" &&\n>> +\tenv GIT_TRACE=1 git echo argument 2>output &&\n>> +\tcp output /tmp/output &&\n>> +\ttest_grep \"^trace: prepare_cmd: /bin/sh -c '\\''echo \\$\\* \\\"\\$@\\\"\" output\n>> +'\n>\n> This is probably too specific search string, ...\n\nAnd of course, 'seen' no longer passes the test with this topic\nmerged when run with SHELL_PATH=/bin/dash for obvious reasons.\n\nI've added the following band-aid to help 'seen' pass CI, but this\nhas to be fixed in the main patch by squashing it in.\n\nThanks.\n\n--- >8 ---\nSubject: [PATCH] fixup! run-command: show prepared command\n\nDo not assume everybody sets SHELL_PATH to /bin/sh\n---\n t/t0014-alias.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\nindex 75c8763a6c..0d42d2b454 100755\n--- a/t/t0014-alias.sh\n+++ b/t/t0014-alias.sh\n@@ -48,7 +48,8 @@ test_expect_success 'tracing a shell alias with arguments shows full prepared co\n \tgit config alias.echo \"!echo \\$*\" &&\n \tenv GIT_TRACE=1 git echo argument 2>output &&\n \tcp output /tmp/output &&\n-\ttest_grep \"^trace: prepare_cmd: /bin/sh -c '\\''echo \\$\\* \\\"\\$@\\\"\" output\n+\t# \".*\" can be \"sh\", or SHELL_PATH (not always \"/bin/sh\")\n+\ttest_grep \"^trace: prepare_cmd: .* -c '\\''echo \\$\\* \\\"\\$@\\\"\" output\n '\n \n test_done\n-- \n2.45.1-246-gb9cfe4845c\n\n"},{"id":"495508","messageId":"Zk_ip35jHUj_5M94@fedora19.localdomain","threadId":"61526","inReplyTo":"xmqqpltcwnqm.fsf@gitster.g","subject":"Re: [PATCH v3 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-24T00:43:19Z","receivedAt":"2024-05-24T00:43:28Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"On Thu, May 23, 2024 at 08:29:21AM -0700, Junio C Hamano wrote:\n> > For example, this can be very helpful when an alias is giving you an\n> > unexpected syntax error that is very difficult figure out from only\n> > the run_command trace point, e.g.\n> >\n> > test = \"!for i in 1 2 3; do echo $i; done\"\n> >\n> > will fail if there is an argument given, we can see why from the\n> > output.\n> \n> ... if the reader truly understands \"the alias gives the command and\n> its leading arguments, to which the invocation can supply even more\n> arguments\", the reader wouldn't be writing such a command line to\n> begin with, no?\n> \n> So I find the example a bit suboptimal. Hopefully additional\n> explanation in patch 2/3 stressed on that point well enough with\n> much more stress than it talks about the implementation detail of\n> using \"sh -c\" and \"$@\", so that readers who read it would not even\n> dream of writing such an alias in the first place.\n\nRight; I was seeing this in a more convoluted way via our tool but\nessentially the same issue. I was just looking for the simplest thing\nthat also gave the syntax error output, which I thought was something\npeople might search for (the \"unexpected \"$@\" stuff).\n\nShould I just leave as is?\n\n> > +test_expect_success 'tracing a shell alias with arguments shows full prepared command' '\n> > +\tgit config alias.echo \"!echo \\$*\" &&\n> > +\tenv GIT_TRACE=1 git echo argument 2>output &&\n> > +\tcp output /tmp/output &&\n> > +\ttest_grep \"^trace: prepare_cmd: /bin/sh -c '\\''echo \\$\\* \\\"\\$@\\\"\" output\n> > +'\n> \n> This is probably too specific search string, I suspect, given that\n> runcommand.c:prepare_shell_cmd() uses SHELL_PATH or \"sh\" so if your\n> SHELL_PATH is anything but /bin/sh (or if you are unlucky enough to\n> be running this test on Windows), the pattern would not match.\n> You'd want to loosen it a bit, perhaps with \"/bin/sh\" -> \".*\", as\n> the rest of the output are expected to stay constant.\n\nOK, should this perhaps just look for '^trace: prepare_cmd.*'?\n\nMy initial thinking was to enforce seeing the \"$@\" appended, but\nperhaps that is implementation details that don't really need to be\ncovered; the interesting thing is we show the person tracing the full\ncommand as constructed, so this is useful just to ensure the\ntracepoint remains in place?\n\n-i\n\n"},{"id":"495510","messageId":"xmqq4janpwqq.fsf@gitster.g","threadId":"61526","inReplyTo":"xmqqttiop05y.fsf@gitster.g","subject":"Re: [PATCH v3 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-24T06:09:01Z","receivedAt":"2024-05-24T06:09:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Subject: [PATCH] fixup! run-command: show prepared command\n>\n> Do not assume everybody sets SHELL_PATH to /bin/sh\n> ---\n> t/t0014-alias.sh | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\n> index 75c8763a6c..0d42d2b454 100755\n> --- a/t/t0014-alias.sh\n> +++ b/t/t0014-alias.sh\n> @@ -48,7 +48,8 @@ test_expect_success 'tracing a shell alias with arguments shows full prepared co\n> \tgit config alias.echo \"!echo \\$*\" &&\n> \tenv GIT_TRACE=1 git echo argument 2>output &&\n> \tcp output /tmp/output &&\n> -\ttest_grep \"^trace: prepare_cmd: /bin/sh -c '\\''echo \\$\\* \\\"\\$@\\\"\" output\n> +\t# \".*\" can be \"sh\", or SHELL_PATH (not always \"/bin/sh\")\n> +\ttest_grep \"^trace: prepare_cmd: .* -c '\\''echo \\$\\* \\\"\\$@\\\"\" output\n> '\n\nSo with this applied on top of the topic, 'seen' does pass with\nSHELL_PATH set to say /bin/dash, but this still fails CI jobs on\nWindows. \n\nA sample run that failed (you'd probably need to be logged in as a\ngithub user there):\n\n https://github.com/git/git/actions/runs/9216303673/job/25360383492#step:5:237\n\nWith the lack of \"trace: prepare_cmd:\" in the \"output\" file, on the\nplatform, perhaps the code is failing to find an executable for\n\"echo\" on the $PATH and taking the early return codepath that sets\nerrno to ENOENT and returns -1? Or it is that even prepare_cmd()\nis not called? I do not do Windows, so I'll stop here.\n\nOh by the way, I just noticed that there is an apparent leftover\ndebugging statement that accesses outside the test playpen. When\nyou reroll the patch, please make sure that \"cp output /tmp/output\"\nis removed.\n\nThanks.\n"},{"id":"495516","messageId":"ZlA_WIn1y7Oo_D88@fedora19.localdomain","threadId":"61526","inReplyTo":"xmqq4janpwqq.fsf@gitster.g","subject":"Re: [PATCH v3 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-24T07:18:48Z","receivedAt":"2024-05-24T07:18:57Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"On Thu, May 23, 2024 at 11:09:01PM -0700, Junio C Hamano wrote:\n> So with this applied on top of the topic, 'seen' does pass with\n> SHELL_PATH set to say /bin/dash, but this still fails CI jobs on\n> Windows. \n\nSigh, it looks like windows uses prepare_shell_cmd() instead. I think\nit's still probably valid to dump the full command there for the same\nreasons, which I'll add in v4.\n\nI've reduced to the check to just look for the trace prefix. This is\nenough to ensure we don't regress by removing it; the exact details of\nwhat comes out really isn't the main point -- it's that the log is\nthere at all to help you debug what is being run.\n\n> Oh by the way, I just noticed that there is an apparent leftover\n> debugging statement that accesses outside the test playpen. When\n> you reroll the patch, please make sure that \"cp output /tmp/output\"\n> is removed.\n\nDone\n\n-i\n\n"},{"id":"495518","messageId":"20240524073411.1355958-1-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240523043806.1223032-1-iwienand@redhat.com","subject":"[PATCH v4 1/3] Documentation: alias: rework notes into points","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-24T07:32:42Z","receivedAt":"2024-05-24T07:34:29Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"There are a number of caveats when using aliases. Rather than\nstuffing them all together in a paragraph, let's separate them out\ninto individual points to make it clearer what's going on.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 01df96fab3..40851ef429 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -21,8 +21,9 @@ If the alias expansion is prefixed with an exclamation point,\n it will be treated as a shell command. For example, defining\n `alias.new = !gitk --all --not ORIG_HEAD`, the invocation\n `git new` is equivalent to running the shell command\n-`gitk --all --not ORIG_HEAD`. Note that shell commands will be\n-executed from the top-level directory of a repository, which may\n-not necessarily be the current directory.\n-`GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n-from the original current directory. See linkgit:git-rev-parse[1].\n+`gitk --all --not ORIG_HEAD`. Note:\n++\n+* Shell commands will be executed from the top-level directory of a\n+ repository, which may not necessarily be the current directory.\n+* `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n+ from the original current directory. See linkgit:git-rev-parse[1].\n-- \n2.45.1\n\n"},{"id":"495519","messageId":"20240524073411.1355958-2-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240524073411.1355958-1-iwienand@redhat.com","subject":"[PATCH v4 2/3] Documentation: alias: add notes on shell expansion","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-24T07:32:43Z","receivedAt":"2024-05-24T07:34:32Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"When writing inline shell for shell-expansion aliases (i.e. prefixed\nwith \"!\"), there are some caveats around argument parsing to be aware\nof. This series of notes attempts to explain what is happening more\nclearly.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 40851ef429..51fa876c91 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -27,3 +27,24 @@ it will be treated as a shell command. For example, defining\n repository, which may not necessarily be the current directory.\n * `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n from the original current directory. See linkgit:git-rev-parse[1].\n+* If the shell alias is the full path to a binary, it will be executed\n+ directly with any arguments.\n+* If the alias contains any whitespace or reserved characters, it will\n+ be considered an inline script and run as an argument to `sh -c`.\n+* When running as a script, Git appends \"$@\" to the alias shell\n+ command when arguments are present. If there are no arguments,\n+ `\"$@\"` will not be appended.\n+** This may initially be confusing if your alias script references\n+ argument variables, or is otherwise not expecting the presence of\n+ `\"$@\"`. For example: `alias.echo = \"!echo $1\"` when run as `git\n+ echo arg` will actually execute `sh -c \"echo $1 $@\" \"echo $1\"\n+ \"arg\"` resulting in output `arg arg`. An alias `alias.for = \"!for\n+ i in 1 2 3; do echo $i; done\"` will fail if any arguments are\n+ specified to `git for` as the appended `\"$@\"` will create invalid\n+ shell syntax.\n+** A convenient way to deal with this is to write your script\n+ operations in an inline function that is then called with any\n+ arguments from the command-line. For example `alias.echo = \"!e() {\n+ echo $* ; }; e\" will work as expected, with the function `e`\n+ receiving any arugments from the command-line.\n+** Setting `GIT_TRACE=1` can help debug the command being run.\n-- \n2.45.1\n\n"},{"id":"495520","messageId":"20240524073411.1355958-3-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240524073411.1355958-1-iwienand@redhat.com","subject":"[PATCH v4 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-24T07:32:44Z","receivedAt":"2024-05-24T07:34:35Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"This adds a trace point in prepare_cmd (for windows,\nprepare_shell_cmd), so we can see the actual command being run without\nhaving to resort to strace/code inspection.\n\ne.g. \"test = !echo\" when run under GIT_TRACE will show:\n\n $ GIT_TRACE=1 git test hello\n 10:58:56.877234 git.c:755 trace: exec: git-test hello\n 10:58:56.877382 run-command.c:657 trace: run_command: git-test hello\n 10:58:56.878655 run-command.c:657 trace: run_command: echo hello\n 10:58:56.878747 run-command.c:437 trace: prepare_cmd: /usr/bin/echo hello\n hello\n\nA \"split\" alias, e.g. test = \"!echo $*\" will show the shell wrapping\nand appending of \"$@\".\n\n $ GIT_TRACE=1 git test hello\n 11:00:45.959420 git.c:755 trace: exec: git-test hello\n 11:00:45.959737 run-command.c:657 trace: run_command: git-test hello\n 11:00:45.961424 run-command.c:657 trace: run_command: 'echo $*' hello\n 11:00:45.961528 run-command.c:437 trace: prepare_cmd: /bin/sh -c 'echo $* \"$@\"' 'echo $*' hello\n hello hello\n\nPrior changes have made the documentation around the internals of the\nalias command execution clearer, but I have still found this detailed\nview of the aliased command being run helpful for debugging purposes.\n\nA test case is added. This simply looks for the trace point output;\nthe details change depending on platform -- the important thing is\nthat we have a verbose log point, not so much the details of what\nhappens on each platform.\n\nSigned-off-by: Ian Wienand \n---\n run-command.c | 2 ++\n t/t0014-alias.sh | 6 ++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 1b821042b4..13e35fb76e 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -296,6 +296,7 @@ static const char **prepare_shell_cmd(struct strvec *out, const char **argv)\n \t}\n \n \tstrvec_pushv(out, argv);\n+\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n \treturn out->v;\n }\n \n@@ -435,6 +436,7 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd)\n \t\t}\n \t}\n \n+\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n \treturn 0;\n }\n \ndiff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\nindex 95568342be..b7affbe93a 100755\n--- a/t/t0014-alias.sh\n+++ b/t/t0014-alias.sh\n@@ -44,4 +44,10 @@ test_expect_success 'run-command formats empty args properly' '\n test_cmp expect actual\n '\n \n+test_expect_success 'tracing a shell alias with arguments shows trace of prepared command' '\n+\tgit config alias.echo \"!echo \\$*\" &&\n+\tenv GIT_TRACE=1 git echo argument 2>output &&\n+\ttest_grep \"^trace: prepare_cmd:.*\" output\n+'\n+\n test_done\n-- \n2.45.1\n\n"},{"id":"495565","messageId":"xmqqsey7ns14.fsf@gitster.g","threadId":"61526","inReplyTo":"ZlA_WIn1y7Oo_D88@fedora19.localdomain","subject":"Re: [PATCH v3 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-24T15:33:43Z","receivedAt":"2024-05-24T15:33:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> On Thu, May 23, 2024 at 11:09:01PM -0700, Junio C Hamano wrote:\n>> So with this applied on top of the topic, 'seen' does pass with\n>> SHELL_PATH set to say /bin/dash, but this still fails CI jobs on\n>> Windows. \n>\n> Sigh, it looks like windows uses prepare_shell_cmd() instead. I think\n> it's still probably valid to dump the full command there for the same\n> reasons, which I'll add in v4.\n\nAhh, it's the large conditional compilation in start_command().\n\nOn non-Windows side, we just do\n\n\tif (prepare_cmd(&argv, cmd) < 0) {\n\t\tfailed_errno = errno;\n\t\t...\n\t\tgoto end_of_spawn;\n\t}\n\t... pipe(), fork(), and exec() ...\n\nbut on Windows side, we have\n\n\tif (cmd->git_cmd)\n\t\tcmd->args.v = prepare_git_cmd(&nargv, sargv);\n\telse if (cmd->use_shell)\n\t\tcmd->args.v = prepare_shell_cmd(&nargv, sargv);\n\n\tcmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v,\n\t\t\t\t (char**) cmd->env.v,\n\t\t\t\t cmd->dir, fhin, fhout, fherr);\n\nAnd the thing is, prepare_cmd() already has\n\n\tif (cmd->git_cmd) {\n\t\tprepare_git_cmd(out, cmd->args.v);\n\t} else if (cmd->use_shell) {\n\t\tprepare_shell_cmd(out, cmd->args.v);\n\t} else {\n\t\tstrvec_pushv(out, cmd->args.v);\n\t}\n\nI am wondering (and not suggesting to do this as a part of this\nseries, but to decide if we should leave a left-over-bits note here)\nif the Windows side can be updated to also use prepare_cmd(). Do\nfolks a lot more familiar with Windows than I (cc'ed j6t and dscho)\nhave comments on this?\n\nAnyway.\n\nIf you unconditionally add the printf's to both prepare_cmd() and\nprepare_shell_cmd(), you'll see two printf output on a non-Windows\nplatform if we are spawning a shell cmd.\n\nIt appears that the best place to \"show the prepared command\" is\n*NOT* in any of these prepare_* helper functions, but after these\ntwo callers in start_command() receives the prepared command from\nthese helpers, namely:\n\n diff --git c/run-command.c w/run-command.c\n index 1b821042b4..9d0fa6afe4 100644\n --- c/run-command.c\n +++ w/run-command.c\n @@ -745,6 +745,7 @@ int start_command(struct child_process *cmd)\n error_errno(\"cannot run %s\", cmd->args.v[0]);\n goto end_of_spawn;\n }\n +\t/* Here show argv in the trace */\n\n if (pipe(notify_pipe))\n notify_pipe[0] = notify_pipe[1] = -1;\n @@ -912,6 +913,7 @@ int start_command(struct child_process *cmd)\n cmd->args.v = prepare_git_cmd(&nargv, sargv);\n else if (cmd->use_shell)\n cmd->args.v = prepare_shell_cmd(&nargv, sargv);\n +\t/* Here show cmd->args.v in the trace */\n\n cmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v,\n (char**) cmd->env.v,\n\nWe would not have to do so and instead have trace-printf(s) only in\nprepare_cmd() if/when the Windows side is updated to stop switching\nbetween the prepare_git/prepare_shell and instead let prepare_cmd()\ndo the switching, (which may require an update to prepare_cmd() if\nneeded). But until that happens, logging at the caller seems to be\nthe best approach among equally bad ones.\n\nThanks.\n"},{"id":"495581","messageId":"xmqqo78vksk6.fsf@gitster.g","threadId":"61526","inReplyTo":"Zk_ip35jHUj_5M94@fedora19.localdomain","subject":"Re: [PATCH v3 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-24T17:50:33Z","receivedAt":"2024-05-24T17:50:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> On Thu, May 23, 2024 at 08:29:21AM -0700, Junio C Hamano wrote:\n>> ... if the reader truly understands \"the alias gives the command and\n>> its leading arguments, to which the invocation can supply even more\n>> arguments\", the reader wouldn't be writing such a command line to\n>> begin with, no?\n>> \n>> So I find the example a bit suboptimal. Hopefully additional\n>> explanation in patch 2/3 stressed on that point well enough with\n>> much more stress than it talks about the implementation detail of\n>> using \"sh -c\" and \"$@\", so that readers who read it would not even\n>> dream of writing such an alias in the first place.\n>\n> Right; I was seeing this in a more convoluted way via our tool but\n> essentially the same issue. I was just looking for the simplest thing\n> that also gave the syntax error output, which I thought was something\n> people might search for (the \"unexpected \"$@\" stuff).\n>\n> Should I just leave as is?\n\nIf I found as-is would be good enough, I wouldn't have been\ncommenting on this.\n\nEven in this third iteration, I still didn't see the added\ndocumentation talk about the principle behind the design, i.e. what\nyou write after the \"git your-alias\" are appended to the command\nline to be used as additional arguments.\n"},{"id":"495586","messageId":"xmqqzfsfja0d.fsf@gitster.g","threadId":"61526","inReplyTo":"20240524073411.1355958-3-iwienand@redhat.com","subject":"Re: [PATCH v4 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-24T19:16:34Z","receivedAt":"2024-05-24T19:16:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> +test_expect_success 'tracing a shell alias with arguments shows trace of prepared command' '\n> +\tgit config alias.echo \"!echo \\$*\" &&\n> +\tenv GIT_TRACE=1 git echo argument 2>output &&\n> +\ttest_grep \"^trace: prepare_cmd:.*\" output\n> +'\n\nIf you run\n\n $ cd t && sh ./t0014-alias.sh -d && cat trash*.t0014-alias/output\n\nyou'll see two \"prepare_cmd\" logged, because you added one to\nprepare_shell_cmd() and another to prepare_cmd(). If you were\ntracing something that uses prepare_git_cmd() on Windows, you would\nnot see any trace output, on the other hand, I would suspect (I do\nnot do Windows so this is from code inspection only).\n\nPerhaps squashing something like this is sufficient?\n\n run-command.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git c/run-command.c w/run-command.c\nindex 13e35fb76e..7eb68a541d 100644\n--- c/run-command.c\n+++ w/run-command.c\n@@ -296,7 +296,6 @@ static const char **prepare_shell_cmd(struct strvec *out, const char **argv)\n \t}\n \n \tstrvec_pushv(out, argv);\n-\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n \treturn out->v;\n }\n \n@@ -436,7 +435,6 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd)\n \t\t}\n \t}\n \n-\ttrace_argv_printf(&out->v[1], \"trace: prepare_cmd:\");\n \treturn 0;\n }\n \n@@ -747,6 +745,7 @@ int start_command(struct child_process *cmd)\n \t\t\terror_errno(\"cannot run %s\", cmd->args.v[0]);\n \t\tgoto end_of_spawn;\n \t}\n+\ttrace_argv_printf(&argv.v[1], \"trace: prepare_cmd:\");\n \n \tif (pipe(notify_pipe))\n \t\tnotify_pipe[0] = notify_pipe[1] = -1;\n@@ -914,6 +913,7 @@ int start_command(struct child_process *cmd)\n \t\tcmd->args.v = prepare_git_cmd(&nargv, sargv);\n \telse if (cmd->use_shell)\n \t\tcmd->args.v = prepare_shell_cmd(&nargv, sargv);\n+\ttrace_argv_printf(&cmd->args.v[1], \"trace: prepare_cmd:\");\n \n \tcmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v,\n \t\t\t\t (char**) cmd->env.v,\n\n\n\n"},{"id":"495601","messageId":"xmqqttinj82d.fsf@gitster.g","threadId":"61526","inReplyTo":"xmqqzfsfja0d.fsf@gitster.g","subject":"Re: [PATCH v4 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-24T19:58:34Z","receivedAt":"2024-05-24T19:58:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> +\ttest_grep \"^trace: prepare_cmd:.*\" output\n>> +'\n>\n> If you run\n>\n> $ cd t && sh ./t0014-alias.sh -d && cat trash*.t0014-alias/output\n>\n> you'll see two \"prepare_cmd\" logged, because you added one to\n> prepare_shell_cmd() and another to prepare_cmd().\n\nTo avoid similar issues, perhaps the test can try to see a bit more\ninto the output? We already know that we cannot expect the output\nto be identical everywhere, but we can do something along the lines\nof the attached patch, or is the expected pattern below a bit too\nspecific?\n\n t/t0014-alias.sh | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git c/t/t0014-alias.sh w/t/t0014-alias.sh\nindex b7affbe93a..a3c133be96 100755\n--- c/t/t0014-alias.sh\n+++ w/t/t0014-alias.sh\n@@ -45,9 +45,17 @@ test_expect_success 'run-command formats empty args properly' '\n '\n \n test_expect_success 'tracing a shell alias with arguments shows trace of prepared command' '\n+\tcat >expect <<-EOF &&\n+\ttrace: exec: git-echo argument\n+\ttrace: run_command: git-echo argument\n+\ttrace: run_command: ${SQ}echo \\$*${SQ} argument\n+\ttrace: prepare_cmd: SHELL -c ${SQ}echo \\$* \"\\$@\"${SQ} ${SQ}echo \\$*${SQ} argument\n+\tEOF\n \tgit config alias.echo \"!echo \\$*\" &&\n \tenv GIT_TRACE=1 git echo argument 2>output &&\n-\ttest_grep \"^trace: prepare_cmd:.*\" output\n+\t# redact platform differences\n+\tsed -e \"s/^\\(trace: prepare_cmd:\\) .* -c /\\1 SHELL -c /\" output >actual &&\n+\ttest_cmp expect actual\n '\n \n test_done\n\n"},{"id":"495611","messageId":"ZlE7LBFkcio2KFoE@fedora19.localdomain","threadId":"61526","inReplyTo":"xmqqo78vksk6.fsf@gitster.g","subject":"Re: [PATCH v3 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T01:13:16Z","receivedAt":"2024-05-25T01:13:24Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"On Fri, May 24, 2024 at 10:50:33AM -0700, Junio C Hamano wrote:\n> > Should I just leave as is?\n> \n> If I found as-is would be good enough, I wouldn't have been\n> commenting on this.\n\nI think this \"for ... done\" example from me is perhaps too specific\nand weird, and not being helpful. I'll trim all this out as I think\nthe point can be made clearer.\n\n> Even in this third iteration, I still didn't see the added\n> documentation talk about the principle behind the design, i.e. what\n> you write after the \"git your-alias\" are appended to the command\n> line to be used as additional arguments.\n\nI'll send an update that makes it clearer in a separate point that for\nsimple arguments, the added \"$@\" just passes through arguments as\nadditional positional arguments.\n\n-i\n\n"},{"id":"495612","messageId":"ZlE7eZ_-f4SaOaPf@fedora19.localdomain","threadId":"61526","inReplyTo":"xmqqzfsfja0d.fsf@gitster.g","subject":"Re: [PATCH v4 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T01:14:33Z","receivedAt":"2024-05-25T01:14:41Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"On Fri, May 24, 2024 at 12:16:34PM -0700, Junio C Hamano wrote:\n> Ian Wienand writes:\n> \n> > +test_expect_success 'tracing a shell alias with arguments shows trace of prepared command' '\n> > +\tgit config alias.echo \"!echo \\$*\" &&\n> > +\tenv GIT_TRACE=1 git echo argument 2>output &&\n> > +\ttest_grep \"^trace: prepare_cmd:.*\" output\n> > +'\n> \n> If you run\n> \n> $ cd t && sh ./t0014-alias.sh -d && cat trash*.t0014-alias/output\n> \n> you'll see two \"prepare_cmd\" logged, because you added one to\n> prepare_shell_cmd() and another to prepare_cmd().\n\nSorry that's a bit embarrasing that I missed that. Reading this more\ncarefully, I agree that the points you've identified in start_command\nare probably the best place. I've updated to this.\n\n-i\n\n"},{"id":"495614","messageId":"20240525012207.1415196-1-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240524073411.1355958-1-iwienand@redhat.com","subject":"[PATCH v5 1/3] Documentation: alias: rework notes into points","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T01:20:50Z","receivedAt":"2024-05-25T01:22:18Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"There are a number of caveats when using aliases. Rather than\nstuffing them all together in a paragraph, let's separate them out\ninto individual points to make it clearer what's going on.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 01df96fab3..40851ef429 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -21,8 +21,9 @@ If the alias expansion is prefixed with an exclamation point,\n it will be treated as a shell command. For example, defining\n `alias.new = !gitk --all --not ORIG_HEAD`, the invocation\n `git new` is equivalent to running the shell command\n-`gitk --all --not ORIG_HEAD`. Note that shell commands will be\n-executed from the top-level directory of a repository, which may\n-not necessarily be the current directory.\n-`GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n-from the original current directory. See linkgit:git-rev-parse[1].\n+`gitk --all --not ORIG_HEAD`. Note:\n++\n+* Shell commands will be executed from the top-level directory of a\n+ repository, which may not necessarily be the current directory.\n+* `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n+ from the original current directory. See linkgit:git-rev-parse[1].\n-- \n2.45.1\n\n"},{"id":"495615","messageId":"20240525012207.1415196-2-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240525012207.1415196-1-iwienand@redhat.com","subject":"[PATCH v5 2/3] Documentation: alias: add notes on shell expansion","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T01:20:51Z","receivedAt":"2024-05-25T01:22:23Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"When writing inline shell for shell-expansion aliases (i.e. prefixed\nwith \"!\"), there are some caveats around argument parsing to be aware\nof. This series of notes attempts to explain what is happening more\nclearly.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 28 ++++++++++++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 40851ef429..f32b86cde3 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -27,3 +27,31 @@ it will be treated as a shell command. For example, defining\n repository, which may not necessarily be the current directory.\n * `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n from the original current directory. See linkgit:git-rev-parse[1].\n+* If the shell alias is the full path to a binary, it will be executed\n+ directly with any arguments as positional arguments.\n+* If the alias contains any white-space or reserved characters, it\n+ will be considered an inline script and run as an argument to `sh\n+ -c`.\n+* When running as a script, if arguments are provided to the alias\n+ call, Git makes them available to the process by appending \"$@\" to\n+ the alias shell command. This is not appended if arguments are not\n+ provided.\n+** For \"simple\" commands, such as calling a single binary\n+ (e.g. `alias.myapp = !myapp --myflag1`) this will result in any\n+ arguments becoming additional regular positional arguments to the\n+ called binary, appended after any arguments specified in the aliased\n+ command.\n+** Care should be taken if your alias script has multiple commands\n+ (e.g. in a pipeline), references argument variables, or is\n+ otherwise not expecting the presence of the appended `\"$@\"`. For\n+ example: `alias.echo = \"!echo $1\"` when run as `git echo arg` will\n+ actually execute `sh -c \"echo $1 $@\" \"echo $1\" \"arg\"` resulting in\n+ output `arg arg`. When writing such aliases, you should ensure\n+ that the appended \"$@\" when arguments are present does not cause\n+ syntax errors or unintended side-effects.\n+** A convenient way to deal with this is to write your script\n+ operations in an inline function that is then called with any\n+ arguments from the command-line. For example `alias.cmd = \"!c() {\n+ cmd $1 | cmd $2 ; }; c\" will allow you to work with separate\n+ arguments.\n+** Setting `GIT_TRACE=1` can help debug the command being run.\n-- \n2.45.1\n\n"},{"id":"495616","messageId":"20240525012207.1415196-3-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240525012207.1415196-1-iwienand@redhat.com","subject":"[PATCH v5 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T01:20:52Z","receivedAt":"2024-05-25T01:22:24Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"This adds a trace point in start_command so we can see the full\ncommand invocation without having to resort to strace/code inspection.\nFor example:\n\n $ GIT_TRACE=1 git test foo\n git.c:755 trace: exec: git-test foo\n run-command.c:657 trace: run_command: git-test foo\n run-command.c:657 trace: run_command: 'echo $*' foo\n run-command.c:749 trace: start_command: /bin/sh -c 'echo $* \"$@\"' 'echo $*' foo\n\nPrior changes have made the documentation around the internals of the\nalias command execution clearer, but I have still found this detailed\nview of the aliased command being run helpful for debugging purposes.\n\nA test case is added. This simply looks for the trace point output;\nthe details change depending on platform -- the important thing is\nthat we have a verbose log point, not so much the details of what\nhappens on each platform.\n\nSigned-off-by: Ian Wienand \n---\n run-command.c | 3 +++\n t/t0014-alias.sh | 6 ++++++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 1b821042b4..9892c4421c 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -746,6 +746,8 @@ int start_command(struct child_process *cmd)\n \t\tgoto end_of_spawn;\n \t}\n \n+\ttrace_argv_printf(&argv.v[1], \"trace: start_command:\");\n+\n \tif (pipe(notify_pipe))\n \t\tnotify_pipe[0] = notify_pipe[1] = -1;\n \n@@ -913,6 +915,7 @@ int start_command(struct child_process *cmd)\n \telse if (cmd->use_shell)\n \t\tcmd->args.v = prepare_shell_cmd(&nargv, sargv);\n \n+\ttrace_argv_printf(&cmd->args.v[1], \"trace: start_command:\");\n \tcmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v,\n \t\t\t\t (char**) cmd->env.v,\n \t\t\t\t cmd->dir, fhin, fhout, fherr);\ndiff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\nindex 95568342be..db747950c7 100755\n--- a/t/t0014-alias.sh\n+++ b/t/t0014-alias.sh\n@@ -44,4 +44,10 @@ test_expect_success 'run-command formats empty args properly' '\n test_cmp expect actual\n '\n \n+test_expect_success 'tracing a shell alias with arguments shows trace of prepared command' '\n+\tgit config alias.echo \"!echo \\$*\" &&\n+\tenv GIT_TRACE=1 git echo argument 2>output &&\n+\ttest_grep \"^trace: start_command:.*\" output\n+'\n+\n test_done\n-- \n2.45.1\n\n"},{"id":"495628","messageId":"xmqqr0dqigx7.fsf@gitster.g","threadId":"61526","inReplyTo":"20240525012207.1415196-3-iwienand@redhat.com","subject":"Re: [PATCH v5 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-25T05:44:52Z","receivedAt":"2024-05-25T05:45:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> diff --git a/run-command.c b/run-command.c\n> index 1b821042b4..9892c4421c 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -746,6 +746,8 @@ int start_command(struct child_process *cmd)\n> \t\tgoto end_of_spawn;\n> \t}\n> \n> +\ttrace_argv_printf(&argv.v[1], \"trace: start_command:\");\n> +\n> \tif (pipe(notify_pipe))\n> \t\tnotify_pipe[0] = notify_pipe[1] = -1;\n\nThis side is OK ...\n\n> @@ -913,6 +915,7 @@ int start_command(struct child_process *cmd)\n> \telse if (cmd->use_shell)\n> \t\tcmd->args.v = prepare_shell_cmd(&nargv, sargv);\n> \n> +\ttrace_argv_printf(&cmd->args.v[1], \"trace: start_command:\");\n> \tcmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v,\n> \t\t\t\t (char**) cmd->env.v,\n> \t\t\t\t cmd->dir, fhin, fhout, fherr);\n\n... but this side should pass \"cmd->args.v\" (i.e., the entire array,\nwithout omitting the zeroth element) to be consistent with the other\nside. I made the same mistake in my \"how about doing it this way\"\ndraft, by the way.\n\nIt is because prepare_cmd() does this:\n\n static int prepare_cmd(struct strvec *out, const struct child_process *cmd)\n {\n ...\n /*\n * Add SHELL_PATH so in the event exec fails with ENOEXEC we can\n * attempt to interpret the command with 'sh'.\n */\n strvec_push(out, SHELL_PATH);\n\n if (cmd->git_cmd) {\n prepare_git_cmd(out, cmd->args.v);\n } else if (cmd->use_shell) {\n prepare_shell_cmd(out, cmd->args.v);\n ...\n\nSo, the other side (i.e. non Windows, that used the result from\nprepare_cmd()) skips the argv.v[0] element (which is the SHELL_PATH\npushed by prepare_cmd()), but because a bare use of\nprepare_shell_cmd() done on the Windows side does not have that\nexcess element at the beginning.\n"},{"id":"495629","messageId":"xmqqed9qifxl.fsf@gitster.g","threadId":"61526","inReplyTo":"20240525012207.1415196-3-iwienand@redhat.com","subject":"Re: [PATCH v5 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-25T06:06:14Z","receivedAt":"2024-05-25T06:06:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> +test_expect_success 'tracing a shell alias with arguments shows trace of prepared command' '\n> +\tgit config alias.echo \"!echo \\$*\" &&\n> +\tenv GIT_TRACE=1 git echo argument 2>output &&\n> +\ttest_grep \"^trace: start_command:.*\" output\n> +'\n\nThis will succeed even if you have two or more instances of this\nlog, which will not help you avoid the mistake we saw in an earlier\niteration.\n\nHow about making sure that the prepared command looks reasonable\nenough and we have only one, by doing something like this?\n\ntest_expect_success 'tracing a shell alias with arg shows trace of prepared command' '\n\tcat >expect <<-EOF &&\n\ttrace: prepare_cmd: SHELL -c ${SQ}echo \\$* \"\\$@\"${SQ} ${SQ}echo \\$*${SQ} arg\n\tEOF\n\tgit config alias.echo \"!echo \\$*\" &&\n\tenv GIT_TRACE=1 git echo arg 2>output &&\n\t# redact platform differences\n\tsed -n -e \"s/^\\(trace: prepare_cmd:\\) .* -c /\\1 SHELL -c /p\" output >actual &&\n\ttest_cmp expect actual\n'\n\nA sample run of CI job with the above change (and others I sent on\nthe list) can be seen at:\n\n https://github.com/git/git/actions/runs/9233329671\n\nYou can fetch 'seen' and can find commit 05ebf54569, which is the\ntip of your topic including a few \"fixup\" commits I made on top.\n\n $ git log --decorate --oneline master..05ebf54569\n 05ebf54569 (iw/trace-argv-on-alias) fixup! run-command: show prepared command\n 11056975bb fixup! run-command: show prepared command\n 770dda8f2b run-command: show prepared command\n 242b9d4d63 Documentation: alias: add notes on shell expansion\n e4331ad0d4 Documentation: alias: rework notes into points\n\nIt hasn't finished running yet, but all the windows tests have\npassed, so I'll happily go to bed now ;-)\n\nThanks.\n"},{"id":"495642","messageId":"20240525234454.1489598-1-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240525012207.1415196-1-iwienand@redhat.com","subject":"[PATCH v6 1/3] Documentation: alias: rework notes into points","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T23:44:33Z","receivedAt":"2024-05-25T23:45:08Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"There are a number of caveats when using aliases. Rather than\nstuffing them all together in a paragraph, let's separate them out\ninto individual points to make it clearer what's going on.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 01df96fab3..40851ef429 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -21,8 +21,9 @@ If the alias expansion is prefixed with an exclamation point,\n it will be treated as a shell command. For example, defining\n `alias.new = !gitk --all --not ORIG_HEAD`, the invocation\n `git new` is equivalent to running the shell command\n-`gitk --all --not ORIG_HEAD`. Note that shell commands will be\n-executed from the top-level directory of a repository, which may\n-not necessarily be the current directory.\n-`GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n-from the original current directory. See linkgit:git-rev-parse[1].\n+`gitk --all --not ORIG_HEAD`. Note:\n++\n+* Shell commands will be executed from the top-level directory of a\n+ repository, which may not necessarily be the current directory.\n+* `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n+ from the original current directory. See linkgit:git-rev-parse[1].\n-- \n2.45.1\n\n"},{"id":"495643","messageId":"20240525234454.1489598-2-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240525234454.1489598-1-iwienand@redhat.com","subject":"[PATCH v6 2/3] Documentation: alias: add notes on shell expansion","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T23:44:34Z","receivedAt":"2024-05-25T23:45:11Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"When writing inline shell for shell-expansion aliases (i.e. prefixed\nwith \"!\"), there are some caveats around argument parsing to be aware\nof. This series of notes attempts to explain what is happening more\nclearly.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 28 ++++++++++++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 40851ef429..f32b86cde3 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -27,3 +27,31 @@ it will be treated as a shell command. For example, defining\n repository, which may not necessarily be the current directory.\n * `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n from the original current directory. See linkgit:git-rev-parse[1].\n+* If the shell alias is the full path to a binary, it will be executed\n+ directly with any arguments as positional arguments.\n+* If the alias contains any white-space or reserved characters, it\n+ will be considered an inline script and run as an argument to `sh\n+ -c`.\n+* When running as a script, if arguments are provided to the alias\n+ call, Git makes them available to the process by appending \"$@\" to\n+ the alias shell command. This is not appended if arguments are not\n+ provided.\n+** For \"simple\" commands, such as calling a single binary\n+ (e.g. `alias.myapp = !myapp --myflag1`) this will result in any\n+ arguments becoming additional regular positional arguments to the\n+ called binary, appended after any arguments specified in the aliased\n+ command.\n+** Care should be taken if your alias script has multiple commands\n+ (e.g. in a pipeline), references argument variables, or is\n+ otherwise not expecting the presence of the appended `\"$@\"`. For\n+ example: `alias.echo = \"!echo $1\"` when run as `git echo arg` will\n+ actually execute `sh -c \"echo $1 $@\" \"echo $1\" \"arg\"` resulting in\n+ output `arg arg`. When writing such aliases, you should ensure\n+ that the appended \"$@\" when arguments are present does not cause\n+ syntax errors or unintended side-effects.\n+** A convenient way to deal with this is to write your script\n+ operations in an inline function that is then called with any\n+ arguments from the command-line. For example `alias.cmd = \"!c() {\n+ cmd $1 | cmd $2 ; }; c\" will allow you to work with separate\n+ arguments.\n+** Setting `GIT_TRACE=1` can help debug the command being run.\n-- \n2.45.1\n\n"},{"id":"495644","messageId":"20240525234454.1489598-3-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240525234454.1489598-1-iwienand@redhat.com","subject":"[PATCH v6 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T23:44:35Z","receivedAt":"2024-05-25T23:45:14Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"This adds a trace point in start_command so we can see the full\ncommand invocation without having to resort to strace/code inspection.\nFor example:\n\n $ GIT_TRACE=1 git test foo\n git.c:755 trace: exec: git-test foo\n run-command.c:657 trace: run_command: git-test foo\n run-command.c:657 trace: run_command: 'echo $*' foo\n run-command.c:749 trace: start_command: /bin/sh -c 'echo $* \"$@\"' 'echo $*' foo\n\nPrior changes have made the documentation around the internals of the\nalias command execution clearer, but I have still found this detailed\nview of the aliased command being run helpful for debugging purposes.\n\nA test case is added. This simply looks for the trace point output;\nthe details change depending on platform -- the important thing is\nthat we have a verbose log point, not so much the details of what\nhappens on each platform.\n\nSigned-off-by: Ian Wienand \n---\n run-command.c | 3 +++\n t/t0014-alias.sh | 12 ++++++++++++\n 2 files changed, 15 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 1b821042b4..31b20123d8 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -746,6 +746,8 @@ int start_command(struct child_process *cmd)\n \t\tgoto end_of_spawn;\n \t}\n \n+\ttrace_argv_printf(&argv.v[1], \"trace: start_command:\");\n+\n \tif (pipe(notify_pipe))\n \t\tnotify_pipe[0] = notify_pipe[1] = -1;\n \n@@ -913,6 +915,7 @@ int start_command(struct child_process *cmd)\n \telse if (cmd->use_shell)\n \t\tcmd->args.v = prepare_shell_cmd(&nargv, sargv);\n \n+\ttrace_argv_printf(cmd->args.v, \"trace: start_command:\");\n \tcmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v,\n \t\t\t\t (char**) cmd->env.v,\n \t\t\t\t cmd->dir, fhin, fhout, fherr);\ndiff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\nindex 95568342be..634f6d78ef 100755\n--- a/t/t0014-alias.sh\n+++ b/t/t0014-alias.sh\n@@ -44,4 +44,16 @@ test_expect_success 'run-command formats empty args properly' '\n test_cmp expect actual\n '\n \n+test_expect_success 'tracing a shell alias with arguments shows trace of prepared command' '\n+\tcat >expect <<-EOF &&\n+\ttrace: start_command: SHELL -c ${SQ}echo \\$* \"\\$@\"${SQ} ${SQ}echo \\$*${SQ} arg\n+\tEOF\n+\tgit config alias.echo \"!echo \\$*\" &&\n+\tenv GIT_TRACE=1 git echo arg 2>output &&\n+\t# redact platform differences\n+\tcat output &&\n+\tsed -n -e \"s/^\\(trace: start_command:\\) .* -c /\\1 SHELL -c /p\" output >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.45.1\n\n"},{"id":"495645","messageId":"ZlJ5G5CT5ZPoGAiY@fedora19.localdomain","threadId":"61526","inReplyTo":"xmqqed9qifxl.fsf@gitster.g","subject":"Re: [PATCH v5 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-25T23:49:47Z","receivedAt":"2024-05-25T23:49:55Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"On Fri, May 24, 2024 at 11:06:14PM -0700, Junio C Hamano wrote:\n> A sample run of CI job with the above change (and others I sent on\n> the list) can be seen at:\n> \n> https://github.com/git/git/actions/runs/9233329671\n\nThanks for this; I had in my head that the pipeline wouldn't run in a\npersonal fork, but after I enabled it, it does ... I don't know who's\nultimately paying for it but I'll take it :)\n\n https://github.com/ianw/git/actions/runs/9238552513\n\nis for the v6 version; I'll make sure to run it through that from now\non!\n\n-i\n\n"},{"id":"495653","messageId":"xmqqh6ekh7dt.fsf@gitster.g","threadId":"61526","inReplyTo":"20240525234454.1489598-3-iwienand@redhat.com","subject":"Re: [PATCH v6 3/3] run-command: show prepared command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-26T16:20:46Z","receivedAt":"2024-05-26T16:20:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> This adds a trace point in start_command so we can see the full\n> command invocation without having to resort to strace/code inspection.\n> For example:\n>\n> $ GIT_TRACE=1 git test foo\n> git.c:755 trace: exec: git-test foo\n> run-command.c:657 trace: run_command: git-test foo\n> run-command.c:657 trace: run_command: 'echo $*' foo\n> run-command.c:749 trace: start_command: /bin/sh -c 'echo $* \"$@\"' 'echo $*' foo\n>\n> Prior changes have made the documentation around the internals of the\n> alias command execution clearer, but I have still found this detailed\n> view of the aliased command being run helpful for debugging purposes.\n>\n> A test case is added. This simply looks for the trace point output;\n> the details change depending on platform -- the important thing is\n> that we have a verbose log point, not so much the details of what\n> happens on each platform.\n>\n> Signed-off-by: Ian Wienand \n> ---\n> run-command.c | 3 +++\n> t/t0014-alias.sh | 12 ++++++++++++\n> 2 files changed, 15 insertions(+)\n>\n> diff --git a/run-command.c b/run-command.c\n> index 1b821042b4..31b20123d8 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -746,6 +746,8 @@ int start_command(struct child_process *cmd)\n> \t\tgoto end_of_spawn;\n> \t}\n> \n> +\ttrace_argv_printf(&argv.v[1], \"trace: start_command:\");\n> +\n> \tif (pipe(notify_pipe))\n> \t\tnotify_pipe[0] = notify_pipe[1] = -1;\n> \n> @@ -913,6 +915,7 @@ int start_command(struct child_process *cmd)\n> \telse if (cmd->use_shell)\n> \t\tcmd->args.v = prepare_shell_cmd(&nargv, sargv);\n> \n> +\ttrace_argv_printf(cmd->args.v, \"trace: start_command:\");\n> \tcmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v,\n> \t\t\t\t (char**) cmd->env.v,\n> \t\t\t\t cmd->dir, fhin, fhout, fherr);\n> diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\n> index 95568342be..634f6d78ef 100755\n> --- a/t/t0014-alias.sh\n> +++ b/t/t0014-alias.sh\n> @@ -44,4 +44,16 @@ test_expect_success 'run-command formats empty args properly' '\n> test_cmp expect actual\n> '\n> \n> +test_expect_success 'tracing a shell alias with arguments shows trace of prepared command' '\n> +\tcat >expect <<-EOF &&\n> +\ttrace: start_command: SHELL -c ${SQ}echo \\$* \"\\$@\"${SQ} ${SQ}echo \\$*${SQ} arg\n> +\tEOF\n> +\tgit config alias.echo \"!echo \\$*\" &&\n> +\tenv GIT_TRACE=1 git echo arg 2>output &&\n> +\t# redact platform differences\n> +\tcat output &&\n\nThat \"cat\" is not about \"redact\"ing platform differences. Leftover\ndebugging? I'll remove it while queuing.\n\n> +\tsed -n -e \"s/^\\(trace: start_command:\\) .* -c /\\1 SHELL -c /p\" output >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nOther than that, this step looks good to me.\n\nThanks.\n"},{"id":"495657","messageId":"xmqqa5kcduis.fsf@gitster.g","threadId":"61526","inReplyTo":"20240525234454.1489598-2-iwienand@redhat.com","subject":"Re: [PATCH v6 2/3] Documentation: alias: add notes on shell expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-26T23:26:51Z","receivedAt":"2024-05-26T23:26:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> +* If the shell alias is the full path to a binary, it will be executed\n> + directly with any arguments as positional arguments.\n> +* If the alias contains any white-space or reserved characters, it\n> + will be considered an inline script and run as an argument to `sh\n> + -c`.\n> +* When running as a script, if arguments are provided to the alias\n> + call, Git makes them available to the process by appending \"$@\" to\n> + the alias shell command. This is not appended if arguments are not\n> + provided.\n\nThese are not technically wrong per-se.\n\n> +** For \"simple\" commands, such as calling a single binary\n> + (e.g. `alias.myapp = !myapp --myflag1`) this will result in any\n> + arguments becoming additional regular positional arguments to the\n> + called binary, appended after any arguments specified in the aliased\n> + command.\n\nBut the single-command script still receives the arguments in\nargv[], so what the alias command has to do is the same. The\nearlier ones that are \"not technically wrong\" are merely\nimplemenation detail.\n\nIn a single command case, e.g., \"[alias] single = !one\", you may\nwrite\n\n #!/bin/sh\n echo \"$1\" | grep \"$2\"\n\nin 'one' script, and \"git single 1 2\" will be turned into\n\n start_command: one 1 2\n\ni.e. one receives two arguments in argv[]. It is an implementation\ndetail that we can bypass \"sh\" or \"-c\" or \"$@\"\n\nIf you write exactly the same thing like\n\n $ git -c 'alias=single=!one ' single 1 2\n\nyou'll instead see\n\n start_command: /bin/sh -c 'one' \"$@\" \"one \" 1 2\n\nbecause the trailing SP in the alias disables the optimization to\nbypass a more generic 'sh -c ... \"$@\"' construction. What gets run\nis an equivalent of the reader saying\n\n $ /bin/sh -c 'one \"$@\"' \"one \" 1 2\n\nbypassing git from the command line.\n\nWhat the script (one) has to write does not change at all either\ncase.\n\nAs I keep saying over multiple iterations, the above three bullet\npoints stress too much on the minute implementation detail while\nfailing to tell readers that the end-user alias receives the rest of\nthe command line as arguments.\n\n> +** Care should be taken if your alias script has multiple commands\n> + (e.g. in a pipeline), references argument variables, or is\n\n\"argument variables\" -> \"arguments\".\n\n> + otherwise not expecting the presence of the appended `\"$@\"`. \n\n\"otherwise not expecting\" is SIMPLY BUGGY but the readers may not\nunderstand it unless you tell them that the arguments are fed to\ntheir aliased command by appending them.\n\nWhen you look at the implementation detail of \"sh -c '... $@' -\n$ARGS\" as something to fight against, readers would lose sight to\nsee the crux of the problem they are trying to solve. I think it is\na wrong way to frame the issue. The problem readers are solving when\ncoming up with their own alias is this:\n\n How would one write a single-liner that can take arguments\n appended at the end?\n\nI think giving that to the readers upfront, i.e. \"when you write an\nalias, you are forming a single-liner that takes arguments appended\nat the end\", would go a long way without having them lose sight in\nthe implementation details of \"sometimes args directly come in\nargv[], sometimes your alias is wrapped in \"sh -c\" and \"$@\" is used.\nThey do the same thing to feed the arguments to your script.\n\nGoing back that 'one' example, if 'echo \"$1\" | grep \"$2\"' was what\nyou wanted to run, how would you write a single-liner that does\n\n echo \"$1\" | grep \"$2\"\n\nand can take its arguments at the end? You do *not* want to see\nyour invocation of the alias\n\n $ git single 1 2\n\nturn into \n\n $ echo \"$1\" | grep \"$2\" 1 2\n\nof course, and remember, \"$@\" is merely an implementation detail\nthat the end-users do not need to see.\n\nOf course the simplest one-liner, if you had the \"one\" script\nalready stored in the file, is to say\n\n $ one 1 2\n\ni.e. \"[alias] single = !one\". But calling that a \"single-liner\" is\ncheating.\n\nYou can do one of two easy things.\n\n $ sh -c 'echo \"$1\" | grep \"$2\"' - 1 2\n $ e(){ echo \"$1\" | grep \"$2\"; };e 1 2\n\nThe earlier string (before \"1 2\" is appended) of either of these\ngives you \"a single-liner that takes arguments at the end\" that does\nthe \"echo the first one, pipe it to grep that looks for the second\none\", which you would make the body of the alias. If the reader\nunderstands the earlier example that stores it in a file, the former\nis more mechanical and straight-forward rewrite of it. The latter\nmay be a bit more convoluted, but says the same thing in the same\nnumber of letters.\n\nHTH.\n"},{"id":"495658","messageId":"ZlPSKfzHejm-Z5Pn@fedora19.localdomain","threadId":"61526","inReplyTo":"xmqqa5kcduis.fsf@gitster.g","subject":"Re: [PATCH v6 2/3] Documentation: alias: add notes on shell expansion","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-27T00:22:01Z","receivedAt":"2024-05-27T00:22:09Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"On Sun, May 26, 2024 at 04:26:51PM -0700, Junio C Hamano wrote:\n> As I keep saying over multiple iterations, the above three bullet\n> points stress too much on the minute implementation detail while\n> failing to tell readers that the end-user alias receives the rest of\n> the command line as arguments.\n\nOK, I can agree that perhaps I've been a bit to fixated on the\naddition of \"$@\" and the mechanics of this. I will propose again with\nthis trimmed. What I didn't have to help me at the time was the full\ncommand in GIT_TRACE, which I think is probably a more appropriate way\nto communicate these details of what's actually hitting the exec\ncalls.\n\n> Of course the simplest one-liner, if you had the \"one\" script\n> already stored in the file, is to say\n\nSo the reason I fell into this, and I wonder how much this plays out\nfor others too, is that shipping these workflow bits as stand-alone\nscripts would mean no !shell tricks required, it's all very logical\nand I would never have looked at any of this :) However, when all you\nhave is a hammer ... since a git config .inc file was needed for other\nthings, it has been overloaded into essentially being mini\npackage-manger that avoids having to install additional dependencies;\none-liner shell script at a time :)\n\n> You can do one of two easy things.\n> \n> $ sh -c 'echo \"$1\" | grep \"$2\"' - 1 2\n\nOk, I think \"sh -c\" in ! aliases is a bit confusing, personally. You\nend up two shells nested deep, and you really have to explain what's\ngoing on with $0; it's very easy to miss.\n\n> $ e(){ echo \"$1\" | grep \"$2\"; };e 1 2\n\nThis method, which is used elsewhere in the docs as well, I think\nmakes the most sense. So I've left that in as the example.\n\n-i\n\n"},{"id":"495659","messageId":"20240527003208.1565249-1-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240525234454.1489598-1-iwienand@redhat.com","subject":"[PATCH v7 1/3] Documentation: alias: rework notes into points","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-27T00:30:47Z","receivedAt":"2024-05-27T00:32:19Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"There are a number of caveats when using aliases. Rather than\nstuffing them all together in a paragraph, let's separate them out\ninto individual points to make it clearer what's going on.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 01df96fab3..40851ef429 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -21,8 +21,9 @@ If the alias expansion is prefixed with an exclamation point,\n it will be treated as a shell command. For example, defining\n `alias.new = !gitk --all --not ORIG_HEAD`, the invocation\n `git new` is equivalent to running the shell command\n-`gitk --all --not ORIG_HEAD`. Note that shell commands will be\n-executed from the top-level directory of a repository, which may\n-not necessarily be the current directory.\n-`GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n-from the original current directory. See linkgit:git-rev-parse[1].\n+`gitk --all --not ORIG_HEAD`. Note:\n++\n+* Shell commands will be executed from the top-level directory of a\n+ repository, which may not necessarily be the current directory.\n+* `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n+ from the original current directory. See linkgit:git-rev-parse[1].\n-- \n2.45.1\n\n"},{"id":"495660","messageId":"20240527003208.1565249-2-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240527003208.1565249-1-iwienand@redhat.com","subject":"[PATCH v7 2/3] Documentation: alias: add notes on shell expansion","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-27T00:30:48Z","receivedAt":"2024-05-27T00:32:22Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"When writing inline shell for shell-expansion aliases (i.e. prefixed\nwith \"!\"), there are some caveats around argument parsing to be aware\nof. This series of notes attempts to explain what is happening more\nclearly.\n\nSigned-off-by: Ian Wienand \n---\n Documentation/config/alias.txt | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\nindex 40851ef429..75f9f5e26f 100644\n--- a/Documentation/config/alias.txt\n+++ b/Documentation/config/alias.txt\n@@ -27,3 +27,17 @@ it will be treated as a shell command. For example, defining\n repository, which may not necessarily be the current directory.\n * `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n from the original current directory. See linkgit:git-rev-parse[1].\n+* Shell command aliases always receive any extra arguments provided to\n+ the Git command-line as positional arguments.\n+** Care should be taken if your shell alias is a \"one-liner\" script\n+ with multiple commands (e.g. in a pipeline), references multiple\n+ arguments, or is otherwise not able to handle positional arguments\n+ appended via `\"$@\"`. For example: `alias.cmd = \"!echo $1 | grep\n+ $2\"` called as `git cmd 1 2` will be executed as 'echo $1 | grep $2\n+ 1 2', which is not what you want.\n+** A convenient way to deal with this is to write your script\n+ operations in an inline function that is then called with any\n+ arguments from the command-line. For example `alias.cmd = \"!c() {\n+ echo $1 | grep $2 ; }; c\" will correctly execute the prior example.\n+** Setting `GIT_TRACE=1` can help you debug the command being run for\n+ your alias.\n-- \n2.45.1\n\n"},{"id":"495661","messageId":"20240527003208.1565249-3-iwienand@redhat.com","threadId":"61526","inReplyTo":"20240527003208.1565249-1-iwienand@redhat.com","subject":"[PATCH v7 3/3] run-command: show prepared command","fromName":"Ian Wienand","fromEmail":"iwienand@redhat.com","sentAt":"2024-05-27T00:30:49Z","receivedAt":"2024-05-27T00:32:26Z","isPatch":true,"sender":{"key":"iwienand@redhat.com","avatar":"https://avatars.githubusercontent.com/u/108448?v=4"},"body":"This adds a trace point in start_command so we can see the full\ncommand invocation without having to resort to strace/code inspection.\nFor example:\n\n $ GIT_TRACE=1 git test foo\n git.c:755 trace: exec: git-test foo\n run-command.c:657 trace: run_command: git-test foo\n run-command.c:657 trace: run_command: 'echo $*' foo\n run-command.c:749 trace: start_command: /bin/sh -c 'echo $* \"$@\"' 'echo $*' foo\n\nPrior changes have made the documentation around the internals of the\nalias command execution clearer, but I have still found this detailed\nview of the aliased command being run helpful for debugging purposes.\n\nA test case is added to ensure the full command output is present in\nthe execution flow.\n\nSigned-off-by: Ian Wienand \n---\n run-command.c | 3 +++\n t/t0014-alias.sh | 11 +++++++++++\n 2 files changed, 14 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 1b821042b4..31b20123d8 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -746,6 +746,8 @@ int start_command(struct child_process *cmd)\n \t\tgoto end_of_spawn;\n \t}\n \n+\ttrace_argv_printf(&argv.v[1], \"trace: start_command:\");\n+\n \tif (pipe(notify_pipe))\n \t\tnotify_pipe[0] = notify_pipe[1] = -1;\n \n@@ -913,6 +915,7 @@ int start_command(struct child_process *cmd)\n \telse if (cmd->use_shell)\n \t\tcmd->args.v = prepare_shell_cmd(&nargv, sargv);\n \n+\ttrace_argv_printf(cmd->args.v, \"trace: start_command:\");\n \tcmd->pid = mingw_spawnvpe(cmd->args.v[0], cmd->args.v,\n \t\t\t\t (char**) cmd->env.v,\n \t\t\t\t cmd->dir, fhin, fhout, fherr);\ndiff --git a/t/t0014-alias.sh b/t/t0014-alias.sh\nindex 95568342be..854d59ec58 100755\n--- a/t/t0014-alias.sh\n+++ b/t/t0014-alias.sh\n@@ -44,4 +44,15 @@ test_expect_success 'run-command formats empty args properly' '\n test_cmp expect actual\n '\n \n+test_expect_success 'tracing a shell alias with arguments shows trace of prepared command' '\n+\tcat >expect <<-EOF &&\n+\ttrace: start_command: SHELL -c ${SQ}echo \\$* \"\\$@\"${SQ} ${SQ}echo \\$*${SQ} arg\n+\tEOF\n+\tgit config alias.echo \"!echo \\$*\" &&\n+\tenv GIT_TRACE=1 git echo arg 2>output &&\n+\t# redact platform differences\n+\tsed -n -e \"s/^\\(trace: start_command:\\) .* -c /\\1 SHELL -c /p\" output >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.45.1\n\n"},{"id":"495701","messageId":"xmqqmsob9mdm.fsf@gitster.g","threadId":"61526","inReplyTo":"20240527003208.1565249-2-iwienand@redhat.com","subject":"Re: [PATCH v7 2/3] Documentation: alias: add notes on shell expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-27T17:48:37Z","receivedAt":"2024-05-27T17:48:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Wienand writes:\n\n> When writing inline shell for shell-expansion aliases (i.e. prefixed\n> with \"!\"), there are some caveats around argument parsing to be aware\n> of. This series of notes attempts to explain what is happening more\n> clearly.\n>\n> Signed-off-by: Ian Wienand \n> ---\n> Documentation/config/alias.txt | 14 ++++++++++++++\n> 1 file changed, 14 insertions(+)\n\nThis one has become much shorter but very much to the point. \n\nLet's further do \"appended via `\"$@\"`\" -> \"added at the end\". The\nexample given in that bullet point would be easier to understand\nthat way.\n\nWill queue.\n\n> diff --git a/Documentation/config/alias.txt b/Documentation/config/alias.txt\n> index 40851ef429..75f9f5e26f 100644\n> --- a/Documentation/config/alias.txt\n> +++ b/Documentation/config/alias.txt\n> @@ -27,3 +27,17 @@ it will be treated as a shell command. For example, defining\n> repository, which may not necessarily be the current directory.\n> * `GIT_PREFIX` is set as returned by running `git rev-parse --show-prefix`\n> from the original current directory. See linkgit:git-rev-parse[1].\n> +* Shell command aliases always receive any extra arguments provided to\n> + the Git command-line as positional arguments.\n> +** Care should be taken if your shell alias is a \"one-liner\" script\n> + with multiple commands (e.g. in a pipeline), references multiple\n> + arguments, or is otherwise not able to handle positional arguments\n> + appended via `\"$@\"`. For example: `alias.cmd = \"!echo $1 | grep\n> + $2\"` called as `git cmd 1 2` will be executed as 'echo $1 | grep $2\n> + 1 2', which is not what you want.\n> +** A convenient way to deal with this is to write your script\n> + operations in an inline function that is then called with any\n> + arguments from the command-line. For example `alias.cmd = \"!c() {\n> + echo $1 | grep $2 ; }; c\" will correctly execute the prior example.\n> +** Setting `GIT_TRACE=1` can help you debug the command being run for\n> + your alias.\n"}]}