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>

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 5e8402617727f9cf9c017df3ea6c576e2d59240d..3f9907fcc994d248ba604509f6feb5bdc0499329 100644 (file)
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -9,6 +9,7 @@
 #include "refs.h"
 #include "run-command.h"
 #include "sigchain.h"
+#include "submodule.h"
 #include "refs.h"
 #include "utf8.h"
 #include "worktree.h"
@@ -724,20 +725,36 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
 static void validate_no_submodules(const struct worktree *wt)
 {
        struct index_state istate = { NULL };
+       struct strbuf path = STRBUF_INIT;
        int i, found_submodules = 0;
 
-       if (read_index_from(&istate, worktree_git_path(wt, "index"),
-                           get_worktree_git_dir(wt)) > 0) {
+       if (is_directory(worktree_git_path(wt, "modules"))) {
+               /*
+                * There could be false positives, e.g. the "modules"
+                * directory exists but is empty. But it's a rare case and
+                * this simpler check is probably good enough for now.
+                */
+               found_submodules = 1;
+       } else if (read_index_from(&istate, worktree_git_path(wt, "index"),
+                                  get_worktree_git_dir(wt)) > 0) {
                for (i = 0; i < istate.cache_nr; i++) {
                        struct cache_entry *ce = istate.cache[i];
+                       int err;
 
-                       if (S_ISGITLINK(ce->ce_mode)) {
-                               found_submodules = 1;
-                               break;
-                       }
+                       if (!S_ISGITLINK(ce->ce_mode))
+                               continue;
+
+                       strbuf_reset(&path);
+                       strbuf_addf(&path, "%s/%s", wt->path, ce->name);
+                       if (!is_submodule_populated_gently(path.buf, &err))
+                               continue;
+
+                       found_submodules = 1;
+                       break;
                }
        }
        discard_index(&istate);
+       strbuf_release(&path);
 
        if (found_submodules)
                die(_("working trees containing submodules cannot be moved or removed"));

diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 33c033773367a135d4cb7eb23f9e9d3131197174..939d18d7286c1be1e58a698e9164fda8e24c654a 100755 (executable)
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -112,6 +112,26 @@ test_expect_success 'move locked worktree (force)' '
        git worktree move --force --force flump ploof
 '
 
+test_expect_success 'move a repo with uninitialized submodule' '
+       git init withsub &&
+       (
+               cd withsub &&
+               test_commit initial &&
+               git submodule add "$PWD"/.git sub &&
+               git commit -m withsub &&
+               git worktree add second HEAD &&
+               git worktree move second third
+       )
+'
+
+test_expect_success 'not move a repo with initialized submodule' '
+       (
+               cd withsub &&
+               git -C third submodule update &&
+               test_must_fail git worktree move third forth
+       )
+'
+
 test_expect_success 'remove main worktree' '
        test_must_fail git worktree remove .
 '
@@ -185,4 +205,21 @@ test_expect_success 'remove cleans up .git/worktrees when empty' '
        )
 '
 
+test_expect_success 'remove a repo with uninitialized submodule' '
+       (
+               cd withsub &&
+               git worktree add to-remove HEAD &&
+               git worktree remove to-remove
+       )
+'
+
+test_expect_success 'not remove a repo with initialized submodule' '
+       (
+               cd withsub &&
+               git worktree add to-remove HEAD &&
+               git -C to-remove submodule update &&
+               test_must_fail git worktree remove to-remove
+       )
+'
+
 test_done