[PATCH 2/4] pack-objects: ensure tree/tag closure with '--stdin-packs=follow'
- From
Taylor Blau <ttaylorr@openai.com>
- Date
- Sep 30, 2026, 01:28 UTC
- Message-ID
- <6348667e2e3fe63aeb139888e877dd8447570253.1790731662.git.me@ttaylorr.com>
- In-Reply-To
- <cover.1790731662.git.me@ttaylorr.com>
Since 5ee86c273bf (repack: exclude cruft pack(s) from the MIDX where possible, 2025-06-23), when the 'repack.midxMustContainCruft' configuration is set to "false", geometric repacks use '--stdin-packs=follow' to copy needed objects out of cruft packs so the MIDX can omit those packs.
In cd846bacc7d (pack-objects: introduce '--stdin-packs=follow', 2025-06-23), this behavior changed such that whenever excluded-open ('!') packs are present, the walk stops at objects in excluded-closed ('^') packs. Geometric repacks use '^' for retained packs already in the MIDX, relying on the indexed object set being closed under reachability.
However, the walk introduced in cd846bacc7d starts only from commit objects. A geometric repack can therefore produce a MIDX that does not maintain reachability closure for lone trees (that are not reachable from any commit otherwise in the closure).
A later walk with '!' packs can stop at that tree in a retained '^' pack even if a new commit reaches it. If the cruft pack remains excluded, and the bitmap selection picks one or more commits which reach that tree, the MIDX cannot generate a bitmap for that commit.
Add trees and tags from included and '!' packs (and loose ones with '--unpacked') as roots in '--stdin-packs=follow' mode. This rescues their descendants even when no input commit reaches them. Walk these roots after the existing traversal, preserving the `SEEN` bit to avoid redundant traversals. Ensure that the walk takes place *after* the existing traversal so that we don't lose the path prefix used for trees and blobs wherever possible.
Objects in '^' packs remain cutoffs to avoid rewalking packs that are known to be closed under reachability.
Signed-off-by: Taylor Blau <ttaylorr@openai.com> --- Documentation/git-pack-objects.adoc | 2 + builtin/pack-objects.c | 32 ++++++++++++ t/t5331-pack-objects-stdin.sh | 81 +++++++++++++++++++++++++++++ t/t7704-repack-cruft.sh | 20 +++++++ 4 files changed, 135 insertions(+)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc index 65cd00c152f..1564d44f49d 100644 --- a/Documentation/git-pack-objects.adoc +++ b/Documentation/git-pack-objects.adoc @@ -112,6 +112,8 @@ pack may include additional objects based on the following: This mode is useful, for example, to resurrect once-unreachable objects found in cruft packs to generate packs which are closed under reachability up to the boundary set by the excluded packs. +Trees and tags in included or `!` packs are followed even when no +commit reaches them, as are loose trees and tags with `--unpacked`. + Incompatible with `--revs`, or options that imply `--revs` (such as `--all`), with the exception of `--unpacked`, which is compatible. diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 01adf80a2bc..05a94305265 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -3807,6 +3807,7 @@ static int stdin_packs_hints_nr; struct stdin_packs_context { struct rev_info *revs; enum stdin_packs_mode mode; + struct oid_array extra_roots; }; static int add_object_entry_from_pack(const struct object_id *oid, @@ -3846,6 +3847,9 @@ static int add_object_entry_from_pack(const struct object_id *oid, * list after checking `want_object_in_pack()` below. */ add_pending_oid(ctx->revs, NULL, oid, 0); + } else if (ctx->mode == STDIN_PACKS_MODE_FOLLOW && + (type == OBJ_TREE || type == OBJ_TAG)) { + oid_array_append(&ctx->extra_roots, oid); } if (!want_object_in_pack(oid, 0, &p, &ofs)) @@ -4103,6 +4107,7 @@ static void read_stdin_packs(struct repository *repo, struct stdin_packs_context ctx = { .revs = &revs, .mode = mode, + .extra_roots = OID_ARRAY_INIT, }; /* @@ -4151,6 +4156,30 @@ static void read_stdin_packs(struct repository *repo, show_object_pack_hint, &mode); + /* + * Trees and tags need closure even when no commit reaches them. + * Defer adding these roots to revs.pending until the commit walk + * finishes. Otherwise a subtree may be visited and marked SEEN + * before its commit's root tree, using "a" instead of "sub/a" for + * a blob's namehash and delta attributes. + */ + for (size_t i = 0; i < ctx.extra_roots.nr; i++) { + const struct object_id *oid = &ctx.extra_roots.oid[i]; + struct object *obj = lookup_object(repo, oid); + + if (!obj || !(obj->flags & SEEN)) + add_pending_oid(&revs, NULL, oid, 0); + } + if (revs.pending.nr) { + if (prepare_revision_walk(&revs)) + die(_("revision walk setup failed")); + traverse_commit_list(&revs, + show_commit_pack_hint, + show_object_pack_hint, + &mode); + } + oid_array_clear(&ctx.extra_roots); + release_revisions(&revs); trace2_data_intmax("pack-objects", the_repository, "stdin_packs_found", @@ -4574,6 +4603,9 @@ static int add_loose_object(const struct object_id *oid, const char *path, if (ctx && type == OBJ_COMMIT) add_pending_oid(ctx->revs, NULL, oid, 0); + else if (ctx && ctx->mode == STDIN_PACKS_MODE_FOLLOW && + (type == OBJ_TREE || type == OBJ_TAG)) + oid_array_append(&ctx->extra_roots, oid); return 0; } diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh index c74b5861af3..aa79ecdf13c 100755 --- a/t/t5331-pack-objects-stdin.sh +++ b/t/t5331-pack-objects-stdin.sh @@ -520,4 +520,85 @@ test_expect_success '--stdin-packs with !-delimited pack without follow' ' ) ' +test_expect_success '--stdin-packs=follow traverses a tree-only input pack' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + tree=$(git rev-parse HEAD^{tree}) && + P=$(echo "$tree" | git pack-objects $packdir/pack) && + echo "pack-$P.pack" >in && + + # Only --stdin-packs=follow should start a walk from the tree. + : >trace.txt && + GIT_TRACE2_EVENT="$(pwd)/trace.txt" git pack-objects \ + --stdin-packs --stdout <in >/dev/null && + + test_trace2_data pack-objects stdin_packs_hints 0 <trace.txt && + + P=$(git pack-objects --stdin-packs=follow $packdir/pack <in) && + git rev-parse "$tree" "$tree:base.t" >expect.raw && + sort expect.raw >expect && + objects_in_packs $P >actual && + + test_cmp expect actual + ) +' + +test_expect_success '--stdin-packs=follow traverses an excluded-open tag' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit --annotate base && + + # Put the commit, tree, and blob in one pack, and the tag in another. + # Give only the second pack as input with a "!" prefix. The result + # must contain the commit, tree, and blob, but not the tag. + P=$(echo HEAD | git pack-objects --revs $packdir/pack) && + objects_in_packs $P >expect && + + git rev-parse base >in && + P=$(git pack-objects $packdir/pack <in) && + git prune-packed && + + echo "!pack-$P.pack" >in && + P=$(git pack-objects --stdin-packs=follow $packdir/pack <in) && + objects_in_packs $P >actual && + + test_cmp expect actual + ) +' + +test_expect_success '--stdin-packs=follow respects delta attributes for subtree contents' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + + echo "sub/* -delta" >.gitattributes && + mkdir sub && + test-tool genrandom seed 8192 >sub/a && + cp sub/a sub/b && + echo modified >>sub/b && + git add sub && + git commit -m base && + + # If the subtree is visited first, the blobs are found as a and + # b, so the sub/* attribute does not apply. + git rev-parse HEAD HEAD:sub >in && + P=$(git pack-objects $packdir/pack <in) && + echo "pack-$P.pack" >in && + + git pack-objects --stdin-packs=follow $packdir/pack <in && + git prune-packed && + + printf "%s\n" HEAD:sub/a HEAD:sub/b | + git cat-file --batch-check="%(deltabase)" >actual && + printf "%s\n" "$ZERO_OID" "$ZERO_OID" >expect && + test_cmp expect actual + ) +' + test_done diff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh index b342e82447d..b49f22878f7 100755 --- a/t/t7704-repack-cruft.sh +++ b/t/t7704-repack-cruft.sh @@ -767,6 +767,26 @@ test_expect_success 'repack --write-midx excludes cruft where possible' ' ) ' +test_expect_success 'geometric repack rescues descendants of loose trees' ' + git init loose-tree-cruft && + ( + cd loose-tree-cruft && + git config repack.midxMustContainCruft false && + test_commit base && + blob=$(echo cruft | git hash-object -w --stdin) && + GIT_TEST_MULTI_PACK_INDEX=0 git repack --cruft -d && + + printf "100644 blob %s\tfile\n" "$blob" | git mktree && + GIT_TEST_MULTI_PACK_INDEX=0 git repack -d --geometric=2 \ + --write-midx --write-bitmap-index && + + test-tool read-midx --show-objects $objdir >midx && + cruft=$(ls $packdir/*.mtimes) && + test_grep ! "$(basename "$cruft" .mtimes).idx" midx && + test_grep "^$blob " midx + ) +' + test_expect_success 'repack --write-midx includes cruft when instructed' ' setup_cruft_exclude_tests exclude-cruft-when-instructed && (
-- 2.56.0.4.gbee41d2fc68