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

[23.1] GraalVM depends on org.graalvm.home.Version which is polyglot API #35714

Closed
jerboaa opened this issue Sep 4, 2023 · 4 comments · Fixed by #36037
Closed

[23.1] GraalVM depends on org.graalvm.home.Version which is polyglot API #35714

jerboaa opened this issue Sep 4, 2023 · 4 comments · Fixed by #36037
Labels
area/native-image kind/bug Something isn't working
Milestone

Comments

@jerboaa
Copy link
Contributor

jerboaa commented Sep 4, 2023

Describe the bug

GraalVM class depends on GraalVM's Version class for version parsing. See for example:

$ grep -rn 'org.graalvm' ./core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/GraalVM.java
141:        final org.graalvm.home.Version version;
152:            this.version = org.graalvm.home.Version.parse(version);

Since GraalVM Community for JDK 21 (internal version 23.1), the Version class will be part of the polyglot.jar (i.e. the org.graalvm.polyglot:polyglot maven artefact after the graal-sdk split). The intention for future versions of Quarkus would be to replace the dependency from graal-sdk to the future dependency org.graalvm.sdk:nativeimage only. See oracle/graal#7224

There are a couple of avenues to tackle this problem:

  1. Remove the dep in Quarkus for GraalVM version parsing. It's fairly trivial code and could get moved to a Quarkus class instead.
  2. When/if the org.graalvm.sdk:graal-sdk dependency gets reduced, it would need to get changed to require both. nativeimage and the polyglot artefact.
  3. Convince upstream to make Version public API.
  4. Keep the graal-sdk dependency forever (has the risk of upstream dropping the empty umbrella maven artefact, though)
  5. Something else I'm probably missing.
@jerboaa jerboaa added the kind/bug Something isn't working label Sep 4, 2023
@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 4, 2023

/cc @zakkak

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 8, 2023

I'm strongly in faviour of getting rid of the dependency and use a custom parser for the versions (in light of upstream no longer showing anything public about the - internal - version). So getting them to make Version.parse() API seems unlikely.

@zakkak
Copy link
Contributor

zakkak commented Sep 8, 2023

Although unfortunate I agree. We need to go back to doing our own parsing.

@zakkak
Copy link
Contributor

zakkak commented Sep 8, 2023

This is also related to #34161

zakkak added a commit to zakkak/quarkus that referenced this issue Sep 20, 2023
gsmet pushed a commit to zakkak/quarkus that referenced this issue Sep 21, 2023
@quarkus-bot quarkus-bot bot added this to the 3.5 - main milestone Sep 21, 2023
michelle-purcell pushed a commit to michelle-purcell/quarkus that referenced this issue Sep 21, 2023
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/native-image kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants