bugfix: Fix the @@ prefix issue if bazel.module and WORKSPACE are used together #533
Conversation
tgodzik
force-pushed
the
fix-bazelmod
branch
from
cd9e5ec
to
8195423
Compare
|
Ok, it seems this hasn't broken anything. Opinions? This is also related to https://youtrack.jetbrains.com/issue/BAZEL-863/Bazel-7.x-will-always-try-to-create-module.bazel |
|
LGTM. I don't remember why we decided to use |
|
Do we have a project in tests that only uses BAZEL.module? I think it should still work with the changes, since it would be very rare to have a target with @ and @@ in the same workspace as far as I can see. |
| in 0..3 -> throw RuntimeException("Unsupported Bazel version, use Bazel 4 or newer") | ||
| in 4..5 -> "//" | ||
| else -> |
There was a problem hiding this comment.
This was basically a guess previously, since the targets can come with @ and @@ within a single workspace.
| @@ -68,7 +68,7 @@ class ProjectResolver( | |||
| val rootTargets = buildAspectResult.bepOutput.rootTargets().let { formatTargetsIfNeeded(it) } | |||
| val targets = measured( | |||
| "Parsing aspect outputs" | |||
| ) { targetInfoReader.readTargetMapFromAspectOutputs(aspectOutputs) } | |||
| ) { targetInfoReader.readTargetMapFromAspectOutputs(aspectOutputs).let { it.mapKeys { it.key.replace("@@", "@") } } } | |||
There was a problem hiding this comment.
Here we instead normalize anything to have single @, which should be safe since this seems to be only a target lookup structure.
|
with this change it's not possible to import this repo |
How does it break though? I didn't think it would break anything that worked previously 🤔 Any other ideas how to fix this? The main problem is that we cannot know if a project should have @ or @@ inside bazel-bsp. Why isn't that saved when exporting via aspect? |
|
If anyone needs a reproduction then https://github.com/susliko/metals-bazel should behave similarly to the private repo I am working on. We don't get any targets there be it Metals or Intellij. |
there are places where we reference the id directly, so it's still with |
|
Created an alternative approach here #534 since something weird happened with this PR when I pushed. |
It's not uncommon to have both BAZEL.module file and WORKSPACE, in that case Bazel BSP will not detect all targets, or in case of the repository I am working on it would detect none of them.
There doesn't seem to be any actual way to discover if a target should have a @ or @@ prefix, so instead I changed the behaviour to replace @@ with @, which should work in most cases.