Skip to content

Commit

Permalink
Fix/changing options from sources should not require reload (#3112)
Browse files Browse the repository at this point in the history
* Test dependencies being removed from classpath after changing the directives in sources and compiling

* Pass only options from cli as initial reloadable bsp options. Those are later embedded in every call to compile or build in BspImpl.run() -> newBloopSession().

* Fix test conditions

* Reloadable options reference should only contain options from CLI in all its fields.

* Apply scalafix

* Review fix
  • Loading branch information
MaciejG604 authored Aug 20, 2024
1 parent 5aa5c33 commit b67eb3e
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 24 deletions.
20 changes: 11 additions & 9 deletions modules/cli/src/main/scala/scala/cli/commands/bsp/Bsp.scala
Original file line number Diff line number Diff line change
Expand Up @@ -130,31 +130,33 @@ object Bsp extends ScalaCommand[BspOptions] {

val (inputs, finalBuildOptions) = preprocessInputs(args.all).orExit(logger)

/** values used for lauching the bsp, especially for launching a bloop server, they include
* options extracted from sources
/** values used for launching the bsp, especially for launching the bloop server, they do not
* include options extracted from sources, except in bloopRifleConfig - it's needed for
* correctly launching the bloop server
*/
val initialBspOptions = {
val sharedOptions = getSharedOptions()
val launcherOptions = getLauncherOptions()
val envs = getEnvsFromFile()
val bspBuildOptions = buildOptions(sharedOptions, launcherOptions, envs)
.orElse(finalBuildOptions)

refreshPowerMode(launcherOptions, sharedOptions, envs)

BspReloadableOptions(
buildOptions = bspBuildOptions,
bloopRifleConfig = sharedOptions.bloopRifleConfig(Some(bspBuildOptions))
bloopRifleConfig = sharedOptions.bloopRifleConfig(Some(finalBuildOptions))
.orExit(sharedOptions.logger),
logger = sharedOptions.logging.logger,
verbosity = sharedOptions.logging.verbosity
)
}

val bspReloadableOptionsReference = BspReloadableOptions.Reference { () =>
val sharedOptions = getSharedOptions()
val launcherOptions = getLauncherOptions()
val envs = getEnvsFromFile()
val bloopRifleConfig = sharedOptions.bloopRifleConfig(Some(finalBuildOptions))
.orExit(sharedOptions.logger)
val sharedOptions = getSharedOptions()
val launcherOptions = getLauncherOptions()
val envs = getEnvsFromFile()
val bloopRifleConfig = sharedOptions.bloopRifleConfig()

refreshPowerMode(launcherOptions, sharedOptions, envs)

BspReloadableOptions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,10 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
|""".stripMargin
)

val actualScalaMajorVersion = actualScalaVersion.split("\\.")
.take(if (actualScalaVersion.startsWith("3")) 1 else 2)
.mkString(".")

withBsp(inputs, Seq(".")) { (root, localClient, remoteServer) =>
async {
val buildTargetsResp = await(remoteServer.workspaceBuildTargets().asScala)
Expand Down Expand Up @@ -821,24 +825,14 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
uri.drop(idx + 1)
}

if (actualScalaVersion.startsWith("2.13")) {
expect(foundDepSources.exists(_.startsWith("utest_2.13-0.7.10")))
expect(foundDepSources.exists(_.startsWith("os-lib_2.13-0.7.8")))
}
else if (actualScalaVersion.startsWith("2.12")) {
expect(foundDepSources.exists(_.startsWith("utest_2.12-0.7.10")))
expect(foundDepSources.exists(_.startsWith("os-lib_2.12-0.7.8")))
}
else {
expect(foundDepSources.exists(_.startsWith("utest_3-0.7.10")))
expect(foundDepSources.exists(_.startsWith("os-lib_3-0.7.8")))
}
expect(foundDepSources.exists(_.startsWith(s"utest_$actualScalaMajorVersion-0.7.10")))
expect(foundDepSources.exists(_.startsWith(s"os-lib_$actualScalaMajorVersion-0.7.8")))

expect(foundDepSources.exists(_.startsWith("test-interface-1.0")))
expect(foundDepSources.forall(_.endsWith("-sources.jar")))
}

localClient.buildTargetDidChange()
val changeFuture = localClient.buildTargetDidChange()

val newFileContent =
"""object Messages {
Expand All @@ -851,6 +845,31 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
val resp = await(remoteServer.buildTargetCompile(new b.CompileParams(targets)).asScala)
expect(resp.getStatusCode == b.StatusCode.OK)
}

expect(changeFuture.isCompleted)

{
val resp = await {
remoteServer
.buildTargetDependencySources(new b.DependencySourcesParams(targets))
.asScala
}
val foundTargets = resp.getItems.asScala.map(_.getTarget.getUri).toSeq
expect(foundTargets == Seq(targetUri))
val foundDepSources = resp.getItems.asScala
.flatMap(_.getSources.asScala)
.toSeq
.map { uri =>
val idx = uri.lastIndexOf('/')
uri.drop(idx + 1)
}

expect(foundDepSources.exists(_.startsWith(s"utest_$actualScalaMajorVersion-0.7.10")))
expect(!foundDepSources.exists(_.startsWith(s"os-lib_$actualScalaMajorVersion-0.7.8")))

expect(foundDepSources.exists(_.startsWith("test-interface-1.0")))
expect(foundDepSources.forall(_.endsWith("-sources.jar")))
}
}
}
}
Expand Down Expand Up @@ -1221,7 +1240,7 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
val ideOptionsPath = root / Constants.workspaceDirName / "ide-options-v2.json"
val jsonOptions = List("--json-options", ideOptionsPath.toString)
withBsp(inputs, Seq("."), bspOptions = jsonOptions, reuseRoot = Some(root)) {
(_, _, remoteServer) =>
(_, localClient, remoteServer) =>
async {
val buildTargetsResp = await(remoteServer.workspaceBuildTargets().asScala)
val targets = buildTargetsResp.getTargets.asScala.map(_.getId).toSeq
Expand All @@ -1247,9 +1266,16 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
|""".stripMargin
os.write.over(root / sourceFilePath, updatedSourceFile)

expect(!localClient.logMessages().exists(_.getMessage.startsWith(
"Error reading API from class file: ReloadTest : java.lang.UnsupportedClassVersionError: ReloadTest has been compiled by a more recent version of the Java Runtime"
)))

val errorResponse =
await(remoteServer.buildTargetCompile(new b.CompileParams(targets.asJava)).asScala)
expect(errorResponse.getStatusCode == b.StatusCode.ERROR)
expect(errorResponse.getStatusCode == b.StatusCode.OK)
expect(localClient.logMessages().exists(_.getMessage.startsWith(
"Error reading API from class file: ReloadTest : java.lang.UnsupportedClassVersionError: ReloadTest has been compiled by a more recent version of the Java Runtime"
)))

val reloadResponse =
extractWorkspaceReloadResponse(await(remoteServer.workspaceReload().asScala))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ class TestBspClient extends b.BuildClient {
p.future
}

def logMessages(): Seq[b.LogMessageParams] =
lock.synchronized {
messages0.reverseIterator.collect {
case p: b.LogMessageParams => p
}.toSeq
}
def diagnostics(): Seq[b.PublishDiagnosticsParams] =
lock.synchronized {
messages0.reverseIterator.collect {
Expand Down

0 comments on commit b67eb3e

Please sign in to comment.