All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] revision: fix --no-walk path filtering regression
@ 2026-07-16 10:47 Kristofer Karlsson via GitGitGadget
  2026-07-16 17:59 ` Junio C Hamano
  2026-07-16 19:35 ` Peter Colberg
  0 siblings, 2 replies; 3+ messages in thread
From: Kristofer Karlsson via GitGitGadget @ 2026-07-16 10:47 UTC (permalink / raw)
  To: git; +Cc: Peter Colberg, Kristofer Karlsson, Kristofer Karlsson

From: Kristofer Karlsson <krka@spotify.com>

Since dd4bc01c0a (revision: use priority queue for non-limited
streaming walks, 2026-05-27), "git rev-list --no-walk <commit>
-- <path>" ignores the path arguments and outputs all commits
regardless of whether they touch the given paths.

That commit introduced a REV_WALK_NO_WALK enum value to separate
--no-walk from the streaming walk in get_revision_1(). The new
case skips process_parents(), which is correct for not enqueuing
parents, but also skips try_to_simplify_commit() which
process_parents() calls to evaluate whether each commit touches
the given paths.

Add a call to try_to_simplify_commit() for the
REV_WALK_NO_WALK case, folding it into the existing
REV_WALK_REFLOG case which already does the same.

Add tests for --no-walk path filtering to t6017. The
"single commit, match" test is defensive and passes without
the fix, while the other two fail without it.

Reported-by: Peter Colberg <pcolberg@redhat.com>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
    revision: fix --no-walk path filtering regression
    
    Fix for a regression reported by Peter Colberg [1] where git rev-list
    --no-walk <commit> -- <path> ignores path arguments since dd4bc01c0a.
    
    Verified against linux.git with the exact example from the report:
    
    git rev-list --topo-order v7.0..v7.1 -- drivers/gpu/drm/ |
    git rev-list --stdin --no-walk=unsorted -- ':!drivers/gpu/drm/'
    
    
    Without fix: 2026 commits (all pass through unfiltered) With fix: 146
    commits (correctly filtered)
    
    [1]
    https://lore.kernel.org/git/CAL71e4NjDTHbKR8z7pSrPpzDrX19JOTR04sArm7P=m5ivqkskA@mail.gmail.com/T/#u

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2181%2Fspkrka%2Fkk%2Fno-walk-pathspec-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2181/spkrka/kk/no-walk-pathspec-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2181

 revision.c                |  2 +-
 t/t6017-rev-list-stdin.sh | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index ccbe2e03d1..e990e3f96b 100644
--- a/revision.c
+++ b/revision.c
@@ -4419,6 +4419,7 @@ static struct commit *get_revision_1(struct rev_info *revs)
 
 		switch (mode) {
 		case REV_WALK_REFLOG:
+		case REV_WALK_NO_WALK:
 			try_to_simplify_commit(revs, commit);
 			break;
 		case REV_WALK_TOPO:
@@ -4432,7 +4433,6 @@ static struct commit *get_revision_1(struct rev_info *revs)
 					    oid_to_hex(&commit->object.oid));
 			}
 			break;
-		case REV_WALK_NO_WALK:
 		case REV_WALK_LIMITED:
 			break;
 		}
diff --git a/t/t6017-rev-list-stdin.sh b/t/t6017-rev-list-stdin.sh
index 4821b90e74..32284f1831 100755
--- a/t/t6017-rev-list-stdin.sh
+++ b/t/t6017-rev-list-stdin.sh
@@ -148,4 +148,22 @@ test_expect_success '--not via stdin does not influence revisions from command l
 	test_cmp expect actual
 '
 
