From 024763fbf1992980bb63fcda1b1a70c1b6a51f37 Mon Sep 17 00:00:00 2001 From: Corbin McNeely-Smith <58151731+restingbull@users.noreply.github.com> Date: Sat, 15 Apr 2023 23:07:54 -0500 Subject: [PATCH] Switch to bazel runfiles * Fix kt_jvm_import to include the deps in the runfiles * Move all includes to use properties in the runfiles --- kotlin/internal/jvm/impl.bzl | 2 +- src/main/kotlin/BUILD.release.bazel | 9 +- .../io/bazel/kotlin/builder/cmd/BUILD.bazel | 7 +- .../builder/toolchain/KotlinToolchain.kt | 102 ++++++++---------- .../io/bazel/kotlin/builder/utils/BUILD.bazel | 1 + .../kotlin/builder/utils/BazelRunFiles.kt | 74 ++----------- .../BUILD.com_github_jetbrains_kotlin.bazel | 45 +++++--- src/test/kotlin/io/bazel/kotlin/defs.bzl | 10 ++ 8 files changed, 107 insertions(+), 143 deletions(-) diff --git a/kotlin/internal/jvm/impl.bzl b/kotlin/internal/jvm/impl.bzl index 8d8977b49..1f4640a7b 100644 --- a/kotlin/internal/jvm/impl.bzl +++ b/kotlin/internal/jvm/impl.bzl @@ -188,7 +188,7 @@ def kt_jvm_import_impl(ctx): runfiles = ctx.runfiles( # Append class jar with the optional sources jar files = [artifact.class_jar] + [artifact.source_jar] if artifact.source_jar else [], - ), + ).merge_all([d[DefaultInfo].default_runfiles for d in ctx.attr.deps]), ), JavaInfo( output_jar = artifact.class_jar, diff --git a/src/main/kotlin/BUILD.release.bazel b/src/main/kotlin/BUILD.release.bazel index 2bad2944c..36653d659 100644 --- a/src/main/kotlin/BUILD.release.bazel +++ b/src/main/kotlin/BUILD.release.bazel @@ -41,11 +41,16 @@ java_binary( ":skip-code-gen", "//src/main/kotlin/io/bazel/kotlin/compiler", "@com_github_jetbrains_kotlin//:jvm-abi-gen", + "@com_github_jetbrains_kotlin//:kotlin-annotation-processing", "@com_github_jetbrains_kotlin//:kotlin-compiler", ], jvm_flags = [ - "-D@com_github_jetbrains_kotlin...jvm-abi-gen=$(rootpath @com_github_jetbrains_kotlin//:jvm-abi-gen)", - "-D@com_github_jetbrains_kotlin...kotlin-compiler=$(rootpath @com_github_jetbrains_kotlin//:kotlin-compiler)", + "-D@com_github_jetbrains_kotlin...jvm-abi-gen=$(rlocationpath @com_github_jetbrains_kotlin//:jvm-abi-gen)", + "-D@com_github_jetbrains_kotlin...kotlin-compiler=$(rlocationpath @com_github_jetbrains_kotlin//:kotlin-compiler)", + "-D@com_github_jetbrains_kotlin...kapt=$(rlocationpath @com_github_jetbrains_kotlin//:kotlin-annotation-processing)", + "-D@rules_kotlin...jdeps-gen=$(rlocationpath //src/main/kotlin:jdeps-gen)", + "-D@rules_kotlin...skip-code-gen=$(rlocationpath //src/main/kotlin:skip-code-gen)", + "-D@rules_kotlin...compiler=$(rlocationpath //src/main/kotlin/io/bazel/kotlin/compiler)", "-XX:-MaxFDLimit", ], main_class = "io.bazel.kotlin.builder.cmd.Build", diff --git a/src/main/kotlin/io/bazel/kotlin/builder/cmd/BUILD.bazel b/src/main/kotlin/io/bazel/kotlin/builder/cmd/BUILD.bazel index b50e1c9b9..2d1a1f1f2 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/cmd/BUILD.bazel +++ b/src/main/kotlin/io/bazel/kotlin/builder/cmd/BUILD.bazel @@ -22,8 +22,11 @@ kt_bootstrap_binary( "@com_github_jetbrains_kotlin//:kotlin-compiler", ], jvm_flags = [ - "-D@com_github_jetbrains_kotlin...jvm-abi-gen=$(rootpath @com_github_jetbrains_kotlin//:jvm-abi-gen)", - "-D@com_github_jetbrains_kotlin...kotlin-compiler=$(rootpath @com_github_jetbrains_kotlin//:kotlin-compiler)", + "-D@com_github_jetbrains_kotlin...jvm-abi-gen=$(rlocationpath @com_github_jetbrains_kotlin//:jvm-abi-gen)", + "-D@com_github_jetbrains_kotlin...kotlin-compiler=$(rlocationpath @com_github_jetbrains_kotlin//:kotlin-compiler)", + "-D@rules_kotlin...jdeps-gen=$(rlocationpath //src/main/kotlin:jdeps-gen)", + "-D@rules_kotlin...skip-code-gen=$(rlocationpath //src/main/kotlin:skip-code-gen)", + "-D@rules_kotlin...compiler=$(rlocationpath //src/main/kotlin/io/bazel/kotlin/compiler)", ], main_class = "io.bazel.kotlin.builder.cmd.Build", runtime_library = ":build_lib", diff --git a/src/main/kotlin/io/bazel/kotlin/builder/toolchain/KotlinToolchain.kt b/src/main/kotlin/io/bazel/kotlin/builder/toolchain/KotlinToolchain.kt index c83c02007..f14a6faec 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/toolchain/KotlinToolchain.kt +++ b/src/main/kotlin/io/bazel/kotlin/builder/toolchain/KotlinToolchain.kt @@ -32,74 +32,62 @@ import javax.inject.Inject import javax.inject.Singleton class KotlinToolchain private constructor( - val kotlinHome: Path, val classLoader: ClassLoader, - val kapt3Plugin: CompilerPlugin = CompilerPlugin( - kotlinHome.resolveVerified("lib", "kotlin-annotation-processing.jar").absolutePath, - "org.jetbrains.kotlin.kapt3", - ), + val kapt3Plugin: CompilerPlugin, val jvmAbiGen: CompilerPlugin, val skipCodeGen: CompilerPlugin, val jdepsGen: CompilerPlugin, ) { companion object { - // TODO(issue/432): Remove this gross hack and pass the file locations on the command line. - private var RULES_REPOSITORY_NAME = - System.getenv("TEST_WORKSPACE")?.takeIf { it.isNotBlank() } - ?: System.getenv("REPOSITORY_NAME")?.takeIf { it.isNotBlank() } - // ?: System.getProperty("TEST_WORKSPACE")?.takeIf { it.isNotBlank() } - ?: error( - "Unable to determine rules_kotlin repository " + - "name.\nenv:${System.getenv()}\nproperties:${System.getProperties()}", - ) + private val JVM_ABI_PLUGIN = BazelRunFiles.resolveVerifiedFromProperty( + "@com_github_jetbrains_kotlin...jvm-abi-gen", + ).toPath() - private val DEFAULT_JVM_ABI_PATH = BazelRunFiles.resolveVerified( - System.getProperty("@com_github_jetbrains_kotlin...jvm-abi-gen"), + private val KAPT_PLUGIN = BazelRunFiles.resolveVerifiedFromProperty( + "@com_github_jetbrains_kotlin...kapt", ).toPath() - private val COMPILER = BazelRunFiles.resolveVerified( - RULES_REPOSITORY_NAME, - "src", "main", "kotlin", "io", "bazel", "kotlin", "compiler", - "compiler.jar", + private val COMPILER = BazelRunFiles.resolveVerifiedFromProperty( + "@rules_kotlin...compiler", ).toPath() - private val SKIP_CODE_GEN_PLUGIN = BazelRunFiles.resolveVerified( - RULES_REPOSITORY_NAME, - "src", - "main", - "kotlin", - "skip-code-gen.jar", + private val SKIP_CODE_GEN_PLUGIN = BazelRunFiles.resolveVerifiedFromProperty( + "@rules_kotlin...skip-code-gen", ).toPath() - private val JDEPS_GEN_PLUGIN = BazelRunFiles.resolveVerified( - RULES_REPOSITORY_NAME, - "src", - "main", - "kotlin", - "jdeps-gen.jar", + private val JDEPS_GEN_PLUGIN = BazelRunFiles.resolveVerifiedFromProperty( + "@rules_kotlin...jdeps-gen", ).toPath() + private val KOTLINC = BazelRunFiles.resolveVerifiedFromProperty( + "@com_github_jetbrains_kotlin...kotlin-compiler", + ) + internal val NO_ARGS = arrayOf() private val isJdk9OrNewer = !System.getProperty("java.version").startsWith("1.") private fun createClassLoader(javaHome: Path, baseJars: List): ClassLoader = - ClassPreloadingUtils.preloadClasses( - mutableListOf().also { - it += baseJars - if (!isJdk9OrNewer) { - it += javaHome.resolveVerified("lib", "tools.jar") - } - }, - Preloader.DEFAULT_CLASS_NUMBER_ESTIMATE, - ClassLoader.getSystemClassLoader(), - null, - ) + runCatching { + ClassPreloadingUtils.preloadClasses( + mutableListOf().also { + it += baseJars + if (!isJdk9OrNewer) { + it += javaHome.resolveVerified("lib", "tools.jar") + } + }, + Preloader.DEFAULT_CLASS_NUMBER_ESTIMATE, + ClassLoader.getSystemClassLoader(), + null, + ) + }.onFailure { + throw RuntimeException("$javaHome, $baseJars", it) + }.getOrThrow() @JvmStatic fun createToolchain(): KotlinToolchain { - return createToolchain(DEFAULT_JVM_ABI_PATH) + return createToolchain(JVM_ABI_PLUGIN) } @JvmStatic @@ -109,41 +97,39 @@ class KotlinToolchain private constructor( path.takeIf { !it.endsWith(Paths.get("jre")) } ?: path.parent }.verifiedPath() - val skipCodeGenFile = SKIP_CODE_GEN_PLUGIN.verified().absoluteFile - val jdepsGenFile = JDEPS_GEN_PLUGIN.verified().absoluteFile - - val kotlinCompilerJar = BazelRunFiles.resolveVerified( - System.getProperty("@com_github_jetbrains_kotlin...kotlin-compiler"), - ) - + val skipCodeGenFile = SKIP_CODE_GEN_PLUGIN.verified() + val jdepsGenFile = JDEPS_GEN_PLUGIN.verified() val jvmAbiGenFile = jvmAbiGenPath.verified() return KotlinToolchain( - kotlinCompilerJar.toPath().parent.parent, createClassLoader( javaHome, listOf( - kotlinCompilerJar, - COMPILER.verified().absoluteFile, + KOTLINC, + COMPILER.verified(), // plugins *must* be preloaded. Not doing so causes class conflicts // (and a NoClassDef err) in the compiler extension interfaces. // This may cause issues in accepting user defined compiler plugins. - jvmAbiGenFile.absoluteFile, + jvmAbiGenFile, skipCodeGenFile, jdepsGenFile, ), ), jvmAbiGen = CompilerPlugin( - jvmAbiGenFile.absolutePath, + jvmAbiGenFile.path, "org.jetbrains.kotlin.jvm.abi", ), skipCodeGen = CompilerPlugin( - skipCodeGenFile.absolutePath, + skipCodeGenFile.path, "io.bazel.kotlin.plugin.SkipCodeGen", ), jdepsGen = CompilerPlugin( - jdepsGenFile.absolutePath, + jdepsGenFile.path, "io.bazel.kotlin.plugin.jdeps.JDepsGen", ), + kapt3Plugin = CompilerPlugin( + KAPT_PLUGIN.verified().path, + "org.jetbrains.kotlin.kapt3", + ), ) } } diff --git a/src/main/kotlin/io/bazel/kotlin/builder/utils/BUILD.bazel b/src/main/kotlin/io/bazel/kotlin/builder/utils/BUILD.bazel index 90be5e793..1f5982c55 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/utils/BUILD.bazel +++ b/src/main/kotlin/io/bazel/kotlin/builder/utils/BUILD.bazel @@ -11,6 +11,7 @@ kt_bootstrap_library( "//src/main/protobuf:deps_java_proto", "//src/main/protobuf:kotlin_model_java_proto", "//src/main/protobuf:worker_protocol_java_proto", + "@bazel_tools//tools/java/runfiles", "@kotlin_rules_maven//:com_google_protobuf_protobuf_java", "@kotlin_rules_maven//:com_google_protobuf_protobuf_java_util", ], diff --git a/src/main/kotlin/io/bazel/kotlin/builder/utils/BazelRunFiles.kt b/src/main/kotlin/io/bazel/kotlin/builder/utils/BazelRunFiles.kt index 108ed6e6e..45573e236 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/utils/BazelRunFiles.kt +++ b/src/main/kotlin/io/bazel/kotlin/builder/utils/BazelRunFiles.kt @@ -15,82 +15,26 @@ */ package io.bazel.kotlin.builder.utils +import com.google.devtools.build.runfiles.Runfiles import java.io.File -import java.io.FileInputStream -import java.nio.charset.Charset -import java.nio.file.Paths -import java.util.Collections +import java.io.FileNotFoundException /** Utility class for getting runfiles on windows and *nix. */ object BazelRunFiles { - private val isWindows = System.getProperty("os.name").lowercase().indexOf("win") >= 0 - - /** - * Set depending on --enable_runfiles, which defaults to false on Windows and true otherwise. - */ - private val manifestFile: String? = - if (isWindows) { - System.getenv("RUNFILES_MANIFEST_FILE") - } else { - null - } - - private val javaRunFiles = Paths.get(System.getenv("JAVA_RUNFILES")) private val runfiles by lazy { - with(mutableMapOf()) { - FileInputStream(manifestFile) - .bufferedReader(Charset.forName("UTF-8")) - .lines() - .forEach { it -> - val line = it.trim { it <= ' ' } - if (!line.isEmpty()) { - // TODO(bazel-team): This is buggy when the path contains spaces, we should fix the manifest format. - line.split(" ").also { - check(it.size == 2) { "RunFiles manifest entry $line contains more than one space" } - put(it[0], it[1]) - } - } - } - Collections.unmodifiableMap(this) - } + Runfiles.preload().unmapped() } /** * Resolve a run file on windows or *nix. */ @JvmStatic - fun resolveVerified(vararg path: String): File { - check(path.isNotEmpty()) - val fromManifest = manifestFile?.let { mf -> - path.joinToString("/").let { rfPath -> - // trim off the external/ prefix if it is present. - val trimmedPath = - if (rfPath.startsWith("external/")) { - rfPath.replaceFirst("external/", "") - } else { - rfPath - } - File( - checkNotNull(runfiles[trimmedPath]) { - "runfile manifest $mf did not contain path mapping for $rfPath" - }, - ) - }.also { - check(it.exists()) { "file $it resolved from runfiles did not exist" } + fun resolveVerifiedFromProperty(key: String) = + System.getProperty(key) + ?.let { path -> runfiles.rlocation(path) } + ?.let { File(it) } + ?: let { + throw FileNotFoundException("no reference for $key in ${System.getProperties()}") } - } - if (fromManifest != null) { - return fromManifest - } - - // / if it could not be resolved from the manifest then first try to resolve it directly. - val resolvedDirect = File(path.joinToString(File.separator)).takeIf { it.exists() } - if (resolvedDirect != null) { - return resolvedDirect - } - - // Try the java runfiles as the last resort. - return javaRunFiles.resolveVerified(path.joinToString(File.separator)) - } } diff --git a/src/main/starlark/core/repositories/BUILD.com_github_jetbrains_kotlin.bazel b/src/main/starlark/core/repositories/BUILD.com_github_jetbrains_kotlin.bazel index 85dbde4cc..7efddcef7 100644 --- a/src/main/starlark/core/repositories/BUILD.com_github_jetbrains_kotlin.bazel +++ b/src/main/starlark/core/repositories/BUILD.com_github_jetbrains_kotlin.bazel @@ -21,13 +21,20 @@ package(default_visibility = ["//visibility:public"]) # Kotlin home filegroup containing everything that is needed. filegroup( name = "home", - srcs = glob(["**"]), + srcs = glob([ + "**", + "**/*.jar", + ]), ) kt_jvm_import( name = "annotations", jar = "lib/annotations-13.0.jar", - neverlink = 1, +) + +kt_jvm_import( + name = "trove4j", + jar = "lib/trove4j.jar", ) kt_jvm_import( @@ -35,19 +42,27 @@ kt_jvm_import( jar = "lib/jvm-abi-gen.jar", ) -# Kotlin dependencies that are internal to this repo and are meant to be loaded manually into a classloader. -[ - kt_jvm_import( - name = "kotlin-%s" % art, - jar = "lib/kotlin-%s.jar" % art, - neverlink = 1, - ) - for art in [ - "annotation-processing", - "annotation-processing-runtime", - "compiler", - ] -] +kt_jvm_import( + name = "kotlin-compiler", + jar = "lib/kotlin-compiler.jar", + deps = [ + ":annotations", + ":kotlin-reflect", + ":kotlin-script-runtime", + ":kotlin-stdlib", + ":trove4j", + ], +) + +kt_jvm_import( + name = "kotlin-annotation-processing-runtime", + jar = "lib/kotlin-annotation-processing-runtime.jar", +) + +kt_jvm_import( + name = "kotlin-annotation-processing", + jar = "lib/kotlin-annotation-processing.jar", +) kt_jvm_import( name = "kotlinx-serialization-compiler-plugin", diff --git a/src/test/kotlin/io/bazel/kotlin/defs.bzl b/src/test/kotlin/io/bazel/kotlin/defs.bzl index f608ed1af..a3436f225 100644 --- a/src/test/kotlin/io/bazel/kotlin/defs.bzl +++ b/src/test/kotlin/io/bazel/kotlin/defs.bzl @@ -28,6 +28,7 @@ def kt_rules_test(name, **kwargs): args.setdefault("size", "small") args.setdefault("data", []) args.setdefault("jvm_flags", []) + args["deps"] = args.setdefault("deps", []) + ["//src/test/kotlin/io/bazel/kotlin/builder:test_lib"] for dep in [ "//src/main/kotlin/io/bazel/kotlin/compiler", @@ -44,6 +45,15 @@ def kt_rules_test(name, **kwargs): args["data"].append(dep) args["jvm_flags"].append("-D%s=$(rootpath %s)" % (dep.replace("/", ".").replace(":", "."), dep)) + # Required by KotlinToolchain.kt to resolve the necessary paths. + args["jvm_flags"].extend([ + "-D@com_github_jetbrains_kotlin...jvm-abi-gen=$(rlocationpath @com_github_jetbrains_kotlin//:jvm-abi-gen)", + "-D@com_github_jetbrains_kotlin...kotlin-compiler=$(rlocationpath @com_github_jetbrains_kotlin//:kotlin-compiler)", + "-D@rules_kotlin...jdeps-gen=$(rlocationpath //src/main/kotlin:jdeps-gen)", + "-D@rules_kotlin...skip-code-gen=$(rlocationpath //src/main/kotlin:skip-code-gen)", + "-D@rules_kotlin...compiler=$(rlocationpath //src/main/kotlin/io/bazel/kotlin/compiler)", + ]) + args.setdefault("test_class", _get_class_name(kwargs)) for f in args.get("srcs"): if f.endswith(".kt"):