Skip to content

Commit

Permalink
Merge branch 'sb/submodule-unset-core-worktree-when-worktree-is-lost'
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gitster committed Jan 18, 2019
2 parents 1ed943e + 8eda5ef commit 3942920
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 4 deletions.
4 changes: 3 additions & 1 deletion builtin/submodule--helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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];

Expand Down
14 changes: 14 additions & 0 deletions submodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions submodule.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions t/lib-submodule-update.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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" &&
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions t/t7400-submodule-basic.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
7 changes: 6 additions & 1 deletion t/t7412-submodule-absorbgitdirs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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' '
Expand Down

0 comments on commit 3942920

Please sign in to comment.