Merge branch 'uk/checkout-ambiguous-ref'

* uk/checkout-ambiguous-ref:
  Rename t2019 with typo "amiguous" that meant "ambiguous"
  checkout: rearrange update_refs_for_switch for clarity
  checkout: introduce --detach synonym for "git checkout foo^{commit}"
  checkout: split off a function to peel away branchname arg
  checkout: fix bug with ambiguous refs

Conflicts:
	builtin/checkout.c

Documentation/git-checkout.txt

@@ -9,6 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git checkout' [-q] [-f] [-m] [<branch>]
+'git checkout' [-q] [-f] [-m] [--detach] [<commit>]
 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
 'git checkout' --patch [<tree-ish>] [--] [<paths>...]
@@ -22,9 +23,10 @@ branch.
 
 'git checkout' [<branch>]::
 'git checkout' -b|-B <new_branch> [<start point>]::
+'git checkout' [--detach] [<commit>]::
 
 	This form switches branches by updating the index, working
-	tree, and HEAD to reflect the specified branch.
+	tree, and HEAD to reflect the specified branch or commit.
 +
 If `-b` is given, a new branch is created as if linkgit:git-branch[1]
 were called and then checked out; in this case you can
@@ -115,6 +117,13 @@ explicitly give a name with '-b' in such a case.
 	Create the new branch's reflog; see linkgit:git-branch[1] for
 	details.
 
+--detach::
+	Rather than checking out a branch to work on it, check out a
+	commit for inspection and discardable experiments.
+	This is the default behavior of "git checkout <commit>" when
+	<commit> is not a branch name.  See the "DETACHED HEAD" section
+	below for details.
+
 --orphan::
 	Create a new 'orphan' branch, named <new_branch>, started from
 	<start_point> and switch to it.  The first commit made on this
@@ -204,7 +213,7 @@ leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
 
 
 
-Detached HEAD
+DETACHED HEAD
 -------------
 
 It is sometimes useful to be able to 'checkout' a commit that is

builtin/checkout.c

@@ -30,6 +30,7 @@ struct checkout_opts {
 	int quiet;
 	int merge;
 	int force;
+	int force_detach;
 	int writeout_stage;
 	int writeout_error;
 
@@ -541,7 +542,17 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 	strbuf_addf(&msg, "checkout: moving from %s to %s",
 		    old_desc ? old_desc : "(invalid)", new->name);
 
-	if (new->path) {
+	if (!strcmp(new->name, "HEAD") && !new->path && !opts->force_detach) {
+		/* Nothing to do. */
+	} else if (opts->force_detach || !new->path) {	/* No longer on any branch. */
+		update_ref(msg.buf, "HEAD", new->commit->object.sha1, NULL,
+			   REF_NODEREF, DIE_ON_ERR);
+		if (!opts->quiet) {
+			if (old->path && advice_detached_head)
+				detach_advice(old->path, new->name);
+			describe_detached_head("HEAD is now at", new->commit);
+		}
+	} else if (new->path) {	/* Switch branches. */
 		create_symref("HEAD", new->path, msg.buf);
 		if (!opts->quiet) {
 			if (old->path && !strcmp(new->path, old->path))
@@ -563,18 +574,11 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 			if (!file_exists(ref_file) && file_exists(log_file))
 				remove_path(log_file);
 		}
-	} else if (strcmp(new->name, "HEAD")) {
-		update_ref(msg.buf, "HEAD", new->commit->object.sha1, NULL,
-			   REF_NODEREF, DIE_ON_ERR);
-		if (!opts->quiet) {
-			if (old->path && advice_detached_head)
-				detach_advice(old->path, new->name);
-			describe_detached_head("HEAD is now at", new->commit);
-		}
 	}
 	remove_branch_state();
 	strbuf_release(&msg);
-	if (!opts->quiet && (new->path || !strcmp(new->name, "HEAD")))
+	if (!opts->quiet &&
+	    (new->path || (!opts->force_detach && !strcmp(new->name, "HEAD"))))
 		report_tracking(new);
 }
 
@@ -675,11 +679,123 @@ static const char *unique_tracking_name(const char *name)
 	return NULL;
 }
 
