Re: [PATCH 12/16] packfile: introduce function to load and add packfiles

[Patrick Steinhardt <ps@pks.im>]

On Wed, Aug 20, 2025 at 06:41:23AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> > index e9d82b31c3..a26e79689d 100644
> > --- a/builtin/fast-import.c
> > +++ b/builtin/fast-import.c
> > @@ -897,11 +897,11 @@ static void end_packfile(void)
> >  		idx_name = keep_pack(create_index());
> >
> >  		/* Register the packfile with core git's machinery. */
> > -		new_p = add_packed_git(pack_data->repo, idx_name, strlen(idx_name), 1);
> > +		new_p = packfile_store_load_pack(pack_data->repo->objects->packfiles,
> > +						 idx_name, 1);
> >
> 
> I assume that the 'packfile_store_load_pack' function here returns a
> new/existing packfile.

Yes, exactly.

> > diff --git a/packfile.c b/packfile.c
> > index 8b5e6b96ce..f7916543a6 100644
> > --- a/packfile.c
> > +++ b/packfile.c
> > @@ -793,6 +793,33 @@ void packfile_store_add_pack(struct packfile_store *store,
> >  	list_add_tail(&pack->mru, &store->mru);
> >  }
> >
> > +struct packed_git *packfile_store_load_pack(struct packfile_store *store,
> > +					    const char *idx_path, int local)
> > +{
> > +	struct strbuf key = STRBUF_INIT;
> > +	struct packed_git *p;
> > +
> > +	/*
> > +	 * We're being called with the path to the index file, but `pack_map`
> > +	 * holds the path to the packfile itself.
> > +	 */
> > +	strbuf_addstr(&key, idx_path);
> > +	strbuf_strip_suffix(&key, ".idx");
> > +	strbuf_addstr(&key, ".pack");
> > +
> > +	p = hashmap_get_entry_from_hash(&store->map, strhash(key.buf), key.buf,
> > +					struct packed_git, packmap_ent);
> 
> I was wondering from an earlier patch too, is there a reason to simply
> not use 'strmap' for 'packfile_store.map'?

Hm. I cannot think of any, no. I'll leave this as-is in this patch
series though and move it into the next one where I'm revamping how
packfiles are stored.

Patrick