-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Improve GraalVM/Mandrel version detection #34161
Comments
@gsmet GraalVM is a jlinked JDK. It adds modules to the standard set of OpenJDK. We've been pointed at looking at the I believe it's true that the GraalVM team plans to continue to populate the |
@gsmet Could you clarify this statement? How is the 23.0 introduced version format forcing us to the 23.0 release? |
Actually, I misread one test and missed you were extracting As for Mandrel not populating Using the |
Yes, agreed. We had a long discussion and that was the only token available to us at this point (other than
Keep in mind that by using the Note that we've discussed this at the time here: |
We could put a hard requirement that for custom build images, they should provide either a |
I am OK with introducing a Note that AFAIU GraalVM's end goal is to completely drop the YY.X (e.g. 23.0) versioning at some point, meaning that even if we get it from the |
This seems a long way out, though. |
My 2c: I would like us to keep parsing what native-image gives you as that is the executable (script) used to run the build. Having something in a text file in GRAALVM_HOME is nice, but it might not belong to the native-image executed. Looking for a text file in |
Temporarily works around quarkusio#36246, till we have a consensus on how to move forward in quarkusio#34161 Closes quarkusio#36246
Temporarily works around quarkusio#36246, till we have a consensus on how to move forward in quarkusio#34161 Closes quarkusio#36246
Temporarily works around quarkusio#36246, till we have a consensus on how to move forward in quarkusio#34161 Closes quarkusio#36246
I thought a bit more about it and my proposal would be to completely drop the old versioning for new versions. E.g. in our When comparing the versions, we would first compare And we would switch the the new GraalVM advertised version as the version instead of relying on the old scheme that is legacy and will go away at some point. Now I don't know what's your plan for Mandrel and if this would work too :). |
For GraalVM for JDK 21 (internal version 23.1) and Mandrel 23.1 we can explore going with |
I like @gsmet's proposal. This way we avoid depending on files (which has the issue @Karm mentions), and we align with upstream GraalVM.
The main issue I see with this approach is for Mandrel releases based on the same source level but on different JDKs (something that upstream GraalVM won't do but we have not crossed out it yet for Mandrel). E.g. Mandrel 24.0.x could potentially have two builds, one "for JDK 21" and another "for JDK 22", in that case we would need to resolve to the internal version (24.0) to distinguish it from 23.1 which is also "for JDK 21" and will be using the same 21 version since it will also get a CPU update. The
from
|
Sorry, I must be missing something :( Why does the classification of versioning schemes free us from knowing what the version actually is? My understanding was that the proposal would treat the single line versioning as |
The suggestion is to keep doing what we do for IIRC the three line version scheme appeared with the introduction of the "GraalVM for JDK XX" versioning scheme, thus if we detect the three line output the suggestion is to just compare the JDK versions. Now to compare across schemas, it's as simple as You would essentially have a So the version object in quarkus would be something like:
Now the issue with that is that |
I see. Thanks for clarifying. |
Temporarily works around quarkusio#36246, till we have a consensus on how to move forward in quarkusio#34161 Closes quarkusio#36246
It has been a while and we haven't made any progress on that, mainly because there was no clear winner (even the schema approach has its drawback). It turns out the work around in #36267 is doing its job just fine. This in conjunction with the fact that with the 25 release the internal and JDK version will get aligned I am considering closing this as "not planned". To give a glimpse into the future, the expected versions to come are:
Note that there is a (small) chance to release non-LTS Mandrel releases (e.g. 26) built on the LTS JDK, so we might end up getting 26 and 27 built with JDK 25 but this should be easier to handle given we already parse the Java version as well. WDYT? |
Sounds good to me. FWIW, the last row should read |
At the moment, we are parsing the output of
native-image --version
to determine the GraalVM/Mandrel version. This is brittle and needs to be adjusted.Note that we need to adjust things before the next minor version of GraalVM because the new format introduced in 23.0 forces the 23.0 version, which won't be true when 23.1 is released.@jerboaa corrected me on this, my mistake.In the GraalVM distribution, there is a
release
file at the root containing some properties with versions:What is unsure is if they will continue to populate the
GRAALVM_VERSION
property now that they switched to using the JDK version scheme but I suppose we won't know that before the next GraalVM release.@zakkak @Karm @jerboaa @galderz could any of you add a comment with the Mandrel output for it? Also do you have any idea how it will evolve in the future and if it's a good idea to use this file?
The text was updated successfully, but these errors were encountered: