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

[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

[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

[Joshua Jensen]

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

[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.

[Junio C Hamano]

Thomas Rast <tr...@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.