diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConfigGenerator.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConfigGenerator.scala index 8f083a8ba8e..6e8ffe082d3 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConfigGenerator.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConfigGenerator.scala @@ -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._ @@ -31,7 +32,7 @@ final class BspConfigGenerator( def runUnconditionally( buildTool: BuildServerProvider, args: List[String], - ): Future[BspConfigGenerationStatus] = + ): CancelableFuture[BspConfigGenerationStatus] = shellRunner .run( s"${buildTool.buildServerName} bspConfig", diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index f321e141098..0d0a73d5348 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -159,6 +159,7 @@ class BspConnector( args => bspConfigGenerator.runUnconditionally(bsp, args), statusBar, ) + .future .flatMap { _ => connect( projectRoot, diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BuildChange.scala b/metals/src/main/scala/scala/meta/internal/bsp/BuildChange.scala index 79c5cf949a6..72cde8f50b7 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BuildChange.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BuildChange.scala @@ -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 } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala b/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala index 44fed0a0699..a3b17f3991a 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala @@ -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 @@ -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( @@ -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 => @@ -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 } } } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala index fa3731e6ca4..d3fa5c0e4c7 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala @@ -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 @@ -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)) } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildServerProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildServerProvider.scala index c47fcb04aff..aa45eda0760 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildServerProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildServerProvider.scala @@ -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 @@ -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())) + ) ) } diff --git a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala index ad0dcb6fdbe..cc40a5a6378 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala @@ -134,6 +134,7 @@ class NewProjectProvider( javaHome, javaOptsMap = javaOpts, ) + .future .flatMap { case ExitCodes.Success => askForWindow(projectPath) diff --git a/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala index d36a4f7b52c..a689db6d2a7 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala @@ -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(" ")}") @@ -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(()) diff --git a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala index b5ad6d1a546..632f47c0d39 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala @@ -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 @@ -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 = @@ -37,7 +38,7 @@ case class ScalaCliBuildTool( bspConfig.writeText( ScalaCli.scalaCliBspJsonContent(projectRoot = projectRoot.toString()) ) - Future.successful(Generated) + CancelableFuture.successful(Generated) } override def createBspFileArgs( diff --git a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala index 2e180f42d08..befa53d2708 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala @@ -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._ @@ -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 @@ -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( @@ -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) } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/CancelableFuture.scala b/metals/src/main/scala/scala/meta/internal/metals/CancelableFuture.scala index 984437b8f77..bd50d31c3b1 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/CancelableFuture.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/CancelableFuture.scala @@ -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 { diff --git a/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala index 9bce8c88c4e..6c5494e0b7a 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala @@ -1,12 +1,14 @@ package scala.meta.internal.metals import java.nio.charset.Charset +import java.util.concurrent.ConcurrentLinkedQueue import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean import scala.concurrent.ExecutionContextExecutorService import scala.concurrent.Future import scala.concurrent.Promise +import scala.util.Failure import scala.util.control.NonFatal import scala.meta.internal.bsp @@ -25,6 +27,7 @@ import scala.meta.internal.builds.Digest.Status import scala.meta.internal.builds.SbtBuildTool import scala.meta.internal.builds.ScalaCliBuildTool import scala.meta.internal.builds.ShellRunner +import scala.meta.internal.metals.Interruptable._ import scala.meta.internal.metals.Messages.IncompatibleBloopVersion import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.doctor.Doctor @@ -103,9 +106,9 @@ class ConnectionProvider( var buildServerPromise: Promise[Unit] = Promise[Unit]() val isConnecting = new AtomicBoolean(false) - override def index(check: () => Unit): Future[Unit] = connect( - Index(check) - ).ignoreValue + override def index(check: () => Unit): Future[Unit] = + connect(Index(check)).ignoreValue + override def cancel(): Unit = { cancelables.cancel() } @@ -296,47 +299,114 @@ class ConnectionProvider( } object Connect { + class RequestInfo(val request: ConnectRequest) { + val promise: Promise[BuildChange] = Promise() + val cancelPromise: Promise[Unit] = Promise() + def cancel(): Boolean = cancelPromise.trySuccess(()) + } + + @volatile private var currentRequest: Option[RequestInfo] = None + private val queue = new ConcurrentLinkedQueue[RequestInfo]() + + def getOngoingRequest(): Option[RequestInfo] = currentRequest + def connect[T](request: ConnectRequest): Future[BuildChange] = { - request match { - case Disconnect(shutdownBuildServer) => disconnect(shutdownBuildServer) - case Index(check) => index(check) - case ImportBuildAndIndex(session) => importBuildAndIndex(session) - case ConnectToSession(session) => connectToSession(session) - case CreateSession(shutdownBuildServer) => - createSession(shutdownBuildServer) - case GenerateBspConfigAndConnect(buildTool, shutdownServer) => - generateBspConfigAndConnect(buildTool, shutdownServer) - case BloopInstallAndConnect( - buildTool, - checksum, - forceImport, - shutdownServer, - ) => - bloopInstallAndConnect( - buildTool, - checksum, - forceImport, - shutdownServer, - ) + val info = addToQueue(request) + pollAndConnect() + info.promise.future + } + + private def addToQueue(request: ConnectRequest): RequestInfo = + synchronized { + val info = new RequestInfo(request) + val iter = queue.iterator() + while (iter.hasNext()) { + val curr = iter.next() + request.cancelCompare(curr.request) match { + case TakeOver => curr.cancel() + case Yield => info.cancel() + case _ => + } + } + queue.add(info) + // maybe cancel ongoing + currentRequest.foreach(ongoing => + if (request.cancelCompare(ongoing.request) == TakeOver) + ongoing.cancel() + ) + info + } + + private def pollAndConnect(): Unit = { + val optRequest = synchronized { + if (currentRequest.isEmpty) { + currentRequest = Option(queue.poll()) + currentRequest + } else None + } + + for (request <- optRequest) { + implicit val cancelPromise = CancelSwitch(request.cancelPromise) + val result = + if (request.cancelPromise.isCompleted) + Interruptable.successful(BuildChange.Cancelled) + else + request.request match { + case Disconnect(shutdownBuildServer) => + disconnect(shutdownBuildServer) + case Index(check) => index(check) + case ImportBuildAndIndex(session) => + importBuildAndIndex(session) + case ConnectToSession(session) => + connectToSession(session) + case CreateSession(shutdownBuildServer) => + createSession(shutdownBuildServer) + case GenerateBspConfigAndConnect(buildTool, shutdownServer) => + generateBspConfigAndConnect( + buildTool, + shutdownServer, + ) + case BloopInstallAndConnect( + buildTool, + checksum, + forceImport, + shutdownServer, + ) => + bloopInstallAndConnect( + buildTool, + checksum, + forceImport, + shutdownServer, + ) + } + result.future.onComplete { res => + res match { + case Failure(CancelConnectException) => + request.promise.trySuccess(BuildChange.Cancelled) + case _ => request.promise.tryComplete(res) + } + currentRequest = None + pollAndConnect() + } } } private def disconnect( shutdownBuildServer: Boolean - ): Future[BuildChange] = { - def shutdownBsp(optMainBsp: Option[String]): Future[Boolean] = { + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { + def shutdownBsp(optMainBsp: Option[String]): Interruptable[Boolean] = { optMainBsp match { case Some(BloopServers.name) => - Future { bloopServers.shutdownServer() } + Interruptable.successful { bloopServers.shutdownServer() } case Some(SbtBuildTool.name) => for { res <- buildToolProvider.buildTool match { case Some(sbt: SbtBuildTool) => - sbt.shutdownBspServer(shellRunner).map(_ == 0) - case _ => Future.successful(false) + sbt.shutdownBspServer(shellRunner).withInterrupt.map(_ == 0) + case _ => Interruptable.successful(false) } } yield res - case s => Future.successful(s.nonEmpty) + case s => Interruptable.successful(s.nonEmpty) } } @@ -348,34 +418,38 @@ class ConnectionProvider( ) for { - _ <- scalaCli.stop() - optMainBsp <- bspSession match { + _ <- scalaCli.stop(storeLast = true).withInterrupt + optMainBsp <- (bspSession match { case None => Future.successful(None) case Some(session) => bspSession = None mainBuildTargetsData.resetConnections(List.empty) session.shutdown().map(_ => Some(session.main.name)) - } + }).withInterrupt _ <- if (shutdownBuildServer) shutdownBsp(optMainBsp) - else Future.successful(()) + else Interruptable.successful(()) } yield BuildChange.None } - private def index(check: () => Unit): Future[BuildChange] = - profiledIndexWorkspace(check).map(_ => BuildChange.None) + private def index(check: () => Unit): Interruptable[BuildChange] = + profiledIndexWorkspace(check) + .map(_ => BuildChange.None) + .withInterrupt private def importBuildAndIndex( session: BspSession - ): Future[BuildChange] = { + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { val importedBuilds0 = timerProvider.timed("Imported build") { session.importBuilds() } for { - bspBuilds <- workDoneProgress.trackFuture( - Messages.importingBuild, - importedBuilds0, - ) + bspBuilds <- workDoneProgress + .trackFuture( + Messages.importingBuild, + importedBuilds0, + ) + .withInterrupt _ = { val idToConnection = bspBuilds.flatMap { bspBuild => val targets = @@ -408,7 +482,9 @@ class ConnectionProvider( DelegateSetting.writeProjectRef(folder, projectRefs) } - private def connectToSession(session: BspSession): Future[BuildChange] = { + private def connectToSession( + session: BspSession + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { scribe.info( s"Connected to Build server: ${session.main.name} v${session.version}" ) @@ -450,7 +526,9 @@ class ConnectionProvider( } } - def createSession(shutdownServer: Boolean): Future[BuildChange] = { + def createSession( + shutdownServer: Boolean + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { def compileAllOpenFiles: BuildChange => Future[BuildChange] = { case change if !change.isFailed => Future @@ -465,22 +543,22 @@ class ConnectionProvider( case other => Future.successful(other) } - val scalaCliPaths = scalaCli.paths - isConnecting.set(true) (for { _ <- disconnect(shutdownServer) - maybeSession <- timerProvider.timed( - "Connected to build server", - true, - ) { - bspConnector.connect( - buildToolProvider.buildTool, - folder, - () => userConfig, - shellRunner, - ) - } + maybeSession <- timerProvider + .timed( + "Connected to build server", + true, + ) { + bspConnector.connect( + buildToolProvider.buildTool, + folder, + () => userConfig, + shellRunner, + ) + } + .withInterrupt result <- maybeSession match { case Some(session) => val result = connectToSession(session) @@ -492,15 +570,13 @@ class ConnectionProvider( } result case None => - Future.successful(BuildChange.None) + Interruptable.successful(BuildChange.None) } - _ <- Future.sequence( - scalaCliPaths - .collect { - case path if (!buildTargets.belongsToBuildTarget(path.toNIO)) => - scalaCli.start(path) - } - ) + _ <- scalaCli + .startForAllLastPaths(path => + !buildTargets.belongsToBuildTarget(path.toNIO) + ) + .withInterrupt _ = initTreeView() } yield result) .recover { case NonFatal(e) => @@ -514,7 +590,7 @@ class ConnectionProvider( scribe.error(message, e) BuildChange.Failed } - .flatMap(compileAllOpenFiles) + .flatMap(compileAllOpenFiles(_).withInterrupt) .map { res => buildServerPromise.trySuccess(()) res @@ -524,23 +600,24 @@ class ConnectionProvider( private def generateBspConfigAndConnect( buildTool: BuildServerProvider, shutdownServer: Boolean, - ): Future[BuildChange] = { + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { tables.buildTool.chooseBuildTool(buildTool.executableName) maybeChooseServer(buildTool.buildServerName, alreadySelected = false) for { _ <- if (shutdownServer) disconnect(shutdownServer) - else Future.unit + else Interruptable.successful(()) status <- buildTool .generateBspConfig( folder, args => bspConfigGenerator.runUnconditionally(buildTool, args), statusBar, ) + .withInterrupt shouldConnect = handleGenerationStatus(buildTool, status) status <- if (shouldConnect) createSession(false) - else Future.successful(BuildChange.Failed) + else Interruptable.successful(BuildChange.Failed) } yield status } @@ -575,28 +652,24 @@ class ConnectionProvider( false } - val isImportInProcess = new AtomicBoolean(false) - private def bloopInstallAndConnect( buildTool: BloopInstallProvider, checksum: String, forceImport: Boolean, shutdownServer: Boolean, - ): Future[BuildChange] = { + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { for { result <- { if (forceImport) bloopInstall.runUnconditionally( - buildTool, - isImportInProcess, + buildTool ) else bloopInstall.runIfApproved( buildTool, checksum, - isImportInProcess, ) - } + }.withInterrupt change <- { if (result.isInstalled) createSession(shutdownServer) else if (result.isFailed) { @@ -617,10 +690,10 @@ class ConnectionProvider( createSession(shutdownServer) } else { languageClient.showMessage(Messages.ImportProjectFailed) - Future.successful(BuildChange.Failed) + Interruptable.successful(BuildChange.Failed) } } yield change - } else Future.successful(BuildChange.None) + } else Interruptable.successful(BuildChange.None) } } yield change } @@ -630,21 +703,84 @@ class ConnectionProvider( sealed trait ConnectKind object SlowConnect extends ConnectKind -sealed trait ConnectRequest extends ConnectKind +sealed trait ConflictBehaviour +case object Yield extends ConflictBehaviour +case object TakeOver extends ConflictBehaviour +case object Queue extends ConflictBehaviour + +sealed trait ConnectRequest extends ConnectKind { + + /** + * Decides what to do with a new connect request + * in presence of an another ongoing/queued request. + * @param other the ongoing or queued request + * @return behavoiur of the incoming request + * Yield -- cancel this + * TakeOver -- cancel other + * Queue -- queue + */ + def cancelCompare(other: ConnectRequest): ConflictBehaviour +} -case class Disconnect(shutdownBuildServer: Boolean) extends ConnectRequest -case class Index(check: () => Unit) extends ConnectRequest -case class ImportBuildAndIndex(bspSession: BspSession) extends ConnectRequest -case class ConnectToSession(bspSession: BspSession) extends ConnectRequest +case class Disconnect(shutdownBuildServer: Boolean) extends ConnectRequest { + def cancelCompare(other: ConnectRequest): ConflictBehaviour = + other match { + case _: Index => Queue + case _ => Yield + } +} +case class Index(check: () => Unit) extends ConnectRequest { + def cancelCompare(other: ConnectRequest): ConflictBehaviour = + other match { + case _: Disconnect => Queue + case _ => Yield + } +} +case class ImportBuildAndIndex(bspSession: BspSession) extends ConnectRequest { + def cancelCompare(other: ConnectRequest): ConflictBehaviour = + other match { + case (_: Index) | (_: ImportBuildAndIndex) => TakeOver + case _: Disconnect => Queue + case _ => Yield + } +} +case class ConnectToSession(bspSession: BspSession) extends ConnectRequest { + def cancelCompare(other: ConnectRequest): ConflictBehaviour = + other match { + case (_: Disconnect) | (_: Index) | (_: ConnectToSession) => TakeOver + case _ => Yield + } +} case class CreateSession(shutdownBuildServer: Boolean = false) - extends ConnectRequest + extends ConnectRequest { + def cancelCompare(other: ConnectRequest): ConflictBehaviour = + other match { + case (_: Disconnect) | (_: Index) | (_: ConnectToSession) | CreateSession( + false + ) => + TakeOver + case _ => Yield + } +} case class GenerateBspConfigAndConnect( buildTool: BuildServerProvider, shutdownServer: Boolean = false, -) extends ConnectRequest +) extends ConnectRequest { + def cancelCompare(other: ConnectRequest): ConflictBehaviour = + other match { + case BloopInstallAndConnect(_, _, _, true) if !shutdownServer => Queue + case _ => TakeOver + } +} case class BloopInstallAndConnect( buildTool: BloopInstallProvider, checksum: String, forceImport: Boolean, shutdownServer: Boolean, -) extends ConnectRequest +) extends ConnectRequest { + def cancelCompare(other: ConnectRequest): ConflictBehaviour = + other match { + case GenerateBspConfigAndConnect(_, true) if !shutdownServer => Queue + case _ => TakeOver + } +} diff --git a/metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala index c24501c421d..a2d9d87e321 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala @@ -565,6 +565,7 @@ final class FileDecoderProvider( propagateError = true, logInfo = false, ) + .future .map(_ => { if (sbErr.nonEmpty) DecoderResponse.failed(path.toURI, sbErr.toString) @@ -675,6 +676,7 @@ final class FileDecoderProvider( else DecoderResponse.success(path.toURI, sbOut.toString) }) + .future } catch { case NonFatal(e) => scribe.error(e.toString()) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Interruptable.scala b/metals/src/main/scala/scala/meta/internal/metals/Interruptable.scala new file mode 100644 index 00000000000..862af05f359 --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/metals/Interruptable.scala @@ -0,0 +1,83 @@ +package scala.meta.internal.metals + +import scala.concurrent.ExecutionContext +import scala.concurrent.Future +import scala.concurrent.Promise + +import scala.meta.internal.metals.Interruptable.CancelConnectException + +class Interruptable[+T] private ( + futureIn: Future[T], + val cancelable: Cancelable, +) { + + def future(implicit + executor: ExecutionContext, + cancelPromise: CancelSwitch, + ): Future[T] = futureIn.map( + if (cancelPromise.promise.isCompleted) throw CancelConnectException else _ + ) + + def flatMap[S]( + f: T => Interruptable[S] + )(implicit + executor: ExecutionContext, + cancelPromise: CancelSwitch, + ): Interruptable[S] = { + val mutCancel = + cancelable match { + case c: MutableCancelable => c + case c => new MutableCancelable().add(c) + } + val newFuture = future.flatMap { res => + val i = f(res) + mutCancel.add(i.cancelable) + i.future + } + new Interruptable(newFuture, mutCancel) + } + + def map[S]( + f: T => S + )(implicit + executor: ExecutionContext, + cancelPromise: CancelSwitch, + ): Interruptable[S] = + new Interruptable(future.map(f(_)), cancelable) + + def recover[U >: T]( + pf: PartialFunction[Throwable, U] + )(implicit + executor: ExecutionContext, + cancelPromise: CancelSwitch, + ): Interruptable[U] = { + val pf0: PartialFunction[Throwable, U] = { case CancelConnectException => + throw CancelConnectException + } + new Interruptable(future.recover(pf0.orElse(pf)), cancelable) + } + + def toCancellable(implicit cancelPromise: CancelSwitch): CancelableFuture[T] = + CancelableFuture( + futureIn, + () => { cancelPromise.promise.trySuccess(()); cancelable.cancel() }, + ) +} + +object Interruptable { + def successful[T](result: T) = + new Interruptable(Future.successful(result), Cancelable.empty) + + object CancelConnectException extends RuntimeException + implicit class XtensionFuture[+T](future: Future[T]) { + def withInterrupt: Interruptable[T] = + new Interruptable(future, Cancelable.empty) + } + + implicit class XtensionCancelFuture[+T](future: CancelableFuture[T]) { + def withInterrupt: Interruptable[T] = + new Interruptable(future.future, future.cancelable) + } +} + +case class CancelSwitch(promise: Promise[Unit]) extends AnyVal diff --git a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala index b6e8ea9747a..bdaab7106da 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala @@ -105,10 +105,7 @@ object Messages { MessageType.Error, "Import project failed, no functionality will work. See the logs for more details", ) - val ImportAlreadyRunning = new MessageParams( - MessageType.Warning, - s"Import already running. \nPlease cancel the current import to run a new one.", - ) + val ImportProjectPartiallyFailed = new MessageParams( MessageType.Warning, "Import project partially failed, limited functionality may work in some parts of the workspace. " + diff --git a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCliServers.scala b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCliServers.scala index 9fd739544bd..268354f22a1 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCliServers.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCliServers.scala @@ -49,6 +49,9 @@ class ScalaCliServers( )(implicit ec: ExecutionContextExecutorService) extends Cancelable { + private val lastServerPaths = + new AtomicReference[Set[AbsolutePath]](Set.empty) + private def localTmpWorkspace(path: AbsolutePath) = { val root = if (path.isDirectory) path else path.parent root.resolve(s".metals-scala-cli/") @@ -128,6 +131,11 @@ class ScalaCliServers( def paths: Iterable[AbsolutePath] = servers.map(_.path) + def startForAllLastPaths(filter: AbsolutePath => Boolean): Future[Set[Unit]] = + Future.sequence( + lastServerPaths.getAndSet(Set.empty).withFilter(filter).map(start) + ) + def start(path: AbsolutePath): Future[Unit] = { val customWorkspace = if (path.isDirectory) None @@ -201,8 +209,11 @@ class ScalaCliServers( } yield () } - def stop(): Future[Unit] = { + def stop(storeLast: Boolean = false): Future[Unit] = { val servers = serversRef.getAndSet(Queue.empty) + if (storeLast) { + lastServerPaths.updateAndGet(_ ++ servers.map(_.path)) + } Future.sequence(servers.map(_.stop()).toSeq).ignoreValue } diff --git a/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala b/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala index 2f1fd0cb7f8..7a2fef9e181 100644 --- a/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala +++ b/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala @@ -295,16 +295,18 @@ class BazelLspSuite def jsonFile = workspace.resolve(Directories.bsp).resolve("bazelbsp.json").readText for { - _ <- shellRunner.runJava( - Dependency.of( - BazelBuildTool.dependency.getModule(), - "3.2.0-20240508-f3a81e7-NIGHTLY", - ), - BazelBuildTool.mainClass, - workspace, - BazelBuildTool.projectViewArgs(workspace), - None, - ) + _ <- shellRunner + .runJava( + Dependency.of( + BazelBuildTool.dependency.getModule(), + "3.2.0-20240508-f3a81e7-NIGHTLY", + ), + BazelBuildTool.mainClass, + workspace, + BazelBuildTool.projectViewArgs(workspace), + None, + ) + .future _ = assertContains(jsonFile, "3.2.0-20240508-f3a81e7-NIGHTLY") _ <- initialize( BazelBuildLayout(workspaceLayout, V.bazelScalaVersion, bazelVersion) diff --git a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala index 516fc0bc51f..c39565b4417 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala @@ -19,6 +19,7 @@ import scala.meta.io.AbsolutePath import com.google.gson.JsonObject import com.google.gson.JsonPrimitive +import org.eclipse.lsp4j.MessageActionItem import tests.BaseImportSuite import tests.JavaHomeChangeTest import tests.ScriptsAssertions @@ -213,18 +214,12 @@ class SbtBloopLspSuite _ = assertNoDiff( client.beginProgressMessages, List( + progressMessage, progressMessage, Messages.importingBuild, Messages.indexing, ).mkString("\n"), ) - _ = assertNoDiff( - client.workspaceShowMessages, - List( - ImportAlreadyRunning.getMessage() - ).mkString("\n"), - ) - } yield () } @@ -884,4 +879,38 @@ class SbtBloopLspSuite |""".stripMargin, ) + test("switch-build-server-while-connect") { + cleanWorkspace() + val layout = + s"""|/project/build.properties + |sbt.version=${V.sbtVersion} + |/build.sbt + |scalaVersion := "${V.scala213}" + |/src/main/scala/A.scala + | + |object A { + | val i: Int = "aaa" + |} + |""".stripMargin + writeLayout(layout) + client.importBuild = ImportBuild.yes + client.selectBspServer = { _ => new MessageActionItem("sbt") } + for { + _ <- server.initialize() + _ = server.initialized() + connectionProvider = server.headServer.connectionProvider.Connect + _ = while (connectionProvider.getOngoingRequest().isEmpty) { + // wait for connect to start + Thread.sleep(100) + } + bloopConnectF = connectionProvider.getOngoingRequest().get.promise.future + bspSwitchF = server.executeCommand(ServerCommands.BspSwitch) + _ <- bloopConnectF + _ = assert(!server.server.indexingPromise.isCompleted) + _ <- bspSwitchF + _ = assert(server.server.indexingPromise.isCompleted) + _ = assert(server.server.bspSession.exists(_.main.isSbt)) + } yield () + } + }