Skip to content

Commit b67eb3e

Browse files
authored
Fix/changing options from sources should not require reload (#3112)
* 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
1 parent 5aa5c33 commit b67eb3e

File tree

3 files changed

+58
-24
lines changed

3 files changed

+58
-24
lines changed

modules/cli/src/main/scala/scala/cli/commands/bsp/Bsp.scala

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,31 +130,33 @@ object Bsp extends ScalaCommand[BspOptions] {
130130

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

133-
/** values used for lauching the bsp, especially for launching a bloop server, they include
134-
* options extracted from sources
133+
/** values used for launching the bsp, especially for launching the bloop server, they do not
134+
* include options extracted from sources, except in bloopRifleConfig - it's needed for
135+
* correctly launching the bloop server
135136
*/
136137
val initialBspOptions = {
137138
val sharedOptions = getSharedOptions()
138139
val launcherOptions = getLauncherOptions()
139140
val envs = getEnvsFromFile()
140141
val bspBuildOptions = buildOptions(sharedOptions, launcherOptions, envs)
141-
.orElse(finalBuildOptions)
142+
142143
refreshPowerMode(launcherOptions, sharedOptions, envs)
144+
143145
BspReloadableOptions(
144146
buildOptions = bspBuildOptions,
145-
bloopRifleConfig = sharedOptions.bloopRifleConfig(Some(bspBuildOptions))
147+
bloopRifleConfig = sharedOptions.bloopRifleConfig(Some(finalBuildOptions))
146148
.orExit(sharedOptions.logger),
147149
logger = sharedOptions.logging.logger,
148150
verbosity = sharedOptions.logging.verbosity
149151
)
150152
}
151153

152154
val bspReloadableOptionsReference = BspReloadableOptions.Reference { () =>
153-
val sharedOptions = getSharedOptions()
154-
val launcherOptions = getLauncherOptions()
155-
val envs = getEnvsFromFile()
156-
val bloopRifleConfig = sharedOptions.bloopRifleConfig(Some(finalBuildOptions))
157-
.orExit(sharedOptions.logger)
155+
val sharedOptions = getSharedOptions()
156+
val launcherOptions = getLauncherOptions()
157+
val envs = getEnvsFromFile()
158+
val bloopRifleConfig = sharedOptions.bloopRifleConfig()
159+
158160
refreshPowerMode(launcherOptions, sharedOptions, envs)
159161

160162
BspReloadableOptions(

modules/integration/src/test/scala/scala/cli/integration/BspTestDefinitions.scala

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,10 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
786786
|""".stripMargin
787787
)
788788

789+
val actualScalaMajorVersion = actualScalaVersion.split("\\.")
790+
.take(if (actualScalaVersion.startsWith("3")) 1 else 2)
791+
.mkString(".")
792+
789793
withBsp(inputs, Seq(".")) { (root, localClient, remoteServer) =>
790794
async {
791795
val buildTargetsResp = await(remoteServer.workspaceBuildTargets().asScala)
@@ -821,24 +825,14 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
821825
uri.drop(idx + 1)
822826
}
823827

824-
if (actualScalaVersion.startsWith("2.13")) {
825-
expect(foundDepSources.exists(_.startsWith("utest_2.13-0.7.10")))
826-
expect(foundDepSources.exists(_.startsWith("os-lib_2.13-0.7.8")))
827-
}
828-
else if (actualScalaVersion.startsWith("2.12")) {
829-
expect(foundDepSources.exists(_.startsWith("utest_2.12-0.7.10")))
830-
expect(foundDepSources.exists(_.startsWith("os-lib_2.12-0.7.8")))
831-
}
832-
else {
833-
expect(foundDepSources.exists(_.startsWith("utest_3-0.7.10")))
834-
expect(foundDepSources.exists(_.startsWith("os-lib_3-0.7.8")))
835-
}
828+
expect(foundDepSources.exists(_.startsWith(s"utest_$actualScalaMajorVersion-0.7.10")))
829+
expect(foundDepSources.exists(_.startsWith(s"os-lib_$actualScalaMajorVersion-0.7.8")))
836830

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

841-
localClient.buildTargetDidChange()
835+
val changeFuture = localClient.buildTargetDidChange()
842836

843837
val newFileContent =
844838
"""object Messages {
@@ -851,6 +845,31 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
851845
val resp = await(remoteServer.buildTargetCompile(new b.CompileParams(targets)).asScala)
852846
expect(resp.getStatusCode == b.StatusCode.OK)
853847
}
848+
849+
expect(changeFuture.isCompleted)
850+
851+
{
852+
val resp = await {
853+
remoteServer
854+
.buildTargetDependencySources(new b.DependencySourcesParams(targets))
855+
.asScala
856+
}
857+
val foundTargets = resp.getItems.asScala.map(_.getTarget.getUri).toSeq
858+
expect(foundTargets == Seq(targetUri))
859+
val foundDepSources = resp.getItems.asScala
860+
.flatMap(_.getSources.asScala)
861+
.toSeq
862+
.map { uri =>
863+
val idx = uri.lastIndexOf('/')
864+
uri.drop(idx + 1)
865+
}
866+
867+
expect(foundDepSources.exists(_.startsWith(s"utest_$actualScalaMajorVersion-0.7.10")))
868+
expect(!foundDepSources.exists(_.startsWith(s"os-lib_$actualScalaMajorVersion-0.7.8")))
869+
870+
expect(foundDepSources.exists(_.startsWith("test-interface-1.0")))
871+
expect(foundDepSources.forall(_.endsWith("-sources.jar")))
872+
}
854873
}
855874
}
856875
}
@@ -1221,7 +1240,7 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
12211240
val ideOptionsPath = root / Constants.workspaceDirName / "ide-options-v2.json"
12221241
val jsonOptions = List("--json-options", ideOptionsPath.toString)
12231242
withBsp(inputs, Seq("."), bspOptions = jsonOptions, reuseRoot = Some(root)) {
1224-
(_, _, remoteServer) =>
1243+
(_, localClient, remoteServer) =>
12251244
async {
12261245
val buildTargetsResp = await(remoteServer.workspaceBuildTargets().asScala)
12271246
val targets = buildTargetsResp.getTargets.asScala.map(_.getId).toSeq
@@ -1247,9 +1266,16 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
12471266
|""".stripMargin
12481267
os.write.over(root / sourceFilePath, updatedSourceFile)
12491268

1269+
expect(!localClient.logMessages().exists(_.getMessage.startsWith(
1270+
"Error reading API from class file: ReloadTest : java.lang.UnsupportedClassVersionError: ReloadTest has been compiled by a more recent version of the Java Runtime"
1271+
)))
1272+
12501273
val errorResponse =
12511274
await(remoteServer.buildTargetCompile(new b.CompileParams(targets.asJava)).asScala)
1252-
expect(errorResponse.getStatusCode == b.StatusCode.ERROR)
1275+
expect(errorResponse.getStatusCode == b.StatusCode.OK)
1276+
expect(localClient.logMessages().exists(_.getMessage.startsWith(
1277+
"Error reading API from class file: ReloadTest : java.lang.UnsupportedClassVersionError: ReloadTest has been compiled by a more recent version of the Java Runtime"
1278+
)))
12531279

12541280
val reloadResponse =
12551281
extractWorkspaceReloadResponse(await(remoteServer.workspaceReload().asScala))

modules/integration/src/test/scala/scala/cli/integration/TestBspClient.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ class TestBspClient extends b.BuildClient {
2828
p.future
2929
}
3030

31+
def logMessages(): Seq[b.LogMessageParams] =
32+
lock.synchronized {
33+
messages0.reverseIterator.collect {
34+
case p: b.LogMessageParams => p
35+
}.toSeq
36+
}
3137
def diagnostics(): Seq[b.PublishDiagnosticsParams] =
3238
lock.synchronized {
3339
messages0.reverseIterator.collect {

0 commit comments

Comments
 (0)