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

Decouple Kotlin compiler from Kotlin rules #899

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

comius
Copy link
Contributor

@comius comius commented Dec 21, 2022

Move Kotlin specific targets (like kt_jvm_import) from BUILD.com_github_jetbrains_kotlin to //kotlin/compiler.

This will make it possible to release kotlin-compiler module on bazel-central-registry without depending on rules_kotlin. The release will contain the new form BUILD.com_github_jetbrains_kotlin which doesn't depend on rules_kotlin.

RELNOTES[INC]: This breaks users that depend on @com_github_jetbrains_kotlin//:* targets. The same targets can be found under @io_bazel_rules_kotlin//kotlin/compiler:*.

It's not possible to add an alias to @com_github_jetbrains_kotlin repository, becuase it would already create a dependency on rules_kotlin repository in the bzlmod world.

Technically it's possible to release 2 bzlmod modules that have a circular dependency. But this might create some unexpected problems. It addition it would be harder to convince com_github_jetbrains_kotlin owners to depend on rules_kotlin.

Works toward: #660

@comius comius changed the title Decouple Kotlin compiler from rules Decouple Kotlin compiler from Kotlin rules Dec 21, 2022
@comius comius marked this pull request as ready for review December 21, 2022 15:34
This was referenced Dec 22, 2022
@@ -11,10 +11,6 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# dev_io_bazel_rules_kotlin
load("@{{.KotlinRulesRepository}}//kotlin:jvm.bzl", "kt_jvm_import")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to provide the old targets with a deprecated warning for at least a version.

The docs have encouraged folks to this repository for a while, so the transition process should be deprecate then remove.

@restingbull restingbull self-assigned this Dec 22, 2022
@restingbull restingbull force-pushed the decouple-kotlin-compiler branch 2 times, most recently from 22b80d6 to 6e1ecc3 Compare December 22, 2022 22:31
@comius
Copy link
Contributor Author

comius commented Dec 23, 2022

I see 2 options, I think are slightly better than current state (java_import in compiler repo and kt_jvm_import in //kotlin/compiler package):

  1. Don't deprecate and only use java_import rules in compiler repo (remove //kotlin/compiler package). This is less work for the users, removes cyclic module dependency (adds a dep to rules_java to Kotlin compiler, but that's not a problem). Does Kotlin setup work well with java_import?
  2. Deprecate using alias instead of java_import. This wouldn't add rules_java dependency to Kotlin compiler module. For alias to work in bzlmod we'd still need to add a dep back to rules_kotlin, however we could export it to bazel-central-registry without aliases and ask users of bzlmod to depend on the new location.

Or we could also go forward with current state - export such Kotlin compiler repo and remove the deprecated targets in one of the future version of Kotlin compilers?

WDYT?

@restingbull
Copy link
Collaborator

I see 2 options, I think are slightly better than current state (java_import in compiler repo and kt_jvm_import in //kotlin/compiler package):

  1. Don't deprecate and only use java_import rules in compiler repo (remove //kotlin/compiler package). This is less work for the users, removes cyclic module dependency (adds a dep to rules_java to Kotlin compiler, but that's not a problem). Does Kotlin setup work well with java_import?
  2. Deprecate using alias instead of java_import. This wouldn't add rules_java dependency to Kotlin compiler module. For alias to work in bzlmod we'd still need to add a dep back to rules_kotlin, however we could export it to bazel-central-registry without aliases and ask users of bzlmod to depend on the new location.

Or we could also go forward with current state - export such Kotlin compiler repo and remove the deprecated targets in one of the future version of Kotlin compilers?

WDYT?

apologies, the holidays became very hectic.

  1. Kotlin is completely fine with java_import. It's a selling point. The kt_jvm_import is slightly more more performant (it doesn't do any of the jvm stamping, etc.) but I think that's fine.
  2. This seems more complicated than it's worth...

Over all, folk's relying on the compiler repo is actually a fairly annoying problem. For example, a lot of kotlin deps bring the jars as part of the transitive closure -- this leads to duplicated jars on the classpath, etc. etc. Ideally, we'd move to just using the maven jars, which helps solve the problem.

Let's go with 1 -- @Bencodes @nkoroste: can you verify that switching to java_import doesn't introduce undue performance regressions?

@chrislovecnm
Copy link

Any update on this?

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.

3 participants