@matheusbsbs / matheus-bagliano / commits / 810382ed37
Fork 0

worktree: make "move" refuse to move atop missing registered worktree

"git worktree add" takes special care to avoid creating a new worktree at a location already registered to an existing worktree even if that worktree is missing (which can happen, for instance, if the worktree resides on removable media). "git worktree move", however, is not so careful when validating the destination location and will happily move the source worktree atop the location of a missing worktree. This leads to the anomalous situation of multiple worktrees being associated with the same path, which is expressly forbidden by design. For example: $ git clone foo.git $ cd foo $ git worktree add ../bar $ git worktree add ../baz $ rm -rf ../bar $ git worktree move ../baz ../bar $ git worktree list .../foo beefd00f [master] .../bar beefd00f [bar] .../bar beefd00f [baz] $ git worktree remove ../bar fatal: validation failed, cannot remove working tree: '.../bar' does not point back to '.git/worktrees/bar' Fix this shortcoming by enhancing "git worktree move" to perform the same additional validation of the destination directory as done by "git worktree add". While at it, add a test to verify that "git worktree move" won't move a worktree atop an existing (non-worktree) path -- a restriction which has always been in place but was never tested. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Eric Sunshine committed Jun 10, 2020 at 02:30 UTC 810382ed378731314c03627a8a2d8962b814ad17
3 files changed +25 -3
Documentation/git-worktree.txt
+3 -1
@@ -126,7 +126,9 @@ OPTIONS
126 locked working tree path, specify `--force` twice.
127 +
128 `move` refuses to move a locked working tree unless `--force` is specified
129 -twice.
129 +twice. If the destination is already assigned to some other working tree but is
130 +missing (for instance, if `<new-path>` was deleted manually), then `--force`
131 +allows the move to proceed; use --force twice if the destination is locked.
132 +
133 `remove` refuses to remove an unclean working tree unless `--force` is used.
134 To remove a locked working tree, specify `--force` twice.
builtin/worktree.c
+1 -2
@@ -860,8 +860,7 @@ static int move_worktree(int ac, const char **av, const char *prefix)
860 strbuf_trim_trailing_dir_sep(&dst);
861 strbuf_addstr(&dst, sep);
862 }
863 - if (file_exists(dst.buf))
864 - die(_("target '%s' already exists"), dst.buf);
863 + check_candidate_path(dst.buf, force, worktrees, "move");
864
865 validate_no_submodules(wt);
866
t/t2403-worktree-move.sh
+21
@@ -112,6 +112,27 @@ test_expect_success 'move locked worktree (force)' '
112 git worktree move --force --force flump ploof
113 '
114
115 +test_expect_success 'refuse to move worktree atop existing path' '
116 + >bobble &&
117 + git worktree add --detach beeble &&
118 + test_must_fail git worktree move beeble bobble
119 +'
120 +
121 +test_expect_success 'move atop existing but missing worktree' '
122 + git worktree add --detach gnoo &&
123 + git worktree add --detach pneu &&
124 + rm -fr pneu &&
125 + test_must_fail git worktree move gnoo pneu &&
126 + git worktree move --force gnoo pneu &&
127 +
128 + git worktree add --detach nu &&
129 + git worktree lock nu &&
130 + rm -fr nu &&
131 + test_must_fail git worktree move pneu nu &&
132 + test_must_fail git worktree --force move pneu nu &&
133 + git worktree move --force --force pneu nu
134 +'
135 +
136 test_expect_success 'move a repo with uninitialized submodule' '
137 git init withsub &&
138 (