revision: fix --no-walk path filtering regression#2181
Open
spkrka wants to merge 1 commit into
Open
Conversation
spkrka
force-pushed
the
kk/no-walk-pathspec-fix
branch
2 times, most recently
from
July 16, 2026 09:40
7645e7c to
c5d7ae3
Compare
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
force-pushed
the
kk/no-walk-pathspec-fix
branch
from
July 16, 2026 09:42
c5d7ae3 to
d03a63d
Compare
Author
|
/submit |
|
Submitted as pull.2181.git.1784198879711.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
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. |
|
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
>
|
|
This patch series was integrated into seen via git@49c2f60. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix for a regression reported by Peter Colberg [1] where
git rev-list --no-walk <commit> -- <path>ignores path argumentssince dd4bc01.
Verified against linux.git with the exact example from the report:
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