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

improvement: restart sbt after java home change #5381

Merged
merged 1 commit into from
Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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 @@ -9,6 +9,7 @@ import scala.meta.internal.bsp.BspConfigGenerationStatus._
import scala.meta.internal.builds.BuildServerProvider
import scala.meta.internal.builds.BuildTools
import scala.meta.internal.builds.SbtBuildTool
import scala.meta.internal.builds.ShellRunner
import scala.meta.internal.metals.BloopServers
import scala.meta.internal.metals.BuildServerConnection
import scala.meta.internal.metals.Messages
Expand Down Expand Up @@ -73,6 +74,7 @@ class BspConnector(
def connect(
workspace: AbsolutePath,
userConfiguration: UserConfiguration,
shellRunner: ShellRunner,
)(implicit ec: ExecutionContext): Future[Option[BspSession]] = {
def connect(
workspace: AbsolutePath
Expand All @@ -90,6 +92,8 @@ class BspConnector(
val shouldReload = SbtBuildTool.writeSbtMetalsPlugins(workspace)
val connectionF =
for {
_ <- SbtBuildTool(workspace, () => userConfiguration)
.ensureCorrectJavaVersion(shellRunner, workspace, client)
connection <- bspServers.newServer(workspace, details)
_ <-
if (shouldReload) connection.workspaceReload()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ import scala.meta.internal.io.FileIO
import scala.meta.internal.metals.BuildServerConnection
import scala.meta.internal.metals.Cancelable
import scala.meta.internal.metals.ClosableOutputStream
import scala.meta.internal.metals.JdkSources
import scala.meta.internal.metals.MetalsBuildClient
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.MetalsServerConfig
import scala.meta.internal.metals.QuietInputStream
import scala.meta.internal.metals.SocketConnection
import scala.meta.internal.metals.Tables
import scala.meta.internal.metals.UserConfiguration
import scala.meta.internal.metals.clients.language.MetalsLanguageClient
import scala.meta.internal.mtags.MD5
import scala.meta.internal.mtags.URIEncoderDecoder
Expand All @@ -43,6 +45,7 @@ final class BspServers(
tables: Tables,
bspGlobalInstallDirectories: List[AbsolutePath],
config: MetalsServerConfig,
userConfig: () => UserConfiguration,
)(implicit ec: ExecutionContextExecutorService) {

def resolve(): BspResolvedResult = {
Expand Down Expand Up @@ -92,7 +95,10 @@ final class BspServers(
args,
projectDirectory,
redirectErrorOutput = false,
Map(),
JdkSources
.defaultJavaHome(userConfig().javaHome)
.map("JAVA_HOME" -> _.toString())
.toMap,
processOut = None,
processErr = Some(l => scribe.info("BSP server: " + l)),
discardInput = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import java.nio.file.Files
import java.nio.file.Path
import java.util.Properties

import scala.concurrent.ExecutionContext
import scala.concurrent.Future

import scala.meta.inputs.Input
Expand All @@ -14,6 +15,7 @@ import scala.meta.internal.semver.SemVer.isCompatibleVersion
import scala.meta.io.AbsolutePath

import org.eclipse.lsp4j.Position
import org.eclipse.lsp4j.services.LanguageClient

case class SbtBuildTool(
workspaceVersion: Option[String],
Expand Down Expand Up @@ -186,6 +188,24 @@ case class SbtBuildTool(
override def toString: String = SbtBuildTool.name

def executableName = SbtBuildTool.name

def ensureCorrectJavaVersion(
shellRunner: ShellRunner,
workspace: AbsolutePath,
languageClient: LanguageClient,
)(implicit ex: ExecutionContext): Future[Unit] =
if (checkCorrectJavaVersion(workspace, userConfig().javaHome)) {
Future.successful(())
} else {
languageClient
.showMessageRequest(Messages.SbtServerJavaHomeUpdate.params())
.asScala
.flatMap {
case Messages.SbtServerJavaHomeUpdate.restart =>
shutdownBspServer(shellRunner, workspace).ignoreValue
case _ => Future.successful(())
}
}
}

object SbtBuildTool {
Expand Down Expand Up @@ -382,4 +402,34 @@ object SbtBuildTool {
val prepend = autoImports.mkString("", "\n", "\n")
prepend + text
}

def checkCorrectJavaVersion(
workspace: AbsolutePath,
userJavaHome: Option[String],
): Boolean = {
val bspConfigFile = workspace.resolve(".bsp").resolve("sbt.json")
if (bspConfigFile.isFile) {
val matchesSbtJavaHome =
for {
text <- bspConfigFile.readTextOpt
json = ujson.read(text)
args <- json("argv").arrOpt
firstArg <- args.headOption
javaArg <- firstArg.strOpt
tgodzik marked this conversation as resolved.
Show resolved Hide resolved
if (javaArg.endsWith("java"))
} yield {
val possibleJavaBinaries =
JavaBinary.allPossibleJavaBinaries(userJavaHome)
val sbtJavaHomeIsCorrect =
possibleJavaBinaries.exists(_.toString == javaArg)
if (!sbtJavaHomeIsCorrect) {
scribe.debug(
s"Java binary used by sbt server $javaArg doesn't match the expected java home. Possible paths considered: $possibleJavaBinaries"
)
}
sbtJavaHomeIsCorrect
}
matchesSbtJavaHome.getOrElse(true)
} else true
}
}
11 changes: 11 additions & 0 deletions metals/src/main/scala/scala/meta/internal/metals/JavaBinary.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,15 @@ object JavaBinary {
case path if path.exists => path
}
}

def allPossibleJavaBinaries(
javaHome: Option[String]
): List[AbsolutePath] = {
JdkSources
.defaultJavaHome(javaHome)
.flatMap(home => List(home.resolve("bin"), home.resolve("bin/jre")))
.flatMap(bin => List("java", "java.exe").map(bin.resolve))
.withFilter(_.exists)
.flatMap(binary => List(binary.dealias, binary))
}
}
23 changes: 23 additions & 0 deletions metals/src/main/scala/scala/meta/internal/metals/Messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -965,4 +965,27 @@ object Messages {
}
}

object SbtServerJavaHomeUpdate {
val restart: MessageActionItem =
new MessageActionItem("Restart sbt server")

val notNow: MessageActionItem =
new MessageActionItem("Not now")

def params(): ShowMessageRequestParams = {
val params = new ShowMessageRequestParams()
params.setMessage(
s"Java home has been updated, do you want to restart the sbt BSP server? (the change will only be picked up after the restart)"
)
params.setType(MessageType.Info)
params.setActions(
List(
restart,
notNow,
).asJava
)
params
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ class MetalsLspService(
tables,
bspGlobalDirectories,
clientConfig.initialConfig,
userConfig,
)

private val bspConnector: BspConnector = new BspConnector(
Expand Down Expand Up @@ -2078,7 +2079,7 @@ class MetalsLspService(
(for {
_ <- disconnectOldBuildServer()
maybeSession <- timerProvider.timed("Connected to build server", true) {
bspConnector.connect(folder, userConfig())
bspConnector.connect(folder, userConfig(), shellRunner)
}
result <- maybeSession match {
case Some(session) =>
Expand Down
Loading