From: Elijah Newren Date: Tue, 25 Aug 2026 07:19:29 GMT Subject: Re: [PATCH 2/2] packfile: recover when a multi-pack-index names a removed pack Message-ID: In-Reply-To: <20260824045529.GB142844@coredump.intra.peff.net> On Sun, Aug 23, 2026 at 9:55 PM Jeff King wrote: > > On Tue, Aug 18, 2026 at 10:34:06PM +0000, Elijah Newren via GitGitGadget wrote: > > > Teach find_pack_entry() to recover. After the normal multi-pack-index > > lookup and the regular pack fallback both miss, check whether the object > > is nonetheless present in a covered multi-pack-index (bsearch_midx()). > > If it is, its recorded owner must have become unavailable, so scan that > > index's packs directly for a surviving copy. The bsearch gate keeps > > genuine misses (i.e. objects absent from the index) on the fast path, and > > because the recovery lives in find_pack_entry() itself it also fixes the > > OBJECT_INFO_QUICK callers that never reprepare. > > You don't even have to pay the bsearch() again. We'd already have looked > in the midx earlier in the function. We just need to distinguish three > cases: > > 1. it was not in the midx (or there is no midx) > > 2. it was in the midx but we could not load it (pack invalid, or > object in the bad_objects list) > > 3. it was in the midx and is available > > In fill_midx_entry() we return a boolean that lumps cases 1+2 together, > versus case 3. It could return a tri-state that would let us distinguish > all three. And then your fallback would kick in only for case 2 (case 3 > already returned with success, and case 1 means the midx does not even > mention the object). You know, I considered putting the logic in fill_midx_entry() as well as putting where it is. You'd think based on that, that I'd have thought about just changing fill_midx_entry()'s return type to get the best of both worlds. You'd be wrong though. ;-) This sounds much nicer; I adopted it and made fill_midx_entry() return MIDX_FILL_MISS / MIDX_FILL_HIT / MIDX_FILL_OWNER_UNAVAILABLE in v2.