gnodet commented on code in PR #268:
URL:
https://github.com/apache/maven-source-plugin/pull/268#discussion_r3716224451
##########
src/main/java/org/apache/maven/plugins/source/SourceJarNoForkMojo.java:
##########
@@ -58,7 +61,7 @@ protected List<Resource> getResources(Project p) {
return Collections.emptyList();
}
- return projectManager.getResources(p, ProjectScope.MAIN);
+ return p.getBuild().getResources();
Review Comment:
⚠️ **Deprecated model API** — `p.getBuild().getResources()` is the Maven 3
model shim, deprecated at rc-6. It returns only what's declared in `pom.xml`
and cannot see resource roots registered at runtime (e.g., by
`maven-remote-resources-plugin`).
The recommended replacement is:
```suggestion
return projectManager
.getEnabledSourceRoots(p, ProjectScope.MAIN,
Language.RESOURCES)
.toList();
```
This also requires changing the return type of the abstract `getResources()`
method in `AbstractSourceJarMojo` from `List<Resource>` to `List<SourceRoot>`,
and updating `archiveProjectContent()` to iterate `SourceRoot` objects. See
[#315's diff](https://github.com/apache/maven-source-plugin/pull/315/files) for
the full migration.
_(Confirms @ascheman's finding)_
##########
src/main/java/org/apache/maven/plugins/source/SourceJarNoForkMojo.java:
##########
@@ -47,7 +47,10 @@ public class SourceJarNoForkMojo extends
AbstractSourceJarMojo {
* {@inheritDoc}
*/
protected List<Path> getSources(Project p) {
- return projectManager.getCompileSourceRoots(p, ProjectScope.MAIN);
+ return projectManager
Review Comment:
💡 **Suggestion:** `p.getLanguage()` returns the project's exact language
(e.g., `Language.JAVA`), while `Language.JAVA_FAMILY` is a broader match that
includes Java and closely-related JVM languages. `maven-compiler-plugin` uses
`Language.JAVA_FAMILY`, and #315 did the same. Consider aligning:
```suggestion
.getEnabledSourceRoots(p, ProjectScope.MAIN,
Language.JAVA_FAMILY)
```
For standard Java projects both produce the same result, but `JAVA_FAMILY`
would also cover source roots from JVM languages like Groovy or Kotlin in
mixed-language builds.
##########
src/test/java/org/apache/maven/plugins/source/SourceJarMojoTest.java:
##########
@@ -31,7 +32,8 @@
import org.apache.maven.api.plugin.testing.MojoTest;
Review Comment:
⚠️ **Silent-failure risk** — These `org.apache.maven.api.plugin.testing.*`
imports compile via deprecated shims, but on rc-6 `MojoExtension` only honours
the relocated `org.apache.maven.testing.plugin.*` annotations. `@Basedir`,
`@MojoParameter`, and `@InjectMojo` can be silently ignored, causing the mojo
to run with uninitialized fields.
This is exactly
[apache/maven#12678](https://github.com/apache/maven/issues/12678), which
silently broke `maven-jar-plugin`'s tests the same way.
All test harness imports in this file (and `TestSourceJarMojoTest`,
`AbstractSourcePluginTestCase`) should be relocated:
```
org.apache.maven.api.plugin.testing.* → org.apache.maven.testing.plugin.*
```
_(Confirms @ascheman's finding)_
--
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]
