[PATCH v3] submodules: fix of regression on fetching of non-init subsub-repo
- From
- Peter Kaestle <peter.kaestle@nokia.com>
- Date
- Dec 8, 2020, 15:42 UTC
- Message-ID
- <1607442126-34705-1-git-send-email-peter.kaestle@nokia.com>
- In-Reply-To
- <d52db91d-af6d-c93c-2c4a-e2460905623d@nokia.com>
A regression has been introduced by a62387b (submodule.c: fetch in submodules git directory instead of in worktree, 2018-11-28).
The scenario in which it triggers is when one has a repository with a submodule inside a submodule like this: superproject/middle_repo/inner_repo
Person A and B have both a clone of it, while Person B is not working with the inner_repo and thus does not have it initialized in his working copy.
Now person A introduces a change to the inner_repo and propagates it through the middle_repo and the superproject.
Once person A pushed the changes and person B wants to fetch them using "git fetch" at the superproject level, B's git call will return with error saying:
Could not access submodule 'inner_repo'
Errors during submodule fetch:
middle_repoExpectation is that in this case the inner submodule will be recognized as uninitialized submodule and skipped by the git fetch command.
This used to work correctly before 'a62387b (submodule.c: fetch in submodules git directory instead of in worktree, 2018-11-28)'.
Starting with a62387b the code wants to evaluate "is_empty_dir()" inside .git/modules for a directory only existing in the worktree, delivering then of course wrong return value.
This patch ensures is_empty_dir() is getting the correct path of the uninitialized submodule by concatenation of the actual worktree and the name of the uninitialized submodule.
Furthermore a regression test case is added, which tests for recursive fetches on a superproject with uninitialized sub repositories. This issue was leading to an infinite loop when doing a revert of a62387b.
The first attempt to fix this regression, in 1b7ac4e6d4 (submodules: fix of regression on fetching of non-init subsub-repo, 2020-11-12), by simply reverting a62387b, resulted in an infinite loop of submodule fetches in the simpler case of a recursive fetch of a superproject with uninitialized submodules, and so this commit was reverted in 7091499bc0 (Revert "submodules: fix of regression on fetching of non-init subsub-repo", 2020-12-02). To prevent future breakages, also add a regression test for this scenario.
Signed-off-by: Peter Kaestle <peter.kaestle@nokia.com> CC: Junio C Hamano <gitster@pobox.com> CC: Philippe Blain <levraiphilippeblain@gmail.com> CC: Ralf Thielow <ralf.thielow@gmail.com> CC: Philippe Blain <levraiphilippeblain@gmail.com> --- submodule.c | 7 ++- t/t5526-fetch-submodules.sh | 124 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 1 deletion(-)
diff --git a/submodule.c b/submodule.c index b3bb59f..b561445 100644 --- a/submodule.c +++ b/submodule.c @@ -1477,6 +1477,7 @@ static int get_next_submodule(struct child_process *cp, strbuf_release(&submodule_prefix); return 1; } else { + struct strbuf empty_submodule_path = STRBUF_INIT; fetch_task_release(task); free(task); @@ -1485,13 +1486,17 @@ static int get_next_submodule(struct child_process *cp, * An empty directory is normal, * the submodule is not initialized */ + strbuf_addf(&empty_submodule_path, "%s/%s/", + spf->r->worktree, + ce->name); if (S_ISGITLINK(ce->ce_mode) && - !is_empty_dir(ce->name)) { + !is_empty_dir(empty_submodule_path.buf)) { spf->result = 1; strbuf_addf(err, _("Could not access submodule '%s'\n"), ce->name); } + strbuf_release(&empty_submodule_path); } } diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index dd8e423..495348a 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -719,4 +719,128 @@ test_expect_success 'fetch new submodule commit intermittently referenced by sup ) ' +add_commit_push () { + dir="$1" && + msg="$2" && + shift 2 && + git -C "$dir" add "$@" && + git -C "$dir" commit -a -m "$msg" && + git -C "$dir" push +} + +compare_refs_in_dir () { + fail= && + if test "x$1" = 'x!' + then + fail='!' && + shift + fi && + git -C "$1" rev-parse --verify "$2" >expect && + git -C "$3" rev-parse --verify "$4" >actual && + eval $fail test_cmp expect actual +} + + +test_expect_success 'setup nested submodule fetch test' ' + # does not depend on any previous test setups + + for repo in outer middle inner + do + git init --bare $repo && + git clone $repo ${repo}_content && + echo "$repo" >"${repo}_content/file" && + add_commit_push ${repo}_content "initial" file || + return 1 + done && + + git clone outer A && + git -C A submodule add "$pwd/middle" && + git -C A/middle/ submodule add "$pwd/inner" && + add_commit_push A/middle/ "adding inner sub" .gitmodules inner && + add_commit_push A/ "adding middle sub" .gitmodules middle && + + git clone outer B && + git -C B/ submodule update --init middle && + + compare_refs_in_dir A HEAD B HEAD && + compare_refs_in_dir A/middle HEAD B/middle HEAD && + test_path_is_file B/file && + test_path_is_file B/middle/file && + test_path_is_missing B/middle/inner/file && + + echo "change on inner repo of A" >"A/middle/inner/file" && + add_commit_push A/middle/inner "change on inner" file && + add_commit_push A/middle "change on inner" inner && + add_commit_push A "change on inner" middle +' + +test_expect_success 'fetching a superproject containing an uninitialized sub/sub project' ' + # depends on previous test for setup + + git -C B/ fetch && + compare_refs_in_dir A origin/HEAD B origin/HEAD +' + +fetch_with_recusion_abort () { + # In a regression the following git call will run into infinite recursion. + # To handle that, we connect the sed command to the git call by a pipe + # so that sed can kill the infinite recusion when detected. + # The recursion creates git output like: + # Fetching submodule sub + # Fetching submodule sub/sub <-- [1] + # Fetching submodule sub/sub/sub + # ... + # [1] sed will stop reading and cause git to eventually stop and die + + git -C "$1" fetch --recurse-submodules 2>&1 | + sed "/Fetching submodule $2[^$]/q" >out && + ! grep "Fetching submodule $2[^$]" out +} + +test_expect_success 'setup recursive fetch with uninit submodule' ' + # does not depend on any previous test setups + + # setup a remote superproject to make git fetch work with an uninit submodule + git init --bare super_bare && + git clone super_bare super && + git init sub && + + >sub/file && + git -C sub add file && + git -C sub commit -m "add file" && + git -C sub rev-parse HEAD >expect && + + # adding submodule without cloning + echo "[submodule \"sub\"]" >super/.gitmodules && + echo "path = sub" >>super/.gitmodules && + echo "url = ../sub" >>super/.gitmodules && + git -C super update-index --add --cacheinfo 160000 $(cat expect) sub && + mkdir super/sub && + + git -C super submodule status >out && + sed -e "s/^-//" -e "s/ sub.*$//" out >actual && + test_cmp expect actual +' + +test_expect_success 'recursive fetch with uninit submodule' ' + # depends on previous test for setup + + fetch_with_recusion_abort super sub && + git -C super submodule status >out && + sed -e "s/^-//" -e "s/ sub$//" out >actual && + test_cmp expect actual +' + +test_expect_success 'recursive fetch after deinit a submodule' ' + # depends on previous test for setup + + git -C super submodule update --init sub && + git -C super submodule deinit -f sub && + + fetch_with_recusion_abort super sub && + git -C super submodule status >out && + sed -e "s/^-//" -e "s/ sub$//" out >actual && + test_cmp expect actual +' + test_done
-- 2.6.2