On 01/24, Stefan Beller wrote:
> + if (read_gitfile_gently(old_git_dir, &err_code) ||
> + err_code == READ_GITFILE_ERR_NOT_A_REPO) {
> + /*
> + * If it is an actual gitfile, it doesn't need migration,
> + * however in case of a recursively nested submodule, the
> + * gitfile content may be stale, as its superproject
> + * (which may be a submodule of another superproject)
> + * may have been moved. So expect a bogus pointer to be read,
> + * which materializes as error READ_GITFILE_ERR_NOT_A_REPO.
> + */
> + connect_work_tree_and_git_dir(path, real_new_git_dir);
So connect_work_tree_and_git_dir() will update the .gitfile if it is
stale.
> + return;
> + }
> +
> + if (submodule_uses_worktrees(path))
> + die(_("relocate_gitdir for submodule '%s' with "
> + "more than one worktree not supported"), path);
No current support for worktrees (yet!).
> +
> if (!prefix)
> prefix = get_super_prefix();
>
> @@ -1437,22 +1448,14 @@ void absorb_git_dir_into_superproject(const char *prefix,
> const char *path,
> unsigned flags)
> {
> - const char *sub_git_dir, *v;
> - char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
> struct strbuf gitdir = STRBUF_INIT;
> -
> strbuf_addf(&gitdir, "%s/.git", path);
> - sub_git_dir = resolve_gitdir(gitdir.buf);
>
> /* Not populated? */
> - if (!sub_git_dir)
> + if (!file_exists(gitdir.buf))
> goto out;
There should be a is_submodule_populated() function now, maybe
we should start using it when performing population checks?
>
> - /* Is it already absorbed into the superprojects git dir? */
> - real_sub_git_dir = real_pathdup(sub_git_dir);
> - real_common_git_dir = real_pathdup(get_git_common_dir());
> - if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
> - relocate_single_git_dir_into_superproject(prefix, path);
> + relocate_single_git_dir_into_superproject(prefix, path);
So the check was just pushed into the relocation function.
>
> if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
> struct child_process cp = CHILD_PROCESS_INIT;
> @@ -1481,6 +1484,4 @@ void absorb_git_dir_into_superproject(const char *prefix,
>
> out:
> strbuf_release(&gitdir);
> - free(real_sub_git_dir);
> - free(real_common_git_dir);
> }
> diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
> index 1c47780e2b..e2bbb449b6 100755
> --- a/t/t7412-submodule-absorbgitdirs.sh
> +++ b/t/t7412-submodule-absorbgitdirs.sh
> @@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested submodule' '
> test_cmp expect.2 actual.2
> '
>
> +test_expect_success 're-setup nested submodule' '
> + # un-absorb the direct submodule, to test if the nested submodule
> + # is still correct (needs a rewrite of the gitfile only)
> + rm -rf sub1/.git &&
> + mv .git/modules/sub1 sub1/.git &&
> + GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
> + # fixup the nested submodule
> + echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
> + 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
> +'
> +
> +test_expect_success 'absorb the git dir in a nested submodule' '
> + git status >expect.1 &&
> + git -C sub1/nested rev-parse HEAD >expect.2 &&
> + git submodule absorbgitdirs &&
> + test -f sub1/.git &&
> + test -f sub1/nested/.git &&
> + test -d .git/modules/sub1/modules/nested &&
> + git status >actual.1 &&
> + git -C sub1/nested rev-parse HEAD >actual.2 &&
> + test_cmp expect.1 actual.1 &&
> + test_cmp expect.2 actual.2
> +'
> +
> test_expect_success 'setup a gitlink with missing .gitmodules entry' '
> git init sub2 &&
> test_commit -C sub2 first &&
> --
> 2.11.0.486.g67830dbe1c
Aside from my one question the rest of this looks good to me.
--
Brandon Williams