Skip to content

Add directive packaging.graalvmJvmId#4223

Merged
Gedochao merged 2 commits intoVirtusLab:mainfrom
zrhmn:main
Apr 13, 2026
Merged

Add directive packaging.graalvmJvmId#4223
Gedochao merged 2 commits intoVirtusLab:mainfrom
zrhmn:main

Conversation

@zrhmn
Copy link
Copy Markdown
Contributor

@zrhmn zrhmn commented Apr 11, 2026

Fixes #4222

Add graalvmJvmId param to the Packaging class in scala.build.preprocessing.directives.
Add relevant @DirectiveExamples and @DirectiveUsage.
Add test at scala.build.tests.PackagingUsingDirectiveTests validating that graalvmJvmId gets passed to nativeImageOptions.

Checklist

  • tested the solution locally and it works
  • ran the code formatter (scala-cli fmt .)
  • ran scalafix (./mill -i __.fix)
  • ran reference docs auto-generation (./mill -i 'generate-reference-doc[]'.run)

How much have your relied on LLM-based tools in this contribution?

I relied moderately on GitHub Copilot. Since I am not a regular developer, it was hard for me to manually browse the codebase, until I ask the Copilot chat for help locating where directives were defined. Making the code change was easy without copilot help because the case class Packaging code was pretty readable and self-explanatory. Then I had to ask Copilot where to add tests, and finished the tests with Copilot-autocomplete.

How was the solution tested?

I created a project with the following directives:

//> using packaging.packageType graalvm
//> using packaging.graalvmJvmId graalvm-java25:25.0.2
//> using packaging.output "sctest01"
//> using packaging.graalvmArgs --no-fallback

... and ran with the built binary

../scala-cli/out/cli/3.3.7/standaloneLauncher.dest/launcher --power package .

(sans the graalvm-jvm-id arg) and it compiled, showing version GraalVM CE 25.0.2+10.1 during compilation. If I repeat this with the scala-cli installed on my system, I get GraalVM CE 17.0.9+9.1 instead, which confirms that the built launcher exhibits desired behavior.

Additional notes

@Gedochao Gedochao self-requested a review April 11, 2026 19:55
@zrhmn
Copy link
Copy Markdown
Contributor Author

zrhmn commented Apr 11, 2026

Hi @Gedochao I'm wondering if graalvmVersion and graalvmJavaVersion should also be directives? It seems just a matter of passing the parameters to the Packaging case class. I am torn because on the one hand, it would be consistent -- all the graalvm* args would also be directives, but on the other hand, I think it may clutter the docs too much. Just graalvmJvmId suffices for my own needs, but I wanna get outside opinions. What do you think?

@zrhmn
Copy link
Copy Markdown
Contributor Author

zrhmn commented Apr 12, 2026

I would also appreciate some help figuring out why the CI is failing.

@Gedochao
Copy link
Copy Markdown
Contributor

I would also appreciate some help figuring out why the CI is failing.

Seems to have been a flaky test, it's green after a re-run.

@Gedochao
Copy link
Copy Markdown
Contributor

Hi @Gedochao I'm wondering if graalvmVersion and graalvmJavaVersion should also be directives? It seems just a matter of passing the parameters to the Packaging case class. I am torn because on the one hand, it would be consistent -- all the graalvm* args would also be directives, but on the other hand, I think it may clutter the docs too much. Just graalvmJvmId suffices for my own needs, but I wanna get outside opinions. What do you think?

Feel free to add them. Them not being already there seems like an omission (just like graalvmJvmId).

Copy link
Copy Markdown
Contributor

@Gedochao Gedochao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@Gedochao Gedochao merged commit 700ee86 into VirtusLab:main Apr 13, 2026
135 of 138 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Directive to mirror the graalvm-jvm-id packaging flag

2 participants