-
Notifications
You must be signed in to change notification settings - Fork 102
Fix package {...} does not exist
in legacyMode
#1243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
_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:
Ideally, the above should be addressed. |
There was a problem hiding this 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
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 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. |
@elharo I've added a test in e1a7473, based on a slimmed-down version of the reproducer. This test fails on
But passes on my branch:
|
There was a problem hiding this 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 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: |
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 |
Partially reverts change in #1217, specifically:
This is required to ensure correct package resolution on projects that mix modular & non-module Java code.
Fixes: #1242