Skip to content

Conversation

JackPGreen
Copy link
Contributor

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

_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.
@JackPGreen
Copy link
Contributor Author

JackPGreen commented Aug 18, 2025

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.

Copy link
Contributor

@elharo elharo left a comment

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

@JackPGreen
Copy link
Contributor Author

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.

@JackPGreen
Copy link
Contributor Author

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)

@JackPGreen JackPGreen requested a review from elharo August 25, 2025 22:49
@JackPGreen
Copy link
Contributor Author

@olamy thanks for re-running the test, I've updated the reproducer to (hopefully) handle the failing legacy Maven cases.

292736e

Copy link
Member

@olamy olamy left a comment

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

@olamy olamy merged commit f310135 into apache:master Sep 4, 2025
100 checks passed
@olamy olamy added the bug Something isn't working label Sep 4, 2025
@github-actions github-actions bot added this to the 3.12.0 milestone Sep 4, 2025
Copy link

github-actions bot commented Sep 4, 2025

@olamy Please assign appropriate label to PR according to the type of change.

@fridrich
Copy link
Contributor

fridrich commented Sep 9, 2025

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

@fridrich
Copy link
Contributor

fridrich commented Sep 9, 2025

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.

@fridrich
Copy link
Contributor

fridrich commented Sep 9, 2025

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

@fridrich fridrich mentioned this pull request Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

package {...} does not exist when upgrading 3.11.2 -> 3.11.3 + legacyMode

4 participants