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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 3041 bytes --]

On Mon, Feb 28, 2022 at 11:23:38AM -0500, Derrick Stolee wrote:
> 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

I initially only applied the first three patches, but after having hit
the fatal error I also applied the rest of this series to have a look at
whether it is indeed fixed already by one of your later patches. The
error remains the same though.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]