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

[infra] Use kotlin standard libraries from the toolchain, as opposed to hard coded. #1225

Merged
merged 14 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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(
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a skylib macro for this that asserts if something builds.

Probably worth using just to minimize the surface area of rules Kotlin a bit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needs to run to test it. Building is easy -- but the kotlin/jvm integration will only fail if it doesn't run.

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",
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we going back to an older version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't merge?

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