Skip to content

Send BSP onBuildTargetDidChange notifications to clients when the build changes #4957

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

Draft
wants to merge 9 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
2 changes: 1 addition & 1 deletion bsp/worker/src/mill/bsp/worker/BspEvaluators.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import mill.api.internal.{BaseModuleApi, BspModuleApi, EvaluatorApi, ModuleApi}

private class BspEvaluators(
workspaceDir: os.Path,
evaluators: Seq[EvaluatorApi],
val evaluators: Seq[EvaluatorApi],
debug: (() => String) => Unit
) {

Expand Down
26 changes: 15 additions & 11 deletions bsp/worker/src/mill/bsp/worker/BspWorkerImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@ import mill.bsp.Constants
import mill.api.{Result, SystemStreams}
import org.eclipse.lsp4j.jsonrpc.Launcher

import java.io.{PrintStream, PrintWriter}
import java.io.PrintWriter
import java.util.concurrent.Executors
import scala.concurrent.duration.Duration
import scala.concurrent.{Await, CancellationException, ExecutionContext, Promise}
import scala.concurrent.{CancellationException, ExecutionContext, Future, Promise}

object BspWorkerImpl {

Expand Down Expand Up @@ -50,14 +49,19 @@ object BspWorkerImpl {
lazy val listening = launcher.startListening()

val bspServerHandle = new BspServerHandle {
override def runSession(evaluators: Seq[EvaluatorApi]): BspServerResult = {
millServer.updateEvaluator(Option(evaluators))
millServer.sessionResult = None
while (millServer.sessionResult.isEmpty) Thread.sleep(1)
millServer.updateEvaluator(None)
val res = millServer.sessionResult.get
streams.err.println(s"Reload finished, result: $res")
res
override def startSession(
evaluators: Seq[EvaluatorApi],
errored: Boolean
): Future[BspServerResult] = {
// FIXME We might be losing some shutdown requests here
val sessionResultPromise = Promise[BspServerResult]()
millServer.sessionResult = sessionResultPromise
millServer.updateEvaluator(Some(evaluators), errored = errored)
sessionResultPromise.future
}

override def resetSession(): Unit = {
millServer.updateEvaluator(None, errored = false)
}

override def close(): Unit = {
Expand Down
93 changes: 85 additions & 8 deletions bsp/worker/src/mill/bsp/worker/MillBuildServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,98 @@ private class MillBuildServer(
// progresses through:
//
// Set when the client connects
protected var client: BuildClient = scala.compiletime.uninitialized
protected var client: BuildClient = null
Copy link
Member

Choose a reason for hiding this comment

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

Why is uninitialized not enough? Can we use a better type then, e.g. Option?

// Set when the `buildInitialize` message comes in
protected var sessionInfo: SessionInfo = scala.compiletime.uninitialized
// Set when the `MillBuildBootstrap` completes and the evaluators are available
private var bspEvaluators: Promise[BspEvaluators] = Promise[BspEvaluators]()
private var savedPreviousEvaluators = Option.empty[BspEvaluators]
// Set when a session is completed, either due to reload or shutdown
private[worker] var sessionResult: Option[BspServerResult] = None
private[worker] var sessionResult = Promise[BspServerResult]()

def initialized = sessionInfo != null

def updateEvaluator(evaluatorsOpt: Option[Seq[EvaluatorApi]]): Unit = {
def updateEvaluator(evaluatorsOpt: Option[Seq[EvaluatorApi]], errored: Boolean): Unit = {
Copy link
Member

@lefou lefou Apr 18, 2025

Choose a reason for hiding this comment

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

Can we have ScalaDoc explaining what error means.

debug(s"Updating Evaluator: $evaluatorsOpt")
val previousEvaluatorsOpt = bspEvaluators.future.value.flatMap(_.toOption)
.orElse(savedPreviousEvaluators)
if (bspEvaluators.isCompleted) bspEvaluators = Promise[BspEvaluators]() // replace the promise
evaluatorsOpt.foreach { evaluators =>
bspEvaluators.success(new BspEvaluators(topLevelProjectRoot, evaluators, s => debug(s())))
evaluatorsOpt match {
case None =>
savedPreviousEvaluators = previousEvaluatorsOpt
case Some(evaluators) =>
val updatedEvaluators =
if (errored)
previousEvaluatorsOpt.map(_.evaluators) match {
case Some(previous) =>
evaluators.headOption match {
case None => // ???
previous
case Some(headEvaluator) =>
val idx = previous.indexWhere(_.outPathJava == headEvaluator.outPathJava)
if (idx < 0) // ???
evaluators
else
previous.take(idx) ++ evaluators
}
case None => evaluators
}
else
evaluators
val bspEvaluators0 =
new BspEvaluators(topLevelProjectRoot, updatedEvaluators, s => debug(s()))
bspEvaluators.success(bspEvaluators0)
if (client != null) {
val newTargetIds = bspEvaluators0.bspModulesIdList.map {
case (id, (_, ev)) =>
id -> ev
}
val newTargetIdsMap = newTargetIds.toMap

val previousTargetIds = previousEvaluatorsOpt.map(_.bspModulesIdList).getOrElse(Nil).map {
case (id, (_, ev)) =>
id -> ev
}

val deleted0 = previousTargetIds.filterNot {
case (id, _) =>
newTargetIdsMap.contains(id)
}
val previousTargetIdsMap = previousTargetIds.toMap
val (modified0, created0) = newTargetIds.partition {
case (id, _) =>
previousTargetIdsMap.contains(id)
}

val deletedEvents = deleted0.map {
case (id, _) =>
val event = new bsp4j.BuildTargetEvent(id)
event.setKind(bsp4j.BuildTargetEventKind.DELETED)
event
}
val createdEvents = created0.map {
case (id, _) =>
val event = new bsp4j.BuildTargetEvent(id)
event.setKind(bsp4j.BuildTargetEventKind.CREATED)
event
}
val modifiedEvents = modified0
.filter {
case (id, ev) =>
!previousTargetIdsMap.get(id).contains(ev)
}
.map {
case (id, ev) =>
val event = new bsp4j.BuildTargetEvent(id)
event.setKind(bsp4j.BuildTargetEventKind.CHANGED)
event
}

val allEvents = deletedEvents ++ createdEvents ++ modifiedEvents

if (allEvents.nonEmpty)
client.onBuildTargetDidChange(new bsp4j.DidChangeBuildTarget(allEvents.asJava))
}
}
}

Expand All @@ -81,7 +158,7 @@ private class MillBuildServer(
val supportedLangs = Constants.languages.asJava
val capabilities = new BuildServerCapabilities

capabilities.setBuildTargetChangedProvider(false)
capabilities.setBuildTargetChangedProvider(true)
capabilities.setCanReload(canReload)
capabilities.setCompileProvider(new CompileProvider(supportedLangs))
capabilities.setDebugProvider(new DebugProvider(Seq().asJava))
Expand Down Expand Up @@ -139,7 +216,7 @@ private class MillBuildServer(
override def onBuildExit(): Unit = {
print("Entered onBuildExit")
SemanticDbJavaModuleApi.resetContext()
sessionResult = Some(BspServerResult.Shutdown)
sessionResult.trySuccess(BspServerResult.Shutdown)
onShutdown()
}

Expand Down Expand Up @@ -192,7 +269,7 @@ private class MillBuildServer(
handlerRaw(checkInitialized = false) {
// Instead stop and restart the command
// BSP.install(evaluator)
sessionResult = Some(BspServerResult.ReloadWorkspace)
sessionResult.trySuccess(BspServerResult.ReloadWorkspace)
().asInstanceOf[Object]
}

Expand Down
5 changes: 0 additions & 5 deletions core/api/src/mill/api/internal/BspContextApi.scala

This file was deleted.

9 changes: 6 additions & 3 deletions core/api/src/mill/api/internal/BspServerHandle.scala
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package mill.api.internal

import scala.concurrent.Future

/** With this server handle you can interact with a running Mill BSP server. */
trait BspServerHandle {

/**
* Runs a new session with the given evaluator. This one blocks until the session ends.
* @return The reason which the session ended, possibly indicating the wish for restart (e.g. in case of workspace reload).
* Starts a new session with the given evaluator. Doesn't block or wait for the session to end.
*/
def runSession(evaluators: Seq[EvaluatorApi]): BspServerResult
def startSession(evaluators: Seq[EvaluatorApi], errored: Boolean): Future[BspServerResult]

def resetSession(): Unit

/** Stops the BSP server. */
def close(): Unit
Expand Down
3 changes: 0 additions & 3 deletions core/api/src/mill/api/internal/BspServerResult.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,4 @@ object BspServerResult {

/** The session or the server ended successfully. */
object Shutdown extends BspServerResult

/** The session or the server ended with a failure. */
object Failure extends BspServerResult
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,21 +110,36 @@ object BspServerTestUtil {

def withBspServer[T](
workspacePath: os.Path,
millTestSuiteEnv: Map[String, String]
millTestSuiteEnv: Map[String, String],
client: b.BuildClient = DummyBuildClient,
millExecutableNoBspFile: Option[os.Path] = None
)(f: (MillBuildServer, b.InitializeBuildResult) => T): T = {

val bspMetadataFile = workspacePath / Constants.bspDir / s"${Constants.serverName}.json"
assert(os.exists(bspMetadataFile))
val contents = os.read(bspMetadataFile)
assert(
!contents.contains("--debug"),
contents.contains(s""""bspVersion":"$bsp4jVersion"""")
)

val outputOnErrorOnly = System.getenv("CI") != null

val contentsJson = ujson.read(contents)
val bspCommand = contentsJson("argv").arr.map(_.str)
val bspCommand = millExecutableNoBspFile match {
case None =>
val bspMetadataFile = workspacePath / Constants.bspDir / s"${Constants.serverName}.json"
assert(os.exists(bspMetadataFile))
val contents = os.read(bspMetadataFile)
assert(
!contents.contains("--debug"),
contents.contains(s""""bspVersion":"$bsp4jVersion"""")
)
val contentsJson = ujson.read(contents)
contentsJson("argv").arr.map(_.str)
case Some(millExecutableNoBspFile0) =>
Seq(
millExecutableNoBspFile0.toString,
"--bsp",
"--disable-ticker",
"--color",
"false",
"--jobs",
"1"
)
}

val stderr = new ByteArrayOutputStream
val proc = os.proc(bspCommand).spawn(
cwd = workspacePath,
Expand All @@ -137,8 +152,6 @@ object BspServerTestUtil {
env = millTestSuiteEnv
)

val client: b.BuildClient = DummyBuildClient

var success = false
try {
val launcher = new l.jsonrpc.Launcher.Builder[MillBuildServer]
Expand Down Expand Up @@ -211,4 +224,64 @@ object BspServerTestUtil {
coursierCache.toString -> "/coursier-cache",
millWorkspace.toString -> "/mill-workspace"
)

def scalaVersionNormalizedValues(): Seq[(String, String)] = {
val scala2Version = sys.props.getOrElse("TEST_SCALA_2_13_VERSION", ???)
val scala3Version = sys.props.getOrElse("MILL_SCALA_3_NEXT_VERSION", ???)
val scala2TransitiveSubstitutions = transitiveDependenciesSubstitutions(
coursierapi.Dependency.of(
"org.scala-lang",
"scala-compiler",
scala2Version
),
_.getModule.getOrganization != "org.scala-lang"
)
val scala3TransitiveSubstitutions = transitiveDependenciesSubstitutions(
coursierapi.Dependency.of(
"org.scala-lang",
"scala3-compiler_3",
scala3Version
),
_.getModule.getOrganization != "org.scala-lang"
)

scala2TransitiveSubstitutions ++ scala3TransitiveSubstitutions ++
Seq(
scala2Version -> "<scala-version>",
scala3Version -> "<scala3-version>"
)
}

def kotlinVersionNormalizedValues(): Seq[(String, String)] = {
val kotlinVersion = sys.props.getOrElse("TEST_KOTLIN_VERSION", ???)
val kotlinTransitiveSubstitutions = transitiveDependenciesSubstitutions(
coursierapi.Dependency.of(
"org.jetbrains.kotlin",
"kotlin-stdlib",
kotlinVersion
),
_.getModule.getOrganization != "org.jetbrains.kotlin"
)
kotlinTransitiveSubstitutions ++ Seq(kotlinVersion -> "<kotlin-version>")
}

def transitiveDependenciesSubstitutions(
dependency: coursierapi.Dependency,
filter: coursierapi.Dependency => Boolean
): Seq[(String, String)] = {
val fetchRes = coursierapi.Fetch.create()
.addDependencies(dependency)
.fetchResult()
fetchRes.getDependencies.asScala
.filter(filter)
.map { dep =>
val organization = dep.getModule.getOrganization
val name = dep.getModule.getName
val prefix = (organization.split('.') :+ name).mkString("/")
def basePath(version: String): String =
s"$prefix/$version/$name-$version"
basePath(dep.getVersion) -> basePath(s"<$name-version>")
}
.toSeq
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package milltests

object TheApp {
def main(args: Array[String]): Unit =
println(Lib.message)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package build

import mill.scalalib._

object thing extends ScalaModule {
def scalaVersion = Option(System.getenv("TEST_SCALA_2_13_VERSION")).getOrElse(???)
def mvnDeps = Seq(
mvn"com.lihaoyi::pprint:0.9.0"
)
}

object lib extends ScalaModule {
def scalaVersion = Option(System.getenv("TEST_SCALA_2_13_VERSION")).getOrElse(???)
def mvnDeps = Seq(
mvn"org.slf4j:slf4j-api:2.0.16"
)
}

object app extends ScalaModule {
def scalaVersion = Option(System.getenv("TEST_SCALA_2_13_VERSION")).getOrElse(???)
def moduleDeps = Seq(lib)
def mvnDeps = Seq(
mvn"ch.qos.logback:logback-core:1.5.15"
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package build

import mill.scalalib._

object thing extends ScalaModule {
def scalaVersion = Option(System.getenv("TEST_SCALA_2_13_VERSION")).getOrElse(???)
def mvnDeps = Seq(
mvn"com.lihaoyi::pprint:0.9.0"
)
}

object lib extends ScalaModule {
def scalaVersion = Option(System.getenv("TEST_SCALA_2_13_VERSION")).getOrElse(???)
def mvnDeps = Seq(
mvnz"org.slf4j:slf4j-api:2.0.16"
)
}

object app extends ScalaModule {
def scalaVersion = Option(System.getenv("TEST_SCALA_2_13_VERSION")).getOrElse(???)
def moduleDeps = Seq(lib)
def mvnDeps = Seq(
mvn"ch.qos.logback:logback-core:1.5.15"
)
}
Loading