Re: [PATCH 14/16] packfile: remove `get_packed_git()`

[Patrick Steinhardt <ps@pks.im>]

On Wed, Aug 20, 2025 at 06:50:05AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > We have two different functions to retrieve packfiles for a packfile
> > store:
> >
> >   - `get_packed_git()` returns the list of packfiles directly.
> >
> >   - `get_all_packs()` does more work and also prepares packfiles that
> >     are being indexed by a multi-pack-index.
> >
> 
> Question, under what situation would a packfile not returned by
> `get_packed_git()` but be indexed by a multi-pack-index.

Okay, this whole commit message isn't all that enlightening. The only
difference between these two functions is that `get_packed_git()` only
calls `packfile_store_prepare()` and then returns the list of packs,
whereas `get_all_packs()` does the same and then also prepares the pack
so that it's properly marked as being indexed by an MIDX.

Now that means that `get_packed_git()` does _less_ work. But both
functions already load the MIDX via `packfile_store_prepare()` anyway,
so the only difference is that we now also call `prepare_midx_pack()`
for each indexed pack. And that function does not do a lot: we have
already loaded the pack itself, so `packfile_store_load_pack()` returns
the already-in-memory pack. So all we end up doing is to figure out
whether the pack is indexed in the MIDX and then store this info in
`p->multi_pack_index` as well as `m->packs[pack_int_id]`.

So the amount of extra work shouldn't really matter. I'll rephrase the
commit message.

> > The distinction is not immediately obvious. Furthermore, to make the
> > situation even worse, `get_packed_git()` would return the same result as
> > `get_all_packs()` once the latter has been called once as they both
> > refer to the same list.
> >
> > As it turns out, the distinction isn't necessary. We only have a couple
> > of callers of `get_packed_git()`, and all of those callers are prepared
> > to call `get_all_packs()` instead:
> >
> >   - "builtin/gc.c": We explicitly check how many packfiles aren't
> >     contained in the multi-pack-index, so loading extra packfiles that
> >     are indexed by it won't change the result.
> >
> >   - "builtin/grep.c": We only care `get_packed_git()` to prepare eagerly
> >     load packfiles. In the preceding commit we have started to expose
> 
> Nit: the first sentence reads a bit weird.

Ah, that was supposed to read "call", not "care".

Patrick