Hi Philippe,
On 03.12.20 16:25, Philippe Blain wrote:
On 03.12.20 00:06, Junio C Hamano wrote:
Philippe Blain <levraiphilippeblain@xxxxxxxxx> 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@xxxxxxxxx/
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.
On quick glance this sounds plausible, but to fully understand it I need
to put some effort in reading this code again. I hope to do so
tomorrow. We can then compile a new set of patches including this real
fix and Ralf's and my test case.
Thanks for digging into it.
--
kind regards
--peter;
