Re: [PATCH 01/10] packfile: create store via its owning source
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Dec 15, 2025, 21:30 UTC
- 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:
Show 8 quoted lines
> 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.
Show 7 quoted lines
> 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]
Show 21 quoted lines
> 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]
Show 10 quoted lines
> 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.
Show 11 quoted lines
> /*
> * 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