JLinkPlugin improvements (#1499)

fixes #1446

fix `jdk.unsupported`, sbt style and optimize allocations and additional tests added

src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkPlugin.scala

@@ -1,6 +1,7 @@
 package com.typesafe.sbt.packager.archetypes
 package jlink
 
+import scala.collection.immutable
 import scala.sys.process.{BasicIO, Process, ProcessBuilder}
 import sbt._
 import sbt.Keys._
@@ -30,29 +31,29 @@ import com.typesafe.sbt.packager.universal.UniversalPlugin
 object JlinkPlugin extends AutoPlugin {
 
   object autoImport extends JlinkKeys {
-    val JlinkIgnore = JlinkPlugin.Ignore
+    val JlinkIgnore: Ignore.type = JlinkPlugin.Ignore
   }
 
   import autoImport._
 
-  override def requires = JavaAppPackaging
+  override def requires: Plugins = JavaAppPackaging
 
   override lazy val projectSettings: Seq[Setting[_]] = Seq(
-    target in jlinkBuildImage := target.value / "jlink" / "output",
+    jlinkBuildImage / target := target.value / "jlink" / "output",
     jlinkBundledJvmLocation := "jre",
     bundledJvmLocation := Some(jlinkBundledJvmLocation.value),
     jlinkIgnoreMissingDependency :=
       (jlinkIgnoreMissingDependency ?? JlinkIgnore.nothing).value,
     // Don't use `fullClasspath in Compile` directly - this way we can inject
     // custom classpath elements for the scan.
-    fullClasspath in jlinkBuildImage := (fullClasspath in Compile).value,
+    jlinkBuildImage / fullClasspath := (Compile / fullClasspath).value,
     jlinkModules := (jlinkModules ?? Nil).value,
     jlinkModules ++= {
       val log = streams.value.log
-      val javaHome0 = javaHome.in(jlinkBuildImage).value.getOrElse(defaultJavaHome)
+      val javaHome0 = (jlinkBuildImage / javaHome).value.getOrElse(defaultJavaHome)
       val run = runJavaTool(javaHome0, log) _
-      val paths = fullClasspath.in(jlinkBuildImage).value.map(_.data.getPath)
-      val modulePath = jlinkModulePath.in(jlinkModules).value
+      val paths = (jlinkBuildImage / fullClasspath).value.map(_.data.getPath)
+      val modulePath = (jlinkModules / jlinkModulePath).value
       val shouldIgnore = jlinkIgnoreMissingDependency.value
 
       // We can find the java toolchain version by parsing the `release` file. This
@@ -79,21 +80,16 @@ object JlinkPlugin extends AutoPlugin {
       // are not flexible enough - we need to parse the full output.
       val jdepsOutput = run("jdeps", "--multi-release" +: javaVersion +: modulePathOpts ++: "-R" +: paths)
 
-      val deps = jdepsOutput.linesIterator
-        // There are headers in some of the lines - ignore those.
-        .flatMap(PackageDependency.parse(_).iterator)
-        .toSeq
+      val deps = parseJdeps(jdepsOutput)
 
       // Check that we don't have any dangling dependencies that were not
       // explicitly ignored.
       val missingDeps = deps
         .collect {
-          case PackageDependency(dependent, dependee, PackageDependency.NotFound) =>
+          case PackageDependency(dependent, dependee, PackageDependency.NotFound)
+              if !shouldIgnore((dependent, dependee)) =>
             (dependent, dependee)
         }
-        .filterNot(shouldIgnore)
-        .distinct
-        .sorted
 
       if (missingDeps.nonEmpty) {
         log.error(
@@ -106,11 +102,6 @@ object JlinkPlugin extends AutoPlugin {
         sys.error("Missing package dependencies")
       }
 
-      val detectedModuleDeps = deps.collect {
-        case PackageDependency(_, _, PackageDependency.Module(module)) =>
-          module
-      }.toSet
-
       // Some JakartaEE artifacts use `java.*` module names, even though
       // they are not a part of the platform anymore.
       // https://github.com/eclipse-ee4j/ee4j/issues/34
@@ -133,12 +124,13 @@ object JlinkPlugin extends AutoPlugin {
         "jdk.xml.bind"
       )
 
-      val filteredModuleDeps = detectedModuleDeps
-        .filter { m =>
-          m.startsWith("jdk.") || m.startsWith("java.")
+      val filteredModuleDeps = deps
+        .collect {
+          case PackageDependency(_, _, PackageDependency.Module(m)) if m.startsWith("jdk.") || m.startsWith("java.") =>
+            m
         }
-        .filterNot(knownJakartaJavaModules.contains)
-        .filterNot(removedJavaModules.contains)
+        .diff(knownJakartaJavaModules)
+        .diff(removedJavaModules)
 
       // We always want `java.base`, and `jlink` requires at least one module.
       (filteredModuleDeps + "java.base").toSeq
@@ -154,39 +146,49 @@ object JlinkPlugin extends AutoPlugin {
 
       JlinkOptions(
         addModules = modules,
-        output = Some(target.in(jlinkBuildImage).value),
-        modulePath = jlinkModulePath.in(jlinkBuildImage).value
+        output = Some((jlinkBuildImage / target).value),
+        modulePath = (jlinkBuildImage / jlinkModulePath).value
       )
     },
     jlinkBuildImage := {
       val log = streams.value.log
-      val javaHome0 = javaHome.in(jlinkBuildImage).value.getOrElse(defaultJavaHome)
+      val javaHome0 = (jlinkBuildImage / javaHome).value.getOrElse(defaultJavaHome)
       val run = runJavaTool(javaHome0, log) _
-      val outDir = target.in(jlinkBuildImage).value
+      val outDir = (jlinkBuildImage / target).value
 
       IO.delete(outDir)
 
       run("jlink", jlinkOptions.value)
 
       outDir
     },
-    mappings in jlinkBuildImage := {
+    jlinkBuildImage / mappings := {
       val prefix = jlinkBundledJvmLocation.value
       // make sure the prefix has a terminating slash
-      val prefix0 = if (prefix.isEmpty) prefix else (prefix + "/")
+      val prefix0 = if (prefix.isEmpty) prefix else prefix + "/"
 
       findFiles(jlinkBuildImage.value).map {
         case (file, string) => (file, prefix0 + string)
       }
     },
-    mappings in Universal ++= mappings.in(jlinkBuildImage).value
+    Universal / mappings ++= (jlinkBuildImage / mappings).value
   )
 
   // Extracts java version from a release file line (`JAVA_VERSION` property):
   // - if the feature version is 1, yield the minor version number (e.g. 1.9.0 -> 9);
   // - otherwise yield the major version number (e.g. 11.0.3 -> 11).
   private[jlink] val javaVersionPattern = """JAVA_VERSION="(?:1\.)?(\d+).*?"""".r
 
+  private[jlink] def parseJdeps(jdepsOutput: String): immutable.TreeSet[PackageDependency] =
+    jdepsOutput.linesIterator.foldLeft(
+      immutable.TreeSet.empty[PackageDependency](PackageDependency.PackageDependencyOrdering)
+    ) { (z, l) =>
+      PackageDependency.parse(l) match {
+        case Some(pd) => z + pd
+        case _        => z
+      }
+    }
+
   // TODO: deduplicate with UniversalPlugin and DebianPlugin
   /** Finds all files in a directory. */
   private def findFiles(dir: File): Seq[(File, String)] =
@@ -224,11 +226,11 @@ object JlinkPlugin extends AutoPlugin {
   // to make it a drop-in replacement for `ProcessBuilder.!!`.
   private def runForOutput(builder: ProcessBuilder, log: scala.sys.process.ProcessLogger): String = {
     val buffer = new StringBuffer
-    val code = builder.run(BasicIO(false, buffer, Some(log))).exitValue()
+    val code = builder.run(BasicIO(withIn = false, buffer, Some(log))).exitValue()
 
     if (code == 0) buffer.toString
     else {
-      log.out(buffer.toString)
+      log.err(buffer.toString)
       scala.sys.error("Nonzero exit value: " + code)
     }
   }
@@ -247,15 +249,46 @@ object JlinkPlugin extends AutoPlugin {
   }
 
   // Jdeps output row
-  private final case class PackageDependency(dependent: String, dependee: String, source: PackageDependency.Source)
+  private[jlink] final case class PackageDependency(
+    dependent: String,
+    dependee: String,
+    source: PackageDependency.Source
+  )
+
+  private[jlink] final object PackageDependency {
+
+    implicit object PackageDependencyOrdering extends Ordering[PackageDependency] {
+      override def compare(x: PackageDependency, y: PackageDependency): Int = {
+        var result = x.dependent.compareTo(y.dependent)
+        if (result == 0)
+          result = x.dependee.compareTo(y.dependee)
+        if (result == 0)
+          result = SourceOrdering.compare(x.source, y.source)
+        result
+      }
+    }
+
+    implicit object SourceOrdering extends Ordering[Source] {
+      override def compare(x: Source, y: Source): Int =
+        (x, y) match {
+          case (`x`, `x`)                    => 0
+          case (NotFound, _) | (_, NotFound) => -1
+          case (Classes, _) | (_, Classes)   => -1
+          case (JarOrDir(n1), JarOrDir(n2))  => n1.compareTo(n2)
+          case (_: JarOrDir, _)              => 1
+          case (_, _: JarOrDir)              => 1
+          case (Module(n1), Module(n2))      => n1.compareTo(n2)
+        }
+    }
 
-  private final object PackageDependency {
     sealed trait Source
 
     object Source {
       def parse(s: String): Source =
         s match {
-          case "not found" => NotFound
+          case "not found"                          => NotFound
+          case "classes"                            => Classes
+          case "JDK internal API (jdk.unsupported)" => Module("jdk.unsupported")
           // We have no foolproof way to separate jars from modules here, so
           // we have to do something flaky.
           case name
@@ -268,6 +301,7 @@ object JlinkPlugin extends AutoPlugin {
     }
 
     case object NotFound extends Source
+    case object Classes extends Source
     final case class Module(name: String) extends Source
     final case class JarOrDir(name: String) extends Source
 
@@ -285,13 +319,14 @@ object JlinkPlugin extends AutoPlugin {
     // There are also jar/directory/module-level dependencies, but we are
     // not interested in those:
     // foo.jar -> scala-library-2.12.8.jar
+    // akka-actor-typed_2.13-2.6.15.jar -> java.base
     // classes -> java.base
     // foo.jar -> not found
     private val pattern = """^\s+([^\s]+)\s+->\s+([^\s]+)\s+([^\s].*?)\s*$""".r
 
     def parse(s: String): Option[PackageDependency] =
       s match {
-        case pattern(dependent, dependee, source) =>
+        case pattern(dependent, dependee, source) if dependent != dependee =>
           Some(PackageDependency(dependent, dependee, Source.parse(source)))
         case _ => None
       }