+test_expect_success '--no-walk filters by path (single commit, match)' '
+	git rev-parse side-1 >expect &&
+	git rev-list --no-walk side-1 -- file-1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--no-walk filters by path (single commit, no match)' '
+	git rev-list --no-walk side-2 -- file-1 >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_success '--no-walk with pathspec exclusion' '
+	git rev-parse side-3 side-2 >expect &&
+	git rev-parse side-1 side-2 side-3 >input &&
+	git rev-list --stdin --no-walk -- ":!file-1" <input >actual &&
+	test_cmp expect actual
+'
+
 test_done

base-commit: d35c5399e3e54ac277bb391fc2f6be3e816d312b
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] revision: fix --no-walk path filtering regression
  2026-07-16 10:47 [PATCH] revision: fix --no-walk path filtering regression Kristofer Karlsson via GitGitGadget
@ 2026-07-16 17:59 ` Junio C Hamano
  2026-07-16 19:35 ` Peter Colberg
  1 sibling, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2026-07-16 17:59 UTC (permalink / raw)
  To: Kristofer Karlsson via GitGitGadget
  Cc: git, Peter Colberg, Kristofer Karlsson

"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Kristofer Karlsson <krka@spotify.com>
>
> Since dd4bc01c0a (revision: use priority queue for non-limited
> streaming walks, 2026-05-27), "git rev-list --no-walk <commit>
> -- <path>" ignores the path arguments and outputs all commits
> regardless of whether they touch the given paths.
>
> That commit introduced a REV_WALK_NO_WALK enum value to separate
> --no-walk from the streaming walk in get_revision_1(). The new
> case skips process_parents(), which is correct for not enqueuing
> parents, but also skips try_to_simplify_commit() which
> process_parents() calls to evaluate whether each commit touches
> the given paths.
>
> Add a call to try_to_simplify_commit() for the
> REV_WALK_NO_WALK case, folding it into the existing
> REV_WALK_REFLOG case which already does the same.
>
> Add tests for --no-walk path filtering to t6017. The
> "single commit, match" test is defensive and passes without
> the fix, while the other two fail without it.
>
> Reported-by: Peter Colberg <pcolberg@redhat.com>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
>     revision: fix --no-walk path filtering regression
>     
>     Fix for a regression reported by Peter Colberg [1] where git rev-list
>     --no-walk <commit> -- <path> ignores path arguments since dd4bc01c0a.
>     
>     Verified against linux.git with the exact example from the report:
>     
>     git rev-list --topo-order v7.0..v7.1 -- drivers/gpu/drm/ |
>     git rev-list --stdin --no-walk=unsorted -- ':!drivers/gpu/drm/'
>     
>     
>     Without fix: 2026 commits (all pass through unfiltered) With fix: 146
>     commits (correctly filtered)
>     
>     [1]
>     https://lore.kernel.org/git/CAL71e4NjDTHbKR8z7pSrPpzDrX19JOTR04sArm7P=m5ivqkskA@mail.gmail.com/T/#u

OK.  Without this fix, a commit that applied this patch (which does
not touch Makefile) shows the commit message in

    $ git show HEAD -- Makefile

but with this fix, just like Git 2.54 did, the command stays silent.

Will queue.  Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] revision: fix --no-walk path filtering regression
  2026-07-16 10:47 [PATCH] revision: fix --no-walk path filtering regression Kristofer Karlsson via GitGitGadget
  2026-07-16 17:59 ` Junio C Hamano
@ 2026-07-16 19:35 ` Peter Colberg
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Colberg @ 2026-07-16 19:35 UTC (permalink / raw)
  To: Kristofer Karlsson; +Cc: git

Hi Kristofer,

