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

Add some BSP integration tests #3608

Merged
merged 33 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
beabf1c
Pass TEST_SCALA_2_13_VERSION around in ITs
alexarchambault Sep 26, 2024
9cdf01e
Ensure mill.main.cli is passed around
alexarchambault Sep 26, 2024
a911227
Use a fixed name for the BSP synthetic root target
alexarchambault Sep 26, 2024
5cb5a33
Make some BSP responses more deterministic
alexarchambault Sep 26, 2024
9f02cc1
Add BSP integration test
alexarchambault Sep 26, 2024
c2db980
Remove mill-build from most BSP IT fixtures
alexarchambault Sep 30, 2024
324b5f0
Use language list from Constants in BSP init
alexarchambault Oct 1, 2024
821e751
fixup Add BSP integration test
alexarchambault Oct 7, 2024
97414fb
debug CI
alexarchambault Oct 7, 2024
6bdfe94
Use a cached thread pool in the BSP server
alexarchambault Sep 27, 2024
4e9edbd
Mark build server JSONRPC threads as daemon
alexarchambault Oct 7, 2024
c23fa26
debug CI
alexarchambault Oct 7, 2024
288bbfd
fixup Mark build server JSONRPC threads as daemon
alexarchambault Oct 7, 2024
2b75a2c
debug CI
alexarchambault Oct 7, 2024
df7f48d
? fixup Add BSP integration test
alexarchambault Oct 7, 2024
8906532
debug CI
alexarchambault Oct 7, 2024
e9583c5
fixup debug CI
alexarchambault Oct 7, 2024
73dac08
?
alexarchambault Oct 7, 2024
ef51057
fixup Add BSP integration test
alexarchambault Oct 7, 2024
15ca7f9
Merge branch 'main' into bsp-integration-tests
alexarchambault Oct 7, 2024
7a1779b
fixup Add BSP integration test
alexarchambault Oct 7, 2024
4293991
Revert "fixup debug CI"
alexarchambault Oct 7, 2024
ff48ad0
Revert "debug CI"
alexarchambault Oct 7, 2024
3403728
Revert "debug CI"
alexarchambault Oct 7, 2024
507a6e4
Revert "debug CI"
alexarchambault Oct 7, 2024
e54280d
Revert "debug CI"
alexarchambault Oct 7, 2024
c56d9e0
.
alexarchambault Oct 7, 2024
b8f0273
.
alexarchambault Oct 7, 2024
1ec1d6e
.
alexarchambault Oct 7, 2024
b49ad0e
.
alexarchambault Oct 8, 2024
78d9f33
.
alexarchambault Oct 8, 2024
ef4e239
.
alexarchambault Oct 8, 2024
8501d62
.
alexarchambault Oct 8, 2024
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
7 changes: 3 additions & 4 deletions bsp/src/mill/bsp/BSP.scala
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,10 @@ object BSP extends ExternalModule with CoursierModule {
}

