-
Notifications
You must be signed in to change notification settings - Fork 214
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
Create a new "bootstrap" toolchain and compile with 2.1.0 #1244
Conversation
bad7f65
to
d54801b
Compare
49fb292
to
aba42a9
Compare
17792ec
to
efec0d5
Compare
Given that Kotlin 2.1.0 now has a version metadata check during compilation, the idea of bootstrapping kotlin by using the previous is pretty much dead. Probably not the worst thing, as bzlmod did not enjoy the shennanigans necessary to make it work. This adds a new "cli" toolchain that invokes kotlinc via the established command line interface. Additionally, it sketches out a simpler, more contained toolchain api to build the more complex usecases (such as multiplex workers, jdeps, compiler plugins, etc.) on top of. the end goal is a more agile rule approach -- with the performance optimization (jdeps, class path reductions, etc) tucked behindthe simple api.Compiler plugins, linting, and the like should be able to live comfortably in the "production" rule and toolchain combinations. It also introduces new "core" rules, while limited in scope, should be IDE friendly and generally easier to work with than the previous bootstrapping effort -- and since they use the common providers, they will interoperate with the existing rules.
Still some outstanding issues iwth the core rules test.
efec0d5
to
c40d2e5
Compare
.bazelci/presubmit.yml
Outdated
@@ -31,8 +31,8 @@ tasks: | |||
integration_tests: | |||
name: "Integration Tests" | |||
platform: ${{ integration_platform }} | |||
test_flags: | |||
- "--config=rbe" | |||
# test_flags: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to leave this one commented out still?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
) | ||
use_repo( | ||
rules_kotlin_bootstrap_extensions, | ||
"released_com_github_google_ksp", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these not being synced anymore for some reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the build-kotlin-with kotlin, so we don't use the released rules anymore.
], | ||
repositories = [ | ||
"https://maven.google.com", | ||
"https://repo1.maven.org/maven2", | ||
], | ||
) | ||
|
||
register_toolchains("//bzl:experimental_toolchain") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this need to be registered here in the example app? Is this requirement moving forward?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defining a toolchain and not registering it means it will never be chosen.
App didn't compile without the toolchain -- complained about all sorts of things.
kotlin/internal/toolchains.bzl
Outdated
doc = "the -jvm_target flag. This is only tested at 1.8.", | ||
default = "1.8", | ||
doc = "the -jvm_target flag.", | ||
default = "11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do this against the main branch, but there are a few more JDK releases that we can add here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got tired of the warnings. Will revert.
@@ -23,30 +29,16 @@ load( | |||
TOOLCHAIN_TYPE = "%s" % Label("//kotlin/internal:kt_toolchain_type") | |||
|
|||
# Java toolchains | |||
JAVA_TOOLCHAIN_TYPE = "@bazel_tools//tools/jdk:toolchain_type" | |||
JAVA_RUNTIME_TOOLCHAIN_TYPE = "@bazel_tools//tools/jdk:runtime_toolchain_type" | |||
JAVA_TOOLCHAIN_TYPE = _JAVA_TOOLCHAIN_TYPE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice clean up here
language_version = ".".join(ctx.attr.api_version.split(".")[:2]), | ||
executable_zip = ctx.attr.zip[DefaultInfo].files_to_run, | ||
kotlinc = ctx.attr.kotlinc[DefaultInfo].files_to_run, | ||
compile_mnemonic = "CliKotlinc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing this locally, i'm seeing a TON of console noise from this action:
logging: loading modules: [java.se, com.azul.tooling, jdk.accessibility, jdk.attach, jdk.compiler, jdk.dynalink, jdk.httpserver, jdk.jartool, jdk.javadoc, jdk.jconsole, jdk.jdi, jdk.jfr, jdk.jshell, jdk.jsobject, jdk.management, jdk.management.jfr, jdk.naming.ldap, jdk.net, jdk.scripting.nashorn, jdk.sctp, jdk.security.auth, jdk.security.jgss, jdk.unsupported, jdk.unsupported.desktop, jdk.xml.dom, java.base, java.compiler, java.datatransfer, java.desktop, java.xml, java.instrument, java.logging, java.management, java.management.rmi, java.rmi, java.naming, java.net.http, java.prefs, java.scripting, java.security.jgss, java.security.sasl, java.sql, java.transaction.xa, java.sql.rowset, java.xml.crypto, jdk.internal.jvmstat, jdk.management.agent, jdk.jdwp.agent, jdk.internal.ed, jdk.internal.le, jdk.internal.opt]
INFO: From CliKotlinc src/main/kotlin/io/bazel/kotlin/builder/cmd/build_lib.jar:
logging: using Kotlin home directory /private/var/tmp/_bazel_blee/705a39c26a4d07ab15f533d9e4d9df56/external/_main~rules_kotlin_extensions~com_github_jetbrains_kotlin_git
logging: using JDK home directory external/rules_java~~toolchains~local_jdk
logging: exception on loading scripting plugin: java.lang.ClassNotFoundException: org.jetbrains.kotlin.scripting.compiler.plugin.ScriptingCompilerConfigurationComponentRegistrar
logging: using JVM IR backend
logging: configuring the compilation environment
logging: loading modules: [java.se, com.azul.tooling, jdk.accessibility, jdk.attach, jdk.compiler, jdk.dynalink, jdk.httpserver, jdk.jartool, jdk.javadoc, jdk.jconsole, jdk.jdi, jdk.jfr, jdk.jshell, jdk.jsobject, jdk.management, jdk.management.jfr, jdk.naming.ldap, jdk.net, jdk.scripting.nashorn, jdk.sctp, jdk.security.auth, jdk.security.jgss, jdk.unsupported, jdk.unsupported.desktop, jdk.xml.dom, java.base, java.compiler, java.datatransfer, java.desktop, java.xml, java.instrument, java.logging, java.management, java.management.rmi, java.rmi, java.naming, java.net.http, java.prefs, java.scripting, java.security.jgss, java.security.sasl, java.sql, java.transaction.xa, java.sql.rowset, java.xml.crypto, jdk.internal.jvmstat, jdk.management.agent, jdk.jdwp.agent, jdk.internal.ed, jdk.internal.le, jdk.internal.opt]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right verbose actually does something with K2.
], | ||
kotlinc = ":kotlinc", | ||
language_version = versions.KOTLIN_CURRENT_COMPILER_RELEASE.version, | ||
zip = "@bazel_tools//tools/zip:zipper", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just make this the toolchain default instead of making it customizable. Could even make it internal with _zip
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After getting kicked off using Bazel's jarjar, I'm leaving options open.
) | ||
needs_runfiles = "0" if java_bin_path.startswith("/") or (len(java_bin_path) > 2 and java_bin_path[1] == ":") else "1" | ||
|
||
actions.expand_template( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can probably be simplified a bunch by using java_binary
here. Less re-creating the wheel with the launcher code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added for completeness of the toolchain. If you want complete freedom to play with the kotlin compiler, you could use this.
Basically, it's a thin wrapper around kotlinc -- it can't support persistent workers (at least I don't see an easy path on it) -- but it will always be forward compatible (until they change the cli interface... I give it two major versions). It's the "no warranty" rules and toolchain for the discerning kotlin developer. I've even added basic plugin support: #1251
@@ -0,0 +1,16 @@ | |||
# Copyright 2024 The Bazel Authors. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the options are missing from here and it needs to be registered in src/main/starlark/core/repositories/compiler.bzl
, otherwise all of the 2.1 options won't be usable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to finish automating the process. :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 pending #1244 (comment)
Apparently, the reflection subtlely changed as well.
29ed523
to
42c2f78
Compare
Given that Kotlin 2.1.0 now has a version metadata check during compilation, the idea of bootstrapping kotlin by using the previous is pretty much dead. Probably not the worst thing, as bzlmod did not enjoy the shennanigans necessary to make it work.
This adds a new "cli" toolchain that invokes kotlinc via the established command line interface. Additionally, it sketches out a simpler, more contained toolchain api to build the more complex usecases (such as multiplex workers, jdeps, compiler plugins, etc.) on top of. the end goal is a more agile rule approach -- with the performance optimization (jdeps, class path reductions, etc) tucked behind the simple api.Compiler plugins, linting, and the like should be able to live comfortably in the "production" rule and toolchain combinations.
It also introduces new "core" rules, while limited in scope, should be IDE friendly and generally easier to work with than the previous bootstrapping effort -- and since they use the common providers, they will interoperate with the existing rules.
Changes: