Re: [PATCH 2/8] packfile: refactor `prepare_packed_git_one()` to work on sources
From: Taylor Blau <hidden>
Date: 2025-07-10 23:14:31
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.
Refactor loading of packfiles in the same way: instead of passing in the object directory, we now pass in the source for which we want to load
s/for/from
quoted hunk ↗ jump to hunk
packfiles. This allows us to simplify the code because we don't have to search for a corresponding MIDX anymore, but we can instead directly use the MIDX that we have already prepared beforehand. Signed-off-by: Patrick Steinhardt <redacted> --- packfile.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-)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. Thanks, Taylor