On Wed, Aug 20, 2025 at 06:50:05AM -0700, Karthik Nayak wrote: > Patrick Steinhardt <ps@xxxxxx> 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
