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

Re: Possible git bug in commit-graph: "invalid commit position"

From
Derrick Stolee <stolee@gmail.com>
Date
Jun 22, 2020, 13:45 UTC
Message-ID
<0f03151c-d8ec-effc-a8f9-c4a3fc1325c7@gmail.com>
In-Reply-To
<CAGw6cBuEshq18O_PrrGYuJi5VZ82XK3T9KuShneUqO2Ju0jtHw@mail.gmail.com>
On 6/21/2020 4:45 PM, Michael Forney wrote:
Show 80 quoted lines
> On 2020-05-15, Alex Riesen <alexander.riesen@cetitec.com> wrote:
>> Gary Oberbrunner, Tue, Feb 04, 2020 23:33:42 +0100:
>>> Sorry for the long reply delay; the bug went away and only just showed
>>> up again. Here's the info you requested.
>>> I'm now running git 2.25.0.
>>
>> I hit a very similar problem today with 2.26.0. Also in a submodule.
>>
>> Removing and regenerating the commit graph did not help and I did not have
>> the
>> commit-graphs directory (only a file). "git commit-graph verify" does not
>> find
>> anything. Switching writeCommitGraph on and regenerating the commit graph
>> makes no difference.
>>
>> I can trigger it reliably by visiting the broken(?) commit in supermodule
>> with:
>>
>>     git show --submodule=log <commit>
>>
>> I see nothing special in the commit invovled. It is just a single commit in
>> my
>> case, and the commit is a merge of two branches.
> 
> I hit this bug a while back, and it went away after I deleted the
> commit-graph in the submodule and regenerated it (IIRC).
> 
> I just ran into it again (on 2.27.0), and this time, I did some digging.
> 
> I have a repository containing a number of submodules, and the bug
> appeared after I updated one of the submodules, and then looked at
> `git log -p` with diff.submodule = log. Just like Alex, I can reliably
> trigger the error with `git show --submodule=log <commit>`.
> 
> I rebuilt git with some print statements to try to see what's going
> on, and got the following:
> 
> 	/src/oasis/.git/modules/pkg/file/src c81d1ccbf4c224af50e6d556419961dba72666c7
> 		pos: 4986, num_commits: 6452, num_commits_in_base: 0
> 	/src/oasis/.git/modules/pkg/file/src 9f2f793847c6aeab9501287b6847dc842c84630f
> 		pos: 3964, num_commits: 6452, num_commits_in_base: 0
> 	/src/oasis/.git/modules/pkg/file/src fd7eb1f793944635b92bfa56a84a4dc1dbefb119
> 		pos: 6383, num_commits: 6452, num_commits_in_base: 0
> 	/src/oasis/.git/modules/pkg/file/src d955cefc956ba537cfc0556023a65fe80bd2d82b
> 		pos: 5436, num_commits: 6452, num_commits_in_base: 0
> 	/src/oasis/.git/modules/pkg/file/src 0c79c693d6a86f7ad7ada2a9a1eb3bdf483f77cc
> 		pos: 301, num_commits: 6452, num_commits_in_base: 0
> 	.git fa09b87efa9b9664e4e53ab768cfa5f51a6c6fa2
> 		pos: 6292, num_commits: 5177, num_commits_in_base: 0
> 	fatal: invalid commit position. commit-graph is likely corrupt
> 
> Using `git commit-graph verify`, I confirmed that the main
> repository's commit graph contains 5177 commits, and the submodule
> repository's commit-graph contains 6452 commits. Commit fa09b8 is part
> of the submodule, not the main repository, so it makes sense that it
> is an invalid commit for the main repositories commit-graph.
> 
> So, this seems a little fishy. fill_commit_in_graph is getting called
> with the main repository and a commit belonging to the submodule.
> Looking through the call stack in gdb, I see that the initial calls to
> fill_commit_in_graph come from show_submodule_header, which computes
> left, right, and merge_bases. Then, those commits are passed to
> prepare_submodule_summary, but this function does *not* accept a
> submodule parameter. prepare_submodule_summary calls
> repo_init_revisions with the_repository, which seems to be the source
> of the problem. I think it should be using the submodule repository
> instead.
> 
> I changed prepare_submodule_summary to accept a repository and to use
> that instead, but the issue persisted. Digging deeper, this is because
> revision.c:process_parents uses parse_commit_gently, which is a
> synonym for repo_parse_commit_gently(the_repository, ...). I changed
> it to use repo_parse_commit_gently(revs->repo, ...), and this time,
> the problem went away.
> 
> I'm not very familiar with the git codebase, but am I on the right
> track here? I also noticed a number of other calls to
> parse_commit_gently in revision.c, and I think those should pass
> revs->repo as well. Does that sound right? If so, I can send a patch
> to fix these issues.

This is some good digging, and I think you are absolutely correct with the root cause. The dependence on the_repository is still something that is being worked on in the Git codebase (but less frequently lately) and trips up submodule things like this.

I think a simple method swap would be a good patch to send, and you can include many of the details above in the commit message. Is that a contribution you have time to make? I'll gladly review it.

Thanks, -Stolee

Previous: Michael Forney
Message 6 of 6 in “Fwd: Possible git bug in commit-graph: "invalid commit position"”
  1. Gary OberbrunnerJan 20, 2020
  2. Derrick StoleeJan 21, 2020
  3. Gary OberbrunnerFeb 4, 2020
  4. Alex RiesenMay 15, 2020
  5. Michael ForneyJun 21, 2020
  6. Derrick StoleeJun 22, 2020

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.