Allow to choose Bloop or BSP in case of build tools supporting both

[tgodzik]

With sbt 1.4.0 we will get an automatic generation of .bsp/sbt.json file in case of running sbt. This will cause Metals to only pick up sbt in case of no .bloop files or only choose Bloop if those exist.

I propose:

1. When detecting both .bloop and .bsp/sbt.json files we show a message request with a choice and remember it as it is in case of multiple build tools
2. When detecting sbt, we offer to either import Bloop or sbt, which will do the fist run, which will generate the needed json file - this should only be supported for sbt versions starting with 1.4.0
3. On connection to sbt server we should increase the timeout, since it might take a while for sbt server to be up.

It would be best to do it all before the release of sbt 1.4.0, since we current behavior might not work well there.

Thoughts? Ideas?

[tgodzik]

Additional problem is reloading sbt server, but that should be solved with sbt/sbt#5783

Or we could run manually sbt -client reload instead of reimporting.

[tgodzik]

Another issue will be getting semanticdb to work with sbt. There is a new setting for sbt:

Global / semanticdbEnabled := true

we should at least display in the doctor to add it it build.sbt. Alternatively, we could try to work on something similar to Bloop, where we create a new file .bloop/bloop.settings.json and if it exists we automatically add the needed options. That however might be harder to push on sbt.

Edit:
We could automatically add metals.sbt to root with Global / semanticdbEnabled := true which might be a good choice since no longer add .bloop and other metals.sbt files in that case, so it will only be that one file.

[gabro]

From a UX point of view, I'm a bit worried that this will confuse users. Most users consider bloop as a Metals implementation details, so they wouldn't know what to choose between sbt and bloop.

I'm assuming this scenario happens when an existing Metals user upgrades to sbt 1.4.0?

What's the status of BSP in sbt? Is it on-par with bloop? What are the advantages of one over the other?

I think the ideal scenario would be for Metals to pick one and use the other as a fallback, because I don't think most users will have enough information to answer a "bloop vs sbt" question when prompted.

[gabro]

Note that this is different than the multiple build tools scenario, since users of repos with multiple build tools (such as a Mill + Sbt project) are generally aware of this and can choose appropriately.

The specific BSP implementation is instead a much more obscure detail, that's "IDE-specific"

[ckipp01]

I think the ideal scenario would be for Metals to pick one and use the other as a fallback, because I don't think most users will have enough information to answer a "bloop vs sbt" question when prompted.

This was actually what I was thinking as well. I would argue that from a purely UX viewpoint, Metals should sort of choose the better (maybe a bit subjective) option automatically, while still allowing the user to use an alternative if they want. For an sbt build, if the choice to use sbt also as a build server is the smoothest option for the user, then I'd just automatically choose that and start the build import process. Vice versa, if the process with Bloop is smoother, then I'd go that route.

[olafurpg]

I personally think the Bloop UX is currently better and should remain the default. For the sbt integration to reach parity we should at least investigate how to

• automatically enable SemanticDB. I think the best way to achieve this is to create a new sbt-metals plugin that configures the correct build settings, semanticdbEnabled :=, semanticdbVersion := semanticdbOptions += s"-P:semanticdb:sourceroot:${baseDirectory.in(ThisBuild).value}", make sure we only do this for supported Scala versions, etc
• provide completions for build.sbt files
• write integration tests against sbt's BSP server. Our repo currently tests primarily with Bloop.
• make sure that sbt progress notifications are correct. The last time I tried the sbt bsp server it seemed like the compilation was taking forever.
• provide a user configuration option that allows users to pick their default BSP server so we don't prompt users all the time with the same question

I want to stress that I am very excited about having native BSP support in sbt and I'm very much looking forward to use it :) A lot of work has gone into the current Bloop integration and we should be careful not to rush and switch until we can provide similar quality.

[tgodzik]

Good points! So currently Bloop is a smoother option and it supports run/debug, which sbt's BSP does not. However, I noticed Bloop is having some issues with Scala 3 and it's sometimes hanging (people need to kill the Bloop server manually) and I haven't been able to figure it out. In that case people might want to use sbt bsp support. We could do it a bit differently then:

• choose Bloop automatically
• add a command to connect via sbt and remember the choice (or user configuration)

This will make it available for the power users, but not confuse other. What do you think about it?

automatically enable SemanticDB

There is a new setting in sbt Global / semanticdbEnabled := true - we can just create a metals.sbt file in the main directory or indeed do an sbt plugin that would do it for us and can be applied in project/metals.sbt, which would mean it's very similar to Bloop integration.

provide completions for build.sbt files

I agree, it seems weird sbt doesn't send us targets for the meta builds. But that needs a bit more work still.

write integration tests against sbt's BSP server. Our repo currently tests primarily with Bloop.

I would do that when we implement the additional command or user configuration.

make sure that sbt progress notifications are correct.

I think this is still a bit buggy, so best to keep Bloop as default.

provide a user configuration option that allows users to pick their default BSP server so we don't prompt users all the time with the same question

User config could be a good alternative to running an sbt command to enable it. I think it might be better.

A lot of work has gone into the current Bloop integration and we should be careful not to rush and switch until we can provide similar quality.

I agree, I just want to provide an easy way for people to start testing it without tinkering too much.

[tgodzik]

TLDR;

Let's keep Bloop default, add an option to run sbt.

[olafurpg]

There is a new setting in sbt Global / semanticdbEnabled := true - we can just create a metals.sbt file in the main directory or indeed do an sbt plugin that would do it for us and can be applied in project/metals.sbt, which would mean it's very similar to Bloop integration.

It's not safe to enable the global option for any arbitrary build since the user may be on an unsupported Scala version. An sbt-metals plugin can override projectSettings and ensure we add the correct settings (semanticdb version, set sourceroot compiler option) to the appropriate projects.

User config could be a good alternative to running an sbt command to enable it. I think it might be better.

If you think it helps, we can have a command to enable/disable the user option. But I think it's best to keep the user option as the single source of truth whether the user prefers sbt BSP.

[tgodzik]

Related issue: https://github.com/scalameta/metals-feature-requests/issues/136

[kpbochenek]

Okay we would like for now to favor bloop and allow users to explicitly enable sbt (maybe for testing, maybe because bloop gives them problems, scala3 works better?).

Right now we will use BSP that we were previously using(we store that in db) which is most likely bloop and we are okay here.

But for unimported project our logic is to favor .bsp entries.
.bsp/sbt.json will only be generated when sbt v>=1.4.0-M1 will be started.

If you import project in metals before you start sbt we should be okay.

But then if you let's say clone a scala project, start sbt then open it in VSCode - we will use sbt bsp.

As you said later we can prompt user for a choice if both bsps provide the same functionality but for now we need to resolve this.

Does that make sense?:
Ignore .bsp directory in automatic bsp resolution(or .bsp/sbt.json explicitly). Always start using bloop, allow with dedicated command to switch bloop servers for users that know they want it

[kpbochenek]

About using sbt as bsp backend then:

• firstly verify build tool is sbt
• if .bsp/sbt.json is present we know sbt can work as bsp. If it isn't present sbt could simply not be started yet but might be able to provide bsp. In that case we would have to:
  a) check project/build.properties and get version and assume >= 1.4.0-M1 (sounds a little error prone)
  b) start sbt and check if it generated .bsp/sbt.json (more resilent, but running sbt will take ages)
• now if we have .bsp/sbt.json we need to add semanticdbEnabled := true(and potentially other settings). It can be done via sbt-plugin, should be okay.
• now we can have 2 situations:
  a) sbt already running with semDbEnabled => just connect?
  b) sbt already running - !semDbEnabled => connect, reload? is that possible?
  c) sbt not running - start and connect?
• when we connect to sbt that was started by user it can be killed, in that case start new sbt and connect?

After talking with @tgodzik wanted to try give it a go but I need better understanding what expectations would be to not waste my time coding something that would not work as we expect

[olafurpg]

now if we have .bsp/sbt.json we need to add semanticdbEnabled := true(and potentially other settings). It can be done via sbt-plugin, should be okay.

Just to repeat the comment above, it's not safe to set semanticdbEnable := true by default. I think we will need a more fine grained solution that respects the user's Scala version and also sets the correct SemanticDB version #2049 (comment)

[kpbochenek]

okay so

1. unsupported scalaVersion - like old scala 2.11 version? e.g. 2.11.5? I would like to test it and see what happens.

2. semanticdb version - there is a legacy, v3 and v4 as I see it, why would we not want to use latest version? I understand that tools generating semanticdb generate it in only one way and client(metals) needs to be able to parse all 3 versions?

3. set sourceroot compiler - don't know what that is :/

Maybe some of the work can be done directly in sbt?

[tgodzik]

okay so

1. unsupported scalaVersion - like old scala 2.11 version? e.g. 2.11.5? I would like to test it and see what happens.
2. semanticdb version - there is a legacy, v3 and v4 as I see it, why would we not want to use latest version? I understand that tools generating semanticdb generate it in only one way and client(metals) needs to be able to parse all 3 versions?
3. set sourceroot compiler - don't know what that is :/

Maybe some of the work can be done directly in sbt?

We should reuse the earlier metals-sbt plugin, which already added all the needed parameters. This time we can easily add it in project/metals.sbt and with scalameta version I would follow the same logic as normally. Anything we don't support we should warn about.

[tgodzik]

I we should refactor the logic to have two working modes:
a) DEFAULT - use Bloop and suggest importing via Bloop without any mention of BSP
b) have a command or tree view button to connect via BSP:

• if a bsp json exists then just use that, but with an increased timeout (default 5 seconds will not be enough)
• if it's an sbt build we should:
  1. Add metals.sbt file with the sbt-metals plugin that will add semanticdb options manually to each project.
  2. Run sbt in a server mode which should also generate the json files and use it to connect.
  3. Connect to the server.
  4. We should be able to reload on any changes by running sbt -client reload (needs to be tested.)

b) should also be remembered in the settings, so that users who want it will not need to connect manually each time and we should also be able to reset that settings.

a) check project/build.properties and get version and assume >= 1.4.0-M1 (sounds a little error prone)

The sbt b) option should only be allowed on >= 1.4.0

• now if we have .bsp/sbt.json we need to add semanticdbEnabled := true(and potentially other settings). It can be done via sbt-plugin, should be okay.
• now we can have 2 situations:
  a) sbt already running with semDbEnabled => just connect?
  b) sbt already running - !semDbEnabled => connect, reload? is that possible?
  c) sbt not running - start and connect?

We should do it the same way we did it with the previous plugin, but within metals.sbt file.

• when we connect to sbt that was started by user it can be killed, in that case start new sbt and connect?

Might be good to run the server by ourselves.

[kpbochenek]

Okay for me to keep track of what we have:

previous MetalsPlugin :
https://github.com/scalameta/metals/blob/v0.7.6/sbt-metals/src/main/scala/scala/meta/metals/MetalsPlugin.scala

current sbt plugin:
https://github.com/sbt/sbt/blob/85a0d08a4eb424f88332bcb1f80c1a69344f1e30/main/src/main/scala/sbt/plugins/SemanticdbPlugin.scala

SemanticdbPlugin is activated via -Dsbt.semanticdb=true. It seems to be conflicting with what is in MetalsPlugin

Okay so the idea would be to not rely on sbt plugin but use our own and don't specify -Dsbt.semanticdb when running sbt.
That's what you prefer, correct?

[tgodzik]

Okay so the idea would be to not rely on sbt plugin but use our own and don't specify -Dsbt.semanticdb when running sbt.
That's what you prefer, correct?

I think that's what Olaf suggested, yes.

[bplommer]

I personally think the Bloop UX is currently better and should remain the default.

This is of course purely anecdotal, but I'm having a better experience with SBT. With bloop I've always had problems with metals randomly becoming unresponsive, but I haven't so far had that with SBT. This no doubt reflects something I'm doing wrong with Bloop, but with SBT it just works.

[tgodzik]

I personally think the Bloop UX is currently better and should remain the default.

This is of course purely anecdotal, but I'm having a better experience with SBT. With bloop I've always had problems with metals randomly becoming unresponsive, but I haven't so far had that with SBT. This no doubt reflects something I'm doing wrong with Bloop, but with SBT it just works.

