-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add GraalVM for JDK 21 support. #1236
Conversation
- no longer register HomeFinderFeature post JDK 21 - no longer link against fdlibm on Windows post JDK 21 - link against extnet and mswsock on Windows post JDK 21
- no longer link against fdlibm on Linux post JDK 21 - add required --add-exports for GluonFeature
d661a57
to
6a65d3b
Compare
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.
I've left some comments.
Tested successfully (with some changes) on Mac with Gluon's GraalVM 22.1.0.1-Final and the official GraalVM 21+35.1 (using also #1234)
In any case, note that the official GraalVM 21 builds will still fail building native images for mobile targets (more changes are required on Substrate, but also probably on the GraalVM builds)
src/main/java/com/gluonhq/substrate/target/AbstractTargetConfiguration.java
Show resolved
Hide resolved
src/main/java/com/gluonhq/substrate/target/AbstractTargetConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gluonhq/substrate/target/AbstractTargetConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gluonhq/substrate/target/LinuxTargetConfiguration.java
Outdated
Show resolved
Hide resolved
- only add '--add-exports' argument when needed
src/main/java/com/gluonhq/substrate/model/InternalProjectConfiguration.java
Show resolved
Hide resolved
src/main/java/com/gluonhq/substrate/model/InternalProjectConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gluonhq/substrate/model/InternalProjectConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gluonhq/substrate/model/InternalProjectConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gluonhq/substrate/target/LinuxTargetConfiguration.java
Outdated
Show resolved
Hide resolved
- update toString to include new fields
403c72e
to
b7e5cdf
Compare
src/main/java/com/gluonhq/substrate/target/AbstractTargetConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gluonhq/substrate/target/AbstractTargetConfiguration.java
Outdated
Show resolved
Hide resolved
@kkriske please fix conflicts (and do the refactoring if needed) |
# Conflicts: # src/main/java/com/gluonhq/substrate/model/InternalProjectConfiguration.java
src/main/java/com/gluonhq/substrate/model/InternalProjectConfiguration.java
Show resolved
Hide resolved
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.
Looks good!
Hey...could you tell me if Gluon is planning on a new official Gluon GraalVM build anytime soon? |
--add-exports
flag required for the GluonFeature internal API usageIssue
Builds against GraalVM for JDK 21 run into some issues:
Windows specific issues:
nio
library now depends onmswsock
, so that needs to be added to the linkerfdlibm
library is no longer part of the JDK so it should be removed from the linker, not sure since what exact JDK versionextnet
library is now used on Windows in theWindowsSocketOptions
class, so it needs to be added to the linkerLinux specific issues:
fdlibm
library is no longer part of the JDK so it should be removed from the linker, not sure since what exact JDK versionOther:
fdlibm
should probably be removed from the remaining targets as well, but I have no way to verify that.Progress