Merge pull request #46 from lucidsoftware/handle-jvm-external-stampin…

…g-and-canonical-repo-names

Fixes a bug caused when using rule_jvm_external's jar stamping for the jars you create an AnnexScalaInstance with.

Fixes a bug that happens when canonical repository names get used for labels when bzlmod is enabled

Stops disabling bzlmod

Stops loading the rules_java toolchains as we supply our own

.bazelrc_shared

@@ -1,6 +1,5 @@
 common --announce_rc
 common --color=yes
-common --noenable_bzlmod
 # Fix "Failed to fetch blobs because they do not exist remotely" errors
 # https://github.com/bazelbuild/bazel/issues/18696#issuecomment-2175561503
 common --remote_download_all

MODULE.bazel

@@ -0,0 +1,6 @@
+###############################################################################
+# Bazel now uses Bzlmod by default to manage external dependencies.
+# Please consider migrating your external dependencies from WORKSPACE to MODULE.bazel.
+#
+# For more details, please check https://github.com/bazelbuild/bazel/issues/18958
+###############################################################################

WORKSPACE

@@ -90,8 +90,6 @@ load("@rules_java//java:repositories.bzl", "rules_java_dependencies", "rules_jav
 
 rules_java_dependencies()
 
-rules_java_toolchains()
-
 register_toolchains("//:repository_default_toolchain_21_definition")
 
 # rules_jvm_external

rules/private/phases/phase_zinc_depscheck.bzl

@@ -22,14 +22,20 @@ def phase_zinc_depscheck(ctx, g):
     deps_configuration = ctx.attr.scala[_DepsConfiguration]
 
     deps_checks = {}
-    labeled_jars = depset(transitive = [dep[_LabeledJars].values for dep in ctx.attr.deps])
+    labeled_jar_groups = depset(transitive = [dep[_LabeledJars].values for dep in ctx.attr.deps])
+
     worker_inputs, _, worker_input_manifests = ctx.resolve_command(tools = [deps_configuration.worker])
     for name in ("direct", "used"):
         deps_check = ctx.actions.declare_file("{}/depscheck_{}.success".format(ctx.label.name, name))
         deps_args = ctx.actions.args()
         deps_args.add(name, format = "--check_%s=true")
         deps_args.add_all("--direct", [dep.label for dep in ctx.attr.deps], format_each = "_%s")
-        deps_args.add_all(labeled_jars, map_each = _depscheck_labeled_group)
+
+        # Check the comment on the function we're calling here to understand why
+        # we're not using map_each
+        for labeled_jar_group in labeled_jar_groups.to_list():
+            _add_args_for_depscheck_labeled_group(labeled_jar_group, deps_args)
+
         deps_args.add("--label", ctx.label, format = "_%s")
         deps_args.add_all("--used_whitelist", [dep.label for dep in ctx.attr.deps_used_whitelist], format_each = "_%s")
         deps_args.add_all("--unused_whitelist", [dep.label for dep in ctx.attr.deps_unused_whitelist], format_each = "_%s")
@@ -63,5 +69,12 @@ def phase_zinc_depscheck(ctx, g):
         toolchain = deps_configuration,
     )
 
-def _depscheck_labeled_group(labeled_jars):
-    return (["--group", "_{}".format(labeled_jars.label)] + [jar.path for jar in labeled_jars.jars.to_list()])
+# If you use avoid using map_each, then labels are converted to their apparent repo name rather than
+# their canonical repo name. The apparent repo name is the human readable one that we want for use
+# with buildozer. See https://bazel.build/rules/lib/builtins/Args.html for more info
+#
+# Avoiding map_each is why we've got this odd section of add and add_all to create a --group
+def _add_args_for_depscheck_labeled_group(labeled_jar_group, deps_args):
+    deps_args.add("--group")
+    deps_args.add(labeled_jar_group.label, format = "_%s")
+    deps_args.add_all(labeled_jar_group.jars)

src/main/scala/higherkindness/rules_scala/workers/common/AnnexScalaInstance.scala

