fix: generate correct module-info requires for JavaFX //MODULE scripts#2525
fix: generate correct module-info requires for JavaFX //MODULE scripts#2525koppor wants to merge 3 commits into
Conversation
A `//MODULE` script depending on JavaFX failed to compile with "package javafx.collections is not visible". JavaFX publishes an empty placeholder jar without a classifier (`javafx-base.jar`, module `javafx.baseEmpty`) next to the platform jar that carries the real `javafx.base` module (`javafx-base-<os>.jar`). generateModuleInfo matched resolved artifacts to root dependencies by the full management key, which includes the classifier, so it only matched the empty placeholder and emitted `requires javafx.baseEmpty;` - leaving `javafx.collections` invisible. Match root dependencies ignoring the classifier so the platform artifact that holds the real module is considered too, and skip the `*Empty` placeholder modules so the real module (e.g. `javafx.base`) is required. Add a //MODULE + JavaFX regression test (pinned to JavaFX 11.0.2 so the module descriptor is readable by the Java 11 test toolchain). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
quintesse
left a comment
There was a problem hiding this comment.
In general LGTM except for that one remark.
Addresses review feedback: matching root dependencies while ignoring the classifier is enough to require the real module, so there is no need to special-case JavaFX's empty placeholder by its `*Empty` name. The empty placeholder module simply ends up required next to the real one, which is harmless and verified to compile and run. This avoids relying on a naming convention that other libraries may not follow and that JavaFX could change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to #2520
Problem
A
//MODULEscript that depends on JavaFX fails to compile:Root cause
JavaFX publishes two artifacts per module:
javafx-base.jar(no classifier, empty placeholder)javafx.baseEmptyjavafx-base-<os>.jar(platform classifier, real classes)javafx.baseModuleUtil.generateModuleInfomatched resolved artifacts to root dependencies by the full management key, which includes the classifier (getManagementKey()→groupId:artifactId:type[:classifier]). So the root deporg.openjfx:javafx-base(key…:jar) matched only the unclassified empty placeholder and emitted:leaving
javafx.collections(which lives in the realjavafx.base) invisible to the generated module.Fix
In
generateModuleInfo:*Emptyplaceholder modules, so the real module (javafx.base) is required.This only changes which module names land in the generated
module-info.java; the launch path (-p <classpath>+-m) is untouched.Verification
//MODULE+ JavaFX script above now compiles and runs.//MODULEscripts (e.g. picocli) are unaffected.TestModule.testModuleJavaFXasserts the generatedmodule-inforequiresjavafx.baseand notjavafx.baseEmpty, and actually compiles the script.TestModuleandDependencyResolverTestsuites pass; spotless clean.