BSP control plugin

[satorg]

Addresses #405.
Note: targets the main branch currently!

Introduces a new tlBspPlatforms setting of Set[Platform] type that controls for which cross-project platforms the bspEnabled setting should be set to true. By default it becomes enabled for JVMPlatform only.

The new setting can be useful to reduce BSP activity for re-compiling projects while editing their code files. Platforms like JSPlatform and NativePlatform usually recompile slower and consume more resources (JVM heap, in particular) upon compilation.

How to change the default

tlBspPlatforms := Set(JVMPlatform, JSPlatform)

How to opt out

.disablePlugins(BspControlPlugin)

[satorg]

Created as "Draft" coz I would like to do some additional checks first. But any feedback is welcome!

[armanbilge]

Thanks, I think we are fairly desperate for some relief in this area. Not your fault of course—but I just wish this wasn't so "clunky" for the non-vanilla usecases.

I wonder if it's worth going upstream to the CrossPlugin to try and expose some additional information e.g. all of a projects cross targets, whether it is a pure/full cross, etc.

Alternatively, I wonder if this is a problem that IDEs should solve, instead of us. Arguably it's a bit weird to hard-code BSPs into the build. Which BSPs should be enabled or disabled depends precisely on what the user is doing at that moment, so it seems like they should configure this via UI in the IDE. To give an example: http4s-core is 99% a pure cross-build, but it has a very small amount of JS-specific sources. On the occasional bad day I have to poke at those, and it would be extremely annoying to have to futz the build to get BSP support. But it would be even worse to force JS BSP onto everyone because of like 3 source files I touch every 6 months when a new Node.js drops and everything breaks /rant.

[armanbilge]

This is good, but to confirm my understanding this plugin will only be enabled for CrossProjects anyway right? Because of the requires: Plugins = CrossPlugin.

[satorg]

Yes, that's correct (sorry for the delay). So it should be pretty much safe simply to reference it via

crossProjectPlatform.value

I'm just not sure though if it is possible to have CrossPlugin required but crossProjectPlatform not defined.
Strictly saying, it is all up to the CrossPlugin implementation.
So I think maybe it makes sense to handle such a possibility, just in case.

[satorg]

Alternatively, I wonder if this is a problem that IDEs should solve, instead of us.

Yeah, I wish if it was on IDEs too. Unfortunately neither IntelliJ nor VSCode seem to provide such kind of filtering.

I wonder if it's worth going upstream to the CrossPlugin to try and expose some additional information e.g. all of a projects cross targets, whether it is a pure/full cross, etc.

I think I can go ahead and create an issue in the sbt-crossproject repo and link this PR as a possible solution example. So we will see if there is any interest in such a feature. Wdyt, does it make sense to give it a try?

[armanbilge]

I opened scalameta/metals-feature-requests#332.

Update: which I discovered is a duplicate of scalameta/metals#6782 :)

[satorg]

Finally, I've made the setting less invasive and verified all the cases I was concerned about.
I think it is pretty much ready to go (if we decide it makes any practical sense, of course).

However, there's a potential catch with this approach (as a follow up of this discussion):

if we define the following combination of settings:

ThisBuild / tlBspExcludePlatforms += NativePlatform // disable BSP in all projects for the "Native" platform
LocalProject("fooJS") / bspEnabled := false // also disable for the "JS" platform in the "foo" project specifically

then such a combination won't work.
It seems that the former setting gets a higher priority rather than the latter regardless of their order.

In other words, if tlBspExcludePlatforms is set to a non-default value, then further adjusting bspEnabled has no effect.

The issue can be solved though in this way:

ThisBuild / tlBspExcludePlatforms += NativePlatform
LocalProject("fooJS") / tlBspExcludePlatforms += JSPlatform

So the new setting shouldn't expose any blocker at the last.

[armanbilge]

Thanks Sergey! Will take a closer look soon. Did you happen to see portable-scala/sbt-crossproject#152, can we still make use of that here?

[satorg]

@armanbilge sorry, I overlooked your question.
But I am not sure if that change in sbt-crossproject is somehow related, isn't it?
As I can see, it ultimately introduces two new settings: crossProjectCrossType and crossProjectBaseDirectory,
but I'm struggling to figure out how it could help to the functionality introduced in here.

Could you throw in a hint please?

[armanbilge]

I thought at some point we had discussed using the CrossType to determine whether we only need BSP for one platform (because it's Pure) or for all platforms (because it's Full). But maybe that's no longer applicable?

[armanbilge]

@satorg I think I'd like to move ahead with this, thanks for all your work! Did you have any thoughts on #465 (comment)?

[satorg]

@armanbilge I added one more commit to the PR. It preserves bspEnabled := false if it was set that way somewhere else. Check it out please.

To you question:

Did you have any thoughts on #465 (comment)?

Honestly, I'm not sure if it can be utilized here somehow.

Currently, the idea is that BspControlPlugin does nothing by default. It only chimes in if tlBspExcludePlatforms is explicitly set to one or more platforms. So it does not make any own decisions.