private def bspConnectionJson(jobs: Int, debug: Boolean): String = {
val props = sys.props
val millPath = props
.get("mill.main.cli")
val millPath = sys.env.get("MILL_MAIN_CLI")
.orElse(sys.props.get("mill.main.cli"))
// we assume, the classpath is an executable jar here
.orElse(props.get("java.class.path"))
.orElse(sys.props.get("java.class.path"))
.getOrElse(throw new IllegalStateException("System property 'java.class.path' not set"))

upickle.default.write(
Expand Down
2 changes: 1 addition & 1 deletion bsp/src/mill/bsp/Constants.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ private[mill] object Constants {
val bspProtocolVersion = BuildInfo.bsp4jVersion
val bspWorkerImplClass = "mill.bsp.worker.BspWorkerImpl"
val bspWorkerBuildInfoClass = "mill.bsp.worker.BuildInfo"
val languages: Seq[String] = Seq("scala", "java")
val languages: Seq[String] = Seq("java", "scala")
val serverName = "mill-bsp"
}
35 changes: 24 additions & 11 deletions bsp/worker/src/mill/bsp/worker/MillBuildServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import ch.epfl.scala.bsp4j
import ch.epfl.scala.bsp4j._
import com.google.gson.JsonObject
import mill.api.{DummyTestReporter, Result, Strict}
import mill.bsp.BspServerResult
import mill.bsp.{BspServerResult, Constants}
import mill.bsp.worker.Utils.{makeBuildTarget, outputPaths, sanitizeUri}
import mill.define.Segment.Label
import mill.define.{Args, Discover, ExternalModule, Task}
Expand Down Expand Up @@ -33,6 +33,8 @@ private class MillBuildServer(
) extends ExternalModule
with BuildServer {

import MillBuildServer._

lazy val millDiscover: Discover = Discover[this.type]

private[worker] var cancellator: Boolean => Unit = shutdownBefore => ()
Expand Down Expand Up @@ -71,7 +73,7 @@ private class MillBuildServer(

// TODO: scan BspModules and infer their capabilities

val supportedLangs = Seq("java", "scala").asJava
val supportedLangs = Constants.languages.asJava
val capabilities = new BuildServerCapabilities

capabilities.setBuildTargetChangedProvider(false)
Expand Down Expand Up @@ -154,7 +156,7 @@ private class MillBuildServer(
override def workspaceBuildTargets(): CompletableFuture[WorkspaceBuildTargetsResult] =
completableTasksWithState(
"workspaceBuildTargets",
targetIds = _.bspModulesById.keySet.toSeq,
targetIds = _.bspModulesIdList.map(_._1),
tasks = { case m: BspModule => m.bspBuildTargetData }
) { (ev, state, id, m: BspModule, bspBuildTargetData) =>
val depsIds = m match {
Expand Down Expand Up @@ -252,7 +254,7 @@ private class MillBuildServer(
override def buildTargetInverseSources(p: InverseSourcesParams)
: CompletableFuture[InverseSourcesResult] = {
completable(s"buildtargetInverseSources ${p}") { state =>
val tasksEvaluators = state.bspModulesById.iterator.collect {
val tasksEvaluators = state.bspModulesIdList.iterator.collect {
case (id, (m: JavaModule, ev)) =>
Task.Anon {
val src = m.allSourceFiles()
Expand All @@ -263,11 +265,9 @@ private class MillBuildServer(
} -> ev
}.toSeq

val ids = tasksEvaluators
.groupMap(_._2)(_._1)
val ids = groupList(tasksEvaluators)(_._2)(_._1)
.flatMap { case (ev, ts) => ev.evalOrThrow()(ts) }
.flatten
.toSeq

new InverseSourcesResult(ids.asJava)
}
Expand Down Expand Up @@ -641,9 +641,8 @@ private class MillBuildServer(
tasks.lift.apply(m).map(ts => (ts, (ev, id)))
}

val evaluated = tasksSeq
// group by evaluator (different root module)
.groupMap(_._2)(_._1)
// group by evaluator (different root module)
val evaluated = groupList(tasksSeq.toSeq)(_._2)(_._1)
.map { case ((ev, id), ts) =>
val results = ev.evaluate(ts)
val failures = results.results.collect {
Expand Down Expand Up @@ -674,7 +673,7 @@ private class MillBuildServer(
}
}

agg(evaluated.flatten.toSeq.asJava, state)
agg(evaluated.flatten.asJava, state)
}
}

Expand Down Expand Up @@ -772,3 +771,17 @@ private class MillBuildServer(
debug("onRunReadStdin is current unsupported")
}
}

private object MillBuildServer {

/**
* Same as Iterable.groupMap, but returns a sequence instead of a map, and preserves
* the order of appearance of the keys from the input sequence
*/
private def groupList[A, K, B](seq: Seq[A])(key: A => K)(f: A => B): Seq[(K, Seq[B])] = {
lefou marked this conversation as resolved.
Show resolved Hide resolved
val keyIndices = seq.map(key).distinct.zipWithIndex.toMap
seq.groupMap(key)(f)
.toSeq
.sortBy { case (k, _) => keyIndices(k) }
}
}
13 changes: 7 additions & 6 deletions bsp/worker/src/mill/bsp/worker/State.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import mill.define.Module
import mill.eval.Evaluator

private class State(workspaceDir: os.Path, evaluators: Seq[Evaluator], debug: String => Unit) {
lazy val bspModulesById: Map[BuildTargetIdentifier, (BspModule, Evaluator)] = {
lazy val bspModulesIdList: Seq[(BuildTargetIdentifier, (BspModule, Evaluator))] = {
alexarchambault marked this conversation as resolved.
Show resolved Hide resolved
val modules: Seq[(Module, Seq[Module], Evaluator)] = evaluators
.map(ev => (ev.rootModule, JavaModuleUtils.transitiveModules(ev.rootModule), ev))

val map = modules
.flatMap { case (rootModule, otherModules, eval) =>
(Seq(rootModule) ++ otherModules).collect {
modules
.flatMap { case (rootModule, modules, eval) =>
modules.collect {
case m: BspModule =>
val uri = Utils.sanitizeUri(
rootModule.millSourcePath /
Expand All @@ -24,9 +24,10 @@ private class State(workspaceDir: os.Path, evaluators: Seq[Evaluator], debug: St
(new BuildTargetIdentifier(uri), (m, eval))
}
}
.toMap
}
lazy val bspModulesById: Map[BuildTargetIdentifier, (BspModule, Evaluator)] = {
val map = bspModulesIdList.toMap
debug(s"BspModules: ${map.view.mapValues(_._1.bspDisplayName).toMap}")

map
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import mill.bsp.worker.Utils.{makeBuildTarget, sanitizeUri}
import mill.scalalib.bsp.{BspBuildTarget, BspModule}
import mill.scalalib.bsp.BspModule.Tag

import java.util.UUID
import scala.jdk.CollectionConverters._
import ch.epfl.scala.bsp4j.BuildTarget

Expand All @@ -15,11 +14,11 @@ import ch.epfl.scala.bsp4j.BuildTarget
*/
class SyntheticRootBspBuildTargetData(topLevelProjectRoot: os.Path) {
val id: BuildTargetIdentifier = new BuildTargetIdentifier(
Utils.sanitizeUri(topLevelProjectRoot / s"synth-build-target-${UUID.randomUUID()}")
Utils.sanitizeUri(topLevelProjectRoot / "mill-synthetic-root-target")
Copy link
Member

Choose a reason for hiding this comment

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

We're removing the UUID here. Do we know that it is not necessary?

Copy link
Contributor Author

@alexarchambault alexarchambault Oct 7, 2024

Choose a reason for hiding this comment

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

My guess is that this avoids conflicts with an actual module users created. But a collision seems quite unlikely.

)

val bt: BspBuildTarget = BspBuildTarget(
displayName = Some(topLevelProjectRoot.last + "-root"),
displayName = Some("mill-synthetic-root"),
baseDirectory = Some(topLevelProjectRoot),
tags = Seq(Tag.Manual),
languageIds = Seq.empty,
Expand Down
15 changes: 0 additions & 15 deletions integration/ide/bsp-install/resources/build.mill

This file was deleted.

23 changes: 0 additions & 23 deletions integration/ide/bsp-install/src/BspInstallTests.scala

This file was deleted.

10 changes: 10 additions & 0 deletions integration/ide/bsp-server/resources/project/build.mill
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package build

import mill._
import mill.scalalib._

object `hello-java` extends JavaModule

object `hello-scala` extends ScalaModule {
def scalaVersion = Option(System.getenv("TEST_SCALA_2_13_VERSION")).getOrElse(???)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"items": [
{
"target": {
"uri": "file:///workspace/hello-java"
},
"classpath": [
"file:///workspace/hello-java/compile-resources"
]
},
{
"target": {
"uri": "file:///workspace/hello-scala"
},
"classpath": [
"file:///coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/scala-library/<scala-version>/scala-library-<scala-version>.jar",
"file:///workspace/hello-scala/compile-resources"
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"items": [
{
"target": {
"uri": "file:///workspace/hello-java"
},
"modules": []
},
{
"target": {
"uri": "file:///workspace/hello-scala"
},
"modules": [
{
"name": "org.scala-lang:scala-library",
"version": "<scala-version>"
}
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"items": [
{
"target": {
"uri": "file:///workspace/hello-java"
},
"sources": []
},
{
"target": {
"uri": "file:///workspace/hello-scala"
},
"sources": [
"file:///coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/scala-library/<scala-version>/scala-library-<scala-version>-sources.jar"
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"items": [
{
"target": {
"uri": "file:///workspace/hello-java"
},
"options": [],
"classpath": [
"file:///workspace/hello-java/compile-resources"
],
"classDirectory": "file:///workspace/out/hello-java/compile.dest/classes"
},
{
"target": {
"uri": "file:///workspace/hello-scala"
},
"options": [],
"classpath": [
"file:///coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/scala-library/<scala-version>/scala-library-<scala-version>.jar",
"file:///workspace/hello-scala/compile-resources"
],
"classDirectory": "file:///workspace/out/hello-scala/compile.dest/classes"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"items": [
{
"target": {
"uri": "file:///workspace/hello-java"
},
"classpath": [
"file:///workspace/hello-java/compile-resources",
"file:///workspace/hello-java/resources",
"file:///workspace/out/hello-java/compile.dest/classes"
],
"jvmOptions": [],
"workingDirectory": "/workspace",
"environmentVariables": {},
"mainClasses": []
},
{
"target": {
"uri": "file:///workspace/hello-scala"
},
"classpath": [
"file:///coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/scala-library/<scala-version>/scala-library-<scala-version>.jar",
"file:///workspace/hello-scala/compile-resources",
"file:///workspace/hello-scala/resources",
"file:///workspace/out/hello-scala/compile.dest/classes"
],
"jvmOptions": [],
"workingDirectory": "/workspace",
"environmentVariables": {},
"mainClasses": []
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"items": [
{
"target": {
"uri": "file:///workspace/hello-java"
},
"classpath": [
"file:///workspace/hello-java/compile-resources",
"file:///workspace/hello-java/resources",
"file:///workspace/out/hello-java/compile.dest/classes"
],
"jvmOptions": [],
"workingDirectory": "/workspace",
"environmentVariables": {},
"mainClasses": []
},
{
"target": {
"uri": "file:///workspace/hello-scala"
},
"classpath": [
"file:///coursier-cache/https/repo1.maven.org/maven2/org/scala-lang/scala-library/<scala-version>/scala-library-<scala-version>.jar",
"file:///workspace/hello-scala/compile-resources",
"file:///workspace/hello-scala/resources",
"file:///workspace/out/hello-scala/compile.dest/classes"
],
"jvmOptions": [],
"workingDirectory": "/workspace",
"environmentVariables": {},
"mainClasses": []
}
]
}
Loading
Loading