From 0cd306ebc8c7a9faeced0a2fa7bddeea434bf285 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 5 Jan 2026 14:16:41 +0100 Subject: [PATCH] builtin/pack-objects: exclude promisor objects with "--stdin-packs" It is currently not possible to combine "--exclude-promisor-objects" with "--stdin-packs" because both flags want to set up a revision walk to enumerate the objects to pack. In a subsequent commit though we want to extend geometric repacks to support promisor objects, and for that we need to handle the combination of both flags. There are two cases we have to think about here: - "--stdin-packs" asks us to pack exactly the objects part of the specified packfiles. It is somewhat questionable what to do in the case where the user asks us to exclude promisor objects, but at the same time explicitly passes a promisor pack to us. For now, we simply abort the request as it is self-contradicting. As we have also been dying before this commit there is no regression here. - "--stdin-packs=follow" does the same as the first flag, but it also asks us to include all objects transitively reachable from any object in the packs we are about to repack. This is done by doing the revision walk mentioned further up. Luckily, fixing this case is trivial: we only need to modify the revision walk to also set the `exclude_promisor_objects` field. Note that we do not support the "--exclude-promisor-objects-best-effort" flag for now as we don't need it to support geometric repacking with promisor objects. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 14 ++++++++++--- t/t5331-pack-objects-stdin.sh | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index ca44b7894f..c53e0c91b9 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -3857,8 +3857,11 @@ static void read_packs_list_from_stdin(struct rev_info *revs) repo_for_each_pack(the_repository, p) { const char *pack_name = pack_basename(p); - if ((item = string_list_lookup(&include_packs, pack_name))) + if ((item = string_list_lookup(&include_packs, pack_name))) { + if (exclude_promisor_objects && p->pack_promisor) + die(_("packfile %s is a promisor but --exclude-promisor-objects was given"), p->pack_name); item->util = p; + } if ((item = string_list_lookup(&exclude_packs, pack_name))) item->util = p; } @@ -3936,6 +3939,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked) revs.tree_objects = 1; revs.tag_objects = 1; revs.ignore_missing_links = 1; + revs.exclude_promisor_objects = exclude_promisor_objects; /* avoids adding objects in excluded packs */ ignore_packed_keep_in_core = 1; @@ -5092,9 +5096,13 @@ int cmd_pack_objects(int argc, exclude_promisor_objects_best_effort, "--exclude-promisor-objects-best-effort"); if (exclude_promisor_objects) { - use_internal_rev_list = 1; fetch_if_missing = 0; - strvec_push(&rp, "--exclude-promisor-objects"); + + /* --stdin-packs handles promisor objects separately. */ + if (!stdin_packs) { + use_internal_rev_list = 1; + strvec_push(&rp, "--exclude-promisor-objects"); + } } else if (exclude_promisor_objects_best_effort) { use_internal_rev_list = 1; fetch_if_missing = 0; diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh index 4a8df5a389..cd949025b9 100755 --- a/t/t5331-pack-objects-stdin.sh +++ b/t/t5331-pack-objects-stdin.sh @@ -319,6 +319,45 @@ test_expect_success '--stdin-packs=follow walks into unknown packs' ' ) ' +test_expect_success '--stdin-packs with promisors' ' + test_when_finished "rm -fr repo" && + git init repo && + ( + cd repo && + git config set maintenance.auto false && + git remote add promisor garbage && + git config set remote.promisor.promisor true && + + for c in A B C D + do + echo "$c" >file && + git add file && + git commit --message "$c" && + git tag "$c" || return 1 + done && + + A="$(echo A | git pack-objects --revs $packdir/pack)" && + B="$(echo A..B | git pack-objects --revs $packdir/pack --filter=blob:none)" && + C="$(echo B..C | git pack-objects --revs $packdir/pack)" && + D="$(echo C..D | git pack-objects --revs $packdir/pack)" && + touch $packdir/pack-$B.promisor && + + test_must_fail git pack-objects --stdin-packs --exclude-promisor-objects pack- 2>err <<-EOF && + pack-$B.pack + EOF + test_grep "is a promisor but --exclude-promisor-objects was given" err && + + PACK=$(git pack-objects --stdin-packs=follow --exclude-promisor-objects $packdir/pack <<-EOF + pack-$D.pack + EOF + ) && + objects_in_packs $C $D >expect && + objects_in_packs $PACK >actual && + test_cmp expect actual && + rm -f $packdir/pack-$PACK.* + ) +' + stdin_packs__follow_with_only () { rm -fr stdin_packs__follow_with_only && git init stdin_packs__follow_with_only && -- 2.47.3