hydraulic-software / conveyor

Gradle plugin, user guide and discussion forums for Conveyor
https://conveyor.hydraulic.dev
Apache License 2.0
123 stars 9 forks source link

jdeps can't find javafx modules on Java 20 toolchain #55

Closed lenis0012 closed 1 year ago

lenis0012 commented 1 year ago

Describe the bug jdeps can't find javafx modules for OpenJFX 20 on Java 20, resulting in fallback to jar-based deployment and an unoptimized JRE.

Error when running conveyor -v make windows-app --rerun:

❌ jdeps failed scanning JAR oraksi-1.0.jar
❯ [Processed Jars for Windows Intel] jdeps failed scanning JAR oraksi-1.0.jar
hydraulic.shell.SubprocessFailed: C:\Users\Lennart\AppData\Local\Packages\Conveyor_fg3qp2cw01ypp\LocalCache\Local\Hydraulic\Conveyor\Cache\entries\9c\00\9c0091819b86a1dc\content/bin/jdeps: Returned non-zero exit code 1
Exception in thread "main" java.lang.module.FindException: Module javafx.controls not found, required by com.lenis0012.oraksi
        at java.base/java.lang.module.Resolver.findFail(Resolver.java:892)
        at java.base/java.lang.module.Resolver.resolve(Resolver.java:192)
        at java.base/java.lang.module.Resolver.resolve(Resolver.java:141)
        at java.base/java.lang.module.Configuration.resolve(Configuration.java:420)
        at java.base/java.lang.module.Configuration.resolve(Configuration.java:254)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsConfiguration$Builder.build(JdepsConfiguration.java:564)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsTask.buildConfig(JdepsTask.java:604)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsTask.run(JdepsTask.java:558)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsTask.run(JdepsTask.java:534)
        at jdk.jdeps/com.sun.tools.jdeps.Main.main(Main.java:50) @ hydraulic.shell.impl.ExecKt.waitForSubProcess(Unknown Source)

To Reproduce Steps to reproduce the behavior:

  1. Generate new javafx application using conveyor
  2. Make sure JDK 20 is installed and active
  3. Set toolchain version and jfx version to 20 in build.gradle
  4. Set gradle wrapper version to 8.1-rc-1 in gradle/wrapper/gradle-wrapper.properties to support JDK 20
  5. Run conveyor -v make windows-app
  6. See error

Expected behavior No errors, optimized JRE using module dependency graph and jimage deployment

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

Additional context Not 100% sure if this is related to Java 20 or JFX 20 but this issue did not occur when using the default (JFX 19 & java 17)

lenis0012 commented 1 year ago

I can confirm that the issue is unrelated to the gradle or jfx update and only happens when you update the gradle toolchain to 20.

mikehearn commented 1 year ago

Could you attach a log please? We can't reproduce this. I was able to upgrade both Gradle and OpenJDK to 20 and the build completed successfully. The jdeps error is talking about a third party library, not jfx itself. Maybe there's something odd about that jar that the new jdeps rejects and old jdeps is OK with?

lenis0012 commented 1 year ago

log.24.zip

lenis0012 commented 1 year ago

System path: Screenshot 2023-04-03 124637

I made a new projecr with only the changes I described and no other modifications and it still occurs:

❌ jdeps failed scanning JAR jdeps-test-1.0.jar

mikehearn commented 1 year ago

Thanks for the bug report. We were able to repro it. The cause is having an explicit module that targets jdk 20 (or any version higher than 17).

The error isn't really an error, it should actually be a warning and will be in the next version. jdeps is optional, it's there as a convenience for cases where your module graph isn't complete. So it shouldn't impact your app. You say the result is an unoptimized JDK but I don't see that, when I do this I get a jdk that only includes the minimal set of modules (about 70mb).

There are a few workarounds:

  1. Add app.jvm.modules = [] to the very top of your file before any include statements. Literally the first line. This will replace the default [detect] list, which is what triggers the use of jdeps.
  2. Strip module-info.class files from your app modules (not for javafx).
  3. Ignore the error/warning.

We'll get a real fix done.

lenis0012 commented 1 year ago

I see. thank you and I apologize for misunderstanding.

lenis0012 commented 1 year ago

It was my understanding from the documentaiton that your application is supposed to be packaged into the modules file and I thought that the jdeps error was preventing that from happening

from the docs:

linked modules get incorporated into the single modules file which uses an more optimized format called "jimage". Classes can be loaded more quickly and take up less space when processed this way.

I saw that the application was being bundled to a jar file instead and assumed that the jdeps issue was causing this leading to a less optimized distribution

mikehearn commented 1 year ago

It's not your mis-understanding, there's a real bug, it's just not especially serious if you already know your dependencies.

The only jars that can go modules are jars that are both explicit and which don't depend on auto modules. That's a jlink limitation, not ours. Conveyor scans the jars to figure out what that set is so they can go into the modules file. It doesn't work if it encounters a module with a java 20 module-info.class file and won't add it into the modules file in that case.

It's arguable what is more desirable. On one hand, being jlinked in to images means slightly better startup time (in theory), and slightly smaller downloads. On the other hand it reduces the effectiveness of delta updates.

mikehearn commented 1 year ago

On Windows only. On macOS the update engine we ship (sparkle) also supports delta updates but Conveyor doesn't currently make them as part of the build process.

mikehearn commented 1 year ago

We shipped a fix for the jdeps issue in 8.1, please upgrade and see if that works for you.