Skip to content

Remove erroneous path normalization optimization + regression test - #12621

Merged
gnodet merged 2 commits into
maven-4.0.xfrom
fix/remove-erroneous-optimization
Jul 29, 2026
Merged

Remove erroneous path normalization optimization + regression test#12621
gnodet merged 2 commits into
maven-4.0.xfrom
fix/remove-erroneous-optimization

Conversation

@gnodet

@gnodet gnodet commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR incorporates the fix from #12617 (by @desruisseaux) and adds the regression test requested in the review.

  • Commit 1 (from Remove erroneous optimization which was skipping the path normalization. #12617): Removes the needRelativize optimization in PathSelector that skipped path relativization when all patterns started with **. The optimization produced false positives when the base directory path itself contained a segment matching the pattern.
  • Commit 2: Adds a regression test testNoFalsePositiveWhenBaseDirectoryMatchesPattern that constructs the exact scenario from the PR description — base directory something/target/test-classes with pattern **/test-classes/** falsely matching files in sibling directories.

Supersedes #12617 (maintainer push was disabled on the fork, so the test could not be added directly to that PR).

Test plan

  • New test fails without the fix (false positive confirmed)
  • All 6 PathSelectorTest tests pass with the fix
  • CI passes

🤖 Generated with Claude Code

desruisseaux and others added 2 commits July 29, 2026 18:56
That optimization can produce false positive if the pattern appears in the base directory.
For example if the base directory is "something/target/test-classes" and if an include or
exclude pattern is "**/test-classes" where "test-classes" is another directory as a child
of the base directory, the optimization would always consider that there is a match.
…tches a pattern

Constructs the exact scenario from the PR description: base directory
"something/target/test-classes" with pattern "**/test-classes/**" would
falsely match files in sibling directories when path relativization was
skipped.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet
gnodet marked this pull request as ready for review July 29, 2026 18:14

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean, well-tested bugfix that removes a flawed path-relativization optimization introduced in #10935.

The first commit (from desruisseaux via #12617) correctly fixes the false-positive matching by making relativization unconditional, and the second commit adds a precise regression test covering the exact scenario (base directory containing test-classes as a segment, pattern **/test-classes/**).

The commit structure properly preserves attribution while superseding #12617 (which lacked a test and had maintainer push disabled).

The regression test is well-constructed: it verifies both the true-positive case (file inside a test-classes/ child directory matches) and the false-positive prevention (file in a sibling other/ directory does not match just because the base path contains test-classes).

CI full-build checks pass on all platforms (macOS/Windows/Linux, JDK 17/21/25). Integration tests still pending at time of review.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

@gnodet
gnodet merged commit c4e54ed into maven-4.0.x Jul 29, 2026
24 checks passed
@gnodet
gnodet deleted the fix/remove-erroneous-optimization branch July 29, 2026 19:24
@gnodet gnodet added this to the 4.0.0-rc-6 milestone Jul 29, 2026
@gnodet gnodet self-assigned this Jul 29, 2026
@gnodet gnodet added the bug Something isn't working label Jul 29, 2026
gnodet added a commit that referenced this pull request Jul 30, 2026
…zation

Forward-port of #12621 from maven-4.0.x to master.

The optimization skipped path relativization when all include/exclude
patterns started with "**". This produced false positives when the base
directory path itself contained a segment matching the pattern (e.g.,
base = "something/target/test-classes" with pattern "**/test-classes/**"
would match every file under the base directory).

Added a regression test for the exact false-positive scenario.

Original fix by: Martin Desruisseaux (@desruisseaux)

Closes #12622
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants