Re: [PATCH 1/1] commit-graph: fix writing first commit-graph during fetch

[Jeff King <peff@peff.net>]

On Tue, Oct 22, 2019 at 08:35:57PM -0400, Derrick Stolee wrote:

> > In the cover letter Derrick mentioned that he used
> > https://github.com/derrickstolee/numbers for testing, and that repo
> > has a submodule as well.
> 
> I completely forgot that I put a submodule in that repo. It makes sense
> that the Git repo also has one. Thanks for the test! I'll get it into
> the test suite tomorrow.

Ah, right. Good catch Gábor. The test below fails for me without your
patch, and succeeds with it (the extra empty commit is necessary so that
we have a parent).

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

I'd also wonder if it would be possible to trigger in another way (say,
due to want/have negotiation, though I guess most of the walking there
is done on the server side). But it may not be worth digging too far
into it.

-Peff

---
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index ecabbe1616..1b092fec0b 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -583,6 +583,21 @@ test_expect_success 'fetch.writeCommitGraph' '
 	)
 '
 
+test_expect_success 'fetch.writeCommitGraph with a submodule' '
+	git init has-submodule &&
+	(
+		cd has-submodule &&
+		git commit --allow-empty -m parent &&
+		git submodule add ../three &&
+		git commit -m "add submodule"
+	) &&
+	git clone has-submodule submod-clone &&
+	(
+		cd submod-clone &&
+		git -c fetch.writeCommitGraph fetch origin
+	)
+'
+
 # configured prune tests
 
 set_config_tristate () {