JlinkPlugin: support multi-release dependencies (#1244)

* Fix error logging in JlinkPlugin (#1243)

* Add a test case for multi-release dependency (#1243)

* Add support for multi-release dependencies (#1243)

Author: nigredo-tori

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

@@ -1,7 +1,7 @@
 package com.typesafe.sbt.packager.archetypes
 package jlink
 
-import scala.sys.process.{Process, ProcessBuilder}
+import scala.sys.process.{BasicIO, Process, ProcessBuilder}
 import sbt._
 import sbt.Keys._
 import com.typesafe.sbt.SbtNativePackager.{Debian, Universal}
@@ -49,15 +49,31 @@ object JlinkPlugin extends AutoPlugin {
     jlinkModules := (jlinkModules ?? Nil).value,
     jlinkModules ++= {
       val log = streams.value.log
-      val run = runJavaTool(javaHome.in(jlinkBuildImage).value, log) _
+      val javaHome0 = javaHome.in(jlinkBuildImage).value.getOrElse(defaultJavaHome)
+      val run = runJavaTool(javaHome0, log) _
       val paths = fullClasspath.in(jlinkBuildImage).value.map(_.data.getPath)
       val shouldIgnore = jlinkIgnoreMissingDependency.value
 
+      // We can find the java toolchain version by parsing the `release` file. This
+      // only works for Java 9+, but so does this whole plugin.
+      // Alternatives:
+      // - Parsing `java -version` output - the format is not standardized, so there
+      // are a lot of weird incompatibilities.
+      // - Parsing `java -XshowSettings:properties` output - the format is nicer,
+      // but the command itself is subject to change without notice.
+      val releaseFile = javaHome0 / "release"
+      val javaVersion = IO
+        .readLines(releaseFile)
+        .collectFirst {
+          case javaVersionPattern(feature) => feature
+        }
+        .getOrElse(sys.error("JAVA_VERSION not found in ${releaseFile.getAbsolutePath}"))
+
       // Jdeps has a few convenient options (like --print-module-deps), but those
       // are not flexible enough - we need to parse the full output.
-      val output = run("jdeps", "-R" +: paths) !! log
+      val jdepsOutput = runForOutput(run("jdeps", "--multi-release" +: javaVersion +: "-R" +: paths), log)
 
-      val deps = output.linesIterator
+      val deps = jdepsOutput.linesIterator
       // There are headers in some of the lines - ignore those.
         .flatMap(PackageDependency.parse(_).iterator)
         .toSeq
@@ -109,12 +125,13 @@ object JlinkPlugin extends AutoPlugin {
     },
     jlinkBuildImage := {
       val log = streams.value.log
-      val run = runJavaTool(javaHome.in(jlinkBuildImage).value, log) _
+      val javaHome0 = javaHome.in(jlinkBuildImage).value.getOrElse(defaultJavaHome)
+      val run = runJavaTool(javaHome0, log) _
       val outDir = target.in(jlinkBuildImage).value
 
       IO.delete(outDir)
 
-      run("jlink", jlinkOptions.value) !! log
+      runForOutput(run("jlink", jlinkOptions.value), log)
 
       outDir
     },
@@ -130,21 +147,43 @@ object JlinkPlugin extends AutoPlugin {
     mappings in Universal ++= mappings.in(jlinkBuildImage).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
+
   // TODO: deduplicate with UniversalPlugin and DebianPlugin
   /** Finds all files in a directory. */
   private def findFiles(dir: File): Seq[(File, String)] =
     ((PathFinder(dir) ** AllPassFilter) --- dir)
       .pair(file => IO.relativize(dir, file))
 
-  private def runJavaTool(jvm: Option[File], log: Logger)(exeName: String, args: Seq[String]): ProcessBuilder = {
-    val jh = jvm.getOrElse(file(sys.props.getOrElse("java.home", sys.error("no java.home"))))
-    val exe = (jh / "bin" / exeName).getAbsolutePath
+  private lazy val defaultJavaHome: File =
+    file(sys.props.getOrElse("java.home", sys.error("no java.home")))
+
+  private def runJavaTool(jvm: File, log: Logger)(exeName: String, args: Seq[String]): ProcessBuilder = {
+    val exe = (jvm / "bin" / exeName).getAbsolutePath
 
     log.info("Running: " + (exe +: args).mkString(" "))
 
     Process(exe, args)
   }
 
+  // Like `ProcessBuilder.!!`, but this logs the output in case of a non-zero
+  // exit code. We need this since some Java tools write their errors to stdout.
+  // This uses `scala.sys.process.ProcessLogger` instead of the SBT `Logger`
+  // 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()
+
+    if (code == 0) buffer.toString
+    else {
+      log.out(buffer.toString)
+      scala.sys.error("Nonzero exit value: " + code)
+    }
+  }
+
   private object JlinkOptions {
     @deprecated("1.3.24", "")
     def apply(addModules: Seq[String] = Nil, output: Option[File] = None): Seq[String] =

src/sbt-test/jlink/test-jlink-misc/build.sbt

@@ -0,0 +1,23 @@
+// Various JlinkPlugin test cases that don't warrant setting up separate
+// `scripted` tests.
+
+import scala.sys.process.Process
+import com.typesafe.sbt.packager.Compat._
+
+val runChecks = taskKey[Unit]("Run checks for a specific issue")
+
+// Exclude Scala by default to simplify the test.
+autoScalaLibrary in ThisBuild := false
+
+// Should succeed for multi-release artifacts
+val issue1243 = project
+  .enablePlugins(JlinkPlugin)
+  .settings(
+    libraryDependencies ++= List(
+      // An arbitrary multi-release artifact
+      "org.apache.logging.log4j" % "log4j-core" % "2.12.0"
+    ),
+    // Don't bother with providing dependencies.
+    jlinkIgnoreMissingDependency := JlinkIgnore.everything,
+    runChecks := jlinkBuildImage.value
+  )

src/sbt-test/jlink/test-jlink-misc/project/plugins.sbt

@@ -0,0 +1,8 @@
+{
+  val pluginVersion = sys.props("project.version")
+  if (pluginVersion == null)
+    throw new RuntimeException("""|The system property 'project.version' is not defined.
+               |Specify this property using the scriptedLaunchOpts -D.""".stripMargin)
+  else
+    addSbtPlugin("com.typesafe.sbt" % "sbt-native-packager" % sys.props("project.version"))
+}

src/sbt-test/jlink/test-jlink-misc/test

@@ -0,0 +1,3 @@
+# These tasks can be aggregated, but running them one by one means
+# more granular output in case of a failure.
+> issue1243/runChecks

src/test/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkSpec.scala

@@ -2,6 +2,7 @@ package com.typesafe.sbt.packager.archetypes.jlink
 
 import org.scalatest.{FlatSpec, Matchers}
 import JlinkPlugin.Ignore.byPackagePrefix
+import JlinkPlugin.javaVersionPattern
 
 class JlinkSpec extends FlatSpec with Matchers {
   "Ignore.byPackagePrefix()" should "match as expected for sample examples" in {
@@ -20,4 +21,11 @@ class JlinkSpec extends FlatSpec with Matchers {
     byPackagePrefix("foo" -> "bar", "" -> "")("baz" -> "qux") should be(true)
     byPackagePrefix("foo" -> "", "" -> "bar")("baz" -> "qux") should be(false)
   }
+
+  "javaVersionPattern" should "match known examples" in {
+    """JAVA_VERSION="11.0.3"""" should fullyMatch regex (javaVersionPattern withGroup "11")
+    // Haven't seen this in the wild, but JEP220 has this example, so we might
+    // as well handle it.
+    """JAVA_VERSION="1.9.0"""" should fullyMatch regex (javaVersionPattern withGroup "9")
+  }
 }