git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Jacob Keller <jacob.keller@gmail.com>
Subject: [PATCH v2 8/8] checkout: restrict @-expansions when finding branch
Date: Thu, 2 Mar 2017 03:23:18 -0500	[thread overview]
Message-ID: <20170302082318.cw7fyll6saittcoh@sigill.intra.peff.net> (raw)
In-Reply-To: <20170302082100.edaretznmlralswa@sigill.intra.peff.net>

When we parse "git checkout $NAME", we try to interpret
$NAME as a local branch-name. If it is, then we point HEAD
to that branch. Otherwise, we detach the HEAD at whatever
commit $NAME points to.

We do the interpretation by calling strbuf_branchname(), and
then blindly sticking "refs/heads/" on the front. This leads
to nonsense results when expansions like "@{upstream}" or
"@" point to something besides a local branch. We end up
with a local branch name like "refs/heads/origin/master" or
"refs/heads/HEAD".

Normally this has no user-visible effect because those
branches don't exist, and so we fallback to feeding the
result to get_sha1(), which resolves them correctly.

But as the new test in t3204 shows, there are corner cases
where the effect is observable, and we check out the wrong
local branch rather than detaching to the correct one.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/checkout.c                    |  2 +-
 t/t3204-branch-name-interpretation.sh | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 05eefd994..81f07c3ef 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -452,7 +452,7 @@ static void setup_branch_path(struct branch_info *branch)
 {
 	struct strbuf buf = STRBUF_INIT;
 
-	strbuf_branchname(&buf, branch->name, 0);
+	strbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
 	if (strcmp(buf.buf, branch->name))
 		branch->name = xstrdup(buf.buf);
 	strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
diff --git a/t/t3204-branch-name-interpretation.sh b/t/t3204-branch-name-interpretation.sh
index 05e88f92d..698d9cc4f 100755
--- a/t/t3204-branch-name-interpretation.sh
+++ b/t/t3204-branch-name-interpretation.sh
@@ -120,4 +120,14 @@ test_expect_success 'delete branch named "@"' '
 	expect_deleted refs/heads/@
 '
 
+test_expect_success 'checkout does not treat remote @{upstream} as a branch' '
+	git update-ref refs/remotes/origin/checkout one &&
+	git branch --set-upstream-to=origin/checkout &&
+	git update-ref refs/heads/origin/checkout two &&
+	git update-ref refs/heads/remotes/origin/checkout two &&
+
+	git checkout @{upstream} &&
+	expect_branch HEAD one
+'
+
 test_done
-- 
2.12.0.367.gb23790f66

  parent reply	other threads:[~2017-03-02 10:10 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-02  8:21 [PATCH v2] fixing corner-cases with interpret_branch_name() Jeff King
2017-03-02  8:21 ` [PATCH v2 1/8] interpret_branch_name: move docstring to header file Jeff King
2017-03-02  8:21 ` [PATCH v2 2/8] strbuf_branchname: drop return value Jeff King
2017-03-02  8:21 ` [PATCH v2 3/8] strbuf_branchname: add docstring Jeff King
2017-03-02  8:23 ` [PATCH v2 4/8] interpret_branch_name: allow callers to restrict expansions Jeff King
2017-03-02  8:23 ` [PATCH v2 5/8] t3204: test git-branch @-expansion corner cases Jeff King
2017-03-02  8:23 ` [PATCH v2 6/8] branch: restrict @-expansions when deleting Jeff King
2017-03-02  8:23 ` [PATCH v2 7/8] strbuf_check_ref_format(): expand only local branches Jeff King
2017-03-02  8:23 ` Jeff King [this message]
2017-03-02  8:47 ` [PATCH v2] fixing corner-cases with interpret_branch_name() Jacob Keller

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170302082318.cw7fyll6saittcoh@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.keller@gmail.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).