From 6b67a3a81ebd9a7c27f9387881b0b46558ee429a Mon Sep 17 00:00:00 2001 From: Martin Desruisseaux Date: Thu, 30 Jul 2026 06:52:51 +0200 Subject: [PATCH] Remove PathSelector include-based directory pre-filtering optimization The optimization was producing false negatives. It is safe to use an exclude filter for directories during FileVisitor traversal, but unsafe to use an include filter because parent directories need to be traversed first before matching children. PR: #12623 Author: Martin Desruisseaux (@desruisseaux) Closes #12623 --- .../org/apache/maven/impl/PathSelector.java | 28 +++---------------- .../impl/DefaultPathMatcherFactoryTest.java | 11 ++++++-- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/PathSelector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/PathSelector.java index bc90337c64ec..736e908b3b36 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/PathSelector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/PathSelector.java @@ -536,13 +536,6 @@ private final class DirectoryPrefiltering implements PathMatcher { */ private final PathMatcher[] dirExcludes; - /** - * Whether to ignore the includes defined by the enclosing class. - * This flag can be {@code false} if we determined that all includes are applicable to directories. - * This flag should be {@code true} in case of doubt since directory filtering is only an optimization. - */ - private final boolean ignoreIncludes; - /** * Creates a new matcher for directories. */ @@ -558,17 +551,9 @@ private final class DirectoryPrefiltering implements PathMatcher { if (excludeDirPatterns.contains(DEFAULT_SYNTAX)) { // A pattern was something like "glob:{/**,}", which exclude everything. dirExcludes = new PathMatcher[] {INCLUDES_ALL}; - ignoreIncludes = true; - return; - } - dirExcludes = matchers(baseDirectory.getFileSystem(), excludeDirPatterns.toArray(String[]::new)); - for (String pattern : includePatterns) { - if (trimSuffixes(pattern) == pattern) { // Identity comparison is sufficient here. - ignoreIncludes = true; - return; - } + } else { + dirExcludes = matchers(baseDirectory.getFileSystem(), excludeDirPatterns.toArray(String[]::new)); } - ignoreIncludes = (includes.length == 0); } /** @@ -593,12 +578,7 @@ private static String trimSuffixes(String pattern) { */ PathMatcher simplify() { if (dirExcludes.length == 0) { - if (ignoreIncludes) { - return INCLUDES_ALL; - } - if (includes.length == 1) { - return includes[0]; - } + return INCLUDES_ALL; } return this; } @@ -619,7 +599,7 @@ public boolean matches(Path directory) { if (isMatched(directory, dirExcludes)) { return false; } - return ignoreIncludes || isMatched(directory, includes); + return true; } } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultPathMatcherFactoryTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultPathMatcherFactoryTest.java index 63c04c129c3e..f7f3dd9bf800 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultPathMatcherFactoryTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultPathMatcherFactoryTest.java @@ -259,9 +259,16 @@ public void testWildcardMatchesAlsoZeroDirectory() { assertTrue(anyMatcher.matches(dir.resolve(Path.of("org", "foo", "more")))); assertTrue(dirMatcher.matches(dir.resolve(Path.of("org", "0foo0", "more")))); assertTrue(anyMatcher.matches(dir.resolve(Path.of("org", "0foo0", "more")))); - assertFalse(dirMatcher.matches(dir.resolve(Path.of("org", "bar", "more")))); + + // Before matching an "org/foo" directory, a `FileVisitor` will need to traverse the "org" directory first. + assertTrue(dirMatcher.matches(dir.resolve(Path.of("org")))); + + // Ideally, `dirMatcher` would return false. But it is hard to implement without false negative on "org". + assertTrue(dirMatcher.matches(dir.resolve(Path.of("org", "bar", "more")))); assertFalse(anyMatcher.matches(dir.resolve(Path.of("org", "bar", "more")))); - assertFalse(dirMatcher.matches(dir.resolve(Path.of("bar")))); + + // Same reason as above. + assertTrue(dirMatcher.matches(dir.resolve(Path.of("bar")))); assertFalse(anyMatcher.matches(dir.resolve(Path.of("bar")))); } }