Weird behavior of shell variables in git aliases

It seems that a variable is available only once?! How can the following shell session be explained:

    $ git config alias.test0
    !echo $1
    $ git test0 foo
    foo
    $ git config alias.test1
    !echo $1 && echo $1
    $ git test1 foo

    foo
    $ git config alias.test2
    !BRANCH=$1 && echo $BRANCH && echo $BRANCH
    $ git test2 foo

    foo

Thanks, D.

On Mon, Mar 21, 2011 at 09:39:43AM -0700, Dun Peal wrote:

Because in v1.7.4 and earlier, we literally just tack the arguments (shell-quoted) onto the end of the string. So your alias ends up expanding to:

  /bin/sh -c "!echo $1 && echo $1 'foo'"

So the first echo is empty, and then the second one echos foo. And what you are trying to do doesn't work with a straight alias (at the bottom I'll show you what you want).

But interestingly, that's _not_ the behavior as of Erik's 7f51f8b (alias: use run_command api to execute aliases, 2011-01-07), which is in master but not yet released. With that, we end up executing:

  sh -c 'echo $1 && echo $1 "$@"' 'echo $1 && echo $1' 'foo'

which prints "foo foo". So it is technically a regression. I don't know how much we care; using positional parameters like this was already nonsensical, as shown above.

For reference, what you actually want (in either system) is:

  $ git config alias.test1
  !sh -c 'echo $1 && echo $1' -
  $ git test1 foo
  foo
  foo

Make sure to include the "-" (or some other string) which ends up as $0.

-Peff

Jeff King <peff@peff.net> writes:

Oh, I should have been paying a bit more attention. I've been assuming that we were turning "!anything" into { "sh", "-c", "anything", "-" } followed by the user supplied arguments.

Junio C Hamano <gitster@pobox.com> writes:

The attached quick hack gives

  $ git config alias.silly
  !echo hello $1; echo $# args, bye!
  $ GIT_TRACE=1 ./git silly world funny
  trace: exec: 'git-silly' 'world' 'funny'
  trace: run_command: 'git-silly' 'world' 'funny'
  trace: run_command: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!'' '-' 'world' 'funny'
  trace: exec: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!''
  '-' 'world' 'funny'
  hello world
  2 args, bye!

but it would penalize a properly written alias that uses "sh -c <it> -" trick itself by double forking, which is not very nice and I am unhappy about.

 git.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

On 21 March 2011 23:33, Junio C Hamano <gitster@pobox.com> wrote:

That would IMHO be The Right Way to do it. Since the documentation for aliases promises to to pass my alias to a shell if I prefix it with "!", I shouldn't have to add the "sh -c ... -" myself...

A properly written alias that uses a trick? I guess that sums up the problem... :-)

Anyway, doesn't the existing way potentially break when passing funky arguments containing spaces/quotes/something? We currently pass the arguments to the alias command as a single quoted string. Passing them as seperate elements on argv seems a lot more robust...

Do we risk breaking any existing "!sh -c ... -" aliases by applying this?

/Lasse

On Tue, Mar 22, 2011 at 11:38:06AM +0100, Lasse Makholm wrote:

It does pass it to the shell. You can do:

  $ git config alias.autolog --oneline
  !repo=`find-git-repo-for $PWD` && git --git-dir="$repo" log

for an example of a shell-based alias. It just doesn't handle positional parameters the way you want. The typical solution is to invoke another shell, but you can also do:

  $ grep -B1 silly .git/config
  [alias]
          silly = "!foo() { echo hello $1; echo $# args, bye!\n}\nfoo"

which unsurprisingly looks like exactly the same solution one would use in the shell to avoid the fact that shell aliases suck for handling positional parameters.

Yeah. Though it also penalizes non-tricky aliases. See my other mail in this thread.

No, the arguments in the current scheme are properly shell-quoted before they are appended to the shell snippet. So they are equally robust.

-Peff

On 22 March 2011 12:28, Jeff King <peff@peff.net> wrote:

Atually, I think that's probably the most elegant way of dealing with the issue... An example like this would be very useful in the alias documentation along with an explanation on how positional parameters are handled...

