Skip to content

[MNG-8673] SourceRoot includes and excludes should be String #2232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 13, 2025
+24 −52
Merged

[MNG-8673] SourceRoot includes and excludes should be String #2232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1326dbe
[MNG-8673] SourceRoot includes and excludes should be String
gnodet Apr 4, 2025
08a7643
Improve javadoc
gnodet Apr 4, 2025
be17bf9
Specify that patterns are matching relative paths (so no root)
gnodet Apr 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions api/maven-api-core/src/main/java/org/apache/maven/api/SourceRoot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.apache.maven.api;

import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.util.List;
import java.util.Optional;

Expand Down Expand Up @@ -47,10 +46,17 @@ default Path directory() {

/**
* {@return the list of pattern matchers for the files to include}.
* The path separator is {@code /} on all platforms, including Windows.
* The patterns are used to match paths relative to the {@code directory}.
* The prefix before the {@code :} character, if present, is the syntax.
* If no syntax is specified, the default is a Maven-specific variation
* of the {@code "glob"} pattern.
*
* <p>
* The default implementation returns an empty list, which means to apply a language-dependent pattern.
Copy link
Contributor

@desruisseaux desruisseaux Apr 4, 2025

Choose a reason for hiding this comment

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

Proposed new block of comment:

    /**
     * {@return the list of patterns for the files to include}.
     * The path separator is {@code /} on all platforms, including Windows.
     * The prefix before the {@code :} character, if present, is the syntax.
     * If no syntax is specified, the default is a Maven-specific variation
     * of the {@code "glob"} pattern.
     *
     * <p>The default implementation returns an empty list, which means to apply a language-dependent pattern.
     * For example, for the Java language, the default pattern is {@code "*.java"}.</p>
     */

Copy link
Contributor Author

@gnodet gnodet Apr 4, 2025

Choose a reason for hiding this comment

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

What if on windows we have a drive specified ? We need to differentiate:

C:/foo/**

So maybe rephrase as: If the pattern does not start with {@code regex:} or {@code glob:}, the default variation of the {@code glob} pattern will be used.

Copy link
Contributor

@desruisseaux desruisseaux Apr 4, 2025

Choose a reason for hiding this comment

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

I have not tested how we can specify a Windows drive to java.nio.file.FileSystem.getPathMatcher(String). I was hopping that, since all paths are made relative to the base directory before to be tested for match, the Windows drive issue would be avoided.

Copy link
Contributor

@desruisseaux desruisseaux Apr 4, 2025

Choose a reason for hiding this comment

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

Just to complete: "glob:" and "regex:" are the two syntaxes that all FileSystem shall support according NIO Javadoc, but a file system may support others syntaxes as well. It may be nice to avoid a formulation that restrict the possibilities to those two syntaxes.

Copy link
Contributor

@desruisseaux desruisseaux Apr 5, 2025

Choose a reason for hiding this comment

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

According Stackoverflow, Windows drives are restricted to 1 letter. So I suggest to replace "If the pattern does not start with regex: or glob:" by "The prefix before the : character, if present and longer than one character, is the syntax".

* For example, for the Java language, the pattern includes all files with the {@code .java} suffix.
*/
default List<PathMatcher> includes() {
default List<String> includes() {
return List.of();
}

Expand All @@ -59,7 +65,7 @@ default List<PathMatcher> includes() {
* The exclusions are applied after the inclusions.
* The default implementation returns an empty list.
*/
default List<PathMatcher> excludes() {
default List<String> excludes() {
return List.of();
}

Expand Down
4 changes: 2 additions & 2 deletions impl/maven-core/src/main/java/org/apache/maven/project/MavenProject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,8 @@ public boolean add(Resource resource) {
private static Resource toResource(SourceRoot sourceRoot) {
return new Resource(org.apache.maven.api.model.Resource.newBuilder()
.directory(sourceRoot.directory().toString())
.includes(sourceRoot.includes().stream().map(Object::toString).toList())
.excludes(sourceRoot.excludes().stream().map(Object::toString).toList())
.includes(sourceRoot.includes())
.excludes(sourceRoot.excludes())
.filtering(Boolean.toString(sourceRoot.stringFiltering()))
.build());
}
Expand Down
60 changes: 13 additions & 47 deletions impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@
*/
package org.apache.maven.impl;

import java.nio.file.FileSystem;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
Expand All @@ -39,9 +37,9 @@
public final class DefaultSourceRoot implements SourceRoot {
private final Path directory;

private final List<PathMatcher> includes;
private final List<String> includes;

private final List<PathMatcher> excludes;
private final List<String> excludes;

private final ProjectScope scope;

Expand All @@ -65,9 +63,8 @@ public final class DefaultSourceRoot implements SourceRoot {
* @param source a source element from the model
*/
public DefaultSourceRoot(final Session session, final Path baseDir, final Source source) {
FileSystem fs = baseDir.getFileSystem();
includes = matchers(fs, source.getIncludes());
excludes = matchers(fs, source.getExcludes());
includes = source.getIncludes();
excludes = source.getExcludes();
stringFiltering = source.isStringFiltering();
enabled = source.isEnabled();
moduleName = nonBlank(source.getModule());
Expand Down Expand Up @@ -106,9 +103,8 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
throw new IllegalArgumentException("Source declaration without directory value.");
}
directory = baseDir.resolve(value).normalize();
FileSystem fs = directory.getFileSystem();
includes = matchers(fs, resource.getIncludes());
excludes = matchers(fs, resource.getExcludes());
includes = resource.getIncludes();
excludes = resource.getExcludes();
stringFiltering = Boolean.parseBoolean(resource.getFiltering());
enabled = true;
moduleName = null;
Expand Down Expand Up @@ -144,13 +140,15 @@ public DefaultSourceRoot(final ProjectScope scope, final Language language, fina
* @param scope scope of source code (main or test)
* @param language language of the source code
* @param directory directory of the source code
*/
* @param includes list of patterns for the files to include, or {@code null} if unspecified
* @param excludes list of patterns for the files to exclude, or {@code null} if unspecified
* */
public DefaultSourceRoot(
final ProjectScope scope,
final Language language,
final Path directory,
List<PathMatcher> includes,
List<PathMatcher> excludes) {
List<String> includes,
List<String> excludes) {
this.scope = Objects.requireNonNull(scope);
this.language = language;
this.directory = Objects.requireNonNull(directory);
Expand All @@ -176,38 +174,6 @@ private static String nonBlank(String value) {
return value;
}

/**
* Creates a path matcher for each pattern.
* The path separator is {@code /} on all platforms, including Windows.
* The prefix before the {@code :} character is the syntax.
* If no syntax is specified, {@code "glob"} is assumed.
*
* @param fs the file system of the root directory
* @param patterns the patterns for which to create path matcher
* @return a path matcher for each pattern
*/
private static List<PathMatcher> matchers(FileSystem fs, List<String> patterns) {
final var matchers = new PathMatcher[patterns.size()];
for (int i = 0; i < matchers.length; i++) {
String rawPattern = patterns.get(i);
String pattern = rawPattern.contains(":") ? rawPattern : "glob:" + rawPattern;
matchers[i] = new PathMatcher() {
final PathMatcher delegate = fs.getPathMatcher(pattern);

@Override
public boolean matches(Path path) {
return delegate.matches(path);
}

@Override
public String toString() {
return rawPattern;
}
};
}
return List.of(matchers);
}

/**
* {@return the root directory where the sources are stored}.
*/
Expand All @@ -221,7 +187,7 @@ public Path directory() {
*/
@Override
@SuppressWarnings("ReturnOfCollectionOrArrayField") // Safe because unmodifiable
public List<PathMatcher> includes() {
public List<String> includes() {
return includes;
}

Expand All @@ -230,7 +196,7 @@ public List<PathMatcher> includes() {
*/
@Override
@SuppressWarnings("ReturnOfCollectionOrArrayField") // Safe because unmodifiable
public List<PathMatcher> excludes() {
public List<String> excludes() {
return excludes;
}

Expand Down
Loading