git rev-parse broken on Git for Windows

git rev-parse broken on Git for Windows

[Joshua Jensen]

  9c7304e3e39ed397b3cc6566573333e2698a52b4 (print the usage string on 
stdout instead of stderr) and then 
47e9cd28f8a404a0d6293935252ddca5fc243931 (parseopt: wrap rev-parse 
--parseopt usage for eval consumption) break the following line from the 
manual and 'git subtree' on msysGit:

eval $(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)

Running 'git revert' on both changelists causes the 'git rev-parse 
--parseopt' to work again.

What was 9c7304e trying to solve?

Thanks.

Josh

Re: [msysGit] git rev-parse broken on Git for Windows

[Johannes Schindelin]

Hi,

On Thu, 29 Jul 2010, Joshua Jensen wrote:

>  9c7304e3e39ed397b3cc6566573333e2698a52b4 (print the usage string on stdout
> instead of stderr) and then 47e9cd28f8a404a0d6293935252ddca5fc243931
> (parseopt: wrap rev-parse --parseopt usage for eval consumption) break the
> following line from the manual and 'git subtree' on msysGit:
> 
> eval $(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)

Both commits are from Junio's 'next' branch. I Cc:ed the authors of both 
commits.

> Running 'git revert' on both changelists causes the 'git rev-parse 
> --parseopt' to work again.
> 
> What was 9c7304e trying to solve?

Probably it tried to follow some GNU convention or some such. It does not 
say in the commit message, and neither in the code.

Thanks,
Johannes

Re: [msysGit] git rev-parse broken on Git for Windows

[Thomas Rast]

Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 29 Jul 2010, Joshua Jensen wrote:
> 
> >  9c7304e3e39ed397b3cc6566573333e2698a52b4 (print the usage string on stdout
> > instead of stderr) and then 47e9cd28f8a404a0d6293935252ddca5fc243931
> > (parseopt: wrap rev-parse --parseopt usage for eval consumption) break the
> > following line from the manual and 'git subtree' on msysGit:
> > 
> > eval $(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)
> 
> Both commits are from Junio's 'next' branch. I Cc:ed the authors of both 
> commits.

Can you elaborate on "break"?

Because as you can see in git-sh-setup.sh, the "official" user of
parseopt does

	eval "$(
		echo "$OPTIONS_SPEC" |
			git rev-parse --parseopt $parseopt_extra -- "$@" ||
		echo exit $?
	)"

So AFAICS they only differ in the quoting.  And the latter works.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [msysGit] git rev-parse broken on Git for Windows

[Joshua Jensen]

  ----- Original Message -----
From: Thomas Rast
Date: 7/30/2010 3:02 AM
> Johannes Schindelin wrote:
>> On Thu, 29 Jul 2010, Joshua Jensen wrote:
>>>   9c7304e3e39ed397b3cc6566573333e2698a52b4 (print the usage string on stdout
>>> instead of stderr) and then 47e9cd28f8a404a0d6293935252ddca5fc243931
>>> (parseopt: wrap rev-parse --parseopt usage for eval consumption) break the
>>> following line from the manual and 'git subtree' on msysGit:
>>>
>>> eval $(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)
>> Both commits are from Junio's 'next' branch. I Cc:ed the authors of both
>> commits.
> Can you elaborate on "break"?
>
> Because as you can see in git-sh-setup.sh, the "official" user of
> parseopt does
>
> 	eval "$(
> 		echo "$OPTIONS_SPEC" |
> 			git rev-parse --parseopt $parseopt_extra -- "$@" ||
> 		echo exit $?
> 	)"
>
> So AFAICS they only differ in the quoting.  And the latter works.
Here is the output from Git Bash:

$ git subtree
C:\Program Files (x86)\Git/libexec/git-core/git-subtree: eval: line 31: 
syntax error near unexpected token `<'
C:\Program Files (x86)\Git/libexec/git-core/git-subtree: eval: line 31: 
`cat <<\EOF usage: git subtree add --prefix=<prefix> <commit
 > or: git subtree merge --prefix=<prefix> <commit> or: git subtree pull 
--prefix=<prefix> <repository> <refspec...> or: git subtree
push --prefix=<prefix> <repository> <refspec...> or: git subtree split 
--prefix=<prefix> <commit...> -h, --help show the help -q qui
et -d show debug messages -P, --prefix ... the name of the subdir to 
split out -m, --message ... use the given message as the commit
  message for the merge commit options for 'split' --annotate ... add a 
prefix to commit message of new commits -b, --branch ... crea
te a new branch from the split subtree --ignore-joins ignore prior 
--rejoin commits --onto ... try connecting new tree to an existin
g one --rejoin merge the new branch back into HEAD options for 'add', 
'merge', 'pull' and 'push' --squash merge subtree changes as a
  single commit EOF exit 129'
