git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 8/8] checkout: restrict @-expansions when finding branch

From
Jeff King <peff@peff.net>
Date
Feb 28, 2017, 12:17 UTC
Message-ID
<20170228121739.kgjwiedotponwjg2@sigill.intra.peff.net>
In-Reply-To
<20170228120633.zkwfqms57fk7dkl5@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 6115ad504..b8e396009 100755
--- a/t/t3204-branch-name-interpretation.sh
+++ b/t/t3204-branch-name-interpretation.sh
@@ -109,4 +109,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.359.gd4c8c42e9
Previous: Jeff KingNext: Jeff King
Message 9 of 15 in “Re: [BUG] branch renamed to 'HEAD'”
  1. Jeff KingFeb 28, 2017
  2. 1/8 interpret_branch_name: move docstring to header fileJeff King, Feb 28, 2017
  3. 4/8 interpret_branch_name: allow callers to restrict expansionsJeff King, Feb 28, 2017
  4. Jeff KingFeb 28, 2017
  5. Jeff KingFeb 28, 2017
  6. Junio C HamanoFeb 28, 2017
  7. Jeff KingFeb 28, 2017
  8. 3/8 strbuf_branchname: add docstringJeff King, Feb 28, 2017
  9. 8/8 checkout: restrict @-expansions when finding branchJeff King, Feb 28, 2017
  10. 6/8 branch: restrict @-expansions when deletingJeff King, Feb 28, 2017
  11. 7/8 strbuf_check_ref_format(): expand only local branchesJeff King, Feb 28, 2017
  12. 5/8 t3204: test git-branch @-expansion corner casesJeff King, Feb 28, 2017
  13. 2/8 strbuf_branchname: drop return valueJeff King, Feb 28, 2017
  14. Jacob KellerFeb 28, 2017
  15. Junio C HamanoMar 1, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.