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:

>> It worries me a lot to lose the warning unconditionally, though.
>> That's the (only) coal-mine canary that lets us notice a problem
>> when we actually start hitting that last-ditch cycle breaking too
>> often.
>
> The dedicated cycle-detection will lose that warning, too (it doesn't
> touch that code, but it's effectively checking the same thing earlier).
>
> I agree it's unfortunate to lose. On the other hand, it is being lost
> because we are correctly handling the cycles, and there is nothing to
> warn about. So it ceases to be a problem, and starts being a normal,
> acceptable thing.

That unfortunately is beside the point.  The existing cycle breaking
was meant to be a last ditch effort to avoid producing a broken pack
(i.e. a suboptimal pack without cycle is better than unusable pack
with delta cycle), while letting us know that we found a case where
the remainder of the pack building machinery does not function well
without it (so that we know we broke something when we tweaked the
machinery without intending to break it).  Squelching the warnings
feels similar to "we see too many valgrind warnings, so let's stop
running valgrind"; I was hoping there would be a solution more like
"instead of not running, let's teach valgrind this and that codepath
is OK".