From: Junio C Hamano Date: Thu, 24 Apr 2025 21:41:45 GMT Subject: Re: [PATCH 3/3] index-pack: allow revisiting REF_DELTA chains Message-ID: In-Reply-To: <27d36402fe9c8262c459d606bf4aace1919aa2ae.1745430004.git.gitgitgadget@gmail.com> "Derrick Stolee via GitGitGadget" writes: > From: Derrick Stolee > > 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.