On Thu, Jul 16, 2026 at 10:47:58AM +0000, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
> 
> Since dd4bc01c0a (revision: use priority queue for non-limited
> streaming walks, 2026-05-27), "git rev-list --no-walk <commit>
> -- <path>" ignores the path arguments and outputs all commits
> regardless of whether they touch the given paths.
> 
> That commit introduced a REV_WALK_NO_WALK enum value to separate
> --no-walk from the streaming walk in get_revision_1(). The new
> case skips process_parents(), which is correct for not enqueuing
> parents, but also skips try_to_simplify_commit() which
> process_parents() calls to evaluate whether each commit touches
> the given paths.
> 
> Add a call to try_to_simplify_commit() for the
> REV_WALK_NO_WALK case, folding it into the existing
> REV_WALK_REFLOG case which already does the same.
> 
> Add tests for --no-walk path filtering to t6017. The
> "single commit, match" test is defensive and passes without
> the fix, while the other two fail without it.
> 
> Reported-by: Peter Colberg <pcolberg@redhat.com>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>

Thank you very much for the fix, which passes as well for my use case.

Peter

> ---
>     revision: fix --no-walk path filtering regression
>     
>     Fix for a regression reported by Peter Colberg [1] where git rev-list
>     --no-walk <commit> -- <path> ignores path arguments since dd4bc01c0a.
>     
>     Verified against linux.git with the exact example from the report:
>     
>     git rev-list --topo-order v7.0..v7.1 -- drivers/gpu/drm/ |
>     git rev-list --stdin --no-walk=unsorted -- ':!drivers/gpu/drm/'
>     
>     
>     Without fix: 2026 commits (all pass through unfiltered) With fix: 146
>     commits (correctly filtered)
>     
>     [1]
>     https://lore.kernel.org/git/CAL71e4NjDTHbKR8z7pSrPpzDrX19JOTR04sArm7P=m5ivqkskA@mail.gmail.com/T/#u
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2181%2Fspkrka%2Fkk%2Fno-walk-pathspec-fix-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2181/spkrka/kk/no-walk-pathspec-fix-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2181
> 
>  revision.c                |  2 +-
>  t/t6017-rev-list-stdin.sh | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/revision.c b/revision.c
> index ccbe2e03d1..e990e3f96b 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -4419,6 +4419,7 @@ static struct commit *get_revision_1(struct rev_info *revs)
>  
>  		switch (mode) {
>  		case REV_WALK_REFLOG:
> +		case REV_WALK_NO_WALK:
>  			try_to_simplify_commit(revs, commit);
>  			break;
>  		case REV_WALK_TOPO:
> @@ -4432,7 +4433,6 @@ static struct commit *get_revision_1(struct rev_info *revs)
>  					    oid_to_hex(&commit->object.oid));
>  			}
>  			break;
> -		case REV_WALK_NO_WALK:
>  		case REV_WALK_LIMITED:
>  			break;
>  		}
> diff --git a/t/t6017-rev-list-stdin.sh b/t/t6017-rev-list-stdin.sh
> index 4821b90e74..32284f1831 100755
> --- a/t/t6017-rev-list-stdin.sh
> +++ b/t/t6017-rev-list-stdin.sh
> @@ -148,4 +148,22 @@ test_expect_success '--not via stdin does not influence revisions from command l
>  	test_cmp expect actual
>  '
>  
> +test_expect_success '--no-walk filters by path (single commit, match)' '
> +	git rev-parse side-1 >expect &&
> +	git rev-list --no-walk side-1 -- file-1 >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '--no-walk filters by path (single commit, no match)' '
> +	git rev-list --no-walk side-2 -- file-1 >actual &&
> +	test_must_be_empty actual
> +'
> +
> +test_expect_success '--no-walk with pathspec exclusion' '
> +	git rev-parse side-3 side-2 >expect &&
> +	git rev-parse side-1 side-2 side-3 >input &&
> +	git rev-list --stdin --no-walk -- ":!file-1" <input >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done
> 
> base-commit: d35c5399e3e54ac277bb391fc2f6be3e816d312b
> -- 
> gitgitgadget
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-16 19:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 10:47 [PATCH] revision: fix --no-walk path filtering regression Kristofer Karlsson via GitGitGadget
2026-07-16 17:59 ` Junio C Hamano
2026-07-16 19:35 ` Peter Colberg

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.