Skip to content

Conversation

desruisseaux
Copy link
Contributor

This is a port of the following pull requests:

After this fix is released, the PathSelector class of the Maven clean and compiler plugins can be removed.

Question

Are maven-clean-plugin and maven-compiler-plugin allowed to use directly a class from maven-impl instead of maven-api? If not, we may need to move PathSelector in another package or module. But where?

this.excludes = matchers(system, excludePatterns);
dirIncludes = matchers(system, directoryPatterns(includePatterns, false));
dirExcludes = matchers(system, directoryPatterns(excludePatterns, true));
FileSystem fs = directory.getFileSystem();
Copy link
Contributor

Choose a reason for hiding this comment

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

fs --> fileSystem

@gnodet
Copy link
Contributor

gnodet commented Jul 15, 2025

Question

Are maven-clean-plugin and maven-compiler-plugin allowed to use directly a class from maven-impl instead of maven-api? If not, we may need to move PathSelector in another package or module. But where?

Not really, we want a clear API to avoid blurring the edges...
One possibility would be to create a PathMatcherFactory, implementing org.apache.maven.api.Service which could be retrieved from Session.getService(PathMatcherFactory.class). This service would provide methods to create PathMatcher objects.

@gnodet
Copy link
Contributor

gnodet commented Jul 16, 2025

Hi @desruisseaux! 👋

I've created a complementary PR #10923 that addresses the same area but focuses on different aspects:

Your PR #10909: Ports existing bug fixes from maven-compiler-plugin and maven-clean-plugin

My PR #10923:

  • Adds the missing createExcludeOnlyMatcher method to PathMatcherFactory interface
  • Fixes a critical bug in PathSelector.effectiveExcludes that was preventing exclude-only patterns from working correctly
  • Provides comprehensive test coverage

Both PRs complement each other well:

  • Your PR brings the proven fixes from the plugins
  • My PR adds the missing API method and fixes the underlying PathSelector bug
  • Together they provide a complete solution for exclude-only functionality

The PRs can be merged independently or together. My PR includes the same PathSelector fix you identified, plus the additional API method that plugins will need.

Thanks for your work on identifying and porting these important fixes! 🙏

gnodet added a commit to gnodet/maven that referenced this pull request Jul 16, 2025
- Remove PathSelector changes to avoid conflicts with existing PR apache#10909
- PR apache#10909 already includes the necessary PathSelector fix for exclude-only patterns
- Update test to be more conservative and document the dependency on PR apache#10909
- The API method is ready and will work fully once PR apache#10909 is merged
@gnodet
Copy link
Contributor

gnodet commented Jul 16, 2025

Update: I've modified PR #10923 to avoid conflicts! 🚀

After reviewing your PR more carefully, I realized you already include the PathSelector fix needed for exclude-only patterns (if (includes.length == 0) { return excludes; }).

Changes made to PR #10923:

  • Removed PathSelector changes to avoid conflicts
  • Kept the API method that plugins need
  • Updated tests to document dependency on your PR

Result:

This way we avoid duplicating your excellent work while still providing the API method that plugins will need. Both PRs complement each other perfectly! 👍

Thanks for the great work on identifying and fixing these PathSelector issues!

gnodet added a commit to gnodet/maven that referenced this pull request Jul 17, 2025
This PR adds a comprehensive PathMatcherFactory service to Maven 4 API with
directory filtering optimization capabilities, addressing the need for
exclude-only pattern matching and performance optimizations.

## New API Features

### PathMatcherFactory Interface
- createPathMatcher(baseDirectory, includes, excludes, useDefaultExcludes)
- createPathMatcher(baseDirectory, includes, excludes) - convenience overload
- createExcludeOnlyMatcher(baseDirectory, excludes, useDefaultExcludes)
- createIncludeOnlyMatcher(baseDirectory, includes) - convenience method
- deriveDirectoryMatcher(fileMatcher) - directory filtering optimization

### DefaultPathMatcherFactory Implementation
- Full implementation of all PathMatcherFactory methods
- Delegates to PathSelector for actual pattern matching
- Provides directory optimization via PathSelector.couldHoldSelected()
- Fail-safe design returning INCLUDES_ALL for unknown matcher types

## PathSelector Enhancements

### Null Safety Improvements
- Added @nonnull annotation to constructor directory parameter
- Added Objects.requireNonNull() validation with descriptive error message
- Moved baseDirectory assignment to beginning for fail-fast behavior
- Updated JavaDoc to document NullPointerException behavior

### Directory Filtering Support
- Added canFilterDirectories() method to check optimization capability
- Made INCLUDES_ALL field package-private for factory reuse
- Enhanced couldHoldSelected() method accessibility

## Directory Filtering Optimization

The deriveDirectoryMatcher() method enables significant performance improvements
by allowing plugins to skip entire directory trees when they definitively
won't contain matching files. This preserves Maven 3's optimization behavior.

### Usage Example:

## Comprehensive Testing

