checkout: fix bug with ambiguous refs

The usual dwim_ref lookup prefers tags to branches. Because
checkout primarily works on branches, though, we switch that
behavior to prefer branches.

However, there was a bug in the implementation in which we
used lookup_commit_reference (which used the regular lookup
rules) to get the actual commit to checkout. Checking out an
ambiguous ref therefore ended up putting us in an extremely
broken state in which we wrote the branch ref into HEAD, but
actually checked out the tree for the tag.

This patch fixes the bug by always attempting to pull the
commit to be checked out from the branch-ified version of
the name we were given.

Patch by Junio, tests and commit message from Jeff King.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin/checkout.c

@@ -678,7 +678,7 @@ static const char *unique_tracking_name(const char *name)
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
 	struct checkout_opts opts;
-	unsigned char rev[20];
+	unsigned char rev[20], branch_rev[20];
 	const char *arg;
 	struct branch_info new;
 	struct tree *source_tree = NULL;
@@ -832,18 +832,21 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		argc--;
 
 		new.name = arg;
-		if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
-			setup_branch_path(&new);
+		setup_branch_path(&new);
 
-			if ((check_ref_format(new.path) == CHECK_REF_FORMAT_OK) &&
-			    resolve_ref(new.path, rev, 1, NULL))
-				;
-			else
-				new.path = NULL;
+		if (check_ref_format(new.path) == CHECK_REF_FORMAT_OK &&
+		    resolve_ref(new.path, branch_rev, 1, NULL))
+			hashcpy(rev, branch_rev);
+		else
+			new.path = NULL; /* not an existing branch */
+
+		if (!(new.commit = lookup_commit_reference_gently(rev, 1))) {
+			/* not a commit */
+			source_tree = parse_tree_indirect(rev);
+		} else {
 			parse_commit(new.commit);
 			source_tree = new.commit->tree;
-		} else
-			source_tree = parse_tree_indirect(rev);
+		}
 
 		if (!source_tree)                   /* case (1): want a tree */
 			die("reference is not a tree: %s", arg);

t/t2019-checkout-amiguous-ref.sh

@@ -0,0 +1,59 @@
+#!/bin/sh
+
+test_description='checkout handling of ambiguous (branch/tag) refs'
+. ./test-lib.sh
+
+test_expect_success 'setup ambiguous refs' '
+	test_commit branch file &&
+	git branch ambiguity &&
+	git branch vagueness &&
+	test_commit tag file &&
+	git tag ambiguity &&
+	git tag vagueness HEAD:file &&
+	test_commit other file
+'
+
+test_expect_success 'checkout ambiguous ref succeeds' '
+	git checkout ambiguity >stdout 2>stderr
+'
+
+test_expect_success 'checkout produces ambiguity warning' '
+	grep "warning.*ambiguous" stderr
+'
+
+test_expect_success 'checkout chooses branch over tag' '
+	echo refs/heads/ambiguity >expect &&
+	git symbolic-ref HEAD >actual &&
+	test_cmp expect actual &&
+	echo branch >expect &&
+	test_cmp expect file
+'
+
+test_expect_success 'checkout reports switch to branch' '
+	grep "Switched to branch" stderr &&
+	! grep "^HEAD is now at" stderr
+'
+
+test_expect_success 'checkout vague ref succeeds' '
+	git checkout vagueness >stdout 2>stderr &&
+	test_set_prereq VAGUENESS_SUCCESS
+'
+
+test_expect_success VAGUENESS_SUCCESS 'checkout produces ambiguity warning' '
+	grep "warning.*ambiguous" stderr
+'
+
+test_expect_success VAGUENESS_SUCCESS 'checkout chooses branch over tag' '
+	echo refs/heads/vagueness >expect &&
+	git symbolic-ref HEAD >actual &&
+	test_cmp expect actual &&
+	echo branch >expect &&
+	test_cmp expect file
+'
+
+test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' '
+	grep "Switched to branch" stderr &&
+	! grep "^HEAD is now at" stderr
+'
+
+test_done