[feature] add support for lucidsoftware/rules_scala
#209
Conversation
|
Hi! Thank you very much for your contribution. Please run Regarding upstream rules_scala support: our codebase has suffered from a bit of bitrot in this area to be honest, because we mostly focused on java + kotlin. But we absolutely want to restore full scala support in the future. |
|
LGTM, but perhaps @abrams27 wants to have a second look |
|
Do you know in which Bazel version this flag was introduced exactly? Google plugin seems to use --noexperimental_run_validations |
...server/src/main/kotlin/org/jetbrains/bsp/bazel/server/bsp/managers/BazelBspAspectsManager.kt
Outdated
Show resolved
Hide resolved
.../server/src/main/kotlin/org/jetbrains/bsp/bazel/server/bsp/managers/BazelToolchainManager.kt
Outdated
Show resolved
Hide resolved
.../server/src/main/kotlin/org/jetbrains/bsp/bazel/server/bsp/managers/BazelToolchainManager.kt
Outdated
Show resolved
Hide resolved
abrams27
changed the title
lucidsoftware/rules_scala
| @@ -53,7 +56,23 @@ class JavaLanguagePlugin( | |||
| private fun getJdk(): Jdk = jdk ?: throw RuntimeException("Failed to resolve JDK for project") | |||
|
|
|||
| override fun dependencySources(targetInfo: TargetInfo, dependencyGraph: DependencyGraph): Set<URI> = | |||
| emptySet() // Provided via workspace/libraries | |||
| targetInfo.getJvmTargetInfoOrNull()?.run { | |||
There was a problem hiding this comment.
and I'd do this calculation conditional as well - it's quite heavy calculation executed for each target, what can add quite some overhead for us (we use a separate mechanism for it, because this one is too slow for us).
By "conditional" everywhere I meant that it's possible to check what rules are imported (org/jetbrains/bsp/bazel/server/sync/ProjectResolver.kt:69), it's a list of all rules in the workspace, and then you can use this list to check if the fork rules are loaded
There was a problem hiding this comment.
Ok, so on this one I have a question. As far as I can tell, the workspace/libraries command isn't a part of the build server protocol that I can see, and more practically speaking, metals lsp doesn't seem to consume it. All that is to say, I enabled this more for metals support than for rules_scala_annex support.
I'm happy to do what you recommended just to make my use case work, but is it possible we could make this something configurable instead? I see there are some FeatureFlags in the repo, would that be a viable place to add a toggle for something like this?
There was a problem hiding this comment.
indeed workspace/libraries are only in our "extension", we use it because dependencySources is too slow for us (and uses too much memory), so we have an endpoint which returns globally all the libraries which are referenced from modules.
You can add a feature flag to org.jetbrains.bsp.protocol.FeatureFlags (calculateDependencySources or useLibraries) with a default which should work for a generic bsp implementation, and override it only in our client, so server will do it only if it's connected to our server which supports custom endpoint
|
Thanks for your feedback! I'm a little slow, but I'll get to work on these! |
It looks to me like it was renamed in 5.0. https://blog.bazel.build/2022/01/19/bazel-5.0.html |
nice, so we dont have to worry about it, bazel 5 is deprecated |
| Language.Scala -> | ||
| when (ruleLanguage.ruleName) { | ||
| "rules_scala_annex" -> Label.parse("@rules_scala_annex//rules/scala:toolchain_type") | ||
| else -> Label.parse("@${ruleLanguage.ruleName}//scala:toolchain_type") |
There was a problem hiding this comment.
it
| else -> Label.parse("@${ruleLanguage.ruleName}//scala:toolchain_type") | |
| else -> Label.parse("@io_bazel_rules_scala//scala:toolchain_type") |
even in bzlmod, it's still prefixed with io_bazel
This should make it possible to run bazel-bsp with metals on projects that rely on lucidsoftware's fork of rules_scala. The main changes are:
As a note, I have tried running metals + bazel-bsp against a few different open source scala projects that were using bazel_io_rules_scala, and I couldn't get any of them to work. I did my best to "do no harm" with these changes if people are using this successfully, but if someone can provide me with steps to get a working setup, I'd appreciate any pointers. Similar comment on tests, I found and ran tests, most passed, but I'm not sure how I could have effected the ones that failed locally, so I'm hoping it's a problem with my environment and not a mistake I made with my changes.