slachiewicz opened a new pull request, #1155:
URL: https://github.com/apache/maven-plugin-tools/pull/1155
`excludedScanDirectories` (GH-944, `@since 3.16.0`) has never worked on
Windows for the usage its own documentation shows.
`DefaultPluginToolsRequest.isExcludedScanDirectory()` built a glob straight
out of the configured value:
```java
sourceFs.getPathMatcher("glob:" + excludedScanDirectory).matches(sourcePath)
```
A backslash is the **escape character** of the glob syntax, and
`sun.nio.fs.Globs.toRegexPattern` applies that on Windows too — the `case
'\\':` branch is not conditional on the file system. So a value interpolated
from `${project.basedir}` or `${project.build.directory}`, which is exactly
what the parameter's javadoc recommends, arrives as
`D:\proj\target\generated-sources/annotations`, every separator is consumed,
and the pattern degenerates to something like
`D:projtargetgenerated-sources/annotations`. It can never match a real path, so
the exclusion silently does nothing. A value ending in a separator is worse: a
glob may not end in an escape character, so it fails the build with a
`PatternSyntaxException` rather than merely failing to match.
## How it surfaced
The `gh-944-exclude-source-directory` IT never asserted that the exclusion
took effect. Strengthening that IT on master (#1150) is what exposed this. The
IT is left untouched here.
## The fix
Compare as paths first. That is the documented form of the parameter ("this
only accepts source roots"), and path comparison is separator-, case- and
trailing-separator-safe on every file system.
Globs stay supported — the javadoc promises them ("Globs are also supported
here") and they are the only way to exclude a subtree. To make them work,
backslashes are translated to forward slashes before the pattern is compiled,
but only on file systems that separate names with a backslash. Per
`Globs.toRegexPattern`, a `/` in a glob compiles to the Windows separator, so
this is the portable spelling; on file systems where a backslash is a legal
name character it is left alone, since there it may be a deliberate escape such
as `\*`.
Also skips null/empty entries, which previously resolved to the working
directory and could exclude a source root by accident.
## Testing
The matching logic moved into a package-private method taking a `Path`, so
the whole decision is tied to one `FileSystem` and the Windows behaviour is
unit-testable off Windows. New `DefaultPluginToolsRequestTest` covers
exact-root matching, trailing separators, siblings, empty entries, glob
matching, and the separator translation in both directions. Full reactor unit
tests pass.
One deliberate limitation: these tests exercise the translation directly
rather than running the whole method against a real Windows file system, since
the JDK ships no alternative provider. A
[Jimfs](https://github.com/google/jimfs) `Configuration.windows()` test would
reproduce the original bug end-to-end on Linux CI — Jimfs's glob compiler is a
faithful port of the JDK's — but that means a new test-scoped dependency on
this module. Happy to add it if you would rather have that coverage than avoid
the dependency.
## Notes
- The feature is unreleased: `git tag --contains` on both GH-944 commits
comes back empty, so it is in neither the 3.x nor the 4.x line and no released
behaviour changes here.
- Relative configured values still resolve against `user.dir` rather than
the module basedir. That is pre-existing, out of scope here, and the documented
usage is interpolated absolute paths.
---
Maintenance-line port of #1154, which carries the same change against
`master`. The bug is present on this branch unchanged;
`maven-plugin-tools-3.14.x` and `maven-plugin-tools-3.7.x` predate the feature
entirely and need nothing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
