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

Strict dependency checking and enforcement. #438

Merged
merged 3 commits into from
Jan 8, 2021
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
1 change: 1 addition & 0 deletions examples/android/bzl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ define_kt_toolchain(
name = "experimental_toolchain",
api_version = "1.4",
experimental_use_abi_jars = True,
experimental_strict_kotlin_deps = "warn",
javac_options = ":default_javac_options",
kotlinc_options = ":default_kotlinc_options",
language_version = "1.4",
Expand Down
1 change: 1 addition & 0 deletions kotlin/internal/js/impl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def kt_js_library_impl(ctx):
args.add("--kotlin_js_dir", out_dir.path)
args.add("--kotlin_output_js_jar", ctx.outputs.jar)
args.add("--kotlin_output_srcjar", ctx.outputs.srcjar)
args.add("--strict_kotlin_deps", "off")

args.add_all("--kotlin_js_libraries", libraries, omit_if_empty = False)
args.add_all("--sources", ctx.files.srcs)
Expand Down
1 change: 1 addition & 0 deletions kotlin/internal/jvm/compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ def _run_kt_builder_action(
# TODO: Implement Strict Kotlin deps: (https://github.com/bazelbuild/rules_kotlin/issues/419)
# This flag is currently unused by the builder but required for the unused_deps tool
args.add_all("--direct_dependencies", _java_infos_to_compile_jars(compile_deps.deps))
args.add("--strict_kotlin_deps", toolchains.kt.experimental_strict_kotlin_deps)
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)
Expand Down
12 changes: 12 additions & 0 deletions kotlin/internal/toolchains.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def _kotlin_toolchain_impl(ctx):
jvm_stdlibs = java_common.merge(compile_time_providers + runtime_providers),
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,
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 @@ -187,6 +188,15 @@ _kt_toolchain = rule(
`kt_abi_plugin_incompatible`""",
default = False,
),
"experimental_strict_kotlin_deps": attr.string(
doc = "Report strict deps violations",
default = "off",
values = [
"off",
"warn",
"error",
],
),
"javac_options": attr.label(
doc = "Compiler options for javac",
providers = [JavacOptions],
Expand Down Expand Up @@ -224,6 +234,7 @@ def define_kt_toolchain(
api_version = None,
jvm_target = None,
experimental_use_abi_jars = False,
experimental_strict_kotlin_deps = None,
javac_options = None,
kotlinc_options = None):
"""Define the Kotlin toolchain."""
Expand All @@ -248,6 +259,7 @@ def define_kt_toolchain(
absolute_target("//kotlin/internal:noexperimental_use_abi_jars"): False,
"//conditions:default": experimental_use_abi_jars,
}),
experimental_strict_kotlin_deps = experimental_strict_kotlin_deps,
javac_options = javac_options,
kotlinc_options = kotlinc_options,
visibility = ["//visibility:public"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ kt_bootstrap_library(
deps = [
"//src/main/kotlin/io/bazel/kotlin/builder/toolchain",
"//src/main/kotlin/io/bazel/kotlin/builder/utils",
"//src/main/kotlin/io/bazel/kotlin/builder/utils/jars",
"//src/main/protobuf:deps_java_proto",
"//src/main/protobuf:kotlin_model_java_proto",
"//src/main/protobuf:worker_protocol_java_proto",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ class KotlinBuilder @Inject internal constructor(
GENERATED_JAVA_SRC_JAR("--generated_java_srcjar"),
GENERATED_JAVA_STUB_JAR("--kapt_generated_stub_jar"),
GENERATED_CLASS_JAR("--kapt_generated_class_jar"),
BUILD_KOTLIN("--build_kotlin");
BUILD_KOTLIN("--build_kotlin"),
STRICT_KOTLIN_DEPS("--strict_kotlin_deps"),
}
}

Expand Down Expand Up @@ -180,6 +181,7 @@ class KotlinBuilder @Inject internal constructor(
argMap.mandatorySingle(KotlinBuilderFlags.API_VERSION)
toolchainInfoBuilder.commonBuilder.languageVersion =
argMap.mandatorySingle(KotlinBuilderFlags.LANGUAGE_VERSION)
strictKotlinDeps = argMap.mandatorySingle(KotlinBuilderFlags.STRICT_KOTLIN_DEPS)
this
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class KotlinJvmTaskExecutor @Inject internal constructor(
inputs.directDependenciesList.forEach {
flag("direct_dependencies", it)
}
flag("strict_kotlin_deps", info.strictKotlinDeps)
}
.given(outputs.jar).notEmpty {
append(codeGenArgs())
Expand Down
27 changes: 27 additions & 0 deletions src/main/kotlin/io/bazel/kotlin/builder/utils/jars/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Copyright 2020 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# 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("@rules_java//java:defs.bzl", "java_library")
load("//src/main/kotlin:bootstrap.bzl", "kt_bootstrap_library")

kt_bootstrap_library(
name = "jars",
srcs = glob([
"*.kt",
"**/*.kt",
]),
visibility = ["//src:__subpackages__"],
deps = [
"//src/main/protobuf:deps_java_proto",
],
)
28 changes: 28 additions & 0 deletions src/main/kotlin/io/bazel/kotlin/builder/utils/jars/JarOwner.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package io.bazel.kotlin.builder.utils.jars

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.IOException
import java.io.UncheckedIOException
import java.nio.file.Path
import java.util.jar.JarFile

data class JarOwner(val jar: Path, val label: String? = null, val aspect: String? = null) {
companion object {
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)
}
}
}
}
1 change: 1 addition & 0 deletions src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ kt_bootstrap_library(
srcs = glob(["*.kt"]),
visibility = ["//src:__subpackages__"],
deps = [
"//src/main/kotlin/io/bazel/kotlin/builder/utils/jars",
"//src/main/protobuf:deps_java_proto",
"@com_github_jetbrains_kotlin//:kotlin-compiler",
"@kotlin_rules_maven//:com_google_protobuf_protobuf_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,21 @@ class JdepsGenCommandLineProcessor : CommandLineProcessor {
CliOption("target_label", "<String>", "Label of target being analyzed", required = true)
val DIRECT_DEPENDENCIES_OPTION: CliOption =
CliOption("direct_dependencies", "<List>", "List of targets direct dependencies", required = false, allowMultipleOccurrences = true)
val STRICT_KOTLIN_DEPS_OPTION: CliOption =
CliOption("strict_kotlin_deps", "<String>", "Report strict deps violations", required = true)
}

override val pluginId: String
get() = COMPILER_PLUGIN_ID
override val pluginOptions: Collection<AbstractCliOption>
get() = listOf(OUTPUT_JDEPS_FILE_OPTION, TARGET_LABEL_OPTION, DIRECT_DEPENDENCIES_OPTION)
get() = listOf(OUTPUT_JDEPS_FILE_OPTION, TARGET_LABEL_OPTION, DIRECT_DEPENDENCIES_OPTION, STRICT_KOTLIN_DEPS_OPTION)

override fun processOption(option: AbstractCliOption, value: String, configuration: CompilerConfiguration) {
when (option) {
OUTPUT_JDEPS_FILE_OPTION -> configuration.put(JdepsGenConfigurationKeys.OUTPUT_JDEPS, value)
TARGET_LABEL_OPTION -> configuration.put(JdepsGenConfigurationKeys.TARGET_LABEL, value)
DIRECT_DEPENDENCIES_OPTION -> configuration.appendList(JdepsGenConfigurationKeys.DIRECT_DEPENDENCIES, value)
STRICT_KOTLIN_DEPS_OPTION -> configuration.put(JdepsGenConfigurationKeys.STRICT_KOTLIN_DEPS, value)
else -> throw CliOptionProcessingException("Unknown option: ${option.optionName}")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ object JdepsGenConfigurationKeys {
val TARGET_LABEL: CompilerConfigurationKey<String> =
CompilerConfigurationKey.create(JdepsGenCommandLineProcessor.TARGET_LABEL_OPTION.description)

/**
* Label of the Bazel target being analyzed.
*/
val STRICT_KOTLIN_DEPS: CompilerConfigurationKey<String> =
CompilerConfigurationKey.create(JdepsGenCommandLineProcessor.STRICT_KOTLIN_DEPS_OPTION.description)

/**
* List of direct dependencies of the target.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ import com.google.devtools.build.lib.view.proto.Deps
import com.intellij.mock.MockProject
import com.intellij.openapi.project.Project
import com.intellij.psi.PsiElement
import io.bazel.kotlin.builder.utils.jars.JarOwner
import org.jetbrains.kotlin.analyzer.AnalysisResult
import org.jetbrains.kotlin.codegen.ClassBuilder
import org.jetbrains.kotlin.codegen.ClassBuilderFactory
import org.jetbrains.kotlin.codegen.extensions.ClassBuilderInterceptorExtension
import org.jetbrains.kotlin.codegen.inline.RemappingClassBuilder
import org.jetbrains.kotlin.compiler.plugin.ComponentRegistrar
import org.jetbrains.kotlin.config.CompilerConfiguration
import org.jetbrains.kotlin.config.JVMConfigurationKeys
import org.jetbrains.kotlin.descriptors.ClassDescriptor
import org.jetbrains.kotlin.descriptors.ModuleDescriptor
import org.jetbrains.kotlin.descriptors.SourceElement
Expand All @@ -22,11 +21,9 @@ import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryJavaClass
import org.jetbrains.kotlin.load.kotlin.KotlinJvmBinarySourceElement
import org.jetbrains.kotlin.load.kotlin.VirtualFileKotlinClass
import org.jetbrains.kotlin.name.ClassId
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.BindingTrace
import org.jetbrains.kotlin.resolve.descriptorUtil.classId
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
import org.jetbrains.kotlin.resolve.descriptorUtil.getAllSuperclassesWithoutAny
import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperInterfaces
Expand All @@ -35,6 +32,7 @@ import org.jetbrains.kotlin.resolve.jvm.extensions.AnalysisHandlerExtension
import org.jetbrains.org.objectweb.asm.commons.Remapper
import java.io.BufferedOutputStream
import java.io.File
import java.nio.file.Paths

/**
* Kotlin compiler extension that tracks classes (and corresponding classpath jars) needed to
Expand Down Expand Up @@ -192,6 +190,36 @@ class JdepsGenExtension(
BufferedOutputStream(File(jdepsOutput).outputStream()).use {
it.write(rootBuilder.build().toByteArray())
}

when (configuration.getNotNull(JdepsGenConfigurationKeys.STRICT_KOTLIN_DEPS)) {
"warn" -> checkStrictDeps(result, directDeps, targetLabel)
"error" -> {
if(checkStrictDeps(result, directDeps, targetLabel)) error("Strict Deps Violations - please fix")
}
}
}

/**
* Prints strict deps warnings and returns true if violations were found.
*/
private fun checkStrictDeps(result: HashMap<String, ArrayList<String>>, directDeps: List<String>, targetLabel: String): Boolean {
val missingStrictDeps = result.keys
.filter { !directDeps.contains(it) }
.map { JarOwner.readJarOwnerFromManifest(Paths.get(it)).label }

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

println(command)
return true
}
return false
}

private fun isExternalModuleClass(className: ClassId): Boolean {
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;
// Enable strict dependency checking for Kotlin
string strict_kotlin_deps = 10;
}

// Mested messages not marked with stable could be refactored.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,12 @@ public void addDirectDependencies(Dep... dependencies) {
});
}

public void addTransitiveDependencies(Dep... dependencies) {
Dep.classpathOf(dependencies).forEach(dependency -> {
taskBuilder.getInputsBuilder().addClasspath(dependency);
});
}

public TaskBuilder outputSrcJar() {
taskBuilder.getOutputsBuilder()
.setSrcjar(instanceRoot().resolve("jar_file-sources.jar").toAbsolutePath().toString());
Expand All @@ -201,6 +207,11 @@ public TaskBuilder outputJdeps() {
return this;
}

public TaskBuilder kotlinStrictDeps(String level) {
taskBuilder.getInfoBuilder().setStrictKotlinDeps(level);
return this;
}

public TaskBuilder outputJavaJdeps() {
taskBuilder.getOutputsBuilder()
.setJavaJdeps(instanceRoot().resolve("java_jdeps_file.jdeps").toAbsolutePath().toString());
Expand Down
6 changes: 6 additions & 0 deletions src/test/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ kt_rules_test(
srcs = ["jvm/KotlinBuilderJvmJdepsTest.kt"],
)

kt_rules_test(
name = "KotlinBuilderJvmStrictDepsTest",
size = "large",
srcs = ["jvm/KotlinBuilderJvmStrictDepsTest.kt"],
)

kt_rules_test(
name = "JdepsMergerTest",
srcs = ["jvm/JdepsMergerTest.kt"],
Expand Down
Loading