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.

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

Bazel Support #5138

Closed
7 tasks done
tanishiking opened this issue Apr 12, 2023 · 20 comments
Closed
7 tasks done

Bazel Support #5138

tanishiking opened this issue Apr 12, 2023 · 20 comments
Assignees
Labels
bazel BSP Generic BSP related tickets
Milestone

Comments

@tanishiking
Copy link
Member

tanishiking commented Apr 12, 2023

scalameta/metals-feature-requests#39

This issue aims to track the progress of enhancing Bazel support in the Metals. There are several tasks that need to be completed in order to achieve better integration with Bazel. The list of pending tasks is as follows:

@tanishiking tanishiking added BSP Generic BSP related tickets bazel labels Apr 12, 2023
@tanishiking tanishiking self-assigned this Apr 12, 2023
tanishiking added a commit to tanishiking/metals that referenced this issue Apr 13, 2023
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
@tanishiking
Copy link
Member Author

tanishiking commented Apr 13, 2023

It looks like bazel-bsp's projectview doesn't reflect to workspace/buildTarget https://youtrack.jetbrains.com/issue/BAZEL-377 and as a result, Metals will index all the build targets even though the presence of projectview.bazelproject file.

Nevermind, I misunderstood how to use projectview 😅 https://youtrack.jetbrains.com/issue/BAZEL-377 JetBrains/bazel-bsp#383

We can use Metals in Bazel project without project view feature, but I believe it is practically essential for a pretty large project.

tanishiking added a commit that referenced this issue Apr 14, 2023
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
@tanishiking
Copy link
Member Author

Regarding automatically install bazel-bsp

Tomasz Godzik — 04/22/2023 1:26 AM
I just remembered the message. Could we install bazel-bsp for users?
We can for sure download the artifacts, so maybe we can also generate the proper .bsp entry?

tanishiking — 04/22/2023 6:52 AM
Yes, actually Kamil's PR did that, we can run cs launch on Metals side
One things I worry about is

What should we do about projectview file https://github.com/JetBrains/bazel-bsp/blob/master/executioncontext/projectview/README.md it
doesn't necessarily put into a specific directory.
Automatic installation may cause a bit of troubles to users who want to pick up the files on some directories. 

https://discord.com/channels/632642981228314653/1098559553571917944/1099008392841924618

@tgodzik
Copy link
Contributor

tgodzik commented Apr 28, 2023

Coming back to this. I think for project view we should pick it up automatically if it's present at root and maybe at a setting otherwise? What do you think? How often would users have the projectview file in another place?

@nigredo-tori
Copy link

nigredo-tori commented Apr 29, 2023

Coming back to this. I think for project view we should pick it up automatically if it's present at root and maybe at a setting otherwise? What do you think? How often would users have the projectview file in another place?

As a personal experience, I have a few projects in a large monorepository, and each has its own projectview. I store those in each project's directories outside VCS, although I could probably move them to the workspace root. I frequently switch between projects, and sometimes I have to work on multiple projects simultaneously, so having to manually change a setting every time would not be great.

Ideally, I'd like Metals to detect projectview files automatically. E.g. if I open a file foo/src/bar.scala, it should notice the file foo/.projectview or some such, and pick up the corresponding build targets.

@tgodzik
Copy link
Contributor

tgodzik commented May 8, 2023

Ideally, I'd like Metals to detect projectview files automatically. E.g. if I open a file foo/src/bar.scala, it should notice the file foo/.projectview or some such, and pick up the corresponding build targets.

I think this is doable, maybe not as a first step, but as a follow up. @tanishiking what do you think, can we move with #3233 ?

@tanishiking
Copy link
Member Author

what do you think, can we move with #3233 ?

Yes, we can indeed move forward with #3233.
However, one thing to be mindful of is that Metals should not install bazel-bsp and overwrite any manually generated BSP files (if there already exists the bazel-bsp file).
This is particularly important if users have manually created a BSP configuration file using the -p <path/to/projectview_file> command. We need to ensure that any existing BSP configurations are not overwritten. 👍
Otherwise, yes we can move based on #3233


Ideally, I'd like Metals to detect projectview files automatically. E.g. if I open a file foo/src/bar.scala, it should notice the file foo/.projectview or some such, and pick up the corresponding build targets.

Tthis may not be feasible option.
The usage of a specific projectview requires the (re)starting of the BSP server with certain specific arguments (like the path to the projectview file).

The common scenario, as far as I understand, involves creating multiple projectview files for different components (A, B, C, etc.) and restarting the BSP server with an appropriate projectview file depending on the component being worked on.
Furthermore, projectview files aren't necessarily located in specific, predictable locations.

Therefore, it would be beneficial if Metals could provide a method for switching projectview file locations. Once the location is changed, the BSP server should restart with the newly specified projectview file.

@tgodzik
Copy link
Contributor

tgodzik commented May 11, 2023

@tanishiking do you want to finish #3233 or do you want someone to take over?

@nigredo-tori
Copy link

Tthis may not be feasible option.
The usage of a specific projectview requires the (re)starting of the BSP server with certain specific arguments (like the path to the projectview file).

Could Metals handle multiple simultaneous BSP servers? From what I understand, they might end up communicating to the same Bazel process under wraps, correct?

As a user, I don't really mind if a BSP server gets started behind the scenes whenever I open a file belonging to a component - as long as that doesn't involve something unnecessarily heavy, like recompiling all the relevant targets from scratch. I could also live with a separate Metals process for each component (spawned by Emacs as needed, with projectview selection also handled by Emacs), as long as those don't step on each other's toes (one Metals/BSP server process regenerating semanticdb or recompiling a common target shouldn't break the others).

Not sure whether any of this is even remotely feasible.

@tanishiking
Copy link
Member Author

@tanishiking do you want to finish #3233 or do you want someone to take over?

It may take some time, but can I finish up that? (if no one is interested in) 😃

@tgodzik
Copy link
Contributor

tgodzik commented May 12, 2023

Tthis may not be feasible option.
The usage of a specific projectview requires the (re)starting of the BSP server with certain specific arguments (like the path to the projectview file).

Could Metals handle multiple simultaneous BSP servers? From what I understand, they might end up communicating to the same Bazel process under wraps, correct?

As a user, I don't really mind if a BSP server gets started behind the scenes whenever I open a file belonging to a component - as long as that doesn't involve something unnecessarily heavy, like recompiling all the relevant targets from scratch. I could also live with a separate Metals process for each component (spawned by Emacs as needed, with projectview selection also handled by Emacs), as long as those don't step on each other's toes (one Metals/BSP server process regenerating semanticdb or recompiling a common target shouldn't break the others).

Not sure whether any of this is even remotely feasible.

We already do a similar thing for sbt meta builds, we have several BSP connection for each level of the build definition. Some work would be neccessary, but hopefully not too much. It's probably soemthing we should look at at some point

@tanishiking
Copy link
Member Author

bazel-bsp 2.7.1 introduces the fix for cleaning up previous diagnostics 🎉
https://github.com/JetBrains/bazel-bsp/blob/master/CHANGELOG.md

I will finish up #3233 to automatically install bazel-bsp 2.7.1 soon.
It would be ideal if we can detect any build configuration change in the workspace (including .bzl files), but detecting changes in BUILD, WORKSPACE. and BUILD.bazel would be a good starting point #5144 :)

@tanishiking
Copy link
Member Author

I spent some time to tweak #3233 so Metals automatically install bazel-bsp 2.7.1 and reindex the workspace when the configuration has changed.
I don't have a bandwidth for that right now, but will continue work on that later this month, maybe.

@aishfenton
Copy link

@tanishiking one thing I'm curious about, is how are you handling indexing the semanticdb files across Jars. One of the assumptions currently in BSP is that they all exist under a single output directory, which really isn't how Bazel works (i.e. instead it produces zillions of separate output directories for separate targets).

@aishfenton
Copy link

aishfenton commented Jul 13, 2023

Side note @tanishiking : I also started a separate Bazel BSP server project here, that I was aiming to be more made for Metals integration from the ground up. https://github.com/bazeltools/bsp4bazel. It's fully functional as BSP server, but I got busy on other stuff this year, so haven't had time to get back to it :( So quite a bit of work to finish integrating it into Metals fully. But let me know if useful for your efforts.

@nigredo-tori
Copy link

As I understand it, SemanticDB support just got merged into rules_scala in bazelbuild/rules_scala#1508 which supersedes bazelbuild/rules_scala#1467 mentioned in this issue's description.

@peterldowns
Copy link

I am so, so excited for this. Keep up the great work!

@ValdemarGr
Copy link

I have created a BSP implementation for Bazel and it works well in the repositories I have tested with (type information, goto, maven dependencies, streamed diagnostics and so on), but is very new.
Feedback is welcome.
https://github.com/ValdemarGr/mezel

@tgodzik
Copy link
Contributor

tgodzik commented Nov 30, 2023

Thanks for sharing! How does it compare to https://github.com/JetBrains/bazel-bsp ?

@ValdemarGr
Copy link

ValdemarGr commented Nov 30, 2023

I'll start off by saying I work in a project where we build Scala with Bazel, so I built the tool because I need it.
My goal is to have a good editor experience.

Some things that come to mind are:

  • mezel supports scala plugins
  • bazelbsp does not seem to propagate build targets, if a change in target B would cause an error in target A you won't see this error until you build target A (by saving the file or something alike).
    Mezel solves this by building everything and using bazel's BEP to stream diagnostics and semanticdb information into metals so errors will appear as soon as they are available, even for transitive build targets.
  • bazelbsp seems to either be moving slow or not prioritizing scala support:
    1. plugins are not passed to the scala compiler.
    2. I have not seen any work towards using generated semanticdb
    3. I cannot build any of my projects with bazelbsp 3.1.0 because of issues such as this
  • bazelbsp does not detect changes to the build graph (which mezel doesn't do either but this is the next thing I'm working on does now)
  • mezel is very new so there are rough edges that can lead to crashes.

@tgodzik
Copy link
Contributor

tgodzik commented Dec 4, 2023

Thanks for the larger picture! We will most likely finish the current Bazel PR, but we will make sure that it's also easy to use other tools and it's as easy as setting up .bsp/mezel.json.

I would prefer to keep Jetbrain's PR as a default for now since it's done by an organization and much more likely to keep being maintained. If it turns up to be not enough for users, we might be able to add some more work into Mezel. Especially if there is a company willing to sponsor as we are a bit tight in terms of a number of maintained projects and the number of developers we can keep on open source.

@tgodzik tgodzik closed this as completed Mar 13, 2024
@tgodzik tgodzik added this to the Metals v1.3.0 milestone Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel BSP Generic BSP related tickets
Projects
None yet
Development

No branches or pull requests

6 participants