-
Notifications
You must be signed in to change notification settings - Fork 232
Add initial support for Kotlin Native toolchain and klib compilation #1351
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
base: master
Are you sure you want to change the base?
Changes from all commits
913b1ad
e5eb0a5
94abd2d
3553ace
d2a3ca9
f0385a3
b108308
5d6557a
fa6291e
685e4d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,9 +12,9 @@ | |
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
load("@com_github_jetbrains_kotlin//:artifacts.bzl", "KOTLINC_ARTIFACTS") | ||
load("@com_github_jetbrains_kotlin//:artifacts.bzl", "KOTLINC_ARTIFACTS", "KOTLIN_NATIVE_ARTIFACTS") | ||
load("//kotlin:jvm.bzl", "kt_jvm_import") | ||
load("//kotlin/internal:defs.bzl", _KT_COMPILER_REPO = "KT_COMPILER_REPO") | ||
load("//kotlin/internal:defs.bzl", _KT_COMPILER_REPO = "KT_COMPILER_REPO", _KT_NATIVE_COMPILER_REPO_PREFIX = "KT_NATIVE_COMPILER_REPO_PREFIX") | ||
|
||
KOTLIN_STDLIBS = [ | ||
"//kotlin/compiler:annotations", | ||
|
@@ -25,18 +25,18 @@ KOTLIN_STDLIBS = [ | |
"//kotlin/compiler:trove4j", | ||
] | ||
|
||
def _import_artifacts(artifacts, rule_kind): | ||
_import_labels(artifacts.plugin, rule_kind) | ||
_import_labels(artifacts.runtime, rule_kind) | ||
_import_labels(artifacts.compile, rule_kind, neverlink = 1) | ||
def _import_artifacts(artifacts, rule_kind, compiler_repo = _KT_COMPILER_REPO): | ||
_import_labels(artifacts.plugin, rule_kind, compiler_repo) | ||
_import_labels(artifacts.runtime, rule_kind, compiler_repo) | ||
_import_labels(artifacts.compile, rule_kind, compiler_repo, neverlink = 1) | ||
|
||
def _import_labels(labels, rule_kind, **rule_args): | ||
def _import_labels(labels, rule_kind, compiler_repo, **rule_args): | ||
for (label, file) in labels.items(): | ||
if not file.endswith(".jar"): | ||
native.filegroup( | ||
name = label, | ||
srcs = [ | ||
"@%s//:%s" % (_KT_COMPILER_REPO, label), | ||
"@%s//:%s" % (compiler_repo, label), | ||
], | ||
) | ||
return | ||
|
@@ -46,10 +46,10 @@ def _import_labels(labels, rule_kind, **rule_args): | |
args = dict(rule_args.items()) | ||
args["visibility"] = ["//visibility:public"] | ||
args["name"] = label | ||
args["jars"] = ["@%s//:%s" % (_KT_COMPILER_REPO, label)] | ||
args["jars"] = ["@%s//:%s" % (compiler_repo, label)] | ||
sources = label + "-sources" | ||
if sources in labels: | ||
args["srcjar"] = "@%s//:%s" % (_KT_COMPILER_REPO, sources) | ||
args["srcjar"] = "@%s//:%s" % (compiler_repo, sources) | ||
rule_kind(**args) | ||
|
||
def kt_configure_compiler(): | ||
|
@@ -63,3 +63,18 @@ def kt_configure_compiler(): | |
|
||
_import_artifacts(KOTLINC_ARTIFACTS.jvm, kt_jvm_import) | ||
_import_artifacts(KOTLINC_ARTIFACTS.core, kt_jvm_import) | ||
_import_artifacts(KOTLIN_NATIVE_ARTIFACTS.linux_x86_64, kt_jvm_import, compiler_repo = _KT_NATIVE_COMPILER_REPO_PREFIX + "_linux_x86_64") | ||
_import_artifacts(KOTLIN_NATIVE_ARTIFACTS.macos_x86_64, kt_jvm_import, compiler_repo = _KT_NATIVE_COMPILER_REPO_PREFIX + "_macos_x86_64") | ||
_import_artifacts(KOTLIN_NATIVE_ARTIFACTS.macos_aarch64, kt_jvm_import, compiler_repo = _KT_NATIVE_COMPILER_REPO_PREFIX + "_macos_aarch64") | ||
_import_artifacts(KOTLIN_NATIVE_ARTIFACTS.windows_x86_64, kt_jvm_import, compiler_repo = _KT_NATIVE_COMPILER_REPO_PREFIX + "_windows_x86_64") | ||
|
||
# a convenience alias for kotlin-native to be referenced in other places | ||
native.alias( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the usecase for this? It's far preferable to use specific toolchains. |
||
actual = select({ | ||
"@bazel_tools//src/conditions:linux_x86_64": "//kotlin/compiler:kotlin-native-linux-x86_64", | ||
"@bazel_tools//src/conditions:darwin": "//kotlin/compiler:kotlin-native-macos-x86_64", | ||
"@bazel_tools//src/conditions:windows": "//kotlin/compiler:kotlin-native-windows_x86_64", | ||
"@bazel_tools//src/conditions:darwin_arm64": "//kotlin/compiler:kotlin-native-macos_aarch64", | ||
}), | ||
name = "kotlin-native", | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
load("//kotlin/internal:defs.bzl", _KtKlibInfo = "KtKlibInfo", _TOOLCHAIN_TYPE = "TOOLCHAIN_TYPE") | ||
load("//kotlin/internal/utils:utils.bzl", "utils") | ||
|
||
def _kt_klib_library(ctx): | ||
module_name = utils.derive_module_name(ctx) | ||
builder_args = utils.init_args(ctx, "kt_klib_library", module_name) | ||
|
||
klib = ctx.actions.declare_file("{}.klib".format(ctx.label.name)) | ||
outputs = [klib] | ||
|
||
toolchains = ctx.toolchains[_TOOLCHAIN_TYPE] | ||
deps_klibs = [] | ||
for dep in ctx.attr.deps: | ||
deps_klibs.append(dep[_KtKlibInfo].klibs) | ||
libraries = depset(transitive = deps_klibs) | ||
builder_args.add_all("--sources", ctx.files.srcs) | ||
builder_inputs, _, input_manifests = ctx.resolve_command(tools = [toolchains.kotlinbuilder, toolchains.konan_home]) | ||
|
||
builder_args.add("--strict_kotlin_deps", "off") | ||
builder_args.add("--reduced_classpath_mode", "off") | ||
builder_args.add("--output_klib", klib.path) | ||
|
||
deps_klibs = [] | ||
for dep in ctx.attr.deps: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is duplicated from line #13? |
||
deps_klibs.append(dep[_KtKlibInfo].klibs) | ||
libraries = depset(transitive = deps_klibs) | ||
builder_args.add_all("--klibs", libraries, omit_if_empty = False) | ||
|
||
# This will be a directory we need to propagate to the compiler | ||
konan_home = toolchains.konan_home[DefaultInfo].files.to_list()[0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rule of thumb: if you unpack a depset with Let's step back and ask the question: does starlark need to know about konan home? Or can we derive it in the action? I suspect we could just pass the files into a well known place and work from there. |
||
if not konan_home.is_directory: | ||
fail("konan home must be a directory!") | ||
|
||
ctx.actions.run( | ||
mnemonic = "KotlinKlibCompile", | ||
inputs = depset(builder_inputs + ctx.files.srcs, transitive = [libraries]), | ||
outputs = outputs, | ||
executable = toolchains.kotlinbuilder.files_to_run.executable, | ||
tools = [ | ||
toolchains.kotlinbuilder.files_to_run, | ||
toolchains.konan_home[DefaultInfo].files_to_run, | ||
], | ||
execution_requirements = {"supports-workers": "1"}, | ||
arguments = [ctx.actions.args().add_all(toolchains.builder_args), builder_args], | ||
progress_message = "Compiling Kotlin to Klib %%{label} { kt: %d }" % len(ctx.files.srcs), | ||
input_manifests = input_manifests, | ||
env = { | ||
"REPOSITORY_NAME": utils.builder_workspace_name(ctx), | ||
"KONAN_HOME": konan_home.path, | ||
}, | ||
) | ||
|
||
return [ | ||
DefaultInfo(files = depset(outputs)), | ||
_KtKlibInfo( | ||
klibs = depset(outputs), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'll want to include the transitive outputs here. |
||
), | ||
] | ||
|
||
kt_klib_library = rule( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
kotlin seems to be moving towards using klib as the core intermediate format (js and native), so I suspect java may follow at some point. |
||
implementation = _kt_klib_library, | ||
doc = """ | ||
This rule is intended to leverage the new Kotlin IR backend to allow for compiling platform-independent Kotlin code | ||
to be shared between Kotlin code for different platforms (JS/JVM/WASM etc.). It produces a klib file as the output. | ||
""", | ||
attrs = { | ||
"srcs": attr.label_list( | ||
doc = "A list of source files to be compiled to klib", | ||
allow_files = [".kt"], | ||
), | ||
"deps": attr.label_list( | ||
doc = "A list of other kt_klib_library targets that this library depends on for compilation", | ||
providers = [_KtKlibInfo], | ||
), | ||
}, | ||
toolchains = [_TOOLCHAIN_TYPE], | ||
provides = [_KtKlibInfo], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ load("@rules_java//java:defs.bzl", "JavaInfo", "java_common") | |
load( | ||
"//kotlin/internal:defs.bzl", | ||
_KT_COMPILER_REPO = "KT_COMPILER_REPO", | ||
_KT_NATIVE_COMPILER_REPO_PREFIX = "KT_NATIVE_COMPILER_REPO_PREFIX", | ||
_TOOLCHAIN_TYPE = "TOOLCHAIN_TYPE", | ||
) | ||
load( | ||
|
@@ -79,6 +80,7 @@ def _kotlin_toolchain_impl(ctx): | |
], | ||
jdeps_merger = ctx.attr.jdeps_merger, | ||
kotlin_home = ctx.attr.kotlin_home, | ||
konan_home = ctx.attr.konan_home, | ||
jvm_stdlibs = java_common.merge(compile_time_providers + runtime_providers), | ||
jvm_emit_jdeps = ctx.attr._jvm_emit_jdeps[BuildSettingInfo].value, | ||
execution_requirements = { | ||
|
@@ -115,6 +117,10 @@ _kt_toolchain = rule( | |
default = Label("@" + _KT_COMPILER_REPO + "//:home"), | ||
allow_files = True, | ||
), | ||
"konan_home": attr.label( | ||
doc = "the filegroup defining the konan/kotlin-native home", | ||
allow_files = True, | ||
), | ||
"kotlinbuilder": attr.label( | ||
doc = "the kotlin builder executable", | ||
default = Label("//src/main/kotlin:build"), | ||
|
@@ -379,6 +385,12 @@ def define_kt_toolchain( | |
jvm_runtime = jvm_runtime if jvm_runtime != None else [ | ||
Label("//kotlin/compiler:kotlin-stdlib"), | ||
], | ||
konan_home = select({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each of these should be a separate toolchain with different platform constraints. If nothing else, it will make debugging toolchain issues sane. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense to put it into its own toolchain. |
||
"@bazel_tools//src/conditions:linux_x86_64": Label("@" + _KT_NATIVE_COMPILER_REPO_PREFIX + "_linux_x86_64//:konan_home"), | ||
"@bazel_tools//src/conditions:darwin_arm64": Label("@" + _KT_NATIVE_COMPILER_REPO_PREFIX + "_macos_aarch64//:konan_home"), | ||
"@bazel_tools//src/conditions:darwin_x86_64": Label("@" + _KT_NATIVE_COMPILER_REPO_PREFIX + "_macos_x86_64//:konan_home"), | ||
"@bazel_tools//src/conditions:windows": Label("@" + _KT_NATIVE_COMPILER_REPO_PREFIX + "_windows_x86_64//:konan_home"), | ||
}), | ||
) | ||
native.toolchain( | ||
name = name, | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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 added this to leverage
copy_to_directory
to expose the Kotlin native compiler distribution as a directory in the toolchain here (as it's massive and has a ton of files)Uh oh!
There was an error while loading. Please reload this page.
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.
Please do not use
copy_to_directory
it's basically antagonistic to RBE.copy_to_directory uses uses a tree artifact (via declare_directory which is, under the hood, a tarball.) It doesn't save time or number of files. Why? Because bazel only works with files, rather than directories. The intent of that feature is handle actions where the outputs are unknown before execution.
copy_to_directory
does know the inputs (all the files in the glob), and then applies filters etc to the them via an action... Which means we take a well known set of inputs, execute an action, place them in a tarball, and then unpack them everytime we need them (give or take -- there are some optimizations under the hood.)For RBE, this is ugly: first, transfer all the files into a remote action; second, tar all the outputs and place them in CAS. Given that the files have not changed, this bloats the CAS size, increases transfer costs, adds additional actions... for no good reason, other than laziness on the rule writers part. After all,
copy_to_directory
could be written in starlark to do the filtering... or the rules could handle the filtering during globbing, or...You get the idea.