Re: [PATCH 2/8] packfile: refactor `prepare_packed_git_one()` to work on sources
On Thu, Jul 10, 2025 at 07:14:29PM -0400, Taylor Blau wrote:
Show 11 quoted lines
> On Wed, Jul 09, 2025 at 09:54:50AM +0200, Patrick Steinhardt wrote:
> > In the preceding commit we have refactored how we load multi-pack
> > indices so that we take take the source as input for which we want to
> > load the MIDX. As part of this refactoring we started to store a pointer
> > to the MIDX in `struct odb_source` itself.
>
> The first sentence here is a little confusing, but may read more clearly
> if written as:
>
> In the preceding commit we refactored how we load multi-pack
> indices to take a corresponding "source" as input.
Show 33 quoted lines
> > diff --git a/packfile.c b/packfile.c
> > index 16efc2fdca3..b43dd2fe6cb 100644
> > --- a/packfile.c
> > +++ b/packfile.c
> > @@ -935,22 +935,17 @@ static void prepare_pack(const char *full_name, size_t full_name_len,
> > report_garbage(PACKDIR_FILE_GARBAGE, full_name);
> > }
> >
> > -static void prepare_packed_git_one(struct repository *r, char *objdir, int local)
> > +static void prepare_packed_git_one(struct odb_source *source, int local)
> > {
> > - struct prepare_pack_data data;
> > struct string_list garbage = STRING_LIST_INIT_DUP;
> > + struct prepare_pack_data data = {
> > + .m = source->multi_pack_index,
> > + .r = source->odb->repo,
> > + .garbage = &garbage,
> > + .local = local,
> > + };
> >
> > - data.m = r->objects->multi_pack_index;
> > -
> > - /* look for the multi-pack-index for this object directory */
> > - while (data.m && strcmp(data.m->object_dir, objdir))
> > - data.m = data.m->next;
>
> Right, since we know that the MIDX corresponding to this source belongs
> to the same "object_dir" path. Having an ASSERT() here may make that
> more clear, but this change looks correct to me.
>
> I am still a little unclear on how sources and ODBs correspond to one
> another, but under my working assumption from the previous patch, I
> think this is right.Yes, your understanding is correct :)
Patrick