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

[Taylor Blau <me@ttaylorr.com>]

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.

Thanks,
Taylor