+static int parse_branchname_arg(int argc, const char **argv,
+				int dwim_new_local_branch_ok,
+				struct branch_info *new,
+				struct tree **source_tree,
+				unsigned char rev[20],
+				const char **new_branch)
+{
+	int argcount = 0;
+	unsigned char branch_rev[20];
+	const char *arg;
+	int has_dash_dash;
+
+	/*
+	 * case 1: git checkout <ref> -- [<paths>]
+	 *
+	 *   <ref> must be a valid tree, everything after the '--' must be
+	 *   a path.
+	 *
+	 * case 2: git checkout -- [<paths>]
+	 *
+	 *   everything after the '--' must be paths.
+	 *
+	 * case 3: git checkout <something> [<paths>]
+	 *
+	 *   With no paths, if <something> is a commit, that is to
+	 *   switch to the branch or detach HEAD at it.  As a special case,
+	 *   if <something> is A...B (missing A or B means HEAD but you can
+	 *   omit at most one side), and if there is a unique merge base
+	 *   between A and B, A...B names that merge base.
+	 *
+	 *   With no paths, if <something> is _not_ a commit, no -t nor -b
+	 *   was given, and there is a tracking branch whose name is
+	 *   <something> in one and only one remote, then this is a short-hand
+	 *   to fork local <something> from that remote-tracking branch.
+	 *
+	 *   Otherwise <something> shall not be ambiguous.
+	 *   - If it's *only* a reference, treat it like case (1).
+	 *   - If it's only a path, treat it like case (2).
+	 *   - else: fail.
+	 *
+	 */
+	if (!argc)
+		return 0;
+
+	if (!strcmp(argv[0], "--"))	/* case (2) */
+		return 1;
+
+	arg = argv[0];
+	has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
+
+	if (!strcmp(arg, "-"))
+		arg = "@{-1}";
+
+	if (get_sha1_mb(arg, rev)) {
+		if (has_dash_dash)          /* case (1) */
+			die("invalid reference: %s", arg);
+		if (dwim_new_local_branch_ok &&
+		    !check_filename(NULL, arg) &&
+		    argc == 1) {
+			const char *remote = unique_tracking_name(arg);
+			if (!remote || get_sha1(remote, rev))
+				return argcount;
+			*new_branch = arg;
+			arg = remote;
+			/* DWIMmed to create local branch */
+		} else {
+			return argcount;
+		}
+	}
+
+	/* we can't end up being in (2) anymore, eat the argument */
+	argcount++;
+	argv++;
+	argc--;
+
+	new->name = arg;
+	setup_branch_path(new);
+
+	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 */
+
+	new->commit = lookup_commit_reference_gently(rev, 1);
+	if (!new->commit) {
+		/* not a commit */
+		*source_tree = parse_tree_indirect(rev);
+	} else {
+		parse_commit(new->commit);
+		*source_tree = new->commit->tree;
+	}
+
+	if (!*source_tree)                   /* case (1): want a tree */
+		die("reference is not a tree: %s", arg);
+	if (!has_dash_dash) {/* case (3 -> 1) */
+		/*
+		 * Do not complain the most common case
+		 *	git checkout branch
+		 * even if there happen to be a file called 'branch';
+		 * it would be extremely annoying.
+		 */
+		if (argc)
+			verify_non_filename(NULL, arg);
+	} else {
+		argcount++;
+		argv++;
+		argc--;
+	}
+
+	return argcount;
+}
+
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
 	struct checkout_opts opts;
 	unsigned char rev[20];
-	const char *arg;
 	struct branch_info new;
 	struct tree *source_tree = NULL;
 	char *conflict_style = NULL;
@@ -692,6 +808,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_STRING('B', NULL, &opts.new_branch_force, "branch",
 			   "create/reset and checkout a branch"),
 		OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "create reflog for new branch"),
