Add module-aware resource handling for modular sources#11505
Add module-aware resource handling for modular sources#11505desruisseaux merged 10 commits intoapache:masterfrom
Conversation
|
Hi @desruisseaux, I'd be happy to get some feedback from you (cf. my mail to the dev mailing list as of today), before making this an official PR. Thanks |
3e2d01f to
bef10a1
Compare
bef10a1 to
f747693
Compare
- Rename 'ctx' to 'context' in DefaultProjectBuilder - Use short form 'ModelProblem' instead of FQCN in test Addresses review comments from elharo on PR apache#11505. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use {@code <element>} instead of <element> escapes in javadoc
for better readability in source code (per Martin's suggestion).
Addresses review comment from desruisseaux on PR apache#11505.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Avoid abbreviations in new code per project conventions. Addresses review comment from elharo on PR apache#11505. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Avoid abbreviations in new code per project conventions. Addresses review comment from elharo on PR apache#11505. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
bdb4c3f to
ee7f1d3
Compare
|
Could someone please add the |
gnodet
left a comment
There was a problem hiding this comment.
I think we should reject any project that mixes <resource> and <source> elements, I don't see that as a valid use case.
I think this should even be done in |
Thanks for the feedback, @gnodet. Having said that, this PR is just about the first part of the problem (make Maven Core handle resources at all). Perhaps we can focus here on that feature? |
|
I would also suggest to reject any project that mixes |
Once the PR is merged (and a new version of Maven is published) we should be able to perform a simplified build wrt Maven resources.
3b2af3a to
d2ce972
Compare
- Rename 'ctx' to 'context' in DefaultProjectBuilder - Use short form 'ModelProblem' instead of FQCN in test Addresses review comments from elharo on PR apache#11505. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename hasOnlySuperPomDefaults to hasExplicitLegacyResources with inverted logic to avoid double negations in predicates - Remove redundant isEmpty() checks now handled by the method - Make extractModules method static Note: Extracting a shared method for main/test resource handling was not possible due to checkstyle ParameterNumber limit (max 7). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Refactor duplicate resource handling code into a shared method: - Add ResourceHandlingContext record to group shared parameters - Add handleResourceConfiguration method (4 parameters vs 9) - Remove unused outputDirectory param from createModularResourceRoot - Replace ~120 lines of duplicate code with 7 lines This addresses PR review comment apache#1 about code duplication between main and test resource handling blocks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Rename 'ctx' to 'context' in DefaultProjectBuilder - Use short form 'ModelProblem' instead of FQCN in test Addresses review comments from elharo on PR apache#11505. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use {@code <element>} instead of <element> escapes in javadoc
for better readability in source code (per Martin's suggestion).
Addresses review comment from desruisseaux on PR apache#11505.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Avoid abbreviations in new code per project conventions. Addresses review comment from elharo on PR apache#11505. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Change debug logging to trace level for reduced verbosity - Add periods to end of all log/warning messages - Use else blocks to avoid redundant logging after warnings - Make hasExplicitLegacyResources and createModularResourceRoot static - Remove low-level debug log from hasExplicitLegacyResources - Simplify loop logging by using context.modules() directly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move resource handling logic to dedicated package-private class: - Create ResourceHandlingContext with project-level state - Method handleResourceConfiguration(scope, hasResourcesInSources) derives resources from scope, takes flag as parameter - Reduces DefaultProjectBuilder by 179 lines 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Per PR apache#11505 review feedback from desruisseaux: the logs in handleResourceConfiguration are the result of more elaborated analysis and should remain at debug level, not trace. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
dcc621c to
dc90ada
Compare
|
Thanks! I will wait a little bit in case there is some comment, and if none I propose to merge this weekend. |
|
@desruisseaux Please assign appropriate label to PR according to the type of change. |
|
Thanks a lot for merging, @desruisseaux. |
|
Yes I thought about that. But as you mentioned on the slack channel, the work done in this pull request could be generalized. I thought that what this PR does for resources, the same thing could be done for tests. I'm thinking to refactor the |
Yes, I have a Phase 2 implementation in my mind which would be more general. Nevertheless, the feature would already be helpful (and I'd be eager to get it in the next Release Candidate for Maven 4.0).
Sounds good, perhaps we can join forces in the next weeks to get a combined implementation? |
|
Maybe you can go ahead with phase 2 when you have time. Looking on the wiki page, I suggest the following changes:
|
|
I have raised a new issue (#11612) for further discussion. |
This is a combination of pull requests apache#11505 and apache#11632 on master branch. Summary: Maven 4.x introduces a unified <sources> element that supports modular project layouts (src/<module>/<scope>/<lang>). However, resource handling did not follow the modular layout - resources were always loaded from the legacy <resources> element which defaults to src/main/resources. This change implements automatic module-aware resource injection. - For modular projects without resource configuration in <sources>, automatically inject resource roots following the modular layout: src/<module>/main/resources and src/<module>/test/resources - Resources configured via <sources> take priority over legacy <resources> - Issue warnings (as ModelProblem) when explicit legacy resources are ignored
This is a combination of pull requests apache#11505, apache#11632 and apache#11702 on master branch. Summary: Maven 4.x introduces a unified <sources> element that supports modular project layouts (src/<module>/<scope>/<lang>). However, resource handling did not follow the modular layout - resources were always loaded from the legacy <resources> element which defaults to src/main/resources. This change implements automatic module-aware resource injection. - For modular projects without resource configuration in <sources>, automatically inject resource roots following the modular layout: src/<module>/main/resources and src/<module>/test/resources. - Resources configured via <sources> take priority over legacy <resources>. - Fail the build (as ModelProblem) when explicit legacy resources are ignored.
This is a combination of pull requests #11505, #11632 and #11702 on master branch. Summary: Maven 4.x introduces a unified <sources> element that supports modular project layouts (src/<module>/<scope>/<lang>). However, resource handling did not follow the modular layout - resources were always loaded from the legacy <resources> element which defaults to src/main/resources. This change implements automatic module-aware resource injection. - For modular projects without resource configuration in <sources>, automatically inject resource roots following the modular layout: src/<module>/main/resources and src/<module>/test/resources. - Resources configured via <sources> take priority over legacy <resources>. - Fail the build (as ModelProblem) when explicit legacy resources are ignored. Co-authored-by: Gerd Aschemann <github@aschemann.net>
Summary
<sources>layoutsrc/<module>/main/resourcesandsrc/<module>/test/resources<resources>configuration is ignored in favor of modular defaultsProblem
Projects using Maven 4.x modular sources (
<source><module>org.foo.bar</module></source>) had to manually configuremaven-resources-pluginfor each module. Resources from the modular layout paths were not automatically discovered.Solution
When a project defines at least one module in
<sources>:<resources>from Super POM defaults are silently replaced<resources>configuration triggers aModelProblemwarningTest plan
testModularSourcesInjectResourceRoots- verifies resource injection for multi-module projectstestModularSourcesWithExplicitResourcesIssuesWarning- verifies warnings are issued when legacy resources are ignored