Merge branch 'sb/submodule-unset-core-worktree-when-worktree-is-lost'

The core.worktree setting in a submodule repository should not be
pointing at a directory when the submodule loses its working tree
(e.g. getting deinit'ed), but the code did not properly maintain
this invariant.

* sb/submodule-unset-core-worktree-when-worktree-is-lost:
submodule deinit: unset core.worktree
submodule--helper: fix BUG message in ensure_core_worktree
submodule: unset core.worktree if no working tree is present
submodule update: add regression test with old style setups

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0c5b2eb5fc6bb7553b71dac5f16d04e312093ae0..6881b6a9cb0fa1cd44118663cd8d7c8eaef1c95a 100644 (file)
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1131,6 +1131,8 @@ static void deinit_submodule(const char *path, const char *prefix,
                if (!(flags & OPT_QUIET))
                        printf(format, displaypath);
 
+               submodule_unset_core_worktree(sub);
+
                strbuf_release(&sb_rm);
        }
 
@@ -2046,7 +2048,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
        struct repository subrepo;
 
        if (argc != 2)
-               BUG("submodule--helper connect-gitdir-workingtree <name> <path>");
+               BUG("submodule--helper ensure-core-worktree <path>");
 
        path = argv[1];
 

diff --git a/submodule.c b/submodule.c
index 6415cc55807c7ed9ee4cbcaa57f41a5804b6e854..d393e947e699bb1d5764f42e94919218496a138d 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -1561,6 +1561,18 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
        return ret;
 }
 
+void submodule_unset_core_worktree(const struct submodule *sub)
+{
+       char *config_path = xstrfmt("%s/modules/%s/config",
+                                   get_git_common_dir(), sub->name);
+
+       if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
+               warning(_("Could not unset core.worktree setting in submodule '%s'"),
+                         sub->path);
+
+       free(config_path);
+}
+
 static const char *get_super_prefix_or_empty(void)
 {
        const char *s = get_super_prefix();
@@ -1726,6 +1738,8 @@ int submodule_move_head(const char *path,
 
                        if (is_empty_dir(path))
                                rmdir_or_warn(path);
+
+                       submodule_unset_core_worktree(sub);
                }
        }
 out:

diff --git a/submodule.h b/submodule.h
index a680214c01a5fda90a21af547389f6333388683c..9e18e9b80760ad1562682125bc90d08fb6e1c0d3 100644 (file)
--- a/submodule.h
+++ b/submodule.h
@@ -131,6 +131,8 @@ int submodule_move_head(const char *path,
                        const char *new_head,
                        unsigned flags);
 
+void submodule_unset_core_worktree(const struct submodule *sub);
+
 /*
  * Prepare the "env_array" parameter of a "struct child_process" for executing
  * a submodule by clearing any repo-specific environment variables, but

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 016391723c09491e53ebf4e0b69972783c7bc491..5b56b23166bb3dea2e89cea38a19eb5698dfff53 100755 (executable)
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -235,7 +235,7 @@ reset_work_tree_to_interested () {
        then
                mkdir -p submodule_update/.git/modules/sub1/modules &&
                cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
-               GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
+               # core.worktree is unset for sub2 as it is not checked out
        fi &&
        # indicate we are interested in the submodule:
        git -C submodule_update config submodule.sub1.url "bogus" &&
@@ -709,7 +709,8 @@ test_submodule_recursing_with_args_common() {
                        git branch -t remove_sub1 origin/remove_sub1 &&
                        $command remove_sub1 &&
                        test_superproject_content origin/remove_sub1 &&
-                       ! test -e sub1
+                       ! test -e sub1 &&
+                       test_must_fail git config -f .git/modules/sub1/config core.worktree
                )
        '
        # ... absorbing a .git directory along the way.

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 76a7cb0af7955aa618556a43cc9f2a568d8de3e8..aba2d4d6ee415ab5528ed67d518cfb26412f1142 100755 (executable)
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -984,6 +984,11 @@ test_expect_success 'submodule deinit should remove the whole submodule section
        rmdir init
 '
 
+test_expect_success 'submodule deinit should unset core.worktree' '
+       test_path_is_file .git/modules/example/config &&
+       test_must_fail git config -f .git/modules/example/config core.worktree
+'
+
 test_expect_success 'submodule deinit from subdirectory' '
        git submodule update --init &&
        git config submodule.example.foo bar &&

diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
index ce74c12da2daccd26edbb92070e5ff4eb1f09f9c..1cfa150768d7f813ba9943b4bd17887469cb1c04 100755 (executable)
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -75,7 +75,12 @@ test_expect_success 're-setup nested submodule' '
        GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
                core.worktree "../../../nested" &&
        # make sure this re-setup is correct
-       git status --ignore-submodules=none
+       git status --ignore-submodules=none &&
+
+       # also make sure this old setup does not regress
+       git submodule update --init --recursive >out 2>err &&
+       test_must_be_empty out &&
+       test_must_be_empty err
 '
 
 test_expect_success 'absorb the git dir in a nested submodule' '