scala-cli
scala-cli copied to clipboard
Platform should be set when non default JVM is used.
Version(s) 1.1.1
Describe the bug Currently, if the user specifies JDK 11 as the JVM to use as in:
The platform section used for Bloop should show that, but it doesn't:
"platform": { "name": "jvm", "config": { "options": [] }, "mainClass": [] },
This means when running the wrong JVM will be used, the one Bloop runs on.
To Reproduce
//> using scala 3.3.1
//> using jvm adopt:1.11
@main
def main =
println(s"Hello java ${sys.props("java.home")}!")
prints:
Hello java /usr/lib/jvm/graal17! in my case, since I have graal17.
Expected behaviour Prints:
Hello java <coursier-java-path>!
This seems to have been done on purpose and the explanation seems to be this comment:
// We don't pass jvm home here, because it applies only to java files compilation (found here)
Is this even valid? If not, then that's a simple revert.
Another curious thing is that running the reproduction yields correct results for me. (although not with the provided jvm as it's not available for my machine I think)
Platform is actually not only used for compilation, but still it would be better to use the exact java compiler I think. No reason to try and be smarter than what bloop provides
It's Scala CLI which only uses Bloop for compilation (effectively treating it as a compiler), and then runs the compiled code itself. Which is why the platform config isn't truly relevant from the perspective of Scala CLI (we don't care what's passed to Bloop there, since we run it ourselves with the Java we want).
Regardless, made a PR to explicitly set it, I guess it can't hurt to have it consistent. I don't think it changes any behaviour from our perspective, let me know if I turn out to be wrong.
Regardless, made a PR to explicitly set it, I guess it can't hurt to have it consistent. I don't think it changes any behaviour from our perspective, let me know if I turn out to be wrong.
Ha, I proved myself wrong, it breaks compat with older JVMs, as Bloop tries to pass flags which aren't supported there.
I tried setting the Java Home in the runtimeConfig section, which kind of works, since we don't use Bloop for JVM runtime and thus the property can be read from Bloop JSON... but then, the value isn't passed in a BuildTarget's data in BSP, so an IDE will have a hard time getting it anyway (buildTarget.jvmBuildTarget.javaHome points to Bloop JVM unless overridden, so that's confusing)
I left the PR as draft, feel free to look at it if anyone has time to push this further.