git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jeff King <peff@peff.net>
Date
Oct 23, 2019, 00:48 UTC
Message-ID
<20191023004820.GA19611@sigill.intra.peff.net>
In-Reply-To
<b7473363-b257-c00b-7338-a7e1d51bb01b@gmail.com>
On Tue, Oct 22, 2019 at 08:35:57PM -0400, Derrick Stolee wrote:
Show 7 quoted lines
> > 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 () {
Previous: Derrick StoleeNext: Jeff King
Message 7 of 23 in “[v2.24.0-rc0 BUG] fetch.writeCommitGraph fails on first fetch”
  1. 0/1 [v2.24.0-rc0 BUG] fetch.writeCommitGraph fails on first fetchDerrick Stolee via GitGitGadget, Oct 22, 2019
  2. 1/1 commit-graph: fix writing first commit-graph during fetchDerrick Stolee via GitGitGadget, Oct 22, 2019
  3. Jeff KingOct 22, 2019
  4. Jeff KingOct 22, 2019
  5. SZEDER GáborOct 22, 2019
  6. Derrick StoleeOct 23, 2019
  7. Jeff KingOct 23, 2019
  8. Jeff KingOct 23, 2019
  9. 0/2 [v2.24.0-rc0 BUG] fetch.writeCommitGraph fails on first fetchDerrick Stolee via GitGitGadget, Oct 23, 2019
  10. 1/2 t5510-fetch.sh: demonstrate fetch.writeCommitGraph bugDerrick Stolee via GitGitGadget, Oct 23, 2019
  11. SZEDER GáborOct 23, 2019
  12. Derrick StoleeOct 23, 2019
  13. SZEDER GáborOct 24, 2019
  14. 2/2 commit-graph: fix writing first commit-graph during fetchDerrick Stolee via GitGitGadget, Oct 23, 2019
  15. SZEDER GáborOct 23, 2019
  16. Derrick StoleeOct 24, 2019
  17. SZEDER GáborOct 30, 2019
  18. 0/2 [v2.24.0-rc0 BUG] fetch.writeCommitGraph fails on first fetchDerrick Stolee via GitGitGadget, Oct 24, 2019
  19. 1/2 t5510-fetch.sh: demonstrate fetch.writeCommitGraph bugDerrick Stolee via GitGitGadget, Oct 24, 2019
  20. 2/2 commit-graph: fix writing first commit-graph during fetchDerrick Stolee via GitGitGadget, Oct 24, 2019
  21. 0/2 [v2.24.0-rc0 BUG] fetch.writeCommitGraph fails on first fetchDerrick Stolee via GitGitGadget, Oct 24, 2019
  22. 1/2 t5510-fetch.sh: demonstrate fetch.writeCommitGraph bugDerrick Stolee via GitGitGadget, Oct 24, 2019
  23. 2/2 commit-graph: fix writing first commit-graph during fetchDerrick Stolee via GitGitGadget, Oct 24, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.