[PATCH 5/9] worktree: disallow adding same path multiple times

[Eric Sunshine <sunshine@xxxxxxxxxxxxx>]

A given path should only ever be associated with a single registered
worktree. This invariant is enforced by refusing to create a new
worktree at a given path if that path already exists. For example:

    $ git worktree add -q --detach foo
    $ git worktree add -q --detach foo
    fatal: 'foo' already exists

However, the check can be fooled, and the invariant broken, if the
path is missing. Continuing the example:

    $ rm -fr foo
    $ git worktree add -q --detach foo
    $ git worktree list
    ...      eadebfe [master]
    .../foo  eadebfe (detached HEAD)
    .../foo  eadebfe (detached HEAD)

This "corruption" leads to the unfortunate situation in which the
worktree can not be removed:

    $ git worktree remove foo
    fatal: validation failed, cannot remove working tree: '.../foo'
    does not point back to '.git/worktrees/foo'

Nor can the bogus entry be pruned:

    $ git worktree prune -v
    $ git worktree list
    ...      eadebfe [master]
    .../foo  eadebfe (detached HEAD)
    .../foo  eadebfe (detached HEAD)

without first deleting the worktree directory manually:

    $ rm -fr foo
    $ git worktree prune -v
    Removing .../foo: gitdir file points to non-existent location
    Removing .../foo1: gitdir file points to non-existent location
    $ git worktree list
    ...  eadebfe [master]

or by manually deleting the worktree entry in .git/worktrees.

To address this problem, upgrade "git worktree add" validation to
allow worktree creation only if the given path is not already
associated with an existing worktree (even if the path itself is
non-existent), thus preventing such bogus worktree entries from being
created in the first place.

Reported-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
---
 builtin/worktree.c      | 25 +++++++++++++++++++++++++
 t/t2025-worktree-add.sh |  7 +++++++
 2 files changed, 32 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 46c93771e8..1122f27b5f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -221,8 +221,33 @@ static const char *worktree_basename(const char *path, int *olen)
 
 static void validate_worktree_add(const char *path, const struct add_opts *opts)
 {
+	struct worktree **worktrees;
+	struct worktree *wt;
+	int locked;
+
 	if (file_exists(path) && !is_empty_dir(path))
 		die(_("'%s' already exists"), path);
+
+	worktrees = get_worktrees(0);
+	/*
+	 * find_worktree()'s suffix matching may undesirably find the main
+	 * rather than a linked worktree (for instance, when the basenames
+	 * of the main worktree and the one being created are the same).
+	 * We're only interested in linked worktrees, so skip the main
+	 * worktree with +1.
+	 */
+	wt = find_worktree(worktrees + 1, NULL, path);
+	if (!wt)
+		goto done;
+
+	locked = !!is_worktree_locked(wt);
+	if (locked)
+		die(_("'%s' is a missing but locked worktree;\nuse 'unlock' and 'prune' or 'remove' to clear"), path);
+	else
+		die(_("'%s' is a missing but already registered worktree;\nuse 'prune' or 'remove' to clear"), path);
+
+done:
+	free_worktrees(worktrees);
 }
 
 static int add_worktree(const char *path, const char *refname,
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 07d292317c..67fccc6591 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -552,4 +552,11 @@ test_expect_success '"add" in bare repo invokes post-checkout hook' '
 	test_cmp hook.expect goozy/hook.actual
 '
 
+test_expect_success '"add" an existing but missing worktree' '
+	git worktree add --detach pneu &&
+	test_must_fail git worktree add --detach pneu &&
+	rm -fr pneu &&
+	test_must_fail git worktree add --detach pneu
+'
+
 test_done
-- 
2.19.0.rc1.350.ge57e33dbd1