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: queue or cancel previous connect request #6691

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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 @@ -10,6 +10,7 @@ import scala.util.control.NonFatal
import scala.meta.internal.bsp.BspConfigGenerationStatus._
import scala.meta.internal.builds.BuildServerProvider
import scala.meta.internal.builds.ShellRunner
import scala.meta.internal.metals.CancelableFuture
import scala.meta.internal.metals.Directories
import scala.meta.internal.metals.Messages.BspProvider
import scala.meta.internal.metals.MetalsEnrichments._
Expand All @@ -31,7 +32,7 @@ final class BspConfigGenerator(
def runUnconditionally(
buildTool: BuildServerProvider,
args: List[String],
): Future[BspConfigGenerationStatus] =
): CancelableFuture[BspConfigGenerationStatus] =
shellRunner
.run(
s"${buildTool.buildServerName} bspConfig",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ class BspConnector(
args => bspConfigGenerator.runUnconditionally(bsp, args),
statusBar,
)
.future
.flatMap { _ =>
connect(
projectRoot,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ object BuildChange {
case object Failed extends BuildChange
case object Reconnected extends BuildChange
case object Reloaded extends BuildChange
case object Cancelled extends BuildChange
}
68 changes: 31 additions & 37 deletions metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package scala.meta.internal.builds

import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicBoolean

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

import scala.meta.internal.builds.Digest.Status
import scala.meta.internal.metals.BuildInfo
import scala.meta.internal.metals.CancelSwitch
import scala.meta.internal.metals.CancelableFuture
import scala.meta.internal.metals.Confirmation
import scala.meta.internal.metals.Interruptable
import scala.meta.internal.metals.Interruptable._
import scala.meta.internal.metals.Messages._
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.Tables
Expand Down Expand Up @@ -37,40 +41,30 @@ final class BloopInstall(
override def toString: String = s"BloopInstall($workspace)"

def runUnconditionally(
buildTool: BloopInstallProvider,
isImportInProcess: AtomicBoolean,
): Future[WorkspaceLoadedStatus] = {
if (isImportInProcess.compareAndSet(false, true)) {
buildTool.bloopInstall(
workspace,
args => {
scribe.info(s"running '${args.mkString(" ")}'")
val process =
runArgumentsUnconditionally(buildTool, args, userConfig().javaHome)
process.foreach { e =>
if (e.isFailed) {
// Record the exact command that failed to help troubleshooting.
scribe.error(s"$buildTool command failed: ${args.mkString(" ")}")
}
buildTool: BloopInstallProvider
): CancelableFuture[WorkspaceLoadedStatus] = {
buildTool.bloopInstall(
workspace,
args => {
scribe.info(s"running '${args.mkString(" ")}'")
val process =
runArgumentsUnconditionally(buildTool, args, userConfig().javaHome)
process.future.foreach { e =>
if (e.isFailed) {
// Record the exact command that failed to help troubleshooting.
scribe.error(s"$buildTool command failed: ${args.mkString(" ")}")
}
process.onComplete(_ => isImportInProcess.set(false))
process
},
)
} else {
Future
.successful {
languageClient.showMessage(ImportAlreadyRunning)
WorkspaceLoadedStatus.Dismissed
}
}
process
},
)
}

private def runArgumentsUnconditionally(
buildTool: BloopInstallProvider,
args: List[String],
javaHome: Option[String],
): Future[WorkspaceLoadedStatus] = {
): CancelableFuture[WorkspaceLoadedStatus] = {
persistChecksumStatus(Status.Started, buildTool)
val processFuture = shellRunner
.run(
Expand All @@ -92,7 +86,7 @@ final class BloopInstall(
case ExitCodes.Cancel => WorkspaceLoadedStatus.Cancelled
case result => WorkspaceLoadedStatus.Failed(result)
}
processFuture.foreach { result =>
processFuture.future.foreach { result =>
try result.toChecksumStatus.foreach(persistChecksumStatus(_, buildTool))
catch {
case _: InterruptedException =>
Expand Down Expand Up @@ -123,36 +117,36 @@ final class BloopInstall(
def runIfApproved(
buildTool: BloopInstallProvider,
digest: String,
isImportInProcess: AtomicBoolean,
): Future[WorkspaceLoadedStatus] =
): CancelableFuture[WorkspaceLoadedStatus] =
synchronized {
oldInstallResult(digest) match {
case Some(result)
if result != WorkspaceLoadedStatus.Duplicate(Status.Requested) =>
scribe.info(s"skipping build import with status '${result.name}'")
Future.successful(result)
CancelableFuture.successful(result)
case _ =>
if (userConfig().shouldAutoImportNewProject) {
runUnconditionally(buildTool, isImportInProcess)
runUnconditionally(buildTool)
} else {
scribe.debug("Awaiting user response...")
for {
implicit val cancelSwitch = CancelSwitch(Promise[Unit]())
(for {
userResponse <- requestImport(
buildTools,
buildTool,
languageClient,
digest,
)
).withInterrupt
installResult <- {
if (userResponse.isYes) {
runUnconditionally(buildTool, isImportInProcess)
runUnconditionally(buildTool).withInterrupt
} else {
// Don't spam the user with requests during rapid build changes.
notification.dismiss(2, TimeUnit.MINUTES)
Future.successful(WorkspaceLoadedStatus.Rejected)
Interruptable.successful(WorkspaceLoadedStatus.Rejected)
}
}
} yield installResult
} yield installResult).toCancellable
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ package scala.meta.internal.builds
import java.io.IOException
import java.nio.file.Files

import scala.concurrent.Future

import scala.meta.internal.metals.CancelableFuture
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.io.AbsolutePath

Expand All @@ -18,8 +17,8 @@ trait BloopInstallProvider extends BuildTool {
*/
def bloopInstall(
workspace: AbsolutePath,
systemProcess: List[String] => Future[WorkspaceLoadedStatus],
): Future[WorkspaceLoadedStatus] = {
systemProcess: List[String] => CancelableFuture[WorkspaceLoadedStatus],
): CancelableFuture[WorkspaceLoadedStatus] = {
cleanupStaleConfig()
systemProcess(bloopInstallArgs(workspace))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import scala.annotation.nowarn
import scala.concurrent.Future

import scala.meta.internal.bsp.BspConfigGenerationStatus._
import scala.meta.internal.metals.CancelableFuture
import scala.meta.internal.metals.Messages
import scala.meta.internal.metals.StatusBar
import scala.meta.io.AbsolutePath
Expand All @@ -20,12 +21,16 @@ trait BuildServerProvider extends BuildTool {
@nowarn("msg=parameter statusBar in method generateBspConfig is never used")
def generateBspConfig(
workspace: AbsolutePath,
systemProcess: List[String] => Future[BspConfigGenerationStatus],
systemProcess: List[String] => CancelableFuture[
BspConfigGenerationStatus
],
statusBar: StatusBar,
): Future[BspConfigGenerationStatus] =
): CancelableFuture[BspConfigGenerationStatus] =
createBspFileArgs(workspace).map(systemProcess).getOrElse {
Future.successful(
Failed(Right(Messages.NoBspSupport.toString()))
CancelableFuture(
Future.successful(
Failed(Right(Messages.NoBspSupport.toString()))
)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class NewProjectProvider(
javaHome,
javaOptsMap = javaOpts,
)
.future
.flatMap {
case ExitCodes.Success =>
askForWindow(projectPath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ case class SbtBuildTool(

def shutdownBspServer(
shellRunner: ShellRunner
): Future[Int] = {
): CancelableFuture[Int] = {
val shutdownArgs =
composeArgs(List("--client", "shutdown"), projectRoot, projectRoot.toNIO)
scribe.info(s"running ${shutdownArgs.mkString(" ")}")
Expand Down Expand Up @@ -242,7 +242,7 @@ case class SbtBuildTool(
if (promise.isCompleted) {
// executes when user chooses `restart` after the timeout
restartSbtBuildServer()
} else shutdownBspServer(shellRunner).ignoreValue
} else shutdownBspServer(shellRunner).future.ignoreValue
case _ =>
promise.trySuccess(())
Future.successful(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ package scala.meta.internal.builds

import java.security.MessageDigest

import scala.concurrent.Future

import scala.meta.internal.bsp.BspConfigGenerationStatus._
import scala.meta.internal.metals.BuildInfo
import scala.meta.internal.metals.CancelableFuture
import scala.meta.internal.metals.Directories
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.StatusBar
Expand All @@ -26,9 +25,11 @@ case class ScalaCliBuildTool(

override def generateBspConfig(
workspace: AbsolutePath,
systemProcess: List[String] => Future[BspConfigGenerationStatus],
systemProcess: List[String] => CancelableFuture[
BspConfigGenerationStatus
],
statusBar: StatusBar,
): Future[BspConfigGenerationStatus] =
): CancelableFuture[BspConfigGenerationStatus] =
createBspFileArgs(workspace).map(systemProcess).getOrElse {
// fallback to creating `.bsp/scala-cli.json` that starts JVM launcher
val bspConfig =
Expand All @@ -37,7 +38,7 @@ case class ScalaCliBuildTool(
bspConfig.writeText(
ScalaCli.scalaCliBspJsonContent(projectRoot = projectRoot.toString())
)
Future.successful(Generated)
CancelableFuture.successful(Generated)
}

override def createBspFileArgs(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import java.io.File

import scala.concurrent.Await
import scala.concurrent.ExecutionContext
import scala.concurrent.Future
import scala.concurrent.Promise
import scala.concurrent.duration.DurationInt
import scala.language.postfixOps
import scala.util.Properties

import scala.meta.internal.metals.Cancelable
import scala.meta.internal.metals.CancelableFuture
import scala.meta.internal.metals.JavaBinary
import scala.meta.internal.metals.JdkSources
import scala.meta.internal.metals.MetalsEnrichments._
Expand Down Expand Up @@ -45,7 +45,7 @@ class ShellRunner(time: Time, workDoneProvider: WorkDoneProgress)(implicit
processErr: String => Unit = scribe.error(_),
propagateError: Boolean = false,
javaOptsMap: Map[String, String] = Map.empty,
): Future[Int] = {
): CancelableFuture[Int] = {

val classpathSeparator = if (Properties.isWin) ";" else ":"
val classpath = Fetch
Expand Down Expand Up @@ -92,7 +92,7 @@ class ShellRunner(time: Time, workDoneProvider: WorkDoneProgress)(implicit
processErr: String => Unit = scribe.error(_),
propagateError: Boolean = false,
logInfo: Boolean = true,
): Future[Int] = {
): CancelableFuture[Int] = {
val elapsed = new Timer(time)
val env = additionalEnv ++ JdkSources.envVariables(javaHome)
val ps = SystemProcess.run(
Expand Down Expand Up @@ -125,7 +125,7 @@ class ShellRunner(time: Time, workDoneProvider: WorkDoneProgress)(implicit
result.trySuccess(code)
}
result.future.onComplete(_ => cancelables.remove(newCancelable))
result.future
CancelableFuture(result.future, newCancelable)
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import scala.concurrent.ExecutionContext
import scala.concurrent.Future
import scala.util.Try

case class CancelableFuture[T](
case class CancelableFuture[+T](
future: Future[T],
cancelable: Cancelable = Cancelable.empty,
) extends Cancelable {
Expand Down
Loading
Loading