checkout/switch: disallow checking out same branch in multiple worktrees

Commands `git switch -C` and `git checkout -B` neglect to check whether
the provided branch is already checked out in some other worktree, as
shown by the following:

  $ git worktree list
  .../foo    beefb00f [main]
  $ git worktree add ../other
  Preparing worktree (new branch 'other')
  HEAD is now at beefb00f first
  $ cd ../other
  $ git switch -C main
  Switched to and reset branch 'main'
  $ git worktree list
  .../foo    beefb00f [main]
  .../other  beefb00f [main]

Fix this problem by teaching `git switch -C` and `git checkout -B` to
check whether the branch in question is already checked out elsewhere.

Unlike what it is done for `git switch` and `git checkout`, that have
an historical exception to ignore other worktrees if the branch to
check is the current one (as required when called as part of other
tools), the logic implemented is more strict and will require the user
to invoke the command with `--ignore-other-worktrees` to explicitly
indicate they want the risky behaviour.

This matches the current behaviour of `git branch -f` and is safer; for
more details see the tests in t2400.

Reported-by: Jinwook Jeong <[email protected]>
Helped-by: Eric Sunshine <[email protected]>
Helped-by: Rubén Justo <[email protected]>
Helped-by: Phillip Wood <[email protected]>
Signed-off-by: Carlo Marcelo Arenas Belón <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: carenas

builtin/checkout.c

@@ -1474,7 +1474,8 @@ static void die_if_some_operation_in_progress(void)
 }
 
 static int checkout_branch(struct checkout_opts *opts,
-			   struct branch_info *new_branch_info)
+			   struct branch_info *new_branch_info,
+			   char *check_branch_path)
 {
 	if (opts->pathspec.nr)
 		die(_("paths cannot be used with switching branches"));
@@ -1533,13 +1534,13 @@ static int checkout_branch(struct checkout_opts *opts,
 	if (!opts->can_switch_when_in_progress)
 		die_if_some_operation_in_progress();
 
-	if (new_branch_info->path && !opts->force_detach && !opts->new_branch &&
-	    !opts->ignore_other_worktrees) {
+	if (!opts->ignore_other_worktrees && !opts->force_detach &&
+	    check_branch_path && ref_exists(check_branch_path)) {
 		int flag;
 		char *head_ref = resolve_refdup("HEAD", 0, NULL, &flag);
-		if (head_ref &&
-		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new_branch_info->path)))
-			die_if_checked_out(new_branch_info->path, 1);
+		if (opts->new_branch_force || (head_ref &&
+		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, check_branch_path))))
+			die_if_checked_out(check_branch_path, 1);
 		free(head_ref);
 	}
 
@@ -1627,7 +1628,9 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 			 const char * const usagestr[],
 			 struct branch_info *new_branch_info)
 {
+	int ret;
 	int parseopt_flags = 0;
+	char *check_branch_path = NULL;
 
 	opts->overwrite_ignore = 1;
 	opts->prefix = prefix;
@@ -1717,6 +1720,13 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 		opts->new_branch = argv0 + 1;
 	}
 
+	if (opts->new_branch && !opts->ignore_other_worktrees) {
+		struct strbuf buf = STRBUF_INIT;
+
+		strbuf_branchname(&buf, opts->new_branch, INTERPRET_BRANCH_LOCAL);
+		strbuf_splice(&buf, 0, 0, "refs/heads/", 11);
+		check_branch_path = strbuf_detach(&buf, NULL);
+	}
 	/*
 	 * Extract branch name from command line arguments, so
 	 * all that is left is pathspecs.
@@ -1741,6 +1751,9 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 					     new_branch_info, opts, &rev);
 		argv += n;
 		argc -= n;
+
+		if (!opts->ignore_other_worktrees && !check_branch_path && new_branch_info->path)
+			check_branch_path = xstrdup(new_branch_info->path);
 	} else if (!opts->accept_ref && opts->from_treeish) {
 		struct object_id rev;
 
@@ -1817,9 +1830,12 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 	}
 
 	if (opts->patch_mode || opts->pathspec.nr)
-		return checkout_paths(opts, new_branch_info);
+		ret = checkout_paths(opts, new_branch_info);
 	else
-		return checkout_branch(opts, new_branch_info);
+		ret = checkout_branch(opts, new_branch_info, check_branch_path);
+
+	free(check_branch_path);
+	return ret;
 }
 
 int cmd_checkout(int argc, const char **argv, const char *prefix)

t/t2400-worktree-add.sh

@@ -118,32 +118,52 @@ test_expect_success '"add" worktree creating new branch' '
 	)
 '
 
-test_expect_success 'die the same branch is already checked out' '
+test_expect_success 'die if the same branch is already checked out' '
 	(
 		cd here &&
-		test_must_fail git checkout newmain
+		test_must_fail git checkout newmain &&
+		test_must_fail git checkout -B newmain &&
+		test_must_fail git switch newmain &&
+		test_must_fail git switch -C newmain
 	)
 '
 
-test_expect_success SYMLINKS 'die the same branch is already checked out (symlink)' '
+test_expect_success SYMLINKS 'die if the same branch is already checked out (symlink)' '
 	head=$(git -C there rev-parse --git-path HEAD) &&
 	ref=$(git -C there symbolic-ref HEAD) &&
 	rm "$head" &&
 	ln -s "$ref" "$head" &&
 	test_must_fail git -C here checkout newmain
 '
 
-test_expect_success 'not die the same branch is already checked out' '
+test_expect_success 'allow creating multiple worktrees for same branch with force' '
+	git worktree add --force anothernewmain newmain
+'
+
+test_expect_success 'allow checkout/reset from the conflicted branch' '
 	(
 		cd here &&
-		git worktree add --force anothernewmain newmain
+		git checkout -b conflictedmain newmain &&
+		git checkout -B conflictedmain newmain &&
+		git switch -C conflictedmain newmain
+	)
+'
+
+test_expect_success 'and not die on re-checking out current branch even if conflicted' '
+	(
+		cd there &&
+		git checkout newmain &&
+		git switch newmain
 	)
 '
 
-test_expect_success 'not die on re-checking out current branch' '
+test_expect_failure 'unless using force without --ignore-other-worktrees' '
 	(
 		cd there &&
-		git checkout newmain
+		test_must_fail git checkout -B newmain &&
+		test_must_fail git switch -C newmain &&
+		git checkout --ignore-other-worktrees -B newmain &&
+		git switch --ignore-other-worktrees -C newmain
 	)
 '