From: Toon Claes <toon@iotcl•com>
To: Patrick Steinhardt <ps@pks•im>, git@vger•kernel.org
Cc: Justin Tobler <jltobler@gmail•com>
Subject: Re: [PATCH v2 05/10] packfile: move packfile store into object source
Date: Wed, 07 Jan 2026 14:11:17 +0100 [thread overview]
Message-ID: <87bjj5pnyi.fsf@iotcl.com> (raw)
In-Reply-To: <20251218-b4-pks-pack-store-via-source-v2-5-62849007ce21@pks.im>
Patrick Steinhardt <ps@pks•im> writes:
> The packfile store is a member of `struct object_database`, which means
> that we have a single store per database. This doesn't really make much
> sense though: each source connected to the database has its own set of
> packfiles, so there is a conceptual mismatch here. This hasn't really
> caused much of a problem in the past, but with the advent of pluggable
> object databases this is becoming more of a problem because some of the
> sources may not even use packfiles in the first place.
>
> Move the packfile store down by one level from the object database into
> the object database source. This ensures that each source now has its
> own packfile store, and we can eventually start to abstract it away
> entirely so that the caller doesn't even know what kind of store it
> uses.
>
> Note that we only need to adjust a relatively small number of callers,
> way less than one might expect. This is because most callers are using
> `repo_for_each_pack()`, which handles enumeration of all packfiles that
> exist in the repository. So for now, none of these callers need to be
> adapted. The remaining callers that iterate through the packfiles
> directly and that need adjustment are those that are a bit more tangled
> with packfiles. These will be adjusted over time.
>
> Note that this patch only moves the packfile store, and there is still a
> bunch of functions that seemingly operate on a packfile store but that
> end up iterating over all sources. These will be adjusted in subsequent
> commits.
>
> Signed-off-by: Patrick Steinhardt <ps@pks•im>
> ---
> builtin/fast-import.c | 37 ++++++++------
> builtin/grep.c | 6 ++-
> builtin/index-pack.c | 2 +-
> builtin/pack-objects.c | 96 +++++++++++++++++++------------------
> http.c | 2 +-
> midx.c | 5 +-
> odb.c | 36 +++++++-------
> odb.h | 6 +--
> odb/streaming.c | 9 ++--
> packfile.c | 127 +++++++++++++++++++++++++++++++------------------
> packfile.h | 62 ++++++++++++++++++++----
> 11 files changed, 243 insertions(+), 145 deletions(-)
>
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 7849005ccb..b8a7757cfd 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -900,7 +900,7 @@ static void end_packfile(void)
> idx_name = keep_pack(create_index());
>
> /* Register the packfile with core git's machinery. */
> - new_p = packfile_store_load_pack(pack_data->repo->objects->packfiles,
> + new_p = packfile_store_load_pack(pack_data->repo->objects->sources->packfiles,
> idx_name, 1);
> if (!new_p)
> die(_("core Git rejected index %s"), idx_name);
> @@ -955,7 +955,7 @@ static int store_object(
> struct object_id *oidout,
> uintmax_t mark)
> {
> - struct packfile_store *packs = the_repository->objects->packfiles;
> + struct odb_source *source;
> void *out, *delta;
> struct object_entry *e;
> unsigned char hdr[96];
> @@ -979,7 +979,11 @@ static int store_object(
> if (e->idx.offset) {
> duplicate_count_by_type[type]++;
> return 1;
> - } else if (packfile_list_find_oid(packfile_store_get_packs(packs), &oid)) {
> + }
> +
> + for (source = the_repository->objects->sources; source; source = source->next) {
> + if (!packfile_list_find_oid(packfile_store_get_packs(source->packfiles), &oid))
> + continue;
> e->type = type;
> e->pack_id = MAX_PACK_ID;
> e->idx.offset = 1; /* just not zero! */
> @@ -1096,10 +1100,10 @@ static void truncate_pack(struct hashfile_checkpoint *checkpoint)
>
> static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
> {
> - struct packfile_store *packs = the_repository->objects->packfiles;
> size_t in_sz = 64 * 1024, out_sz = 64 * 1024;
> unsigned char *in_buf = xmalloc(in_sz);
> unsigned char *out_buf = xmalloc(out_sz);
> + struct odb_source *source;
> struct object_entry *e;
> struct object_id oid;
> unsigned long hdrlen;
> @@ -1179,24 +1183,29 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
> if (e->idx.offset) {
> duplicate_count_by_type[OBJ_BLOB]++;
> truncate_pack(&checkpoint);
> + goto out;
> + }
>
> - } else if (packfile_list_find_oid(packfile_store_get_packs(packs), &oid)) {
> + for (source = the_repository->objects->sources; source; source = source->next) {
> + if (!packfile_list_find_oid(packfile_store_get_packs(source->packfiles), &oid))
> + continue;
> e->type = OBJ_BLOB;
> e->pack_id = MAX_PACK_ID;
> e->idx.offset = 1; /* just not zero! */
> duplicate_count_by_type[OBJ_BLOB]++;
> truncate_pack(&checkpoint);
> -
> - } else {
> - e->depth = 0;
> - e->type = OBJ_BLOB;
> - e->pack_id = pack_id;
> - e->idx.offset = offset;
> - e->idx.crc32 = crc32_end(pack_file);
> - object_count++;
> - object_count_by_type[OBJ_BLOB]++;
> + goto out;
> }
>
> + e->depth = 0;
> + e->type = OBJ_BLOB;
> + e->pack_id = pack_id;
> + e->idx.offset = offset;
> + e->idx.crc32 = crc32_end(pack_file);
> + object_count++;
> + object_count_by_type[OBJ_BLOB]++;
> +
> +out:
> free(in_buf);
> free(out_buf);
> }
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 53cccf2d25..4855b871dd 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -1213,8 +1213,12 @@ int cmd_grep(int argc,
> */
> if (recurse_submodules)
> repo_read_gitmodules(the_repository, 1);
> + /*
> + * Note: `packfile_store_prepare()` prepares stores from all
> + * sources. This will be fixed in a subsequent commit.
I assume you mean the opposite. But no problem since this will be
addressed in a later commit.
--
Cheers,
Toon
next prev parent reply other threads:[~2026-01-07 13:11 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-15 7:36 [PATCH 00/10] Start tracking packfiles per object database source Patrick Steinhardt
2025-12-15 7:36 ` [PATCH 01/10] packfile: create store via its owning source Patrick Steinhardt
2025-12-15 21:30 ` Justin Tobler
2025-12-16 8:36 ` Patrick Steinhardt
2025-12-15 7:36 ` [PATCH 02/10] packfile: pass source to `prepare_pack()` Patrick Steinhardt
2025-12-15 21:38 ` Justin Tobler
2025-12-15 7:36 ` [PATCH 03/10] packfile: refactor kept-pack cache to work with packfile stores Patrick Steinhardt
2025-12-15 21:56 ` Justin Tobler
2025-12-16 9:09 ` Patrick Steinhardt
2025-12-15 7:36 ` [PATCH 04/10] packfile: refactor misleading code when unusing pack windows Patrick Steinhardt
2025-12-15 7:36 ` [PATCH 05/10] packfile: move packfile store into object source Patrick Steinhardt
2025-12-18 0:52 ` Justin Tobler
2025-12-18 6:50 ` Patrick Steinhardt
2025-12-15 7:36 ` [PATCH 06/10] packfile: only prepare owning store in `packfile_store_get_packs()` Patrick Steinhardt
2025-12-18 0:58 ` Justin Tobler
2025-12-15 7:36 ` [PATCH 07/10] packfile: only prepare owning store in `packfile_store_prepare()` Patrick Steinhardt
2025-12-15 7:36 ` [PATCH 08/10] packfile: inline `find_kept_pack_entry()` Patrick Steinhardt
2025-12-18 1:06 ` Justin Tobler
2025-12-18 6:48 ` Patrick Steinhardt
2025-12-15 7:36 ` [PATCH 09/10] packfile: refactor `find_pack_entry()` to work on the packfile store Patrick Steinhardt
2025-12-15 7:36 ` [PATCH 10/10] packfile: move MIDX into " Patrick Steinhardt
2025-12-18 6:55 ` [PATCH v2 00/10] Start tracking packfiles per object database source Patrick Steinhardt
2025-12-18 6:55 ` [PATCH v2 01/10] packfile: create store via its owning source Patrick Steinhardt
2025-12-18 6:55 ` [PATCH v2 02/10] packfile: pass source to `prepare_pack()` Patrick Steinhardt
2025-12-18 6:55 ` [PATCH v2 03/10] packfile: refactor kept-pack cache to work with packfile stores Patrick Steinhardt
2026-01-06 20:42 ` Toon Claes
2025-12-18 6:55 ` [PATCH v2 04/10] packfile: refactor misleading code when unusing pack windows Patrick Steinhardt
2026-01-07 10:13 ` Toon Claes
2025-12-18 6:55 ` [PATCH v2 05/10] packfile: move packfile store into object source Patrick Steinhardt
2026-01-07 13:11 ` Toon Claes [this message]
2025-12-18 6:55 ` [PATCH v2 06/10] packfile: only prepare owning store in `packfile_store_get_packs()` Patrick Steinhardt
2025-12-18 6:55 ` [PATCH v2 07/10] packfile: only prepare owning store in `packfile_store_prepare()` Patrick Steinhardt
2026-01-07 13:15 ` Toon Claes
2025-12-18 6:55 ` [PATCH v2 08/10] packfile: inline `find_kept_pack_entry()` Patrick Steinhardt
2026-01-08 7:16 ` Kristoffer Haugsbakk
2026-01-09 6:17 ` Patrick Steinhardt
2025-12-18 6:55 ` [PATCH v2 09/10] packfile: refactor `find_pack_entry()` to work on the packfile store Patrick Steinhardt
2025-12-18 6:55 ` [PATCH v2 10/10] packfile: move MIDX into " Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 00/10] Start tracking packfiles per object database source Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 01/10] packfile: create store via its owning source Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 02/10] packfile: pass source to `prepare_pack()` Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 03/10] packfile: refactor kept-pack cache to work with packfile stores Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 04/10] packfile: refactor misleading code when unusing pack windows Patrick Steinhardt
2026-01-12 14:52 ` Karthik Nayak
2026-01-09 8:33 ` [PATCH v3 05/10] packfile: move packfile store into object source Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 06/10] packfile: only prepare owning store in `packfile_store_get_packs()` Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 07/10] packfile: only prepare owning store in `packfile_store_prepare()` Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 08/10] packfile: inline `find_kept_pack_entry()` Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 09/10] packfile: refactor `find_pack_entry()` to work on the packfile store Patrick Steinhardt
2026-01-09 8:33 ` [PATCH v3 10/10] packfile: move MIDX into " Patrick Steinhardt
2026-01-11 5:46 ` [PATCH v3 00/10] Start tracking packfiles per object database source Junio C Hamano
2026-01-12 15:27 ` Justin Tobler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87bjj5pnyi.fsf@iotcl.com \
--to=toon@iotcl$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jltobler@gmail$(echo .)com \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox