From d505490fd4e8503c4427bb6956db6422926963d0 Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Wed, 15 May 2019 22:51:07 +0700 Subject: [PATCH 1/3] Fix missing dependency handling in JlinkPlugin --- .travis.yml | 4 + .../packager/archetypes/jlink/JlinkKeys.scala | 8 ++ .../archetypes/jlink/JlinkPlugin.scala | 101 ++++++++++++++++-- .../bar/src/main/java/bar/Bar.java | 3 + .../jlink/test-jlink-missing-deps/build.sbt | 21 ++++ .../foo/src/main/java/foo/Foo.java | 7 ++ .../project/plugins.sbt | 8 ++ .../jlink/test-jlink-missing-deps/test | 5 + .../withIgnore/src/main/java/WithIgnore.java | 5 + .../src/main/java/WithoutIgnore.java | 5 + src/sphinx/archetypes/misc_archetypes.rst | 15 +++ 11 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 src/sbt-test/jlink/test-jlink-missing-deps/bar/src/main/java/bar/Bar.java create mode 100644 src/sbt-test/jlink/test-jlink-missing-deps/build.sbt create mode 100644 src/sbt-test/jlink/test-jlink-missing-deps/foo/src/main/java/foo/Foo.java create mode 100644 src/sbt-test/jlink/test-jlink-missing-deps/project/plugins.sbt create mode 100644 src/sbt-test/jlink/test-jlink-missing-deps/test create mode 100644 src/sbt-test/jlink/test-jlink-missing-deps/withIgnore/src/main/java/WithIgnore.java create mode 100644 src/sbt-test/jlink/test-jlink-missing-deps/withoutIgnore/src/main/java/WithoutIgnore.java diff --git a/.travis.yml b/.travis.yml index dec68d0de..e12fc44b0 100644 --- a/.travis.yml +++ b/.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: oraclejdk12 + 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) diff --git a/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkKeys.scala b/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkKeys.scala index 203edd9b2..d5501d7c1 100644 --- a/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkKeys.scala +++ b/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkKeys.scala @@ -11,6 +11,14 @@ 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" + ) + val jlinkOptions = TaskKey[Seq[String]]("jlinkOptions", "Options for the jlink utility") diff --git a/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkPlugin.scala b/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkPlugin.scala index ca4a378c9..ff399e568 100644 --- a/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkPlugin.scala +++ b/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") + } + + // Collect all the found modules + deps.collect { + case PackageDependency(_, _, PackageDependency.Module(module)) => + module + }.distinct + }, + jlinkOptions := (jlinkOptions ?? Nil).value, + jlinkOptions ++= { + val modules = jlinkModules.value - val paths = fullClasspath.in(Compile).value.map(_.data.getPath) - val modules = - (run("jdeps", "-R" +: "--print-module-deps" +: paths) !! log).trim - .split(",") + if (modules.isEmpty) { + sys.error("jlinkModules is empty") + } JlinkOptions(addModules = modules, output = Some(target.in(jlinkBuildImage).value)) }, @@ -102,4 +150,43 @@ 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 + + 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 + } } diff --git a/src/sbt-test/jlink/test-jlink-missing-deps/bar/src/main/java/bar/Bar.java b/src/sbt-test/jlink/test-jlink-missing-deps/bar/src/main/java/bar/Bar.java new file mode 100644 index 000000000..e94fe68bd --- /dev/null +++ b/src/sbt-test/jlink/test-jlink-missing-deps/bar/src/main/java/bar/Bar.java @@ -0,0 +1,3 @@ +package bar; + +public class Bar {} diff --git a/src/sbt-test/jlink/test-jlink-missing-deps/build.sbt b/src/sbt-test/jlink/test-jlink-missing-deps/build.sbt new file mode 100644 index 000000000..794699b5d --- /dev/null +++ b/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") + ) diff --git a/src/sbt-test/jlink/test-jlink-missing-deps/foo/src/main/java/foo/Foo.java b/src/sbt-test/jlink/test-jlink-missing-deps/foo/src/main/java/foo/Foo.java new file mode 100644 index 000000000..367d8bba8 --- /dev/null +++ b/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(); + } +} diff --git a/src/sbt-test/jlink/test-jlink-missing-deps/project/plugins.sbt b/src/sbt-test/jlink/test-jlink-missing-deps/project/plugins.sbt new file mode 100644 index 000000000..c7fe8be80 --- /dev/null +++ b/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")) +} diff --git a/src/sbt-test/jlink/test-jlink-missing-deps/test b/src/sbt-test/jlink/test-jlink-missing-deps/test new file mode 100644 index 000000000..13c322ed0 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/src/sbt-test/jlink/test-jlink-missing-deps/withIgnore/src/main/java/WithIgnore.java b/src/sbt-test/jlink/test-jlink-missing-deps/withIgnore/src/main/java/WithIgnore.java new file mode 100644 index 000000000..cba889a12 --- /dev/null +++ b/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(); + } +} diff --git a/src/sbt-test/jlink/test-jlink-missing-deps/withoutIgnore/src/main/java/WithoutIgnore.java b/src/sbt-test/jlink/test-jlink-missing-deps/withoutIgnore/src/main/java/WithoutIgnore.java new file mode 100644 index 000000000..6d51e8eec --- /dev/null +++ b/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(); + } +} diff --git a/src/sphinx/archetypes/misc_archetypes.rst b/src/sphinx/archetypes/misc_archetypes.rst index a1f11f6e1..a54744e36 100644 --- a/src/sphinx/archetypes/misc_archetypes.rst +++ b/src/sphinx/archetypes/misc_archetypes.rst @@ -41,6 +41,21 @@ 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, or disable the check altogether: + +.. code-block:: scala + + jlinkIgnoreMissingDependency := JlinkIgnore.everything + For further details on the capabilities of `jlink`, see the `jlink `_ and `jdeps `_ references. From c1c5a50ff1de1e903f8a7d327175bb050f56791b Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Sat, 18 May 2019 01:05:57 +0700 Subject: [PATCH 2/3] Switch to OpenJDK 12 for JLink test --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index e12fc44b0..2aae32c8f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -83,7 +83,7 @@ jobs: if: type = pull_request OR (type = push AND branch = master) - script: sbt "^validateJlink" name: "scripted jlink tests" - jdk: oraclejdk12 + jdk: openjdk12 if: type = pull_request OR (type = push AND branch = master) - script: sbt "^validateDocker" name: "scripted docker integration-tests" From f24c452a3d2a37b295925c2b63f82f9c5df0a3c9 Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Sat, 25 May 2019 13:36:53 +0700 Subject: [PATCH 3/3] Add comments/documentation following PR discussion --- .../packager/archetypes/jlink/JlinkKeys.scala | 7 ++++++- .../packager/archetypes/jlink/JlinkPlugin.scala | 16 ++++++++++++++++ src/sphinx/archetypes/misc_archetypes.rst | 11 ++++++++++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkKeys.scala b/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkKeys.scala index d5501d7c1..1639092c2 100644 --- a/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkKeys.scala +++ b/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkKeys.scala @@ -16,7 +16,12 @@ private[packager] trait JlinkKeys { val jlinkIgnoreMissingDependency = TaskKey[((String, String)) => Boolean]( "jlinkIgnoreMissingDependency", - "A hook to mask missing package dependency issues" + """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 = diff --git a/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkPlugin.scala b/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkPlugin.scala index ff399e568..59aef1ed8 100644 --- a/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkPlugin.scala +++ b/src/main/scala/com/typesafe/sbt/packager/archetypes/jlink/JlinkPlugin.scala @@ -175,6 +175,22 @@ object JlinkPlugin extends AutoPlugin { 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 { diff --git a/src/sphinx/archetypes/misc_archetypes.rst b/src/sphinx/archetypes/misc_archetypes.rst index a54744e36..648451224 100644 --- a/src/sphinx/archetypes/misc_archetypes.rst +++ b/src/sphinx/archetypes/misc_archetypes.rst @@ -50,7 +50,16 @@ The plugin analyzes the dependencies between packages using `jdeps`, and raises "foo.bar" -> "bar.qux" ) -For large projects with a lot of dependencies this can get unwieldy. You can implement a more flexible ignore strategy, or disable the check altogether: +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