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

WIP: Coverage in Kotlin #429

Closed
wants to merge 6 commits into from
Closed
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
22 changes: 21 additions & 1 deletion kotlin/internal/jvm/compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,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 @@ -511,12 +515,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 @@ -616,6 +622,19 @@ def kt_jvm_produce_jar_actions(ctx, rule_kind):
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 @@ -847,3 +866,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")
),
),
] + list(additional_providers),
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,
"%jvm_flags%": jvm_flags,
"%workspace_prefix%": workspace_prefix,
}

if ctx.configuration.coverage_enabled:
metadata = ctx.actions.declare_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.to_list()]
ctx.actions.write(metadata, content="\n".join(metadata_entries))
substitutions = _utils.add_dicts(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 = _utils.add_dicts(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 @@ -152,6 +152,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
5 changes: 4 additions & 1 deletion src/main/kotlin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ load("@rules_java//java:defs.bzl", "java_binary")
load("//kotlin:kotlin.bzl", "kt_jvm_library")
load("//third_party:jarjar.bzl", "jar_jar")
load("//kotlin/internal/utils:packager.bzl", "release_archive")
load("//src/main/kotlin:deps.bzl", "ASM_DEPS")

java_binary(
name = "builder_raw",
Expand Down Expand Up @@ -60,7 +61,9 @@ java_binary(
],
main_class = "io.bazel.kotlin.builder.KotlinBuilderMain",
visibility = ["//visibility:public"],
runtime_deps = [":builder_jar_jar"],
runtime_deps = [
":builder_jar_jar",
] + ASM_DEPS,
)

java_binary(
Expand Down
6 changes: 5 additions & 1 deletion src/main/kotlin/BUILD.release.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("//src/main/kotlin:deps.bzl", "ASM_DEPS")

java_import(
name = "worker",
jars = ["kotlin_worker.jar"],
Expand Down Expand Up @@ -42,7 +44,9 @@ java_binary(
],
main_class = "io.bazel.kotlin.builder.KotlinBuilderMain",
visibility = ["//visibility:public"],
runtime_deps = [":worker"],
runtime_deps = [
":worker",
] + ASM_DEPS,
)

java_binary(
Expand Down
12 changes: 12 additions & 0 deletions src/main/kotlin/deps.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@

ASM_DEPS = [
"@kotlin_rules_maven//:org_ow2_asm_asm_tree",
"@kotlin_rules_maven//:org_ow2_asm_asm",
"@kotlin_rules_maven//:org_ow2_asm_asm_commons",
"@kotlin_rules_maven//:org_ow2_asm_asm_analysis",

]

JACOCO_DEPS = [
"@kotlin_rules_maven//:org_jacoco_org_jacoco_core",
]
3 changes: 2 additions & 1 deletion src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

load("//src/main/kotlin:bootstrap.bzl", "kt_bootstrap_library")
load("//src/main/kotlin:deps.bzl", "JACOCO_DEPS")

kt_bootstrap_library(
name = "tasks",
Expand All @@ -31,5 +32,5 @@ kt_bootstrap_library(
"@kotlin_rules_maven//:com_google_protobuf_protobuf_java",
"@kotlin_rules_maven//:com_google_protobuf_protobuf_java_util",
"@kotlin_rules_maven//:javax_inject_javax_inject",
],
] + JACOCO_DEPS,
)
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 @@ -177,6 +178,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 @@ -114,6 +115,11 @@ class KotlinJvmTaskExecutor @Inject internal constructor(
}
}

if (this.info.postProcessor == "jacoco") {
context.execute("instrument class files") {
jacocoProcessor.instrument(this)
}
}
Copy link

Choose a reason for hiding this comment

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

There may have an issue with the inline function across modules.

If the compiled jar has only instrumented byte code, calling the inline function when running the test will result in an error like

java.lang.IllegalAccessError: class com.package.Foo tried to access private method 'boolean[] com.package.BarKt.$jacocoInit()'

The reason is that when compiling Bar.kt, it has only the instrumented version of Foo. And the compiler embeds the bytecode of the instrumented inline function.

I think the solution is to create two jars, the uninstrumented one for compilation and the instrumented one for runtime.

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 @@ -82,6 +82,8 @@ message CompilationTaskInfo {
string strict_kotlin_deps = 10;
// Optimize classpath by removing dependencies not required for compilation
string reduced_classpath_mode = 11;
// Named post-processor to apply, if any.
string post_processor = 12;
}

// Mested messages not marked with stable could be refactored.
Expand Down
4 changes: 3 additions & 1 deletion third_party/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ load("@rules_java//java:defs.bzl", "java_binary", "java_library", "java_plugin")
load("//kotlin:kotlin.bzl", "kt_jvm_import")
load("//kotlin/internal/utils:packager.bzl", "release_archive")
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load("//src/main/kotlin:deps.bzl", "ASM_DEPS")

exports_files([
"empty.jar",
Expand Down Expand Up @@ -86,7 +87,8 @@ release_archive(
"empty.jar",
"empty.jdeps",
"jarjar.bzl",
],
"@kotlin_rules_maven//:org_pantsbuild_jarjar",
] + ASM_DEPS,
src_map = {
"jarjar_runner_deploy.jar": "jarjar.jar",
"BUILD.release.bazel": "BUILD.bazel",
Expand Down