patchRe: [PATCH] git-parse-remote: fix ambiguous shell bug in expand_refs_wildcard

Bash and dash parse "${name%'^{}'}" differently, with bash quoting the ^{}, and dash assuming the first } is the end of the variable (and thus tacking '} to the end). Instead, use backslash to quote the closing brace.

Signed-off-by: Jeff King <peff@peff.net>
---
Please sanity check that I understand what the code is supposed to be
doing. The bug I was getting was this:

$ readlink /bin/sh
dash
$ git-clone git://git.kernel.org/pub/scm/git/git.git
[...]
$ cd git && git-pull
Warning: No merge candidate found because value of config option
         "branch.master.merge" does not match any remote branch fetched.

A shell trace showed lots of tests against strings like "refs/heads/master'}". Expand_refs_wildcard returned no entries, so it couldn't match the branch.master.merge field.

 git-parse-remote.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

On Mon, Dec 18, 2006 at 03:09:41AM -0500, Jeff King wrote:

Urgh, sorry, this is wrong. It should be:
  ${name%^{\}}
IOW, the \ replaces the quote, not in addition to.

On Mon, Dec 18, 2006 at 11:57:33AM -0800, Junio C Hamano wrote:

Interestingly, this works in dash:

$ foo=bar} $ echo ${foo%'}'} bar

but doing it inside an interpolated string doesn't:

$ foo=bar} $ echo "${foo%'}'}" bar}'}

That seems reasonable to me.

On Mon, Dec 18, 2006 at 05:45:05PM -0500, Jeff King wrote:

Yes it's a bug in dash. Both quote marks (" and ') are represented by the same char internally before processing which is where the mix-up occurs.

I'll work on a fix.

Cheers,

On Tue, Dec 19, 2006 at 11:35:57AM +1100, herbert wrote:

Sorry for the delay. I've finally looked at fixing this. It turns out that dash's behaviour is actually correct and POSIX compliant.

It's correct because dash treats all single quotes within double quotes (except those within command substitutions) as literals. This interpretation is also supported by POSIX.

In fact the rationale (C.2.2.3) in the POSIX document explicitly disallows the aformentioned usage as it violates the rule that an even number of single quotes if any can occur in an ${...} expression enclosed by double quotes.

So the correct and portable expression in this case would be either

echo "${foo%\}}"

or

brace=} echo "${foo%$brace}"

Cheers,

On Sat, May 05, 2007 at 06:03:13PM +1000, Herbert Xu wrote:

Yes, there's not much room for interpretation; the old git code was clearly bogus (we are working around it by using sed instead). Thanks for tracking this down, Herbert.

It looks like bash is actually broken in POSIXLY_CORRECT mode, then:

$ echo $BASH_VERSION 3.1.17(1)-release $ POSIXLY_CORRECT=1 $ foo=bar} $ echo "${foo%'}'}" bar

My interpretation of the correct behavior is that it should remove a single quote from the end of foo, and then print '} literally (that is, single quote and brace).

-Peff

On Mon, May 07, 2007 at 02:36:22AM -0400, Jeff King wrote:

Well strictly speaking this is allowed by the standard as this usage contains an odd number of single quotes inside an ${...} expression enclosed by double quotes, which behaves in an implementation-specific manner.

Cheers,