Re: [PATCH 3/6] builtin/grep: simplify how we preload packs

[Taylor Blau <me@ttaylorr.com>]

On Tue, Oct 07, 2025 at 02:41:09PM +0200, Patrick Steinhardt wrote:
> When using multiple threads in git-grep(1) we eagerly preload both the
> gitmodules file as well as the packfiles so that the threads won't race
> with one another to initialize these data structures.
>
> For packfiles, this is done by calling `packfile_store_get_packs()`,
> which first loads our packfiles and then returns a pointer to the first
> such packfile. This pointer is ignored though, as all we really care
> about is that `packfile_store_prepare()` was called.
>
> Historyically, that function was file-local to "packfile.c", but that

s/Historyically/Historically

> changed with 4188332569 (packfile: move `get_multi_pack_index()` into
> "midx.c", 2025-09-02). We can thus simplify the code by calling that
> function directly.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/grep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 13841fbf00..53cccf2d25 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -1214,7 +1214,7 @@ int cmd_grep(int argc,
>  		if (recurse_submodules)
>  			repo_read_gitmodules(the_repository, 1);
>  		if (startup_info->have_repository)
> -			(void)packfile_store_get_packs(the_repository->objects->packfiles);
> +			packfile_store_prepare(the_repository->objects->packfiles);

Makes sense. That function literally calls packfile_store_prepare() on
its argument, and then returns store->packs. Since we don't care about
the result as you note, calling packfile_store_prepare() directly makes
sense.

Thanks,
Taylor