+		OPT_BOOLEAN(0, "detach", &opts.force_detach, "detach the HEAD at named commit"),
 		OPT_SET_INT('t', "track",  &opts.track, "set upstream info for new branch",
 			BRANCH_TRACK_EXPLICIT),
 		OPT_STRING(0, "orphan", &opts.new_orphan_branch, "new branch", "new unparented branch"),
@@ -709,7 +826,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
 		OPT_END(),
 	};
-	int has_dash_dash;
 
 	memset(&opts, 0, sizeof(opts));
 	memset(&new, 0, sizeof(new));
@@ -731,9 +847,15 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		opts.new_branch = opts.new_branch_force;
 
 	if (patch_mode && (opts.track > 0 || opts.new_branch
-			   || opts.new_branch_log || opts.merge || opts.force))
+			   || opts.new_branch_log || opts.merge || opts.force
+			   || opts.force_detach))
 		die ("--patch is incompatible with all other options");
 
+	if (opts.force_detach && (opts.new_branch || opts.new_orphan_branch))
+		die("--detach cannot be used with -b/-B/--orphan");
+	if (opts.force_detach && 0 < opts.track)
+		die("--detach cannot be used with -t");
+
 	/* --track without -b should DWIM */
 	if (0 < opts.track && !opts.new_branch) {
 		const char *argv0 = argv[0];
@@ -766,105 +888,30 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		die("git checkout: -f and -m are incompatible");
 
 	/*
-	 * case 1: git checkout <ref> -- [<paths>]
+	 * Extract branch name from command line arguments, so
+	 * all that is left is pathspecs.
 	 *
-	 *   <ref> must be a valid tree, everything after the '--' must be
+	 * Handle
-	 *   a path.
 	 *
-	 * case 2: git checkout -- [<paths>]
+	 *  1) git checkout <tree> -- [<paths>]
-	 *
+	 *  2) git checkout -- [<paths>]
-	 *   everything after the '--' must be paths.
+	 *  3) git checkout <something> [<paths>]
-	 *
-	 * case 3: git checkout <something> [<paths>]
-	 *
-	 *   With no paths, if <something> is a commit, that is to
-	 *   switch to the branch or detach HEAD at it.  As a special case,
-	 *   if <something> is A...B (missing A or B means HEAD but you can
-	 *   omit at most one side), and if there is a unique merge base
-	 *   between A and B, A...B names that merge base.
-	 *
-	 *   With no paths, if <something> is _not_ a commit, no -t nor -b
-	 *   was given, and there is a remote-tracking branch whose name is
-	 *   <something> in one and only one remote, then this is a short-hand
-	 *   to fork local <something> from that remote-tracking branch.
-	 *
-	 *   Otherwise <something> shall not be ambiguous.
-	 *   - If it's *only* a reference, treat it like case (1).
-	 *   - If it's only a path, treat it like case (2).
-	 *   - else: fail.
 	 *
+	 * including "last branch" syntax and DWIM-ery for names of
+	 * remote branches, erroring out for invalid or ambiguous cases.
 	 */
 	if (argc) {
-		if (!strcmp(argv[0], "--")) {       /* case (2) */
+		int dwim_ok =
-			argv++;
+			!patch_mode &&
-			argc--;
+			dwim_new_local_branch &&
-			goto no_reference;
+			opts.track == BRANCH_TRACK_UNSPECIFIED &&
-		}
+			!opts.new_branch;
-
+		int n = parse_branchname_arg(argc, argv, dwim_ok,
-		arg = argv[0];
+				&new, &source_tree, rev, &opts.new_branch);
-		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
+		argv += n;
-
+		argc -= n;
-		if (!strcmp(arg, "-"))
-			arg = "@{-1}";
-
-		if (get_sha1_mb(arg, rev)) {
-			if (has_dash_dash)          /* case (1) */
-				die("invalid reference: %s", arg);
-			if (!patch_mode &&
-			    dwim_new_local_branch &&
-			    opts.track == BRANCH_TRACK_UNSPECIFIED &&
-			    !opts.new_branch &&
-			    !check_filename(NULL, arg) &&
-			    argc == 1) {
-				const char *remote = unique_tracking_name(arg);
-				if (!remote || get_sha1(remote, rev))
-					goto no_reference;
-				opts.new_branch = arg;
-				arg = remote;
-				/* DWIMmed to create local branch */
-			}
-			else
-				goto no_reference;
-		}
-
-		/* we can't end up being in (2) anymore, eat the argument */
-		argv++;
-		argc--;
-
-		new.name = arg;
-		if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
-			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;
-			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);
-		if (!has_dash_dash) {/* case (3 -> 1) */
-			/*
-			 * Do not complain the most common case
-			 *	git checkout branch
-			 * even if there happen to be a file called 'branch';
-			 * it would be extremely annoying.
-			 */
-			if (argc)
-				verify_non_filename(NULL, arg);
-		}
-		else {
-			argv++;
-			argc--;
-		}
 	}
 
