From: Taylor Blau <me@ttaylorr•com>
To: Patrick Steinhardt <ps@pks•im>
Cc: git@vger•kernel.org, Karthik Nayak <karthik.188@gmail•com>,
Jeff King <peff@peff•net>
Subject: Re: [PATCH v2 15/16] packfile: refactor `get_all_packs()` to work on packfile store
Date: Tue, 26 Aug 2025 21:45:15 -0400 [thread overview]
Message-ID: <aK5jK2Q3McLnJ9Uw@nand.local> (raw)
In-Reply-To: <20250821-b4-pks-packfiles-store-v2-15-d10623355e9f@pks.im>
On Thu, Aug 21, 2025 at 09:39:13AM +0200, Patrick Steinhardt wrote:
> The `get_all_packs()` function prepares the packfile store and then
> returns its packfiles. Refactor it to accept a packfile store instead of
> a repository to clarify its scope.
I think that clarifying the scope here is a good idea. But I am a little
sad to see this patch proposing that we drop get_all_packs(), which IMHO
is a useful convenience function.
In effect this is pushing out the implementation details of the
packfile_store out to every caller that wants to use get_all_packs(),
which I am not sure is a win. Should those callers care where the array
of packs is found, or have to write
packfile_store_get_packs(the_repository->objects->packfiles)
each time they want to get the set of packs in a repository?
I could see an argument in the future where we have object stores that
aren't packfile-based and thus calling "get_all_packs()" is not
meaningful. But I don't think we are there yet, so I think that this
patch is pushing the burden of that future hypothetical on all existing
callers of get_all_packs().
> Signed-off-by: Patrick Steinhardt <ps@pks•im>
> ---
> builtin/cat-file.c | 2 +-
> builtin/count-objects.c | 2 +-
> builtin/fast-import.c | 4 ++--
> builtin/fsck.c | 8 ++++----
> builtin/gc.c | 8 ++++----
> builtin/pack-objects.c | 18 +++++++++---------
> builtin/pack-redundant.c | 4 ++--
> builtin/repack.c | 6 +++---
> connected.c | 2 +-
> http-backend.c | 4 ++--
> http.c | 2 +-
> object-name.c | 4 ++--
> pack-bitmap.c | 4 ++--
> pack-objects.c | 2 +-
> packfile.c | 14 +++++++-------
> packfile.h | 7 ++++++-
> server-info.c | 2 +-
> t/helper/test-find-pack.c | 2 +-
> t/helper/test-pack-mtimes.c | 2 +-
> 19 files changed, 51 insertions(+), 46 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index fce0b06451c..7124c43fb14 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -854,7 +854,7 @@ static void batch_each_object(struct batch_options *opt,
> batch_one_object_bitmapped, &payload)) {
> struct packed_git *pack;
>
> - for (pack = get_all_packs(the_repository); pack; pack = pack->next) {
> + for (pack = packfile_store_get_packs(the_repository->objects->packfiles); pack; pack = pack->next) {
If we do go this route, it might be nice to introduce the pattern of
having a stack variable to hold the packfile_store pointer, since the
line above here is getting a little long at >100 characters just to
enumerate packs.
Thanks,
Taylor
next prev parent reply other threads:[~2025-08-27 1:45 UTC|newest]
Thread overview: 181+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-19 8:19 [PATCH 00/16] packfile: carve out a new packfile store Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 01/16] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-08-19 9:47 ` Karthik Nayak
2025-08-20 4:58 ` Patrick Steinhardt
2025-08-19 17:32 ` Junio C Hamano
2025-08-20 4:58 ` Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 02/16] odb: move list of packfiles into " Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 03/16] odb: move initialization bit " Patrick Steinhardt
2025-08-19 9:57 ` Karthik Nayak
2025-08-19 16:24 ` Junio C Hamano
2025-08-20 8:04 ` Karthik Nayak
2025-08-22 23:50 ` Junio C Hamano
2025-08-26 12:19 ` [PATCH] Documentation: note styling for bit fields Karthik Nayak
2025-08-20 4:58 ` [PATCH 03/16] odb: move initialization bit into `struct packfile_store` Patrick Steinhardt
2025-08-20 6:24 ` Junio C Hamano
2025-08-19 8:19 ` [PATCH 04/16] odb: move packfile map " Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 05/16] odb: move MRU list of packfiles " Patrick Steinhardt
2025-08-20 12:44 ` Karthik Nayak
2025-08-20 19:20 ` Jeff King
2025-08-21 6:40 ` Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 06/16] odb: move kept cache " Patrick Steinhardt
2025-08-19 18:56 ` Junio C Hamano
2025-08-20 4:58 ` Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 07/16] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-08-19 19:18 ` Junio C Hamano
2025-08-19 8:19 ` [PATCH 08/16] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 09/16] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-08-20 13:17 ` Karthik Nayak
2025-08-19 8:19 ` [PATCH 10/16] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 11/16] packfile: always add packfiles to MRU when adding a pack Patrick Steinhardt
2025-08-20 13:35 ` Karthik Nayak
2025-08-19 8:19 ` [PATCH 12/16] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-08-20 13:41 ` Karthik Nayak
2025-08-21 6:40 ` Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 13/16] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 14/16] packfile: remove `get_packed_git()` Patrick Steinhardt
2025-08-20 13:50 ` Karthik Nayak
2025-08-21 6:40 ` Patrick Steinhardt
2025-08-20 13:51 ` Karthik Nayak
2025-08-19 8:19 ` [PATCH 15/16] packfile: refactor `get_all_packs()` to work on packfile store Patrick Steinhardt
2025-08-20 13:53 ` Karthik Nayak
2025-08-21 6:40 ` Patrick Steinhardt
2025-08-19 8:19 ` [PATCH 16/16] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-08-19 17:13 ` [PATCH 00/16] packfile: carve out a new " Junio C Hamano
2025-08-20 13:55 ` Karthik Nayak
2025-08-21 7:38 ` [PATCH v2 " Patrick Steinhardt
2025-08-21 7:38 ` [PATCH v2 01/16] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-08-21 7:39 ` [PATCH v2 02/16] odb: move list of packfiles into " Patrick Steinhardt
2025-08-25 23:42 ` Taylor Blau
2025-09-02 8:50 ` Patrick Steinhardt
2025-09-02 17:21 ` Taylor Blau
2025-09-02 17:42 ` Junio C Hamano
2025-09-03 5:58 ` Patrick Steinhardt
2025-09-11 23:16 ` Taylor Blau
2025-09-15 7:44 ` Patrick Steinhardt
2025-08-21 7:39 ` [PATCH v2 03/16] odb: move initialization bit " Patrick Steinhardt
2025-08-26 1:40 ` Taylor Blau
2025-08-21 7:39 ` [PATCH v2 04/16] odb: move packfile map " Patrick Steinhardt
2025-08-26 1:41 ` Taylor Blau
2025-08-21 7:39 ` [PATCH v2 05/16] odb: move MRU list of packfiles " Patrick Steinhardt
2025-08-21 7:39 ` [PATCH v2 06/16] odb: move kept cache " Patrick Steinhardt
2025-08-26 1:46 ` Taylor Blau
2025-09-02 8:50 ` Patrick Steinhardt
2025-08-21 7:39 ` [PATCH v2 07/16] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-08-26 1:47 ` Taylor Blau
2025-08-21 7:39 ` [PATCH v2 08/16] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-08-26 1:58 ` Taylor Blau
2025-08-21 7:39 ` [PATCH v2 09/16] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-08-26 2:10 ` Taylor Blau
2025-09-02 8:50 ` Patrick Steinhardt
2025-08-21 7:39 ` [PATCH v2 10/16] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-08-26 2:11 ` Taylor Blau
2025-09-02 8:50 ` Patrick Steinhardt
2025-08-21 7:39 ` [PATCH v2 11/16] packfile: always add packfiles to MRU when adding a pack Patrick Steinhardt
2025-08-27 1:04 ` Taylor Blau
2025-09-02 8:50 ` Patrick Steinhardt
2025-08-21 7:39 ` [PATCH v2 12/16] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-08-27 1:12 ` Taylor Blau
2025-08-21 7:39 ` [PATCH v2 13/16] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-08-27 1:20 ` Taylor Blau
2025-08-21 7:39 ` [PATCH v2 14/16] packfile: remove `get_packed_git()` Patrick Steinhardt
2025-08-27 1:38 ` Taylor Blau
2025-09-02 8:50 ` Patrick Steinhardt
2025-09-11 23:25 ` Taylor Blau
2025-09-15 7:30 ` Patrick Steinhardt
2025-08-21 7:39 ` [PATCH v2 15/16] packfile: refactor `get_all_packs()` to work on packfile store Patrick Steinhardt
2025-08-27 1:45 ` Taylor Blau [this message]
2025-09-02 8:51 ` Patrick Steinhardt
2025-09-11 23:33 ` Taylor Blau
2025-09-15 7:44 ` Patrick Steinhardt
2025-08-21 7:39 ` [PATCH v2 16/16] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 00/15] packfile: carve out a new " Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 01/15] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-09-09 7:49 ` Karthik Nayak
2025-09-02 10:48 ` [PATCH v3 02/15] odb: move list of packfiles into " Patrick Steinhardt
2025-09-09 8:00 ` Karthik Nayak
2025-09-09 11:09 ` Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 03/15] odb: move initialization bit " Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 04/15] odb: move packfile map " Patrick Steinhardt
2025-09-09 8:22 ` Karthik Nayak
2025-09-09 11:01 ` Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 05/15] odb: move MRU list of packfiles " Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 06/15] odb: move kept cache " Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 07/15] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 08/15] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 09/15] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 10/15] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 11/15] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 12/15] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 13/15] packfile: remove `get_packed_git()` Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 14/15] packfile: refactor `get_all_packs()` to work on packfile store Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 15/15] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-02 16:40 ` [PATCH v3 00/15] packfile: carve out a new " Junio C Hamano
2025-09-11 23:34 ` Taylor Blau
2025-09-09 9:33 ` Karthik Nayak
2025-09-09 11:02 ` [PATCH v4 " Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 01/15] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 02/15] odb: move list of packfiles into " Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 03/15] odb: move initialization bit " Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 04/15] odb: move packfile map " Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 05/15] odb: move MRU list of packfiles " Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 06/15] odb: move kept cache " Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 07/15] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 08/15] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 09/15] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 10/15] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 11/15] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 12/15] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 13/15] packfile: remove `get_packed_git()` Patrick Steinhardt
2025-09-11 23:37 ` Taylor Blau
2025-09-09 11:03 ` [PATCH v4 14/15] packfile: refactor `get_all_packs()` to work on packfile store Patrick Steinhardt
2025-09-09 11:03 ` [PATCH v4 15/15] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-10 7:35 ` [PATCH v4 00/15] packfile: carve out a new " Karthik Nayak
2025-09-11 23:40 ` Taylor Blau
2025-09-11 23:42 ` Taylor Blau
2025-09-15 7:25 ` Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 " Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 01/15] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-09-17 21:26 ` Justin Tobler
2025-09-23 9:34 ` Patrick Steinhardt
2025-09-24 21:56 ` Justin Tobler
2025-09-15 8:54 ` [PATCH v5 02/15] odb: move list of packfiles into " Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 03/15] odb: move initialization bit " Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 04/15] odb: move packfile map " Patrick Steinhardt
2025-09-17 22:15 ` Justin Tobler
2025-09-23 9:35 ` Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 05/15] odb: move MRU list of packfiles " Patrick Steinhardt
2025-09-17 21:59 ` Justin Tobler
2025-09-15 8:54 ` [PATCH v5 06/15] odb: move kept cache " Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 07/15] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 08/15] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 09/15] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-09-17 22:32 ` Justin Tobler
2025-09-23 9:34 ` Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 10/15] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 11/15] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 12/15] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 13/15] packfile: refactor `get_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 14/15] packfile: refactor `get_all_packs()` " Patrick Steinhardt
2025-09-15 8:54 ` [PATCH v5 15/15] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-23 10:16 ` [PATCH v6 00/15] packfile: carve out a new " Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 01/15] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 02/15] odb: move list of packfiles into " Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 03/15] odb: move initialization bit " Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 04/15] odb: move packfile map " Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 05/15] odb: move MRU list of packfiles " Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 06/15] odb: move kept cache " Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 07/15] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 08/15] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 09/15] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 10/15] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 11/15] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 12/15] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 13/15] packfile: refactor `get_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 14/15] packfile: refactor `get_all_packs()` " Patrick Steinhardt
2025-09-23 10:17 ` [PATCH v6 15/15] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-24 21:58 ` [PATCH v6 00/15] packfile: carve out a new " Justin Tobler
2025-09-25 16:08 ` Junio C Hamano
2025-09-26 5:26 ` Patrick Steinhardt
2025-09-28 22:05 ` Taylor Blau
2025-09-29 21:39 ` Patrick Steinhardt
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=aK5jK2Q3McLnJ9Uw@nand.local \
--to=me@ttaylorr$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=karthik.188@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
--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