Re: [PATCH v2 1/4] pack-bitmap.c: check preferred pack validity when opening MIDX bitmap

> On Tue, May 24, 2022 at 09:36:45PM +0200, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Tue, May 24 2022, Taylor Blau wrote:
>>
>> Just nits on the error reporting:
>>
>> > @@ -353,6 +355,20 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
>> >  		warning(_("multi-pack bitmap is missing required reverse index"));
>> >  		goto cleanup;
>> >  	}
>> > +
>> > +	for (i = 0; i < bitmap_git->midx->num_packs; i++) {
>> > +		if (prepare_midx_pack(the_repository, bitmap_git->midx, i))
>> > +			die(_("could not open pack %s"),
>> > +			    bitmap_git->midx->pack_names[i]);
>>
>> Some existing API users of this & their error handling suggest that this
>> message is wrong. I.e. it's not that we couldn't open it, but that we
>> could open it and there's something wrong with it. Or perhaps their
>> messages are misleading?
>
> I tried to reuse some similar message based on "git grep 'if
> (.*prepare_midx_pack'", so this was inspired by:
>
>   - the caller in midx.c::write_midx_internal(), whose error is "could
>     not load pack", and
>   - the caller in midx.c::verify_midx_file(), whose error is "failed to
>     load pack"
>
> Are you suggesting we should s/open/load here and use the above error
> message? My feeling at the time was that "load" was basically synonymous
> with "open" given the context, but if you think they're different
> enough, or have a different suggestion LMK.