Fix package {...} does not exist in legacyMode by JackPGreen · Pull Request #1243 · apache/maven-javadoc-plugin (original) (raw)
Partially reverts change in #1217, specifically:
Do not use --source-path in legacy mode as not to suck any of those module-info.java files back
This is required to ensure correct package resolution on projects that mix modular & non-module Java code.
Fixes: #1242
- I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
Partially reverts change in apache#1217, specifically:
Do not use --source-path in legacy mode as not to suck any of those module-info.java files back
This is required to ensure correct package resolution on projects that mix modular & non-module Java code.
Note for reviewers:
- I've not analysed in detail what why this issue occurred
There is no test coverage for this case- I've tested the fix both against the tests (e.g.
-Prun-its verify) and the original reproducer
Ideally, the above should be addressed.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test really is mandatory for a bug like this
test really is mandatory for a bug like this
I understand the value but adding one is non-trivial - would need to slim down the reproducer to suit, I don't think any existing test covers this scenario (specifically - the legacy + multi-module case I saw has no imports). I'll try to revisit.
Also - the fact that this fix doesn't affect any existing tests but does fix my issues suggests that (in hindsight) there was insufficient test coverage for the previous PR.
test really is mandatory for a bug like this
@elharo I've added a test in e1a7473, based on a slimmed-down version of the reproducer.
This test fails on master:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.11.4-SNAPSHOT:aggregate (default-cli) on project GITHUB-1242: An error has occurred in Javadoc report generation:
But passes on my branch:
[INFO] Building: GITHUB-1242/pom.xml
[INFO] GITHUB-1242/pom.xml .............................. SUCCESS (1.861 s)
@olamy thanks for re-running the test, I've updated the reproducer to (hopefully) handle the failing legacy Maven cases.
olamy left a comment • Loading
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have been tested this on a large project such https://github.com/jetty/jetty.project with mvn clean install -Pjavadoc-aggregate javadoc:aggregate -DskipTests
and all good
Thanks for your contribution
Something isn't working
label
@olamy Please assign appropriate label to PR according to the type of change.
The problem of the sourcepath is that it sucks in the module-info.java files in aggregate goal. I am getting this for instance:[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.11.3:aggregate (default-cli) on project angus-activation-project: An error has occurred in Javadoc report generation: [ERROR] Exit code: 1 [ERROR] /home/abuild/rpmbuild/BUILD/angus-activation-2.0.2-build/angus-activation-2.0.2/activation-registry/src/main/java/module-info.java:17: error: module not found: jakarta.activation [ERROR] requires transitive jakarta.activation; [ERROR] ^ [ERROR] error: cannot access module-info [ERROR] cannot resolve modules [ERROR] 2 errors [ERROR] Command line was: /usr/lib64/jvm/java-21-openjdk-21/bin/javadoc -J-Duser.language= -J-Duser.country= @options @argfile
I don't know how to do that differently. Excluding the module-info.java files from the source list was easy. Since the javadoc tool only can exclude packages or -- in the standard doclet -- the subdirectories, there is no easy way to prevent the module-info.java being sucked in. I am just wondering whether there was no other way to fix your bug then by reverting this one.
But what I tried also to avoid in the original commit was not to refer to the sources by package. Maybe I missed a place. If one does not use the package names in legacy mode, one does not need the sourcepath
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})