[PATCH] submodules: fix of regression on fetching of non-init subsub-repo
- From
- Peter Kaestle <peter.kaestle@nokia.com>
- Date
- Dec 4, 2020, 15:23 UTC
- Message-ID
- <1607095412-40109-1-git-send-email-peter.kaestle@nokia.com>
- In-Reply-To
- <0b6a34a0-428e-5fc4-307d-1217b112659c@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 remote repository with a subrepository inside a subrepository 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" on 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 subrepository 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.
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> --- submodule.c | 7 ++- t/t5526-fetch-submodules.sh | 94 +++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-)
diff --git a/submodule.c b/submodule.c index b3bb59f066..b561445329 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 dd8e423d25..04eb587862 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -719,4 +719,98 @@ 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 -f B/file && + test -f B/middle/file && + ! test -f 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/master B origin/master +' + + +test_expect_success 'setup recursive fetch with uninit submodule' ' + # does not depend on any previous test setups + + git init main && + git init sub && + + touch sub/file && + git -C sub add file && + git -C sub commit -m "add file" && + git -C sub rev-parse HEAD >expect && + + git -C main submodule add ../sub && + git -C main submodule init && + git -C main submodule update --checkout && + git -C main submodule status | + sed -e "s/^ //" -e "s/ sub .*$//" >actual && + test_cmp expect actual +' + +test_expect_success 'recursive fetch with uninit submodule' ' + # depends on previous test for setup + + git -C main submodule deinit -f sub && + ! git -C main fetch --recurse-submodules |& + grep -v -m1 "Fetching submodule sub$" && + git -C main submodule status | + sed -e "s/^-//" -e "s/ sub$//" >actual && + test_cmp expect actual +' + test_done
-- 2.29.2