Thanks for mentioning it! It's always good to hear some feedback. Sbt doesn't yet support everything out of the box, but once it's at the same level we will most likely offer both tools.

What kind of problems did you have with Bloop? Was it using CPU too much or was there any errors in the logs? It would also be nice to see if we could fix that.

[julienrf]

• automatically enable SemanticDB. I think the best way to achieve this is to create a new sbt-metals plugin that configures the correct build settings, semanticdbEnabled :=, semanticdbVersion := semanticdbOptions += s"-P:semanticdb:sourceroot:${baseDirectory.in(ThisBuild).value}", make sure we only do this for supported Scala versions, etc

Could this be configured via BSP? ie, the BSP client, Metals, asks the BSP server, sbt, to enable the semantic db. I’m not sure this would be easy to do on the sbt/mill/server side, though :)

• provide completions for build.sbt files

Which completions do we talk about?

[tgodzik]

• automatically enable SemanticDB. I think the best way to achieve this is to create a new sbt-metals plugin that configures the correct build settings, semanticdbEnabled :=, semanticdbVersion := semanticdbOptions += s"-P:semanticdb:sourceroot:${baseDirectory.in(ThisBuild).value}", make sure we only do this for supported Scala versions, etc

Could this be configured via BSP? ie, the BSP client, Metals, asks the BSP server, sbt, to enable the semantic db. I’m not sure this would be easy to do on the sbt/mill/server side, though :)

I think it's a good idea, but also not 100% sure how hard it will be to do it in every build tool.

• provide completions for build.sbt files

Which completions do we talk about?

Bloop generates a separate target for each sbt meta build, thanks to which we are able to get the right classpath with sbt dependencies and give that to the presentation compiler. Without that, completions will only work for basic Scala things, not things like for example scalaVer@@

[adpi2]

The matter of enabling semanticdb in sbt for Metals has already been discussed here: sbt/sbt#5616

It has appeared that using BSP to enable semanticdb or even the -Dsbt.semanticdb=true is not ideal because it would cause full recompilation each time Metals is open.

Also the -Dsbt.semanticdb=true option does not work if sbt has already started.

I personally think a metals.sbt plugin is a better solution:

• it can be loaded by a workspace/reload BSP request
• once installed it does not trigger recompilation depending on whether Metals has started sbt or not
• it can be tweaked by the user if needed

[DestyNova]

There is a new setting in sbt Global / semanticdbEnabled := true - we can just create a metals.sbt file in the main directory or indeed do an sbt plugin that would do it for us and can be applied in project/metals.sbt, which would mean it's very similar to Bloop integration.

It's not safe to enable the global option for any arbitrary build since the user may be on an unsupported Scala version. An sbt-metals plugin can override projectSettings and ensure we add the correct settings (semanticdb version, set sourceroot compiler option) to the appropriate projects.

I was prompted to do that when trying sbt 1.4.0's BSP support with Intellij. The following message popped up in sbt's stdout on my terminal:

[warn] IntelliJ-BSP requires the semanticdb compiler plugin
[warn] consider setting 'Global / semanticdbEnabled := true' in your global sbt settings ($HOME/.sbt/1.0)

When I added that line to the end of ~/.sbt/1.0/global.sbt and ran sbt -bsp again, it exited immediately with a return code of 1. Not sure if it's caused by a Scala version incompatibility -- I'm running 2.13.1 globally and 2.12.10 in the current project.

[tgodzik]

@DestyNova Hmm... I don't think Intellij needs semanticdb plugin, could maybe raise it in sbt or scala plugin?

[DestyNova]

Thanks @tgodzik. I removed that line and sbt is behaving the same, so it's probably something else (although it was working earlier... 😕 ) -- sorry for the noise.

[adpi2]

I was prompted to do that when trying sbt 1.4.0's BSP support with Intellij. The following message popped up in sbt's stdout on my terminal:

[warn] IntelliJ-BSP requires the semanticdb compiler plugin
[warn] consider setting 'Global / semanticdbEnabled := true' in your global sbt settings ($HOME/.sbt/1.0)

@DestyNova
That's an awful bug in sbt that I am going to report. Thank you for catching it!