feature: Support Bazel as a build tool #3233

kpodsiad · 2021-10-27T13:38:43Z

Add support for Bazel.
I created a simple project which uses Bazel and could be used for testing this integration.

Known todos:

blocked by ~~SemanticDB Generation (gives find references) JetBrains/bazel-bsp#105~~ SemanticDB phases bazelbuild/rules_scala#952 - without semanticDB metals capabilities are very limited. However, one can add to his BUILD scalac option which enables semanticdb plugin - scalacopts = ["-Xplugin:<path_to_semanticdb_jar>", "-Yrangepos", "-P:semanticdb:targetroot:<path_to_generated_semanticdb>"]
improve user experience for build servers that cannot generate bloop config (like bazel). Now, with fresh repo users have to execute Attempt to generate bsp config for build tool and then switch build server. This could be done automatically, I think.
diagnostics aren't refreshed after compilation and they're displayed for wrong file - Invalid diagnostics JetBrains/bazel-bsp#111

kpodsiad · 2021-10-28T10:25:30Z

I managed to generate SemanticDB in my test-project but it turns out that bazel doesn't have any folder where it stores compilation artifacts (at least I didn't find anything + metals decode command didn't point me to any useful directory), everything is loaded directly to the jar in bazel-out/bazel-bindirectory. Both classfiles and semanticDB files are stored in this JAR when metals treat this as a directory and tries to reach the content inside JAR :/

@kpodsiad

While we can generate `.bsp/bazelbsp.json` by `cs launch org.jetbrains.bsp;bazel-bsp:2.6.1 -M org.jetbrains.bsp.bazel.instal.Install` https://github.com/JetBrains/bazel-bsp#easy-way-coursier Metals couldn't connect to the bazel-bsp because it wan't autoConnectable. This change enable Metals to see bazel-bsp as autoConnectable, and Metals will spawn and connct to bazel bsp if there's `.bsp/bazelbsp.json`. As @kpodsiad did in scalameta#3233 we should - reindex if the build configuration changed - I'm wondering we should check all *.bzl files and WORKSPACE and BUILD / BUILD.bazel files. And maybe we should honor the project view settings. - Auto install bazel-bsp when we see `WORKSPACE` file. scalameta#5138 partially fix for scalameta#5064

@kpodsiad

While we can generate `.bsp/bazelbsp.json` by `cs launch org.jetbrains.bsp;bazel-bsp:2.6.1 -M org.jetbrains.bsp.bazel.instal.Install` https://github.com/JetBrains/bazel-bsp#easy-way-coursier Metals couldn't connect to the bazel-bsp because it wan't autoConnectable. This change enable Metals to see bazel-bsp as autoConnectable, and Metals will spawn and connct to bazel bsp if there's `.bsp/bazelbsp.json`. As @kpodsiad did in #3233 we should - reindex if the build configuration changed - I'm wondering we should check all *.bzl files and WORKSPACE and BUILD / BUILD.bazel files. And maybe we should honor the project view settings. - Auto install bazel-bsp when we see `WORKSPACE` file. #5138 partially fix for #5064

If `WORKSPACE` is found, and `.bsp/bazelbsp.json` is not found, install bazel-bsp 2.7.1 and write the config file to `.bsp/bazelbsp.json`.

we should update metals-vscode so it fires textDocument/didSave

tanishiking · 2023-06-19T04:22:17Z

I added some changes so that

Metals will automatically install bazel-bsp 2.7.1 if WORKSPACE file is found && .bsp/bazelbsp.json isn't found.
Metals reloads workspace if Bazel build configuration has changed (though we need changes in metals-vscode too, so it fires textDocument/didSave on change Bazel related build configurations).

have to fix tests

tgodzik · 2023-06-19T13:45:15Z

I added some changes so that

Metals will automatically install bazel-bsp 2.7.1 if WORKSPACE file is found && .bsp/bazelbsp.json isn't found.

Metals reloads workspace if Bazel build configuration has changed (though we need changes in metals-vscode too, so it fires textDocument/didSave on change Bazel related build configurations).

have to fix tests

Do you need any help to finish? If you don't have time we can take over.

tanishiking · 2023-06-20T02:09:19Z

Thanks @tgodzik maybe it's better someone picks up to take over this and finishing it up :)

Scope