### DefaultPathMatcherFactoryTest
- Tests all factory methods with various parameter combinations
- Verifies null parameter handling (NullPointerException)
- Tests directory matcher derivation functionality
- Includes edge cases and fail-safe behavior verification

### Backward Compatibility
- All existing PathSelector functionality preserved
- No breaking changes to existing APIs
- Enhanced error handling with better exception types

## Benefits

1. **Plugin Compatibility**: Enables maven-clean-plugin and other plugins
   to use exclude-only patterns efficiently
2. **Performance**: Directory filtering optimization preserves Maven 3 behavior
3. **Developer Experience**: Clean service interface with comprehensive JavaDoc
4. **Robustness**: Fail-fast null validation and defensive programming
5. **Future-Proof**: Extensible design for additional pattern matching needs

## Related Work

This implementation complements PR apache#10909 by @desruisseaux which addresses
PathSelector bug fixes. Both PRs can be merged independently and work
together to provide complete exclude-only functionality.

Addresses performance optimization suggestions and provides the missing
API methods needed by Maven plugins for efficient file filtering.
gnodet added a commit that referenced this pull request Jul 17, 2025
…10923)

This PR adds a comprehensive PathMatcherFactory service to Maven 4 API with
directory filtering optimization capabilities, addressing the need for
exclude-only pattern matching and performance optimizations.

## New API Features

### PathMatcherFactory Interface
- createPathMatcher(baseDirectory, includes, excludes, useDefaultExcludes)
- createPathMatcher(baseDirectory, includes, excludes) - convenience overload
- createExcludeOnlyMatcher(baseDirectory, excludes, useDefaultExcludes)
- createIncludeOnlyMatcher(baseDirectory, includes) - convenience method
- deriveDirectoryMatcher(fileMatcher) - directory filtering optimization

### DefaultPathMatcherFactory Implementation
- Full implementation of all PathMatcherFactory methods
- Delegates to PathSelector for actual pattern matching
- Provides directory optimization via PathSelector.couldHoldSelected()
- Fail-safe design returning INCLUDES_ALL for unknown matcher types

## PathSelector Enhancements

### Null Safety Improvements
- Added @nonnull annotation to constructor directory parameter
- Added Objects.requireNonNull() validation with descriptive error message
- Moved baseDirectory assignment to beginning for fail-fast behavior
- Updated JavaDoc to document NullPointerException behavior

### Directory Filtering Support
- Added canFilterDirectories() method to check optimization capability
- Made INCLUDES_ALL field package-private for factory reuse
- Enhanced couldHoldSelected() method accessibility

## Directory Filtering Optimization

The deriveDirectoryMatcher() method enables significant performance improvements
by allowing plugins to skip entire directory trees when they definitively
won't contain matching files. This preserves Maven 3's optimization behavior.

### Usage Example:

## Comprehensive Testing

### DefaultPathMatcherFactoryTest
- Tests all factory methods with various parameter combinations
- Verifies null parameter handling (NullPointerException)
- Tests directory matcher derivation functionality
- Includes edge cases and fail-safe behavior verification

### Backward Compatibility
- All existing PathSelector functionality preserved
- No breaking changes to existing APIs
- Enhanced error handling with better exception types

## Benefits

1. **Plugin Compatibility**: Enables maven-clean-plugin and other plugins
   to use exclude-only patterns efficiently
2. **Performance**: Directory filtering optimization preserves Maven 3 behavior
3. **Developer Experience**: Clean service interface with comprehensive JavaDoc
4. **Robustness**: Fail-fast null validation and defensive programming
5. **Future-Proof**: Extensible design for additional pattern matching needs

## Related Work

This implementation complements PR #10909 by @desruisseaux which addresses
PathSelector bug fixes. Both PRs can be merged independently and work
together to provide complete exclude-only functionality.

Addresses performance optimization suggestions and provides the missing
API methods needed by Maven plugins for efficient file filtering.
gnodet added a commit to gnodet/maven that referenced this pull request Jul 17, 2025
…pache#10923)

This PR adds a comprehensive PathMatcherFactory service to Maven 4 API with
directory filtering optimization capabilities, addressing the need for
exclude-only pattern matching and performance optimizations.

## New API Features

### PathMatcherFactory Interface
- createPathMatcher(baseDirectory, includes, excludes, useDefaultExcludes)
- createPathMatcher(baseDirectory, includes, excludes) - convenience overload
- createExcludeOnlyMatcher(baseDirectory, excludes, useDefaultExcludes)
- createIncludeOnlyMatcher(baseDirectory, includes) - convenience method
- deriveDirectoryMatcher(fileMatcher) - directory filtering optimization

### DefaultPathMatcherFactory Implementation
- Full implementation of all PathMatcherFactory methods
- Delegates to PathSelector for actual pattern matching
- Provides directory optimization via PathSelector.couldHoldSelected()
- Fail-safe design returning INCLUDES_ALL for unknown matcher types

