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