Closed samypr100 closed 1 year ago
Note: I am not sure if all corner-cases of the migration are covered. I tried a rather simple non-modular application to update and it fails with
Execution failed for task ':xjcGenerate'.
> Could not resolve all files for configuration ':xjcCatalogResolution'.
> Could not resolve org.openjfx:javafx-controls:17.
Required by:
project :
> Cannot choose between the following variants of org.openjfx:javafx-controls:17:
- linux-aarch64Runtime
- linuxRuntime
- mac-aarch64Runtime
- macRuntime
- runtime
- winRuntime
The project can be found here https://github.com/danielpeintner/Java11Test/tree/non-modular and the commit that makes it fail is https://github.com/danielpeintner/Java11Test/commit/2a38e31b352d07d98b79b1fba45e98e36b8fd4e1 where I essentially just upgrade to 'org.openjfx.javafxplugin' version '0.1.0' and modularity.inferModulePath.set(false)
The JavaFX dependencies are/were already excluded before
The failing task is xjcGenerate
which is responsible to create Java classes out of XML schema. I don't really understand the reason since it does not really have to do with JavaFX at all !?
@danielpeintner Luckily I don't think you'd need to add modularity.inferModulePath
at all since its not a modular project. That being said, definitely something going on with org.unbroken-dome.xjc
likely causing this. Might take a deeper look later unless @jjohannes has faster insight on what's happening.
This is a typical problem once a library does more with variants. Similar issues came up recently when Guava started to publish Gradle Metadata (https://github.com/google/guava/issues/6612).
When you use JavaFX now with the new setup, Gradle needs to know which "Platform" variant to choose. The JavaFX plugin makes this easy for the standard cases by configuring all configurations.*Classspath
for this. But if a build or a plugin creates its own "classpath" this might not be automatically configured. In this case the "classpath" is the configuration.xjcCatalogResolution
, created by one of the other plugins.
@danielpeintner for explanation, you can now explicitly set which variation of JavaFX you want to have for that classpath, by doing something like this:
configurations.xjcCatalogResolution {
attributes {
attribute(Usage.USAGE_ATTRIBUTE, objects.named(Usage, Usage.JAVA_RUNTIME))
attribute(OperatingSystemFamily.OPERATING_SYSTEM_ATTRIBUTE, objects.named(OperatingSystemFamily, "linux"))
attribute(MachineArchitecture.ARCHITECTURE_ATTRIBUTE, objects.named(MachineArchitecture, "x86-64"))
}
}
You probably do not want to do that, but just for explanation that now you could choose different Platforms (linux or windows or ...) in different places of the same build if desired.
To get back to the old behavior, you can do this to "copy" the configuration from the runtimeClasspath
over to xjcCatalogResolution
:
configurations.xjcCatalogResolution {
def rtCpAttributes = configurations.runtimeClasspath.attributes
rtCpAttributes.keySet().each { key ->
attributes.attribute(key, rtCpAttributes.getAttribute(key))
}
}
@samypr100 I am not sure if anything should be done in the plugin about this. We could think about some kind of convenience to let users define additional "classpaths" that should be configured. But depending on how individual these are, it might not be enough to just set the OperatingSystemFamily
and MachineArchitecture
attribute. This is because often things "just work" due to some fallback mechanisms in Gradle (from the time before the attributes existed). Maybe we can put the snippet above in the migration notes and link to the Gradle docs on attribute matching/variants.
To get back to the old behavior, you can do this to "copy" the configuration from the
runtimeClasspath
over toxjcCatalogResolution
:
FYI: At first I thought this fixes the issue. I can now properly execute gradle run
and the app starts and works fine.
Having said that, once I run gradle jpackage
and I try to use the jpackaged app it is very slow and then somehow unresponsive. I mean I can launch it but than the JavaFX UI freezes... (this is not specific to this simple test app but happens with other "real" apps also) ...
@danielpeintner I'm assuming you're talking about a modular one instead. Does jlink also become unresponsive as a result or is it just jpackage?
Can you post which app this is about and its build file? Also you said other "real" apps, can you post examples on those as well?
I'm assuming you're talking about a modular one instead. Does jlink also become unresponsive as a result or is it just jpackage?
No, it is still the non-modular one, see https://github.com/danielpeintner/Java11Test/tree/non-modular It uses the Badass Runtime Plugin (to create custom runtime images for non-modular applications) which used to work fine so far.
Also you said other "real" apps, can you post examples on those as well?
Unfortunately not...
@danielpeintner Not sure I can reproduce. I cloned the repo and the non-modular
branch and tried jpackage and it ran fine.
8:44:01 PM: Executing 'clean jpackage'...
> Task :clean
> Task :xjcGenerate
> Task :compileJava
> Task :processResources
> Task :classes
> Task :jar
> Task :startScripts
> Task :installDist
> Task :jre
> Task :jpackageImage
> Task :jpackage
BUILD SUCCESSFUL in 23s
10 actionable tasks: 10 executed
8:44:25 PM: Execution finished 'clean jpackage'.
Only changes I did were adding a toolchain to make sure I was using a proper jpackage compatible jdk since it was trying to use my JDK 11 install by default.
java {
toolchain.languageVersion = JavaLanguageVersion.of(17)
}
Also, app runs fine, no freezing.
Thought: Might be worth comparing the build/jpackage/hello-fx
directories before and after the update to see if there's anything worthy of note that could help troubleshoot what you're seeing(e.g. .jpackage.xml
, hello-fx.cfg
, list of jars, etc.).
@danielpeintner Not sure I can reproduce. I cloned the repo and the
non-modular
branch and tried jpackage and it ran fine....
Also, app runs fine, no freezing.
Thought: Might be worth comparing the
build/jpackage/hello-fx
directories before and after the update to see if there's anything worthy of note that could help troubleshoot what you're seeing(e.g..jpackage.xml
,hello-fx.cfg
, list of jars, etc.).
@samypr100 Thank you very much for looking into. In my case the jpackage task runs fine too but the app freezes.
I did compare the build/jpackage/
folders (with WinMerge) and there are lots of differences which are not easy to share.
Note: I am on Windows.
For example in the "broken version" the hello-fx.cfg
is different in the sense that
app.classpath=$APPDIR\javafx-base-17.jar
app.classpath=$APPDIR\javafx-graphics-17.jar
is missing. Hence also the according files javafx-base-17.jar
and javafx-graphics-17.jar
etc are missing.
Similarly the jfr.exe
is missing in runtime\bin\
and many more differences.
Finally also the resulting installer hello-fx-21.07.29.msi
changes from 90.599.778 bytes to 90.114.539 bytes.
If you want to take a look I can share the jpackage results with you. broken vs. fine
@danielpeintner Definitely weird that they were there to begin with since those are empty jars. Those where removed on 0.1.0
from the classpath, but since they're empty to begin with so it shouldn't be a problem. One concerning thing I did notice was that the final release
file modules are different which explains the lack of jfr.
release
file diff:
+MODULES="java.base java.compiler java.datatransfer java.xml java.prefs java.desktop java.logging java.security.sasl java.naming java.security.jgss java.transaction.xa java.sql java.sql.rowset java.xml.crypto"
-MODULES="java.base java.compiler java.datatransfer java.xml java.prefs java.desktop java.logging java.security.sasl java.naming java.security.jgss java.transaction.xa java.sql java.sql.rowset java.xml.crypto jdk.jfr jdk.unsupported"
This tells me the issue is with the runtime plugin detecting modules. I ran suggestModules
and jdk.jfr
, jdk.unsupported
were indeed missing.
You can add them back in the runtime task via:
additive = true
modules = ['jdk.jfr', 'jdk.unsupported']
My hunch is that the missing jdk.unsupported
is the performance problem you're seeing as it includes some packages (e.g. sun.misc.Unsafe) which have been used historically when doing raw memory accesses for performance reasons.
I'm definitely curious why those modules are not "detected" anymore by org.beryx.runtime
. Anyone else have thoughts why this would be happen after 0.1.0
but not in 0.0.14
in this non-modular project?
@samypr100 thank you once again for your insights 👍
Wrapping it into the jpackage
task does work!
runtime {
...
jpackage {
modules = ['jdk.jfr', 'jdk.unsupported']
additive = true
...
}
}
Anyhow, it is somewhat weird to be required to do so...
@danielpeintner Might be worth opening a separate issue and describing the discussion here thus far as otherwise it will be hard to track this. It's possible it can be a legitimate issue.
requires static jdk.jfr
in its module-inforequires jdk.unsupported
in its module-infoSo those two entries should likely be there. The fact that they're missing between 0.0.14
to 0.1.0
tells me something changed that's making org.beryx.runtime
not detect those entries. Hence this could be an issue in org.beryx.runtime
or due to the combination of org.beryx.runtime
and org.unbroken-dome.xjc
. Does the same happen with the org.beryx.jlink
plugin on a modular project?
@danielpeintner I think I figured it out by taking a peek at the source code of badass-runtime-plugin
I noticed they use project.configurations.runtimeClasspath.resolvedConfiguration.firstLevelModuleDependencies
to get the list of modules to walk.
On 0.0.14
that would yield [..., org.openjfx:javafx-base:17;runtime, org.openjfx:javafx-graphics:17;runtime, org.openjfx:javafx-controls:17;runtime, ...]
On 0.1.0
it now yields only [..., org.openjfx:javafx-controls:17;runtime, ...]
The badass-jlink-plugin seems unnafected.
Now that I see the root of the issue, this can be resolved temporarily on your Java11Test non-modular project by simply doing
modules = ['javafx.base', 'javafx.graphics', 'javafx.controls']
instead of modules = ['javafx.controls']
.
@jjohannes thoughts if this should be addressed on this plugin after the changes; for non-modular apps?
It does feels weird to have to tell users you might have to declare all transitive dependencies on the migration guide in cases like this one where another plugin relied on the firstLevelModuleDependencies
for non-modular apps.
@samypr100 without having looked at all the details, for me using only firstLevelModuleDependencies
sounds wrong for what the plugin aims to do. But I might be wrong. I don't think though that there are other plugins doing something similar (but I might be wrong about that as well).
Conceptually, I think what we do in this plugin is right. We rely on the JavaFX modules defining there transitive dependencies. Only the JavaFX modules you need directly are the ones you define.
However, you could argue that, from the user perspective, it somehow was more useable before. If you combine it with the badass-runtime-plugin
. And since we have the information about the transitive module dependencies in the code already (for using the Jars from a local SDK) it would be easy to take this change back and add all transitive dependencies as direct dependencies as well. It should not influence the overall dependency resolution result.
The change would be to always do what's in the else
block here I think:
https://github.com/openjfx/javafx-gradle-plugin/blob/master/src/main/java/org/openjfx/gradle/JavaFXOptions.java#L202
I would probably not do that change. But I can also understand if you like to adjust that.
Agreed. I did locally try that change earlier and it did fix the issue with the badass-runtime-plugin
.
var javaFXModules = JavaFXModule.getJavaFXModules(this.modules);
var javaFXModulesWithTransitives = Stream.concat(
javaFXModules.stream(),
javaFXModules.stream()
.flatMap(m -> m.getDependentModules().stream()))
.distinct()
.sorted();
if (customSDKArtifactRepository == null) {
javaFXModulesWithTransitives.forEach(javaFXModule ->
dependencySet.add(getDependencies().create(
MAVEN_JAVAFX_ARTIFACT_GROUP_ID + ":" +
javaFXModule.getArtifactName() + ":" +
getVersion())));
} else {
// Use the list of dependencies of each module to also add direct dependencies for those.
// This is needed, because there is no information about transitive dependencies in the metadata
// of the modules (as there is no such metadata in the local sdk).
javaFXModulesWithTransitives.forEach(javaFXModule ->
dependencySet.add(getDependencies().create(
Map.of("name", javaFXModule.getModuleName()))));
}
Like you said, main question is should this change be done here to support plugins that do the same thing as badass-runtime-plugin
or just add a migration guide note on this situation.
Another thought is that runtime-plugin/jlink-plugin are used quite extensively, so there's high likelyhood of this affecting others in a wider scope as they update to 0.1.0.
Edit (Sep 19, 2023): Added migration notes for this particular situation in af3c0dfa214c6ba2be4a051f2ef1810a88b884f2
After https://github.com/openjfx/javafx-gradle-plugin/pull/151 got merged, I think adding these migration notes is worthwhile as it's easy to encounter the org.javamodularity.moduleplugin missing plugin issue on other larger projects.