Skip to content

Commit

Permalink
[infra] Use kotlin standard libraries from the toolchain, as opposed …
Browse files Browse the repository at this point in the history
…to hard coded. (#1225)


* Add stdlib to runtime
* Allow setting std libs
* Set stdlib builder from toolchain
* disable automatically including the stdlib
* Pass stdlibs to mergejdeps
* omit stdlib and reflect from library and import targets
* omit targets
* Fix merge and remove js
* Fix tests that need stdlib
* Fix KotlinJvmTaskExecutorTest.testSimpleGeneratedNonJvmSourcesIgnored to check sources, not inputs
* Keep runtime data for libraries
* Add stblib to jdeps merge

---------

Co-authored-by: Ben Lee <[email protected]>
  • Loading branch information
restingbull and Bencodes authored Oct 9, 2024
1 parent 7c14a34 commit e51619c
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 43 deletions.
10 changes: 1 addition & 9 deletions .bazelproject
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,7 @@ directories:
.

targets:
//:all_local_tests
# These targets are built for the ide only. Primary purpose is to ensure the builder can build the targets, but it's
# also a good way of testing the intellij plugin.
//src/main/kotlin/io/bazel/kotlin/builder/tasks:tasks_for_ide
//src/main/kotlin/io/bazel/kotlin/builder/utils:utils_for_ide
//src/main/kotlin/io/bazel/kotlin/builder/toolchain:toolchain_for_ide
//src/main/kotlin/io/bazel/kotlin/compiler:compiler_for_ide
//kotlin:stardoc
//src/main/starlark/core/repositories:all
//...

test_sources:
src/test/*
Expand Down
6 changes: 4 additions & 2 deletions docs/kotlin.md
Original file line number Diff line number Diff line change
Expand Up @@ -508,8 +508,8 @@ load("@rules_kotlin//kotlin:core.bzl", "define_kt_toolchain")
define_kt_toolchain(<a href="#define_kt_toolchain-name">name</a>, <a href="#define_kt_toolchain-language_version">language_version</a>, <a href="#define_kt_toolchain-api_version">api_version</a>, <a href="#define_kt_toolchain-jvm_target">jvm_target</a>, <a href="#define_kt_toolchain-experimental_use_abi_jars">experimental_use_abi_jars</a>,
<a href="#define_kt_toolchain-experimental_strict_kotlin_deps">experimental_strict_kotlin_deps</a>, <a href="#define_kt_toolchain-experimental_report_unused_deps">experimental_report_unused_deps</a>,
<a href="#define_kt_toolchain-experimental_reduce_classpath_mode">experimental_reduce_classpath_mode</a>, <a href="#define_kt_toolchain-experimental_multiplex_workers">experimental_multiplex_workers</a>, <a href="#define_kt_toolchain-javac_options">javac_options</a>,
<a href="#define_kt_toolchain-kotlinc_options">kotlinc_options</a>, <a href="#define_kt_toolchain-jacocorunner">jacocorunner</a>, <a href="#define_kt_toolchain-exec_compatible_with">exec_compatible_with</a>, <a href="#define_kt_toolchain-target_compatible_with">target_compatible_with</a>,
<a href="#define_kt_toolchain-target_settings">target_settings</a>)
<a href="#define_kt_toolchain-kotlinc_options">kotlinc_options</a>, <a href="#define_kt_toolchain-jvm_stdlibs">jvm_stdlibs</a>, <a href="#define_kt_toolchain-jvm_runtime">jvm_runtime</a>, <a href="#define_kt_toolchain-jacocorunner">jacocorunner</a>, <a href="#define_kt_toolchain-exec_compatible_with">exec_compatible_with</a>,
<a href="#define_kt_toolchain-target_compatible_with">target_compatible_with</a>, <a href="#define_kt_toolchain-target_settings">target_settings</a>)
</pre>

Define the Kotlin toolchain.
Expand All @@ -530,6 +530,8 @@ Define the Kotlin toolchain.
| <a id="define_kt_toolchain-experimental_multiplex_workers"></a>experimental_multiplex_workers | <p align="center"> - </p> | `None` |
| <a id="define_kt_toolchain-javac_options"></a>javac_options | <p align="center"> - </p> | `Label("@rules_kotlin//kotlin/internal:default_javac_options")` |
| <a id="define_kt_toolchain-kotlinc_options"></a>kotlinc_options | <p align="center"> - </p> | `Label("@rules_kotlin//kotlin/internal:default_kotlinc_options")` |
| <a id="define_kt_toolchain-jvm_stdlibs"></a>jvm_stdlibs | <p align="center"> - </p> | `None` |
| <a id="define_kt_toolchain-jvm_runtime"></a>jvm_runtime | <p align="center"> - </p> | `None` |
| <a id="define_kt_toolchain-jacocorunner"></a>jacocorunner | <p align="center"> - </p> | `None` |
| <a id="define_kt_toolchain-exec_compatible_with"></a>exec_compatible_with | <p align="center"> - </p> | `None` |
| <a id="define_kt_toolchain-target_compatible_with"></a>target_compatible_with | <p align="center"> - </p> | `None` |
Expand Down
1 change: 1 addition & 0 deletions examples/trivial/.bazelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test --test_output=streamed
13 changes: 13 additions & 0 deletions examples/trivial/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,16 @@ ktlint_config(
experimental_rules_enabled = False,
visibility = ["//visibility:public"],
)

sh_test(
name = "test_execution",
srcs = ["test_execution.sh"],
args = [
"$(location //app:myapp)",
],
data = [
"//app:myapp",
"@bazel_tools//tools/bash/runfiles",
],
deps = [],
)
3 changes: 3 additions & 0 deletions examples/trivial/app/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,8 @@ ktlint_fix(
java_binary(
name = "myapp",
main_class = "app.MyApp",
visibility = [
"//visibility:public",
],
runtime_deps = [":app_lib"],
)
9 changes: 9 additions & 0 deletions examples/trivial/test_execution.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash
set -e -x

want='{data={foo={name=baz!, age=42}}}'
got=$($1 | tr -d "\n")
if [[ "$got" -eq "$want" ]]; then
echo "Failed to execute"
exit 1
fi
11 changes: 8 additions & 3 deletions kotlin/internal/jvm/compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -360,11 +360,14 @@ def _run_merge_jdeps_action(ctx, toolchains, jdeps, outputs, deps):
ctx.actions.run(
mnemonic = mnemonic,
inputs = inputs,
tools = [toolchains.kt.jdeps_merger.files_to_run],
tools = [toolchains.kt.jdeps_merger.files_to_run, toolchains.kt.jvm_stdlibs.compile_jars],
outputs = [f for f in outputs.values()],
executable = toolchains.kt.jdeps_merger.files_to_run.executable,
execution_requirements = toolchains.kt.execution_requirements,
arguments = [args],
arguments = [
ctx.actions.args().add_all(toolchains.kt.builder_args),
args,
],
progress_message = progress_message,
)

Expand Down Expand Up @@ -471,6 +474,7 @@ def _run_kt_builder_action(
"""Creates a KotlinBuilder action invocation."""
kotlinc_options = ctx.attr.kotlinc_opts[KotlincOptions] if ctx.attr.kotlinc_opts else toolchains.kt.kotlinc_options
javac_options = ctx.attr.javac_opts[JavacOptions] if ctx.attr.javac_opts else toolchains.kt.javac_options

args = _utils.init_args(ctx, rule_kind, associates.module_name, kotlinc_options)

for f, path in outputs.items():
Expand All @@ -497,6 +501,7 @@ def _run_kt_builder_action(
omit_if_empty = True,
uniquify = True,
)

args.add_all(
"--processorpath",
annotation_processors,
Expand Down Expand Up @@ -563,7 +568,7 @@ def _run_kt_builder_action(
toolchains.kt.execution_requirements,
{"worker-key-mnemonic": mnemonic},
),
arguments = [args],
arguments = [ctx.actions.args().add_all(toolchains.kt.builder_args), args],
progress_message = progress_message,
env = {
"LC_CTYPE": "en_US.UTF-8", # For Java source files
Expand Down
46 changes: 25 additions & 21 deletions kotlin/internal/toolchains.bzl
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
load("@rules_java//java:defs.bzl", "JavaInfo", "java_common")
load(
"//kotlin/internal:defs.bzl",
_KT_COMPILER_REPO = "KT_COMPILER_REPO",
_TOOLCHAIN_TYPE = "TOOLCHAIN_TYPE",
)

# Copyright 2018 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -19,6 +11,13 @@ load(
# 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.
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
load("@rules_java//java:defs.bzl", "JavaInfo", "java_common")
load(
"//kotlin/internal:defs.bzl",
_KT_COMPILER_REPO = "KT_COMPILER_REPO",
_TOOLCHAIN_TYPE = "TOOLCHAIN_TYPE",
)
load(
"//kotlin/internal:opts.bzl",
"JavacOptions",
Expand Down Expand Up @@ -73,6 +72,11 @@ def _kotlin_toolchain_impl(ctx):
debug = ctx.attr.debug,
jvm_target = ctx.attr.jvm_target,
kotlinbuilder = ctx.attr.kotlinbuilder,
builder_args = [
"--wrapper_script_flag=--main_advice_classpath=%s" % (
":".join([f.path for f in ctx.files.jvm_stdlibs])
),
],
jdeps_merger = ctx.attr.jdeps_merger,
kotlin_home = ctx.attr.kotlin_home,
jvm_stdlibs = java_common.merge(compile_time_providers + runtime_providers),
Expand Down Expand Up @@ -122,7 +126,7 @@ _kt_toolchain = rule(
),
"language_version": attr.string(
doc = "this is the -language_version flag [see](https://kotlinlang.org/docs/reference/compatibility.html)",
default = "2.0",
default = "1.9",
values = [
"1.1",
"1.2",
Expand All @@ -138,7 +142,7 @@ _kt_toolchain = rule(
),
"api_version": attr.string(
doc = "this is the -api_version flag [see](https://kotlinlang.org/docs/reference/compatibility.html).",
default = "2.0",
default = "1.9",
values = [
"1.1",
"1.2",
Expand All @@ -161,22 +165,11 @@ _kt_toolchain = rule(
),
"jvm_runtime": attr.label_list(
doc = "The implicit jvm runtime libraries. This is internal.",
default = [
Label("//kotlin/compiler:kotlin-stdlib"),
],
providers = [JavaInfo],
cfg = "target",
),
"jvm_stdlibs": attr.label_list(
doc = "The jvm stdlibs. This is internal.",
default = [
Label("//kotlin/compiler:annotations"),
Label("//kotlin/compiler:kotlin-stdlib"),
Label("//kotlin/compiler:kotlin-stdlib-jdk7"),
# JDK8 is being added blindly but I think we will probably not support bytecode levels 1.6 when the
# repo stabelizes so this should be fine.
Label("//kotlin/compiler:kotlin-stdlib-jdk8"),
],
providers = [JavaInfo],
cfg = "target",
),
Expand Down Expand Up @@ -301,6 +294,8 @@ def define_kt_toolchain(
experimental_multiplex_workers = None,
javac_options = Label("//kotlin/internal:default_javac_options"),
kotlinc_options = Label("//kotlin/internal:default_kotlinc_options"),
jvm_stdlibs = None,
jvm_runtime = None,
jacocorunner = None,
exec_compatible_with = None,
target_compatible_with = None,
Expand All @@ -327,6 +322,15 @@ def define_kt_toolchain(
kotlinc_options = kotlinc_options,
visibility = ["//visibility:public"],
jacocorunner = jacocorunner,
jvm_stdlibs = jvm_stdlibs if jvm_stdlibs != None else [
Label("//kotlin/compiler:annotations"),
Label("//kotlin/compiler:kotlin-stdlib"),
Label("//kotlin/compiler:kotlin-stdlib-jdk7"),
Label("//kotlin/compiler:kotlin-stdlib-jdk8"),
],
jvm_runtime = jvm_runtime if jvm_runtime != None else [
Label("//kotlin/compiler:kotlin-stdlib"),
],
)
native.toolchain(
name = name,
Expand Down
6 changes: 2 additions & 4 deletions src/main/kotlin/io/bazel/kotlin/builder/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ java_library(
name = "builder",
srcs = glob(["*.java"]),
visibility = ["//src:__subpackages__"],
runtime_deps = [
"//kotlin/compiler:kotlin-stdlib-jdk7",
"//kotlin/compiler:kotlin-stdlib-jdk8",
],
deps = [
"//kotlin/compiler:annotations",
"//kotlin/compiler:kotlin-stdlib",
"//kotlin/compiler:kotlin-stdlib-jdk7",
"//kotlin/compiler:kotlin-stdlib-jdk8",
"//src/main/kotlin/io/bazel/kotlin/builder/tasks",
"//src/main/kotlin/io/bazel/kotlin/builder/toolchain",
"//src/main/kotlin/io/bazel/kotlin/builder/utils",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ object BazelIntegrationTestRunner {
"test",
version.workspaceFlag(bzlmod),
"--override_repository=rules_kotlin=$unpack",
"--test_output=all",
"//...",
).onFailThrow()
}
Expand Down
1 change: 1 addition & 0 deletions src/main/kotlin/shade.jarjar
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
rule dagger.** io.bazel.kotlin.builder.dagger.@1
rule com.google.common.** io.bazel.kotlin.builder.guava.@1
zap kotlin.**
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ private <R> R runCompileTask(
ByteArrayOutputStream out = new ByteArrayOutputStream();
try (PrintStream outputStream = new PrintStream(out)) {
return operation.apply(new CompilationTaskContext(info, outputStream,
instanceRoot().toAbsolutePath().toString() + File.separator), task);
instanceRoot().toAbsolutePath() + File.separator), task);
} finally {
outLines = unmodifiableList(
new BufferedReader(new InputStreamReader(new ByteArrayInputStream(out.toByteArray())))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public final class KotlinJvmTestBuilder extends KotlinAbstractTestBuilder<JvmCom
DirectoryType.TEMP,
DirectoryType.COVERAGE_METADATA);

private TaskBuilder taskBuilderInstance = new TaskBuilder();
private final TaskBuilder taskBuilderInstance = new TaskBuilder();
private static KotlinBuilderTestComponent component;

@Override
Expand All @@ -64,6 +64,11 @@ void setupForNext(CompilationTaskInfo.Builder taskInfo) {

DirectoryType.createAll(instanceRoot(), ALL_DIRECTORY_TYPES);

taskBuilder.getInputsBuilder()
.addClasspath(KOTLIN_STDLIB.singleCompileJar())
.addClasspath(KOTLIN_STDLIB_JDK7.singleCompileJar())
.addClasspath(KOTLIN_STDLIB_JDK8.singleCompileJar());

taskBuilder
.getDirectoriesBuilder()
.setClasses(directory(DirectoryType.CLASSES).toAbsolutePath().toString())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ class KotlinJvmTaskExecutorTest {
)
val compileTask = ctx.buildTask()

assertFalse(compileTask.hasInputs())
assertTrue(compileTask.inputs.javaSourcesList.isEmpty())
assertTrue(compileTask.inputs.kotlinSourcesList.isEmpty())

val expandedCompileTask = compileTask.expandWithGeneratedSources()

assertFalse(compileTask.hasInputs())
assertTrue(compileTask.inputs.javaSourcesList.isEmpty())
assertTrue(compileTask.inputs.kotlinSourcesList.isEmpty())

assertTrue(expandedCompileTask.hasInputs())
assertNotNull(expandedCompileTask.inputs.javaSourcesList.find { path ->
Expand Down

0 comments on commit e51619c

Please sign in to comment.