On 2/28/2022 10:18 AM, Patrick Steinhardt wrote:
> On Thu, Feb 24, 2022 at 08:38:32PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
...
>> diff --git a/commit-graph.c b/commit-graph.c
>> index a19bd96c2ee..8e52bb09552 100644
>> --- a/commit-graph.c
>> +++ b/commit-graph.c
>> @@ -407,6 +407,9 @@ struct commit_graph *parse_commit_graph(struct repository *r,
>> &graph->chunk_generation_data);
>> pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW,
>> &graph->chunk_generation_data_overflow);
>> +
>> + if (graph->chunk_generation_data)
>> + graph->read_generation_data = 1;
>> }
>>
>> if (r->settings.commit_graph_read_changed_paths) {
>
> I wanted to test your changes because they seem quite exciting in the
> context of my work as well, but this commit seems to uncover a bug with
> how we handle overflows. I originally triggered the bug when trying to
> do a mirror-fetch, but as it turns it seems to trigger now whenever the
> commit-graph is being read:
>
> $ git commit-graph verify
> fatal: commit-graph requires overflow generation data but has none
>
> $ git commit-graph write --split
> Finding commits for commit graph among packed objects: 100% (10235119/10235119), done.
> fatal: commit-graph requires overflow generation data but has none
>
> $ git commit-graph write --split=replace
> Finding commits for commit graph among packed objects: 100% (10235119/10235119), done.
> fatal: commit-graph requires overflow generation data but has none
>
> I initially assumed this may be a bug with how we previously wrote the
> commit-graph, but removing all chains still reliably triggers it:
>
> $ rm -f objects/info/commit-graphs/*
> $ git commit-graph write --split
> Finding commits for commit graph among packed objects: 100% (10235119/10235119), done.
> fatal: commit-graph requires overflow generation data but has none
>
> I haven't yet found the time to dig deeper into why this is happening.
> While the repository is publicly accessible at [1], unfortunately the
> bug seems to be triggered by a commit that's only kept alive by an
> internal reference.
>
> Patrick
>
> [1]: https://gitlab.com/gitlab-com/www-gitlab-com.git
Thanks for including this information. Just to be clear: did you
include patch 4 in your tests, or not? Patch 4 includes a fix
related to overflow values, so it would be helpful to know if you
found a _different_ bug or if it is the same one.
Thanks,
-Stolee
