All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, ttaylorr@openai.com,
	Derrick Stolee <stolee@gmail.com>,
	Derrick Stolee <stolee@gmail.com>
Subject: [PATCH] path-walk: allow revisiting paths from tagged trees
Date: Wed, 04 Nov 2026 19:07:48 +0000	[thread overview]
Message-ID: <pull.2296.git.1793819268371.gitgitgadget@gmail.com> (raw)

From: Derrick Stolee <stolee@gmail.com>

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
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
    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(-)

diff --git a/path-walk.c b/path-walk.c
index 9ca3248ea8..e3a5ba201c 100644
--- a/path-walk.c
+++ b/path-walk.c
@@ -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;
 
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index f0a390e3c6..338f622614 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -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
-- 
gitgitgadget

             reply	other threads:[~2026-11-04 19:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04 19:07 Derrick Stolee via GitGitGadget [this message]
2026-11-04 22:20 ` [PATCH] path-walk: allow revisiting paths from tagged trees Taylor Blau
2026-11-05 17:42   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.2296.git.1793819268371.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stolee@gmail.com \
    --cc=ttaylorr@openai.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.