Fix dependency handling in JlinkPlugin (+ general improvements) (#1226)

* Fix missing dependency handling in JlinkPlugin

* Switch to OpenJDK 12 for JLink test

* Add comments/documentation following PR discussion

Author: nigredo-tori

.travis.yml

@@ -81,6 +81,10 @@ jobs:
       name: "scripted jlink tests"
       jdk: oraclejdk11
       if: type = pull_request OR (type = push AND branch = master)
+    - script: sbt "^validateJlink"
+      name: "scripted jlink tests"
+      jdk: openjdk12
+      if: type = pull_request OR (type = push AND branch = master)
     - script: sbt "^validateDocker"
       name: "scripted docker integration-tests"
       if: type = pull_request OR (type = push AND branch = master)

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

@@ -11,6 +11,19 @@ private[packager] trait JlinkKeys {
   val jlinkBundledJvmLocation =
     TaskKey[String]("jlinkBundledJvmLocation", "The location of the resulting JVM image")
 
+  val jlinkModules = TaskKey[Seq[String]]("jlinkModules", "Modules to link")
+
+  val jlinkIgnoreMissingDependency =
+    TaskKey[((String, String)) => Boolean](
+      "jlinkIgnoreMissingDependency",
+      """A hook to mask missing package dependency issues.
+        |This receives a pair of dependent and dependee packages (where the dependee package is NOT
+        |present in the classpath), and returns true if this dependency should be ignored. Any
+        |missing dependencies that are not ignored will result in an error when running
+        |jlinkBuildImage.
+      """.stripMargin
+    )
+
   val jlinkOptions =
     TaskKey[Seq[String]]("jlinkOptions", "Options for the jlink utility")
 

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

@@ -29,7 +29,9 @@ import com.typesafe.sbt.packager.universal.UniversalPlugin
   */
 object JlinkPlugin extends AutoPlugin {
 
-  object autoImport extends JlinkKeys
+  object autoImport extends JlinkKeys {
+    val JlinkIgnore = JlinkPlugin.Ignore
+  }
 
   import autoImport._
 
@@ -39,15 +41,61 @@ object JlinkPlugin extends AutoPlugin {
     target in jlinkBuildImage := target.value / "jlink" / "output",
     jlinkBundledJvmLocation := "jre",
     bundledJvmLocation := Some(jlinkBundledJvmLocation.value),
-    jlinkOptions := (jlinkOptions ?? Nil).value,
-    jlinkOptions ++= {
+    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,
+    jlinkModules := (jlinkModules ?? Nil).value,
+    jlinkModules ++= {
       val log = streams.value.log
       val run = runJavaTool(javaHome.in(jlinkBuildImage).value, log) _
+      val paths = fullClasspath.in(jlinkBuildImage).value.map(_.data.getPath)
+      val shouldIgnore = jlinkIgnoreMissingDependency.value
+
+      // 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 deps = output.linesIterator
+      // There are headers in some of the lines - ignore those.
+        .flatMap(PackageDependency.parse(_).iterator)
+        .toSeq
+
+      // Check that we don't have any dangling dependencies that were not
+      // explicitly ignored.
+      val missingDeps = deps
+        .collect {
+          case PackageDependency(dependent, dependee, PackageDependency.NotFound) =>
+            (dependent, dependee)
+        }
+        .filterNot(shouldIgnore)
+        .distinct
+
+      if (missingDeps.nonEmpty) {
+        log.error(
+          "Dependee packages not found in classpath. You can use jlinkIgnoreMissingDependency to silence these."
+        )
+        missingDeps.foreach {
+          case (a, b) =>
+            log.error(s"  $a -> $b")
+        }
+        sys.error("Missing package dependencies")
+      }
 
-      val paths = fullClasspath.in(Compile).value.map(_.data.getPath)
-      val modules =
-        (run("jdeps", "-R" +: "--print-module-deps" +: paths) !! log).trim
-          .split(",")
+      // Collect all the found modules
+      deps.collect {
+        case PackageDependency(_, _, PackageDependency.Module(module)) =>
+          module
+      }.distinct
+    },
+    jlinkOptions := (jlinkOptions ?? Nil).value,
+    jlinkOptions ++= {
+      val modules = jlinkModules.value
+
+      if (modules.isEmpty) {
+        sys.error("jlinkModules is empty")
+      }
 
       JlinkOptions(addModules = modules, output = Some(target.in(jlinkBuildImage).value))
     },
@@ -102,4 +150,59 @@ object JlinkPlugin extends AutoPlugin {
     private def list(arg: String, values: Seq[String]): Seq[String] =
       if (values.nonEmpty) Seq(arg, values.mkString(",")) else Nil
   }
+
+  // Jdeps output row
+  private final case class PackageDependency(dependent: String, dependee: String, source: PackageDependency.Source)
+
+  private final object PackageDependency {
+    sealed trait Source
+
+    object Source {
+      def parse(s: String): Source = s match {
+        case "not found" => NotFound
+        // We have no foolproof way to separate jars from modules here, so
+        // we have to do something flaky.
+        case name
+            if name.toLowerCase.endsWith(".jar") ||
+              !name.contains('.') ||
+              name.contains(' ') =>
+          JarOrDir(name)
+        case name => Module(name)
+      }
+    }
+
+    case object NotFound extends Source
+    final case class Module(name: String) extends Source
+    final case class JarOrDir(name: String) extends Source
+
+    // Examples of package dependencies in jdeps output (whitespace may vary,
+    // but there will always be some leading whitespace):
+    // Dependency on a package(java.lang) in a module (java.base):
+    //   foo.bar -> java.lang java.base
+    // Dependency on a package (scala.collection) in a JAR
+    // (scala-library-2.12.8.jar):
+    //   foo.bar -> scala.collection scala-library-2.12.8.jar
+    // Dependency on a package (foo.baz) in a class directory (classes):
+    //   foo.bar -> foo.baz classes
+    // Missing dependency on a package (qux.quux):
+    //   foo.bar -> qux.quux not found
+    // There are also jar/directory/module-level dependencies, but we are
+    // not interested in those:
+    // foo.jar -> scala-library-2.12.8.jar
+    // 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) =>
+        Some(PackageDependency(dependent, dependee, Source.parse(source)))
+      case _ => None
+    }
+  }
+
+  object Ignore {
+    val nothing: ((String, String)) => Boolean = Function.const(false)
+    val everything: ((String, String)) => Boolean = Function.const(true)
+    def only(dependencies: (String, String)*): ((String, String)) => Boolean = dependencies.toSet.contains
+  }
 }

src/sbt-test/jlink/test-jlink-missing-deps/bar/src/main/java/bar/Bar.java

@@ -0,0 +1,3 @@
+package bar;
+
+public class Bar {}

src/sbt-test/jlink/test-jlink-missing-deps/build.sbt

@@ -0,0 +1,21 @@
+// Tests jlink behavior with missing dependencies.
+
+import scala.sys.process.Process
+import com.typesafe.sbt.packager.Compat._
+
+
+// Exclude Scala to simplify the test
+autoScalaLibrary in ThisBuild := false
+
+// Simulate a missing dependency (foo -> bar)
+lazy val foo = project.dependsOn(bar % "provided")
+lazy val bar = project
+
+lazy val withoutIgnore = project.dependsOn(foo)
+  .enablePlugins(JlinkPlugin)
+
+lazy val withIgnore = project.dependsOn(foo)
+  .enablePlugins(JlinkPlugin)
+  .settings(
+    jlinkIgnoreMissingDependency := JlinkIgnore.only("foo" -> "bar")
+  )

src/sbt-test/jlink/test-jlink-missing-deps/foo/src/main/java/foo/Foo.java

@@ -0,0 +1,7 @@
+package foo;
+
+public class Foo {
+  public Foo() {
+    new bar.Bar();
+  }
+}

src/sbt-test/jlink/test-jlink-missing-deps/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-missing-deps/test

@@ -0,0 +1,5 @@
+> compile
+# Should fail since we have a missing dependency.
+-> withoutIgnore/jlinkBuildImage
+# Should work OK since the issue is silenced
+> withIgnore/jlinkBuildImage

src/sbt-test/jlink/test-jlink-missing-deps/withIgnore/src/main/java/WithIgnore.java

@@ -0,0 +1,5 @@
+class WithIgnore {
+  public WithIgnore() {
+    new foo.Foo();
+  }
+}

src/sbt-test/jlink/test-jlink-missing-deps/withoutIgnore/src/main/java/WithoutIgnore.java

@@ -0,0 +1,5 @@
+class WithoutIgnore {
+  public WithoutIgnore() {
+    new foo.Foo();
+  }
+}

src/sphinx/archetypes/misc_archetypes.rst

@@ -41,6 +41,30 @@ addressed in the current plugin version.
 This plugin must be run on the platform of the target installer. The tooling does *not*
 provide a means of creating, say, Windows installers on MacOS, or MacOS on Linux, etc.
 
+The plugin analyzes the dependencies between packages using `jdeps`, and raises an error in case of a missing dependency (e.g. for a provided transitive dependency). The missing dependencies can be suppressed on a case-by-case basis (e.g. if you are sure the missing dependency is properly handled):
+
+.. code-block:: scala
+
+    jlinkIgnoreMissingDependency := JlinkIgnore.only(
+      "foo.bar" -> "bar.baz",
+      "foo.bar" -> "bar.qux"
+    )
+
+For large projects with a lot of dependencies this can get unwieldy. You can implement a more flexible ignore strategy:
+
+.. code-block:: scala
+
+  jlinkIgnoreMissingDependency := {
+    case ("foo.bar", dependee) if dependee.startsWith("bar") => true
+    case _ => false
+  }
+
+Otherwise you may opt out of the check altogether (which is not recommended):
+
+.. code-block:: scala
+
+   jlinkIgnoreMissingDependency := JlinkIgnore.everything
+
 For further details on the capabilities of `jlink`, see the
 `jlink <https://docs.oracle.com/en/java/javase/11/tools/jlink.html>`_ and
 `jdeps <https://docs.oracle.com/en/java/javase/11/tools/jdeps.html>`_ references.