From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Keith Derrick <keith.derrick@lge.com>,
"git@vger.kernel.org" <git@vger.kernel.org>
Subject: [PATCH 5/5] interpret_branch_name: find all possible @-marks
Date: Wed, 15 Jan 2014 03:40:46 -0500 [thread overview]
Message-ID: <20140115084045.GE19132@sigill.intra.peff.net> (raw)
In-Reply-To: <20140115082528.GA18974@sigill.intra.peff.net>
When we parse a string like "foo@{upstream}", we look for
the first "@"-sign, and check to see if it is an upstream
mark. However, since branch names can contain an @, we may
also see "@foo@{upstream}". In this case, we check only the
first @, and ignore the second. As a result, we do not find
the upstream.
We can solve this by iterating through all @-marks in the
string, and seeing if any is a legitimate upstream or
empty-at mark.
Another strategy would be to parse from the right-hand side
of the string. However, that does not work for the
"empty_at" case, which allows "@@{upstream}". We need to
find the left-most one in this case (and we then recurse as
"HEAD@{upstream}").
Signed-off-by: Jeff King <peff@peff.net>
---
And this one actually fixes Keith's bug.
The diff is noisy due to indentation changes; try it with "-b" for
increased reading pleasure.
sha1_name.c | 20 +++++++++++---------
t/t1507-rev-parse-upstream.sh | 21 +++++++++++++++++++++
2 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index b253a88..6fca869 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1124,6 +1124,7 @@ static int interpret_upstream_mark(const char *name, int namelen,
int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
{
char *at;
+ const char *start;
int len = interpret_nth_prior_checkout(name, namelen, buf);
if (!namelen)
@@ -1138,17 +1139,18 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
return reinterpret(name, namelen, len, buf);
}
- at = memchr(name, '@', namelen);
- if (!at)
- return -1;
+ for (start = name;
+ (at = memchr(start, '@', namelen - (start - name)));
+ start = at + 1) {
- len = interpret_empty_at(name, namelen, at - name, buf);
- if (len > 0)
- return reinterpret(name, namelen, len, buf);
+ len = interpret_empty_at(name, namelen, at - name, buf);
+ if (len > 0)
+ return reinterpret(name, namelen, len, buf);
- len = interpret_upstream_mark(name, namelen, at - name, buf);
- if (len > 0)
- return len;
+ len = interpret_upstream_mark(name, namelen, at - name, buf);
+ if (len > 0)
+ return len;
+ }
return -1;
}
diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index cace1ca..178694e 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -17,6 +17,9 @@ test_expect_success 'setup' '
test_commit 4 &&
git branch --track my-side origin/side &&
git branch --track local-master master &&
+ git branch --track fun@ny origin/side &&
+ git branch --track @funny origin/side &&
+ git branch --track funny@ origin/side &&
git remote add -t master master-only .. &&
git fetch master-only &&
git branch bad-upstream &&
@@ -54,6 +57,24 @@ test_expect_success 'my-side@{upstream} resolves to correct full name' '
test refs/remotes/origin/side = "$(full_name my-side@{u})"
'
+test_expect_success 'upstream of branch with @ in middle' '
+ full_name fun@ny@{u} >actual &&
+ echo refs/remotes/origin/side >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'upstream of branch with @ at start' '
+ full_name @funny@{u} >actual &&
+ echo refs/remotes/origin/side >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'upstream of branch with @ at end' '
+ full_name funny@@{u} >actual &&
+ echo refs/remotes/origin/side >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'refs/heads/my-side@{upstream} does not resolve to my-side{upstream}' '
test_must_fail full_name refs/heads/my-side@{upstream}
'
--
1.8.5.2.500.g8060133
next prev parent reply other threads:[~2014-01-15 8:40 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-14 23:04 BUG: check-ref-format and rev-parse can not handle branches with an @ in their name combined with @{u} Keith Derrick
2014-01-14 23:45 ` Junio C Hamano
2014-01-15 5:00 ` Jeff King
2014-01-15 7:46 ` Junio C Hamano
2014-01-15 7:47 ` Jeff King
2014-01-15 8:25 ` [PATCH 0/5] interpret_branch_name bug potpourri Jeff King
2014-01-15 8:26 ` [PATCH 1/5] interpret_branch_name: factor out upstream handling Jeff King
2014-01-15 8:27 ` [PATCH 2/5] interpret_branch_name: rename "cp" variable to "at" Jeff King
2014-01-15 8:31 ` [PATCH 3/5] interpret_branch_name: always respect "namelen" parameter Jeff King
2014-01-15 8:37 ` [PATCH 4/5] interpret_branch_name: avoid @{upstream} past colon Jeff King
2014-01-15 8:40 ` Jeff King [this message]
2014-01-15 21:03 ` [PATCH 0/5] interpret_branch_name bug potpourri Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140115084045.GE19132@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=keith.derrick@lge.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).