Re: BUG in fetching non-checked out submodule

From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Dec 3, 2020, 15:25 UTC
Message-ID: <CADtb9DxTgEWfOF7jDGGt3eQSCaaqeiyJfS4V-e0SyPenE2SXWA@mail.gmail.com>
In-Reply-To: <04968f5c-c8bd-c57e-d646-7c9f7691e1a8@nokia.com>

Hello Peter,

Le jeu. 3 déc. 2020, à 02 h 54, Peter Kästle <peter.kaestle@nokia.com> a écrit :

Show 34 quoted lines

>
> Hi,
>
> On 03.12.20 00:06, Junio C Hamano wrote:
> > Philippe Blain <levraiphilippeblain@gmail.com> writes:
> >
> >> Thanks for bisecting it. That commit wanted to fix a different bug
> >> related to nested submodules, and the route taken was simply
> >> reverting an earlier commit (a62387b (submodule.c: fetch in
> >> submodules git directory instead of in worktree, 2018-11-28).
> >>
> >> As you discovered, it breaks other scenarios.
> >>
> >>>
> >>> $ git version
> >>> git version 2.29.2.435.g72ffeb997e
> >>>
> >>> $ git config --get submodule.recurse
> >>> true
> >
> > I think the current situation is probably worse.
> >
> > As a short-term fix, we should revert 1b7ac4e6d4 until we can come
> > up with a real fix, probably.
>
> Junio: This is why I originally intended to commit the test case for the
> testsuite separated from the revert and wanted to start a discussion
> about the actual real fix for the issue:
> https://public-inbox.org/git/1604413399-63090-1-git-send-email-peter.kaestle@nokia.com/
>
> My proposal would be to revert 1b7ac4e6d4 and isolate the test case
> "test_expect_success 'setup nested submodule fetch test' '" make it
> "test_expect_failure" and apply it instead, until we come up with a real
> solution.

I think I have the real solution. I did some debugging and I think it is quite easy: In 'get_next_submodule', 'get_submodule_repo_for(spf->r, task->sub)' fails to get a repo pointer for the submodule repository, since it is not initialized. That is normal. Then we go in the "else" branch, and hit this code:

/*
* An empty directory is normal,
* the submodule is not initialized
*/
if (S_ISGITLINK(ce->ce_mode) &&
!is_empty_dir(ce->name)) {

'is_empty_dir' receives ce->name, but the current working directory is the Git directory of 'middle', so clearly is_empty_dir returns false, as

/path/to/git/t/trash directory.t5526-fetch-submodules/B/.git/modules/middle/inner

is a non-existent path. The path that we should send is the worktree of inner, ie. the concatenation of spf->r->worktree and ce->name. This would give

/path/to/git/t/trash directory.t5526-fetch-submodules/B/middle/inner,

which is an empty directory since the inner submodule is not initialized, and we would not get the "Could not access submodule inner" error that you wanted to solve.

Cheers,

Philippe.