Re: [PATCH 3/7] commit-graph: start parsing generation v2 (again)

[Derrick Stolee <derrickstolee@github.com>]

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@github.com>
...
>> 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