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

feat: add setting to choose preferred build server #6121

Merged
merged 7 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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 @@ -7,7 +7,6 @@ import scala.concurrent.ExecutionContext
import scala.concurrent.Future

import scala.meta.internal.builds.Digest.Status
import scala.meta.internal.metals.AutoImportBuildKind
import scala.meta.internal.metals.BuildInfo
import scala.meta.internal.metals.Confirmation
import scala.meta.internal.metals.Messages._
Expand Down Expand Up @@ -133,9 +132,7 @@ final class BloopInstall(
scribe.info(s"skipping build import with status '${result.name}'")
Future.successful(result)
case _ =>
if (
userConfig().automaticImportBuild == AutoImportBuildKind.Initial || userConfig().automaticImportBuild == AutoImportBuildKind.All
) {
if (userConfig().shouldAutoImportNewProject) {
runUnconditionally(buildTool, isImportInProcess)
} else {
scribe.debug("Awaiting user response...")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ trait BuildTool {
*/
def buildServerName = executableName

def possibleBuildServerNames: List[String] = List(buildServerName)

def isBspGenerated(workspace: AbsolutePath): Boolean =
possibleBuildServerNames
.map(name => workspace.resolve(".bsp").resolve(s"$name.json"))
.exists(_.isFile)
}

object BuildTool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,13 @@ class ScalaCliBuildTool(

override val forcesBuildServer = true

def isBspGenerated(workspace: AbsolutePath): Boolean =
ScalaCliBuildTool.pathsToScalaCliBsp(workspace).exists(_.isFile)
override def possibleBuildServerNames = ScalaCli.names.toList

}

object ScalaCliBuildTool {
def name = "scala-cli"

def pathsToScalaCliBsp(root: AbsolutePath): List[AbsolutePath] =
ScalaCli.names.toList.map(name =>
root.resolve(".bsp").resolve(s"$name.json")
Expand Down
25 changes: 25 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 @@ -189,6 +189,31 @@ object Messages {
}
}

object GenerateBspAndConnect {
def yes = new MessageActionItem("Connect")

def notNow: MessageActionItem = Messages.notNow

def params(
buildToolName: String,
buildServerName: String,
): ShowMessageRequestParams = {
val params = new ShowMessageRequestParams()
params.setMessage(
s"New $buildToolName workspace detected, would you like connect to $buildServerName build server?"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
s"New $buildToolName workspace detected, would you like connect to $buildServerName build server?"
s"New $buildToolName workspace detected, would you like connect to the $buildServerName build server?"

Nit

)
params.setType(MessageType.Info)
params.setActions(
List(
yes,
notNow,
dontShowAgain,
).asJava
)
params
}
}

object MainClass {
val message = "Multiple main classes found. Which would you like to run?"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2091,23 +2091,17 @@ class MetalsLspService(
buildTool: Option[BuildTool.Found],
chosenBuildServer: Option[String],
): Future[BuildChange] = {
val isBloopOrEmpty = chosenBuildServer.isEmpty || chosenBuildServer.exists(
def isBloopOrEmpty = chosenBuildServer.exists(
_ == BloopServers.name
)
) || chosenBuildServer.isEmpty

def useBuildToolBsp(buildTool: BuildServerProvider) =
buildTool match {
case _: BloopInstallProvider => userConfig.defaultBSPToBuildTool
case _ => true
}

buildTool match {
case Some(BuildTool.Found(buildTool: BloopInstallProvider, digest))
if isBloopOrEmpty =>
slowConnectToBloopServer(forceImport, buildTool, digest)
case Some(BuildTool.Found(buildTool: ScalaCliBuildTool, _))
if !buildTool.isBspGenerated(folder) =>
tables.buildServers.chooseServer(buildTool.buildServerName)
buildTool
.generateBspConfig(
folder,
args => bspConfigGenerator.runUnconditionally(buildTool, args),
statusBar,
)
.flatMap(_ => quickConnectToBuildServer())
// If there is no .bazelbsp present, we ask user to write bsp config
// After that, we should fall into the last case and index workspace
case Some(BuildTool.Found(_: BazelBuildTool, _))
Expand All @@ -2122,12 +2116,22 @@ class MetalsLspService(
forceImport,
)
.flatMap(_ => quickConnectToBuildServer())
case Some(BuildTool.Found(buildTool: BuildServerProvider, _))
if chosenBuildServer.isEmpty && useBuildToolBsp(buildTool) =>
slowConnectToBuildToolBsp(buildTool)
case Some(BuildTool.Found(buildTool: BloopInstallProvider, digest))
if isBloopOrEmpty =>
slowConnectToBloopServer(forceImport, buildTool, digest)
case Some(BuildTool.Found(buildTool, _))
if !chosenBuildServer.exists(
_ == buildTool.buildServerName
) && buildTool.forcesBuildServer =>
tables.buildServers.chooseServer(buildTool.buildServerName)
quickConnectToBuildServer()
case Some(BuildTool.Found(buildTool: BuildServerProvider, _))
if chosenBuildServer.contains(buildTool.buildServerName) && !buildTool
.isBspGenerated(folder) =>
generateBspAndConnect(buildTool)
case Some(found) =>
indexer.reloadWorkspaceAndIndex(
forceImport,
Expand All @@ -2141,6 +2145,51 @@ class MetalsLspService(
}
}

private def slowConnectToBuildToolBsp(
buildTool: BuildServerProvider
) = {
val notification = tables.dismissedNotifications.ImportChanges
if (buildTool.isBspGenerated(folder)) {
tables.buildServers.chooseServer(buildTool.buildServerName)
quickConnectToBuildServer()
} else if (
userConfig.shouldAutoImportNewProject || buildTool.forcesBuildServer
) {
generateBspAndConnect(buildTool)
} else if (notification.isDismissed) {
Future.successful(BuildChange.None)
} else {
scribe.debug("Awaiting user response...")
languageClient
.showMessageRequest(
Messages.GenerateBspAndConnect
.params(buildTool.executableName, buildTool.buildServerName)
)
.asScala
.flatMap { item =>
if (item == Messages.dontShowAgain) {
notification.dismissForever()
Future.successful(BuildChange.None)
} else if (item == Messages.GenerateBspAndConnect.yes) {
generateBspAndConnect(buildTool)
} else Future.successful(BuildChange.None)
}
}
}

private def generateBspAndConnect(
buildTool: BuildServerProvider
): Future[BuildChange] = {
tables.buildServers.chooseServer(buildTool.buildServerName)
buildTool
.generateBspConfig(
folder,
args => bspConfigGenerator.runUnconditionally(buildTool, args),
statusBar,
)
.flatMap(_ => quickConnectToBuildServer())
}

/**
* If there is no auto-connectable build server and no supported build tool is found
* we assume it's a scala-cli project.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ case class UserConfiguration(
verboseCompilation: Boolean = false,
automaticImportBuild: AutoImportBuildKind = AutoImportBuildKind.Off,
scalaCliLauncher: Option[String] = None,
defaultBSPToBuildTool: Boolean = false,
) {

def shouldAutoImportNewProject: Boolean =
automaticImportBuild != AutoImportBuildKind.Off

def currentBloopVersion: String =
bloopVersion.getOrElse(BuildInfo.bloopVersion)

Expand Down Expand Up @@ -349,6 +353,15 @@ object UserConfiguration {
|only automatically import a build when a project is first opened, "all" will automate
|build imports after subsequent changes as well.""".stripMargin,
),
UserConfigurationOption(
"default-bsp-to-build-tool",
"false",
"true",
"If used build server should default to build tool.",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"If used build server should default to build tool.",
"Default to using build tool as your build server.",

"""|If used build server should default to the one provided by the build tool
|instead of the default Bloop.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""|If used build server should default to the one provided by the build tool
|instead of the default Bloop.
"""|If your build tool can also serve as a build server, default to using it instead of Bloop.

|""".stripMargin,
),
)

def fromJson(
Expand Down Expand Up @@ -570,6 +583,11 @@ object UserConfiguration {
case _ => AutoImportBuildKind.Off
}

val scalaCliLauncher = getStringKey("scala-cli-launcher")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this because it seems to me it should be there but maybe there was some great scheme behind not adding this that I'm not aware of.


val defaultBSPToBuildTool =
getBooleanKey("default-bsp-to-build-tool").getOrElse(false)

if (errors.isEmpty) {
Right(
UserConfiguration(
Expand Down Expand Up @@ -602,6 +620,8 @@ object UserConfiguration {
customProjectRoot,
verboseCompilation,
autoImportBuilds,
scalaCliLauncher,
defaultBSPToBuildTool,
)
)
} else {
Expand Down
80 changes: 80 additions & 0 deletions tests/slow/src/test/scala/tests/PreferredBuildServer.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package tests

import scala.meta.internal.metals.Messages
import scala.meta.internal.metals.UserConfiguration
import scala.meta.internal.metals.{BuildInfo => V}

class PreferredBuildServer extends BaseLspSuite("preferred-build-server") {
override def userConfig: UserConfiguration =
super.userConfig.copy(defaultBSPToBuildTool = true)

test("start-sbt-when-preferred-no-bsp") {
cleanWorkspace()

val importMessage =
Messages.GenerateBspAndConnect.params("sbt", "sbt").getMessage()

client.showMessageRequestHandler = msg => {
if (msg.getMessage() == importMessage)
Some(Messages.GenerateBspAndConnect.notNow)
else None
}

val fileLayout =
s"""|/project/build.properties
|sbt.version=${V.sbtVersion}
|/build.sbt
|${SbtBuildLayout.commonSbtSettings}
|ThisBuild / scalaVersion := "${V.scala213}"
|val a = project.in(file("a"))
|/a/src/main/scala/a/A.scala
|package a
|object A {
| val a = 1
|}
|""".stripMargin
FileLayout.fromString(fileLayout, workspace)

for {
_ <- server.initialize()
_ <- server.initialized()
_ = assertNoDiff(
client.workspaceMessageRequests,
importMessage,
)
_ = assert(server.server.bspSession.isEmpty)
} yield ()
}

test("start-sbt-when-preferred-with-bsp") {
cleanWorkspace()

val fileLayout =
s"""|/project/build.properties
|sbt.version=${V.sbtVersion}
|/build.sbt
|${SbtBuildLayout.commonSbtSettings}
|ThisBuild / scalaVersion := "${V.scala213}"
|val a = project.in(file("a"))
|/a/src/main/scala/a/A.scala
|package a
|object A {
| val a = 1
|}
|""".stripMargin

FileLayout.fromString(fileLayout, workspace)
SbtServerInitializer.generateBspConfig(workspace, V.sbtVersion)

for {
_ <- server.initialize()
_ <- server.initialized()
_ <- server.server.buildServerPromise.future
_ = assertNoDiff(
server.server.tables.buildServers.selectedServer().get,
"sbt",
)
_ = assert(server.server.bspSession.exists(_.main.isSbt))
} yield ()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ class ScalaCliSuite extends BaseScalaCliSuite(V.scala3) {
}

test("inner") {
cleanWorkspace()
for {
_ <- scalaCliInitialize(useBsp = false)(
s"""|/inner/project.scala
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ object SbtServerInitializer extends BuildServerInitializer {
}
}

private def generateBspConfig(
def generateBspConfig(
workspace: AbsolutePath,
sbtVersion: String,
): Unit = {
Expand Down
Loading