@matheusbsbs / matheus-bagliano / commits / 00a6d4d1d2

worktree: allow to (re)move worktrees with uninitialized submodules

Uninitialized submodules have nothing valueable for us to be worried about. They are just SHA-1. Let "worktree remove" and "worktree move" continue in this case so that people can still use multiple worktrees on repos with optional submodules that are never populated, like sha1collisiondetection in git.git when checked out by doc-diff script. Note that for "worktree remove", it is possible that a user initializes a submodule (*), makes some commits (but not push), then deinitializes it. At that point, the submodule is unpopulated, but the precious new commits are still in $GIT_COMMON_DIR/worktrees/<worktree>/modules/<submodule> directory and we should not allow removing the worktree or we lose those commits forever. The new directory check is added to prevent this. (*) yes they are screwed anyway by doing this since "git submodule" would add submodule.* in $GIT_COMMON_DIR/config, which is shared across multiple worktrees. But it does not mean we let them be screwed even more. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Jan 5, 2019 at 12:08 UTC 00a6d4d1d2260a07ca1372acfae692d9821f165f
2 files changed +60 -6
builtin/worktree.c
+23 -6
@@ -9,6 +9,7 @@
9 #include "refs.h"
10 #include "run-command.h"
11 #include "sigchain.h"
12 +#include "submodule.h"
13 #include "refs.h"
14 #include "utf8.h"
15 #include "worktree.h"
@@ -724,20 +725,36 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
725 static void validate_no_submodules(const struct worktree *wt)
726 {
727 struct index_state istate = { NULL };
728 + struct strbuf path = STRBUF_INIT;
729 int i, found_submodules = 0;
730
729 - if (read_index_from(&istate, worktree_git_path(wt, "index"),
730 - get_worktree_git_dir(wt)) > 0) {
731 + if (is_directory(worktree_git_path(wt, "modules"))) {
732 + /*
733 + * There could be false positives, e.g. the "modules"
734 + * directory exists but is empty. But it's a rare case and
735 + * this simpler check is probably good enough for now.
736 + */
737 + found_submodules = 1;
738 + } else if (read_index_from(&istate, worktree_git_path(wt, "index"),
739 + get_worktree_git_dir(wt)) > 0) {
740 for (i = 0; i < istate.cache_nr; i++) {
741 struct cache_entry *ce = istate.cache[i];
742 + int err;
743
734 - if (S_ISGITLINK(ce->ce_mode)) {
735 - found_submodules = 1;
736 - break;
737 - }
744 + if (!S_ISGITLINK(ce->ce_mode))
745 + continue;
746 +
747 + strbuf_reset(&path);
748 + strbuf_addf(&path, "%s/%s", wt->path, ce->name);
749 + if (!is_submodule_populated_gently(path.buf, &err))
750 + continue;
751 +
752 + found_submodules = 1;
753 + break;
754 }
755 }
756 discard_index(&istate);
757 + strbuf_release(&path);
758
759 if (found_submodules)
760 die(_("working trees containing submodules cannot be moved or removed"));
t/t2028-worktree-move.sh
+37
@@ -112,6 +112,26 @@ test_expect_success 'move locked worktree (force)' '
112 git worktree move --force --force flump ploof
113 '
114
115 +test_expect_success 'move a repo with uninitialized submodule' '
116 + git init withsub &&
117 + (
118 + cd withsub &&
119 + test_commit initial &&
120 + git submodule add "$PWD"/.git sub &&
121 + git commit -m withsub &&
122 + git worktree add second HEAD &&
123 + git worktree move second third
124 + )
125 +'
126 +
127 +test_expect_success 'not move a repo with initialized submodule' '
128 + (
129 + cd withsub &&
130 + git -C third submodule update &&
131 + test_must_fail git worktree move third forth
132 + )
133 +'
134 +
135 test_expect_success 'remove main worktree' '
136 test_must_fail git worktree remove .
137 '
@@ -185,4 +205,21 @@ test_expect_success 'remove cleans up .git/worktrees when empty' '
205 )
206 '
207
208 +test_expect_success 'remove a repo with uninitialized submodule' '
209 + (
210 + cd withsub &&
211 + git worktree add to-remove HEAD &&
212 + git worktree remove to-remove
213 + )
214 +'
215 +
216 +test_expect_success 'not remove a repo with initialized submodule' '
217 + (
218 + cd withsub &&
219 + git worktree add to-remove HEAD &&
220 + git -C to-remove submodule update &&
221 + test_must_fail git worktree remove to-remove
222 + )
223 +'
224 +
225 test_done