Re: [PATCH 02/49] builtin/repack.c: avoid "the_repository" in existing packs API
From: Jeff King <hidden>
Date: 2025-10-10 05:19:57
On Sun, Sep 28, 2025 at 06:07:18PM -0400, Taylor Blau wrote:
quoted hunk ↗ jump to hunk
struct existing_packs { + struct repository *repo; struct string_list kept_packs; struct string_list non_kept_packs; struct string_list cruft_packs;@@ -265,7 +266,7 @@ static void existing_packs_release(struct existing_packs *existing) static void collect_pack_filenames(struct existing_packs *existing, const struct string_list *extra_keep) { - struct packfile_store *packs = the_repository->objects->packfiles; + struct packfile_store *packs = existing->repo->objects->packfiles;
I found it a little funny to pass around a repository struct as part of existing_packs, since they're not directly related. But I think it is mostly just a convenience to do so, and "existing_packs" is really storing an overall context. It could probably be scoped down to pass around an object_database instead, or maybe even a packfile store, which somehow feels a little more "correct" to me as context. But I have trouble imagining that being helpful to any new code (why would it have an object_store but a not a repo object?). And I can easily imagine having the repo available being useful for future refactorings. So I think what you've written here is probably a good path forward. I wanted to outline my thinking because I suspect the same will apply to other patches in this series (i.e., they don't always need a repo object, but it's simplest to pass them one). -Peff