# git rev-parse broken on Git for Windows 7 messages from 2010-07-30 to 2010-07-30. Participants: Joshua Jensen, Johannes Schindelin, Thomas Rast, Junio C Hamano. Thread: https://gitlist.dev/t/24580 ## Joshua Jensen, 2010-07-30 05:25 Subject: git rev-parse broken on Git for Windows Message-ID: <4C526260.6000104@workspacewhiz.com> URL: https://gitlist.dev/e/4C526260.6000104%40workspacewhiz.com ``` 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, 2010-07-30 08:26 Subject: Re: [msysGit] git rev-parse broken on Git for Windows Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.1007301022310.2983%40bonsai2 In-Reply-To: <4C526260.6000104@workspacewhiz.com> ``` 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, 2010-07-30 09:02 Subject: Re: [msysGit] git rev-parse broken on Git for Windows Message-ID: <201007301102.15274.trast@student.ethz.ch> URL: https://gitlist.dev/e/201007301102.15274.trast%40student.ethz.ch In-Reply-To: ``` 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, 2010-07-30 14:26 Subject: Re: [msysGit] git rev-parse broken on Git for Windows Message-ID: <4C52E125.1020004@workspacewhiz.com> URL: https://gitlist.dev/e/4C52E125.1020004%40workspacewhiz.com In-Reply-To: <201007301102.15274.trast@student.ethz.ch> ``` ----- 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= or: git subtree merge --prefix= or: git subtree pull --prefix= or: git subtree push --prefix= or: git subtree split --prefix= -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, 2010-07-30 14:43 Subject: Re: [msysGit] git rev-parse broken on Git for Windows Message-ID: <201007301643.50730.trast@student.ethz.ch> URL: https://gitlist.dev/e/201007301643.50730.trast%40student.ethz.ch In-Reply-To: <4C52E125.1020004@workspacewhiz.com> ``` 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 ``` ## Thomas Rast, 2010-07-30 15:01 Subject: [PATCH] Documentation/rev-parse: quoting is required with --parseopt Message-ID: <8ef69f9855ca0719c03d7dac320b3a3beb657bca.1280501949.git.trast@student.ethz.ch> URL: https://gitlist.dev/e/8ef69f9855ca0719c03d7dac320b3a3beb657bca.1280501949.git.trast%40student.ethz.ch In-Reply-To: <201007301643.50730.trast@student.ethz.ch> ``` 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 Signed-off-by: Thomas Rast --- 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 ``` ## Junio C Hamano, 2010-07-30 16:44 Subject: Re: [msysGit] git rev-parse broken on Git for Windows Message-ID: <7vd3u4rkbf.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd3u4rkbf.fsf%40alter.siamese.dyndns.org In-Reply-To: <201007301643.50730.trast@student.ethz.ch> ``` Thomas Rast 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. ```