Re: [PATCH] checkout: don't check worktrees when not necessary

[Duy Nguyen <pclouds@gmail.com>]

On Sun, May 31, 2015 at 07:16:29PM -0400, Spencer Baugh wrote:
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1237,6 +1237,7 @@ static int parse_branchname_arg(int argc, const char **argv,
>  		char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
>  		if (head_ref &&
>  		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) &&
> +		    !(opts->patch_mode || opts->pathspec.nr) &&
>  		    !opts->ignore_other_worktrees)
>  			check_linked_checkouts(new);
>  		free(head_ref);

Simple and effective. But if in future we add more options for
non-switching-branch checkout, we need to update both places, here and
near the end of cmd_checkout().

Perhaps we can move all this block inside checkout_branch() so we only
need to test "opts->patch_mode || opts->pathspec.nr" once, at the end
of cmd_checkout(). Something like below?

I'm not opposed to your change, but if you go with it, you should
cherry pick my test in the below patch. Or create a similar test.

-- 8< --
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2f92328..e9aee58 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1110,7 +1110,6 @@ static int parse_branchname_arg(int argc, const char **argv,
 {
 	struct tree **source_tree = &opts->source_tree;
 	const char **new_branch = &opts->new_branch;
-	int force_detach = opts->force_detach;
 	int argcount = 0;
 	unsigned char branch_rev[20];
 	const char *arg;
@@ -1231,17 +1230,6 @@ static int parse_branchname_arg(int argc, const char **argv,
 	else
 		new->path = NULL; /* not an existing branch */
 
-	if (new->path && !force_detach && !*new_branch) {
-		unsigned char sha1[20];
-		int flag;
-		char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
-		if (head_ref &&
-		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) &&
-		    !opts->ignore_other_worktrees)
-			check_linked_checkouts(new);
-		free(head_ref);
-	}
-
 	new->commit = lookup_commit_reference_gently(rev, 1);
 	if (!new->commit) {
 		/* not a commit */
@@ -1321,6 +1309,17 @@ static int checkout_branch(struct checkout_opts *opts,
 		die(_("Cannot switch branch to a non-commit '%s'"),
 		    new->name);
 
+	if (new->path && !opts->force_detach && !opts->new_branch) {
+		unsigned char sha1[20];
+		int flag;
+		char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
+		if (head_ref &&
+		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) &&
+		    !opts->ignore_other_worktrees)
+			check_linked_checkouts(new);
+		free(head_ref);
+	}
+
 	if (opts->new_worktree)
 		return prepare_linked_checkout(opts, new);
 
diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh
index f8e4df4..a8d9336 100755
--- a/t/t2025-checkout-to.sh
+++ b/t/t2025-checkout-to.sh
@@ -28,6 +28,14 @@ test_expect_success 'checkout --to refuses to checkout locked branch' '
 	! test -d .git/worktrees/zere
 '
 
+test_expect_success 'checking out paths not complaining about linked checkouts' '
+	(
+	cd existing_empty &&
+	echo dirty >>init.t &&
+	git checkout master -- init.t
+	)
+'
+
 test_expect_success 'checkout --to a new worktree' '
 	git rev-parse HEAD >expect &&
 	git checkout --detach --to here master &&
-- 8< --
--
Duy