Usage: git subtree

The example from the git rev-parse documentation fails in the same way:

eval `echo "$OPTS_SPEC" | git rev-parse --parseopt $parseopt_extra -- 
"$@" || echo exit $?`

What does work is the example you gave with the quotes:

eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt $parseopt_extra -- 
"$@" || echo exit $?)"

I can live with modifying 'git subtree' in this manner, but something 
about one or both of those rev-parse commits cause the non-quoted 
version $(echo...) version in git subtree and the `echo...` version to 
break.

Josh

Re: [msysGit] git rev-parse broken on Git for Windows

[Thomas Rast]

Joshua Jensen wrote:
> Thomas Rast wrote:
> > Johannes Schindelin wrote:
> >>> eval $(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)
> > Can you elaborate on "break"?
> >
> > Because as you can see in git-sh-setup.sh, the "official" user of
> > parseopt does
> >
> > 	eval "$(
> > 		echo "$OPTIONS_SPEC" |
> > 			git rev-parse --parseopt $parseopt_extra -- "$@" ||
> > 		echo exit $?
> > 	)"
> >
> > So AFAICS they only differ in the quoting.  And the latter works.
> Here is the output from Git Bash:
> 
> $ git subtree
> C:\Program Files (x86)\Git/libexec/git-core/git-subtree: eval: line 31: 
> syntax error near unexpected token `<'
[...]
> The example from the git rev-parse documentation fails in the same way:
> 
> eval `echo "$OPTS_SPEC" | git rev-parse --parseopt $parseopt_extra -- 
> "$@" || echo exit $?`

Oh, sorry that I was so dense in the first reply.  Of course the
quoting is the problem.  When unquoted, the shell first splits along
whitespace and then eval reassembles with *one space* between each
pair of words.  The change just exacerbated the issue; there are other
ways to trigger bad behaviour if the eval uses an unquoted --parseopt:

  # what you saw
  $ echo eval $( (echo 'description'; echo --; echo "s,long= foo") | git rev-parse --parseopt -- --help || echo exit $?)
  eval cat <<\EOF usage: description -s, --long ... foo EOF exit 129

  # newlines are clobbered
  $ echo eval $( (echo 'description'; echo --; echo "s,long= foo") | git rev-parse --parseopt -- --long="$(printf 'argument\nwith\nnewlines')" || echo exit $?)
  eval set -- -s 'argument with newlines' --

  # consecutive spaces are also clobbered
  echo eval $( (echo 'description'; echo --; echo "s,long= foo") | git rev-parse --parseopt -- --long="three   spaces" || echo exit $?)
  eval set -- -s 'three spaces' --

So I'm afraid Avery will have to fix this in git-subtree.  I'll also
follow up with a doc patch for git-rev-parse.  Luckily there are no
users in git outside of tests and git-sh-setup, so there are no bugs.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[PATCH] Documentation/rev-parse: quoting is required with --parseopt

[Thomas Rast]

When calling rev-parse --parseopt, as in the (now fixed) documented
example

  eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"

the outermost quoting is required, as otherwise all runs of arbitrary
whitespace inside the resulting 'set -- ...' call would be collapsed
into a single space.

This was exposed as a result of our new use of cat <<\EOF since
47e9cd2 (parseopt: wrap rev-parse --parseopt usage for eval
consumption, 2010-06-12), but has always been a problem when handling
arguments containing e.g. newlines.

Point this out in the documentation, and in particular correct the
example that did not have the quotes.

Noticed-by: Joshua Jensen <jjensen@workspacewhiz.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/git-rev-parse.txt |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0727f43..be4c053 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -184,10 +184,13 @@ scripts the same facilities C builtins have. It works as an option normalizer
 (e.g. splits single switches aggregate values), a bit like `getopt(1)` does.
 
 It takes on the standard input the specification of the options to parse and
-understand, and echoes on the standard output a line suitable for `sh(1)` `eval`
+understand, and echoes on the standard output a string suitable for `sh(1)` `eval`
 to replace the arguments with normalized ones.  In case of error, it outputs
 usage on the standard error stream, and exits with code 129.
 
+Note: Make sure you quote the result when passing it to `eval`.  See
+below for an example.
+
 Input Format
 ~~~~~~~~~~~~
 
@@ -244,7 +247,7 @@ bar=      some cool option --bar with an argument
   An option group Header
 C?        option C with an optional argument"
 
-eval `echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?`
+eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
 ------------
 
 SQ-QUOTE
-- 
1.7.2.1.342.g676a4

Re: [msysGit] git rev-parse broken on Git for Windows

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> ...  I'll also
> follow up with a doc patch for git-rev-parse.  Luckily there are no
> users in git outside of tests and git-sh-setup, so there are no bugs.

Thanks.