Skip to content

revision: fix --no-walk path filtering regression#2181

Open
spkrka wants to merge 1 commit into
gitgitgadget:masterfrom
spkrka:kk/no-walk-pathspec-fix
Open

revision: fix --no-walk path filtering regression#2181
spkrka wants to merge 1 commit into
gitgitgadget:masterfrom
spkrka:kk/no-walk-pathspec-fix

Conversation

@spkrka

@spkrka spkrka commented Jul 16, 2026

Copy link
Copy Markdown

Fix for a regression reported by Peter Colberg [1] where
git rev-list --no-walk <commit> -- <path> ignores path arguments
since dd4bc01.

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

CC: Peter Colberg pcolberg@redhat.com

@spkrka
spkrka force-pushed the kk/no-walk-pathspec-fix branch 2 times, most recently from 7645e7c to c5d7ae3 Compare July 16, 2026 09:40
Since dd4bc01 (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>
@spkrka
spkrka force-pushed the kk/no-walk-pathspec-fix branch from c5d7ae3 to d03a63d Compare July 16, 2026 09:42
@spkrka

spkrka commented Jul 16, 2026

Copy link
Copy Markdown
Author

/submit

@gitgitgadget

gitgitgadget Bot commented Jul 16, 2026

Copy link
Copy Markdown

Submitted as pull.2181.git.1784198879711.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2181/spkrka/kk/no-walk-pathspec-fix-v1

To fetch this version to local tag pr-2181/spkrka/kk/no-walk-pathspec-fix-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2181/spkrka/kk/no-walk-pathspec-fix-v1

@gitgitgadget

gitgitgadget Bot commented Jul 16, 2026

Copy link
Copy Markdown

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"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.

@gitgitgadget

gitgitgadget Bot commented Jul 16, 2026

Copy link
Copy Markdown

Peter Colberg wrote on the Git mailing list (how to reply to this email):

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
> 

@gitgitgadget

gitgitgadget Bot commented Jul 16, 2026

Copy link
Copy Markdown

This patch series was integrated into seen via git@49c2f60.

@gitgitgadget gitgitgadget Bot added the seen label Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant