On Thu, Aug 21, 2025 at 09:39:06AM +0200, Patrick Steinhardt wrote:
> The `prepare_packed_git()` function and its friends are responsible for
> loading packfiles as well as the multi-pack index for a given object
> database. Refactor these functions to accept a packfile store instead of
> a repository to clarify their scope.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> packfile.c | 43 +++++++++++++++++++------------------------
> 1 file changed, 19 insertions(+), 24 deletions(-)
>
> diff --git a/packfile.c b/packfile.c
> index 90f15b0c20..2e45a3a05f 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -974,38 +974,33 @@ static int sort_pack(const struct packed_git *a, const struct packed_git *b)
> return -1;
> }
>
> -static void rearrange_packed_git(struct repository *r)
> -{
> - sort_packs(&r->objects->packfiles->packs, sort_pack);
> -}
OK, makes sense -- it looks like you inlined rearrange_packed_git() in
its sole caller packfile_store_prepare() below. I think that could have
been done as a separate step, but it's equally fine to include it here,
too.
> -static void prepare_packed_git_mru(struct repository *r)
> +static void packfile_store_prepare_mru(struct packfile_store *store)
> {
> struct packed_git *p;
>
> - INIT_LIST_HEAD(&r->objects->packfiles->mru);
> + INIT_LIST_HEAD(&store->mru);
>
> - for (p = r->objects->packfiles->packs; p; p = p->next)
> - list_add_tail(&p->mru, &r->objects->packfiles->mru);
> + for (p = store->packs; p; p = p->next)
> + list_add_tail(&p->mru, &store->mru);
Looks all good.
> -static void prepare_packed_git(struct repository *r)
> +static void packfile_store_prepare(struct packfile_store *store)
> {
> struct odb_source *source;
>
> - if (r->objects->packfiles->initialized)
> + if (store->initialized)
> return;
>
> - odb_prepare_alternates(r->objects);
> - for (source = r->objects->sources; source; source = source->next) {
> - int local = (source == r->objects->sources);
> + odb_prepare_alternates(store->odb);
Hmmm. I admit that I don't love that the packfile_store knows about the
object_store that it belongs to. This feels like a layering violation
and makes me worry that we pushed too much down into the new
packfile_store. I'm not sure I have a better idea off the top of my
head, though.
Everything down from here looks good to me.
Thanks,
Taylor