On Mon, Mar 21, 2011 at 23:33, Junio C Hamano <gitster@pobox.com> wrote:

It would also be bizarre on Solaris where /bin/sh isn't a POSIX shell.

But we can substitute the relevant user-supplied shell during the build process.

On Mon, Mar 21, 2011 at 03:33:25PM -0700, Junio C Hamano wrote:

Yeah, I think that would be more useful in general, but...

Doesn't it also break a lot of other more garden-variety aliases that rely on the automagic "$@", like:

  $ git config alias.log-nopager
  !git --no-pager log

  $ GIT_TRACE=1 git.v1.7.4 log-nopager --oneline
  trace: exec: 'git-log-nopager' '--oneline'
  trace: run_command: 'git-log-nopager' '--oneline'
  trace: alias to shell cmd: log-nopager => git --no-pager log '--oneline'
  trace: built-in: git 'log' '--oneline'
  93c7d44 foo

  $ GIT_TRACE=1 git.master log-nopager --oneline
  trace: exec: 'git-log-nopager' '--oneline'
  trace: run_command: 'git-log-nopager' '--oneline'
  trace: run_command: 'git --no-pager log' '--oneline'
  trace: exec: 'sh' '-c' 'git --no-pager log "$@"' 'git --no-pager log' '--oneline'
  trace: built-in: git 'log' '--oneline'
  93c7d44 foo

  $ GIT_TRACE=1 git.jch.shell-alias log-nopager --oneline
  trace: exec: 'git-log-nopager' '--oneline'
  trace: run_command: 'git-log-nopager' '--oneline'
  trace: run_command: 'sh' '-c' 'git --no-pager log' '-' '--oneline'
  trace: exec: 'sh' '-c' 'git --no-pager log' '-' '--oneline'
  trace: built-in: git 'log'
  commit 93c7d44635e8bb56a4fd864d024ce75a2ad4ffcf
  Author: Jeff King <peff@peff.net>
  Date:   Tue Mar 22 06:57:02 2011 -0400

      foo

I think the evolution of the alias code was something like:

 1. Let's have alias.foo, so that "git foo XXX" can be aliased to "git
    log --whatever XXX", just like shell aliases.

 2. Oops, git doesn't allow some things in (1) that we might want to do,
    like turning off the pager or choosing a new repo. For that we need
    to respawn git, so now we have "!git --whatever1 log --whatever2".
    It appends the arguments, just like the form in (1), so it is
    consistent.

 3. Oops, (2) doesn't allow complex mini-scripts that access the
    parameters in a non-sequential way. You have to do "!sh -c 'git
    --whatever $1 log --whatever2 $2'".

Knowing that step (3) exists, I think your solution is a better one. The question is whether it is better enough to be worth breaking the people who were helped by step (2).

Another way of looking at it is that types (1) and (2) are like shell aliases. And they suck for complex things, just the way that shell aliases do. The solution in the shell is to put your complex alias into a shell function, or to push it into its own script. In git, you can push things into their own script, but we have no equivalent to a shell function. So one solution is:

  git config function.silly '!echo hello $1; echo $# args, bye!'

The name is of course terrible. But I think the idea is sound that there are two different types of interface people may want. On the other hand, this is getting kind of grossly complex. The separate script option is always available.

-Peff

On Tue, Mar 22, 2011 at 07:18:44AM -0400, Jeff King wrote:

One other solution would be to make the "$@" more magic by detecting when the alias uses positional parameters and omitting it in that case. Something like (on top of your patch):

On 22 March 2011 14:28, Jeff King <peff@peff.net> wrote:

Then how about simply:

On Tue, Mar 22, 2011 at 02:35:10PM +0100, Lasse Makholm wrote:

Yeah, that certainly improves the situation.

A small formatting nit: Those long commands with punctuation get hard to read in the middle of a paragraph. Maybe something like this on top:

On 22 March 2011 14:43, Jeff King <peff@peff.net> wrote:

Thought about it but wasn't sure of the asciidoc formatting... I'll fix it and submit a proper patch...

