From: Peter Kästle <peter.kaestle@nokia.com>
Date: Tue, 08 Dec 2020 15:51:07 GMT
Subject: Re: [PATCH v3] submodules: fix of regression on fetching of non-init subsub-repo
Message-ID: <2da143e1-d186-e87d-87e6-134c60f4428b@nokia.com>
In-Reply-To: <1607442126-34705-1-git-send-email-peter.kaestle@nokia.com>

On 08.12.20 16:42, Peter Kaestle wrote:
> 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_repo
> 
> Expectation 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

[...]


These are my test run outputs:
final_fix [PATCH v3]
==========
# passed all 41 test(s)


recursion_regression [1b7ac4e6d4]
==========
not ok 41 - recursive fetch after deinit a submodule


no_fix [3a0b884c - origin/master]
==========
not ok 38 - fetching a superproject containing an uninitialized sub/sub 
project


-- 
kind regards
--peter;