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

[Derrick Stolee <derrickstolee@github.com>]

On 3/2/2022 1:15 PM, Junio C Hamano wrote:
> Derrick Stolee <derrickstolee@github.com> writes:
> 
>> Since our repro relies on private information, but is consistent, I
>> wonder if we should take the patch below, which starts to ignore the
>> older generation number v2 data and only writes freshly-computed
>> numbers.
> 
> ;-)
> 
>> Clearly, there is something else going on. The situation is not
>> completely understood, but the errors do not reproduce if the
>> commit-graphs are all generated by a Git version including these recent
>> fixes.
> 
> Do you mean "we know doing X and then Y and then Z on this
> particular private data with older version of Git without those two
> fixes will lead to a broken timestamp, but doing exactly the same
> with the two fixes, the breakage does not reproduce"?  If so, that
> is quite encouraging news.  Thanks for working well together.

Yes, that is my understanding.

>> If we cannot trust the existing data in the GDAT and GDOV chunks, then
>> we can alter the format to change the chunk IDs for these chunks. This
>> causes the new version of Git to silently ignore the older chunks (and
>> disabling generation number v2 in the process) while writing new
>> commit-graph files with correct data in the GDA2 and GDO2 chunks.
>>
>> Update commit-graph-format.txt including a historical note about these
>> deprecated chunks.
> 
> Sensible.
> 
>> @@ -156,3 +156,11 @@ CHUNK DATA:
>>  TRAILER:
>>  
>>  	H-byte HASH-checksum of all of the above.
>> +
>> +== Historical Notes:
>> +
>> +The Generation Data (GDA2) and Generation Data Overflow (GDO2) chunks have
>> +the number '2' in their chunk IDs because a previous version of Git wrote
>> +possibly erroneous data in these chunks with the IDs "GDAT" and "GDOV". By
>> +changing the IDs, newer versions of Git will silently ignore those older
>> +chunks and write the new information without trusting the incorrect data.
> 
> Good.  How does a new version of Git skip and ignore GDAT and GDOV
> in existing files?  By not having any code to recognize what they
> are?
> 
> I am wondering if there is some notion of "if you do not understand
> what this chunk is, you are incapable of handling this file
> correctly, so do not use it" kind of bit per chunks (similar to the
> index extensions where ones that begin with [A-Z] are optional) that
> may negatively affect this plan.

The chunk IDs do not have this special casing rule. This is a
bit unfortunate for certain cases like adding something that _must_
be understood. Here, it works to our benefit that GDAT and GDOV are
optional and can be safely ignored. Thus, clients with this patch
will ignore GDAT and GDOV and continue using topological levels
form the CDAT chunk. Older clients without this patch will ignore
the new GDA2 and GDO2 chunks and continue using topological levels.

For Git versions without this topic branch, this "continue using
topological levels" means no change of behavior at all.

Thanks,
-Stolee