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