@@ -97,16 +97,19 @@ object AnnexScalaInstance {
   // These are not the most robust checks for these jars, but it is more or
   // less what Zinc and Bloop are doing. They're also fast, so if it works it
   // works.
-  private final def isScala2CompilerJar(jarName: String): Boolean = {
+  private final def isScala2CompilerJar(jar: File): Boolean = {
+    val jarName = FileUtil.getNameWithoutRulesJvmExternalStampPrefix(jar)
     jarName.startsWith("scala-compiler") || jarName.startsWith("scala-reflect")
   }
 
-  private final def isScala3CompilerJar(jarName: String): Boolean = {
+  private final def isScala3CompilerJar(jar: File): Boolean = {
+    val jarName = FileUtil.getNameWithoutRulesJvmExternalStampPrefix(jar)
     jarName.startsWith("scala3-compiler") || jarName.startsWith("scala3-interfaces") ||
     jarName.startsWith("tasty-core_3") || jarName.startsWith("scala-asm")
   }
 
-  private final def isScalaLibraryJar(jarName: String): Boolean = {
+  private final def isScalaLibraryJar(jar: File): Boolean = {
+    val jarName = FileUtil.getNameWithoutRulesJvmExternalStampPrefix(jar)
     jarName.startsWith("scala-library") || jarName.startsWith("scala3-library")
   }
 }
@@ -125,25 +128,40 @@ private[common] class AnnexScalaInstance(override val allJars: Array[File]) exte
   // We need to include the full classpath for the Scala 2 or Scala 3 compilers.
   // Thankfully that classpath doesn't seem to change very often.
   override val compilerJars: Array[File] = allJars.filter { jar =>
-    val jarName = jar.getName
-    AnnexScalaInstance.isScala2CompilerJar(jarName) ||
-    AnnexScalaInstance.isScala3CompilerJar(jarName) ||
-    AnnexScalaInstance.isScalaLibraryJar(jarName)
+    AnnexScalaInstance.isScala2CompilerJar(jar) ||
+    AnnexScalaInstance.isScala3CompilerJar(jar) ||
+    AnnexScalaInstance.isScalaLibraryJar(jar)
   }
 
   // Jars for the Scala library classes
-  override val libraryJars: Array[File] =
-    allJars.filter(jar => AnnexScalaInstance.isScalaLibraryJar(jar.getName))
+  override val libraryJars: Array[File] = allJars.filter(AnnexScalaInstance.isScalaLibraryJar)
 
   // All the jars that are not compiler or library jars
   override val otherJars: Array[File] = allJars.diff(compilerJars ++ libraryJars)
 
+  // Loader for only the classes and resources in the library jars of this Scala instance
+  override val loaderLibraryOnly: ClassLoader = AnnexScalaInstance.getClassLoader(libraryJars)
+
+  // Loader for all the classes and resources in all the jars of this Scala instance
+  override val loader: ClassLoader = AnnexScalaInstance.getClassLoader(allJars)
+
+  // Loader for all the classes and resources in all the compiler jars of this
+  // Scala instance
+  override val loaderCompilerOnly: ClassLoader = AnnexScalaInstance.getClassLoader(compilerJars)
+
   // Version for this Scala instance
   override val actualVersion: String = {
-    val stream = AnnexScalaInstance
-      .getClassLoader(compilerJars)
+    val stream = loaderCompilerOnly
       .getResourceAsStream("compiler.properties")
 
+    if (stream == null) {
+      throw new Exception(
+        "The resource stream for the compiler.properties file in the compiler jar is null." +
+          " Something went wrong getting the version in that file in the compiler jar. The jars" +
+          s" which were searched are as follows: ${compilerJars.mkString}",
+      )
+    }
+
     try {
       val props = new Properties
       props.load(stream)
@@ -152,15 +170,6 @@ private[common] class AnnexScalaInstance(override val allJars: Array[File]) exte
   }
   override val version: String = actualVersion
 
-  // Loader for only the classes and resources in the library jars of this Scala instance
-  override val loaderLibraryOnly: ClassLoader = AnnexScalaInstance.getClassLoader(libraryJars)
-
-  // Loader for all the classes and resources in all the jars of this Scala instance
-  override val loader: ClassLoader = AnnexScalaInstance.getClassLoader(allJars)
-
-  // Loader for all the classes and resources in all the compiler jars of this
-  // Scala instance
-  override val loaderCompilerOnly: ClassLoader = AnnexScalaInstance.getClassLoader(compilerJars)
 }
 
 /**

src/main/scala/higherkindness/rules_scala/workers/common/FileUtil.scala

@@ -2,7 +2,7 @@ package higherkindness.rules_scala
 package workers.common
 
 import scala.annotation.tailrec
-import java.io.IOException
+import java.io.{File, IOException}
 import java.nio.channels.FileChannel
 import java.nio.file.{FileAlreadyExistsException, FileVisitResult, Files, Path, SimpleFileVisitor, StandardCopyOption, StandardOpenOption}
 import java.nio.file.attribute.BasicFileAttributes
@@ -45,6 +45,20 @@ class ZipFileVisitor(root: Path, zip: ZipOutputStream) extends SimpleFileVisitor
 
 object FileUtil {
 
+  /**
+   * Given a jar, return the file name without the rules_jvm_external prefix added to it when stamping.
+   *
+   * rules_jvm_external adds a prefix to jars when it stamps them. We sometimes need to look at jar names for various
+   * things. The rules_jvm_external prefix often breaks that string comparison.
+   */
+  def getNameWithoutRulesJvmExternalStampPrefix(file: File): String = {
+    file.getName.stripPrefix("header_").stripPrefix("processed_")
+  }
+
+  def getNameWithoutRulesJvmExternalStampPrefix(path: Path): String = {
+    getNameWithoutRulesJvmExternalStampPrefix(path.toFile)
+  }
+
   def copy(source: Path, target: Path) = Files.walkFileTree(source, new CopyFileVisitor(source, target))
 
   def delete(path: Path) = Files.walkFileTree(path, new DeleteFileVisitor)

src/main/scala/higherkindness/rules_scala/workers/deps/DepsRunner.scala

@@ -60,8 +60,21 @@ object DepsRunner extends WorkerMain[Unit] {
         case _             => throw new Exception(s"Unexpected case in DepsRunner")
       }
     val labelToPaths = groups.toMap
-    def pathsForLabel(depLabel: String) =
-      Seq(depLabel, s"@${depLabel}", depLabel.stripPrefix("@")).collect(labelToPaths).flatten
+    def pathsForLabel(depLabel: String): Seq[String] = {
+      // A label could have no @ prefix, a single @ prefix, or a double @@ prefix.
+      // In an ideal world, the label we see here would always match the label in
+      // the --group, but that's not always the case. So we need to be able to handle
+      // moving from any of the forms to any of the other forms.
+      val potentialLabels = if (depLabel.startsWith("@@")) {
+        Seq(depLabel.stripPrefix("@@"), depLabel.stripPrefix("@"), depLabel)
+      } else if (depLabel.startsWith("@")) {
+        Seq(depLabel.stripPrefix("@"), depLabel, s"@${depLabel}")
+      } else {
+        Seq(depLabel, s"@${depLabel}", s"@@${depLabel}")
+      }
+
+      potentialLabels.collect(labelToPaths).flatten
+    }
     val usedPaths = Files.readAllLines(namespace.get[File]("used").toPath).asScala.toSet
 
     val remove = if (namespace.getBoolean("check_used") == true) {