On Sun, Aug 30, 2020 at 09:15:10AM -0700, Junio C Hamano wrote:
> René Scharfe <l.s.r@xxxxxx> writes:
>
> >> Will that work? I'd expect that modern pack files end up being
> >> offset deltas, rather than reference deltas.
> >
> > True, but going down all the way would work:
>
> Perhaps, but I'd rather use pack-objects to prepare the repository
> with no-delta-base-offset to force ref deltas.
Yeah, that seems like a much better test setup.
It does raise an interesting question, though. I had imagined we would
limit the depth of all delta chains here, not just ref-deltas. But it is
true that ofs deltas can't cycle. Without cycles, neither type can go on
indefinitely (they are limited by the number of entries in the
packfile). I could see arguments going either way:
- ofs deltas cannot cycle, so we do not need a counter that limits
them (and which _could_ find a false positive). So we should not
limit them.
- a counter is preventing us from following cycles indefinitely, but
also hardening us against misbehavior due to bugs or insanely large
delta chains (intentional or not). So we should include ofs deltas
in our limit.
A related point is that delta chains might be composed of both types. If
we don't differentiate between the two types, then the limit is clearly
total chain length. If we do, then is the limit the total number of
ref-deltas found in the current lookup, or is it the number of
consecutive ref-deltas? I guess it would have to be the former if our
goal is to catch cycles (since a cycle could include an ofs-delta, as
long as a ref-delta is the part that forms the loop).
-Peff
