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

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

[Gary Oberbrunner]

I'm running git 2.24.1 on Linux/WSL, and sometimes git fails with this error:
```
% git diff --cached
fatal: invalid commit position. commit-graph is likely corrupt
```
Usually that error goes away after I do a few more git operations, but
I haven't been able to discern a pattern yet. It's not always with
"diff", can be any git command as far as I can tell.
Perhaps it started after I enabled the commit-graph stuff:
```
[core]
        editor = emacsclient -c -a \"\"
        preloadindex = true
        fscache = true
        autocrlf = false
        commitGraph = true
        writeCommitGraph = true
```
In fact, when I turn off `commitGraph`, the error goes away. But as I
say, sometimes it goes away of its own accord.

Googling turns up almost nothing on this error. I have no idea if I've
done something wrong to corrupt the commit graph, or if it's a git
bug. If there's anything I can do to help debug it, I'd be happy to --
just let me know.

-- 
Gary

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

[Derrick Stolee]

On 1/20/2020 12:32 PM, Gary Oberbrunner wrote:
> I'm running git 2.24.1 on Linux/WSL, and sometimes git fails with this error:
> ```
> % git diff --cached
> fatal: invalid commit position. commit-graph is likely corrupt
> ```
> Usually that error goes away after I do a few more git operations, but
> I haven't been able to discern a pattern yet. It's not always with
> "diff", can be any git command as far as I can tell.
> Perhaps it started after I enabled the commit-graph stuff:
> ```
> [core]
>         editor = emacsclient -c -a \"\"
>         preloadindex = true
>         fscache = true
>         autocrlf = false
>         commitGraph = true
>         writeCommitGraph = true
> ```
> In fact, when I turn off `commitGraph`, the error goes away. But as I
> say, sometimes it goes away of its own accord.

Disabling the featuere will definitely remove the error. Some commands may
also just not "visit" the corrupt commit.

> Googling turns up almost nothing on this error. I have no idea if I've
> done something wrong to corrupt the commit graph, or if it's a git
> bug. If there's anything I can do to help debug it, I'd be happy to --
> just let me know.

Please run "git commit-graph verify" to see if that catches the
corruption. If not, then the verify feature is broken.

Then, you can delete your .git/objects/info/commit-graph file
and rebuild it with "git commit-graph write". (If you have
fetch.writeCommitGraph enabled, then you may need to instead
delete the .git/objects/info/commit-graphs directory.) If that
process does not fix the problem, then there is definitely a bug
in the feature. At the moment, it may just be a flipped bit in
the file.

Thanks,
-Stolee

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

[Gary Oberbrunner]

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.

Showing the bug:
% git show --patch df2f
fatal: invalid commit position. commit-graph is likely corrupt
commit df2f311c78b87d16509f352c66d7309957740711 (origin/feature/renderer)
Author: [redacted]
Date: Mon Jan 20 12:37:01 2020 -0500

WIP: add outline of back-end render web service

Submodule packages/ngl f713ed5..7de8821:
%

Run "git commit-graph verify": doesn't detect any problem.
% git commit-graph verify
Verifying commits in commit graph: 100% (1/1), done.
Verifying commits in commit graph: 100% (2/2), done.
Verifying commits in commit graph: 100% (10/10), done.
Verifying commits in commit graph: 100% (14/14), done.
Verifying commits in commit graph: 100% (71/71), done.
Verifying commits in commit graph: 100% (478/478), done

You asked me to delete .git/objects/info/commit-graph -- I don't have
such a file, but  I do have a subdir "commit-graphs":
% ls .git/objects/info/commit-graphs/
commit-graph-chain graph-e96da2872d5c5fc1c4e399f99b7800d08fcdd273.graph
graph-4f5807bbdcba22f8fdc081c91205c40d40e99d38.graph
graph-e9f543a1389f8f5d6e91c09caa6658a1905d00fe.graph
graph-79f0f3bff07f7e7dda9d2438a30ef6483a2b531f.graph tmp_graph_GHI48b
graph-83ce268fda98e26938ce702bb261e1719ff816ba.graph tmp_graph_Ypgl2f
graph-93a966dc8be3040189367f047f5111e9f29c2d6c.graph

After removing that subdir and running "git commit-graph write", I
*do* have a .git/objects/info/commit-graph file.

The bug is still present though! However, I do have a git submodule
with its own .git/objects/info/commit-graphs; let's look into that.
In the git submodule dir, "git commit-graph verify" succeeds with no
errors (just like the parent).
Deleted the .git/objects/info/commit-graphs subdir, regenerated with
"git commit-graph write".
Retest original problem:
- Now the problem is fixed!

So long story short: the corrupted git commit-graph was in a
submodule. It does appear that "git commit-graph verify" failed to
detect the corruption. Removing and regenerating fixes it.

I do have the old git submodule's commit-graphs data in case it's
useful. Let me know and I can send it along.


On Mon, Jan 20, 2020 at 7:37 PM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 1/20/2020 12:32 PM, Gary Oberbrunner wrote:
> > I'm running git 2.24.1 on Linux/WSL, and sometimes git fails with this error:
> > ```
> > % git diff --cached
> > fatal: invalid commit position. commit-graph is likely corrupt
> > ```
> > Usually that error goes away after I do a few more git operations, but
> > I haven't been able to discern a pattern yet. It's not always with
> > "diff", can be any git command as far as I can tell.
> > Perhaps it started after I enabled the commit-graph stuff:
> > ```
> > [core]
> >         editor = emacsclient -c -a \"\"
> >         preloadindex = true
> >         fscache = true
> >         autocrlf = false
> >         commitGraph = true
> >         writeCommitGraph = true
> > ```
> > In fact, when I turn off `commitGraph`, the error goes away. But as I
> > say, sometimes it goes away of its own accord.
>
> Disabling the featuere will definitely remove the error. Some commands may
> also just not "visit" the corrupt commit.
>
> > Googling turns up almost nothing on this error. I have no idea if I've
> > done something wrong to corrupt the commit graph, or if it's a git
> > bug. If there's anything I can do to help debug it, I'd be happy to --
> > just let me know.
>
> Please run "git commit-graph verify" to see if that catches the
> corruption. If not, then the verify feature is broken.
>
> Then, you can delete your .git/objects/info/commit-graph file
> and rebuild it with "git commit-graph write". (If you have
> fetch.writeCommitGraph enabled, then you may need to instead
> delete the .git/objects/info/commit-graphs directory.) If that
> process does not fix the problem, then there is definitely a bug
> in the feature. At the moment, it may just be a flipped bit in
> the file.
>
> Thanks,
> -Stolee



-- 
Gary

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

[Alex Riesen]

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.

Regards,
Alex

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

[Michael Forney]

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.

-Michael

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

[Derrick Stolee]

On 6/21/2020 4:45 PM, Michael Forney wrote:
> 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