Yeah, gmail is sucky in that regard... :-/

Thanks a lot for the explanations, I was really wondering about that.

Incidentally, I had a real reason to prefer using the `!echo...` syntax over the `!sh -c 'echo...'` one; I need my users to define the following alias:

    git checkout $1 && git submodule foreach 'git checkout $sha1'

Due to the necessity of single-quoting the `foreach` expression, I couldn't define it with the `!sh -c` syntax. And because of the problems discussed in this thread, I still don't know how to have my users define this alias.

Thanks, D.

P.S. Incidentally, this alias is addressing a pitfall in the use of submodules that I think should not exist in the first place:

If a repo has a submodule, and head 'foo' of the repo has the submodule's HEAD pointing to head 'foo' of its origin, while head 'bar' of the repo has the submodule HEAD pointing to head 'bar' of its origin, checking out bar/foo on the repository leaves the submodule in an outdated state (and `submodule update` doesn't help).

Jeff King <peff@peff.net> writes:

The first addition is indeed a huge improvement.

    Note that any argument you pass when running aliases are simply
    appended to the shell command.

The original didn't explicitly say it but it really should have. The example that comes before it, "alias.new = !...", should be updated with an invocation that takes a parameter, perhaps like this:

    With this alias defined:

	[alias] since = "!gitk --all --since"

    you can view commits in the last week with:

        $ git since 7.days

    because this expands to "gitk --all --since 7.days" by concatenating
    the arguments supplied at runtime to the alias.

Then say that "Note ..." to stress that point. The description at that point has become much better.

With that understanding already there,

    This means that "alias.foo = !echo $# args: $1, $2 and $3" will
    not do what you expect.

is no longer true; nobody sane would expect that if you made them realize that "simply appended" already. Just dropping that sentence would make the resulting text flow much better.

    If you want to refer to arguments given to the alias, you can
    wrap it as a shell script, e.g.

	[alias] reversed = "!sh -c 'echo $2 $1' -"

    or a shell function, e.g.

	[alias] reversed = "!reversed() { echo $2 $1 } && reversed"

    and invoke it like so:

	$ git reversed one two
        two one

I personally think the former "sh -c <str> -" is the more traditional and well understood form (iow, an idiom) for people who breathe shells.

While I totally agree with the formatting advice you gave here, we may want to avoid this notation; neither "git config alias.foo = !echo ..." nor writing "alias.foo = ..." in .git/config file would work.

On Tue, Mar 22, 2011 at 10:57:08AM -0700, Junio C Hamano wrote:

Agreed, your version is better.

Yeah, that is probably true. One advantage of the function idiom is that it doesn't happen inside single-quotes, so it's one less layer of quoting to deal with. And of course it saves a shell invocation. So I think mentioning both is reasonable.

Yeah, I didn't even think about that, but you are right.

-Peff

On 22 March 2011 19:32, Jeff King <peff@peff.net> wrote:

Good points...

As one of those shell-breathing people I prefer the function-style for its lack of quotes:

git config alias.foo = !foo () { echo $# args: $1, $2 and $3; }; foo

Whlie we're at it, is "sh -c ... -" and "sh -c ... --" equally portable, I wonder?

Good point, I was wondering about that but decided to take my clue from existing examples of the same notation...

Right, I'll sleep() on it and cook a patch tomorow, attempting to take all of the above into account...

Lasse Makholm <lasse.makholm@gmail.com> writes:

I suspect anything would work, but it is customary to pass the dummy $0 by writing "-" there. People who breathe shells would be able to recognize the idiom more easily if you stick to that well-known form.

Jeff King <peff@peff.net> writes:

Let's not go there.

Jeff King <peff@peff.net> writes:

Of course, silly me.

Because the most common usage of the alias mechanism is to package the command name and common options and allow tacking other command line arguments after them at runtime, and it is silly to require the user to say "$@" (i.e. 'lg = "log --oneline"' example), any alias that wants to use positional parameter should do the 'sh -c "$str" -' packaging itself.

So there is nothing to fix after all; I was just confused by seeing the initial report which was just a usage error.

Sorry for the noise, and thanks.