I think, the best usage pattern of this plugin could be adding a separate file like user.sbt (or any other of one's preference) where to put a line like

ThisBuild / tlBspExcludePlatforms += NativePlatform

It should be a simple and quite harmless approach, I think.

Let me know what you think please. Thank you!

[armanbilge]

Currently, the idea is that BspControlPlugin does nothing by default.

Ah, I missed this. I thought it might be reasonable for the default to disable BSP for alternate platforms in pure crosses. The goal would be to lower the burden for casual contributors, who are unlikely to have a user.sbt configuration. Meanwhile regular contributors could invest in something like that.

[satorg]

@armanbilge
Just to clarify: this is the output of the crossProjectCrossType command for some of the projects from Cats:

Project	crossProjectCrossType
coreJS / crossProjectCrossType	sbtcrossproject.CrossType$Pure$@16e33a3b
coreJVM / crossProjectCrossType	sbtcrossproject.CrossType$Pure$@16e33a3b
coreNative / crossProjectCrossType	sbtcrossproject.CrossType$Pure$@16e33a3b
kernelJS / crossProjectCrossType	sbtcrossproject.CrossType$Pure$@16e33a3b
kernelJVM / crossProjectCrossType	sbtcrossproject.CrossType$Pure$@16e33a3b
kernelNative / crossProjectCrossType	sbtcrossproject.CrossType$Pure$@16e33a3b
kernelLawsJS / crossProjectCrossType	sbtcrossproject.CrossType$Full$@657dc936
kernelLawsJVM / crossProjectCrossType	sbtcrossproject.CrossType$Full$@657dc936
kernelLawsNative / crossProjectCrossType	sbtcrossproject.CrossType$Full$@657dc936
lawsJS / crossProjectCrossType	sbtcrossproject.CrossType$Pure$@16e33a3b
lawsJVM / crossProjectCrossType	sbtcrossproject.CrossType$Pure$@16e33a3b
lawsNative / crossProjectCrossType	sbtcrossproject.CrossType$Pure$@16e33a3b

It looks like crossProjectCrossType is set to the same value for every platform within some x project: xJVM, xJS and xNative. So how can we figure out from the the crossProjectCrossType value that, let's say, coreJVM should have BSP enabled, whereas coreJS and coreNative should get it disabled?

[armanbilge]

So how can we figure out from the the crossProjectCrossType value that, let's say, coreJVM should have BSP enabled, whereas coreJS and coreNative should get it disabled?

We can have a setting for the "blessed" platform (defaults to JVM). then if it's a pure cross, it will only enable bsp if it's that blessed platform

... yeah, it's not great, but I think it would work in practice. idk, what do you think?

[satorg]

@armanbilge, personally, I do not mind. But to be honest the "blessed platform" approach would work for me even for CrossProject.Full repos :) So I am not the best judge on that.

My only concern though is that if we take a look at a real big repo like Cats for an example, we will find out that roughly a half of its modules are CP.Pure whereas the second half is PC.Full. That means that the "blessed platform" approach will be only a half useful for Cats (or half useless, depending on your perception :) )

Also, maybe it'd make sense to add yet another settings: tlBspIncludePlatforms. I.e. if it happens that someone is not happy with the default logic, then they always can tackle that by adding an external user.sbt file with

ThisBuild / tlBspIncludePlatforms += NativePlatform

Yeah, we can opt-out with disablePlugins, but it would require changes to the main build.sbt, whereas tlBspIncludePlatforms would allow to opt-out without such changes.

Let me know what you think please. Thanks!

[armanbilge]

Ok, thanks for that exposition, I see what you are saying!

I think, the best usage pattern of this plugin could be adding a separate file like user.sbt (or any other of one's preference) where to put a line like

I propose a slight revision to this. The best usage pattern of this plugin is that:

1. the build.sbt sets some reasonable default for project (possibly per-module), to optimize for best experience for typical/casual contribution, maintainer preferences, etc.

2. the power user adds a user.sbt with additional customization

Does that sound about right?

With that in mind can we just have one setting tlBspPlatforms? I guess we should default it to all 3. Big projects will have to opt-out, but I think thats better than non-JVM projects being confused.

[satorg]

With that in mind can we just have one setting tlBspPlatforms?

Yes, I think it would work even better (because looks simpler). The only caveat though, I think it should be set to empty by default, which would mean that it is not configured and the default logic applies per se. Once it is set to some non-empty value, it should supersede the default logic.

[armanbilge]

The only caveat though, I think it should be set to empty by default, which would mean that it is not configured and the default logic applies per se.

Hmm, it feels weird to special-case an empty value. We could either use an Option for the key, or actually not set a default value at all—in sbt you can add .? when reading a key so you get an Option depending on whether it is set or not.

[satorg]

or actually not set a default value at all—in sbt you can add .? when reading a key so you get an Option depending on whether it is set or not.

Yes agree, I think it would work too (maybe even better).

it feels weird to special-case an empty value.

it does indeed, but on the other hand, an empty value for tlBspPlatforms does not make any sense per se, does it? I mean, if tlBspPlatforms would be set to an empty value, then what would be do about it, how would we handle this?

[armanbilge]

I mean, if tlBspPlatforms would be set to an empty value, then what would be do about it, how would we handle this?

That means they don't want any BSP 😛 you are right, there are better ways to indicate that.

Also, I just realize this setting only works for crossProject. So maybe we should make that clear e.g. tlBspCrossPlatforms ?

Ok, enough bike-shed 😁 I'm okay with any of the approaches, even just the empty value. I trust your judgement!

[satorg]

@armanbilge I've made the change, check it out. To summarize, the last behavior is the following:

• The new setting tlBspCrossProjectPlatforms: Set[Platform] is not initialized by default. Therefore, it should be assigned as a whole, like tlBspCrossProjectPlatforms := Set(JSPlatform), but not with ++=, etc.
• If the setting is not assigned, then the default logic is applied, i.e. if a crossProject is Pure then only JVMPlatform gets bspEnabled := true whereas all other platform get it false.
• for projects of other types (Full and Dummy) the value of bspEnabled remains unchanged (i.e. true by default).
• if tlBspCrossProjectPlatforms is initialized, then its values are taken into account instead and bspEnabled is set for the platforms listed in the setting regardless of their projects' crossType.

[armanbilge]

Looks like the CrossPlugin is enabled for all projects anyway.

[armanbilge]

Thank you so much for all the iterations on this. This is great!