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

Native Language Server integration with PM #11880

Merged
merged 22 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ class LauncherRunner(
configurationManager,
editionManager,
environment,
loggerConnection,
false
loggerConnection
) {

/** Creates [[RunSettings]] for launching the REPL.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class LauncherRunnerSpec extends RuntimeVersionManagerTest with FlakySpec {
val jvmOptions = Seq(("locally-added-options", "value1"))
val runnerEntryPoint = "org.enso.runner/org.enso.runner.Main"

def checkCommandLine(command: Command): Unit = {
def checkCommandLine(command: RawCommand): Unit = {
val arguments = command.command.tail
val javaArguments = arguments.takeWhile(_ != "-jar")
val appArguments = arguments.dropWhile(_ != runnerEntryPoint).tail
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ object Cli {
val NO_LOG_MASKING = "no-log-masking"
val VERBOSE_OPTION = "verbose"
val VERSION_OPTION = "version"
val NATIVE_OPTION = "native-language-server"
val PROFILING_PATH = "profiling-path"
val PROFILING_TIME = "profiling-time"
val PROJECTS_DIRECTORY = "projects-directory"
Expand Down Expand Up @@ -46,13 +45,6 @@ object Cli {
.desc("Checks the version of the Enso executable.")
.build()

val native: cli.Option = cli.Option.builder
.longOpt(NATIVE_OPTION)
.desc(
"(experimental) Attempts to use the native-image of any subprocess."
)
.build()

val json: cli.Option = cli.Option.builder
.longOpt(JSON_OPTION)
.desc("Switches the --version option to JSON output.")
Expand Down Expand Up @@ -172,7 +164,6 @@ object Cli {
.addOption(option.version)
.addOption(option.json)
.addOption(option.noLogMasking)
.addOption(option.native)
.addOption(option.profilingPath)
.addOption(option.profilingTime)
.addOption(option.projectsDirectory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ object ProjectManager extends ZIOAppDefault with LazyLogging {
logLevel <- setupLogging(verbosity, logMasking)
procConf = MainProcessConfig(
logLevel,
options.hasOption(Cli.NATIVE_OPTION),
opts.profilingPath,
opts.profilingTime
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ object configuration {
*/
case class MainProcessConfig(
logLevel: Level,
nativeImage: Boolean,
profilingPath: Option[Path],
profilingTime: Option[FiniteDuration]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ object ExecutorWithUnlimitedPool extends LanguageServerExecutor {
globalConfigurationManager = configurationManager,
editionManager = distributionConfiguration.editionManager,
environment = distributionConfiguration.environment,
loggerConnection = descriptor.deferredLoggingServiceEndpoint,
nativeImage = descriptor.nativeImage
loggerConnection = descriptor.deferredLoggingServiceEndpoint
)
val profilingPathArguments =
descriptor.profilingPath.toSeq
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ class LanguageServerController(
profilingPath = processConfig.profilingPath,
profilingTime = processConfig.profilingTime,
deferredLoggingServiceEndpoint = loggingServiceDescriptor.getEndpoint,
skipGraalVMUpdater = bootloaderConfig.skipGraalVMUpdater,
nativeImage = processConfig.nativeImage
skipGraalVMUpdater = bootloaderConfig.skipGraalVMUpdater
)

override def supervisorStrategy: SupervisorStrategy =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ case class LanguageServerDescriptor(
engineVersion: SemVer,
jvmSettings: JVMSettings,
discardOutput: Boolean,
nativeImage: Boolean,
profilingPath: Option[Path],
profilingTime: Option[FiniteDuration],
deferredLoggingServiceEndpoint: Future[Option[URI]],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ class ProjectCreationService[
globalConfigurationManager = configurationManager,
editionManager = distributionConfiguration.editionManager,
environment = distributionConfiguration.environment,
loggerConnection = loggingServiceDescriptor.getEndpoint,
nativeImage = false
loggerConnection = loggingServiceDescriptor.getEndpoint
)

val settings =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ class BaseServerSpec extends JsonRpcServerTestKit with BeforeAndAfterAll {
val processConfig: MainProcessConfig =
MainProcessConfig(
logLevel = if (debugLogs) Level.TRACE else Level.ERROR,
nativeImage = false,
profilingPath = profilingPath,
profilingTime = None
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import org.enso.runtimeversionmanager.releases.{
ReleaseProvider,
SimpleReleaseProvider
}
import org.enso.runtimeversionmanager.runner.{JVMSettings, JavaCommand}
import org.enso.runtimeversionmanager.runner.{JVMSettings, JavaExecCommand}
import org.enso.runtimeversionmanager.test.{
FakeEnvironment,
TestLocalLockManager
Expand Down Expand Up @@ -94,7 +94,7 @@ class TestDistributionConfiguration(
override def defaultJVMSettings: JVMSettings = {
val currentProcess =
ProcessHandle.current().info().command().toScala.getOrElse("java")
val javaCommand = new JavaCommand(currentProcess, None)
val javaCommand = new JavaExecCommand(currentProcess, None)
new JVMSettings(
javaCommandOverride = Some(javaCommand),
jvmOptions = Seq(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.enso.runtimeversionmanager.runner

import org.enso.runtimeversionmanager.components.Engine

/** Executable command used to trigger a Runner. Can be either a Java command or a native image executable.
*/
trait ExecCommand {
// Path to executable
def path: String
def cmdArguments(engine: Engine, jvmSettings: JVMSettings): Seq[String]
def javaHome: Option[String]
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ package org.enso.runtimeversionmanager.runner
* @param extraOptions extra options that should be added to the launched JVM
*/
case class JVMSettings(
javaCommandOverride: Option[JavaCommand],
javaCommandOverride: Option[JavaExecCommand],
jvmOptions: Seq[(String, String)],
extraOptions: Seq[(String, String)],
nativeImage: Boolean
Expand All @@ -32,7 +32,7 @@ object JVMSettings {
nativeImage: Boolean = false
): JVMSettings =
new JVMSettings(
if (useSystemJVM) Some(JavaCommand.systemJavaCommand) else None,
if (useSystemJVM) Some(JavaExecCommand.defaultSystem) else None,
jvmOptions,
extraOptions,
nativeImage
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package org.enso.runtimeversionmanager.runner

import org.enso.runtimeversionmanager.components.{Engine, GraalRuntime}
import org.enso.runtimeversionmanager.components.Manifest.JVMOptionsContext

/** Represents a way of launching the JVM.
*
* @param executableName name of the `java` executable to run
* @param javaHomeOverride if set, asks to override the JAVA_HOME environment
* variable when launching the JVM
*/
class JavaExecCommand(
val executableName: String,
val javaHome: Option[String]
) extends ExecCommand {
def path: String = executableName

override def cmdArguments(
engine: Engine,
jvmSettings: JVMSettings
): Seq[String] = {
def translateJVMOption(
option: (String, String),
standardOption: Boolean
): String = {
val name = option._1
val value = option._2
if (standardOption) s"-D$name=$value" else s"--$name=$value"
}

val context = JVMOptionsContext(enginePackagePath = engine.path)

val manifestOptions =
engine.defaultJVMOptions
.filter(_.isRelevant)
.map(_.substitute(context))
val commandLineOptions = jvmSettings.jvmOptions.map(
translateJVMOption(_, standardOption = true)
) ++ jvmSettings.extraOptions.map(
translateJVMOption(_, standardOption = false)
)
val componentPath = engine.componentDirPath.toAbsolutePath.normalize
val modulePathOptions =
Seq(
"--module-path",
componentPath.toString,
"-m",
"org.enso.runner/org.enso.runner.Main"
)

manifestOptions ++ commandLineOptions ++ modulePathOptions
}
}

object JavaExecCommand {

/** The [[JavaExecCommand]] representing the system-configured JVM.
*/
def defaultSystem: JavaExecCommand = new JavaExecCommand("java", None)

/** The [[JavaExecCommand]] representing a managed [[GraalRuntime]].
*/
def forRuntime(runtime: GraalRuntime): JavaExecCommand =
new JavaExecCommand(
executableName = runtime.javaExecutable.toAbsolutePath.normalize.toString,
javaHome = Some(runtime.javaHome.toAbsolutePath.normalize.toString)
)

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package org.enso.runtimeversionmanager.runner

import org.enso.cli.OS
import org.enso.distribution.{DistributionManager, Environment}
import org.enso.runtimeversionmanager.components.Engine

import java.nio.file.Path

case class NativeExecCommand(executablePath: Path) extends ExecCommand {
override def path: String = executablePath.toString

override def cmdArguments(
engine: Engine,
jvmSettings: JVMSettings
): Seq[String] =
Seq("-Dcom.oracle.graalvm.isaot=true")
Copy link
Member

Choose a reason for hiding this comment

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

Interesting. Why do we need to provide this argument?

E.g. if we can access org.graalvm.nativeimage.ImageInfo then we can query "properly" and even distinguish between build time and runtime time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The property is completely missing during runtime, hence not allowing me to provide different Context configuration.


override def javaHome: Option[String] = None
}

object NativeExecCommand {
def apply(version: String): Option[NativeExecCommand] = {
val env = new Environment() {}
val dm = new DistributionManager(env)
val execName = OS.executableName("enso")
val fullExecPath =
dm.paths.engines.resolve(version).resolve("bin").resolve(execName)

if (fullExecPath.toFile.exists()) Some(NativeExecCommand(fullExecPath))
else None
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import scala.util.{Failure, Try}
* @param extraEnv environment variables that should be overridden
* @param workingDirectory the working directory in which the command should be executed (if None, the working directory is not overridden and is inherited instead)
*/
case class Command(
case class RawCommand(
command: Seq[String],
extraEnv: Seq[(String, String)],
workingDirectory: Option[Path]
) {
private val logger = Logger[Command]
private val logger = Logger[RawCommand]

/** Runs the command and returns its exit code.
*
Expand Down
Loading
Loading