On 7/12/2022 7:10 PM, Taylor Blau wrote: > When upgrading a commit-graph using generation v1 to one using > generation v2, it is possible to force Git into a corrupt state where it > (incorrectly) believes that a GDO2 chunk is necessary, *after* deciding > not to write one. > > This makes subsequent reads using the commit-graph produce the following > error message: > > fatal: commit-graph requires overflow generation data but has none > > Demonstrate this bug by increasing our test coverage to include a > minimal example of upgrading a commit-graph from generation v1 to v2. > The only notable components of this test are: > > - The committer date of the commit is chosen carefully so that the > offset underflows when computed using a v1 generation number, but > would not overflow when using v2 generation numbers. > > - The upgrade to generation number v2 must read in the v1 generation > numbers, which we can do by passing `--changed-paths`, which will > force the commit-graph internals to call `fill_commit_graph_info()`. > > A future patch will squash this bug. Thanks for finding a good test. > + # This commit will have a date at two seconds past the Epoch, > + # and a (v1) generation number of 1, since it is a root commit. > + # > + # The offset will then be computed as 2-1, which will underflow I have verified that your test works, but this explanation is confusing me. "2 - 1" is 1, which does not underflow. There must be something else going on. Looking ahead, you describe the situation correctly in Patch 3 to show that we take "generation - date", so you really just need s/2-1/1-2/ here. > + # to 2^31, which is greater than the v2 offset small limit of > + # 2^31-1. > + # > + # This is sufficient to need a large offset table for the v2 > + # generation numbers. > + test_commit --date "@2 +0000" base && > + git repack -d && > + > + # Test that upgrading from generation v1 to v2 correctly > + # produces the overflow table. > + git -c commitGraph.generationVersion=1 commit-graph write && > + git -c commitGraph.generationVersion=2 commit-graph write \ > + --changed-paths && Simple and fast to set up and test. Thanks for using the config explicitly in both commands so it is robust to possible default changes in the future. Thanks, -Stolee
