-
Notifications
You must be signed in to change notification settings - Fork 719
chore: set Java header at native image runtime #2069
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
@@ -164,6 +165,16 @@ static class ApiClientVersion { | |||
} | |||
|
|||
public String toString() { | |||
// When running the application as a native image, append `-graalvm` to the | |||
// version. | |||
String imageCode = System.getProperty("org.graalvm.nativeimage.imagecode"); |
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.
Please use constants instead of string literals here.
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.
That's a valid point! However, this might come down to the question of whether it is worth having dependency on graal-sdk in this repo? If we use the latest version (22.1.0) then we'll see the following incompatibility:
bad class file: /home/runner/.m2/repository/org/graalvm/sdk/graal-sdk/22.1.0/graal-sdk-22.1.0.jar(org/graalvm/nativeimage/ImageInfo.class)
Error: class file has wrong version 55.0, should be 52.0
Error: Please remove or make sure it appears in the correct subdirectory of the classpath.
Error: -> [Help 1]
In which case, we may need to do something similar to googleapis/gax-java#1671. This could still pose a slight risk since we still target Java 7. But open to hearing about what you think!
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'm fine with either. @suztomo what do you think? Is it worth it to bring the whole setup for compiling with 11 and testing on 7 in this case?
* chore: set Java header during native image runtime
* chore: set Java header during native image runtime
This contributes towards removing the use of the SVM dependency for native image compilation. This code makes use of the org.graalvm.nativeimage.imagecode system property instead of
Substitutions
to set the Java version at image runtime. For reference: https://www.graalvm.org/sdk/javadoc/index.html?constant-values.htmlSee reproducer: https://github.com/mpeddada1/avoid-substitutions