Add additional packaging.graalvm* directives#4225
Merged
Gedochao merged 2 commits intoVirtusLab:mainfrom Apr 15, 2026
Merged
Conversation
Gedochao
reviewed
Apr 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4224.
packaging.graalvmVersion(string)packaging.graalvmJavaVersion(int)graalvmJavaVersiongraalvmJvmId)Checklist
scala-cli fmt .)scalafix(./mill -i __.fix)./mill -i 'generate-reference-doc[]'.run)How much have your relied on LLM-based tools in this contribution?
Moderately. In a previous PR #4223 I got familiarized with the
preprocessing.directives.Packagingcase class as well as relevant unit tests with the help of GitHub Copilot (using Claude Sonnet 4.6), so I did not need much AI assistance this time around. I did trip on the fact thatgraalvmJvmVersionarg ofNativeImageOptionswas supposed to be anOption[Int]while the arg I was getting from thePackagingargs was anOption[String]. I tried using a simple.flatMap(_.toIntOption)but then realized if a bad (non-int)graalvmJvmVersionis passed, it would be replaced silently. So I used AI help to figure out how to weave-in validation into thebuildOptionsmethod. A hint was all it took; it was easy enough to follow existing patterns.How was the solution tested?
Unit tests included.
Additionally, I created a test project with the following lines:
And ran with
I saw the version
GraalVM CE 25.0.2+10.1in the build output, which verified thatgraalvmVersionandgraalvmJavaVersionwere being correctly compiled into a valid JVM ID by the just-compiled launcher.I also tried with an invalid graalvmJavaVersion and saw the expected error output:
verifying that integer validation of
graalvmJavaVersionwas working as intended.Additional notes
I think enhancements could be made to
NativeImageOptions.Currently, passing just a whole JVM ID (e.g.
graalvm-community:25.0.2) works smoothly. If the JVM ID is not passed, and instead a Java version (e.g. 25) along with a JVM version (e.g. 25.0.2) is passed, thenNativeImageOptionsuses that to construct a JVM ID (in case of e.g. it would begraalvm-java25:25.0.2). If both a JVM ID as well as the Java version + JVM version pair are passed, then the latter get ignored entirely. This is all expected behavior.If user mistakenly passes an unmatched Java version and JVM version pair (e.g.
graalvm-version=24.0.2 graalvm-java-version=22),NativeImageOptionsnaively compiles that to an invalid JVM ID (in case of e.g.graalvm-java22:24.0.2). This, too, may be considered expected behavior and an obvious user error.However, something that IMO could be more common and not an obvious user error, would be that user passes a JVM version but neglects to pass a Java version or vice versa (e.g. user passes
graalvm-version=24.0.2, resulting ingraalvm-jvm-id=graalvm-java17:24.0.2or user passesgraalvm-java-version=24, resulting ingraalvm-jvm-id=graalvm-java24:17.0.9).So for the user, passing a JVM ID directly may be the recommended action, rendering the separate Java version and JVM version parameters useless and superficial in practice. Alternatively, we can have
NativeImageOptionsresolve JVM version + Java version pair in a more intelligent, slightly better way. Perhaps, if a JVM version is provided without a Java version, the Java version can be determined from the first part of the JVM version (e.g. for JVM version 24.0.2 assume the Java version will always be 24). If Java version is provided without a JVM version, the latest JVM version for the given Java version could automatically be determined somehow. An error could be shown if the JVM version and Java version are both given but do not match. etc. etc.This is just something I was considering when making the changes in this PR. For now I think the changes in the PR are fine and in-line with the currently expected behavior. I would welcome comments on the suggested improvements to NativeImageOptions and consider making the changes in the future if I can get folks onboard with my thinking.