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

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:

quoted

quoted

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