From 495db022827bb498462b9ef486e2c97110dc1f00 Mon Sep 17 00:00:00 2001 From: Sam Gammon Date: Tue, 15 Dec 2020 20:54:34 -0800 Subject: [PATCH] WIP: Coverage in Kotlin Builds on previous work (bazelbuild/rules_kotlin#52) to add coverage support to Kotlin targets in Bazel. So far, it's working (in basic form), but seems to omit major swaths of our code when I test it on a live codebase. I'm not sure why that happens yet. --- kotlin/internal/jvm/compile.bzl | 22 +++++++- kotlin/internal/jvm/impl.bzl | 52 +++++++++++++++---- kotlin/internal/jvm/jvm.bzl | 8 +++ kotlin/internal/repositories/setup.bzl | 4 ++ src/main/kotlin/BUILD | 7 ++- .../io/bazel/kotlin/builder/tasks/BUILD.bazel | 1 + .../kotlin/builder/tasks/KotlinBuilder.kt | 3 ++ .../builder/tasks/jvm/JacocoProcessor.kt | 47 +++++++++++++++++ .../tasks/jvm/KotlinJvmTaskExecutor.kt | 8 ++- src/main/protobuf/kotlin_model.proto | 2 + 10 files changed, 142 insertions(+), 12 deletions(-) create mode 100644 src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/JacocoProcessor.kt diff --git a/kotlin/internal/jvm/compile.bzl b/kotlin/internal/jvm/compile.bzl index e94a69c4f..e22ef2e3f 100644 --- a/kotlin/internal/jvm/compile.bzl +++ b/kotlin/internal/jvm/compile.bzl @@ -427,6 +427,10 @@ def _run_kt_builder_action( omit_if_empty = True, ) + # Post-process class files with the Jacoco offline instrumenter, if needed. + if ctx.coverage_instrumented(): + args.add("--post_processor", "jacoco") + args.add("--build_java", build_java) args.add("--build_kotlin", build_kotlin) @@ -466,12 +470,14 @@ def _run_kt_builder_action( # MAIN ACTIONS ######################################################################################################### -def kt_jvm_produce_jar_actions(ctx, rule_kind): +def kt_jvm_produce_jar_actions(ctx, rule_kind, transitive_files=depset(order="default"), extra_runfiles=[]): """This macro sets up a compile action for a Kotlin jar. Args: ctx: Invoking rule ctx, used for attr, actions, and label. rule_kind: The rule kind --e.g., `kt_jvm_library`. + transitive_files: Transitive files to inject into the output set. + extra_runfiles: Extra runfiles to append to the output set. Returns: A struct containing the providers JavaInfo (`java`) and `kt` (KtJvmInfo). This struct is not intended to be used as a legacy provider -- rather the caller should transform the result. @@ -552,6 +558,19 @@ def kt_jvm_produce_jar_actions(ctx, rule_kind): java_toolchain = toolchains.java, host_javabase = toolchains.java_runtime) + outs = [output_jar] + if hasattr(ctx.outputs, "executable"): + outs.append(ctx.outputs.executable) + + default_info = DefaultInfo( + files = depset(outs), + runfiles = ctx.runfiles( + files = extra_runfiles + [output_jar], + transitive_files = transitive_files, + collect_default = True, + ), + ) + return struct( java = JavaInfo( output_jar = output_jar, @@ -778,3 +797,4 @@ def export_only_providers(ctx, actions, attr, outputs): ), ), ) + diff --git a/kotlin/internal/jvm/impl.bzl b/kotlin/internal/jvm/impl.bzl index 22982763c..4280eef52 100644 --- a/kotlin/internal/jvm/impl.bzl +++ b/kotlin/internal/jvm/impl.bzl @@ -45,6 +45,11 @@ def _make_providers(ctx, providers, transitive_files = depset(order = "default") ), ), ], + instrumented_files = struct( + source_attributes = ["srcs"], + dependency_attributes = ["deps", "runtime_deps"], + extensions = ["kt", "java"], + ), ) def _write_launcher_action(ctx, rjars, main_class, jvm_flags, args = "", wrapper_preamble = ""): @@ -59,23 +64,52 @@ def _write_launcher_action(ctx, rjars, main_class, jvm_flags, args = "", wrapper jvm_flags = " ".join([ctx.expand_location(f, ctx.attr.data) for f in jvm_flags]) template = ctx.attr._java_stub_template.files.to_list()[0] javabin = "JAVABIN=" + str(ctx.attr._java_runtime[java_common.JavaRuntimeInfo].java_executable_exec_path) + workspace_prefix = ctx.workspace_name + "/" - ctx.actions.expand_template( - template = template, - output = ctx.outputs.executable, - substitutions = { - "%classpath%": classpath, + extra_runfiles = [] + substitutions = { + "%classpath%": classpath, + "%javabin%": "JAVABIN=${RUNPATH}" + ctx.executable._java.short_path, + "%jvm_flags%": jvm_flags, + "%workspace_prefix%": workspace_prefix, + } + + if ctx.configuration.coverage_enabled: + metadata = ctx.new_file("coverage_runtime_classpath/%s/runtime-classpath.txt" % ctx.attr.name) + extra_runfiles.append(metadata) + # We replace '../' to get a runtime-classpath.txt as close as possible to the one + # produced by java_binary. + metadata_entries = [rjar.short_path.replace("../", "external/") for rjar in rjars] + ctx.file_action(metadata, content="\n".join(metadata_entries)) + substitutions += { + "%java_start_class%": "com.google.testing.coverage.JacocoCoverageRunner", + # %set_jacoco_main_class% and %set_jacoco_java_runfiles_root% are not + # taken into account, so we cram everything with %set_jacoco_metadata%. + "%set_jacoco_metadata%": "\n".join([ + "export JACOCO_METADATA_JAR=${JAVA_RUNFILES}/" + workspace_prefix + metadata.short_path, + "export JACOCO_MAIN_CLASS=" + main_class, + "export JACOCO_JAVA_RUNFILES_ROOT=${JAVA_RUNFILES}/" + workspace_prefix, + ]), + "%set_jacoco_main_class%": "", + "%set_jacoco_java_runfiles_root%": "", + "%set_java_coverage_new_implementation%": "", + } + else: + substitutions += { "%java_start_class%": main_class, - "%javabin%": javabin, - "%jvm_flags%": jvm_flags, "%set_jacoco_metadata%": "", "%set_jacoco_main_class%": "", "%set_jacoco_java_runfiles_root%": "", "%set_java_coverage_new_implementation%": "", - "%workspace_prefix%": ctx.workspace_name + "/", - }, + } + + ctx.actions.expand_template( + template = template, + output = ctx.outputs.executable, + substitutions = substitutions, is_executable = True, ) + return extra_runfiles def _is_source_jar_stub(jar): """Workaround for intellij plugin expecting a source jar""" diff --git a/kotlin/internal/jvm/jvm.bzl b/kotlin/internal/jvm/jvm.bzl index dcbb5c4ca..5963a710e 100644 --- a/kotlin/internal/jvm/jvm.bzl +++ b/kotlin/internal/jvm/jvm.bzl @@ -147,6 +147,14 @@ _implicit_deps = { "_java_runtime": attr.label( default = Label("@bazel_tools//tools/jdk:current_java_runtime"), ), + "_jacocorunner": attr.label( + default = Label("@bazel_tools//tools/jdk:JacocoCoverage"), + ), + "_lcov_merger": attr.label( + default = Label("@bazel_tools//tools/test:lcov_merger"), + executable = True, + cfg = "target", + ), } _common_attr = utils.add_dicts( diff --git a/kotlin/internal/repositories/setup.bzl b/kotlin/internal/repositories/setup.bzl index 293e798b9..d8fc218dd 100644 --- a/kotlin/internal/repositories/setup.bzl +++ b/kotlin/internal/repositories/setup.bzl @@ -39,12 +39,16 @@ def kt_configure(): "javax.annotation:javax.annotation-api:1.3.2", "javax.inject:javax.inject:1", "org.pantsbuild:jarjar:1.7.2", + "org.jacoco:org.jacoco.core:0.8.3", "org.jetbrains.kotlinx:atomicfu-js:0.14.0", "org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9", "org.jetbrains.kotlinx:kotlinx-coroutines-core-js:1.3.9", "org.jetbrains.kotlinx:kotlinx-coroutines-test:1.3.9", "org.jetbrains.kotlinx:kotlinx-coroutines-debug:1.3.9", "org.jetbrains.kotlinx:kotlinx-serialization-runtime:1.0-M1-1.4.0-rc", + "org.ow2.asm:asm-commons:9.0", + "org.ow2.asm:asm-tree:9.0", + "org.ow2.asm:asm:9.0", ], repositories = [ "https://maven-central.storage.googleapis.com/repos/central/data/", diff --git a/src/main/kotlin/BUILD b/src/main/kotlin/BUILD index b9d5be448..e1d9b027c 100644 --- a/src/main/kotlin/BUILD +++ b/src/main/kotlin/BUILD @@ -52,7 +52,12 @@ java_binary( ], main_class = "io.bazel.kotlin.builder.KotlinBuilderMain", visibility = ["//visibility:public"], - runtime_deps = [":builder_jar_jar"], + runtime_deps = [ + ":builder_jar_jar", + "@kotlin_rules_maven//:org_ow2_asm_asm", + "@kotlin_rules_maven//:org_ow2_asm_asm_commons", + "@kotlin_rules_maven//:org_ow2_asm_asm_tree", + ], ) java_binary( diff --git a/src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel b/src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel index b07e40672..349becacc 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel +++ b/src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel @@ -29,6 +29,7 @@ kt_bootstrap_library( "@com_github_jetbrains_kotlin//:kotlin-preloader", "@kotlin_rules_maven//:com_google_protobuf_protobuf_java", "@kotlin_rules_maven//:com_google_protobuf_protobuf_java_util", + "@kotlin_rules_maven//:org_jacoco_org_jacoco_core", "@kotlin_rules_maven//:javax_inject_javax_inject", ], ) diff --git a/src/main/kotlin/io/bazel/kotlin/builder/tasks/KotlinBuilder.kt b/src/main/kotlin/io/bazel/kotlin/builder/tasks/KotlinBuilder.kt index da2b07898..9a94611e9 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/tasks/KotlinBuilder.kt +++ b/src/main/kotlin/io/bazel/kotlin/builder/tasks/KotlinBuilder.kt @@ -98,6 +98,7 @@ class KotlinBuilder @Inject internal constructor( enum class KotlinBuilderFlags(override val flag: String) : Flag { MODULE_NAME("--kotlin_module_name"), + POST_PROCESSOR("--post_processor"), PASSTHROUGH_FLAGS("--kotlin_passthrough_flags"), API_VERSION("--kotlin_api_version"), LANGUAGE_VERSION("--kotlin_language_version"), @@ -175,6 +176,8 @@ class KotlinBuilder @Inject internal constructor( } addAllPassthroughFlags(argMap.optional(KotlinBuilderFlags.PASSTHROUGH_FLAGS) ?: emptyList()) + argMap.optionalSingle(KotlinBuilderFlags.POST_PROCESSOR)?.let(::setPostProcessor) + argMap.optional(KotlinBuilderFlags.FRIEND_PATHS)?.let(::addAllFriendPaths) toolchainInfoBuilder.commonBuilder.apiVersion = argMap.mandatorySingle(KotlinBuilderFlags.API_VERSION) diff --git a/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/JacocoProcessor.kt b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/JacocoProcessor.kt new file mode 100644 index 000000000..7d2fcb01f --- /dev/null +++ b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/JacocoProcessor.kt @@ -0,0 +1,47 @@ +package io.bazel.kotlin.builder.tasks.jvm + +import io.bazel.kotlin.builder.toolchain.KotlinToolchain +import org.jacoco.core.instr.Instrumenter +import org.jacoco.core.runtime.OfflineInstrumentationAccessGenerator +import java.io.BufferedInputStream +import java.io.BufferedOutputStream +import java.io.IOException +import java.nio.file.FileVisitResult +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.Paths +import java.nio.file.SimpleFileVisitor +import java.nio.file.attribute.BasicFileAttributes +import io.bazel.kotlin.model.JvmCompilationTask +import com.google.devtools.build.lib.view.proto.Deps +import javax.inject.Inject + + +/** Implements Jacoco instrumentation. */ +class JacocoProcessor @Inject constructor(val compiler: KotlinToolchain.KotlincInvoker) { + fun instrument(command: JvmCompilationTask) { + val classDir = Paths.get(command.directories.classes) + val instr = Instrumenter(OfflineInstrumentationAccessGenerator()) + + // Runs Jacoco instrumentation processor over all .class files. + Files.walkFileTree( + classDir, + object : SimpleFileVisitor() { + override fun visitFile(file: Path, attrs: BasicFileAttributes): FileVisitResult { + if (!file.fileName.toString().endsWith(".class")) { + return FileVisitResult.CONTINUE + } + + val uninstrumentedCopy = Paths.get(file.toString() + ".uninstrumented") + Files.move(file, uninstrumentedCopy) + BufferedInputStream(Files.newInputStream(uninstrumentedCopy)).use { input -> + BufferedOutputStream(Files.newOutputStream(file)).use { output -> + instr.instrument(input, output, file.toString()) + } + } + return FileVisitResult.CONTINUE + } + }) + } +} + diff --git a/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt index ef88fe269..817fb188c 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt +++ b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt @@ -34,7 +34,8 @@ class KotlinJvmTaskExecutor @Inject internal constructor( private val compiler: KotlinToolchain.KotlincInvoker, private val javaCompiler: JavaCompiler, private val jDepsGenerator: JDepsGenerator, - private val plugins: InternalCompilerPlugins + private val plugins: InternalCompilerPlugins, + private val jacocoProcessor: JacocoProcessor ) { private fun combine(one: Throwable?, two: Throwable?): Throwable? { @@ -105,6 +106,11 @@ class KotlinJvmTaskExecutor @Inject internal constructor( } } + if (this.info.postProcessor == "jacoco") { + context.execute("instrument class files") { + jacocoProcessor.instrument(this) + } + } if (outputs.jar.isNotEmpty()) { context.execute("create jar", ::createOutputJar) } diff --git a/src/main/protobuf/kotlin_model.proto b/src/main/protobuf/kotlin_model.proto index 0224f65c9..21bf1aaa1 100644 --- a/src/main/protobuf/kotlin_model.proto +++ b/src/main/protobuf/kotlin_model.proto @@ -78,6 +78,8 @@ message CompilationTaskInfo { // trace: enables trace logging. // timings: causes timing information to be printed at the of an action. repeated string debug = 9; + // Named post-processor to apply, if any. + string post_processor = 10; } // Mested messages not marked with stable could be refactored.