Re: [PATCH v2 14/24] pack-bitmap: write multi-pack bitmaps

[Taylor Blau <me@ttaylorr.com>]

On Wed, Jul 28, 2021 at 01:52:37PM -0400, Jeff King wrote:
> On Tue, Jul 27, 2021 at 04:33:01PM -0400, Taylor Blau wrote:
>
> > > It's interesting that your earlier iteration didn't call
> > > open_pack_index(). Is it necessary, or not? From your description, it
> > > seems like it should be. But maybe some later step lazy-loads it? Even
> > > if so, I can see how prepare_midx_pack() would still be required
> > > (because we want to make sure we are using the same struct).
> >
> > It's only necessary now (at least for determining a preferred pack if
> > the caller didn't specify one with `--preferred-pack`) because we care
> > about reading the `num_objects` field, which the index must be loaded
> > for.
>
> I guess I'm a little confused about "now" in your sentence. I understand
> that it's not necessary before your series to have loaded all of the
> index files ahead of time. But didn't we need to do so in v2 of your
> series, which has the preferred-pack logic?
>
> If so, then was the v2 version buggy, since it only called
> prepare_midx_pack() and not open_pack_index()? And then v3 is fixing
> that? Or is something else opening the pack index for us?

In earlier versions of this series, I don't think we needed to have the
indexes loaded by this point, since (before v3) we didn't care about
ignoring the empty packs when finding a default preferred-pack.

But now we do, and so we need to call open_pack_index() ourselves.
Confusingly, we only need to do that on packs that *are* included in the
MIDX, since prepare_midx_pack() doesn't do it for us, but
add_pack_to_midx() does.

Thanks,
Taylor