-no_reference:
-
 	if (opts.track == BRANCH_TRACK_UNSPECIFIED)
 		opts.track = git_branch_track;
 
@@ -886,6 +933,9 @@ no_reference:
 			}
 		}
 
+		if (opts.force_detach)
+			die("git checkout: --detach does not take a path argument");
+
 		if (1 < !!opts.writeout_stage + !!opts.force + !!opts.merge)
 			die("git checkout: --ours/--theirs, --force and --merge are incompatible when\nchecking out of the index.");
 

t/t2019-checkout-ambiguous-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

t/t2020-checkout-detach.sh

@@ -0,0 +1,95 @@
+#!/bin/sh
+
+test_description='checkout into detached HEAD state'
+. ./test-lib.sh
+
+check_detached () {
+	test_must_fail git symbolic-ref -q HEAD >/dev/null
+}
+
+check_not_detached () {
+	git symbolic-ref -q HEAD >/dev/null
+}
+
+reset () {
+	git checkout master &&
+	check_not_detached
+}
+
+test_expect_success 'setup' '
+	test_commit one &&
+	test_commit two &&
+	git branch branch &&
+	git tag tag
+'
+
+test_expect_success 'checkout branch does not detach' '
+	reset &&
+	git checkout branch &&
+	check_not_detached
+'
+
+test_expect_success 'checkout tag detaches' '
+	reset &&
+	git checkout tag &&
+	check_detached
+'
+
+test_expect_success 'checkout branch by full name detaches' '
+	reset &&
+	git checkout refs/heads/branch &&
+	check_detached
+'
+
+test_expect_success 'checkout non-ref detaches' '
+	reset &&
+	git checkout branch^ &&
+	check_detached
+'
+
+test_expect_success 'checkout ref^0 detaches' '
+	reset &&
+	git checkout branch^0 &&
+	check_detached
+'
+
+test_expect_success 'checkout --detach detaches' '
+	reset &&
+	git checkout --detach branch &&
+	check_detached
+'
+
+test_expect_success 'checkout --detach without branch name' '
+	reset &&
+	git checkout --detach &&
+	check_detached
+'
+
+test_expect_success 'checkout --detach errors out for non-commit' '
+	reset &&
+	test_must_fail git checkout --detach one^{tree} &&
+	check_not_detached
+'
+
+test_expect_success 'checkout --detach errors out for extra argument' '
+	reset &&
+	git checkout master &&
+	test_must_fail git checkout --detach tag one.t &&
+	check_not_detached
+'
+
+test_expect_success 'checkout --detached and -b are incompatible' '
+	reset &&
+	test_must_fail git checkout --detach -b newbranch tag &&
+	check_not_detached
+'
+
+test_expect_success 'checkout --detach moves HEAD' '
+	reset &&
+	git checkout one &&
+	git checkout --detach two &&
+	git diff --exit-code HEAD &&
+	git diff --exit-code two
+'
+
+test_done