## PathSelector Enhancements

### Null Safety Improvements
- Added @nonnull annotation to constructor directory parameter
- Added Objects.requireNonNull() validation with descriptive error message
- Moved baseDirectory assignment to beginning for fail-fast behavior
- Updated JavaDoc to document NullPointerException behavior

### Directory Filtering Support
- Added canFilterDirectories() method to check optimization capability
- Made INCLUDES_ALL field package-private for factory reuse
- Enhanced couldHoldSelected() method accessibility

## Directory Filtering Optimization

The deriveDirectoryMatcher() method enables significant performance improvements
by allowing plugins to skip entire directory trees when they definitively
won't contain matching files. This preserves Maven 3's optimization behavior.

### Usage Example:

## Comprehensive Testing

### DefaultPathMatcherFactoryTest
- Tests all factory methods with various parameter combinations
- Verifies null parameter handling (NullPointerException)
- Tests directory matcher derivation functionality
- Includes edge cases and fail-safe behavior verification

### Backward Compatibility
- All existing PathSelector functionality preserved
- No breaking changes to existing APIs
- Enhanced error handling with better exception types

## Benefits

1. **Plugin Compatibility**: Enables maven-clean-plugin and other plugins
   to use exclude-only patterns efficiently
2. **Performance**: Directory filtering optimization preserves Maven 3 behavior
3. **Developer Experience**: Clean service interface with comprehensive JavaDoc
4. **Robustness**: Fail-fast null validation and defensive programming
5. **Future-Proof**: Extensible design for additional pattern matching needs

## Related Work

This implementation complements PR apache#10909 by @desruisseaux which addresses
PathSelector bug fixes. Both PRs can be merged independently and work
together to provide complete exclude-only functionality.

Addresses performance optimization suggestions and provides the missing
API methods needed by Maven plugins for efficient file filtering.

(cherry picked from commit 38e0a71)
gnodet added a commit that referenced this pull request Jul 18, 2025
…10923) (#10926)

This PR adds a comprehensive PathMatcherFactory service to Maven 4 API with
directory filtering optimization capabilities, addressing the need for
exclude-only pattern matching and performance optimizations.

## New API Features

### PathMatcherFactory Interface
- createPathMatcher(baseDirectory, includes, excludes, useDefaultExcludes)
- createPathMatcher(baseDirectory, includes, excludes) - convenience overload
- createExcludeOnlyMatcher(baseDirectory, excludes, useDefaultExcludes)
- createIncludeOnlyMatcher(baseDirectory, includes) - convenience method
- deriveDirectoryMatcher(fileMatcher) - directory filtering optimization

### DefaultPathMatcherFactory Implementation
- Full implementation of all PathMatcherFactory methods
- Delegates to PathSelector for actual pattern matching
- Provides directory optimization via PathSelector.couldHoldSelected()
- Fail-safe design returning INCLUDES_ALL for unknown matcher types

## PathSelector Enhancements

### Null Safety Improvements
- Added @nonnull annotation to constructor directory parameter
- Added Objects.requireNonNull() validation with descriptive error message
- Moved baseDirectory assignment to beginning for fail-fast behavior
- Updated JavaDoc to document NullPointerException behavior

### Directory Filtering Support
- Added canFilterDirectories() method to check optimization capability
- Made INCLUDES_ALL field package-private for factory reuse
- Enhanced couldHoldSelected() method accessibility

## Directory Filtering Optimization

The deriveDirectoryMatcher() method enables significant performance improvements
by allowing plugins to skip entire directory trees when they definitively
won't contain matching files. This preserves Maven 3's optimization behavior.

### Usage Example:

## Comprehensive Testing

### DefaultPathMatcherFactoryTest
- Tests all factory methods with various parameter combinations
- Verifies null parameter handling (NullPointerException)
- Tests directory matcher derivation functionality
- Includes edge cases and fail-safe behavior verification

### Backward Compatibility
- All existing PathSelector functionality preserved
- No breaking changes to existing APIs
- Enhanced error handling with better exception types

## Benefits

1. **Plugin Compatibility**: Enables maven-clean-plugin and other plugins
   to use exclude-only patterns efficiently
2. **Performance**: Directory filtering optimization preserves Maven 3 behavior
3. **Developer Experience**: Clean service interface with comprehensive JavaDoc
4. **Robustness**: Fail-fast null validation and defensive programming
5. **Future-Proof**: Extensible design for additional pattern matching needs

## Related Work

This implementation complements PR #10909 by @desruisseaux which addresses
PathSelector bug fixes. Both PRs can be merged independently and work
together to provide complete exclude-only functionality.

Addresses performance optimization suggestions and provides the missing
API methods needed by Maven plugins for efficient file filtering.

(cherry picked from commit 38e0a71)
@gnodet
Copy link
Contributor

gnodet commented Jul 18, 2025

I raised #10935 as I could not push to this branch to resolve the conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.0.x bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants