[fix] fix the @@ prefix issue if bazel.module and WORKSPACE are…

… used together (#534)
JetBrains · Mar 8, 2024 · 86fd20b · 86fd20b
1 parent 3037649
commit 86fd20b
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 29 deletions.
diff --git a/bazelrunner/src/main/kotlin/org/jetbrains/bsp/bazel/bazelrunner/BazelInfo.kt b/bazelrunner/src/main/kotlin/org/jetbrains/bsp/bazel/bazelrunner/BazelInfo.kt
@@ -16,12 +16,16 @@ data class BazelRelease(
   val major: Int
 ) {
 
-  fun mainRepositoryReferencePrefix(isBzlModEnabled: Boolean) = when (major) {
+  fun isRelativeWorkspacePath(label: String) = when (major) {
     in 0..3 -> throw RuntimeException("Unsupported Bazel version, use Bazel 4 or newer")
-    in 4..5 -> "//"
-    else ->
-      if (isBzlModEnabled) "@@//"
-      else "@//"
+    in 4..5 -> label.startsWith("//")
+    else -> label.startsWith("@//") || label.startsWith("@@//")
+  }
+
+  fun stripPrefix(label: String) = when (major) {
+    in 0..3 -> throw RuntimeException("Unsupported Bazel version, use Bazel 4 or newer")
+    in 4..5 -> label.removePrefix("//")
+    else -> label.dropWhile { it == '@' }.removePrefix("//")
   }
 
   companion object {

diff --git a/bazelrunner/src/test/kotlin/org/jetbrains/bsp/bazel/bazelrunner/BazelReleaseTest.kt b/bazelrunner/src/test/kotlin/org/jetbrains/bsp/bazel/bazelrunner/BazelReleaseTest.kt
@@ -17,7 +17,7 @@ class BazelReleaseTest {
 
     // then
     release?.major shouldBe 4
-    release?.mainRepositoryReferencePrefix(false) shouldBe "//"
+    release?.isRelativeWorkspacePath("//abc") shouldBe true
   }
 
   @Test
@@ -27,7 +27,8 @@ class BazelReleaseTest {
 
     // then
     release?.major shouldBe 6
-    release?.mainRepositoryReferencePrefix(false) shouldBe "@//"
+    release?.isRelativeWorkspacePath("@//abc") shouldBe true
+    release?.isRelativeWorkspacePath("@@//abc") shouldBe true
   }
 
   @Test

diff --git a/server/src/main/kotlin/org/jetbrains/bsp/bazel/server/paths/BazelPathsResolver.kt b/server/src/main/kotlin/org/jetbrains/bsp/bazel/server/paths/BazelPathsResolver.kt
@@ -90,8 +90,7 @@ class BazelPathsResolver(private val bazelInfo: BazelInfo) {
         Paths.get(bazelInfo.execRoot, path)
 
     fun isRelativeWorkspacePath(label: String): Boolean {
-        val prefix = bazelInfo.release.mainRepositoryReferencePrefix(bazelInfo.isBzlModEnabled)
-        return label.startsWith(prefix)
+        return bazelInfo.release.isRelativeWorkspacePath(label)
     }
 
     fun extractExternalPath(label: String): String {
@@ -107,10 +106,9 @@ class BazelPathsResolver(private val bazelInfo: BazelInfo) {
     }
 
     fun extractRelativePath(label: String): String {
-        val prefix = bazelInfo.release.mainRepositoryReferencePrefix(bazelInfo.isBzlModEnabled)
 
-        require(isRelativeWorkspacePath(label)) { "$label didn't start with $prefix" }
-        val labelWithoutPrefix = label.substring(prefix.length)
+        require(bazelInfo.release.isRelativeWorkspacePath(label)) { "$label didn't start with correct prefix" }
+        val labelWithoutPrefix = bazelInfo.release.stripPrefix(label)
         val parts = labelWithoutPrefix.split(":".toRegex()).toTypedArray()
         require(parts.size == 2) { "Label $label didn't contain exactly one ':'" }
         return parts[0]

diff --git a/server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/BazelProjectMapper.kt b/server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/BazelProjectMapper.kt
@@ -363,7 +363,7 @@ class BazelProjectMapper(
     }
 
   private fun isWorkspaceTarget(target: TargetInfo): Boolean =
-    target.id.startsWith(bazelInfo.release.mainRepositoryReferencePrefix(bazelInfo.isBzlModEnabled)) &&
+    bazelInfo.release.isRelativeWorkspacePath(target.id) &&
       (hasKnownSources(target) ||
         target.kind in setOf(
         "java_library",
@@ -518,8 +518,7 @@ class BazelProjectMapper(
     targetInfo.envInheritList.associateWith { System.getenv(it) }
 
   private fun removeDotBazelBspTarget(targets: List<String>): List<String> {
-    val prefix = bazelInfo.release.mainRepositoryReferencePrefix(bazelInfo.isBzlModEnabled) + ".bazelbsp"
-    return targets.filter { !it.startsWith(prefix) }
+    return targets.filter { bazelInfo.release.isRelativeWorkspacePath(it) && !bazelInfo.release.stripPrefix(it).startsWith(".bazelbsp") }
   }
 
   private fun createRustExternalModules(

diff --git a/server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/ProjectResolver.kt b/server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/ProjectResolver.kt
@@ -2,6 +2,7 @@ package org.jetbrains.bsp.bazel.server.sync
 
 import org.eclipse.lsp4j.jsonrpc.CancelChecker
 import org.jetbrains.bsp.bazel.bazelrunner.BazelInfo
+import org.jetbrains.bsp.bazel.info.BspTargetInfo
 import org.jetbrains.bsp.bazel.logger.BspClientLogger
 import org.jetbrains.bsp.bazel.server.bsp.managers.BazelBspAspectsManager
 import org.jetbrains.bsp.bazel.server.bsp.managers.BazelBspAspectsManagerResult
@@ -55,20 +56,20 @@ class ProjectResolver(
     val buildAspectResult = measured(
       "Building project with aspect"
     ) { buildProjectWithAspect(cancelChecker, workspaceContext) }
+    val aspectOutputs = measured(
+        "Reading aspect output paths"
+    ) { buildAspectResult.bepOutput.filesByOutputGroupNameTransitive(BSP_INFO_OUTPUT_GROUP) }
+    val targets = measured(
+        "Parsing aspect outputs"
+    ) { targetInfoReader.readTargetMapFromAspectOutputs(aspectOutputs).let { it } }
     val allTargetNames =
       if (buildAspectResult.isFailure)
         measured(
           "Fetching all possible target names"
-        ) { formatTargetsIfNeeded(bazelBspFallbackAspectsManager.getAllPossibleTargets(cancelChecker)) }
+        ) { formatTargetsIfNeeded(bazelBspFallbackAspectsManager.getAllPossibleTargets(cancelChecker), targets) }
       else
         emptyList()
-    val aspectOutputs = measured(
-      "Reading aspect output paths"
-    ) { buildAspectResult.bepOutput.filesByOutputGroupNameTransitive(BSP_INFO_OUTPUT_GROUP) }
-    val rootTargets = buildAspectResult.bepOutput.rootTargets().let { formatTargetsIfNeeded(it) }
-    val targets = measured(
-      "Parsing aspect outputs"
-    ) { targetInfoReader.readTargetMapFromAspectOutputs(aspectOutputs) }
+    val rootTargets = buildAspectResult.bepOutput.rootTargets().let { formatTargetsIfNeeded(it, targets) }
     return measured(
       "Mapping to internal model"
     ) { bazelProjectMapper.createProject(targets, rootTargets.toSet(), allTargetNames, workspaceContext, bazelInfo) }
@@ -82,16 +83,16 @@ class ProjectResolver(
       listOf(BSP_INFO_OUTPUT_GROUP, ARTIFACTS_OUTPUT_GROUP, RUST_ANALYZER_OUTPUT_GROUP)
     )
 
-  private fun formatTargetsIfNeeded(targets: Collection<String>): List<String> =
+  private fun formatTargetsIfNeeded(targets: Collection<String>, targetsInfo: Map<String, BspTargetInfo.TargetInfo >): List<String> =
     when (bazelInfo.release.major) {
       // Since bazel 6, the main repository targets are stringified to "@//"-prefixed labels,
       // contrary to "//"-prefixed in older Bazel versions. Unfortunately this does not apply
-      // to BEP data, probably due to a bug, so we need to add the "@" prefix here.
+      // to BEP data, probably due to a bug, so we need to add the "@" or "@@" prefix here.
       in 0..5 -> targets.toList()
-      else ->
-        if (bazelInfo.isBzlModEnabled)
-          targets.map { "@@$it" }
-        else targets.map { "@$it" }
+      else -> targets.map {
+          if (targetsInfo.contains("@@$it")) "@@$it"
+          else "@$it"
+        }
     }.toList()
 
   companion object {