{"thread":{"id":"64978","subject":"[PATCH] builtin/pack-objects: don't fetch objects when merging packs","startedAt":"2026-02-11T12:45:07Z","lastAt":"2026-02-12T23:46:18Z","messageCount":4,"participants":["Patrick Steinhardt","Junio C Hamano","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"535767","messageId":"20260211-pks-pack-objects-stdin-skip-backfill-fetch-v1-1-870cad56d8ae@pks.im","threadId":"64978","inReplyTo":null,"subject":"[PATCH] builtin/pack-objects: don't fetch objects when merging packs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-11T12:44:59Z","receivedAt":"2026-02-11T12:45:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The \"--stdin-packs\" option can be used to merge objects from multiple\npackfiles given via stdin into a new packfile. One big upside of this\noption is that we don't have to perform a complete rev walk to enumerate\nobjects. Instead, we can simply enumerate all objects that are part of\nthe specified packfiles, which can be significantly faster in very large\nrepositories.\n\nThere is one downside though: when we don't perform a rev walk we also\ndon't have a good way to learn about the respective object's names. As a\nconsequence, we cannot use the name hashes as a heuristic to get better\ndelta selection.\n\nWe try to offset this downside though by performing a localized rev\nwalk: we queue all objects that we're about to repack as interesting,\nand all objects from excluded packfiles as uninteresting. We then\nperform a best-effort rev walk that allows us to fill in object names.\n\nThere is one gotcha here though: when \"--exclude-promisor-objects\" has\nnot been given we will perform backfill fetches for any promised objects\nthat are missing. This used to not be an issue though as this option was\nmutually exclusive with \"--stdin-packs\". But that has changed recently,\nand starting with dcc9c7ef47 (builtin/repack: handle promisor packs with\ngeometric repacking, 2026-01-05) we will now repack promisor packs\nduring geometric compaction. The consequence is that a geometric repack\nmay now perform a bunch of backfill fetches.\n\nWe of course cannot passe \"--exclude-promisor-objects\" to fix this\nissue -- after all, the whole intent is to repack objects part of a\npromisor pack. But arguably we don't have to: the rev walk is intended\nas best effort, and we already configure it to ignore missing links to\nother objects. So we can adapt the walk to unconditionally disable\nfetching any missing objects.\n\nDo so and add a test that verifies we don't backfill any objects.\n\nReported-by: Lukas Wanko \nSigned-off-by: Patrick Steinhardt \n---\nHi,\n\nwe've recently encountered this issue in a partial clone of one of our\nown repositoires. Thanks!\n\nPatrick\n---\n builtin/pack-objects.c | 10 ++++++++++\n t/t5331-pack-objects-stdin.sh | 18 ++++++++++++++++++\n 2 files changed, 28 insertions(+)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 9807dd0eff..4053f9659f 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -3925,8 +3925,16 @@ static void add_unreachable_loose_objects(struct rev_info *revs);\n \n static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)\n {\n+\tint prev_fetch_if_missing = fetch_if_missing;\n \tstruct rev_info revs;\n \n+\t/*\n+\t * The revision walk may hit objects that are promised, only. As the\n+\t * walk is best-effort though we don't want to perform backfill fetches\n+\t * for them.\n+\t */\n+\tfetch_if_missing = 0;\n+\n \trepo_init_revisions(the_repository, &revs, NULL);\n \t/*\n \t * Use a revision walk to fill in the namehash of objects in the include\n@@ -3962,6 +3970,8 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)\n \t\t\t stdin_packs_found_nr);\n \ttrace2_data_intmax(\"pack-objects\", the_repository, \"stdin_packs_hints\",\n \t\t\t stdin_packs_hints_nr);\n+\n+\tfetch_if_missing = prev_fetch_if_missing;\n }\n \n static void add_cruft_object_entry(const struct object_id *oid, enum object_type type,\ndiff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh\nindex cd949025b9..c3bbc76b0d 100755\n--- a/t/t5331-pack-objects-stdin.sh\n+++ b/t/t5331-pack-objects-stdin.sh\n@@ -358,6 +358,24 @@ test_expect_success '--stdin-packs with promisors' '\n \t)\n '\n \n+test_expect_success '--stdin-packs does not perform backfill fetch' '\n+\ttest_when_finished \"rm -rf remote client\" &&\n+\n+\tgit init remote &&\n+\ttest_commit_bulk -C remote 10 &&\n+\tgit -C remote config set --local uploadpack.allowfilter 1 &&\n+\tgit -C remote config set --local uploadpack.allowanysha1inwant 1 &&\n+\n+\tgit clone --filter=tree:0 \"file://$(pwd)/remote\" client &&\n+\t(\n+\t\tcd client &&\n+\t\tls .git/objects/pack/*.promisor | sed \"s|.*/||; s/\\.promisor$/.pack/\" >packs &&\n+\t\ttest_line_count -gt 1 packs &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/event.log\" git pack-objects --stdin-packs pack writes:\n\n> The \"--stdin-packs\" option can be used to merge objects from multiple\n> packfiles given via stdin into a new packfile. One big upside of this\n> option is that we don't have to perform a complete rev walk to enumerate\n> objects. Instead, we can simply enumerate all objects that are part of\n> the specified packfiles, which can be significantly faster in very large\n> repositories.\n>\n> There is one downside though: when we don't perform a rev walk we also\n> don't have a good way to learn about the respective object's names. As a\n> consequence, we cannot use the name hashes as a heuristic to get better\n> delta selection.\n>\n> We try to offset this downside though by performing a localized rev\n> walk: we queue all objects that we're about to repack as interesting,\n> and all objects from excluded packfiles as uninteresting. We then\n> perform a best-effort rev walk that allows us to fill in object names.\n>\n> There is one gotcha here though: when \"--exclude-promisor-objects\" has\n> not been given we will perform backfill fetches for any promised objects\n> that are missing. This used to not be an issue though as this option was\n> mutually exclusive with \"--stdin-packs\". But that has changed recently,\n> and starting with dcc9c7ef47 (builtin/repack: handle promisor packs with\n> geometric repacking, 2026-01-05) we will now repack promisor packs\n> during geometric compaction. The consequence is that a geometric repack\n> may now perform a bunch of backfill fetches.\n>\n> We of course cannot passe \"--exclude-promisor-objects\" to fix this\n> issue -- after all, the whole intent is to repack objects part of a\n> promisor pack. But arguably we don't have to: the rev walk is intended\n> as best effort, and we already configure it to ignore missing links to\n> other objects. So we can adapt the walk to unconditionally disable\n> fetching any missing objects.\n\n\"passe\" -> \"pass\".\n\nOther than that, very nicely described, and the implementation is\nsurprisingly simple (thanks to a single global variable, and\nasumption that makes it safe to use such a single global variable,\ni.e., there is just one packing operation running at a time).\n\nWill queue. Thanks.\n"},{"id":"535823","messageId":"aY11O5pc1Sty7IaJ@pks.im","threadId":"64978","inReplyTo":"xmqqseb7urg3.fsf@gitster.g","subject":"Re: [PATCH] builtin/pack-objects: don't fetch objects when merging packs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-12T06:37:47Z","receivedAt":"2026-02-12T06:37:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Feb 11, 2026 at 09:21:16AM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > The \"--stdin-packs\" option can be used to merge objects from multiple\n> > packfiles given via stdin into a new packfile. One big upside of this\n> > option is that we don't have to perform a complete rev walk to enumerate\n> > objects. Instead, we can simply enumerate all objects that are part of\n> > the specified packfiles, which can be significantly faster in very large\n> > repositories.\n> >\n> > There is one downside though: when we don't perform a rev walk we also\n> > don't have a good way to learn about the respective object's names. As a\n> > consequence, we cannot use the name hashes as a heuristic to get better\n> > delta selection.\n> >\n> > We try to offset this downside though by performing a localized rev\n> > walk: we queue all objects that we're about to repack as interesting,\n> > and all objects from excluded packfiles as uninteresting. We then\n> > perform a best-effort rev walk that allows us to fill in object names.\n> >\n> > There is one gotcha here though: when \"--exclude-promisor-objects\" has\n> > not been given we will perform backfill fetches for any promised objects\n> > that are missing. This used to not be an issue though as this option was\n> > mutually exclusive with \"--stdin-packs\". But that has changed recently,\n> > and starting with dcc9c7ef47 (builtin/repack: handle promisor packs with\n> > geometric repacking, 2026-01-05) we will now repack promisor packs\n> > during geometric compaction. The consequence is that a geometric repack\n> > may now perform a bunch of backfill fetches.\n> >\n> > We of course cannot passe \"--exclude-promisor-objects\" to fix this\n> > issue -- after all, the whole intent is to repack objects part of a\n> > promisor pack. But arguably we don't have to: the rev walk is intended\n> > as best effort, and we already configure it to ignore missing links to\n> > other objects. So we can adapt the walk to unconditionally disable\n> > fetching any missing objects.\n> \n> \"passe\" -> \"pass\".\n\nOops, right. Fixed locally, and I saw that you also fixed it in your\nversion.\n\n> Other than that, very nicely described, and the implementation is\n> surprisingly simple (thanks to a single global variable, and\n> asumption that makes it safe to use such a single global variable,\n> i.e., there is just one packing operation running at a time).\n> \n> Will queue. Thanks.\n\nThanks!\n\nPatrick\n"},{"id":"535902","messageId":"aY5mRiYr7icW4lE2@nand.local","threadId":"64978","inReplyTo":"20260211-pks-pack-objects-stdin-skip-backfill-fetch-v1-1-870cad56d8ae@pks.im","subject":"Re: [PATCH] builtin/pack-objects: don't fetch objects when merging packs","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2026-02-12T23:46:14Z","receivedAt":"2026-02-12T23:46:18Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Feb 11, 2026 at 01:44:59PM +0100, Patrick Steinhardt wrote:\n> The \"--stdin-packs\" option can be used to merge objects from multiple\n> packfiles given via stdin into a new packfile. One big upside of this\n> option is that we don't have to perform a complete rev walk to enumerate\n> objects. Instead, we can simply enumerate all objects that are part of\n> the specified packfiles, which can be significantly faster in very large\n> repositories.\n>\n> There is one downside though: when we don't perform a rev walk we also\n> don't have a good way to learn about the respective object's names. As a\n> consequence, we cannot use the name hashes as a heuristic to get better\n> delta selection.\n>\n> We try to offset this downside though by performing a localized rev\n> walk: we queue all objects that we're about to repack as interesting,\n> and all objects from excluded packfiles as uninteresting. We then\n> perform a best-effort rev walk that allows us to fill in object names.\n\nNicely explained. I think it's reasonable to ask \"well why are we doing\na revwalk, you just told me that --stdin-packs does not require a\n(potentially expensive) traversal?\". I think that the exposition you\nprovided here answers that question nicely.\n\n> There is one gotcha here though: when \"--exclude-promisor-objects\" has\n> not been given we will perform backfill fetches for any promised objects\n> that are missing. This used to not be an issue though as this option was\n> mutually exclusive with \"--stdin-packs\". But that has changed recently,\n> and starting with dcc9c7ef47 (builtin/repack: handle promisor packs with\n> geometric repacking, 2026-01-05) we will now repack promisor packs\n> during geometric compaction. The consequence is that a geometric repack\n> may now perform a bunch of backfill fetches.\n\nGreat find!\n\n> We of course cannot passe \"--exclude-promisor-objects\" to fix this\n\ns/passe/pass, though I think Junio noted this below.\n\n> issue -- after all, the whole intent is to repack objects part of a\n> promisor pack. But arguably we don't have to: the rev walk is intended\n> as best effort, and we already configure it to ignore missing links to\n> other objects. So we can adapt the walk to unconditionally disable\n> fetching any missing objects.\n\nYep, I think this is the right trade-off.\n\n> ---\n> builtin/pack-objects.c | 10 ++++++++++\n> t/t5331-pack-objects-stdin.sh | 18 ++++++++++++++++++\n> 2 files changed, 28 insertions(+)\n\nThe implementation and test look exactly as expected. Thanks for jumping\non this and fixing it!\n\nThanks,\nTaylor\n"}]}