On Tue, Oct 22, 2019 at 08:48:20PM -0400, Jeff King wrote: > I admit I am puzzled, though, _why_ the presence of the submodule > matters. That is, from your explanation, I thought the issue was simply > that `fetch` walked (and marked) some commits, and the flags overlapped > with what the commit-graph code expected. > > I could guess that the presence of the submodule triggers some analysis > for --recurse-submodules. But then we don't actually recurse (maybe > because they're not activated? In which case maybe we shouldn't be doing > that extra walk to look for submodules if there aren't any activated > ones in our local repo).
Indeed, that seems to be it. If I do this: git init repo cd repo cat >.gitmodules <<\EOF [submodule "foo"] path = foo url = https://example.com EOF time git fetch /path/to/git.git then we end up traversing the whole git.git history a second time, even though we should know off the bat that there are no active submodules that we would recurse to. Doing this makes the problem go away: diff --git a/submodule.c b/submodule.c index 0f199c5137..0db2f18b93 100644 --- a/submodule.c +++ b/submodule.c @@ -1193,7 +1193,7 @@ static void calculate_changed_submodule_paths(struct repository *r, struct string_list_item *name; /* No need to check if there are no submodules configured */ - if (!submodule_from_path(r, NULL, NULL)) + if (!is_submodule_active(r, NULL)) return; argv_array_push(&argv, "--"); /* argv[0] program name */ but causes some tests to fail (I think that in some cases we're supposed to auto-initialize, and we'd probably need to cover that case, too). All of this is outside of your fix, of course, but: 1. I'm satisfied now that I understand why the test triggers the problem. 2. You may want have a real activated submodule in your test. Right now we'll trigger the submodule-recursion check even without that, but in the future we might do something like the hunk above. In which case your test wouldn't be checking anything interesting anymore. -Peff