Metals should auto install bazel-bsp using coursier (.bsp/bazelbsp.json file should be written) for Bazel project.
- Currently users have to manually install it https://github.com/JetBrains/bazel-bsp#easy-way-coursier
- We should be able to detect Bazel project by checking there's a WORKSPACE file at project root.
- Note that we shouldn't overwrite existing .bsp/bazelbsp.json because it may contain project view setting
Metals should show pop-up to prompt re-index the workspace when build configuration has changed.
- I guess detecting change in WORKSPACE, BUILD, BUILD.bazel, *.bzl files are sufficient for now.
- metals-vscode to fire textDocument/didSave on change above files: not 100% sure, but maybe we have to update https://github.com/scalameta/metals-vscode/blob/318862ece3628641b1799def7606167bbaf63068/packages/metals-vscode/package.json#L49-L57 ?
- server side, update workspace on didSave against above files.
Fix tests: looks like cs install fails because there's no cs command on CI.

SemanticDB support shouldn't be a scope of this PR.

Future work

Documentation: how to use Metals on Bazel project.
It would be great if we can bump bazel-bsp's version by scala-steward or something.
- Actually, bazel-bsp 2.7.2 is already out...

You should be able to test against a small Bazel project here: https://github.com/tanishiking/bazel-playground/tree/main/01-scala-tutorial By default, rules_scala will use Scala 2.13.6.

tanishiking · 2023-06-30T09:20:08Z

@jkciesluk kindly took over the rest of work 💯

When aths where empty, FileWatcher was watching the entire file system

Adds two improvements: 1. After changing BUILD, WORKSPACE or *.bzl file, user will be prompted to import build. 2. If .bazelbsp is not present, user will be asked if they want to create it (the same way as with .bloop)

Previously cheksum was not persisted there, so first save of build file would result in unnecessary reload

Adds tests for connecting to build server using GenerateBspConfig and ImportBuild commands

kasiaMarek

A few generic comments:

It would be great to have some docs about bazel suport.
It seems we don't have semanticdb and so on for bazel, and this results in multiple problems detected, it'd be great to let the user know, that this is expected for bazel not actual problem with their build. So maybe a different message?
When I tired changing the scala version I got:

bazelbsp does not support `workspace/reload`, unable to reload

in logs, and those changes weren't actually imported. Am I missing something?

metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala

metals/src/main/scala/scala/meta/internal/builds/BazelBuildTool.scala

metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala

kasiaMarek · 2023-10-04T11:46:58Z

metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala

+      !tables.buildServers
+        .selectedServer()
+        .contains(path.filename.stripSuffix(".json"))


Can it ever actually switch between build servers? Or can we check tables.buildServers.isEmpty && no bloop instead?

I think it works good? If the currently chosen server is the same as the one that appeared in the .bsp, we don't want to call quickConnectToBuildServer, because it's most likely already been called in other place, but if some other build server appears we want to notify the user?

If you have an explicitly chosen build server (saved in tables.buildServers) and call quickConnectToBuildServer it will connect to that choice and ignore other possibilities. So you'll just reconnect to the same build server.

So maybe if the newly added build tool is different from the one in tables.buildServers, we can clean this table and then call quickConnectToBuildServer?

If someone adds sbt you'll have to call slowConnect for the bloop config to generate.

We can wait for someone invoking Switch build server command in that case I think. If they are already using a bsp server. This will be a rare enough scenario.

metals/src/main/scala/scala/meta/internal/metals/Compilations.scala

metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala

kasiaMarek · 2023-10-04T12:36:22Z

tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala

+      _ <- server.fullServer
+        .didChangeWatchedFiles(


Is that needed? Shouldn't the file watcher detect the change?

File watcher detects changes from the other didChangeWatchedFiles, but this change is from lsp workspace/didChangeWatchedFiles. I'm not sure why we need both and which notifications should belong where

My understanding is that we need both, since not all editors do workspace/didChangeWatchedFiles (at least not reliably), so workspace/didChangeWatchedFiles is sort of "additional". @tgodzik, right?

That's right, at least from what I remember. For build tool files we use the LSP since this should be fairly reliable.

metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala

metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala

metals/src/main/scala/scala/meta/internal/metals/Compilations.scala

tgodzik · 2023-11-10T17:59:37Z

metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala

@@ -343,7 +343,8 @@ object MetalsEnrichments

    def isDependencySource(workspace: AbsolutePath): Boolean = {
      (isLocalFileSystem(workspace) &&
-        isInReadonlyDirectory(workspace)) || isJarFileSystem
+        (isInReadonlyDirectory(workspace) || isInTmpDirectory(workspace))) ||


When do we put sources into tmp?

In ScalafixProvier.load(), but looks like this check is not needed anymore, I will remove it.

tgodzik · 2023-11-10T18:01:56Z

metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala

+      !tables.buildServers
+        .selectedServer()
+        .contains(path.filename.stripSuffix(".json"))


We can wait for someone invoking Switch build server command in that case I think. If they are already using a bsp server. This will be a rare enough scenario.

tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala

tgodzik · 2024-01-03T14:19:44Z

It also looks like the presentation compiler is not started with the right classpath, I can't see any hover or completions from dependency module (like Hello in the sample).

tgodzik · 2024-01-03T14:20:33Z

metals/src/main/scala/scala/meta/internal/metals/Messages.scala

+    def multipleProblemsDetected: String =
+      s"Multiple problems detected in your build."
+
+    def bazelNavigation: String =


Could we suggest the flag to use to enable it?

Currently there is no easy way to enable this, it would require changes in both metals and the users project. I think its best to work on it in a follow up.

tgodzik

Two minor things otherwise LGTM

metals/src/main/scala/scala/meta/internal/builds/BazelBuildTool.scala

tgodzik · 2024-01-12T13:09:19Z

metals/src/main/scala/scala/meta/internal/metals/doctor/ProblemResolver.scala

@@ -176,16 +176,30 @@ class ProblemResolver(
      futureMessage,
    ).flatten

+    def noSemanticDBMessages = {


Could we instead create bazel message instead of the basic semanticdb ones if the current build server is bazel?

If I understand correctly, we do (see Messages.CheckDoctor.bazelNavigation)

What I mean is that noSemanticDBMessages is used to figure out if there are no semanticdb messages, why do we need this as a fllback, instead of checking for semanticdb flags together with bazel?

Ah right, I will try to change that

tgodzik

LGTM!

tgodzik · 2024-01-15T20:36:35Z

Let's make sure we squash here, great work @jkciesluk ! 🚀

Support Bazel as a build tool - wip

3519a46

tgodzik mentioned this pull request Sep 1, 2022

Bazel support scalameta/metals-feature-requests#39

Closed

ckipp01 added BSP Generic BSP related tickets bazel labels Jan 2, 2023

tanishiking mentioned this pull request Apr 12, 2023

Bazel Support #5138

Closed

7 tasks

tanishiking mentioned this pull request Apr 13, 2023

Auto spawn/connect to bazel-bsp if it's installed #5139

Merged

tanishiking mentioned this pull request Apr 14, 2023

Reload workspace and reindex on Bazel build configuration has changed #5144

Closed

tanishiking added 3 commits June 16, 2023 14:09

Merge branch 'main' into bazel-bsp

98f5624

Auto-install bazel-bsp

c73f1fa

If `WORKSPACE` is found, and `.bsp/bazelbsp.json` is not found, install bazel-bsp 2.7.1 and write the config file to `.bsp/bazelbsp.json`.

Refresh workspace when build changed

84f4baf

we should update metals-vscode so it fires textDocument/didSave

Merge remote-tracking branch 'origin/main' into bazel-bsp

1c354d4

tanishiking assigned jkciesluk Jun 30, 2023

jkciesluk added 5 commits July 18, 2023 13:58

fix: Fix PathTrie with empty paths

dc18814

When aths where empty, FileWatcher was watching the entire file system

feat: Detect changes in Bazel-related files

1d58063

Adds two improvements: 1. After changing BUILD, WORKSPACE or *.bzl file, user will be prompted to import build. 2. If .bazelbsp is not present, user will be asked if they want to create it (the same way as with .bloop)

improvement: Persist checksum on connecting to new build server

bb878d8

Previously cheksum was not persisted there, so first save of build file would result in unnecessary reload

improvement: Add basic BazelBsp test

9d9e769

improvement: Strip ANSI coloring from BazelBsp logs

716ae39

jkciesluk force-pushed the bazel-bsp branch from 5849530 to e8f8361 Compare July 18, 2023 16:29

jkciesluk added 2 commits July 19, 2023 09:08

Merge remote-tracking branch 'origin/main' into bazel-bsp

855d9d6

improvement: Add Bazel slow tests to CI

8e42fff

jkciesluk force-pushed the bazel-bsp branch from e8f8361 to 035a81b Compare July 19, 2023 09:25

improvement: Add more test for BazelBsp

f8aa610

Adds tests for connecting to build server using GenerateBspConfig and ImportBuild commands

jkciesluk force-pushed the bazel-bsp branch from 035a81b to f8aa610 Compare July 19, 2023 09:44

kasiaMarek reviewed Oct 4, 2023

View reviewed changes

Merge branch 'main' into bazel-merge

d5157e2

tgodzik reviewed Nov 10, 2023

View reviewed changes

jkciesluk added 2 commits December 18, 2023 16:45

Merge branch 'main' into bazel-merge

c81c453

Merge branch 'main' into bazel-merge

96071fa

jkciesluk force-pushed the bazel-bsp branch 2 times, most recently from 1648f50 to e5ec695 Compare January 2, 2024 13:13

jkciesluk requested review from kasiaMarek and tgodzik January 3, 2024 09:12

tgodzik changed the title ~~[WIP] Support Bazel as a build tool~~ Support Bazel as a build tool Jan 3, 2024

tgodzik changed the title ~~Support Bazel as a build tool~~ feature: Support Bazel as a build tool Jan 3, 2024

tgodzik requested changes Jan 3, 2024

View reviewed changes

tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala Show resolved Hide resolved

tgodzik reviewed Jan 3, 2024

View reviewed changes

jkciesluk added 2 commits January 5, 2024 10:33

Merge branch 'main' into bazel-merge

cb36ea8

bugfix: Ignore non scala or java build targets

82af4be

jkciesluk force-pushed the bazel-bsp branch from e5ec695 to 82af4be Compare January 5, 2024 09:34

jkciesluk added 2 commits January 10, 2024 08:54

Merge branch 'main' into bazel-merge

a56ae7a

improvement: Look for projectview in ijwb directory

f8b9ea5

jkciesluk requested a review from tgodzik January 11, 2024 15:36

improvement: Add diagnostics check to bazel tests

c360dcf

tgodzik approved these changes Jan 12, 2024

View reviewed changes

jkciesluk added 2 commits January 15, 2024 14:59

improvement: Warning message for no semanticdb in bazel projects

a2f451c

improvement: Download bazelbsp dependency

c631604

jkciesluk force-pushed the bazel-bsp branch from 6e162a6 to c631604 Compare January 15, 2024 14:24

jkciesluk requested a review from tgodzik January 15, 2024 15:21

tgodzik approved these changes Jan 15, 2024

View reviewed changes

Merge branch 'main' into bazel-bsp

504f703

tgodzik merged commit 76caf3b into scalameta:main Jan 16, 2024
24 of 26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: Support Bazel as a build tool #3233

feature: Support Bazel as a build tool #3233

kpodsiad commented Oct 27, 2021 •

edited by tgodzik

Loading

kpodsiad commented Oct 28, 2021 •

edited

Loading

tanishiking commented Jun 19, 2023 •

edited

Loading

tgodzik commented Jun 19, 2023

tanishiking commented Jun 20, 2023 •

edited

Loading

tanishiking commented Jun 30, 2023

kasiaMarek left a comment

kasiaMarek Oct 4, 2023

jkciesluk Oct 18, 2023

kasiaMarek Oct 18, 2023

jkciesluk Nov 6, 2023

kasiaMarek Nov 7, 2023

tgodzik Nov 10, 2023

kasiaMarek Oct 4, 2023

jkciesluk Oct 18, 2023

kasiaMarek Oct 18, 2023

tgodzik Nov 10, 2023

tgodzik Nov 10, 2023

jkciesluk Jan 2, 2024

tgodzik Nov 10, 2023

tgodzik commented Jan 3, 2024

tgodzik Jan 3, 2024

jkciesluk Jan 11, 2024

tgodzik left a comment

tgodzik Jan 12, 2024

jkciesluk Jan 12, 2024

tgodzik Jan 12, 2024

jkciesluk Jan 12, 2024

tgodzik left a comment

tgodzik commented Jan 15, 2024

feature: Support Bazel as a build tool #3233

feature: Support Bazel as a build tool #3233

Conversation

kpodsiad commented Oct 27, 2021 • edited by tgodzik Loading

kpodsiad commented Oct 28, 2021 • edited Loading

tanishiking commented Jun 19, 2023 • edited Loading

tgodzik commented Jun 19, 2023

tanishiking commented Jun 20, 2023 • edited Loading

Scope

Future work

tanishiking commented Jun 30, 2023

kasiaMarek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgodzik commented Jan 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgodzik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgodzik left a comment

Choose a reason for hiding this comment

tgodzik commented Jan 15, 2024

kpodsiad commented Oct 27, 2021 •

edited by tgodzik

Loading

kpodsiad commented Oct 28, 2021 •

edited

Loading

tanishiking commented Jun 19, 2023 •

edited

Loading

tanishiking commented Jun 20, 2023 •

edited

Loading