Two problems on alias of git

Hello.  I found 2 problems on alias of git.

The first one is that git crashes with the following situation. Without GIT_TRACE, everthing works well. But with GIT_TRACE=1, git crashes every time.

---- 8< ---- 8< ---- 8< ---- 8< ---- 8< ----
$ git remote show origin
* remote origin
  URL: git://repo.or.cz/git.git
  Remote branch merged with 'git pull' while on branch master
    master
  Tracked remote branches
    html
    maint
    man
    master
    next
    pu
    todo

$ git --version git version 1.6.2.rc0.90.g0753

$ git show HEAD | grep commit commit 3536ae331014c68a25c80b3fb530a19c8dee0f11

$ uname -a Darwin foobarbaz 9.6.1 Darwin Kernel Version 9.6.1: Sun Dec 21 19:45:33 PST 2008; root:xnu-1228.9.75~4/RELEASE_I386 i386 i386 MacBook5,2 Darwin

$ git config --get alias.l1 log --pretty=oneline

$ GIT_TRACE=1 git l1 trace: exec: 'git-l1' trace: run_command: 'git-l1' trace: exec 'git-l1' failed: No such file or directory [1] 41772 segmentation fault GIT_TRACE=1 git l1 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ----

The second one is that git doesn't expand a kind of aliases properly, especially with double quotation marks ("). I used the following alias recently to list the last 10 commits on HEAD:

[alias]
        lr = !git l1 | head | tac

Then I want to extend this alias to list the last N commits on a branch with "git lr 20", "git lr master", "git lr master 20", etc. So that I wrote the following definition (note that the actual definition is written in a single line, though the quoted definition is folded in multiple lines for readability):

[alias]
        lr = !$SHELL -c '
                n=10;
                1="${1:-$n}";
                if ! [ "${1##[0-9]*}" = "" ]; then
                  t="$1";
                  1="${2:-$n}";
                  2="$t";
                fi;
                git --no-pager l1 --reverse -"$1" "${2:-HEAD}"
              ' __dummy__

But it doesn't work because git expands as follows:

$ git config --get alias.lr !$SHELL -c '1=${1:-10}

Double quotation marks (") are removed and the aliased string is cut at a random position. I expect that the aliased string is passed to system() as-is, but git doesn't so. Why does git behave so? Is it a bug or an intentional behavior?

"Kana Natsuno" <kana@whileimautomaton.net> writes:

[...]

I don't know if it is a bug or a feature, but git-config supports quoted strings (required if you want to have value which has trailing or leading whitespace, or which contains '#' which is beginning of comment character). Inside quoted string you need to escape '"':

   [string]
        quotes = "quoted \" string ' with # character"

expands as intended. Perhaps stripping of double quotes inside string are artifact of that feature. Try escaping or double escaping quotes: \" or \\\".

On Tue, 05 May 2009 23:03:49 +0900, Jakub Narebski <jnareb@gmail.com> wrote:

Thank you for the information. I've read the source code of git and I confirmed that this stripping is caused by parse_value() in config.c.

And as I changed the old definition

        lr = !$SHELL -c 'n=10; 1="${1:-$n}"; ...' __dummy__

by enclosing all text in double quotes and substituting '"' with '\"' as follows,

	lr = "!$SHELL -c 'n=10; 1=\"${1:-$n}\"; ...' __dummy__"

it works well now.

	$ git config --get alias.lr
	!$SHELL -c 'n=10; 1="${1:-$n}"; if ! [ "${1##[0-9]*}" ...

On Tue, 5 May 2009, Kana Natsuno wrote:

Errr... what I meant here is that I don't know whether stripping quotes inside value which is not quoted (does not begin and end in '"') is intended or unintended consequence of git-config behaviour.

By the way, you can use continuation-of-line character (end line with '\') and/or can embed newlines using C escape sequence, i.e. "\n".

So your code can look like this (although I am not sure it is worth it):

  [alias]
        lr = "!$SHELL -c '                               \n\
                n=10;                                    \n\
                1=\"${1:-$n}\";                          \n\
                if ! [ \"${1##[0-9]*}\" = \"\" ]; then   \n\
                  t=\"$1\";                              \n\
                  1=\"${2:-$n}\";                        \n\
                  2=\"$t\";                              \n\
                fi;                                      \n\
                git --no-pager l1 --reverse -\"$1\" \"${2:-HEAD}\" \n\
              ' __dummy__"

BTW. you need to quote value because it contains comment character '#' in 4th line of script.

On Wed, 06 May 2009 00:42:49 +0900, Jakub Narebski <jnareb@gmail.com> wrote:

Thank you again, I didn't know that.  It is better than what I wrote.

Really? As far as I read the code of git, especially parse_value() in config.c, it is not necessary to escape '#'s because they are inside of the outermost doublequotes and they should not be escaped, because \# is an unknown escape sequence and git rejects them. If #s are escaped, it causes an error as follows:

$ git config --get alias.lr fatal: bad config file line 29 in /Users/kana/.gitconfig

On Wed, 5 May 2009 (yes, different timezones), Kana Natsuno wrote:

By the way, "\n" is there only for nice looking output of git-config
for this variable.  For config file you need only to escape end of line. 
 

Errrr... what I meant is that you have to _quote_ value if it contains comment character, compare

  [string]
  	unquoted =  a # b
  	quoted   = "a # b"

not that you have to escape comment character '#'. But I forgot about the fact that ## in your example is inside quotes anyway.

On Tue, May 05, 2009 at 09:42:04PM +0900, Kana Natsuno wrote:

I wasn't able to reproduce on Linux, but valgrind found it. The patch below should fix it.

-- >8 --
Subject: [PATCH] fix GIT_TRACE segfault with shell-quoted aliases

The alias argv comes from the split_cmdline function, which splits the config text for the alias into an array of strings. It returns the number of elements in the array, but does not actually put a NULL at the end of the array. Later, the trace function tries to print this argv and assumes that it has the trailing NULL.

The split_cmdline function is probably at fault, since argv lists almost always end with a NULL signal. This patch adds one, in addition to the returned count; this doesn't hurt the other callers at all, since they were presumably using the count already (and will never look at the NULL).

While we're there and using ALLOC_GROW, let's clean up the other manual grow.

Signed-off-by: Jeff King <peff@peff.net>
---
 alias.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)