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

Add Kotlin-specific data in BSP BuildTarget #3619

Closed
wants to merge 14 commits into from

Conversation

alexarchambault
Copy link
Collaborator

This adds Kotlin-specific custom data in BSP's BuildTarget for KotlinModules.

Not tested in IntelliJ yet, seems BSP support in my EAP version doesn't advertize Kotlin support in its BSP InitializeBuildParams, unlike how the sources of intellij-bsp do (trying things to get that…)

@alexarchambault
Copy link
Collaborator Author

Includes #3608 for now

@alexarchambault
Copy link
Collaborator Author

alexarchambault commented Sep 27, 2024

Seems there's some mixed Scala / Kotlin support in intellij-scala though: https://github.com/search?q=repo%3AJetBrains%2Fintellij-scala%20kotlin&type=code

@alexarchambault
Copy link
Collaborator Author

alexarchambault commented Sep 27, 2024

I confirm things seem to work fine with mixed Kotlin and Scala modules from sbt, like this project.

I've managed to look at the detailed BSP messages between sbt and IntelliJ. They don't rely on declaring Kotlin as a supported language anywhere, nor with any Kotlin-specific data or extension. My guess is that declaring Kotlin modules as Scala ones (Java ones might work too, that will need to be tested), and maybe also putting the Kotlin compiler (org.jetbrains.kotlin:kotlin-scripting-compiler-embeddable and its dependencies at least), makes IntelliJ happy with Kotlin sources (it doesn't complain saying "Kotlin not configured" in a yellow banner above sources, and code navigation seem to work fine, from Scala to Kotlin at least).

I should give that a try in Mill next Monday.

@alexarchambault
Copy link
Collaborator Author

That seems to be a way to proceed via intellij-scala, and its BSP support, that differs from the BSP support from intellij-bsp, that intellij-bazel relies on, and that seems to have explicit Kotlin support in BSP.

@lihaoyi
Copy link
Member

lihaoyi commented Sep 28, 2024

@alexarchambault as a first pass, proceeding via intellij-scala seems reasonable. People will need it installed anyway to support Mill Build files.

@jastice sorry to bother you, but do you happen to know what the story is around IntelliJ/BSP/Kotlin? Should we be using intellij-scala, intellij-bsp, or something else? Does the answer to that differ between short-term and long-term plans?

@lefou
Copy link
Member

lefou commented Sep 28, 2024

There is at least one open Kotlin issue in the BSP project. Can we clarify how it releates? Also, I think any workaround / quirk should come with a link to an upstream issue/ticket. We are almost the only BSP server outside of the IntelliJ developer hood, so if we don't report issues and ensure the BSP spec is alligned with the existing implementations, probably nobody will do and the whole spec isn't worth anything.

@jastice
Copy link

jastice commented Sep 30, 2024

do you happen to know what the story is around IntelliJ/BSP/Kotlin? Should we be using intellij-scala, intellij-bsp, or something else? Does the answer to that differ between short-term and long-term plans?

@lihaoyi mid-term, intellij-bsp will be the way to go. @AnthonyGrod has been working on improving specifically the Mill support for it. It's currently compatible with Nightly releases of the Scala plugin.

@vasilmkd
Copy link

Hello 👋🏻 Sorry for the late reply. How may the Scala Plugin for IDEA team help?

One specific comment about sbt-kotlin-plugin (used in this project), it doesn't support any incremental compilation of the Kotlin sources. In fact, they are recompiled every time sbt compile is invoked. I assume this issue will be very visible when used in a BSP project.

I opened a discussion in Zinc about potentially adding support including other JVM languages in the incremental compilation algorithm, but I'm not sure the discussion has produced anything immediately actionable yet.
sbt/zinc#1394

@lefou
Copy link
Member

lefou commented Sep 30, 2024

@vasilmkd You could try to pitch for opening up a public API for incremental compilation in Kotlin. I mean, the Kotlin compiler seems to be able to compile incrementally, it's just not available to tools other than IntelliJ IDEA or Gradle. And why should we add something to zinc, for which any Kotlin specific aspects are an afterthought, if it's already implemented in the Kotlin compiler? I think there is something highly experimental, but IIUC it's not that easy to use from outside of Gradle.

There is a (incomplete) list of related tickets in my tracker issue for incremental compilation: lefou/mill-kotlin#131

@vasilmkd
Copy link

I agree. My question would be how your plans for Kotlin incremental compilation in Mill tie in with incremental compilation of Scala + Kotlin + Java projects. I would think eventually, some sort of Zinc analysis will need to be emitted. I'm personally not only interested in pure Kotlin incremental compilation.

Maybe I'm missing something. I would love to know more. Maybe we can collaborate on this. Thank you.

@lefou
Copy link
Member

lefou commented Sep 30, 2024

I don't think that reliable incremental compilation between mixed Java/Scala/Kotlin can become a think in any near future. Kotlin isn't the nicest player in the JVM ecosystem when it comes to API contracts1 mixing Scala and Kotlin can become quite hard.

We should first strive for making the live of the majority of developers easier, which only mix those languages up- or downstream, but not in the same (Mill) module. We should focus on making incremental compilation of Java/Kotlin projects work, which is what the Kotlin compiler already supports (but doesn't provide to us, yet). Downstream compilations can handle arbitrary compile output as any other binary dependency.

Footnotes

  1. (for example, Kotlin is silently altering generics to either in or out when implementing Java interfaces, so that the Scala compiler isn't even accepting those Kotlin classes as worthy overrides of an Java interface. To fix this, you have to change the upstream Java interface)

@lefou
Copy link
Member

lefou commented Sep 30, 2024

In Mill, we should allow to pass arbitrary additional data via the compile task, like incremental analysis data. So Kotlin modules can use the Kotlin data from upstream Kotlin modules, and Scala modules can use the zinc analysis data of upstream Scala modules.

There is a related issue:

@lihaoyi
Copy link
Member

lihaoyi commented Sep 30, 2024

Thanks for the context @vasilmkd.

  • I agree with @lefou that combining Java+Scala+Kotlin in the same module and trying to compile them all in Zinc is probably not feasible.

  • Mill does its own module-level caching, so if no Kotlin sources are changed, we re-use the output without invoking the Kotlin compiler at all, so the situation won't be quite as dire as in SBT where it re-compiles every time.

  • However, one thing Mill needs help from the Kotlin compiler is for file-level incremental compilation. Mill is not aware of the file-level dependencies and needs the Kotlin compiler to do the file-level incremental compilation for us.

  • Mill currently models compilation as two long-lived paths on disk: output classFiles + incremental analysisFile, and what's exactly in each of those paths, and how they are updated or invalidated, is up to the language implementation. We can also expand this if necessary.

  • A reasonable target to aim for is to allow Kotlin-only and Java+Kotlin modules to incrementally compile, and to ensure that inter-module dependencies between Scala and Kotlin modules work as best possible.

@vasilmkd do you know who we could talk to about the best way to wire up the Kotlin compiler's incremental compilation into Mill? We're happy to do work on the Mill side, just would like to know what the best way forward is

So that it gets used when writing .bsp/mill-bsp.json

This inlines and removes the 'dist.prependShellScript' task, so that the
output path of 'dist.launcher' can be hard-coded in the output of
prependShellScript.
So that its value doesn't change in the test fixtures of the upcoming
BSP integration tests
So that they don't return results in random order, which helps when
serializing fixtures in the upcoming BSP ITs
These fixtures hard-code the Mill launcher class path, so they contain
its transitive dependencies and their versions. That means they would
needs to be updated every time a dependency of Mill is updated, which
would be quite cumbersome.

Note that they also contain issues: missing source JARs, dependencies
marked as unmanaged, …
That's going to be helpful for Kotlin too
Ideally, I'd like to pass the whole InitializeBuildParams here, but
scalalib doesn't pull bsp4j (not sure if there's a reason for that)
So that they're handled by intellij-scala, that deals fine with Kotlin
through BSP apparently
@lefou
Copy link
Member

lefou commented Oct 1, 2024

@vasilmkd do you know who we could talk to about the best way to wire up the Kotlin compiler's incremental compilation into Mill? We're happy to do work on the Mill side, just would like to know what the best way forward is

@lihaoyi According to this comment from Alexander Likhachev, there is some highly experimental API we could try to use.

The entry point is org.jetbrains.kotlin.buildtools.api.CompilationService. Instance of this class is expected to be loaded via org.jetbrains.kotlin.buildtools.api.CompilationService.loadImplementation. The passed classloader must contain org.jetbrains.kotlin:kotlin-build-tools-impl and all its runtime dependencies. There's org.jetbrains.kotlin.buildtools.api.SharedApiClassesClassLoader that one may find useful. An example of usage is org.jetbrains.kotlin.compilerRunner.btapi.BuildToolsApiCompilationWork in KGP. KGP can be switched to use the build tools API by passing the Gradle property kotlin.compiler.runViaBuildToolsApi=true

Of course, you need to fiddle with classpaths and probably unclear dependencies and maybe, the implementation expects most likely some environment as Gradle provides it. Don't know whether a "Gradle Property" is something special or just a Java system property.

@alexarchambault
Copy link
Collaborator Author

Closing this PR, as I'm splitting it into #3643 and #3644, but the discussion here can keep going if you'd like 😅

@alexarchambault alexarchambault deleted the kotlin-bsp branch October 1, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants