-
Notifications
You must be signed in to change notification settings - Fork 70
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
Prepare NBT for release of GraalVM for JDK 21. #496
Conversation
...ve-gradle-plugin/src/main/java/org/graalvm/buildtools/gradle/tasks/BuildNativeImageTask.java
Outdated
Show resolved
Hide resolved
@@ -119,13 +125,15 @@ public List<String> asArguments() { | |||
appendBooleanOption(cliArgs, options.getVerbose(), "--verbose"); | |||
appendBooleanOption(cliArgs, options.getSharedLibrary(), "--shared"); | |||
appendBooleanOption(cliArgs, options.getQuickBuild(), "-Ob"); | |||
appendBooleanOption(cliArgs, options.getRichOutput(), "-H:+BuildOutputColorful"); | |||
appendBooleanOption(cliArgs, options.getRichOutput(), majorJDKVersion >= 21 ? "--color" : "-H:+BuildOutputColorful"); |
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.
Note: this option is only used in the Gradle plugin. Since Maven doesn't need it, it also doesn't need to check native-image --version
unconditionally.
...ve-gradle-plugin/src/main/java/org/graalvm/buildtools/gradle/tasks/BuildNativeImageTask.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 overall, except for the change to the command line provider. We should simply add another provider as a constructor argument. It's a breaking change, but I think it's fine (the only known consumer is the Micronaut Gradle plugin). This will be easier to maintain.
Also I need to have a method which computes the JDK version for the Micronaut plugin too, so having this code shared would be simpler.
Thanks!
...gin/src/main/java/org/graalvm/buildtools/gradle/internal/NativeImageCommandLineProvider.java
Outdated
Show resolved
Hide resolved
...ve-gradle-plugin/src/main/java/org/graalvm/buildtools/gradle/tasks/BuildNativeImageTask.java
Outdated
Show resolved
Hide resolved
...ve-gradle-plugin/src/main/java/org/graalvm/buildtools/gradle/tasks/BuildNativeImageTask.java
Show resolved
Hide resolved
...ve-gradle-plugin/src/main/java/org/graalvm/buildtools/gradle/tasks/BuildNativeImageTask.java
Outdated
Show resolved
Hide resolved
b931846
to
97955df
Compare
Please mention this change in our changelog before merging |
5fc1a64
to
3855c06
Compare
3855c06
to
2f191a2
Compare
Any idea why I'm still seeing
despite using version 0.9.27 of the Gradle plugin, which should include this PR? |
@sschuberth is that still the case with 0.9.28? Which GraalVM distribution are you using? /cc @dnestoro |
I cannot reproduce this anymore with plugin version 0.9.28 and GraalVM runtime version 21.0.1+12-jvmci-23.1-b22. So I was probably just too impatient WRT the version this got fixed in... |
This PR changes NBT so that it only uses public / stable Native Image API, which otherwise would require uses to unlock experimental options (see oracle/graal#7105 for context) in the next release of GraalVM.