Skip to contentSkip to conversation

path-walk: allow revisiting paths from tagged trees

PATCH In review Derrick Stolee via GitGitGadget 3 participants 3 messages Nov 4 – Nov 5

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 path-walk: allow revisiting paths from tagged trees

From: Derrick Stolee <[email protected]>

When adding support for tree-depth filters, 5111520e2a1 (path-walk:
support tree:0 filter, 2026-05-22) moved tagged root trees into a separate batch. These trees are expanded after commit root trees and their root-level blob batches. If a tagged tree introduces another blob at an already consumed path, push_to_stack() refuses to queue that path again because path_stack_pushed remembers every previously queued path. The final remaining-path drain makes the same check and also skips the blob, leading to a repository with missing objects.

This regression was first released in Git 2.55.0. The torvalds/linux repository has such tagged trees that revealed the bug.

To resolve this issue, remove a path from path_stack_pushed when consuming its queue entry. The set then suppresses duplicate pending entries without preventing new objects from being visited at a previously consumed path. This preserves the existing visit priorities instead of reordering tagged trees relative to other batches.

Add a regression test with a commit and an annotated tree tag that contain distinct blobs at the same path. After an initial repack to clear loose objects, repack everything with path-walk and run an fsck connectivity check to reveal the error without this fix.

Assisted-by GPT-6.1 Sol

---
path-walk: allow revisiting paths from tagged trees

While exploring potential future improvements to path-walk repacks, I discovered this regression. It's slightly subtle in a way that the existing tree-tag tests didn't catch, due to needing to revisit paths.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2296%2Fderrickstolee%2Fpath-walk-tree-tags-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2296/derrickstolee/path-walk-tree-tags-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2296

 path-walk.c       |  5 ++++-
 t/t7700-repack.sh | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)
path-walk.c +4 -1
index 9ca3248ea8..e3a5ba201c 100644
@@ -62,7 +62,8 @@ struct path_walk_context {
 	 * like a stack and performs a DFS within the trees.
 	 *
 	 * Use path_stack_pushed to indicate whether a path
-	 * was previously added to path_stack.
+	 * is currently queued in path_stack. Tagged trees can
+	 * discover new objects at an already visited path.
 	 */
 	struct prio_queue path_stack;
 	struct strset path_stack_pushed;
@@ -309,6 +310,8 @@ static int walk_path(struct path_walk_context *ctx,
 	if (!list)
 		BUG("provided path '%s' that had no associated list", path);
 
+	strset_remove(&ctx->path_stack_pushed, path);
+
 	if (!list->oids.nr)
 		return 0;
 
t/t7700-repack.sh +23 -0
index f0a390e3c6..338f622614 100755
@@ -904,4 +904,27 @@ test_expect_success 'pending objects are repacked appropriately' '
 	)
 '
 
+test_expect_success '--path-walk keeps blobs from tagged trees at already visited paths' '
+	git init --bare tagged-tree.git &&
+	(
+		cd tagged-tree.git &&
+		GIT_DIR=. &&
+		export GIT_DIR &&
+		old=$(echo old | git hash-object -w --stdin) &&
+		tree=$(printf "100644 blob %s\tfile\n" "$old" | git mktree) &&
+		git tag -a -m "historical tree" tree-tag "$tree" &&
+		new=$(echo new | git hash-object -w --stdin) &&
+		tree=$(printf "100644 blob %s\tfile\n" "$new" | git mktree) &&
+		commit=$(echo new | git commit-tree "$tree") &&
+		git update-ref HEAD "$commit" &&
+
+		# collect all loose objects into packs
+		git repack -adf --no-path-walk &&
+
+		# now confirm that --path-walk keeps all objects.
+		git repack -adf --path-walk &&
+		git fsck --connectivity-only
+	)
+'
+
 test_done

base-commit: a018953688f1b10bddf91bff8747068f5f4746a4

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 3 lines · Derrick Stolee via GitGitGadget
 path-walk.c       |  5 ++++-
 t/t7700-repack.sh | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

Nice find and fix! Thanks for looking at it, this patch LGTM.

Thanks,
Taylor

#2 lore raw
Quoted 6 lines · Taylor Blau
Quoted 3 lines · Derrick Stolee via GitGitGadget
 path-walk.c       |  5 ++++-
 t/t7700-repack.sh | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

Nice find and fix! Thanks for looking at it, this patch LGTM.

Thanks, both.