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 <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ca44b7894fc064afe4f55c252a930d8c8bddc815..c53e0c91b992f735b2cb9ef1019c773f0468ee15 100644 (file)
--- 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 4a8df5a389d29fb4dbff9ad30d6cd0f2204cf117..cd949025b982a446d54830cdd8efec5655c8946d 100755 (executable)
--- 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 &&