On 06/19/2017 09:53 PM, Jeff King wrote: > On Mon, Jun 19, 2017 at 03:43:15PM -0400, Jeff King wrote: > >>>> Is the iterator over packed-refs correctly skipping over what are >>>> covered by loose refs? The entries in the packed-refs file that are >>>> superseded by loose refs should be allowed to point at an already >>>> expired object. >>> >>> Here it is in a test form for easier diagnosis. >> >> Thanks, I was just starting to do that myself. The problem is in >> ca6b06eb7 (packed_ref_store: support iteration, 2017-05-15) and is >> pretty obvious: the packed_ref iterator checks whether the entry >> resolves. >> >> I think that _neither_ of the loose and packed iterators should be >> checking this. It's only the merged result (where loose trumps packed) >> that should bother checking. Thanks for the bug report and the analysis, which is exactly right. But I'd like to fix the problem a *little* differently than Peff suggested. To keep `packed_ref_store` from deviating more than necessary from the `ref_store` interface, I propose that we leave the code for rejecting broken refs where it is, and instead invoke `packed_ref_iterator_begin()` with the `DO_FOR_EACH_INCLUDE_BROKEN` flag. I have prepared a re-roll of the patch series, but I can't submit it until I have Junio's signoff on the test that he suggested [1]. Junio? Thanks, Michael [1] http://public-inbox.org/git/xmqqvanrsru4.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxx/
