Enabled per-project JVM mode configuration #12550
Conversation
`package.yaml` now supports `jvm` field that takes a Boolean. If set to true, the project will be run in JVM mode even if Language Server's Native Image is available.
hubertp
added
the
CI: No changelog needed
There was a problem hiding this comment.
- unless I am mistaken
jvm: truehas no effect on CLI execution - we need CLI execution to behave the same as IDE execution (thus requesting changes)
- e.g.
ensolauncher needs to honor thejvm:value inpackage.yaml - having a test to ensure that would be great
- other than that OK
- I just left few inline comments/opinions about tri-state and binary state
| @@ -628,6 +628,7 @@ private void createNew( | |||
| authors, | |||
| nil(), | |||
| "", | |||
| Option$.MODULE$.empty(), | |||
There was a problem hiding this comment.
This is the only change in engine/runner. Is that sufficient to switch to JVM mode when there is a project with jvm: true and one does:
enso --run the_project
? I am afraid that it is not sufficient.
| @@ -280,7 +280,8 @@ object ComponentGroupsResolverSpec { | |||
| maintainers = Nil, | |||
| edition = None, | |||
| preferLocalLibraries = true, | |||
| componentGroups = Some(componentGroups) | |||
| componentGroups = Some(componentGroups), | |||
| jvm = None | |||
There was a problem hiding this comment.
Is it important to have a tri-state value for jvm? Isn't binary Boolean enough?
There was a problem hiding this comment.
It's not enough if we want to be backwards compatible with configs that don't have that field.
| @@ -307,7 +308,8 @@ class PackageManager[F](implicit val fileSystem: FileSystem[F]) { | |||
| authors: List[Contact] = List(), | |||
| maintainers: List[Contact] = List(), | |||
| license: String = "", | |||
| componentGroups: Option[ComponentGroups] = None | |||
| componentGroups: Option[ComponentGroups] = None, | |||
| jvm: Option[Boolean] = None | |||
There was a problem hiding this comment.
A tri-state value. Shouldn't we use just Boolean - e.g. binary state here?
There was a problem hiding this comment.
I don't believe so, no.
| @@ -4,3 +4,4 @@ enso-version: default | |||
| version: "0.0.1" | |||
| author: "Enso Team <[email protected]>" | |||
| maintainer: "Enso Team <[email protected]>" | |||
| jvm: false | |||
There was a problem hiding this comment.
Why do we need jvm: false here? Isn't false the default when there is no jvm:?
| engineVersion = engineVersion, | ||
| jvmSettings = distributionConfiguration.defaultJVMSettings, | ||
| jvmModeEnabled = | ||
| processConfig.jvmMode || project.jvmModeEnabled.getOrElse(false), |
There was a problem hiding this comment.
This is where the tri-state gets converted to binary state, right? Is LanguageServerController the right place to encode the default value? Shouldn't there be project.isJvmMode(): Boolean?
That is a valid point indeed. |
Pull Request Description
package.yamlnow supportsjvmfield that takes a Boolean. If set to true, the project will be run in JVM mode even if Language Server's Native Image is available.Closes #12529.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.