[PATCH 2/2] worktree: allow sharing a checked-out branch across worktrees

["Jason Newton via GitGitGadget" <gitgitgadget@gmail.com>]

From: Jason Newton <nevion@gmail.com>

When spinning up several worktrees on the same checkout for parallel
work (for example a fleet of agents working from one branch), git's
refusal to check out a branch that is already checked out elsewhere is
just in the way. The restriction exists to stop two worktrees from
moving the same branch underneath each other, but plain parallel
checkouts do not need that protection.

Drop the restriction: "git worktree add <branch>" now checks out a
branch even if it is in use by another worktree. The genuinely
dangerous case is kept -- a branch that another worktree is in the
middle of rebasing or bisecting is still refused, because a second
checkout could corrupt that operation. die_if_branch_busy() performs
that narrower check in place of the old die_if_checked_out(). The
separate guard against force-updating (e.g. with -B) a branch in use
elsewhere is left untouched.

Signed-off-by: Jason Newton <nevion@gmail.com>
---
 Documentation/git-worktree.adoc | 17 +++++++++--------
 builtin/worktree.c              | 30 +++++++++++++++++++++++++++++-
 t/t2400-worktree-add.sh         | 31 ++++++++++++++++++++++++-------
 3 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-worktree.adoc b/Documentation/git-worktree.adoc
index 1ca81718b7..cc4c91b787 100644
--- a/Documentation/git-worktree.adoc
+++ b/Documentation/git-worktree.adoc
@@ -93,8 +93,9 @@ then, as a convenience, the new worktree is associated with a branch (call
 it _<branch>_) named after `$(basename <path>)`.  If _<branch>_ doesn't
 exist, a new branch based on `HEAD` is automatically created as if
 `-b <branch>` was given.  If _<branch>_ does exist, it will be checked out
-in the new worktree, if it's not checked out anywhere else, otherwise the
-command will refuse to create the worktree (unless `--force` is used).
+in the new worktree, even if it is already checked out in another worktree.
+(A branch that another worktree is in the middle of rebasing or bisecting is
+refused unless `--force` is used.)
 +
 If _<commit-ish>_ is omitted, neither `--detach`, or `--orphan` is
 used, and there are no valid local branches (or remote branches if
@@ -177,12 +178,12 @@ OPTIONS
 
 `-f`::
 `--force`::
-	By default, `add` refuses to create a new worktree when
-	_<commit-ish>_ is a branch name and is already checked out by
-	another worktree, or if _<path>_ is already assigned to some
-	worktree but is missing (for instance, if _<path>_ was deleted
-	manually). This option overrides these safeguards. To add a missing but
-	locked worktree path, specify `--force` twice.
+	`add` refuses to create a new worktree when _<commit-ish>_ is a
+	branch that another worktree is in the middle of rebasing or
+	bisecting, or if _<path>_ is already assigned to some worktree but
+	is missing (for instance, if _<path>_ was deleted manually). This
+	option overrides these safeguards. To add a missing but locked
+	worktree path, specify `--force` twice.
 +
 `move` refuses to move a locked worktree unless `--force` is specified
 twice. If the destination is already assigned to some other worktree but is
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 973da33051..b457b015d1 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -648,6 +648,34 @@ static void setup_alternate_ref_dir(struct worktree *wt, const char *wt_git_path
 	strbuf_release(&sb);
 }
 
+/*
+ * Checking out a branch that is already checked out in another worktree is
+ * fine -- it is exactly what you want when spinning up several worktrees on
+ * the same checkout for parallel work. The one case that is still unsafe is a
+ * branch that another worktree is in the middle of rebasing or bisecting,
+ * since a second checkout could corrupt that operation, so refuse only that.
+ */
+static void die_if_branch_busy(const char *branch)
+{
+	struct worktree **worktrees = get_worktrees();
+	int i;
+
+	for (i = 0; worktrees[i]; i++) {
+		const struct worktree *wt = worktrees[i];
+
+		if (is_worktree_being_rebased(wt, branch) ||
+		    is_worktree_being_bisected(wt, branch)) {
+			const char *shortname = branch;
+
+			skip_prefix(branch, "refs/heads/", &shortname);
+			die(_("'%s' is already used by worktree at '%s'"),
+			    shortname, wt->path);
+		}
+	}
+
+	free_worktrees(worktrees);
+}
+
 static int add_worktree(const char *path, const char *refname,
 			const struct add_opts *opts)
 {
@@ -675,7 +703,7 @@ static int add_worktree(const char *path, const char *refname,
 	    refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) {
 		is_branch = 1;
 		if (!opts->force)
-			die_if_checked_out(symref.buf, 0);
+			die_if_branch_busy(symref.buf);
 	}
 	commit = lookup_commit_reference_by_name(refname);
 	if (!commit && !opts->orphan)
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 56fb79179a..6a1eb72ac7 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -40,10 +40,24 @@ test_expect_success '"add" using - shorthand' '
 	test_cmp expect actual
 '
 
-test_expect_success '"add" refuses to checkout locked branch' '
-	test_must_fail git worktree add zere main &&
-	test_path_is_missing zere &&
-	test_path_is_missing .git/worktrees/zere
+test_expect_success '"add" can check out a branch in use by another worktree' '
+	test_when_finished "git worktree remove -f zere || :" &&
+	git worktree add zere main &&
+	echo refs/heads/main >expect &&
+	git -C zere symbolic-ref HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'the same branch can be checked out in several worktrees' '
+	test_when_finished "git worktree remove -f shared1 || :; git worktree remove -f shared2 || :" &&
+	git branch -f sharedbr main &&
+	git worktree add shared1 sharedbr &&
+	git worktree add shared2 sharedbr &&
+	echo refs/heads/sharedbr >expect &&
+	git -C shared1 symbolic-ref HEAD >actual &&
+	test_cmp expect actual &&
+	git -C shared2 symbolic-ref HEAD >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'checking out paths not complaining about linked checkouts' '
@@ -304,10 +318,13 @@ test_expect_success '"add" checks out existing branch of dwimd name' '
 	)
 '
 
-test_expect_success '"add <path>" dwim fails with checked out branch' '
+test_expect_success '"add <path>" dwim shares a checked out branch' '
 	git checkout -b test-branch &&
-	test_must_fail git worktree add test-branch &&
-	test_path_is_missing test-branch
+	git worktree add test-branch &&
+	echo refs/heads/test-branch >expect &&
+	git -C test-branch symbolic-ref HEAD >actual &&
+	test_cmp expect actual &&
+	git worktree remove test-branch
 '
 
 test_expect_success '"add --force" with existing dwimd name doesnt die' '
-- 
gitgitgadget