Re: [PATCH v3 4/8] commit-graph: return generation from memory

[Jonathan Tan <jonathantanmy@google.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Derrick Stolee <derrickstolee@github.com>
> 
> The commit_graph_generation() method used to report a value of
> GENERATION_NUMBER_INFINITY if the commit_graph_data_slab had an instance
> for the given commit but the graph_pos indicated the commit was not in
> the commit-graph file.
> 
> Instead, trust the 'generation' member if the commit has a value in the
> slab _and_ the 'generation' member is non-zero. Otherwise, treat it as
> GENERATION_NUMBER_INFINITY.

I would replace "Instead" with "However, a future commit intends to
compute and use commit generation numbers even for commits that are
not in the commit-graph file (and thus have no graph_pos). Therefore,
we need a new criterion for deciding if a generation number can be
trusted:" (or something to that effect).

> This only makes a difference for a very old case for the commit-graph:
> the very first Git release to write commit-graph files wrote zeroes in
> the topological level positions. If we are parsing a commit-graph with
> all zeroes, those commits will now appear to have
> GENERATION_NUMBER_INFINITY (as if they were not parsed from the
> commit-graph).
> 
> I attempted several variations to work around the need for providing an
> uninitialized 'generation' member, but this was the best one I found. It
> does require a change to a verification test in t5318 because it reports
> a different error than the one about non-zero generation numbers.

Thanks for investigating, and I think the method in this patch would
work. As you have stated, this only affects the commit-graph files that
once upon a time were written with no generation numbers, and this patch
makes those behave as if there were no generation numbers in the first
place (which is exactly what happened).