Skip to content
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

Remove native-image.properties from micrometer-core #4245

Closed
zakkak opened this issue Oct 16, 2023 · 4 comments · Fixed by #4246
Closed

Remove native-image.properties from micrometer-core #4245

zakkak opened this issue Oct 16, 2023 · 4 comments · Fixed by #4246
Labels
enhancement A general enhancement theme: native-image Issues related to GraalVM native or Project Leyden
Milestone

Comments

@zakkak
Copy link
Contributor

zakkak commented Oct 16, 2023

Please describe the feature request.

Remove -H:ReflectionConfigurationResources=${.}/reflect-config.json from native-image.properties file.
The option should not be necessary for native-image to pick up the reflect-config.json file, while with this option included native builds using GraalVM for JDK 21 print the following warning:

Warning: The option '-H:ReflectionConfigurationResources=META-INF/native-image/io.micrometer/micrometer-core/reflect-config.json' is experimental and must be enabled via '-H:+UnlockExperimentalVMOptions' in the future.

The option is passed in https://github.com/micrometer-metrics/micrometer/blob/3513df3780ab2fe1119ec2f9a6099ddcff7e7544/micrometer-core/src/main/resources/META-INF/native-image/io.micrometer/micrometer-core/native-image.properties#L17C8-L17C68

Rationale
As noted in the warning the use of '-H:ReflectionConfigurationResources=` is experimental and should thus be avoided.

Additional context

Reproducer:

wget https://github.com/graalvm/graalvm-ce-builds/releases/download/jdk-21.0.0/graalvm-community-jdk-21.0.0_linux-x64_bin.tar.gz
wget https://repo1.maven.org/maven2/io/micrometer/micrometer-core/1.11.5/micrometer-core-1.11.5.jar
tar xf graalvm-community-jdk-21.0.0_linux-x64_bin.tar.gz
cat > Main.java <<EOF                                                                                                                                                                     
public class Main {
    public static void main(String[] args) {
        System.out.println("Hello World!");
    }
}
EOF
./graalvm-community-openjdk-21+35.1/bin/javac Main.java
./graalvm-community-openjdk-21+35.1/bin/native-image -cp ./micrometer-core-1.11.5.jar:./ Main
@shakuzen
Copy link
Member

shakuzen commented Oct 16, 2023

Thank you for bringing this to our attention.

I believe this warning is something new since GraalVM 23 perhaps. It did not used to give such a warning. I checked with the latest patch of 22.3 (22.3.3) and it does not give a warning.

It does indeed seem native-image can find the reflect-config.json file now (I assume it did not before) without the native-image.properties. I was trying to find some documentation on this so we can know what minimum version of GraalVM we would be requiring by removing the explicit reference to the reflect-config.json from the native-image.properties. It seems to be documented here, but I'm not sure from what version that became the case. Do you happen to know?

@shakuzen shakuzen added enhancement A general enhancement theme: native-image Issues related to GraalVM native or Project Leyden and removed waiting-for-triage labels Oct 16, 2023
@shakuzen shakuzen added this to the 1.12.0 milestone Oct 16, 2023
@shakuzen
Copy link
Member

I did a little more digging in the documentation, and it looks like GraalVM 22.2 also mentions including the config, but it mentions reflection-config.json rather than reflect-config.json.

@zakkak
Copy link
Contributor Author

zakkak commented Oct 16, 2023

I did a little more digging in the documentation, and it looks like GraalVM 22.2 also mentions including the config, but it mentions reflection-config.json rather than reflect-config.json.

Well it also mentions reflect-config.json in https://www.graalvm.org/22.2/reference-manual/native-image/metadata/#specifying-reflection-metadata-in-json

Digging through the code instead of the documentation I was able to track its presence all the way back to GraalVM 19.1.

Probably GraalVM always detected reflect-config.json but not reflection-config.json (which probably by mistake was mentioned in the docs in the past), thus why there was the need for passing it explicitly through the command line options.

@shakuzen
Copy link
Member

Thank you for the investigation and reference. It sounds like we should be okay to remove our native-image.properties in our next minor release, then.

shakuzen pushed a commit that referenced this issue Oct 20, 2023
This is not needed because reflect-config.json will be picked up without being specified in native-image.properties. The configuration option used in native-image.properties causes a deprecation warning on the latest GraalVM native-image versions.

Closes #4245
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement theme: native-image Issues related to GraalVM native or Project Leyden
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants