git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/8] pack-objects: ensure tree/tag closure with '--stdin-packs=follow'

From
Taylor Blau <ttaylorr@openai.com>
Date
Oct 1, 2026, 04:11 UTC
Message-ID
<940953e5c407046ac6789367f3341dc8ea73d07a.1790827875.git.me@ttaylorr.com>
In-Reply-To
<cover.1790827875.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 subsequent repack 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. This gives directly enumerated commits priority for the path prefixes used by name hashes and delta attributes. Tags can introduce commits in the second walk, so path selection remains best-effort.

Collect the extra roots in an oidset to avoid queuing duplicates. This uses memory for each distinct root and walks its unvisited descendants.

Objects in '^' packs remain cutoffs to avoid rewalking packs that are known to be closed under reachability, provided '!' packs are present. This does not repair existing MIDXs lacking closure; those need a full repack.

Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
 Documentation/git-pack-objects.adoc |  2 +
 builtin/pack-objects.c              | 43 ++++++++++++++-
 t/t5331-pack-objects-stdin.sh       | 81 +++++++++++++++++++++++++++++
 t/t7704-repack-cruft.sh             | 20 +++++++
 4 files changed, 145 insertions(+), 1 deletion(-)
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 a553064fcce..fb603059a92 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; /* must be non-NULL */
 	enum stdin_packs_mode mode;
+	struct oidset 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)) {
+		oidset_insert(&ctx->extra_roots, oid);
 	}
 
 	if (!want_object_in_pack(oid, 0, &p, &ofs))
@@ -4103,7 +4107,10 @@ static void read_stdin_packs(struct repository *repo,
 	struct stdin_packs_context ctx = {
 		.revs = &revs,
 		.mode = mode,
+		.extra_roots = OIDSET_INIT,
 	};
+	struct oidset_iter iter;
+	const struct object_id *oid;
 
 	/*
 	 * The revision walk may hit objects that are promised, only. As the
@@ -4151,6 +4158,34 @@ 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 first 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.
+	 *
+	 * Tags may introduce more commits in the second walk, so this
+	 * does not *always* guarantee that trees are always visited
+	 * with their full paths.
+	 */
+	oidset_iter_init(&ctx.extra_roots, &iter);
+	while ((oid = oidset_iter_next(&iter))) {
+		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);
+	}
+	oidset_clear(&ctx.extra_roots);
+
 	release_revisions(&revs);
 
 	trace2_data_intmax("pack-objects", the_repository, "stdin_packs_found",
@@ -4572,8 +4607,14 @@ static int add_loose_object(const struct object_id *oid, const char *path,
 		add_object_entry(oid, type, "", 0);
 	}
 
-	if (ctx && type == OBJ_COMMIT)
+	if (!ctx)
+		return 0;
+
+	if (type == OBJ_COMMIT)
 		add_pending_oid(ctx->revs, NULL, oid, 0);
+	else if (ctx->mode == STDIN_PACKS_MODE_FOLLOW &&
+		 (type == OBJ_TREE || type == OBJ_TAG))
+		oidset_insert(&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.8.ga42f775cbe2
Previous: Taylor BlauNext: Jeff King
Message 24 of 41 in “repack: various corner cases for cruft-less MIDXs”
  1. 0/4 repack: various corner cases for cruft-less MIDXsTaylor Blau, Sep 30, 2026
  2. 1/4 pack-objects: introduce `stdin_packs_context` structTaylor Blau, Sep 30, 2026
  3. Junio C HamanoSep 30, 2026
  4. Taylor BlauOct 1, 2026
  5. 2/4 pack-objects: ensure tree/tag closure with '--stdin-packs=follow'Taylor Blau, Sep 30, 2026
  6. Junio C HamanoSep 30, 2026
  7. Derrick StoleeSep 30, 2026
  8. Taylor BlauOct 1, 2026
  9. Elijah NewrenOct 1, 2026
  10. Taylor BlauOct 2, 2026
  11. Jeff KingOct 2, 2026
  12. Jeff KingSep 30, 2026
  13. Taylor BlauOct 1, 2026
  14. 3/4 repack: retain cruft packs in MIDXs after incremental repacksTaylor Blau, Sep 30, 2026
  15. Jeff KingSep 30, 2026
  16. Taylor BlauOct 1, 2026
  17. 4/4 repack: retain cruft packs in MIDXs containing kept packsTaylor Blau, Sep 30, 2026
  18. Jeff KingSep 30, 2026
  19. Taylor BlauOct 1, 2026
  20. Jeff KingSep 30, 2026
  21. Taylor BlauOct 1, 2026
  22. 0/8 repack: various corner cases for cruft-less MIDXsTaylor Blau, Oct 1, 2026
  23. 1/8 pack-objects: introduce `stdin_packs_context` structTaylor Blau, Oct 1, 2026
  24. 2/8 pack-objects: ensure tree/tag closure with '--stdin-packs=follow'Taylor Blau, Oct 1, 2026
  25. Jeff KingOct 2, 2026
  26. Taylor BlauOct 3, 2026
  27. Jeff KingOct 3, 2026
  28. 3/8 repack: retain cruft packs in MIDXs after incremental repacksTaylor Blau, Oct 1, 2026
  29. 4/8 repack: use a sorted list for explicitly kept packsTaylor Blau, Oct 1, 2026
  30. Jeff KingOct 2, 2026
  31. 5/8 repack: follow kept packs when omitting cruft from the MIDXTaylor Blau, Oct 1, 2026
  32. Jeff KingOct 2, 2026
  33. Taylor BlauOct 3, 2026
  34. 6/8 repack: track the preferred pack explicitly in MIDX write stepsTaylor Blau, Oct 1, 2026
  35. Jeff KingOct 2, 2026
  36. Taylor BlauOct 3, 2026
  37. Jeff KingOct 3, 2026
  38. 7/8 repack: defer allocating the append plan's write stepTaylor Blau, Oct 1, 2026
  39. 8/8 repack: include required packs in incremental MIDX writesTaylor Blau, Oct 1, 2026
  40. Jeff KingOct 2, 2026
  41. Taylor BlauOct 3, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.