Skip to content

Commit

Permalink
WIP: Coverage in Kotlin
Browse files Browse the repository at this point in the history
Builds on previous work (bazelbuild#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.
  • Loading branch information
Sam Gammon committed Dec 16, 2020
1 parent 77b6458 commit 495db02
Show file tree
Hide file tree
Showing 10 changed files with 142 additions and 12 deletions.
22 changes: 21 additions & 1 deletion kotlin/internal/jvm/compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -778,3 +797,4 @@ def export_only_providers(ctx, actions, attr, outputs):
),
),
)

52 changes: 43 additions & 9 deletions kotlin/internal/jvm/impl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""):
Expand All @@ -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"""
Expand Down
8 changes: 8 additions & 0 deletions kotlin/internal/jvm/jvm.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions kotlin/internal/repositories/setup.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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/",
Expand Down
7 changes: 6 additions & 1 deletion src/main/kotlin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Path>() {
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
}
})
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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? {
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/protobuf/kotlin_model.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 495db02

Please sign in to comment.