On Thursday 05 June 2008 13:05, [EMAIL PROTECTED] wrote:
> ---- snip ----
> I posted the following on the git list earlier today, but after
> Johannes mentioned that
> it might be msys-specific I figured I might as well fix it. [Johannes
> - I know that you're
> out of the msysgit game now, but perhaps in time you'll reconsider?]
>
> BUG: "git stash apply <arg>" broken, "git-stash apply <arg>" works
>
> Synopsis:
> "git stash apply [EMAIL PROTECTED]" doesn't work, but
> "git-stash apply [EMAIL PROTECTED]" does.
>
> Platform:
> win xp (yeah, I know, I know)
> msys (Git-1.5.5-preview20080413.exe)
> git version 1.5.5.1015.g9d258
>
> To reproduce:
> # Create a minimal repo
> mkdir stash-bug
> cd stash-bug
> git init
> echo line1 > file
> git add file
> git commit -m initial
>
> # Create the stash that we'll use
> echo "This is correct" > file
> git stash save "Save off correct line in what will be [EMAIL PROTECTED]"
>
> # Create another stash (the default one) which we won't use
> echo "This should not be shown" > file
> git stash save "Save off incorrect line in what will be [EMAIL
> PROTECTED]"
>
> # They are correctly created
> git stash list
>
> # "git-stash apply arg" works
> git reset --hard
> git-stash apply [EMAIL PROTECTED]
> echo "git-stash apply respects its argument"
> cat file
>
> # But "git stash apply arg" doesn't
> git reset --hard
> git stash apply [EMAIL PROTECTED]
> echo "git stash apply (no dash) doesn't respect its argument"
> cat file
>
> A fix is attached.
> ---- snap ----
>
> Although seemingly undocumented, CreateProcess() filters curly
> brackets out of the command line unless they are quoted.
I've meditated over a cup of coffee on this subject.
It's not CreateProcess() that is to blame. It's MSYS again, specifically,
something that I consider a bug in the glob() implementation of msys/cygwin.
Namely, a pattern containing braces should be expanded only if it contains a
comma, but this particular implementation expands single-element brace lists
as well and thereby removes the braces:
foo.{bar,baz} -> foo.bar foo.baz (correct)
foo.{bar} -> foo.bar (not correct), should be foo.{bar}
(Try this with 'echo' in your favorite shell.)
This is going on:
Since Windows command line does not have globbing, the Microsoft engineer had
the brilliant idea that the expansion of file name patterns must be
implemented by the commands themselves. For this purpose, the startup code
word-splits the command line (obeying quotes, which are removed). Then the
words must be file name pattern expanded, and the result passed on to the
main() function.
If you are writing a "console application" with MS's own tools, you simply
link in the file setargv.obj, and it does all the magic.
Now there is bash. It is not compiled nor linked with MS's tools, but MSYS's
tools, and it has its own startup code to do the equvalent. Well, not
exactly: It does more expansions than MS's setargv.obj, in particular, it
does also the brace expansion. (And it is not optional.) See globify() in
rt/src/winsup/cygwin/dcrt0.cc, which calls glob(). Now study glob.c in the
same directory, and you will come to the conclusion that glob() expands
single-element brace lists, which is IMO incorrect.
> This commit fixes (at least) the case of "git stash apply [EMAIL PROTECTED]".
> Note that "git-stash apply [EMAIL PROTECTED]" works even without this patch
> (because it is executed directly by /bin/sh).
...
> diff --git a/compat/mingw.c b/compat/mingw.c
> index a5b43bc..95ba563 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -380,7 +380,7 @@ static const char *quote_arg(const char *arg)
> const char *p = arg;
> if (!*p) force_quotes = 1;
> while (*p) {
> - if (isspace(*p) || *p == '*' || *p == '?')
> + if (isspace(*p) || *p == '*' || *p == '?' || *p == '{')
> force_quotes = 1;
> else if (*p == '"')
> n++;
But for the time being, this workaround should be good enough. Sign-off?
-- Hannes
