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.

Sign up for GitHub

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

Jump to bottom

[fix] fix the @@ prefix issue if bazel.module and WORKSPACE are used together #534

Merged
merged 1 commit into from Mar 8, 2024
Merged

[fix] fix the @@ prefix issue if bazel.module and WORKSPACE are used together #534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 9 additions & 5 deletions bazelrunner/src/main/kotlin/org/jetbrains/bsp/bazel/bazelrunner/BazelInfo.kt
View file
Open in desktop
Expand Up @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions bazelrunner/src/test/kotlin/org/jetbrains/bsp/bazel/bazelrunner/BazelReleaseTest.kt
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ class BazelReleaseTest {

// then
release?.major shouldBe 4
release?.mainRepositoryReferencePrefix(false) shouldBe "//"
release?.isRelativeWorkspacePath("//abc") shouldBe true
}

@Test
Expand All @@ -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
Expand Down
8 changes: 3 additions & 5 deletions server/src/main/kotlin/org/jetbrains/bsp/bazel/server/paths/BazelPathsResolver.kt
View file
Open in desktop
Expand Up @@ -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 {
Expand All @@ -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]
Expand Down
5 changes: 2 additions & 3 deletions server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/BazelProjectMapper.kt
View file
Open in desktop
Expand Up @@ -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",
Expand Down Expand Up @@ -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(
Expand Down
29 changes: 15 additions & 14 deletions server/src/main/kotlin/org/jetbrains/bsp/bazel/server/sync/ProjectResolver.kt
View file
Open in desktop
Expand Up @@ -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
Expand Down Expand Up @@ -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) }
abrams27 marked this conversation as resolved.
Show resolved Hide resolved
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) }
Expand All @@ -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"
tgodzik marked this conversation as resolved.
Show resolved Hide resolved
else "@$it"
}
}.toList()

companion object {
Expand Down