{"thread":{"id":"64263","subject":"[PATCH 0/6] packfile: remove `packfile_store_get_packs()`","startedAt":"2025-10-07T12:41:13Z","lastAt":"2025-10-14T19:19:29Z","messageCount":27,"participants":["Patrick Steinhardt","Taylor Blau","Justin Tobler"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"528105","messageId":"20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im","threadId":"64263","inReplyTo":null,"subject":"[PATCH 0/6] packfile: remove `packfile_store_get_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:41:06Z","receivedAt":"2025-10-07T12:41:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series follows up on the discussion at [1]. Originally, these\npatches were part of the series that introduced the packfile store, but\nwe decided to split them out into a separate series.\n\nIn any case, the goal of this series is to remove the confusing\ndifference between `packfiles_store_get_{,all_}packs()`. It's not really\nobvious to the caller what the difference is. But even worse, the result\nof these functions depends on whether or not `get_all_packs()` was ever\nexecuted before `get_packs()`.\n\nThe series is built on top of 821f583da6 (The thirteenth batcn,\n2025-09-29) with tb/incremental-midx-part-3.1 at c886af90f8 (SQUASH???\nplay well with other topics by preemptively including \"repository.h\",\n2025-09-29) merged into it. This is done to fix some minor conflicts\nwith that patch series.\n\nThanks!\n\nPatrick\n\n[1]: \n\n---\nPatrick Steinhardt (6):\n object-name: convert to use `packfile_store_get_all_packs()`\n builtin/gc: convert to use `packfile_store_get_all_packs()`\n builtin/grep: simplify how we preload packs\n packfile: drop `packfile_store_get_packs()`\n packfile: introduce macro to iterate through packs\n packfile: rename `packfile_store_get_all_packs()`\n\n builtin/cat-file.c | 2 +-\n builtin/count-objects.c | 2 +-\n builtin/fast-import.c | 4 ++--\n builtin/fsck.c | 12 ++++++------\n builtin/gc.c | 14 +++++++-------\n builtin/grep.c | 2 +-\n builtin/pack-objects.c | 18 +++++++++---------\n builtin/pack-redundant.c | 12 ++++--------\n connected.c | 2 +-\n http-backend.c | 4 ++--\n http.c | 2 +-\n object-name.c | 8 +++++---\n pack-bitmap.c | 6 +++---\n pack-objects.c | 4 ++--\n packfile.c | 10 ++--------\n packfile.h | 8 ++++----\n repack-cruft.c | 2 +-\n repack-geometry.c | 2 +-\n repack.c | 2 +-\n server-info.c | 2 +-\n t/helper/test-find-pack.c | 3 ++-\n t/helper/test-pack-mtimes.c | 2 +-\n 22 files changed, 58 insertions(+), 65 deletions(-)\n\n\n---\nbase-commit: a0cb7ee3bf6c8398ab18e7b0dfabec106312b2f8\nchange-id: 20250926-pks-packfiles-convert-get-all-4236f6e2ed9b\n\n"},{"id":"528106","messageId":"20251007-pks-packfiles-convert-get-all-v1-1-428227657a89@pks.im","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im","subject":"[PATCH 1/6] object-name: convert to use `packfile_store_get_all_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:41:07Z","receivedAt":"2025-10-07T12:41:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When searching for abbreviated or when trying to disambiguate object IDs\nwe do this in two steps:\n\n 1. We search through the multi-pack index.\n\n 2. We search through all packfiles not part of any multi-pack index.\n\nThe second step uses `packfile_store_get_packs()`, which knows to skip\nloading any packfiles that are indexed by an MIDX; this is exactly what\nwe want.\n\nBut that function is somewhat problematic, as its behaviour is stateful\nand is influenced by `packfile_store_get_all_packs()`. This function\nbasically does the same as `packfile_store_get_packs()`, but in addition\nit also loads all packfiles indexed by an MIDX. The problem here is that\nboth of these functions act on the same linked list of packfiles, and\nthus depending on whether or not `get_all_packs()` was called the result\nreturned by `get_packs()` will be different. Consequently, all callers\nof `get_packs()` need to be prepared to see MIDX'd packs even though\nthese should in theory be excluded.\n\nThis interface is confusing and thus potentially dangerous, which is why\nwe're converting all callers of `get_packs()` to use `get_all_packs()`\ninstead.\n\nDo so for the above functions in \"object-name.c\". As explained, we\nalready know to skip any MIDX'd packs in both `find_abbrev_len_packed()`\nand `find_short_packed_object()`, so it's fine to start loading MIDX'd\npackfiles.\n\nSigned-off-by: Patrick Steinhardt \n---\n object-name.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex f6902e140d..4e62bfa330 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -213,7 +213,7 @@ static void find_short_packed_object(struct disambiguate_state *ds)\n \t\t\tunique_in_midx(m, ds);\n \t}\n \n-\tfor (p = packfile_store_get_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n+\tfor (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n \t p = p->next)\n \t\tunique_in_pack(p, ds);\n }\n@@ -805,7 +805,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)\n \t\t\tfind_abbrev_len_for_midx(m, mad);\n \t}\n \n-\tfor (p = packfile_store_get_packs(mad->repo->objects->packfiles); p; p = p->next)\n+\tfor (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next)\n \t\tfind_abbrev_len_for_pack(p, mad);\n }\n \n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528107","messageId":"20251007-pks-packfiles-convert-get-all-v1-2-428227657a89@pks.im","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im","subject":"[PATCH 2/6] builtin/gc: convert to use `packfile_store_get_all_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:41:08Z","receivedAt":"2025-10-07T12:41:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When running maintenance tasks via git-maintenance(1) we have a couple\nof auto-conditions that check whether or not a specific task should be\nrunning. One such check is for incremental repacks, which essentially\nuse `git multi-pack-index repack` to repack a set of smaller packfiles\ninto one larger packfile.\n\nThe auto-condition for this task checks how many packfiles there are\nthat aren't indexed by any multi-pack index. If there is a sufficient\nnumber then we execute the above command to combine those into a single\npack and add them to the MIDX.\n\nAs we don't care about MIDX'd packs we use `packfile_store_get_packs()`,\nwhich knows to not load any packs that are indexed by a MIDX. But as\nexplained in the preceding commit, we want to get rid of that function.\n\nWe already handle packfiles that have an MIDX alright by the very nature\nof this function, as we explicitly count non-MIDX'd packs. As such, we\ncan trivially switch over to use `packfile_store_get_all_packs()`\ninstead.\n\nDo so.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/gc.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex e19e13d978..ab6d6d3bd1 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1425,7 +1425,7 @@ static int incremental_repack_auto_condition(struct gc_config *cfg UNUSED)\n \tif (incremental_repack_auto_limit < 0)\n \t\treturn 1;\n \n-\tfor (p = packfile_store_get_packs(the_repository->objects->packfiles);\n+\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles);\n \t count < incremental_repack_auto_limit && p;\n \t p = p->next) {\n \t\tif (!p->multi_pack_index)\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528108","messageId":"20251007-pks-packfiles-convert-get-all-v1-3-428227657a89@pks.im","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im","subject":"[PATCH 3/6] builtin/grep: simplify how we preload packs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:41:09Z","receivedAt":"2025-10-07T12:41:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When using multiple threads in git-grep(1) we eagerly preload both the\ngitmodules file as well as the packfiles so that the threads won't race\nwith one another to initialize these data structures.\n\nFor packfiles, this is done by calling `packfile_store_get_packs()`,\nwhich first loads our packfiles and then returns a pointer to the first\nsuch packfile. This pointer is ignored though, as all we really care\nabout is that `packfile_store_prepare()` was called.\n\nHistoryically, that function was file-local to \"packfile.c\", but that\nchanged with 4188332569 (packfile: move `get_multi_pack_index()` into\n\"midx.c\", 2025-09-02). We can thus simplify the code by calling that\nfunction directly.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/grep.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 13841fbf00..53cccf2d25 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -1214,7 +1214,7 @@ int cmd_grep(int argc,\n \t\tif (recurse_submodules)\n \t\t\trepo_read_gitmodules(the_repository, 1);\n \t\tif (startup_info->have_repository)\n-\t\t\t(void)packfile_store_get_packs(the_repository->objects->packfiles);\n+\t\t\tpackfile_store_prepare(the_repository->objects->packfiles);\n \n \t\tstart_threads(&opt);\n \t} else {\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528109","messageId":"20251007-pks-packfiles-convert-get-all-v1-4-428227657a89@pks.im","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im","subject":"[PATCH 4/6] packfile: drop `packfile_store_get_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:41:10Z","receivedAt":"2025-10-07T12:41:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In the preceding commits we have removed all remaining callers of\n`packfile_store_get_packs()`, the function is thus unused now. Remove\nit.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 6 ------\n packfile.h | 6 ------\n 2 files changed, 12 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 5a7caec292..db748b0bd4 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1027,12 +1027,6 @@ void packfile_store_reprepare(struct packfile_store *store)\n \tpackfile_store_prepare(store);\n }\n \n-struct packed_git *packfile_store_get_packs(struct packfile_store *store)\n-{\n-\tpackfile_store_prepare(store);\n-\treturn store->packs;\n-}\n-\n struct packed_git *packfile_store_get_all_packs(struct packfile_store *store)\n {\n \tpackfile_store_prepare(store);\ndiff --git a/packfile.h b/packfile.h\nindex e7a5792b6c..3f38c63476 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -136,12 +136,6 @@ void packfile_store_reprepare(struct packfile_store *store);\n void packfile_store_add_pack(struct packfile_store *store,\n \t\t\t struct packed_git *pack);\n \n-/*\n- * Get packs managed by the given store. Does not load the MIDX or any packs\n- * referenced by it.\n- */\n-struct packed_git *packfile_store_get_packs(struct packfile_store *store);\n-\n /*\n * Get all packs managed by the given store, including packfiles that are\n * referenced by multi-pack indices.\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528110","messageId":"20251007-pks-packfiles-convert-get-all-v1-5-428227657a89@pks.im","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im","subject":"[PATCH 5/6] packfile: introduce macro to iterate through packs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:41:11Z","receivedAt":"2025-10-07T12:41:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We have a bunch of different sites that want to iterate through all\npacks of a given `struct packfile_store`. This pattern is somewhat\nverbose and repetitive, which makes it somewhat cumbersome.\n\nIntroduce a new macro `packfile_store_for_each_pack()` that removes some\nof the boilerplate.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/cat-file.c | 2 +-\n builtin/count-objects.c | 2 +-\n builtin/fsck.c | 12 ++++++------\n builtin/gc.c | 14 +++++++-------\n builtin/pack-objects.c | 14 +++++++-------\n builtin/pack-redundant.c | 12 ++++--------\n connected.c | 2 +-\n http-backend.c | 4 ++--\n http.c | 2 +-\n object-name.c | 8 +++++---\n pack-bitmap.c | 6 +++---\n pack-objects.c | 4 ++--\n packfile.c | 4 ++--\n packfile.h | 6 ++++++\n repack-cruft.c | 2 +-\n repack-geometry.c | 2 +-\n repack.c | 2 +-\n server-info.c | 2 +-\n t/helper/test-find-pack.c | 3 ++-\n t/helper/test-pack-mtimes.c | 2 +-\n 20 files changed, 55 insertions(+), 50 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex ee6715fa52..a565c5f02b 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -855,7 +855,7 @@ static void batch_each_object(struct batch_options *opt,\n \t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *pack;\n \n-\t\tfor (pack = packfile_store_get_all_packs(packs); pack; pack = pack->next) {\n+\t\tpackfile_store_for_each_pack(packs, pack) {\n \t\t\tif (bitmap_index_contains_pack(bitmap, pack) ||\n \t\t\t open_pack_index(pack))\n \t\t\t\tcontinue;\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex f2f407c2a7..09e3696823 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -130,7 +130,7 @@ int cmd_count_objects(int argc,\n \t\tstruct strbuf pack_buf = STRBUF_INIT;\n \t\tstruct strbuf garbage_buf = STRBUF_INIT;\n \n-\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\t\tpackfile_store_for_each_pack(packs, p) {\n \t\t\tif (!p->pack_local)\n \t\t\t\tcontinue;\n \t\t\tif (open_pack_index(p))\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 8ee95e0d67..5462c442dc 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -869,18 +869,19 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress)\n {\n \tstruct packfile_store *packs = r->objects->packfiles;\n \tstruct progress *progress = NULL;\n+\tstruct packed_git *p;\n \tuint32_t pack_count = 0;\n \tint res = 0;\n \n \tif (show_progress) {\n-\t\tfor (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next)\n+\t\tpackfile_store_for_each_pack(packs, p)\n \t\t\tpack_count++;\n \t\tprogress = start_delayed_progress(the_repository,\n \t\t\t\t\t\t \"Verifying reverse pack-indexes\", pack_count);\n \t\tpack_count = 0;\n \t}\n \n-\tfor (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tint load_error = load_pack_revindex_from_disk(p);\n \n \t\tif (load_error < 0) {\n@@ -1012,8 +1013,7 @@ int cmd_fsck(int argc,\n \t\t\tstruct progress *progress = NULL;\n \n \t\t\tif (show_progress) {\n-\t\t\t\tfor (p = packfile_store_get_all_packs(packs); p;\n-\t\t\t\t p = p->next) {\n+\t\t\t\tpackfile_store_for_each_pack(packs, p) {\n \t\t\t\t\tif (open_pack_index(p))\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\ttotal += p->num_objects;\n@@ -1022,8 +1022,8 @@ int cmd_fsck(int argc,\n \t\t\t\tprogress = start_progress(the_repository,\n \t\t\t\t\t\t\t _(\"Checking objects\"), total);\n \t\t\t}\n-\t\t\tfor (p = packfile_store_get_all_packs(packs); p;\n-\t\t\t p = p->next) {\n+\n+\t\t\tpackfile_store_for_each_pack(packs, p) {\n \t\t\t\t/* verify gives error messages itself */\n \t\t\t\tif (verify_pack(the_repository,\n \t\t\t\t\t\tp, fsck_obj_buffer,\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex ab6d6d3bd1..16fc56e5cc 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -490,7 +490,7 @@ static struct packed_git *find_base_packs(struct string_list *packs,\n \tstruct packfile_store *packfiles = the_repository->objects->packfiles;\n \tstruct packed_git *p, *base = NULL;\n \n-\tfor (p = packfile_store_get_all_packs(packfiles); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packfiles, p) {\n \t\tif (!p->pack_local || p->is_cruft)\n \t\t\tcontinue;\n \t\tif (limit) {\n@@ -511,12 +511,12 @@ static int too_many_packs(struct gc_config *cfg)\n {\n \tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct packed_git *p;\n-\tint cnt;\n+\tint cnt = 0;\n \n \tif (cfg->gc_auto_pack_limit <= 0)\n \t\treturn 0;\n \n-\tfor (cnt = 0, p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tif (!p->pack_local)\n \t\t\tcontinue;\n \t\tif (p->pack_keep)\n@@ -1425,9 +1425,9 @@ static int incremental_repack_auto_condition(struct gc_config *cfg UNUSED)\n \tif (incremental_repack_auto_limit < 0)\n \t\treturn 1;\n \n-\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles);\n-\t count < incremental_repack_auto_limit && p;\n-\t p = p->next) {\n+\tpackfile_store_for_each_pack(the_repository->objects->packfiles, p) {\n+\t\tif (count >= incremental_repack_auto_limit)\n+\t\t\tbreak;\n \t\tif (!p->multi_pack_index)\n \t\t\tcount++;\n \t}\n@@ -1494,7 +1494,7 @@ static off_t get_auto_pack_size(void)\n \tstruct repository *r = the_repository;\n \n \todb_reprepare(r->objects);\n-\tfor (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {\n+\tpackfile_store_for_each_pack(r->objects->packfiles, p) {\n \t\tif (p->pack_size > max_size) {\n \t\t\tsecond_largest_size = max_size;\n \t\t\tmax_size = p->pack_size;\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex fe987fbb8b..65fb70f806 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3856,7 +3856,7 @@ static void read_packs_list_from_stdin(struct rev_info *revs)\n \tstring_list_sort(&exclude_packs);\n \tstring_list_remove_duplicates(&exclude_packs, 0);\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tconst char *pack_name = pack_basename(p);\n \n \t\tif ((item = string_list_lookup(&include_packs, pack_name)))\n@@ -4107,7 +4107,7 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs\n \t * Re-mark only the fresh packs as kept so that objects in\n \t * unknown packs do not halt the reachability traversal early.\n \t */\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next)\n+\tpackfile_store_for_each_pack(packs, p)\n \t\tp->pack_keep_in_core = 0;\n \tmark_pack_kept_in_core(fresh_packs, 1);\n \n@@ -4145,7 +4145,7 @@ static void read_cruft_objects(void)\n \tstring_list_sort(&discard_packs);\n \tstring_list_sort(&fresh_packs);\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tconst char *pack_name = pack_basename(p);\n \t\tstruct string_list_item *item;\n \n@@ -4446,7 +4446,7 @@ static void loosen_unused_packed_objects(void)\n \tuint32_t loosened_objects_nr = 0;\n \tstruct object_id oid;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tif (!p->pack_local || p->pack_keep || p->pack_keep_in_core)\n \t\t\tcontinue;\n \n@@ -4753,7 +4753,7 @@ static void add_extra_kept_packs(const struct string_list *names)\n \tif (!names->nr)\n \t\treturn;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tconst char *name = basename(p->pack_name);\n \t\tint i;\n \n@@ -5194,7 +5194,7 @@ int cmd_pack_objects(int argc,\n \t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *p;\n \n-\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next)\n+\t\tpackfile_store_for_each_pack(packs, p)\n \t\t\tif (p->pack_local && p->pack_keep)\n \t\t\t\tbreak;\n \t\tif (!p) /* no keep-able packs found */\n@@ -5209,7 +5209,7 @@ int cmd_pack_objects(int argc,\n \t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *p;\n \n-\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\t\tpackfile_store_for_each_pack(packs, p) {\n \t\t\tif (!p->pack_local) {\n \t\t\t\thave_non_local_packs = 1;\n \t\t\t\tbreak;\ndiff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c\nindex dd28171f0a..035a2c86a2 100644\n--- a/builtin/pack-redundant.c\n+++ b/builtin/pack-redundant.c\n@@ -567,28 +567,24 @@ static struct pack_list * add_pack(struct packed_git *p)\n static struct pack_list * add_pack_file(const char *filename)\n {\n \tstruct packfile_store *packs = the_repository->objects->packfiles;\n-\tstruct packed_git *p = packfile_store_get_all_packs(packs);\n+\tstruct packed_git *p;\n \n \tif (strlen(filename) < 40)\n \t\tdie(\"Bad pack filename: %s\", filename);\n \n-\twhile (p) {\n+\tpackfile_store_for_each_pack(packs, p)\n \t\tif (strstr(p->pack_name, filename))\n \t\t\treturn add_pack(p);\n-\t\tp = p->next;\n-\t}\n \tdie(\"Filename %s not found in packed_git\", filename);\n }\n \n static void load_all(void)\n {\n \tstruct packfile_store *packs = the_repository->objects->packfiles;\n-\tstruct packed_git *p = packfile_store_get_all_packs(packs);\n+\tstruct packed_git *p;\n \n-\twhile (p) {\n+\tpackfile_store_for_each_pack(packs, p)\n \t\tadd_pack(p);\n-\t\tp = p->next;\n-\t}\n }\n \n int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, struct repository *repo UNUSED) {\ndiff --git a/connected.c b/connected.c\nindex b288a18b17..e6b048d176 100644\n--- a/connected.c\n+++ b/connected.c\n@@ -77,7 +77,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,\n \t\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\t\tstruct packed_git *p;\n \n-\t\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\t\t\tpackfile_store_for_each_pack(packs, p) {\n \t\t\t\tif (!p->pack_promisor)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (find_pack_entry_one(oid, p))\ndiff --git a/http-backend.c b/http-backend.c\nindex 9084058f1e..8ba2d37dad 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -609,13 +609,13 @@ static void get_info_packs(struct strbuf *hdr, char *arg UNUSED)\n \tsize_t cnt = 0;\n \n \tselect_getanyfile(hdr);\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tif (p->pack_local)\n \t\t\tcnt++;\n \t}\n \n \tstrbuf_grow(&buf, cnt * 53 + 2);\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tif (p->pack_local)\n \t\t\tstrbuf_addf(&buf, \"P %s\\n\", p->pack_name + objdirlen + 6);\n \t}\ndiff --git a/http.c b/http.c\nindex 7e3af1e72f..2730352f0e 100644\n--- a/http.c\n+++ b/http.c\n@@ -2425,7 +2425,7 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head,\n \t * If we already have the pack locally, no need to fetch its index or\n \t * even add it to list; we already have all of its objects.\n \t */\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tif (hasheq(p->hash, sha1, the_repository->hash_algo))\n \t\t\treturn 0;\n \t}\ndiff --git a/object-name.c b/object-name.c\nindex 4e62bfa330..e998dc31b4 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -213,9 +213,11 @@ static void find_short_packed_object(struct disambiguate_state *ds)\n \t\t\tunique_in_midx(m, ds);\n \t}\n \n-\tfor (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n-\t p = p->next)\n+\tpackfile_store_for_each_pack(ds->repo->objects->packfiles, p) {\n+\t\tif (ds->ambiguous)\n+\t\t\tbreak;\n \t\tunique_in_pack(p, ds);\n+\t}\n }\n \n static int finish_object_disambiguation(struct disambiguate_state *ds,\n@@ -805,7 +807,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)\n \t\t\tfind_abbrev_len_for_midx(m, mad);\n \t}\n \n-\tfor (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next)\n+\tpackfile_store_for_each_pack(mad->repo->objects->packfiles, p)\n \t\tfind_abbrev_len_for_pack(p, mad);\n }\n \ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex ac71035d77..2e30066b27 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -664,7 +664,7 @@ static int open_pack_bitmap(struct repository *r,\n \tstruct packed_git *p;\n \tint ret = -1;\n \n-\tfor (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {\n+\tpackfile_store_for_each_pack(r->objects->packfiles, p) {\n \t\tif (open_pack_bitmap_1(bitmap_git, p) == 0) {\n \t\t\tret = 0;\n \t\t\t/*\n@@ -3347,6 +3347,7 @@ static int verify_bitmap_file(const struct git_hash_algo *algop,\n int verify_bitmap_files(struct repository *r)\n {\n \tstruct odb_source *source;\n+\tstruct packed_git *p;\n \tint res = 0;\n \n \todb_prepare_alternates(r->objects);\n@@ -3362,8 +3363,7 @@ int verify_bitmap_files(struct repository *r)\n \t\tfree(midx_bitmap_name);\n \t}\n \n-\tfor (struct packed_git *p = packfile_store_get_all_packs(r->objects->packfiles);\n-\t p; p = p->next) {\n+\tpackfile_store_for_each_pack(r->objects->packfiles, p) {\n \t\tchar *pack_bitmap_name = pack_bitmap_filename(p);\n \t\tres |= verify_bitmap_file(r->hash_algo, pack_bitmap_name);\n \t\tfree(pack_bitmap_name);\ndiff --git a/pack-objects.c b/pack-objects.c\nindex d8eb679735..2dfaae4886 100644\n--- a/pack-objects.c\n+++ b/pack-objects.c\n@@ -96,13 +96,13 @@ static void prepare_in_pack_by_idx(struct packing_data *pdata)\n \t * (i.e. in_pack_idx also zero) should return NULL.\n \t */\n \tmapping[cnt++] = NULL;\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next, cnt++) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tif (cnt == nr) {\n \t\t\tfree(mapping);\n \t\t\treturn;\n \t\t}\n \t\tp->index = cnt;\n-\t\tmapping[cnt] = p;\n+\t\tmapping[cnt++] = p;\n \t}\n \tpdata->in_pack_by_idx = mapping;\n }\ndiff --git a/packfile.c b/packfile.c\nindex db748b0bd4..00abb058db 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2099,7 +2099,7 @@ struct packed_git **kept_pack_cache(struct repository *r, unsigned flags)\n \t\t * covers, one kept and one not kept, but the midx returns only\n \t\t * the non-kept version.\n \t\t */\n-\t\tfor (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {\n+\t\tpackfile_store_for_each_pack(r->objects->packfiles, p) {\n \t\t\tif ((p->pack_keep && (flags & ON_DISK_KEEP_PACKS)) ||\n \t\t\t (p->pack_keep_in_core && (flags & IN_CORE_KEEP_PACKS))) {\n \t\t\t\tALLOC_GROW(packs, nr + 1, alloc);\n@@ -2196,7 +2196,7 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb,\n \tint r = 0;\n \tint pack_errors = 0;\n \n-\tfor (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next) {\n+\tpackfile_store_for_each_pack(repo->objects->packfiles, p) {\n \t\tif ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)\n \t\t\tcontinue;\n \t\tif ((flags & FOR_EACH_OBJECT_PROMISOR_ONLY) &&\ndiff --git a/packfile.h b/packfile.h\nindex 3f38c63476..b80f79c9aa 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -136,6 +136,12 @@ void packfile_store_reprepare(struct packfile_store *store);\n void packfile_store_add_pack(struct packfile_store *store,\n \t\t\t struct packed_git *pack);\n \n+/*\n+ * Load and iterate through all packs of the given packfile store.\n+ */\n+#define packfile_store_for_each_pack(store, p) \\\n+\tfor (p = packfile_store_get_all_packs(store); p; p = p->next)\n+\n /*\n * Get all packs managed by the given store, including packfiles that are\n * referenced by multi-pack indices.\ndiff --git a/repack-cruft.c b/repack-cruft.c\nindex accb98bcdb..34e7a640de 100644\n--- a/repack-cruft.c\n+++ b/repack-cruft.c\n@@ -12,7 +12,7 @@ static void combine_small_cruft_packs(FILE *in, off_t combine_cruft_below_size,\n \tstruct strbuf buf = STRBUF_INIT;\n \tsize_t i;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tif (!(p->is_cruft && p->pack_local))\n \t\t\tcontinue;\n \ndiff --git a/repack-geometry.c b/repack-geometry.c\nindex 56d651c9ce..7bb940ad10 100644\n--- a/repack-geometry.c\n+++ b/repack-geometry.c\n@@ -33,7 +33,7 @@ void pack_geometry_init(struct pack_geometry *geometry,\n \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tif (args->local && !p->pack_local)\n \t\t\t/*\n \t\t\t * When asked to only repack local packfiles we skip\ndiff --git a/repack.c b/repack.c\nindex 1982a48165..f169df86ce 100644\n--- a/repack.c\n+++ b/repack.c\n@@ -128,7 +128,7 @@ void existing_packs_collect(struct existing_packs *existing,\n \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\tsize_t i;\n \t\tconst char *base;\n \ndiff --git a/server-info.c b/server-info.c\nindex 1d33de821e..804bc3c7c4 100644\n--- a/server-info.c\n+++ b/server-info.c\n@@ -293,7 +293,7 @@ static void init_pack_info(struct repository *r, const char *infofile, int force\n \tint i;\n \tsize_t alloc = 0;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\tpackfile_store_for_each_pack(packs, p) {\n \t\t/* we ignore things on alternate path since they are\n \t\t * not available to the pullers in general.\n \t\t */\ndiff --git a/t/helper/test-find-pack.c b/t/helper/test-find-pack.c\nindex e001dc3066..fdc29a6b69 100644\n--- a/t/helper/test-find-pack.c\n+++ b/t/helper/test-find-pack.c\n@@ -39,11 +39,12 @@ int cmd__find_pack(int argc, const char **argv)\n \tif (repo_get_oid(the_repository, argv[0], &oid))\n \t\tdie(\"cannot parse %s as an object name\", argv[0]);\n \n-\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next)\n+\tpackfile_store_for_each_pack(the_repository->objects->packfiles, p) {\n \t\tif (find_pack_entry_one(&oid, p)) {\n \t\t\tprintf(\"%s\\n\", p->pack_name);\n \t\t\tactual_count++;\n \t\t}\n+\t}\n \n \tif (count > -1 && count != actual_count)\n \t\tdie(\"bad packfile count %d instead of %d\", actual_count, count);\ndiff --git a/t/helper/test-pack-mtimes.c b/t/helper/test-pack-mtimes.c\nindex 7c428c1601..6322473fbf 100644\n--- a/t/helper/test-pack-mtimes.c\n+++ b/t/helper/test-pack-mtimes.c\n@@ -37,7 +37,7 @@ int cmd__pack_mtimes(int argc, const char **argv)\n \tif (argc != 2)\n \t\tusage(pack_mtimes_usage);\n \n-\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next) {\n+\tpackfile_store_for_each_pack(the_repository->objects->packfiles, p) {\n \t\tstrbuf_addstr(&buf, basename(p->pack_name));\n \t\tstrbuf_strip_suffix(&buf, \".pack\");\n \t\tstrbuf_addstr(&buf, \".mtimes\");\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528111","messageId":"20251007-pks-packfiles-convert-get-all-v1-6-428227657a89@pks.im","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im","subject":"[PATCH 6/6] packfile: rename `packfile_store_get_all_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:41:12Z","receivedAt":"2025-10-07T12:41:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In a preceding commit we have removed `packfile_store_get_packs()`. With\nthis function removed it's somewhat useless to still have the \"all\"\ninfix in `packfile_store_get_all_packs()`. Rename the latter to drop\nthat infix.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/fast-import.c | 4 ++--\n builtin/pack-objects.c | 4 ++--\n packfile.c | 2 +-\n packfile.h | 4 ++--\n 4 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex b1d5549815..fea914cf9e 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -976,7 +976,7 @@ static int store_object(\n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[type]++;\n \t\treturn 1;\n-\t} else if (find_oid_pack(&oid, packfile_store_get_all_packs(packs))) {\n+\t} else if (find_oid_pack(&oid, packfile_store_get_packs(packs))) {\n \t\te->type = type;\n \t\te->pack_id = MAX_PACK_ID;\n \t\te->idx.offset = 1; /* just not zero! */\n@@ -1177,7 +1177,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)\n \t\tduplicate_count_by_type[OBJ_BLOB]++;\n \t\ttruncate_pack(&checkpoint);\n \n-\t} else if (find_oid_pack(&oid, packfile_store_get_all_packs(packs))) {\n+\t} else if (find_oid_pack(&oid, packfile_store_get_packs(packs))) {\n \t\te->type = OBJ_BLOB;\n \t\te->pack_id = MAX_PACK_ID;\n \t\te->idx.offset = 1; /* just not zero! */\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 65fb70f806..821598c261 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -4398,7 +4398,7 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)\n \tstruct packed_git *p;\n \n \tp = (last_found != (void *)1) ? last_found :\n-\t\t\t\t\tpackfile_store_get_all_packs(packs);\n+\t\t\t\t\tpackfile_store_get_packs(packs);\n \n \twhile (p) {\n \t\tif ((!p->pack_local || p->pack_keep ||\n@@ -4408,7 +4408,7 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)\n \t\t\treturn 1;\n \t\t}\n \t\tif (p == last_found)\n-\t\t\tp = packfile_store_get_all_packs(packs);\n+\t\t\tp = packfile_store_get_packs(packs);\n \t\telse\n \t\t\tp = p->next;\n \t\tif (p == last_found)\ndiff --git a/packfile.c b/packfile.c\nindex 00abb058db..45a75c0307 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1027,7 +1027,7 @@ void packfile_store_reprepare(struct packfile_store *store)\n \tpackfile_store_prepare(store);\n }\n \n-struct packed_git *packfile_store_get_all_packs(struct packfile_store *store)\n+struct packed_git *packfile_store_get_packs(struct packfile_store *store)\n {\n \tpackfile_store_prepare(store);\n \ndiff --git a/packfile.h b/packfile.h\nindex b80f79c9aa..a8f96b1459 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -140,13 +140,13 @@ void packfile_store_add_pack(struct packfile_store *store,\n * Load and iterate through all packs of the given packfile store.\n */\n #define packfile_store_for_each_pack(store, p) \\\n-\tfor (p = packfile_store_get_all_packs(store); p; p = p->next)\n+\tfor (p = packfile_store_get_packs(store); p; p = p->next)\n \n /*\n * Get all packs managed by the given store, including packfiles that are\n * referenced by multi-pack indices.\n */\n-struct packed_git *packfile_store_get_all_packs(struct packfile_store *store);\n+struct packed_git *packfile_store_get_packs(struct packfile_store *store);\n \n /*\n * Get all packs in most-recently-used order.\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528306","messageId":"aObHw6rxrQG22b5B@nand.local","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-1-428227657a89@pks.im","subject":"Re: [PATCH 1/6] object-name: convert to use `packfile_store_get_all_packs()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-10-08T20:21:23Z","receivedAt":"2025-10-08T20:21:28Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 07, 2025 at 02:41:07PM +0200, Patrick Steinhardt wrote:\n> When searching for abbreviated or when trying to disambiguate object IDs\n> we do this in two steps:\n>\n> 1. We search through the multi-pack index.\n>\n> 2. We search through all packfiles not part of any multi-pack index.\n>\n> The second step uses `packfile_store_get_packs()`, which knows to skip\n> loading any packfiles that are indexed by an MIDX; this is exactly what\n> we want.\n>\n> But that function is somewhat problematic, as its behaviour is stateful\n> and is influenced by `packfile_store_get_all_packs()`. This function\n> basically does the same as `packfile_store_get_packs()`, but in addition\n> it also loads all packfiles indexed by an MIDX. The problem here is that\n> both of these functions act on the same linked list of packfiles, and\n> thus depending on whether or not `get_all_packs()` was called the result\n> returned by `get_packs()` will be different. Consequently, all callers\n> of `get_packs()` need to be prepared to see MIDX'd packs even though\n> these should in theory be excluded.\n>\n> This interface is confusing and thus potentially dangerous, which is why\n> we're converting all callers of `get_packs()` to use `get_all_packs()`\n> instead.\n\nWell explained.\n\n> diff --git a/object-name.c b/object-name.c\n> index f6902e140d..4e62bfa330 100644\n> --- a/object-name.c\n> +++ b/object-name.c\n> @@ -213,7 +213,7 @@ static void find_short_packed_object(struct disambiguate_state *ds)\n> \t\t\tunique_in_midx(m, ds);\n> \t}\n>\n> -\tfor (p = packfile_store_get_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n> +\tfor (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n> \t p = p->next)\n> \t\tunique_in_pack(p, ds);\n> }\n\nGood. As you noted, this function already handles MIDX'd packs as a\nseparate case, and `unique_in_pack()` discards any packs that have\ntheir `multi_pack_index` bit set. So this caller can be converted\nwithout issue.\n\n> @@ -805,7 +805,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)\n> \t\t\tfind_abbrev_len_for_midx(m, mad);\n> \t}\n>\n> -\tfor (p = packfile_store_get_packs(mad->repo->objects->packfiles); p; p = p->next)\n> +\tfor (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next)\n> \t\tfind_abbrev_len_for_pack(p, mad);\n> }\n\nAnd the exact same is true for this case as well. Looking good.\n\nThanks,\nTaylor\n"},{"id":"528307","messageId":"aObIw/et7DxB/NrZ@nand.local","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-2-428227657a89@pks.im","subject":"Re: [PATCH 2/6] builtin/gc: convert to use `packfile_store_get_all_packs()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-10-08T20:25:39Z","receivedAt":"2025-10-08T20:25:42Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 07, 2025 at 02:41:08PM +0200, Patrick Steinhardt wrote:\n> When running maintenance tasks via git-maintenance(1) we have a couple\n> of auto-conditions that check whether or not a specific task should be\n> running. One such check is for incremental repacks, which essentially\n> use `git multi-pack-index repack` to repack a set of smaller packfiles\n> into one larger packfile.\n>\n> The auto-condition for this task checks how many packfiles there are\n> that aren't indexed by any multi-pack index. If there is a sufficient\n> number then we execute the above command to combine those into a single\n> pack and add them to the MIDX.\n\ns/them/that pack/\n\n> As we don't care about MIDX'd packs we use `packfile_store_get_packs()`,\n> which knows to not load any packs that are indexed by a MIDX. But as\n> explained in the preceding commit, we want to get rid of that function.\n>\n> We already handle packfiles that have an MIDX alright by the very nature\n\ns/an/a/, s/alright//\n\n> of this function, as we explicitly count non-MIDX'd packs. As such, we\n> can trivially switch over to use `packfile_store_get_all_packs()`\n> instead.\n>\n> Do so.\n\n;-)\n\nThanks,\nTaylor\n"},{"id":"528309","messageId":"aObNDRTffcWsYGZ4@nand.local","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-3-428227657a89@pks.im","subject":"Re: [PATCH 3/6] builtin/grep: simplify how we preload packs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-10-08T20:43:57Z","receivedAt":"2025-10-08T20:43:59Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 07, 2025 at 02:41:09PM +0200, Patrick Steinhardt wrote:\n> When using multiple threads in git-grep(1) we eagerly preload both the\n> gitmodules file as well as the packfiles so that the threads won't race\n> with one another to initialize these data structures.\n>\n> For packfiles, this is done by calling `packfile_store_get_packs()`,\n> which first loads our packfiles and then returns a pointer to the first\n> such packfile. This pointer is ignored though, as all we really care\n> about is that `packfile_store_prepare()` was called.\n>\n> Historyically, that function was file-local to \"packfile.c\", but that\n\ns/Historyically/Historically\n\n> changed with 4188332569 (packfile: move `get_multi_pack_index()` into\n> \"midx.c\", 2025-09-02). We can thus simplify the code by calling that\n> function directly.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/grep.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 13841fbf00..53cccf2d25 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -1214,7 +1214,7 @@ int cmd_grep(int argc,\n> \t\tif (recurse_submodules)\n> \t\t\trepo_read_gitmodules(the_repository, 1);\n> \t\tif (startup_info->have_repository)\n> -\t\t\t(void)packfile_store_get_packs(the_repository->objects->packfiles);\n> +\t\t\tpackfile_store_prepare(the_repository->objects->packfiles);\n\nMakes sense. That function literally calls packfile_store_prepare() on\nits argument, and then returns store->packs. Since we don't care about\nthe result as you note, calling packfile_store_prepare() directly makes\nsense.\n\nThanks,\nTaylor\n"},{"id":"528310","messageId":"aObNP9yyOoNYPYtQ@nand.local","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-4-428227657a89@pks.im","subject":"Re: [PATCH 4/6] packfile: drop `packfile_store_get_packs()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-10-08T20:44:47Z","receivedAt":"2025-10-08T20:44:49Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 07, 2025 at 02:41:10PM +0200, Patrick Steinhardt wrote:\n> In the preceding commits we have removed all remaining callers of\n> `packfile_store_get_packs()`, the function is thus unused now. Remove\n> it.\n\nWoohoo!\n\nThanks for tackling these one at a time. I agree with your reasoning in\nthe commits leading up to this point, but it was very helpful to see\nplainly that each step was safe to perform.\n\nThanks,\nTaylor\n"},{"id":"528312","messageId":"aObOFciwYsvTWT0e@nand.local","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-6-428227657a89@pks.im","subject":"Re: [PATCH 6/6] packfile: rename `packfile_store_get_all_packs()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-10-08T20:48:21Z","receivedAt":"2025-10-08T20:48:24Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 07, 2025 at 02:41:12PM +0200, Patrick Steinhardt wrote:\n> ---\n> builtin/fast-import.c | 4 ++--\n> builtin/pack-objects.c | 4 ++--\n> packfile.c | 2 +-\n> packfile.h | 4 ++--\n> 4 files changed, 7 insertions(+), 7 deletions(-)\n\nHmm. I wonder if we should perform this step at a later date. My fear is\nthat another topic in fight might introduce a new use of the \"get_packs\"\nassuming the old semantics.\n\nMerging this topic and that hypothetical one together wouldn't produce a\ntextual conflict, but it could introduce bugs where the hypothetical new\ncode expects the old behavior.\n\nPerhaps I'm overthinking this, but figured I'd write down the concern\nnonetheless.\n\nThanks,\nTaylor\n"},{"id":"528320","messageId":"aObUivWiMdtprQSu@nand.local","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-5-428227657a89@pks.im","subject":"Re: [PATCH 5/6] packfile: introduce macro to iterate through packs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-10-08T21:15:54Z","receivedAt":"2025-10-08T21:15:57Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 07, 2025 at 02:41:11PM +0200, Patrick Steinhardt wrote:\n> We have a bunch of different sites that want to iterate through all\n> packs of a given `struct packfile_store`. This pattern is somewhat\n> verbose and repetitive, which makes it somewhat cumbersome.\n>\n> Introduce a new macro `packfile_store_for_each_pack()` that removes some\n> of the boilerplate.\n\nMakes sense.\n\nSince we're talking about adding a convenience macro, I wonder if it\nmight make sense to name/treat this one slightly differently. I would\nimagine something like:\n\n repo_for_each_pack(the_repository, p) { ... }\n\nI understand that packfile_store_for_each_pack() is trying to operate on\na single packfile_source. In practice, would someone ever want to\niterate over the packfiles in some source but not another one? It's hard\nto know the answer to that question right now without any other\nimplementations.\n\nPerhaps in the meantime you could imagine doing something similar to\nyour macro, but instead of reading from the store directly, it could\nread from the repository's store _and_ BUG() if there is more than one\nstore to choose from.\n\nI get that that may be counterproductive to your goal here of getting\nthe tree prepared to handle multiple object sources. Another approach\nmight be to keep the proposed semantics, but call it \"for_each_pack()\"\ninstead of \"packfile_store_for_each_pack()\".\n\nI know that the latter is our preferred convention, but I am not sure\nthat the rule makes total sense in this case. It is obvious from\n\"for_each_pack()\" alone that we are iterating over packs, and the only\nway to obtain a collection of packs is from the a packfile_store. So\nhere I think that the \"packfile_store_\" prefix is doing more harm than\ngood by making the code overly verbose.\n\n> diff --git a/builtin/fsck.c b/builtin/fsck.c\n> index 8ee95e0d67..5462c442dc 100644\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -869,18 +869,19 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress)\n> {\n> \tstruct packfile_store *packs = r->objects->packfiles;\n> \tstruct progress *progress = NULL;\n> +\tstruct packed_git *p;\n\nI wondered if this needed to be in scope for the whole function, and\nlooking just at the first portion of this hunk it seems like it does\nnot. But continuing to read further down shows other spots where it is\nbeneficial to have \"p\" defined at the scope of the whole function.\n\n> \tuint32_t pack_count = 0;\n> \tint res = 0;\n>\n> \tif (show_progress) {\n> -\t\tfor (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next)\n> +\t\tpackfile_store_for_each_pack(packs, p)\n> \t\t\tpack_count++;\n\nThis conversion looks obviously correct. Though I wonder, as an aside,\ndo you think it makes sense to have a packfile_store_num_packs()\nfunction or similar that does this for us?\n\nThanks,\nTaylor\n"},{"id":"528362","messageId":"aOdX01_MrVkDKu9D@pks.im","threadId":"64263","inReplyTo":"aObIw/et7DxB/NrZ@nand.local","subject":"Re: [PATCH 2/6] builtin/gc: convert to use `packfile_store_get_all_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T06:36:03Z","receivedAt":"2025-10-09T06:36:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 08, 2025 at 04:25:39PM -0400, Taylor Blau wrote:\n> On Tue, Oct 07, 2025 at 02:41:08PM +0200, Patrick Steinhardt wrote:\n> > When running maintenance tasks via git-maintenance(1) we have a couple\n> > of auto-conditions that check whether or not a specific task should be\n> > running. One such check is for incremental repacks, which essentially\n> > use `git multi-pack-index repack` to repack a set of smaller packfiles\n> > into one larger packfile.\n> >\n> > The auto-condition for this task checks how many packfiles there are\n> > that aren't indexed by any multi-pack index. If there is a sufficient\n> > number then we execute the above command to combine those into a single\n> > pack and add them to the MIDX.\n> \n> s/them/that pack/\n> \n> > As we don't care about MIDX'd packs we use `packfile_store_get_packs()`,\n> > which knows to not load any packs that are indexed by a MIDX. But as\n> > explained in the preceding commit, we want to get rid of that function.\n> >\n> > We already handle packfiles that have an MIDX alright by the very nature\n> \n> s/an/a/, s/alright//\n\nHeh, I guess the first s/an/a/ depends on how you pronounce MIDX. I\ntypically say \"em ei di ex\", and in that case it's correct to say \"an\".\nBut I think I heard you pronounce it as a single word like \"midex\",\nwhere it's indeed correct to say \"a\".\n\nIn any case, I don't care, let's just say \"a midex\".\n\nPatrick\n"},{"id":"528363","messageId":"aOdX3ZmITr0FVq4b@pks.im","threadId":"64263","inReplyTo":"aObUivWiMdtprQSu@nand.local","subject":"Re: [PATCH 5/6] packfile: introduce macro to iterate through packs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T06:36:13Z","receivedAt":"2025-10-09T06:36:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 08, 2025 at 05:15:54PM -0400, Taylor Blau wrote:\n> On Tue, Oct 07, 2025 at 02:41:11PM +0200, Patrick Steinhardt wrote:\n> > We have a bunch of different sites that want to iterate through all\n> > packs of a given `struct packfile_store`. This pattern is somewhat\n> > verbose and repetitive, which makes it somewhat cumbersome.\n> >\n> > Introduce a new macro `packfile_store_for_each_pack()` that removes some\n> > of the boilerplate.\n> \n> Makes sense.\n> \n> Since we're talking about adding a convenience macro, I wonder if it\n> might make sense to name/treat this one slightly differently. I would\n> imagine something like:\n> \n> repo_for_each_pack(the_repository, p) { ... }\n> \n> I understand that packfile_store_for_each_pack() is trying to operate on\n> a single packfile_source. In practice, would someone ever want to\n> iterate over the packfiles in some source but not another one? It's hard\n> to know the answer to that question right now without any other\n> implementations.\n\nI don't think so, so I'm fine with this direction. In fact, it might\neven make my life easier. See further down.\n\n> Perhaps in the meantime you could imagine doing something similar to\n> your macro, but instead of reading from the store directly, it could\n> read from the repository's store _and_ BUG() if there is more than one\n> store to choose from.\n\nI don't think we should BUG() when there's multiple different packfile\nstores, as it is mostly likely the intent of the caller anyway to\niterate through all of them.\n\nBut that being said, there is a different reason why we may want to\nBUG() eventually: if we get pluggable ODBs, then one of the object\nsources may not be a source that uses packs in the first place. And in\n_that_ case we definitely should alert the developer that something is\nwrong and BUG().\n\nSo I'll adapt this series to use your proposed `repo_for_each_pack()`\nsolution.\n\n> > diff --git a/builtin/fsck.c b/builtin/fsck.c\n> > index 8ee95e0d67..5462c442dc 100644\n> > --- a/builtin/fsck.c\n> > +++ b/builtin/fsck.c\n> > @@ -869,18 +869,19 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress)\n> > {\n> > \tstruct packfile_store *packs = r->objects->packfiles;\n> > \tstruct progress *progress = NULL;\n> > +\tstruct packed_git *p;\n> \n> I wondered if this needed to be in scope for the whole function, and\n> looking just at the first portion of this hunk it seems like it does\n> not. But continuing to read further down shows other spots where it is\n> beneficial to have \"p\" defined at the scope of the whole function.\n> \n> > \tuint32_t pack_count = 0;\n> > \tint res = 0;\n> >\n> > \tif (show_progress) {\n> > -\t\tfor (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next)\n> > +\t\tpackfile_store_for_each_pack(packs, p)\n> > \t\t\tpack_count++;\n> \n> This conversion looks obviously correct. Though I wonder, as an aside,\n> do you think it makes sense to have a packfile_store_num_packs()\n> function or similar that does this for us?\n\nWe probably could, but I don't think we have enough callsites to warrant\nit now.\n\nThanks!\n\nPatrick\n"},{"id":"528364","messageId":"aOdX4_e9K_1p1pyv@pks.im","threadId":"64263","inReplyTo":"aObOFciwYsvTWT0e@nand.local","subject":"Re: [PATCH 6/6] packfile: rename `packfile_store_get_all_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T06:36:19Z","receivedAt":"2025-10-09T06:36:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 08, 2025 at 04:48:21PM -0400, Taylor Blau wrote:\n> On Tue, Oct 07, 2025 at 02:41:12PM +0200, Patrick Steinhardt wrote:\n> > ---\n> > builtin/fast-import.c | 4 ++--\n> > builtin/pack-objects.c | 4 ++--\n> > packfile.c | 2 +-\n> > packfile.h | 4 ++--\n> > 4 files changed, 7 insertions(+), 7 deletions(-)\n> \n> Hmm. I wonder if we should perform this step at a later date. My fear is\n> that another topic in fight might introduce a new use of the \"get_packs\"\n> assuming the old semantics.\n> \n> Merging this topic and that hypothetical one together wouldn't produce a\n> textual conflict, but it could introduce bugs where the hypothetical new\n> code expects the old behavior.\n> \n> Perhaps I'm overthinking this, but figured I'd write down the concern\n> nonetheless.\n\nI think that in many cases, a callsite that doesn't handle MIDX'd packs\nspecifically with `get_packs()` is almost guaranteed to be wrong in some\ncases anyway due to `get_packs()` and `get_all_packs()` influencing each\nother's results. In this series we already saw that the callsites handle\nthis correctly even though they use `get_packs()`, and any new callsites\nwould probably have to do the same.\n\nSo I don't think this is too worrisome overall.\n\nPatrick\n"},{"id":"528371","messageId":"20251009-pks-packfiles-convert-get-all-v2-0-0d73b87ce711@pks.im","threadId":"64263","inReplyTo":"20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im","subject":"[PATCH v2 0/6] packfile: remove `packfile_store_get_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T08:01:34Z","receivedAt":"2025-10-09T08:01:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series follows up on the discussion at [1]. Originally, these\npatches were part of the series that introduced the packfile store, but\nwe decided to split them out into a separate series.\n\nIn any case, the goal of this series is to remove the confusing\ndifference between `packfiles_store_get_{,all_}packs()`. It's not really\nobvious to the caller what the difference is. But even worse, the result\nof these functions depends on whether or not `get_all_packs()` was ever\nexecuted before `get_packs()`.\n\nThe series is built on top of 821f583da6 (The thirteenth batcn,\n2025-09-29) with tb/incremental-midx-part-3.1 at c886af90f8 (SQUASH???\nplay well with other topics by preemptively including \"repository.h\",\n2025-09-29) merged into it. This is done to fix some minor conflicts\nwith that patch series.\n\nChanges in v2:\n - Some commit message improvements.\n - Adapt `packfile_store_for_each_pack()` to `repo_for_each_pack()`. On\n the one hand this provides easier ergonomics. On the other hand this\n will eventually allow us to handle object databases of a different\n type more easily.\n - Link to v1: https://lore.kernel.org/r/20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im\n\nThanks!\n\nPatrick\n\n[1]: \n\n---\nPatrick Steinhardt (6):\n object-name: convert to use `packfile_store_get_all_packs()`\n builtin/gc: convert to use `packfile_store_get_all_packs()`\n builtin/grep: simplify how we preload packs\n packfile: drop `packfile_store_get_packs()`\n packfile: introduce macro to iterate through packs\n packfile: rename `packfile_store_get_all_packs()`\n\n builtin/cat-file.c | 3 +--\n builtin/count-objects.c | 3 +--\n builtin/fast-import.c | 4 ++--\n builtin/fsck.c | 15 ++++++---------\n builtin/gc.c | 16 +++++++---------\n builtin/grep.c | 2 +-\n builtin/pack-objects.c | 26 +++++++++-----------------\n builtin/pack-redundant.c | 14 ++++----------\n connected.c | 3 +--\n http-backend.c | 5 ++---\n http.c | 3 +--\n object-name.c | 8 +++++---\n pack-bitmap.c | 6 +++---\n pack-objects.c | 5 ++---\n packfile.c | 10 ++--------\n packfile.h | 10 ++++++----\n repack-cruft.c | 3 +--\n repack-geometry.c | 3 +--\n repack.c | 3 +--\n server-info.c | 3 +--\n t/helper/test-find-pack.c | 3 ++-\n t/helper/test-pack-mtimes.c | 2 +-\n 22 files changed, 60 insertions(+), 90 deletions(-)\n\nRange-diff versus v1:\n\n1: c52009c363 = 1: 04550dbdcf object-name: convert to use `packfile_store_get_all_packs()`\n2: 20389a8027 ! 2: 2677bc631f builtin/gc: convert to use `packfile_store_get_all_packs()`\n @@ Commit message\n The auto-condition for this task checks how many packfiles there are\n that aren't indexed by any multi-pack index. If there is a sufficient\n number then we execute the above command to combine those into a single\n - pack and add them to the MIDX.\n + pack and add that pack to the MIDX.\n \n As we don't care about MIDX'd packs we use `packfile_store_get_packs()`,\n which knows to not load any packs that are indexed by a MIDX. But as\n explained in the preceding commit, we want to get rid of that function.\n \n - We already handle packfiles that have an MIDX alright by the very nature\n - of this function, as we explicitly count non-MIDX'd packs. As such, we\n - can trivially switch over to use `packfile_store_get_all_packs()`\n - instead.\n + We already handle packfiles that have a MIDX by the very nature of this\n + function, as we explicitly count non-MIDX'd packs. As such, we can\n + trivially switch over to use `packfile_store_get_all_packs()` instead.\n \n Do so.\n \n3: 1357bdce48 ! 3: 7bee72fd98 builtin/grep: simplify how we preload packs\n @@ Commit message\n such packfile. This pointer is ignored though, as all we really care\n about is that `packfile_store_prepare()` was called.\n \n - Historyically, that function was file-local to \"packfile.c\", but that\n + Historically, that function was file-local to \"packfile.c\", but that\n changed with 4188332569 (packfile: move `get_multi_pack_index()` into\n \"midx.c\", 2025-09-02). We can thus simplify the code by calling that\n function directly.\n4: f2275c97ae = 4: e709bd4ac9 packfile: drop `packfile_store_get_packs()`\n5: 6e57bdd081 ! 5: 2000ef6983 packfile: introduce macro to iterate through packs\n @@ Commit message\n packs of a given `struct packfile_store`. This pattern is somewhat\n verbose and repetitive, which makes it somewhat cumbersome.\n \n - Introduce a new macro `packfile_store_for_each_pack()` that removes some\n - of the boilerplate.\n + Introduce a new macro `repo_for_each_pack()` that removes some of the\n + boilerplate.\n \n Signed-off-by: Patrick Steinhardt \n \n ## builtin/cat-file.c ##\n @@ builtin/cat-file.c: static void batch_each_object(struct batch_options *opt,\n - \t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \n + \tif (bitmap && !for_each_bitmapped_object(bitmap, &opt->objects_filter,\n + \t\t\t\t\t\t batch_one_object_bitmapped, &payload)) {\n +-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *pack;\n \n -\t\tfor (pack = packfile_store_get_all_packs(packs); pack; pack = pack->next) {\n -+\t\tpackfile_store_for_each_pack(packs, pack) {\n ++\t\trepo_for_each_pack(the_repository, pack) {\n \t\t\tif (bitmap_index_contains_pack(bitmap, pack) ||\n \t\t\t open_pack_index(pack))\n \t\t\t\tcontinue;\n \n ## builtin/count-objects.c ##\n +@@ builtin/count-objects.c: int cmd_count_objects(int argc,\n + \t\t\t\t count_loose, count_cruft, NULL, NULL);\n + \n + \tif (verbose) {\n +-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \t\tstruct packed_git *p;\n + \t\tunsigned long num_pack = 0;\n + \t\toff_t size_pack = 0;\n @@ builtin/count-objects.c: int cmd_count_objects(int argc,\n \t\tstruct strbuf pack_buf = STRBUF_INIT;\n \t\tstruct strbuf garbage_buf = STRBUF_INIT;\n \n -\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\t\tpackfile_store_for_each_pack(packs, p) {\n ++\t\trepo_for_each_pack(the_repository, p) {\n \t\t\tif (!p->pack_local)\n \t\t\t\tcontinue;\n \t\t\tif (open_pack_index(p))\n \n ## builtin/fsck.c ##\n -@@ builtin/fsck.c: static int check_pack_rev_indexes(struct repository *r, int show_progress)\n +@@ builtin/fsck.c: static int mark_packed_for_connectivity(const struct object_id *oid,\n + \n + static int check_pack_rev_indexes(struct repository *r, int show_progress)\n {\n - \tstruct packfile_store *packs = r->objects->packfiles;\n +-\tstruct packfile_store *packs = r->objects->packfiles;\n \tstruct progress *progress = NULL;\n +\tstruct packed_git *p;\n \tuint32_t pack_count = 0;\n @@ builtin/fsck.c: static int check_pack_rev_indexes(struct repository *r, int show\n \n \tif (show_progress) {\n -\t\tfor (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next)\n -+\t\tpackfile_store_for_each_pack(packs, p)\n ++\t\trepo_for_each_pack(r, p)\n \t\t\tpack_count++;\n \t\tprogress = start_delayed_progress(the_repository,\n \t\t\t\t\t\t \"Verifying reverse pack-indexes\", pack_count);\n @@ builtin/fsck.c: static int check_pack_rev_indexes(struct repository *r, int show\n \t}\n \n -\tfor (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(r, p) {\n \t\tint load_error = load_pack_revindex_from_disk(p);\n \n \t\tif (load_error < 0) {\n +@@ builtin/fsck.c: int cmd_fsck(int argc,\n + \t\tfor_each_packed_object(the_repository,\n + \t\t\t\t mark_packed_for_connectivity, NULL, 0);\n + \t} else {\n +-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n +-\n + \t\todb_prepare_alternates(the_repository->objects);\n + \t\tfor (source = the_repository->objects->sources; source; source = source->next)\n + \t\t\tfsck_source(source);\n @@ builtin/fsck.c: int cmd_fsck(int argc,\n \t\t\tstruct progress *progress = NULL;\n \n \t\t\tif (show_progress) {\n -\t\t\t\tfor (p = packfile_store_get_all_packs(packs); p;\n -\t\t\t\t p = p->next) {\n -+\t\t\t\tpackfile_store_for_each_pack(packs, p) {\n ++\t\t\t\trepo_for_each_pack(the_repository, p) {\n \t\t\t\t\tif (open_pack_index(p))\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\ttotal += p->num_objects;\n @@ builtin/fsck.c: int cmd_fsck(int argc,\n -\t\t\tfor (p = packfile_store_get_all_packs(packs); p;\n -\t\t\t p = p->next) {\n +\n -+\t\t\tpackfile_store_for_each_pack(packs, p) {\n ++\t\t\trepo_for_each_pack(the_repository, p) {\n \t\t\t\t/* verify gives error messages itself */\n \t\t\t\tif (verify_pack(the_repository,\n \t\t\t\t\t\tp, fsck_obj_buffer,\n \n ## builtin/gc.c ##\n -@@ builtin/gc.c: static struct packed_git *find_base_packs(struct string_list *packs,\n - \tstruct packfile_store *packfiles = the_repository->objects->packfiles;\n +@@ builtin/gc.c: static int too_many_loose_objects(struct gc_config *cfg)\n + static struct packed_git *find_base_packs(struct string_list *packs,\n + \t\t\t\t\t unsigned long limit)\n + {\n +-\tstruct packfile_store *packfiles = the_repository->objects->packfiles;\n \tstruct packed_git *p, *base = NULL;\n \n -\tfor (p = packfile_store_get_all_packs(packfiles); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packfiles, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tif (!p->pack_local || p->is_cruft)\n \t\t\tcontinue;\n \t\tif (limit) {\n -@@ builtin/gc.c: static int too_many_packs(struct gc_config *cfg)\n +@@ builtin/gc.c: static struct packed_git *find_base_packs(struct string_list *packs,\n + \n + static int too_many_packs(struct gc_config *cfg)\n {\n - \tstruct packfile_store *packs = the_repository->objects->packfiles;\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct packed_git *p;\n -\tint cnt;\n +\tint cnt = 0;\n @@ builtin/gc.c: static int too_many_packs(struct gc_config *cfg)\n \t\treturn 0;\n \n -\tfor (cnt = 0, p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tif (!p->pack_local)\n \t\t\tcontinue;\n \t\tif (p->pack_keep)\n @@ builtin/gc.c: static int incremental_repack_auto_condition(struct gc_config *cfg\n -\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles);\n -\t count < incremental_repack_auto_limit && p;\n -\t p = p->next) {\n -+\tpackfile_store_for_each_pack(the_repository->objects->packfiles, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n +\t\tif (count >= incremental_repack_auto_limit)\n +\t\t\tbreak;\n \t\tif (!p->multi_pack_index)\n @@ builtin/gc.c: static off_t get_auto_pack_size(void)\n \n \todb_reprepare(r->objects);\n -\tfor (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(r->objects->packfiles, p) {\n ++\trepo_for_each_pack(r, p) {\n \t\tif (p->pack_size > max_size) {\n \t\t\tsecond_largest_size = max_size;\n \t\t\tmax_size = p->pack_size;\n \n ## builtin/pack-objects.c ##\n +@@ builtin/pack-objects.c: static int pack_mtime_cmp(const void *_a, const void *_b)\n + \n + static void read_packs_list_from_stdin(struct rev_info *revs)\n + {\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \tstruct strbuf buf = STRBUF_INIT;\n + \tstruct string_list include_packs = STRING_LIST_INIT_DUP;\n + \tstruct string_list exclude_packs = STRING_LIST_INIT_DUP;\n + \tstruct string_list_item *item = NULL;\n +-\n + \tstruct packed_git *p;\n + \n + \twhile (strbuf_getline(&buf, stdin) != EOF) {\n @@ builtin/pack-objects.c: static void read_packs_list_from_stdin(struct rev_info *revs)\n \tstring_list_sort(&exclude_packs);\n \tstring_list_remove_duplicates(&exclude_packs, 0);\n \n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tconst char *pack_name = pack_basename(p);\n \n \t\tif ((item = string_list_lookup(&include_packs, pack_name)))\n +@@ builtin/pack-objects.c: static void enumerate_cruft_objects(void)\n + \n + static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs)\n + {\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \tstruct packed_git *p;\n + \tstruct rev_info revs;\n + \tint ret;\n @@ builtin/pack-objects.c: static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs\n \t * Re-mark only the fresh packs as kept so that objects in\n \t * unknown packs do not halt the reachability traversal early.\n \t */\n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next)\n -+\tpackfile_store_for_each_pack(packs, p)\n ++\trepo_for_each_pack(the_repository, p)\n \t\tp->pack_keep_in_core = 0;\n \tmark_pack_kept_in_core(fresh_packs, 1);\n \n +@@ builtin/pack-objects.c: static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs\n + \n + static void read_cruft_objects(void)\n + {\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \tstruct strbuf buf = STRBUF_INIT;\n + \tstruct string_list discard_packs = STRING_LIST_INIT_DUP;\n + \tstruct string_list fresh_packs = STRING_LIST_INIT_DUP;\n @@ builtin/pack-objects.c: static void read_cruft_objects(void)\n \tstring_list_sort(&discard_packs);\n \tstring_list_sort(&fresh_packs);\n \n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tconst char *pack_name = pack_basename(p);\n \t\tstruct string_list_item *item;\n \n -@@ builtin/pack-objects.c: static void loosen_unused_packed_objects(void)\n +@@ builtin/pack-objects.c: static int loosened_object_can_be_discarded(const struct object_id *oid,\n + \n + static void loosen_unused_packed_objects(void)\n + {\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \tstruct packed_git *p;\n + \tuint32_t i;\n \tuint32_t loosened_objects_nr = 0;\n \tstruct object_id oid;\n \n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tif (!p->pack_local || p->pack_keep || p->pack_keep_in_core)\n \t\t\tcontinue;\n \n -@@ builtin/pack-objects.c: static void add_extra_kept_packs(const struct string_list *names)\n +@@ builtin/pack-objects.c: static void get_object_list(struct rev_info *revs, struct strvec *argv)\n + \n + static void add_extra_kept_packs(const struct string_list *names)\n + {\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \tstruct packed_git *p;\n + \n \tif (!names->nr)\n \t\treturn;\n \n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tconst char *name = basename(p->pack_name);\n \t\tint i;\n \n @@ builtin/pack-objects.c: int cmd_pack_objects(int argc,\n - \t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \n + \tadd_extra_kept_packs(&keep_pack_list);\n + \tif (ignore_packed_keep_on_disk) {\n +-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *p;\n \n -\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next)\n -+\t\tpackfile_store_for_each_pack(packs, p)\n ++\t\trepo_for_each_pack(the_repository, p)\n \t\t\tif (p->pack_local && p->pack_keep)\n \t\t\t\tbreak;\n \t\tif (!p) /* no keep-able packs found */\n @@ builtin/pack-objects.c: int cmd_pack_objects(int argc,\n - \t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \t\t * want to unset \"local\" based on looking at packs, as\n + \t\t * it also covers non-local objects\n + \t\t */\n +-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *p;\n \n -\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\t\tpackfile_store_for_each_pack(packs, p) {\n ++\t\trepo_for_each_pack(the_repository, p) {\n \t\t\tif (!p->pack_local) {\n \t\t\t\thave_non_local_packs = 1;\n \t\t\t\tbreak;\n \n ## builtin/pack-redundant.c ##\n @@ builtin/pack-redundant.c: static struct pack_list * add_pack(struct packed_git *p)\n + \n static struct pack_list * add_pack_file(const char *filename)\n {\n - \tstruct packfile_store *packs = the_repository->objects->packfiles;\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n -\tstruct packed_git *p = packfile_store_get_all_packs(packs);\n +\tstruct packed_git *p;\n \n @@ builtin/pack-redundant.c: static struct pack_list * add_pack(struct packed_git *\n \t\tdie(\"Bad pack filename: %s\", filename);\n \n -\twhile (p) {\n -+\tpackfile_store_for_each_pack(packs, p)\n ++\trepo_for_each_pack(the_repository, p)\n \t\tif (strstr(p->pack_name, filename))\n \t\t\treturn add_pack(p);\n -\t\tp = p->next;\n @@ builtin/pack-redundant.c: static struct pack_list * add_pack(struct packed_git *\n \n static void load_all(void)\n {\n - \tstruct packfile_store *packs = the_repository->objects->packfiles;\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n -\tstruct packed_git *p = packfile_store_get_all_packs(packs);\n +\tstruct packed_git *p;\n \n -\twhile (p) {\n -+\tpackfile_store_for_each_pack(packs, p)\n ++\trepo_for_each_pack(the_repository, p)\n \t\tadd_pack(p);\n -\t\tp = p->next;\n -\t}\n @@ builtin/pack-redundant.c: static struct pack_list * add_pack(struct packed_git *\n \n ## connected.c ##\n @@ connected.c: int check_connected(oid_iterate_fn fn, void *cb_data,\n - \t\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \t\t */\n + \t\todb_reprepare(the_repository->objects);\n + \t\tdo {\n +-\t\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\t\tstruct packed_git *p;\n \n -\t\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\t\t\tpackfile_store_for_each_pack(packs, p) {\n ++\t\t\trepo_for_each_pack(the_repository, p) {\n \t\t\t\tif (!p->pack_promisor)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (find_pack_entry_one(oid, p))\n \n ## http-backend.c ##\n -@@ http-backend.c: static void get_info_packs(struct strbuf *hdr, char *arg UNUSED)\n +@@ http-backend.c: static void get_head(struct strbuf *hdr, char *arg UNUSED)\n + static void get_info_packs(struct strbuf *hdr, char *arg UNUSED)\n + {\n + \tsize_t objdirlen = strlen(repo_get_object_directory(the_repository));\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \tstruct strbuf buf = STRBUF_INIT;\n + \tstruct packed_git *p;\n \tsize_t cnt = 0;\n \n \tselect_getanyfile(hdr);\n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tif (p->pack_local)\n \t\t\tcnt++;\n \t}\n \n \tstrbuf_grow(&buf, cnt * 53 + 2);\n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tif (p->pack_local)\n \t\t\tstrbuf_addf(&buf, \"P %s\\n\", p->pack_name + objdirlen + 6);\n \t}\n \n ## http.c ##\n +@@ http.c: static char *fetch_pack_index(unsigned char *hash, const char *base_url)\n + static int fetch_and_setup_pack_index(struct packed_git **packs_head,\n + \tunsigned char *sha1, const char *base_url)\n + {\n +-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n + \tstruct packed_git *new_pack, *p;\n + \tchar *tmp_idx = NULL;\n + \tint ret;\n @@ http.c: static int fetch_and_setup_pack_index(struct packed_git **packs_head,\n \t * If we already have the pack locally, no need to fetch its index or\n \t * even add it to list; we already have all of its objects.\n \t */\n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tif (hasheq(p->hash, sha1, the_repository->hash_algo))\n \t\t\treturn 0;\n \t}\n @@ object-name.c: static void find_short_packed_object(struct disambiguate_state *d\n \n -\tfor (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n -\t p = p->next)\n -+\tpackfile_store_for_each_pack(ds->repo->objects->packfiles, p) {\n ++\trepo_for_each_pack(ds->repo, p) {\n +\t\tif (ds->ambiguous)\n +\t\t\tbreak;\n \t\tunique_in_pack(p, ds);\n @@ object-name.c: static void find_abbrev_len_packed(struct min_abbrev_data *mad)\n \t}\n \n -\tfor (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next)\n -+\tpackfile_store_for_each_pack(mad->repo->objects->packfiles, p)\n ++\trepo_for_each_pack(mad->repo, p)\n \t\tfind_abbrev_len_for_pack(p, mad);\n }\n \n @@ pack-bitmap.c: static int open_pack_bitmap(struct repository *r,\n \tint ret = -1;\n \n -\tfor (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(r->objects->packfiles, p) {\n ++\trepo_for_each_pack(r, p) {\n \t\tif (open_pack_bitmap_1(bitmap_git, p) == 0) {\n \t\t\tret = 0;\n \t\t\t/*\n @@ pack-bitmap.c: int verify_bitmap_files(struct repository *r)\n \n -\tfor (struct packed_git *p = packfile_store_get_all_packs(r->objects->packfiles);\n -\t p; p = p->next) {\n -+\tpackfile_store_for_each_pack(r->objects->packfiles, p) {\n ++\trepo_for_each_pack(r, p) {\n \t\tchar *pack_bitmap_name = pack_bitmap_filename(p);\n \t\tres |= verify_bitmap_file(r->hash_algo, pack_bitmap_name);\n \t\tfree(pack_bitmap_name);\n \n ## pack-objects.c ##\n +@@ pack-objects.c: struct object_entry *packlist_find(struct packing_data *pdata,\n + \n + static void prepare_in_pack_by_idx(struct packing_data *pdata)\n + {\n +-\tstruct packfile_store *packs = pdata->repo->objects->packfiles;\n + \tstruct packed_git **mapping, *p;\n + \tint cnt = 0, nr = 1U << OE_IN_PACK_BITS;\n + \n @@ pack-objects.c: static void prepare_in_pack_by_idx(struct packing_data *pdata)\n \t * (i.e. in_pack_idx also zero) should return NULL.\n \t */\n \tmapping[cnt++] = NULL;\n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next, cnt++) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(pdata->repo, p) {\n \t\tif (cnt == nr) {\n \t\t\tfree(mapping);\n \t\t\treturn;\n @@ packfile.c: struct packed_git **kept_pack_cache(struct repository *r, unsigned f\n \t\t * the non-kept version.\n \t\t */\n -\t\tfor (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {\n -+\t\tpackfile_store_for_each_pack(r->objects->packfiles, p) {\n ++\t\trepo_for_each_pack(r, p) {\n \t\t\tif ((p->pack_keep && (flags & ON_DISK_KEEP_PACKS)) ||\n \t\t\t (p->pack_keep_in_core && (flags & IN_CORE_KEEP_PACKS))) {\n \t\t\t\tALLOC_GROW(packs, nr + 1, alloc);\n @@ packfile.c: int for_each_packed_object(struct repository *repo, each_packed_obje\n \tint pack_errors = 0;\n \n -\tfor (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(repo->objects->packfiles, p) {\n ++\trepo_for_each_pack(repo, p) {\n \t\tif ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)\n \t\t\tcontinue;\n \t\tif ((flags & FOR_EACH_OBJECT_PROMISOR_ONLY) &&\n @@ packfile.h: void packfile_store_reprepare(struct packfile_store *store);\n \t\t\t struct packed_git *pack);\n \n +/*\n -+ * Load and iterate through all packs of the given packfile store.\n ++ * Load and iterate through all packs of the given repository. This helper\n ++ * function will yield packfiles from all object sources connected to the\n ++ * repository.\n + */\n -+#define packfile_store_for_each_pack(store, p) \\\n -+\tfor (p = packfile_store_get_all_packs(store); p; p = p->next)\n ++#define repo_for_each_pack(repo, p) \\\n ++\tfor (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next)\n +\n /*\n * Get all packs managed by the given store, including packfiles that are\n * referenced by multi-pack indices.\n \n ## repack-cruft.c ##\n -@@ repack-cruft.c: static void combine_small_cruft_packs(FILE *in, off_t combine_cruft_below_size,\n +@@\n + static void combine_small_cruft_packs(FILE *in, off_t combine_cruft_below_size,\n + \t\t\t\t struct existing_packs *existing)\n + {\n +-\tstruct packfile_store *packs = existing->repo->objects->packfiles;\n + \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \tsize_t i;\n \n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(existing->repo, p) {\n \t\tif (!(p->is_cruft && p->pack_local))\n \t\t\tcontinue;\n \n \n ## repack-geometry.c ##\n @@ repack-geometry.c: void pack_geometry_init(struct pack_geometry *geometry,\n + \t\t\tstruct existing_packs *existing,\n + \t\t\tconst struct pack_objects_args *args)\n + {\n +-\tstruct packfile_store *packs = existing->repo->objects->packfiles;\n \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(existing->repo, p) {\n \t\tif (args->local && !p->pack_local)\n \t\t\t/*\n \t\t\t * When asked to only repack local packfiles we skip\n \n ## repack.c ##\n -@@ repack.c: void existing_packs_collect(struct existing_packs *existing,\n +@@ repack.c: int finish_pack_objects_cmd(const struct git_hash_algo *algop,\n + void existing_packs_collect(struct existing_packs *existing,\n + \t\t\t const struct string_list *extra_keep)\n + {\n +-\tstruct packfile_store *packs = existing->repo->objects->packfiles;\n \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(existing->repo, p) {\n \t\tsize_t i;\n \t\tconst char *base;\n \n \n ## server-info.c ##\n -@@ server-info.c: static void init_pack_info(struct repository *r, const char *infofile, int force\n +@@ server-info.c: static int compare_info(const void *a_, const void *b_)\n + \n + static void init_pack_info(struct repository *r, const char *infofile, int force)\n + {\n +-\tstruct packfile_store *packs = r->objects->packfiles;\n + \tstruct packed_git *p;\n + \tint stale;\n \tint i;\n \tsize_t alloc = 0;\n \n -\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(packs, p) {\n ++\trepo_for_each_pack(r, p) {\n \t\t/* we ignore things on alternate path since they are\n \t\t * not available to the pullers in general.\n \t\t */\n @@ t/helper/test-find-pack.c: int cmd__find_pack(int argc, const char **argv)\n \t\tdie(\"cannot parse %s as an object name\", argv[0]);\n \n -\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next)\n -+\tpackfile_store_for_each_pack(the_repository->objects->packfiles, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tif (find_pack_entry_one(&oid, p)) {\n \t\t\tprintf(\"%s\\n\", p->pack_name);\n \t\t\tactual_count++;\n @@ t/helper/test-pack-mtimes.c: int cmd__pack_mtimes(int argc, const char **argv)\n \t\tusage(pack_mtimes_usage);\n \n -\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next) {\n -+\tpackfile_store_for_each_pack(the_repository->objects->packfiles, p) {\n ++\trepo_for_each_pack(the_repository, p) {\n \t\tstrbuf_addstr(&buf, basename(p->pack_name));\n \t\tstrbuf_strip_suffix(&buf, \".pack\");\n \t\tstrbuf_addstr(&buf, \".mtimes\");\n6: ceceb9cc71 ! 6: a688fef987 packfile: rename `packfile_store_get_all_packs()`\n @@ packfile.c: void packfile_store_reprepare(struct packfile_store *store)\n \n ## packfile.h ##\n @@ packfile.h: void packfile_store_add_pack(struct packfile_store *store,\n - * Load and iterate through all packs of the given packfile store.\n + * repository.\n */\n - #define packfile_store_for_each_pack(store, p) \\\n --\tfor (p = packfile_store_get_all_packs(store); p; p = p->next)\n -+\tfor (p = packfile_store_get_packs(store); p; p = p->next)\n + #define repo_for_each_pack(repo, p) \\\n +-\tfor (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next)\n ++\tfor (p = packfile_store_get_packs(repo->objects->packfiles); p; p = p->next)\n \n /*\n * Get all packs managed by the given store, including packfiles that are\n\n---\nbase-commit: a0cb7ee3bf6c8398ab18e7b0dfabec106312b2f8\nchange-id: 20250926-pks-packfiles-convert-get-all-4236f6e2ed9b\n\n"},{"id":"528372","messageId":"20251009-pks-packfiles-convert-get-all-v2-1-0d73b87ce711@pks.im","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-0-0d73b87ce711@pks.im","subject":"[PATCH v2 1/6] object-name: convert to use `packfile_store_get_all_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T08:01:35Z","receivedAt":"2025-10-09T08:01:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When searching for abbreviated or when trying to disambiguate object IDs\nwe do this in two steps:\n\n 1. We search through the multi-pack index.\n\n 2. We search through all packfiles not part of any multi-pack index.\n\nThe second step uses `packfile_store_get_packs()`, which knows to skip\nloading any packfiles that are indexed by an MIDX; this is exactly what\nwe want.\n\nBut that function is somewhat problematic, as its behaviour is stateful\nand is influenced by `packfile_store_get_all_packs()`. This function\nbasically does the same as `packfile_store_get_packs()`, but in addition\nit also loads all packfiles indexed by an MIDX. The problem here is that\nboth of these functions act on the same linked list of packfiles, and\nthus depending on whether or not `get_all_packs()` was called the result\nreturned by `get_packs()` will be different. Consequently, all callers\nof `get_packs()` need to be prepared to see MIDX'd packs even though\nthese should in theory be excluded.\n\nThis interface is confusing and thus potentially dangerous, which is why\nwe're converting all callers of `get_packs()` to use `get_all_packs()`\ninstead.\n\nDo so for the above functions in \"object-name.c\". As explained, we\nalready know to skip any MIDX'd packs in both `find_abbrev_len_packed()`\nand `find_short_packed_object()`, so it's fine to start loading MIDX'd\npackfiles.\n\nSigned-off-by: Patrick Steinhardt \n---\n object-name.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex f6902e140d..4e62bfa330 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -213,7 +213,7 @@ static void find_short_packed_object(struct disambiguate_state *ds)\n \t\t\tunique_in_midx(m, ds);\n \t}\n \n-\tfor (p = packfile_store_get_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n+\tfor (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n \t p = p->next)\n \t\tunique_in_pack(p, ds);\n }\n@@ -805,7 +805,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)\n \t\t\tfind_abbrev_len_for_midx(m, mad);\n \t}\n \n-\tfor (p = packfile_store_get_packs(mad->repo->objects->packfiles); p; p = p->next)\n+\tfor (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next)\n \t\tfind_abbrev_len_for_pack(p, mad);\n }\n \n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528373","messageId":"20251009-pks-packfiles-convert-get-all-v2-2-0d73b87ce711@pks.im","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-0-0d73b87ce711@pks.im","subject":"[PATCH v2 2/6] builtin/gc: convert to use `packfile_store_get_all_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T08:01:36Z","receivedAt":"2025-10-09T08:01:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When running maintenance tasks via git-maintenance(1) we have a couple\nof auto-conditions that check whether or not a specific task should be\nrunning. One such check is for incremental repacks, which essentially\nuse `git multi-pack-index repack` to repack a set of smaller packfiles\ninto one larger packfile.\n\nThe auto-condition for this task checks how many packfiles there are\nthat aren't indexed by any multi-pack index. If there is a sufficient\nnumber then we execute the above command to combine those into a single\npack and add that pack to the MIDX.\n\nAs we don't care about MIDX'd packs we use `packfile_store_get_packs()`,\nwhich knows to not load any packs that are indexed by a MIDX. But as\nexplained in the preceding commit, we want to get rid of that function.\n\nWe already handle packfiles that have a MIDX by the very nature of this\nfunction, as we explicitly count non-MIDX'd packs. As such, we can\ntrivially switch over to use `packfile_store_get_all_packs()` instead.\n\nDo so.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/gc.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex e19e13d978..ab6d6d3bd1 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1425,7 +1425,7 @@ static int incremental_repack_auto_condition(struct gc_config *cfg UNUSED)\n \tif (incremental_repack_auto_limit < 0)\n \t\treturn 1;\n \n-\tfor (p = packfile_store_get_packs(the_repository->objects->packfiles);\n+\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles);\n \t count < incremental_repack_auto_limit && p;\n \t p = p->next) {\n \t\tif (!p->multi_pack_index)\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528374","messageId":"20251009-pks-packfiles-convert-get-all-v2-3-0d73b87ce711@pks.im","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-0-0d73b87ce711@pks.im","subject":"[PATCH v2 3/6] builtin/grep: simplify how we preload packs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T08:01:37Z","receivedAt":"2025-10-09T08:01:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When using multiple threads in git-grep(1) we eagerly preload both the\ngitmodules file as well as the packfiles so that the threads won't race\nwith one another to initialize these data structures.\n\nFor packfiles, this is done by calling `packfile_store_get_packs()`,\nwhich first loads our packfiles and then returns a pointer to the first\nsuch packfile. This pointer is ignored though, as all we really care\nabout is that `packfile_store_prepare()` was called.\n\nHistorically, that function was file-local to \"packfile.c\", but that\nchanged with 4188332569 (packfile: move `get_multi_pack_index()` into\n\"midx.c\", 2025-09-02). We can thus simplify the code by calling that\nfunction directly.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/grep.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 13841fbf00..53cccf2d25 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -1214,7 +1214,7 @@ int cmd_grep(int argc,\n \t\tif (recurse_submodules)\n \t\t\trepo_read_gitmodules(the_repository, 1);\n \t\tif (startup_info->have_repository)\n-\t\t\t(void)packfile_store_get_packs(the_repository->objects->packfiles);\n+\t\t\tpackfile_store_prepare(the_repository->objects->packfiles);\n \n \t\tstart_threads(&opt);\n \t} else {\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528375","messageId":"20251009-pks-packfiles-convert-get-all-v2-4-0d73b87ce711@pks.im","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-0-0d73b87ce711@pks.im","subject":"[PATCH v2 4/6] packfile: drop `packfile_store_get_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T08:01:38Z","receivedAt":"2025-10-09T08:01:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In the preceding commits we have removed all remaining callers of\n`packfile_store_get_packs()`, the function is thus unused now. Remove\nit.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 6 ------\n packfile.h | 6 ------\n 2 files changed, 12 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 5a7caec292..db748b0bd4 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1027,12 +1027,6 @@ void packfile_store_reprepare(struct packfile_store *store)\n \tpackfile_store_prepare(store);\n }\n \n-struct packed_git *packfile_store_get_packs(struct packfile_store *store)\n-{\n-\tpackfile_store_prepare(store);\n-\treturn store->packs;\n-}\n-\n struct packed_git *packfile_store_get_all_packs(struct packfile_store *store)\n {\n \tpackfile_store_prepare(store);\ndiff --git a/packfile.h b/packfile.h\nindex e7a5792b6c..3f38c63476 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -136,12 +136,6 @@ void packfile_store_reprepare(struct packfile_store *store);\n void packfile_store_add_pack(struct packfile_store *store,\n \t\t\t struct packed_git *pack);\n \n-/*\n- * Get packs managed by the given store. Does not load the MIDX or any packs\n- * referenced by it.\n- */\n-struct packed_git *packfile_store_get_packs(struct packfile_store *store);\n-\n /*\n * Get all packs managed by the given store, including packfiles that are\n * referenced by multi-pack indices.\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528376","messageId":"20251009-pks-packfiles-convert-get-all-v2-5-0d73b87ce711@pks.im","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-0-0d73b87ce711@pks.im","subject":"[PATCH v2 5/6] packfile: introduce macro to iterate through packs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T08:01:39Z","receivedAt":"2025-10-09T08:02:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We have a bunch of different sites that want to iterate through all\npacks of a given `struct packfile_store`. This pattern is somewhat\nverbose and repetitive, which makes it somewhat cumbersome.\n\nIntroduce a new macro `repo_for_each_pack()` that removes some of the\nboilerplate.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/cat-file.c | 3 +--\n builtin/count-objects.c | 3 +--\n builtin/fsck.c | 15 ++++++---------\n builtin/gc.c | 16 +++++++---------\n builtin/pack-objects.c | 22 +++++++---------------\n builtin/pack-redundant.c | 14 ++++----------\n connected.c | 3 +--\n http-backend.c | 5 ++---\n http.c | 3 +--\n object-name.c | 8 +++++---\n pack-bitmap.c | 6 +++---\n pack-objects.c | 5 ++---\n packfile.c | 4 ++--\n packfile.h | 8 ++++++++\n repack-cruft.c | 3 +--\n repack-geometry.c | 3 +--\n repack.c | 3 +--\n server-info.c | 3 +--\n t/helper/test-find-pack.c | 3 ++-\n t/helper/test-pack-mtimes.c | 2 +-\n 20 files changed, 57 insertions(+), 75 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex ee6715fa52..0ab076aeb3 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -852,10 +852,9 @@ static void batch_each_object(struct batch_options *opt,\n \n \tif (bitmap && !for_each_bitmapped_object(bitmap, &opt->objects_filter,\n \t\t\t\t\t\t batch_one_object_bitmapped, &payload)) {\n-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *pack;\n \n-\t\tfor (pack = packfile_store_get_all_packs(packs); pack; pack = pack->next) {\n+\t\trepo_for_each_pack(the_repository, pack) {\n \t\t\tif (bitmap_index_contains_pack(bitmap, pack) ||\n \t\t\t open_pack_index(pack))\n \t\t\t\tcontinue;\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex f2f407c2a7..18f6e33b6f 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -122,7 +122,6 @@ int cmd_count_objects(int argc,\n \t\t\t\t count_loose, count_cruft, NULL, NULL);\n \n \tif (verbose) {\n-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *p;\n \t\tunsigned long num_pack = 0;\n \t\toff_t size_pack = 0;\n@@ -130,7 +129,7 @@ int cmd_count_objects(int argc,\n \t\tstruct strbuf pack_buf = STRBUF_INIT;\n \t\tstruct strbuf garbage_buf = STRBUF_INIT;\n \n-\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\t\trepo_for_each_pack(the_repository, p) {\n \t\t\tif (!p->pack_local)\n \t\t\t\tcontinue;\n \t\t\tif (open_pack_index(p))\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 8ee95e0d67..b1a650c673 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -867,20 +867,20 @@ static int mark_packed_for_connectivity(const struct object_id *oid,\n \n static int check_pack_rev_indexes(struct repository *r, int show_progress)\n {\n-\tstruct packfile_store *packs = r->objects->packfiles;\n \tstruct progress *progress = NULL;\n+\tstruct packed_git *p;\n \tuint32_t pack_count = 0;\n \tint res = 0;\n \n \tif (show_progress) {\n-\t\tfor (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next)\n+\t\trepo_for_each_pack(r, p)\n \t\t\tpack_count++;\n \t\tprogress = start_delayed_progress(the_repository,\n \t\t\t\t\t\t \"Verifying reverse pack-indexes\", pack_count);\n \t\tpack_count = 0;\n \t}\n \n-\tfor (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(r, p) {\n \t\tint load_error = load_pack_revindex_from_disk(p);\n \n \t\tif (load_error < 0) {\n@@ -1000,8 +1000,6 @@ int cmd_fsck(int argc,\n \t\tfor_each_packed_object(the_repository,\n \t\t\t\t mark_packed_for_connectivity, NULL, 0);\n \t} else {\n-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n-\n \t\todb_prepare_alternates(the_repository->objects);\n \t\tfor (source = the_repository->objects->sources; source; source = source->next)\n \t\t\tfsck_source(source);\n@@ -1012,8 +1010,7 @@ int cmd_fsck(int argc,\n \t\t\tstruct progress *progress = NULL;\n \n \t\t\tif (show_progress) {\n-\t\t\t\tfor (p = packfile_store_get_all_packs(packs); p;\n-\t\t\t\t p = p->next) {\n+\t\t\t\trepo_for_each_pack(the_repository, p) {\n \t\t\t\t\tif (open_pack_index(p))\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\ttotal += p->num_objects;\n@@ -1022,8 +1019,8 @@ int cmd_fsck(int argc,\n \t\t\t\tprogress = start_progress(the_repository,\n \t\t\t\t\t\t\t _(\"Checking objects\"), total);\n \t\t\t}\n-\t\t\tfor (p = packfile_store_get_all_packs(packs); p;\n-\t\t\t p = p->next) {\n+\n+\t\t\trepo_for_each_pack(the_repository, p) {\n \t\t\t\t/* verify gives error messages itself */\n \t\t\t\tif (verify_pack(the_repository,\n \t\t\t\t\t\tp, fsck_obj_buffer,\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex ab6d6d3bd1..541d7471f1 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -487,10 +487,9 @@ static int too_many_loose_objects(struct gc_config *cfg)\n static struct packed_git *find_base_packs(struct string_list *packs,\n \t\t\t\t\t unsigned long limit)\n {\n-\tstruct packfile_store *packfiles = the_repository->objects->packfiles;\n \tstruct packed_git *p, *base = NULL;\n \n-\tfor (p = packfile_store_get_all_packs(packfiles); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tif (!p->pack_local || p->is_cruft)\n \t\t\tcontinue;\n \t\tif (limit) {\n@@ -509,14 +508,13 @@ static struct packed_git *find_base_packs(struct string_list *packs,\n \n static int too_many_packs(struct gc_config *cfg)\n {\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct packed_git *p;\n-\tint cnt;\n+\tint cnt = 0;\n \n \tif (cfg->gc_auto_pack_limit <= 0)\n \t\treturn 0;\n \n-\tfor (cnt = 0, p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tif (!p->pack_local)\n \t\t\tcontinue;\n \t\tif (p->pack_keep)\n@@ -1425,9 +1423,9 @@ static int incremental_repack_auto_condition(struct gc_config *cfg UNUSED)\n \tif (incremental_repack_auto_limit < 0)\n \t\treturn 1;\n \n-\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles);\n-\t count < incremental_repack_auto_limit && p;\n-\t p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n+\t\tif (count >= incremental_repack_auto_limit)\n+\t\t\tbreak;\n \t\tif (!p->multi_pack_index)\n \t\t\tcount++;\n \t}\n@@ -1494,7 +1492,7 @@ static off_t get_auto_pack_size(void)\n \tstruct repository *r = the_repository;\n \n \todb_reprepare(r->objects);\n-\tfor (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {\n+\trepo_for_each_pack(r, p) {\n \t\tif (p->pack_size > max_size) {\n \t\t\tsecond_largest_size = max_size;\n \t\t\tmax_size = p->pack_size;\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex fe987fbb8b..50618e1073 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3831,12 +3831,10 @@ static int pack_mtime_cmp(const void *_a, const void *_b)\n \n static void read_packs_list_from_stdin(struct rev_info *revs)\n {\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct string_list include_packs = STRING_LIST_INIT_DUP;\n \tstruct string_list exclude_packs = STRING_LIST_INIT_DUP;\n \tstruct string_list_item *item = NULL;\n-\n \tstruct packed_git *p;\n \n \twhile (strbuf_getline(&buf, stdin) != EOF) {\n@@ -3856,7 +3854,7 @@ static void read_packs_list_from_stdin(struct rev_info *revs)\n \tstring_list_sort(&exclude_packs);\n \tstring_list_remove_duplicates(&exclude_packs, 0);\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tconst char *pack_name = pack_basename(p);\n \n \t\tif ((item = string_list_lookup(&include_packs, pack_name)))\n@@ -4077,7 +4075,6 @@ static void enumerate_cruft_objects(void)\n \n static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs)\n {\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct packed_git *p;\n \tstruct rev_info revs;\n \tint ret;\n@@ -4107,7 +4104,7 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs\n \t * Re-mark only the fresh packs as kept so that objects in\n \t * unknown packs do not halt the reachability traversal early.\n \t */\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next)\n+\trepo_for_each_pack(the_repository, p)\n \t\tp->pack_keep_in_core = 0;\n \tmark_pack_kept_in_core(fresh_packs, 1);\n \n@@ -4124,7 +4121,6 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs\n \n static void read_cruft_objects(void)\n {\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct string_list discard_packs = STRING_LIST_INIT_DUP;\n \tstruct string_list fresh_packs = STRING_LIST_INIT_DUP;\n@@ -4145,7 +4141,7 @@ static void read_cruft_objects(void)\n \tstring_list_sort(&discard_packs);\n \tstring_list_sort(&fresh_packs);\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tconst char *pack_name = pack_basename(p);\n \t\tstruct string_list_item *item;\n \n@@ -4440,13 +4436,12 @@ static int loosened_object_can_be_discarded(const struct object_id *oid,\n \n static void loosen_unused_packed_objects(void)\n {\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct packed_git *p;\n \tuint32_t i;\n \tuint32_t loosened_objects_nr = 0;\n \tstruct object_id oid;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tif (!p->pack_local || p->pack_keep || p->pack_keep_in_core)\n \t\t\tcontinue;\n \n@@ -4747,13 +4742,12 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)\n \n static void add_extra_kept_packs(const struct string_list *names)\n {\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct packed_git *p;\n \n \tif (!names->nr)\n \t\treturn;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tconst char *name = basename(p->pack_name);\n \t\tint i;\n \n@@ -5191,10 +5185,9 @@ int cmd_pack_objects(int argc,\n \n \tadd_extra_kept_packs(&keep_pack_list);\n \tif (ignore_packed_keep_on_disk) {\n-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *p;\n \n-\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next)\n+\t\trepo_for_each_pack(the_repository, p)\n \t\t\tif (p->pack_local && p->pack_keep)\n \t\t\t\tbreak;\n \t\tif (!p) /* no keep-able packs found */\n@@ -5206,10 +5199,9 @@ int cmd_pack_objects(int argc,\n \t\t * want to unset \"local\" based on looking at packs, as\n \t\t * it also covers non-local objects\n \t\t */\n-\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\tstruct packed_git *p;\n \n-\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\t\trepo_for_each_pack(the_repository, p) {\n \t\t\tif (!p->pack_local) {\n \t\t\t\thave_non_local_packs = 1;\n \t\t\t\tbreak;\ndiff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c\nindex dd28171f0a..fca7f195d6 100644\n--- a/builtin/pack-redundant.c\n+++ b/builtin/pack-redundant.c\n@@ -566,29 +566,23 @@ static struct pack_list * add_pack(struct packed_git *p)\n \n static struct pack_list * add_pack_file(const char *filename)\n {\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n-\tstruct packed_git *p = packfile_store_get_all_packs(packs);\n+\tstruct packed_git *p;\n \n \tif (strlen(filename) < 40)\n \t\tdie(\"Bad pack filename: %s\", filename);\n \n-\twhile (p) {\n+\trepo_for_each_pack(the_repository, p)\n \t\tif (strstr(p->pack_name, filename))\n \t\t\treturn add_pack(p);\n-\t\tp = p->next;\n-\t}\n \tdie(\"Filename %s not found in packed_git\", filename);\n }\n \n static void load_all(void)\n {\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n-\tstruct packed_git *p = packfile_store_get_all_packs(packs);\n+\tstruct packed_git *p;\n \n-\twhile (p) {\n+\trepo_for_each_pack(the_repository, p)\n \t\tadd_pack(p);\n-\t\tp = p->next;\n-\t}\n }\n \n int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, struct repository *repo UNUSED) {\ndiff --git a/connected.c b/connected.c\nindex b288a18b17..79403108dd 100644\n--- a/connected.c\n+++ b/connected.c\n@@ -74,10 +74,9 @@ int check_connected(oid_iterate_fn fn, void *cb_data,\n \t\t */\n \t\todb_reprepare(the_repository->objects);\n \t\tdo {\n-\t\t\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \t\t\tstruct packed_git *p;\n \n-\t\t\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\t\t\trepo_for_each_pack(the_repository, p) {\n \t\t\t\tif (!p->pack_promisor)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (find_pack_entry_one(oid, p))\ndiff --git a/http-backend.c b/http-backend.c\nindex 9084058f1e..52f0483dd3 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -603,19 +603,18 @@ static void get_head(struct strbuf *hdr, char *arg UNUSED)\n static void get_info_packs(struct strbuf *hdr, char *arg UNUSED)\n {\n \tsize_t objdirlen = strlen(repo_get_object_directory(the_repository));\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct packed_git *p;\n \tsize_t cnt = 0;\n \n \tselect_getanyfile(hdr);\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tif (p->pack_local)\n \t\t\tcnt++;\n \t}\n \n \tstrbuf_grow(&buf, cnt * 53 + 2);\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tif (p->pack_local)\n \t\t\tstrbuf_addf(&buf, \"P %s\\n\", p->pack_name + objdirlen + 6);\n \t}\ndiff --git a/http.c b/http.c\nindex 7e3af1e72f..17130823f0 100644\n--- a/http.c\n+++ b/http.c\n@@ -2416,7 +2416,6 @@ static char *fetch_pack_index(unsigned char *hash, const char *base_url)\n static int fetch_and_setup_pack_index(struct packed_git **packs_head,\n \tunsigned char *sha1, const char *base_url)\n {\n-\tstruct packfile_store *packs = the_repository->objects->packfiles;\n \tstruct packed_git *new_pack, *p;\n \tchar *tmp_idx = NULL;\n \tint ret;\n@@ -2425,7 +2424,7 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head,\n \t * If we already have the pack locally, no need to fetch its index or\n \t * even add it to list; we already have all of its objects.\n \t */\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tif (hasheq(p->hash, sha1, the_repository->hash_algo))\n \t\t\treturn 0;\n \t}\ndiff --git a/object-name.c b/object-name.c\nindex 4e62bfa330..766c757042 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -213,9 +213,11 @@ static void find_short_packed_object(struct disambiguate_state *ds)\n \t\t\tunique_in_midx(m, ds);\n \t}\n \n-\tfor (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n-\t p = p->next)\n+\trepo_for_each_pack(ds->repo, p) {\n+\t\tif (ds->ambiguous)\n+\t\t\tbreak;\n \t\tunique_in_pack(p, ds);\n+\t}\n }\n \n static int finish_object_disambiguation(struct disambiguate_state *ds,\n@@ -805,7 +807,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)\n \t\t\tfind_abbrev_len_for_midx(m, mad);\n \t}\n \n-\tfor (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next)\n+\trepo_for_each_pack(mad->repo, p)\n \t\tfind_abbrev_len_for_pack(p, mad);\n }\n \ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex ac71035d77..291e1a9cf4 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -664,7 +664,7 @@ static int open_pack_bitmap(struct repository *r,\n \tstruct packed_git *p;\n \tint ret = -1;\n \n-\tfor (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {\n+\trepo_for_each_pack(r, p) {\n \t\tif (open_pack_bitmap_1(bitmap_git, p) == 0) {\n \t\t\tret = 0;\n \t\t\t/*\n@@ -3347,6 +3347,7 @@ static int verify_bitmap_file(const struct git_hash_algo *algop,\n int verify_bitmap_files(struct repository *r)\n {\n \tstruct odb_source *source;\n+\tstruct packed_git *p;\n \tint res = 0;\n \n \todb_prepare_alternates(r->objects);\n@@ -3362,8 +3363,7 @@ int verify_bitmap_files(struct repository *r)\n \t\tfree(midx_bitmap_name);\n \t}\n \n-\tfor (struct packed_git *p = packfile_store_get_all_packs(r->objects->packfiles);\n-\t p; p = p->next) {\n+\trepo_for_each_pack(r, p) {\n \t\tchar *pack_bitmap_name = pack_bitmap_filename(p);\n \t\tres |= verify_bitmap_file(r->hash_algo, pack_bitmap_name);\n \t\tfree(pack_bitmap_name);\ndiff --git a/pack-objects.c b/pack-objects.c\nindex d8eb679735..d6adf0759c 100644\n--- a/pack-objects.c\n+++ b/pack-objects.c\n@@ -86,7 +86,6 @@ struct object_entry *packlist_find(struct packing_data *pdata,\n \n static void prepare_in_pack_by_idx(struct packing_data *pdata)\n {\n-\tstruct packfile_store *packs = pdata->repo->objects->packfiles;\n \tstruct packed_git **mapping, *p;\n \tint cnt = 0, nr = 1U << OE_IN_PACK_BITS;\n \n@@ -96,13 +95,13 @@ static void prepare_in_pack_by_idx(struct packing_data *pdata)\n \t * (i.e. in_pack_idx also zero) should return NULL.\n \t */\n \tmapping[cnt++] = NULL;\n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next, cnt++) {\n+\trepo_for_each_pack(pdata->repo, p) {\n \t\tif (cnt == nr) {\n \t\t\tfree(mapping);\n \t\t\treturn;\n \t\t}\n \t\tp->index = cnt;\n-\t\tmapping[cnt] = p;\n+\t\tmapping[cnt++] = p;\n \t}\n \tpdata->in_pack_by_idx = mapping;\n }\ndiff --git a/packfile.c b/packfile.c\nindex db748b0bd4..ab5859518d 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2099,7 +2099,7 @@ struct packed_git **kept_pack_cache(struct repository *r, unsigned flags)\n \t\t * covers, one kept and one not kept, but the midx returns only\n \t\t * the non-kept version.\n \t\t */\n-\t\tfor (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) {\n+\t\trepo_for_each_pack(r, p) {\n \t\t\tif ((p->pack_keep && (flags & ON_DISK_KEEP_PACKS)) ||\n \t\t\t (p->pack_keep_in_core && (flags & IN_CORE_KEEP_PACKS))) {\n \t\t\t\tALLOC_GROW(packs, nr + 1, alloc);\n@@ -2196,7 +2196,7 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb,\n \tint r = 0;\n \tint pack_errors = 0;\n \n-\tfor (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next) {\n+\trepo_for_each_pack(repo, p) {\n \t\tif ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)\n \t\t\tcontinue;\n \t\tif ((flags & FOR_EACH_OBJECT_PROMISOR_ONLY) &&\ndiff --git a/packfile.h b/packfile.h\nindex 3f38c63476..49484a9b09 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -136,6 +136,14 @@ void packfile_store_reprepare(struct packfile_store *store);\n void packfile_store_add_pack(struct packfile_store *store,\n \t\t\t struct packed_git *pack);\n \n+/*\n+ * Load and iterate through all packs of the given repository. This helper\n+ * function will yield packfiles from all object sources connected to the\n+ * repository.\n+ */\n+#define repo_for_each_pack(repo, p) \\\n+\tfor (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next)\n+\n /*\n * Get all packs managed by the given store, including packfiles that are\n * referenced by multi-pack indices.\ndiff --git a/repack-cruft.c b/repack-cruft.c\nindex accb98bcdb..544a6f2df3 100644\n--- a/repack-cruft.c\n+++ b/repack-cruft.c\n@@ -7,12 +7,11 @@\n static void combine_small_cruft_packs(FILE *in, off_t combine_cruft_below_size,\n \t\t\t\t struct existing_packs *existing)\n {\n-\tstruct packfile_store *packs = existing->repo->objects->packfiles;\n \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \tsize_t i;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(existing->repo, p) {\n \t\tif (!(p->is_cruft && p->pack_local))\n \t\t\tcontinue;\n \ndiff --git a/repack-geometry.c b/repack-geometry.c\nindex 56d651c9ce..4e51003ecc 100644\n--- a/repack-geometry.c\n+++ b/repack-geometry.c\n@@ -29,11 +29,10 @@ void pack_geometry_init(struct pack_geometry *geometry,\n \t\t\tstruct existing_packs *existing,\n \t\t\tconst struct pack_objects_args *args)\n {\n-\tstruct packfile_store *packs = existing->repo->objects->packfiles;\n \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(existing->repo, p) {\n \t\tif (args->local && !p->pack_local)\n \t\t\t/*\n \t\t\t * When asked to only repack local packfiles we skip\ndiff --git a/repack.c b/repack.c\nindex 1982a48165..d690e68718 100644\n--- a/repack.c\n+++ b/repack.c\n@@ -124,11 +124,10 @@ int finish_pack_objects_cmd(const struct git_hash_algo *algop,\n void existing_packs_collect(struct existing_packs *existing,\n \t\t\t const struct string_list *extra_keep)\n {\n-\tstruct packfile_store *packs = existing->repo->objects->packfiles;\n \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(existing->repo, p) {\n \t\tsize_t i;\n \t\tconst char *base;\n \ndiff --git a/server-info.c b/server-info.c\nindex 1d33de821e..b9a710544a 100644\n--- a/server-info.c\n+++ b/server-info.c\n@@ -287,13 +287,12 @@ static int compare_info(const void *a_, const void *b_)\n \n static void init_pack_info(struct repository *r, const char *infofile, int force)\n {\n-\tstruct packfile_store *packs = r->objects->packfiles;\n \tstruct packed_git *p;\n \tint stale;\n \tint i;\n \tsize_t alloc = 0;\n \n-\tfor (p = packfile_store_get_all_packs(packs); p; p = p->next) {\n+\trepo_for_each_pack(r, p) {\n \t\t/* we ignore things on alternate path since they are\n \t\t * not available to the pullers in general.\n \t\t */\ndiff --git a/t/helper/test-find-pack.c b/t/helper/test-find-pack.c\nindex e001dc3066..fc4b8a77b3 100644\n--- a/t/helper/test-find-pack.c\n+++ b/t/helper/test-find-pack.c\n@@ -39,11 +39,12 @@ int cmd__find_pack(int argc, const char **argv)\n \tif (repo_get_oid(the_repository, argv[0], &oid))\n \t\tdie(\"cannot parse %s as an object name\", argv[0]);\n \n-\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next)\n+\trepo_for_each_pack(the_repository, p) {\n \t\tif (find_pack_entry_one(&oid, p)) {\n \t\t\tprintf(\"%s\\n\", p->pack_name);\n \t\t\tactual_count++;\n \t\t}\n+\t}\n \n \tif (count > -1 && count != actual_count)\n \t\tdie(\"bad packfile count %d instead of %d\", actual_count, count);\ndiff --git a/t/helper/test-pack-mtimes.c b/t/helper/test-pack-mtimes.c\nindex 7c428c1601..7a8ee1de24 100644\n--- a/t/helper/test-pack-mtimes.c\n+++ b/t/helper/test-pack-mtimes.c\n@@ -37,7 +37,7 @@ int cmd__pack_mtimes(int argc, const char **argv)\n \tif (argc != 2)\n \t\tusage(pack_mtimes_usage);\n \n-\tfor (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next) {\n+\trepo_for_each_pack(the_repository, p) {\n \t\tstrbuf_addstr(&buf, basename(p->pack_name));\n \t\tstrbuf_strip_suffix(&buf, \".pack\");\n \t\tstrbuf_addstr(&buf, \".mtimes\");\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528377","messageId":"20251009-pks-packfiles-convert-get-all-v2-6-0d73b87ce711@pks.im","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-0-0d73b87ce711@pks.im","subject":"[PATCH v2 6/6] packfile: rename `packfile_store_get_all_packs()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T08:01:40Z","receivedAt":"2025-10-09T08:02:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In a preceding commit we have removed `packfile_store_get_packs()`. With\nthis function removed it's somewhat useless to still have the \"all\"\ninfix in `packfile_store_get_all_packs()`. Rename the latter to drop\nthat infix.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/fast-import.c | 4 ++--\n builtin/pack-objects.c | 4 ++--\n packfile.c | 2 +-\n packfile.h | 4 ++--\n 4 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex b1d5549815..fea914cf9e 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -976,7 +976,7 @@ static int store_object(\n \tif (e->idx.offset) {\n \t\tduplicate_count_by_type[type]++;\n \t\treturn 1;\n-\t} else if (find_oid_pack(&oid, packfile_store_get_all_packs(packs))) {\n+\t} else if (find_oid_pack(&oid, packfile_store_get_packs(packs))) {\n \t\te->type = type;\n \t\te->pack_id = MAX_PACK_ID;\n \t\te->idx.offset = 1; /* just not zero! */\n@@ -1177,7 +1177,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)\n \t\tduplicate_count_by_type[OBJ_BLOB]++;\n \t\ttruncate_pack(&checkpoint);\n \n-\t} else if (find_oid_pack(&oid, packfile_store_get_all_packs(packs))) {\n+\t} else if (find_oid_pack(&oid, packfile_store_get_packs(packs))) {\n \t\te->type = OBJ_BLOB;\n \t\te->pack_id = MAX_PACK_ID;\n \t\te->idx.offset = 1; /* just not zero! */\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 50618e1073..3a19bddd57 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -4394,7 +4394,7 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)\n \tstruct packed_git *p;\n \n \tp = (last_found != (void *)1) ? last_found :\n-\t\t\t\t\tpackfile_store_get_all_packs(packs);\n+\t\t\t\t\tpackfile_store_get_packs(packs);\n \n \twhile (p) {\n \t\tif ((!p->pack_local || p->pack_keep ||\n@@ -4404,7 +4404,7 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)\n \t\t\treturn 1;\n \t\t}\n \t\tif (p == last_found)\n-\t\t\tp = packfile_store_get_all_packs(packs);\n+\t\t\tp = packfile_store_get_packs(packs);\n \t\telse\n \t\t\tp = p->next;\n \t\tif (p == last_found)\ndiff --git a/packfile.c b/packfile.c\nindex ab5859518d..1ae2b2fe1e 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1027,7 +1027,7 @@ void packfile_store_reprepare(struct packfile_store *store)\n \tpackfile_store_prepare(store);\n }\n \n-struct packed_git *packfile_store_get_all_packs(struct packfile_store *store)\n+struct packed_git *packfile_store_get_packs(struct packfile_store *store)\n {\n \tpackfile_store_prepare(store);\n \ndiff --git a/packfile.h b/packfile.h\nindex 49484a9b09..c9d0b93446 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -142,13 +142,13 @@ void packfile_store_add_pack(struct packfile_store *store,\n * repository.\n */\n #define repo_for_each_pack(repo, p) \\\n-\tfor (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next)\n+\tfor (p = packfile_store_get_packs(repo->objects->packfiles); p; p = p->next)\n \n /*\n * Get all packs managed by the given store, including packfiles that are\n * referenced by multi-pack indices.\n */\n-struct packed_git *packfile_store_get_all_packs(struct packfile_store *store);\n+struct packed_git *packfile_store_get_packs(struct packfile_store *store);\n \n /*\n * Get all packs in most-recently-used order.\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528753","messageId":"j6kcv2zs2k5346mol2ojy6xcrtsszgfrm2dlhy263xxygnqus2@jcw4ppho4cnc","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-1-0d73b87ce711@pks.im","subject":"Re: [PATCH v2 1/6] object-name: convert to use `packfile_store_get_all_packs()`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-14T19:07:11Z","receivedAt":"2025-10-14T19:07:13Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/09 10:01AM, Patrick Steinhardt wrote:\n> When searching for abbreviated or when trying to disambiguate object IDs\n> we do this in two steps:\n> \n> 1. We search through the multi-pack index.\n> \n> 2. We search through all packfiles not part of any multi-pack index.\n> \n> The second step uses `packfile_store_get_packs()`, which knows to skip\n> loading any packfiles that are indexed by an MIDX; this is exactly what\n> we want.\n> \n> But that function is somewhat problematic, as its behaviour is stateful\n> and is influenced by `packfile_store_get_all_packs()`. This function\n> basically does the same as `packfile_store_get_packs()`, but in addition\n> it also loads all packfiles indexed by an MIDX. The problem here is that\n> both of these functions act on the same linked list of packfiles, and\n> thus depending on whether or not `get_all_packs()` was called the result\n> returned by `get_packs()` will be different. Consequently, all callers\n> of `get_packs()` need to be prepared to see MIDX'd packs even though\n> these should in theory be excluded.\n\nSo IIUC, calling packfile_store_get_packs() before\npackfile_store_get_all_packs() has been invoked results in all the packs\nbeing returned anyways. This is indeed confusing.\n\n> This interface is confusing and thus potentially dangerous, which is why\n> we're converting all callers of `get_packs()` to use `get_all_packs()`\n> instead.\n>\n> Do so for the above functions in \"object-name.c\". As explained, we\n> already know to skip any MIDX'd packs in both `find_abbrev_len_packed()`\n> and `find_short_packed_object()`, so it's fine to start loading MIDX'd\n> packfiles.\n\nOk, converting all the callers to `get_all_packs()` requires them to\nhandle receiving MIDX packfiles also. It sounds like these callsites are\nalready prepared for this reality though.\n\n> Signed-off-by: Patrick Steinhardt \n> ---\n> object-name.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/object-name.c b/object-name.c\n> index f6902e140d..4e62bfa330 100644\n> --- a/object-name.c\n> +++ b/object-name.c\n> @@ -213,7 +213,7 @@ static void find_short_packed_object(struct disambiguate_state *ds)\n> \t\t\tunique_in_midx(m, ds);\n> \t}\n> \n> -\tfor (p = packfile_store_get_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n> +\tfor (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous;\n> \t p = p->next)\n> \t\tunique_in_pack(p, ds);\n> }\n> @@ -805,7 +805,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)\n> \t\t\tfind_abbrev_len_for_midx(m, mad);\n> \t}\n> \n> -\tfor (p = packfile_store_get_packs(mad->repo->objects->packfiles); p; p = p->next)\n> +\tfor (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next)\n> \t\tfind_abbrev_len_for_pack(p, mad);\n> }\n\nLooks good.\n\n-Justin\n"},{"id":"528754","messageId":"yakvujhjgio2g2ynu5735tov2bmsq6cwtaxksvyxik77mrjiuz@c2rspc2k77iq","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-4-0d73b87ce711@pks.im","subject":"Re: [PATCH v2 4/6] packfile: drop `packfile_store_get_packs()`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-14T19:11:45Z","receivedAt":"2025-10-14T19:11:47Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/09 10:01AM, Patrick Steinhardt wrote:\n> In the preceding commits we have removed all remaining callers of\n> `packfile_store_get_packs()`, the function is thus unused now. Remove\n> it.\n> \n> Signed-off-by: Patrick Steinhardt \n> ---\n[snip]\n> -/*\n> - * Get packs managed by the given store. Does not load the MIDX or any packs\n> - * referenced by it.\n> - */\n> -struct packed_git *packfile_store_get_packs(struct packfile_store *store);\n\nNice cleanup. Now with `packfile_store_get_packs()` gone, the confusing\nstateful behavior here is no more. :)\n\n-Justin\n"},{"id":"528755","messageId":"6w2h4qvnwwci65puh6rmluy4br6m3xku3553473rn3k2dqnx2b@fs7gvglg4666","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-5-0d73b87ce711@pks.im","subject":"Re: [PATCH v2 5/6] packfile: introduce macro to iterate through packs","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-14T19:17:11Z","receivedAt":"2025-10-14T19:17:13Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/09 10:01AM, Patrick Steinhardt wrote:\n> We have a bunch of different sites that want to iterate through all\n> packs of a given `struct packfile_store`. This pattern is somewhat\n> verbose and repetitive, which makes it somewhat cumbersome.\n> \n> Introduce a new macro `repo_for_each_pack()` that removes some of the\n> boilerplate.\n> \n> Signed-off-by: Patrick Steinhardt \n> ---\n[snip]\n> diff --git a/packfile.h b/packfile.h\n> index 3f38c63476..49484a9b09 100644\n> --- a/packfile.h\n> +++ b/packfile.h\n> @@ -136,6 +136,14 @@ void packfile_store_reprepare(struct packfile_store *store);\n> void packfile_store_add_pack(struct packfile_store *store,\n> \t\t\t struct packed_git *pack);\n> \n> +/*\n> + * Load and iterate through all packs of the given repository. This helper\n> + * function will yield packfiles from all object sources connected to the\n> + * repository.\n> + */\n> +#define repo_for_each_pack(repo, p) \\\n> +\tfor (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next)\n\nThe added macro here is a nice quality of life improvement and the\nupdated call sites all look good.\n\n-Justin\n"},{"id":"528756","messageId":"snzz23patquqnveygmcqema2mogrehe6ndnjom5zfzu6mrkhzl@j36yhxbdcgdt","threadId":"64263","inReplyTo":"20251009-pks-packfiles-convert-get-all-v2-0-0d73b87ce711@pks.im","subject":"Re: [PATCH v2 0/6] packfile: remove `packfile_store_get_packs()`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-14T19:19:27Z","receivedAt":"2025-10-14T19:19:29Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/09 10:01AM, Patrick Steinhardt wrote:\n> Changes in v2:\n> - Some commit message improvements.\n> - Adapt `packfile_store_for_each_pack()` to `repo_for_each_pack()`. On\n> the one hand this provides easier ergonomics. On the other hand this\n> will eventually allow us to handle object databases of a different\n> type more easily.\n> - Link to v1: https://lore.kernel.org/r/20251007-pks-packfiles-convert-get-all-v1-0-428227657a89@pks.im\n\nThanks Patrick for the pleasant read. I've reviewed this version at it\nlooks good to me. :)\n\n-Justin\n"}]}