Skip to content

Commit f50432b

Browse files
authored
Merge pull request #4273 from Gedochao/fix/wonky-base-directory
Restore BSP base directory to the original workspace regardless of permissions/ownership
2 parents d199b5e + 6060cd6 commit f50432b

7 files changed

Lines changed: 163 additions & 23 deletions

File tree

modules/build/src/main/scala/scala/build/Build.scala

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,10 @@ object Build {
303303
workspace = inputs.workspace,
304304
updateSemanticDbs = true,
305305
scalaVersion = sv,
306-
buildOptions = build.options
306+
buildOptions =
307+
inputs.originalWorkspaceOpt.fold(build.options)(
308+
build.options.withResolvedSemanticDbSourceRoot
309+
)
307310
).left.foreach(_.foreach(logger.message(_)))
308311
case _ =>
309312
}
@@ -942,7 +945,8 @@ object Build {
942945
options.scalaOptions.semanticDbOptions.generateSemanticDbs.getOrElse(false)
943946
val semanticDbTargetRoot = options.scalaOptions.semanticDbOptions.semanticDbTargetRoot
944947
val semanticDbSourceRoot =
945-
options.scalaOptions.semanticDbOptions.semanticDbSourceRoot.getOrElse(inputs.workspace)
948+
options.scalaOptions.semanticDbOptions.semanticDbSourceRoot
949+
.getOrElse(inputs.originalWorkspaceOpt.getOrElse(inputs.workspace))
946950

947951
val scalaCompilerParamsOpt = artifacts.scalaOpt match {
948952
case Some(scalaArtifacts) =>

modules/build/src/main/scala/scala/build/bsp/BspImpl.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,10 @@ final class BspImpl(
363363
currentBloopSession.inputs.workspace,
364364
updateSemanticDbs = true,
365365
scalaVersion = sv,
366-
buildOptions = data.buildOptions
366+
buildOptions =
367+
currentBloopSession.inputs.originalWorkspaceOpt.fold(data.buildOptions)(
368+
data.buildOptions.withResolvedSemanticDbSourceRoot
369+
)
367370
).left.foreach(_.foreach(showGlobalWarningOnce))
368371

369372
if (res.getStatusCode == b.StatusCode.OK)

modules/build/src/main/scala/scala/build/bsp/BspServer.scala

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import java.util as ju
99
import java.util.concurrent.{CompletableFuture, TimeUnit}
1010

1111
import scala.build.Logger
12+
import scala.build.input.Inputs
1213
import scala.build.internal.Constants
1314
import scala.build.options.Scope
1415
import scala.concurrent.{Future, Promise}
@@ -31,6 +32,13 @@ class BspServer(
3132
@volatile private var intelliJ: Boolean = presetIntelliJ
3233
def isIntelliJ: Boolean = intelliJ
3334

35+
@volatile private var bspBaseDirectoryOverride: Option[os.Path] = None
36+
37+
override def newInputs(inputs: Inputs): Unit = {
38+
super.newInputs(inputs)
39+
bspBaseDirectoryOverride = inputs.originalWorkspaceOpt
40+
}
41+
3442
def clientOpt: Option[BuildClient] = client
3543

3644
@volatile private var extraDependencySources: Seq[os.Path] = Nil
@@ -276,16 +284,16 @@ class BspServer(
276284
val res0 = res.duplicate()
277285
stripInvalidTargets(res0)
278286
for (target <- res0.getTargets.asScala) {
279-
val capabilities = target.getCapabilities
280-
capabilities.setCanDebug(true)
287+
target.getCapabilities.setCanDebug(true)
281288
val baseDirectory = new File(new URI(target.getBaseDirectory))
282-
if (
283-
isIntelliJ && baseDirectory.getName == Constants.workspaceDirName &&
284-
baseDirectory
285-
.getParentFile != null
286-
) {
287-
val newBaseDirectory = baseDirectory.getParentFile.toPath.toUri.toASCIIString
288-
target.setBaseDirectory(newBaseDirectory)
289+
bspBaseDirectoryOverride match {
290+
case Some(originalWs) =>
291+
target.setBaseDirectory(originalWs.toNIO.toUri.toASCIIString)
292+
case None
293+
if isIntelliJ && baseDirectory.getName == Constants.workspaceDirName
294+
&& baseDirectory.getParentFile != null =>
295+
target.setBaseDirectory(baseDirectory.getParentFile.toPath.toUri.toASCIIString)
296+
case _ => // leave Bloop's value untouched
289297
}
290298
}
291299
res0

modules/build/src/main/scala/scala/build/input/Inputs.scala

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ final case class Inputs(
2424
mayAppendHash: Boolean,
2525
workspaceOrigin: Option[WorkspaceOrigin],
2626
enableMarkdown: Boolean,
27-
allowRestrictedFeatures: Boolean
27+
allowRestrictedFeatures: Boolean,
28+
originalWorkspaceOpt: Option[os.Path]
2829
) {
2930

3031
def isEmpty: Boolean = elements.isEmpty
@@ -75,7 +76,8 @@ final case class Inputs(
7576
copy(
7677
workspace = elements.homeWorkspace(directories),
7778
mayAppendHash = false,
78-
workspaceOrigin = Some(WorkspaceOrigin.HomeDir)
79+
workspaceOrigin = Some(WorkspaceOrigin.HomeDir),
80+
originalWorkspaceOpt = originalWorkspaceOpt.orElse(Some(workspace))
7981
)
8082
def avoid(forbidden: Seq[os.Path], directories: Directories): Inputs =
8183
if forbidden.exists(workspace.startsWith) then inHomeDir(directories) else this
@@ -159,7 +161,8 @@ object Inputs {
159161
mayAppendHash = needsHash,
160162
workspaceOrigin = Some(workspaceOrigin),
161163
enableMarkdown = enableMarkdown,
162-
allowRestrictedFeatures = allowRestrictedFeatures
164+
allowRestrictedFeatures = allowRestrictedFeatures,
165+
originalWorkspaceOpt = None
163166
)
164167
}
165168

@@ -476,11 +479,12 @@ object Inputs {
476479
mayAppendHash = true,
477480
workspaceOrigin = None,
478481
enableMarkdown = enableMarkdown,
479-
allowRestrictedFeatures = false
482+
allowRestrictedFeatures = false,
483+
originalWorkspaceOpt = None
480484
)
481485

482486
def empty(projectName: String): Inputs =
483-
Inputs(Nil, None, os.pwd, projectName, false, None, true, false)
487+
Inputs(Nil, None, os.pwd, projectName, false, None, true, false, None)
484488

485489
def baseName(p: os.Path) = if (p == os.root || p.lastOpt.isEmpty) "" else p.baseName
486490

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,12 @@ import scala.util.{Failure, Success, Try}
2222

2323
trait BspSuite { this: ScalaCliSuite =>
2424
protected def extraOptions: Seq[String]
25-
def initParams(root: os.Path): b.InitializeBuildParams =
25+
def initParams(
26+
root: os.Path,
27+
clientName: String = "Scala CLI ITs"
28+
): b.InitializeBuildParams =
2629
new b.InitializeBuildParams(
27-
"Scala CLI ITs",
30+
clientName,
2831
"0",
2932
Constants.bspVersion,
3033
root.toNIO.toUri.toASCIIString,
@@ -74,7 +77,8 @@ trait BspSuite { this: ScalaCliSuite =>
7477
bspEnvs: Map[String, String] = Map.empty,
7578
reuseRoot: Option[os.Path] = None,
7679
stdErrOpt: Option[os.RelPath] = None,
77-
extraOptionsOverride: Seq[String] = extraOptions
80+
extraOptionsOverride: Seq[String] = extraOptions,
81+
bspClientName: String = "Scala CLI ITs"
7882
)(
7983
f: (
8084
os.Path,
@@ -91,7 +95,8 @@ trait BspSuite { this: ScalaCliSuite =>
9195
bspEnvs,
9296
reuseRoot,
9397
stdErrOpt,
94-
extraOptionsOverride
98+
extraOptionsOverride,
99+
bspClientName
95100
)((root, client, server, _: b.InitializeBuildResult) => f(root, client, server))
96101

97102
def withBspInitResults[T](
@@ -103,7 +108,8 @@ trait BspSuite { this: ScalaCliSuite =>
103108
bspEnvs: Map[String, String] = Map.empty,
104109
reuseRoot: Option[os.Path] = None,
105110
stdErrOpt: Option[os.RelPath] = None,
106-
extraOptionsOverride: Seq[String] = extraOptions
111+
extraOptionsOverride: Seq[String] = extraOptions,
112+
bspClientName: String = "Scala CLI ITs"
107113
)(
108114
f: (
109115
os.Path,
@@ -151,7 +157,9 @@ trait BspSuite { this: ScalaCliSuite =>
151157
TestBspClient.connect(proc.stdout, proc.stdin, pool)
152158
remoteServer = remoteServer0
153159
val initRes: b.InitializeBuildResult = Await.result(
154-
whileBspServerIsRunning(remoteServer.buildInitialize(initParams(root)).asScala),
160+
whileBspServerIsRunning(
161+
remoteServer.buildInitialize(initParams(root, bspClientName)).asScala
162+
),
155163
Duration.Inf
156164
)
157165
Await.result(

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2445,4 +2445,101 @@ abstract class BspTestDefinitions extends ScalaCliSuite
24452445
}
24462446
}
24472447
}
2448+
2449+
for {
2450+
isIntelliJ <- Seq(false, true)
2451+
clientDescription = if isIntelliJ then "IntelliJ" else "any client"
2452+
bspClientName = if isIntelliJ then "IntelliJ" else "test"
2453+
} {
2454+
def mainTargetOf(
2455+
remoteServer: b.BuildServer & b.ScalaBuildServer & b.JavaBuildServer & b.JvmBuildServer &
2456+
TestBspClient.WrappedSourcesBuildServer
2457+
): b.BuildTarget = {
2458+
val buildTargetsResp = remoteServer.workspaceBuildTargets().asScala.await
2459+
val targets = buildTargetsResp.getTargets.asScala.toSeq
2460+
targets.find(t => !t.getId.getUri.contains("-test")).getOrElse {
2461+
sys.error(s"No main build target found in ${targets.map(_.getId.getUri)}")
2462+
}
2463+
}
2464+
2465+
def baseDirectoryOf(target: b.BuildTarget): os.Path =
2466+
os.Path(Paths.get(new URI(target.getBaseDirectory)))
2467+
2468+
def collectSemDbFiles(root: os.Path): Seq[os.Path] =
2469+
os.walk(root)
2470+
.filter(_.last.endsWith(".semanticdb"))
2471+
.filter(p => !p.segments.exists(_ == "bloop-internal-classes"))
2472+
2473+
test(s"workspaceBuildTargets baseDirectory: default workspace ($clientDescription)") {
2474+
withBsp(
2475+
inputs = TestInputs(
2476+
os.rel / "Hello.scala" ->
2477+
"""object Hello {
2478+
| def main(args: Array[String]): Unit = println("Hello")
2479+
|}
2480+
|""".stripMargin
2481+
),
2482+
args = Seq("."),
2483+
bspClientName = bspClientName
2484+
) { (root, _, remoteServer) =>
2485+
Future {
2486+
val mainTarget = mainTargetOf(remoteServer)
2487+
val baseDir = baseDirectoryOf(mainTarget)
2488+
if isIntelliJ then expect(baseDir == root)
2489+
else expect(baseDir == root / Constants.workspaceDirName)
2490+
2491+
val compileResp =
2492+
remoteServer
2493+
.buildTargetCompile(new b.CompileParams(List(mainTarget.getId).asJava))
2494+
.asScala
2495+
.await
2496+
expect(compileResp.getStatusCode == b.StatusCode.OK)
2497+
2498+
val semDbFiles = collectSemDbFiles(root)
2499+
expect(semDbFiles.nonEmpty)
2500+
expect(semDbFiles.forall(_.segments.contains(Constants.workspaceDirName)))
2501+
}
2502+
}
2503+
}
2504+
2505+
if !Properties.isWin then
2506+
test(s"workspaceBuildTargets baseDirectory: non-writable workspace ($clientDescription)") {
2507+
val simpleHelloInputsInDir = TestInputs(
2508+
os.rel / "dir" / "Hello.scala" ->
2509+
"""object Hello {
2510+
| def main(args: Array[String]): Unit = println("Hello")
2511+
|}
2512+
|""".stripMargin
2513+
)
2514+
simpleHelloInputsInDir.fromRoot { root =>
2515+
os.perms.set(root / "dir", "r-xr-xr-x")
2516+
try
2517+
withBsp(
2518+
simpleHelloInputsInDir,
2519+
Seq("dir"),
2520+
reuseRoot = Some(root),
2521+
bspClientName = bspClientName
2522+
) {
2523+
(_, _, remoteServer) =>
2524+
Future {
2525+
val mainTarget = mainTargetOf(remoteServer)
2526+
val baseDir = baseDirectoryOf(mainTarget)
2527+
val expectedBaseDir = root / "dir"
2528+
expect(baseDir == expectedBaseDir)
2529+
2530+
val compileResp =
2531+
remoteServer
2532+
.buildTargetCompile(new b.CompileParams(List(mainTarget.getId).asJava))
2533+
.asScala
2534+
.await
2535+
expect(compileResp.getStatusCode == b.StatusCode.OK)
2536+
2537+
val semDbFilesUnderRoot = collectSemDbFiles(root)
2538+
expect(semDbFilesUnderRoot.isEmpty)
2539+
}
2540+
}
2541+
finally os.perms.set(root / "dir", "rwxr-xr-x")
2542+
}
2543+
}
2544+
}
24482545
}

modules/options/src/main/scala/scala/build/options/BuildOptions.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,22 @@ final case class BuildOptions(
5353

5454
import BuildOptions.JavaHomeInfo
5555

56+
/** When the build workspace was moved to a virtual directory (e.g. after
57+
* [[scala.build.input.Inputs.checkAttributes]] fallback) and the user did not set an explicit
58+
* semanticdb source root, use the given original workspace so semanticdb paths stay relative to
59+
* the user's project root.
60+
*/
61+
def withResolvedSemanticDbSourceRoot(originalWorkspace: os.Path): BuildOptions =
62+
if scalaOptions.semanticDbOptions.semanticDbSourceRoot.isEmpty then
63+
copy(scalaOptions =
64+
scalaOptions.copy(
65+
semanticDbOptions = scalaOptions.semanticDbOptions.copy(
66+
semanticDbSourceRoot = Some(originalWorkspace)
67+
)
68+
)
69+
)
70+
else this
71+
5672
lazy val platform: Positioned[Platform] =
5773
scalaOptions.platform.getOrElse(Positioned(List(Position.Custom("DEFAULT")), Platform.JVM))
5874

0 commit comments

Comments
 (0)