Switch to bazel runfiles

* Fix kt_jvm_import to include the deps in the runfiles * Move all includes to use properties in the runfiles
bazelbuild · Apr 16, 2023 · 024763f · 024763f
1 parent 0b75e94
commit 024763f
Show file tree

Hide file tree

Showing 8 changed files with 107 additions and 143 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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<Any>()
 
     private val isJdk9OrNewer = !System.getProperty("java.version").startsWith("1.")
 
     private fun createClassLoader(javaHome: Path, baseJars: List<File>): ClassLoader =
-      ClassPreloadingUtils.preloadClasses(
-        mutableListOf<File>().also {
-          it += baseJars
-          if (!isJdk9OrNewer) {
-            it += javaHome.resolveVerified("lib", "tools.jar")
-          }
-        },
-        Preloader.DEFAULT_CLASS_NUMBER_ESTIMATE,
-        ClassLoader.getSystemClassLoader(),
-        null,
-      )
+      runCatching {
+        ClassPreloadingUtils.preloadClasses(
+          mutableListOf<File>().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
@@ -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
@@ -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<String, String>()) {
-      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
@@ -21,33 +21,48 @@ 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(
     name = "jvm-abi-gen",
     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
@@ -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"):