From: Justin Tobler <jltobler@gmail.com>
Date: Mon, 15 Dec 2025 21:30:44 GMT
Subject: Re: [PATCH 01/10] packfile: create store via its owning source
Message-ID: <7rbnw67kn3xe3mpkpssiy22ewvjihzteole3sjhosocqo4sr7a@cig7o2dauljd>
In-Reply-To: <20251215-b4-pks-pack-store-via-source-v1-1-433aac465295@pks.im>

On 25/12/15 08:36AM, Patrick Steinhardt wrote:
> In subsequent patches we're about to move the packfile store from the
> object database layer into the object database source layer. Once done,
> we'll have one packfile store per source, where the source is owning the
> store.
> 
> Prepare for this future and refactor `packfile_store_new()` to be
> initialized via an object database source instead of via the object
> database itself.

Makes sense.

> This refactoring leads to a weird in-between state where the store is
> owned by the object database but created via the source. But this makes
> subsequent refactorings easier because we can now start to access the
> owning source of a given store.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
[snip]
> diff --git a/packfile.c b/packfile.c
> index c88bd92619..0a05a10daa 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -876,7 +876,7 @@ struct packed_git *packfile_store_load_pack(struct packfile_store *store,
>  
>  	p = strmap_get(&store->packs_by_path, key.buf);
>  	if (!p) {
> -		p = add_packed_git(store->odb->repo, idx_path,
> +		p = add_packed_git(store->source->odb->repo, idx_path,
>  				   strlen(idx_path), local);
>  		if (p)
>  			packfile_store_add_pack(store, p);
> @@ -1068,8 +1068,8 @@ void packfile_store_prepare(struct packfile_store *store)
>  	if (store->initialized)
>  		return;
>  
> -	odb_prepare_alternates(store->odb);
> -	for (source = store->odb->sources; source; source = source->next) {
> +	odb_prepare_alternates(store->source->odb);
> +	for (source = store->source->odb->sources; source; source = source->next) {

huh so IIUC, even though there is a packfile store per ODB source, we
will add the alternate sources to the same packfile store? This is feels
very awkward, but is maybe part of the "weird in-between state" you
mentioned in the commit message.

>  		prepare_multi_pack_index_one(source);
>  		prepare_packed_git_one(source);
>  	}
[snip]
> diff --git a/packfile.h b/packfile.h
> index 59d162a3f4..33cc1c1654 100644
> --- a/packfile.h
> +++ b/packfile.h
> @@ -77,7 +77,7 @@ struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
>   * A store that manages packfiles for a given object database.
>   */
>  struct packfile_store {
> -	struct object_database *odb;
> +	struct odb_source *source;

The packfile store now stores a reference to the object source instead
of the ODB itself. The ODB source has a reference to the ODB so
callsites that were orginally referencing the ODB can still go through
the source. Makes sense.

>  	/*
>  	 * The list of packfiles in the order in which they have been most
> @@ -129,9 +129,9 @@ struct packfile_store {
>  
>  /*
>   * Allocate and initialize a new empty packfile store for the given object
> - * database.
> + * database source.
>   */
> -struct packfile_store *packfile_store_new(struct object_database *odb);
> +struct packfile_store *packfile_store_new(struct odb_source *source);

The packfile store is now initialized with the ODB source. Looks good.

-Justin