You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2025/11/05 15:15:28 UTC
[PR] [GH-11381] Fix resource targetPath resolution to be relative to output directory [maven]
gnodet opened a new pull request, #11394:
URL: https://github.com/apache/maven/pull/11394
This PR fixes a regression in Maven 4 where resource `targetPath` was being resolved relative to the project base directory instead of the output directory (`target/classes` or `target/test-classes`).
## Problem
In Maven 3.x, when a resource has a relative `targetPath`, it's resolved relative to the output directory. For example:
```xml
<resource>
<directory>${project.basedir}/rest</directory>
<targetPath>target-dir</targetPath>
<includes>
<include>**/*.yml</include>
</includes>
</resource>
```
In Maven 3.x, files would be copied to `target/classes/target-dir/`, but in Maven 4, they were being copied to `target-dir/` (relative to project root).
## Root Cause
The bug was in the `DefaultSourceRoot` constructor that takes a `Resource` parameter. On line 172, the targetPath was being resolved as:
```java
nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null)
```
This resolves the targetPath relative to the base directory, not the output directory.
## Solution
1. **Created a new constructor** in `DefaultSourceRoot.java` that accepts an `outputDir` parameter and correctly resolves targetPath relative to the output directory
2. **Deprecated the old constructor** to maintain backward compatibility
3. **Updated all call sites** to use the new constructor:
- `DefaultProjectBuilder.java`
- `ConnectedResource.java`
- `MavenProject.java`
4. **Updated unit tests** in `DefaultSourceRootTest.java` to verify the correct behavior
5. **Created an integration test** `MavenITgh11381ResourceTargetPathTest.java` to verify the fix works end-to-end
## Testing
The integration test verifies that:
- Resources with relative `targetPath` are copied to the correct location under the output directory
- The fix works for both main and test scopes
Test results:
```
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
```
Fixes #11381
---
Pull Request opened by [Augment Code](https://www.augmentcode.com/) with guidance from the PR author
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2499540230
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
And maybe rename the `baseDir` parameter as `outputDir` (but without change in the call to `Path.resolve(String)`).
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2500339155
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
> `Path.of(String)` creates a path relative the to current working directory, which is not the desired output directory. I think that `DefaultSourceRoot` is okay, and the change rather needs to be applied in the codes that call this constructor. The problem seems to be in the following classes:
>
> * `org.apache.maven.project.ConnectedResource` line 124.
> * `org.apache.maven.project.DefaultProjectBuilder` lines 704 and 707.
> * `org.apache.maven.project.MavenProject` line 840
>
> The above-cited codes set the first parameter to `project.getBaseDirectory()` while it should be `project.getBuild().getOutputDirectory()`.
>
> Note: the `baseDir` parameter name of `DefaultSourceRoot` is misleading. The Javadoc said _"the base directory for resolving relative paths"_, which can be the `baseDir` of the Maven project but not necessarily. It should be understood as any base directory for resolving paths. This parameter could be renamed `parentDir` if `baseDir` is too misleading.
`Path.of(String)` just creates a relative path. But not relative to anything. It's only when resolved against a directory that it will become relative to that directory.
The built `Path` will be resolved against the correct directory, that's what happens in the maven-resource-plugin for example, which will resolve this path against either `project.build.outputDirectory` or `project.build.testOutputDirectory`, depending which mojo is called.
I agree that there may be a mismatch, but it sounds difficult to just thrown away the m-resources-p at the moment.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2500353222
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
Fwiw, the expectation in Maven 3 is that `Resource.getTargetPath()` will be resolved against either `project.build.outputDirectory` or `project.build.testOutputDirectory`. The main usage *is* the maven-resource-plugin here, which is currently broken.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2499531199
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
It may be absolute, but I'm not sure that this guaranteed. It may depend on which code path as leaded to this constructor. I don't think that there is any reason to not be robust. We just need to find all calls to this constructor.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2499401868
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
`Path.of(String)` creates a path relative the to current working directory, which is not the desired output directory. I think that `DefaultSourceRoot` is okay, and the change rather needs to be applied in the codes that call this constructor. The problem seems to be in the following classes:
* `org.apache.maven.project.ConnectedResource` line 124.
* `org.apache.maven.project.DefaultProjectBuilder` lines 704 and 707.
* `org.apache.maven.project.MavenProject` line 840
The above-cited codes set the first parameter to `project.getBaseDirectory()` while it should be `project.getBuild().getOutputDirectory()`.
Note: the `baseDir` parameter name of `DefaultSourceRoot` is misleading. The Javadoc said _"the base directory for resolving relative paths"_, which can be the `baseDir` of the Maven project but not necessarily. It should be understood as any base directory for resolving paths. This parameter could be renamed `parentDir` if `baseDir` is too misleading.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2501112237
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
> So the behavior would be different between mvn3 and mvn4 api
If the difference would be that mvn3 would be relative to `target` while mvn4 would be relative to `baseDir`, I think that this difference was a bug partially fixed in #11322, but we forgot the case of resources in that fix.
I will prepare an alternative pull request tomorrow for illustrating the fix that I propose. The actual pull request at the end may be, possibly, a mix of the two.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #11394:
URL: https://github.com/apache/maven/pull/11394#issuecomment-3502348741
## Commit Description for Merge
Here's a comprehensive commit message explaining the chosen fix:
---
```
Fix resource targetPath resolution to maintain Maven 3 compatibility
This commit fixes a regression in Maven 4 where resource targetPath was being
resolved relative to the project base directory instead of the output directory,
breaking compatibility with Maven 3 and causing resources to be copied to
incorrect locations.
## Problem
In Maven 3.x, when a resource has a relative targetPath, it is resolved relative
to the output directory (target/classes for main, target/test-classes for test).
Example configuration:
```xml
<resource>
<directory>${project.basedir}/rest</directory>
<targetPath>target-dir</targetPath>
</resource>
```
Maven 3 behavior: Files copied to target/classes/target-dir ✓
Maven 4 behavior (before fix): Files copied to target-dir (project root) ✗
## Root Cause
The DefaultSourceRoot constructor that creates SourceRoot instances from Resource
model objects was incorrectly resolving the targetPath against the project base
directory instead of storing it as-is for later resolution against the output
directory.
## Solution Design
The fix implements a clear separation of concerns between storage and resolution:
### 1. Storage Layer (SourceRoot.targetPath())
This method returns the targetPath exactly as specified in the configuration:
- Relative paths (e.g., "META-INF/resources") are stored as relative paths
- Absolute paths (e.g., "/tmp/custom") are stored as absolute paths
- Empty/null means no explicit target path was specified
No resolution happens at this layer. The path is stored as-is.
### 2. Resolution Layer (SourceRoot.targetPath(Project))
This method performs the actual path resolution:
Algorithm:
1. Get the configured targetPath from targetPath()
2. If absolute → return it unchanged
3. Get the output directory for the source root's scope:
- ProjectScope.MAIN → target/classes
- ProjectScope.TEST → target/test-classes
4. If targetPath is empty → return output directory
5. If targetPath is relative → resolve against output directory
This matches Maven 3 behavior exactly.
### 3. Implementation Changes
**DefaultSourceRoot.java:**
- Constructor from Resource now stores targetPath as-is (line 176):
```java
nonBlank(resource.getTargetPath()).map(Path::of).orElse(null)
```
- Does NOT resolve against baseDir (which would make it project-relative)
- Added comprehensive javadoc explaining this preserves Maven 3 behavior
**ConnectedResource.java:**
- Extracts targetPath as a string (still relative) for maven-resources-plugin
- The plugin receives the relative path and resolves it (Maven 3 behavior)
**Project.getOutputDirectory():**
- Returns the scope-specific output directory
- Used by SourceRoot.targetPath(Project) for resolution
- Documented as the base for targetPath resolution
### 4. Maven 4 API Documentation
Extensive javadoc enhancements make the design explicit:
**SourceRoot.targetPath():**
- Clearly states it returns path AS CONFIGURED (no resolution)
- Documents return value semantics for empty/relative/absolute cases
- Provides usage guidance for different audiences:
* Maven 4 API consumers: use targetPath(Project)
* Maven 3 compatibility: use this method
* Implementers: store as-is, don't resolve
**SourceRoot.targetPath(Project):**
- Documents complete resolution algorithm
- Provides table with concrete examples
- Explains relationship to targetPath() (storage vs resolution)
**Project.getOutputDirectory():**
- Documents scope-to-directory mapping
- Explains role in targetPath resolution
- Shows Maven 3 compatibility with XML examples
## Why This Approach
This design was chosen because:
1. **Preserves Maven 3 Compatibility:**
- maven-resources-plugin receives relative paths (as in Maven 3)
- Plugin resolves against output directory (as in Maven 3)
- Existing POMs work without modification
2. **Clean Maven 4 API:**
- Clear separation: storage (targetPath) vs resolution (targetPath(Project))
- Maven 4 consumers get absolute paths via targetPath(Project)
- Legacy plugins get relative paths via targetPath()
3. **No Premature Resolution:**
- Paths stored as configured (relative or absolute)
- Resolution happens when needed, with proper context (Project)
- Allows different resolution strategies for different scopes
4. **Explicit Contracts:**
- Comprehensive javadoc explains semantics
- Clear guidance for API consumers, plugin developers, implementers
- Examples show expected behavior
## Testing
Integration test MavenITgh11381ResourceTargetPathTest verifies:
- Resources with relative targetPath are copied to correct location
- Works for both main and test scopes
- Maintains Maven 3 compatibility
## Impact
This fix:
✓ Restores Maven 3 compatibility for resource targetPath
✓ Provides clean Maven 4 API with clear semantics
✓ Maintains backward compatibility with existing POMs
✓ Enables maven-resources-plugin to work correctly
✓ Documents the design for future maintainers
Fixes #11381
```
---
Feel free to use this as-is or adapt it for the merge commit!
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #11394:
URL: https://github.com/apache/maven/pull/11394#issuecomment-3502475735
## 💚 All backports created successfully
| Status | Branch | Result |
|:------:|:------:|:------|
|✅|maven-4.0.x|[<img src="https://img.shields.io/github/pulls/detail/state/apache/maven/11406">](https://github.com/apache/maven/pull/11406)|
### Questions ?
Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)
<!--- Backport version: 10.0.1 -->
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2501101236
##########
impl/maven-core/src/main/java/org/apache/maven/project/ConnectedResource.java:
##########
@@ -42,13 +42,53 @@ class ConnectedResource extends Resource {
.includes(sourceRoot.includes())
.excludes(sourceRoot.excludes())
.filtering(Boolean.toString(sourceRoot.stringFiltering()))
- .targetPath(sourceRoot.targetPath().map(Path::toString).orElse(null))
+ .targetPath(computeRelativeTargetPath(sourceRoot, scope, project))
.build());
this.originalSourceRoot = sourceRoot;
this.scope = scope;
this.project = project;
}
+ /**
+ * Computes the targetPath relative to the output directory.
+ * In Maven 3 API, Resource.getTargetPath() is expected to be relative to the output directory
+ * (e.g., "custom-output"), while SourceRoot.targetPath() is relative to the project basedir
Review Comment:
> while SourceRoot.targetPath() is relative to the project basedir
Actually it was a bug. The specification that we wrote in `maven.mdo` said that `targetPath` shall be relative to the target directory. Fixing that bug was the subject of #11322, but that correction was incomplete. We could said that this pull request #11394 completes #11322.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #11394:
URL: https://github.com/apache/maven/pull/11394#issuecomment-3495347893
This does not look correct to me, I need to have another look.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #11394:
URL: https://github.com/apache/maven/pull/11394#issuecomment-3502316697
## Latest Update: Enhanced Javadoc for API Clarity
I've just pushed an update that significantly improves the documentation to clarify the Maven 4 API semantics while making the Maven 3 backward compatibility guarantees explicit.
### What Changed
Enhanced javadoc for three key methods:
1. **`SourceRoot.targetPath()`** - Now clearly documents:
- Relative paths are resolved relative to the output directory (not project basedir)
- Absolute paths are used as-is
- Empty/null means files go to output directory root
- Explicitly states Maven 3 compatibility is maintained
2. **`SourceRoot.targetPath(Project)`** - Now includes:
- Step-by-step resolution algorithm
- Concrete examples with actual paths
- Cross-references to related methods
3. **`Project.getOutputDirectory(ProjectScope)`** - Now clarifies:
- Returns directory for both compiled classes AND resources
- Its role in SourceRoot targetPath resolution
- Maven 3 compatibility semantics
4. **`DefaultSourceRoot` constructor from Resource** - Now documents:
- targetPath is stored as-is (not resolved against basedir)
- baseDir parameter is only used for source directory resolution
- How this preserves Maven 3.x behavior
### Why This Matters
The enhanced documentation makes it crystal clear that:
- **For Maven 4 API users**: Use `targetPath(Project)` to get the fully resolved absolute path
- **For Maven 3 compatibility**: The `targetPath()` method returns the relative path that plugins expect
- **For implementers**: The separation of concerns between storage and resolution is now explicit
### Implementation Verification
The documentation now matches the actual implementation:
✅ `DefaultSourceRoot` stores `targetPath` as-is (relative or absolute)
✅ `ConnectedResource` extracts `targetPath` as a relative path string
✅ maven-resources-plugin receives the relative path and resolves it (Maven 3 behavior)
✅ Maven 4 API consumers can use `targetPath(Project)` for absolute paths
See the detailed explanation in [this comment on #11381](https://github.com/apache/maven/issues/11381#issuecomment-3502314999) for the complete design rationale.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2499457296
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
Is maybe `resource.getTargetPath()` absolute? So CWD does not matter?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2500556228
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
Why do you think it does not ?
It makes the path truly relative, and later resolved against the correct directory.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #11394:
URL: https://github.com/apache/maven/pull/11394#issuecomment-3502344456
## Further Javadoc Refinements for Maximum Clarity
I've pushed another update that significantly refines the Maven 4 API javadoc to make it even more explicit and clear. The documentation now leaves no ambiguity about how `targetPath` handling works.
### Key Enhancements
#### 1. **SourceRoot.targetPath()** - The Storage Method
**New sections added:**
- **"Important" callout**: Explicitly states this returns the path AS CONFIGURED (no resolution)
- **"Return Value Semantics"**: Detailed explanation of what each return value means:
- Empty Optional → files go to output directory root
- Relative Path → intended to be resolved against output directory (resolution happens in `targetPath(Project)`)
- Absolute Path → used as-is
- **"Usage Guidance"**: Clear instructions for three audiences:
- Maven 4 API consumers: use `targetPath(Project)` instead
- Maven 3 compatibility layer: use this method for legacy plugins
- Implementers: store path as-is, don't resolve at storage time
**Why this matters**: Makes it crystal clear that this method is for STORAGE, not RESOLUTION.
#### 2. **SourceRoot.targetPath(Project)** - The Resolution Method
**New sections added:**
- **"Purpose"**: Explicitly states this is THE method for path resolution
- **Detailed algorithm**: Step-by-step explanation with all edge cases
- **Comprehensive table**: Shows concrete examples with:
- Configuration input (what `targetPath()` returns)
- Output directory (from `getOutputDirectory()`)
- Final result (what this method returns)
- Explanation of each case
- **"Relationship to targetPath()"**: Explains the storage vs resolution separation
- **"Implementation Note"**: Shows equivalent code for clarity
**Example from the table:**
| Configuration | Output Directory | Result | Explanation |
|--------------|------------------|--------|-------------|
| `Optional.empty()` | `/home/user/myproject/target/classes` | `/home/user/myproject/target/classes` | No explicit path → use output directory |
| `Optional.of(Path.of("META-INF"))` | `/home/user/myproject/target/classes` | `/home/user/myproject/target/classes/META-INF` | Relative path → resolve against output directory |
| `Optional.of(Path.of("/tmp/custom"))` | `/home/user/myproject/target/classes` | `/tmp/custom` | Absolute path → use as-is |
**Why this matters**: Developers can see exactly how each case is handled with concrete examples.
#### 3. **Project.getOutputDirectory()** - The Base Directory Provider
**New sections added:**
- **"Purpose"**: Explains this provides the base for path resolution
- **Comprehensive table**: Maps scope to directory with build configuration and contents
- **"Role in SourceRoot Path Resolution"**: Shows concrete examples:
- Main resources with `targetPath="META-INF"` → `target/classes/META-INF`
- Test resources with `targetPath="test-data"` → `target/test-classes/test-data`
- **Expanded Maven 3 compatibility**: Includes actual XML configuration example showing how maven-resources-plugin uses this
- **Code examples**: Demonstrates usage for all three scopes
**Why this matters**: Makes the connection between this method and `SourceRoot.targetPath(Project)` explicit.
### HTML5 Compliance
Fixed javadoc generation errors by replacing deprecated HTML attributes:
- ❌ `<table border="1" cellpadding="5" cellspacing="0">`
- ✅ `<table class="striped">`
Javadoc now builds cleanly without errors.
### Documentation Philosophy
The refined documentation follows a clear pattern:
1. **Separation of Concerns**:
- `targetPath()` = STORAGE (returns as configured)
- `targetPath(Project)` = RESOLUTION (returns absolute path)
- `getOutputDirectory()` = BASE PROVIDER (provides resolution base)
2. **Audience-Specific Guidance**:
- Maven 4 API consumers know to use `targetPath(Project)`
- Maven 3 compatibility layer knows to use `targetPath()`
- Implementers know to store without resolving
3. **Concrete Examples**:
- Tables with actual paths
- XML configuration snippets
- Code examples
4. **Explicit Contracts**:
- What each method returns
- When to use which method
- How methods relate to each other
### Verification
✅ Javadoc builds successfully without errors
✅ All three methods have coherent, cross-referenced documentation
✅ Maven 3 compatibility guarantees are explicit
✅ Maven 4 API semantics are clear
The documentation now provides complete clarity on the targetPath handling design, making it easy for developers to understand and use the API correctly.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2499290159
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
This is CWD relative?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2500583160
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
> It makes the path truly relative, and later resolved against the correct directory.
Ah, I see. The responsibility of resolving the path is shifted to the `SourceRoot` user instead of the `SourceRoot` constructor. But why not resolving at construction time? It would probably be less code to check for correctness.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2500435556
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
> Fwiw, the expectation in Maven 3 is that `Resource.getTargetPath()` will be resolved against either `project.build.outputDirectory` or `project.build.testOutputDirectory`.
Yes I agree, but the replacement of `baseDir::resolve` by `Path::of` does not fix that. The correction is rather to fix the `baseDir` argument given in *calls* to the `DefaultSourceRoot` constructor. Actually, the [first commit of this pull request](https://github.com/apache/maven/pull/11394/commits/eb23cfcee98d3a4d9136ccc69f115d3d51f70607) seems correct to me, but it seems to have been reverted in subsequent commits.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11394:
URL: https://github.com/apache/maven/pull/11394#discussion_r2500878311
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java:
##########
@@ -169,7 +169,7 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
resource.getIncludes(),
resource.getExcludes(),
Boolean.parseBoolean(resource.getFiltering()),
- nonBlank(resource.getTargetPath()).map(baseDir::resolve).orElse(null),
+ nonBlank(resource.getTargetPath()).map(Path::of).orElse(null),
Review Comment:
I agree. I think that would be desired behavior, but then we need to make it truly relative in the ConnectedResource to not break the existing plugin.
So the behavior would be different between mvn3 and mvn4 api, but that's fine.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #11394:
URL: https://github.com/apache/maven/pull/11394#issuecomment-3499840550
## Implementation Details
I've implemented a fix for this issue that ensures `SourceRoot.targetPath()` and `ConnectedResource` behave correctly according to the Maven 4 API while preserving Maven 3 compatibility.
### Root Cause
The regression occurred because `DefaultSourceRoot.fromModel()` was storing `targetPath` as an **absolute path** (resolved against `baseDir` and `outputDir`), when it should have been stored as a **relative path** according to the Maven 4 API design.
### Maven 4 API Design
The Maven 4 API defines two methods for `SourceRoot`:
1. **`targetPath()`** - Returns an `Optional<Path>` containing the explicit target path. According to the javadoc and the presence of the resolution method below, this should be a **relative path** (or absolute if explicitly specified as absolute).
2. **`targetPath(Project)`** - Returns the resolved absolute path by taking the relative `targetPath()` and resolving it against the project's output directory:
```java
Path base = project.getOutputDirectory(scope());
return targetPath.map(base::resolve).orElse(base);
```
This two-method design clearly indicates that `targetPath()` should return a **relative** path that gets resolved by `targetPath(Project)`.
### Implementation
**1. Fixed `DefaultSourceRoot.fromModel()`** (impl/maven-impl)
```java
// Before (incorrect):
nonBlank(source.getTargetPath())
.map((targetPath) -> baseDir.resolve(outputDir.apply(scope)).resolve(targetPath))
.orElse(null)
// After (correct):
nonBlank(source.getTargetPath()).map(Path::of).orElse(null)
```
Now `targetPath()` returns a **relative path** as intended, and `targetPath(Project)` handles the resolution.
**2. Simplified `ConnectedResource.computeRelativeTargetPath()`** (impl/maven-core)
Since `SourceRoot.targetPath()` now always returns a relative path, we can simply return it as-is:
```java
return sourceRoot.targetPath().map(Path::toString).orElse(null);
```
This removed ~40 lines of complex logic that was trying to convert absolute paths back to relative paths.
### Maven 3 Compatibility
In Maven 3, `Resource.getTargetPath()` was always **relative to the output directory**. This behavior is preserved:
- `SourceRoot.targetPath()` stores the path as relative (e.g., `"custom-dir"`)
- `ConnectedResource` converts it back to a String for the Maven 3 `Resource` API
- Resources are copied to `target/classes/custom-dir` (Maven 3 behavior) ✅
### Example
With `<targetPath>custom-dir</targetPath>` in a resource configuration:
- **Maven 3**: Resources copied to `target/classes/custom-dir` ✅
- **Maven 4 (before fix)**: Resources copied to `project-root/custom-dir` ❌
- **Maven 4 (after fix)**: Resources copied to `target/classes/custom-dir` ✅
### Testing
All existing tests pass, including:
- `DefaultSourceRootTest` - Updated to expect relative paths from `targetPath()`
- `ResourceIncludeTest` - Validates the Maven 3 compatibility layer
- Integration test `MavenITgh11381ResourceTargetPathTest` should now pass
The fix is minimal, focused, and aligns with the Maven 4 API design while maintaining backward compatibility.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix resource targetPath resolution to be relative to output directory (fixes #11381) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #11394:
URL: https://github.com/apache/maven/pull/11394
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org