Re: [PATCH 2/2] packfile: recover when a multi-pack-index names a removed pack

[Patrick Steinhardt <ps@pks.im>]

On Mon, Aug 24, 2026 at 12:55:29AM -0400, 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).
> 
> This is all assuming the fallback is worth pursuing. I'm still puzzled
> why this specific case would matter when we have the same (already
> solved) problem of reading a regular .idx whose .pack has gone away.

I've tried to clarify in a parallel message already, but the issue is
that we skip over any packfiles that covered by a MIDX when doing the
lookup. So any secondary packfiles that contain the object would be
completely ignored, and that's why we don't find the object there.

But this mail here suggests an alternative fix: instead of re-scanning
all packfiles like the patch proposes, wouldn't the proper fix be to not
ignore _all_ MIDX'd packs, but only the pack that _should_ have
contained the object?

Ultimately though, this would be equivalent to turning the function's
return value into a tri-state as suggested by Peff here. The only case
where the issue can occur is in case (2), and in that case we should not
skip MIDX'd packs at all as the MIDX'd pack that should've contained the
pack does not exist anyway.

Patrick