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
