On Tue, Sep 02, 2025 at 10:50:54AM +0200, Patrick Steinhardt wrote: > On Tue, Aug 26, 2025 at 09:38:47PM -0400, Taylor Blau wrote: > > On Thu, Aug 21, 2025 at 09:39:12AM +0200, Patrick Steinhardt wrote: > > > We have two different functions to retrieve packfiles for a packfile > > > store: > > > > > > - `get_packed_git()` returns the list of packfiles after having called > > > `prepare_packed_git()`. > > > > > > - `get_all_packs()` calls `prepare_packed_git()`, as well, but also > > > calls `prepare_midx_pack()` for each pack. > > > > Yeah, having two of these functions that are named so similarly as to > > suggest they do the same thing (even though they don't) is unfortunate, > > and I am glad that we are looking at it here. > > > > > This means that the latter function also properly loads the info of > > > whether or not a packfile is part of a multi-pack index. Preparing this > > > extra information also shouldn't be significantly more expensive: > > > > Right; get_packed_git() only loads the non-MIDX'd packs, and > > get_all_packs() loads everything (regardless whether or not a pack is > > part of the MIDX or not). > > I initially understood the distinction of these functions to be exactly > this. But after looking further I don't think this is the actual > distinction: both functions end up loading all packfiles in the repo, > with the only distinction being that `get_all_packs()` also prepares the > MIDX for each MIDX'd packfile. Calling both get_packed_git() and get_all_packs() both result in: - prepare_packed_git(), which calls - prepare_packed_git_one(), which calls - for_each_file_in_pack_dir(), which calls (via callback) - prepare_pack prepare_pack() only calls add_packed_git() and install_packed_git() if the file it's look at ends in "*.idx", *and* there is either no MIDX, or (if there is one) the packfile is not listed in the MIDX. get_all_packs(), on the other hand, *does* call prepare_midx_pack() on all MIDXs across all ODB sources, after having called prepare_packed_git(). And prepare_midx_pack() will add MIDX'd packs to the global list of packs, which is what differentiates these two calls. I think this gets a little funky in practice if you have already called prepare_midx_pack() on one or more MIDX'd packfiles before calling get_packed_git(), because of the side effect of prepare_midx_pack() installing packs into the global list. So the above behavior isn't guaranteed, but there definitely is a difference between those two functions depending on whether or not some other site has called prepare_midx_pack(). > > So I think that want_object_in_pack_mtime() may need a small tweak, and > > I am not 100% certain that cmd_grep() is OK to convert. > > I initially misunderstood the distinction between these two functions > the same as you did, and had a similar list to the above in the initial > commit message. But with the adjusted understanding of the actual > difference between these functions I think it shouldn't be necessary > anymore to go through each caller one by one.
