git rev-parse broken on Git for Windows

  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

Hi,

On Thu, 29 Jul 2010, Joshua Jensen wrote:

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

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

Johannes Schindelin wrote:

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.

  ----- Original Message -----
From: Thomas Rast
Date: 7/30/2010 3:02 AM

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

Joshua Jensen wrote:

[...]

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.

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(-)

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

Thanks.