Skip to content

Commit

Permalink
Add a tristate (off/warn/error) experimental_report_unused_deps
Browse files Browse the repository at this point in the history 
attribute to the toolchain.

Implement unused dependency checking in `JDepsMerger` tool.

Collect jdeps dependency artifacts if experimental_report_unused_deps
is enabled (warn/error) and pass to KotlinBuilder. Passing these inputs
triggers `JDepsMerge` action for each target causing the unused deps
checking logic to be executed. Currently these inputs are not used by
the `KotlinBuilder` but will be required for future work implementing
a reduced classpath feature.

Add tests for experimental_report_unused_deps off/warn/error

Enable experimental_report_unused_deps as "warn" for examples/android
  • Loading branch information
@jongerrish
jongerrish committed Jan 8, 2021
1 parent 9e36fca commit 97beff8
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 21 deletions.
1 change: 1 addition & 0 deletions examples/android/bzl/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ define_kt_toolchain(
api_version = "1.4",
experimental_use_abi_jars = True,
experimental_strict_kotlin_deps = "warn",
experimental_report_unused_deps = "warn",
javac_options = ":default_javac_options",
kotlinc_options = ":default_kotlinc_options",
language_version = "1.4",
Expand Down
21 changes: 19 additions & 2 deletions kotlin/internal/jvm/compile.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@ load(
def _java_info(target):
return target[JavaInfo] if JavaInfo in target else None

def _deps_artifacts(toolchains, targets):
"""Collect Jdeps artifacts if required."""
if not toolchains.kt.experimental_report_unused_deps == "off":
deps_artifacts = [t[JavaInfo].outputs.jdeps for t in targets if JavaInfo in t and t[JavaInfo].outputs.jdeps]
else:
deps_artifacts = []

return depset(deps_artifacts)

def _partitioned_srcs(srcs):
"""Creates a struct of srcs sorted by extension. Fails if there are no sources."""
kt_srcs = []
Expand Down Expand Up @@ -320,6 +329,7 @@ def _run_merge_jdeps_action(ctx, rule_kind, toolchains, jdeps, outputs):
args.add("--" + f, path)

args.add_all("--inputs", jdeps, omit_if_empty = True)
args.add("--report_unused_deps", toolchains.kt.experimental_report_unused_deps)

mnemonic = "JdepsMerge"
progress_message = "%s %s { jdeps: %d }" % (
Expand Down Expand Up @@ -356,6 +366,7 @@ def _run_kt_builder_action(
generated_src_jars,
friend,
compile_deps,
deps_artifacts,
annotation_processors,
transitive_runtime_jars,
plugins,
Expand All @@ -382,6 +393,7 @@ def _run_kt_builder_action(
args.add_all("--classpath", compile_deps.compile_jars)
args.add_all("--sources", srcs.all_srcs, omit_if_empty = True)
args.add_all("--source_jars", srcs.src_jars + generated_src_jars, omit_if_empty = True)
args.add_all("--deps_artifacts", deps_artifacts, omit_if_empty = True)

args.add_joined("--kotlin_friend_paths", friend.paths, join_with = "\n")

Expand Down Expand Up @@ -481,7 +493,7 @@ def _run_kt_builder_action(
mnemonic = mnemonic,
inputs = depset(
srcs.all_srcs + srcs.src_jars + generated_src_jars,
transitive = [compile_deps.compile_jars, transitive_runtime_jars] + [p.classpath for p in compiler_plugins],
transitive = [compile_deps.compile_jars, transitive_runtime_jars, deps_artifacts] + [p.classpath for p in compiler_plugins],
),
tools = tools,
input_manifests = input_manifests,
Expand Down Expand Up @@ -520,6 +532,7 @@ def kt_jvm_produce_jar_actions(ctx, rule_kind):
annotation_processors = _plugin_mappers.targets_to_annotation_processors(ctx.attr.plugins + ctx.attr.deps)
transitive_runtime_jars = _plugin_mappers.targets_to_transitive_runtime_jars(ctx.attr.plugins + ctx.attr.deps)
plugins = ctx.attr.plugins + _exported_plugins(deps = ctx.attr.deps)
deps_artifacts = _deps_artifacts(toolchains, ctx.attr.deps + friend.targets)

generated_src_jars = []
if toolchains.kt.experimental_use_abi_jars:
Expand All @@ -532,6 +545,7 @@ def kt_jvm_produce_jar_actions(ctx, rule_kind):
generated_src_jars = [],
friend = friend,
compile_deps = compile_deps,
deps_artifacts = deps_artifacts,
annotation_processors = annotation_processors,
transitive_runtime_jars = transitive_runtime_jars,
plugins = plugins,
Expand All @@ -552,6 +566,7 @@ def kt_jvm_produce_jar_actions(ctx, rule_kind):
generated_src_jars = [],
friend = friend,
compile_deps = compile_deps,
deps_artifacts = deps_artifacts,
annotation_processors = annotation_processors,
transitive_runtime_jars = transitive_runtime_jars,
plugins = plugins,
Expand Down Expand Up @@ -634,7 +649,7 @@ def kt_jvm_produce_jar_actions(ctx, rule_kind):
"""Runs the necessary KotlinBuilder and JavaBuilder actions to compile a jar
"""

def _run_kt_java_builder_actions(ctx, rule_kind, toolchains, srcs, generated_src_jars, friend, compile_deps, annotation_processors, transitive_runtime_jars, plugins, compile_jar):
def _run_kt_java_builder_actions(ctx, rule_kind, toolchains, srcs, generated_src_jars, friend, compile_deps, deps_artifacts, annotation_processors, transitive_runtime_jars, plugins, compile_jar):
compile_jars = []
output_jars = []
kt_stubs_for_java = []
Expand All @@ -652,6 +667,7 @@ def _run_kt_java_builder_actions(ctx, rule_kind, toolchains, srcs, generated_src
generated_src_jars = [],
friend = friend,
compile_deps = compile_deps,
deps_artifacts = deps_artifacts,
annotation_processors = annotation_processors,
transitive_runtime_jars = transitive_runtime_jars,
plugins = plugins,
Expand Down Expand Up @@ -696,6 +712,7 @@ def _run_kt_java_builder_actions(ctx, rule_kind, toolchains, srcs, generated_src
generated_src_jars = generated_src_jars,
friend = friend,
compile_deps = compile_deps,
deps_artifacts = deps_artifacts,
annotation_processors = [],
transitive_runtime_jars = transitive_runtime_jars,
plugins = plugins,
Expand Down
12 changes: 12 additions & 0 deletions kotlin/internal/toolchains.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def _kotlin_toolchain_impl(ctx):
js_stdlibs = ctx.attr.js_stdlibs,
experimental_use_abi_jars = ctx.attr.experimental_use_abi_jars,
experimental_strict_kotlin_deps = ctx.attr.experimental_strict_kotlin_deps,
experimental_report_unused_deps = ctx.attr.experimental_report_unused_deps,
javac_options = ctx.attr.javac_options[JavacOptions] if ctx.attr.javac_options else None,
kotlinc_options = ctx.attr.kotlinc_options[KotlincOptions] if ctx.attr.kotlinc_options else None,
empty_jar = ctx.file._empty_jar,
Expand Down Expand Up @@ -199,6 +200,15 @@ _kt_toolchain = rule(
"error",
],
),
"experimental_report_unused_deps": attr.string(
doc = "Report unused dependencies",
default = "off",
values = [
"off",
"warn",
"error",
],
),
"javac_options": attr.label(
doc = "Compiler options for javac",
providers = [JavacOptions],
Expand Down Expand Up @@ -237,6 +247,7 @@ def define_kt_toolchain(
jvm_target = None,
experimental_use_abi_jars = False,
experimental_strict_kotlin_deps = None,
experimental_report_unused_deps = None,
javac_options = None,
kotlinc_options = None):
"""Define the Kotlin toolchain."""
Expand All @@ -262,6 +273,7 @@ def define_kt_toolchain(
"//conditions:default": experimental_use_abi_jars,
}),
experimental_strict_kotlin_deps = experimental_strict_kotlin_deps,
experimental_report_unused_deps = experimental_report_unused_deps,
javac_options = javac_options,
kotlinc_options = kotlinc_options,
visibility = ["//visibility:public"],
Expand Down
57 changes: 50 additions & 7 deletions src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/JdepsMerger.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ import io.bazel.kotlin.builder.tasks.CommandLineProgram
import io.bazel.kotlin.builder.utils.ArgMap
import io.bazel.kotlin.builder.utils.ArgMaps
import io.bazel.kotlin.builder.utils.Flag
import java.io.BufferedInputStream
import java.io.BufferedOutputStream
import java.io.File
import io.bazel.kotlin.builder.utils.jars.JarHelper.Companion.INJECTING_RULE_KIND
import io.bazel.kotlin.builder.utils.jars.JarHelper.Companion.TARGET_LABEL
import java.io.*
import java.nio.charset.StandardCharsets
import java.nio.file.FileSystems
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.util.jar.JarFile

/**
* Persistent worker capable command line program for merging multiple Jdeps files into a single
Expand All @@ -31,12 +32,30 @@ class JdepsMerger: CommandLineProgram {
INPUTS("--inputs"),
OUTPUT("--output"),
TARGET_LABEL("--target_label"),
REPORT_UNUSED_DEPS("--report_unused_deps"),
}

private fun readJarOwnerFromManifest(jarPath: Path): JarOwner {
try {
JarFile(jarPath.toFile()).use { jarFile ->
val manifest = jarFile.manifest ?: return JarOwner(jarPath)
val attributes = manifest.mainAttributes
val label = attributes[TARGET_LABEL] as String?
?: return JarOwner(jarPath)
val injectingRuleKind = attributes[INJECTING_RULE_KIND] as String?
return JarOwner(jarPath, label, injectingRuleKind)
}
} catch (e: IOException) {
// This jar file pretty much has to exist.
throw UncheckedIOException(e)
}
}

fun merge(
label: String,
inputs: List<String>,
output: String) {
output: String,
reportUnusedDeps: String): Int {

val rootBuilder = Deps.Dependencies.newBuilder()
rootBuilder.success = false
Expand Down Expand Up @@ -64,9 +83,33 @@ class JdepsMerger: CommandLineProgram {
BufferedOutputStream(File(output).outputStream()).use {
it.write(rootBuilder.build().toByteArray())
}

if (reportUnusedDeps != "off") {
val unusedLabels = dependencyMap.values
.filter { it.kind == Deps.Dependency.Kind.UNUSED }
.mapNotNull { readJarOwnerFromManifest(Paths.get(it.path)).label }
.filter {it != label}

if (unusedLabels.isNotEmpty()) {
val open = "\u001b[35m\u001b[1m"
val close = "\u001b[0m"
val command =
"""
|$open ** Please remove the following dependencies:$close ${unusedLabels.joinToString(" ")} from $label
|$open ** You can use the following buildozer command:$close buildozer 'remove deps ${unusedLabels.joinToString(" ")}' $label
""".trimMargin()

println(command)
return if(reportUnusedDeps == "error") 1 else 0
}
}

return 0
}
}

private data class JarOwner(val jar: Path, val label: String? = null, val aspect: String? = null)

private fun getArgs(args: List<String>): ArgMap {
check(args.isNotEmpty()) { "expected at least a single arg got: ${args.joinToString(" ")}" }
val lines = FLAGFILE_RE.matchEntire(args[0])?.groups?.get(1)?.let {
Expand All @@ -77,12 +120,12 @@ class JdepsMerger: CommandLineProgram {
}

override fun apply(workingDir: Path, args: List<String>): Int {
var status = 0
val argMap = getArgs(args)
val inputs = argMap.mandatory(JdepsMergerFlags.INPUTS)
val output = argMap.mandatorySingle(JdepsMergerFlags.OUTPUT)
val label = argMap.mandatorySingle(JdepsMergerFlags.TARGET_LABEL)
merge(label, inputs, output)
return status
val reportUnusedDeps = argMap.mandatorySingle(JdepsMergerFlags.REPORT_UNUSED_DEPS)

return merge(label, inputs, output, reportUnusedDeps)
}
}
3 changes: 3 additions & 0 deletions src/test/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ kt_rules_test(
kt_rules_test(
name = "JdepsMergerTest",
srcs = ["jvm/JdepsMergerTest.kt"],
deps = [
"//src/main/kotlin/io/bazel/kotlin/builder/utils/jars",
],
)

kt_jvm_test(
Expand Down
Loading

0 comments on commit 97beff8

Please sign in to comment.