Re: [PATCH 3/3] index-pack: allow revisiting REF_DELTA chains

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 24, 2025, 21:41 UTC
Message-ID: <xmqqjz79texy.fsf@gitster.g>
In-Reply-To: <27d36402fe9c8262c459d606bf4aace1919aa2ae.1745430004.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 13 quoted lines

> From: Derrick Stolee <stolee@gmail.com>
>
> As detailed in the previous changes to t5309-pack-delta-cycles.sh, the
> logic within 'git index-pack' to analyze an incoming thin packfile with
> REF_DELTAs is suspect. The algorithm is overly cautious around delta
> cycles, and that leads in fact to failing even when there is no cycle.
>
> This change adjusts the algorithm to no longer fail in these cases. In
> fact, these cycle cases will no longer fail but more importantly the
> valid cases will no longer fail, either. The resulting packfile from the
> --fix-thin operation will not have cycles either since REF_DELTAs are
> forbidden from the on-disk format and OFS_DELTAs are impossible to write
> as a cycle.

Loosening cycle avoidance indeed is worrysome. How do we guarantee "since REF_DELTAs are forbidden from the on-disk format" (it is obvious with OFS_DELTA only you cannot form a cycle)? By code inspection and in-code comment for the code path of "--fix-thin"?

I think the most interesting question would be how, with the loosening of the cycle check in this patch, we would still protect against a malicious on-the-wire packdata that has a cycle in it already, where deltas cannot be resolved.