Re: MIDX woes, was Re: [ANNOUNCE] Git v2.54.0-rc2

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

Jeff King <peff@peff.net> writes:

> On Thu, Apr 16, 2026 at 01:12:50PM -0700, Junio C Hamano wrote:
>
>> Looking at the later part of load_multi_pack_index_one() function,
>> it is full of die() calls, not prepared to degrade gracefully at
>> all.
>
> OK, I confess I did not look past the two goto calls below the lines I
> touched, since I didn't think we'd be applying it immediately anyway.

Heh, I confess I did not look, either, until I decided I have to
respond to your message ;-)

>> The reason why I do not think reword is worth doing is that as far
>> as this function (i.e., "Here is an MIDX file---please read it and
>> give me struct multi_pack_index back") is concerned, these error
>> returns are giving diagnosis on the nature of errors.  Sometimes we
>> cannot learn the length of the file, sometimes we find that the file
>> is unreasonably short, sometimes it is not sorted properly, etc.
>> None of the existing "error + cleanup" says "we are continuing
>> anyway" and that is good.  The decision to keep going without using
>> midx file (or not) belongs to the caller, so if we wanted to say
>> that, we should say so in the caller, not here.
>
> Yep, I agree with all of that.

Yup, I would feel safer to have those who know this code path better
to help us decide if it makes sense to turn all of these die() calls
to error + cleanup.

In any case, that would be an issue for much later.

Thanks.