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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, May 24 2022, Taylor Blau wrote:

> 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.

Perhaps "parse" or something? Anyway with "could not open" I'd assume
open() failed, but in this case it looks like we could open it, but
(mostly?) failed later.

Maybe "could not load midx"? I don't know...