Re: [PATCH v2 7/7] pack-objects: use mru list when iterating over packs

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Fri, Jul 29, 2016 at 12:15:24AM -0400, Jeff King wrote:
>
> But because this series switches the order of pack-lookup between
> objects, it is possible for us to find a `B` which is a delta against
> `A` in one pack, and then another copy of `A` which is a delta against
> another copy of `B` from another pack. We add both of the deltas to our
> packing list, but at write time when we try to write out all of the
> bases for `A`, we realize that whoops, we are recursing infinitely.
>
> As it turns out, Git actually handles this pretty well! Upon noticing
> the recursion, it breaks the delta chain and writes out one of the
> objects as a full base. This is due to Junio's f63c79d (pack-object:
> tolerate broken packs that have duplicated objects, 2011-11-16), though
> I think we later decided that duplicated objects were simply insane.
>
> So one option is to simply silence the warning, because the resulting
> pack is perfectly fine.

Thanks for a great analysis.

My gut feeling is that keeping the warning is preferred if possible,
because f63c79db (pack-object: tolerate broken packs that have
duplicated objects, 2011-11-16) was made as the last ditch effort to
warn about the presence of the problem in the delta-base selection
code without harming the users.

> So it's possible that the resulting pack
> is not as small as it could be (i.e., we break the chain with a base
> object, but it's possible if we looked that we could have broken the
> chain by making a delta against an existing base object). So I wonder if
> it's possible to detect this case earlier, during the "can we reuse this
> delta" bits of check_object().

I'd let the issue simmer in my mind a bit for now, as I do not
think of a neat trick offhand.