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
  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))
        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

  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


Reply via email to