Simplify declaration of module path in sbt build (#10836)

* Introduce new setting key moduleDependencies.

modulePath is based on that setting.

* runtime-language-arrow uses moduleDependencies and not modulePath

* Use moduleDependencies instead of modulePath

* Fix moduleDependencies of ydoc-server

* Fix project-manager/Test/compile

* Make error messages in JPMSUtils more descriptive

* Remove unnecessary libraryDependencies from ydoc-server.

JPMSPlugin handles Runtime/moduleDependencies

* Fix engine-benchmarks/run

build.sbt

@@ -583,6 +583,25 @@ val jnaVersion              = "5.14.0"
 // === Utility methods =====================================================
 // ============================================================================
 
+lazy val componentModulesIds =
+  taskKey[Seq[ModuleID]](
+    "Gather all sbt module IDs that will be put on the module-path for the engine runner"
+  )
+(ThisBuild / componentModulesIds) := {
+  GraalVM.modules ++ GraalVM.langsPkgs ++ GraalVM.toolsPkgs ++ helidon ++ Seq(
+    "org.slf4j"        % "slf4j-api"                    % slf4jVersion,
+    "ch.qos.logback"   % "logback-classic"              % logbackClassicVersion,
+    "ch.qos.logback"   % "logback-core"                 % logbackClassicVersion,
+    "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion,
+    (`runtime-language-arrow` / projectID).value,
+    (`syntax-rust-definition` / projectID).value,
+    (`ydoc-server` / projectID).value,
+    (`profiling-utils` / projectID).value,
+    (`runtime-fat-jar` / projectID).value
+  )
+}
+
+// TODO[pm]: this is now deprecated and should be removed
 lazy val componentModulesPaths =
   taskKey[Seq[File]](
     "Gathers all component modules (Jar archives that should be put on module-path" +
@@ -836,15 +855,10 @@ lazy val `profiling-utils` = project
       "junit"          % "junit"           % junitVersion   % Test,
       "com.github.sbt" % "junit-interface" % junitIfVersion % Test
     ),
-    modulePath := {
+    moduleDependencies := {
-      JPMSUtils.filterModulesFromUpdate(
+      Seq(
-        update.value,
+        "org.slf4j"        % "slf4j-api"                    % slf4jVersion,
-        Seq(
+        "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion
-          "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion,
-          "org.slf4j"        % "slf4j-api"                    % slf4jVersion
-        ),
-        streams.value.log,
-        shouldContainAll = true
       )
     }
   )
@@ -1094,33 +1108,14 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager"))
       (`profiling-utils` / javaModuleName).value,
       (`ydoc-server` / javaModuleName).value
     ),
-    Test / modulePath := {
+    Test / moduleDependencies := {
-      val updateReport = (Test / update).value
+      GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ helidon ++ Seq(
-      val requiredModIds =
+        "org.slf4j"        % "slf4j-api"                    % slf4jVersion,
-        GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ helidon ++ Seq(
+        "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion,
-          "org.slf4j"        % "slf4j-api"                    % slf4jVersion,
+        (`runtime-fat-jar` / projectID).value,
-          "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion
+        (`ydoc-server` / projectID).value,
-        )
+        (`syntax-rust-definition` / projectID).value,
-      val requiredMods = JPMSUtils.filterModulesFromUpdate(
+        (`profiling-utils` / projectID).value
-        updateReport,
-        requiredModIds,
-        streams.value.log,
-        shouldContainAll = true
-      )
-      val runtimeMod =
-        (`runtime-fat-jar` / Compile / productDirectories).value.head
-      val ydocMod =
-        (`ydoc-server` / Compile / exportedProducts).value.head.data
-      val syntaxMod =
-        (`syntax-rust-definition` / Compile / exportedProducts).value.head.data
-      val profilingMod =
-        (`profiling-utils` / Compile / exportedProducts).value.head.data
-
-      requiredMods ++ Seq(
-        runtimeMod,
-        ydocMod,
-        syntaxMod,
-        profilingMod
       )
     },
     Test / javaOptions ++= testLogProviderOptions
@@ -1168,6 +1163,9 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager"))
   .dependsOn(testkit % Test)
   .dependsOn(`runtime-version-manager-test` % Test)
   .dependsOn(`logging-service-logback` % "test->test")
+  .dependsOn(`runtime-fat-jar` % Test)
+  .dependsOn(`ydoc-server` % Test)
+  .dependsOn(`profiling-utils` % Test)
 
 /* Note [Classpath Separation]
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -1268,24 +1266,19 @@ lazy val `ydoc-server` = project
     autoScalaLibrary := false,
     Test / fork := true,
     commands += WithDebugCommand.withDebug,
-    // GraalVM and helidon modules (3rd party modules)
+    moduleDependencies := {
-    modulePath := {
+      helidon ++ Seq(
-      JPMSUtils.filterModulesFromUpdate(
+        "org.graalvm.polyglot" % "polyglot"        % graalMavenPackagesVersion,
-        update.value,
+        "org.graalvm.truffle"  % "truffle-api"     % graalMavenPackagesVersion,
-        GraalVM.modules ++ GraalVM.jsPkgs ++ GraalVM.chromeInspectorPkgs ++ helidon ++ Seq(
+        "org.slf4j"            % "slf4j-api"       % slf4jVersion,
-          "org.slf4j"      % "slf4j-api"       % slf4jVersion,
+        "ch.qos.logback"       % "logback-classic" % logbackClassicVersion,
-          "ch.qos.logback" % "logback-classic" % logbackClassicVersion,
+        "ch.qos.logback"       % "logback-core"    % logbackClassicVersion,
-          "ch.qos.logback" % "logback-core"    % logbackClassicVersion
+        (`syntax-rust-definition` / projectID).value,
-        ),
+        (`profiling-utils` / projectID).value
-        streams.value.log,
+      ),
-        shouldContainAll = true
-      )
     },
-    // Internal project modules
+    Runtime / moduleDependencies ++=
-    modulePath ++= Seq(
+      GraalVM.modules ++ GraalVM.jsPkgs ++ GraalVM.chromeInspectorPkgs,
-      (`syntax-rust-definition` / Compile / productDirectories).value.head,
-      (`profiling-utils` / Compile / productDirectories).value.head
-    ),
     libraryDependencies ++= Seq(
       "org.graalvm.truffle"        % "truffle-api"                 % graalMavenPackagesVersion % "provided",
       "org.graalvm.polyglot"       % "inspect"                     % graalMavenPackagesVersion % "runtime",
@@ -1585,32 +1578,14 @@ lazy val `language-server` = (project in file("engine/language-server"))
       (`profiling-utils` / javaModuleName).value,
       (`ydoc-server` / javaModuleName).value
     ),
-    Test / modulePath := {
+    Test / moduleDependencies := {
-      val updateReport = (Test / update).value
+      GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ helidon ++ Seq(
-      val requiredModIds =
+        "org.slf4j"        % "slf4j-api"                    % slf4jVersion,
-        GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ helidon ++ Seq(
+        "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion,
-          "org.slf4j"        % "slf4j-api"                    % slf4jVersion,
+        (`runtime-fat-jar` / projectID).value,
-          "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion
+        (`syntax-rust-definition` / projectID).value,
-        )
+        (`ydoc-server` / projectID).value,
-      val requiredMods = JPMSUtils.filterModulesFromUpdate(
+        (`profiling-utils` / projectID).value
-        updateReport,
-        requiredModIds,
-        streams.value.log,
-        shouldContainAll = true
-      )
-      val runtimeMod =
-        (`runtime-fat-jar` / Compile / productDirectories).value.head
-      val syntaxMod =
-        (`syntax-rust-definition` / Compile / productDirectories).value.head
-      val ydocMod =
-        (`ydoc-server` / Compile / productDirectories).value.head
-      val profilingMod =
-        (`profiling-utils` / Compile / productDirectories).value.head
-      requiredMods ++ Seq(
-        runtimeMod,
-        syntaxMod,
-        ydocMod,
-        profilingMod
       )
     },
     Test / javaOptions ++= testLogProviderOptions,
@@ -1698,6 +1673,7 @@ lazy val `language-server` = (project in file("engine/language-server"))
   .dependsOn(filewatcher)
   .dependsOn(testkit % Test)
   .dependsOn(`logging-service-logback` % "test->test")
+  .dependsOn(`runtime-fat-jar` % Test)
   .dependsOn(`library-manager-test` % Test)
   .dependsOn(`runtime-version-manager-test` % Test)
   .dependsOn(`ydoc-server`)
@@ -1843,19 +1819,9 @@ lazy val `runtime-language-arrow` =
         "org.apache.arrow" % "arrow-memory-netty" % apacheArrowVersion % Test
       ),
       javaModuleName := "org.enso.interpreter.arrow",
-      modulePath := {
+      moduleDependencies := GraalVM.modules,
-        val updateReport = (Test / update).value
+      Test / moduleDependencies +=
-        JPMSUtils.filterModulesFromUpdate(
+        (LocalProject("runtime-language-arrow") / projectID).value,
-          updateReport,
-          GraalVM.modules,
-          streams.value.log,
-          shouldContainAll = true
-        ) ++ Seq(
-          (LocalProject(
-            "runtime-language-arrow"
-          ) / Compile / productDirectories).value.head
-        )
-      },
       Test / patchModules := {
         val testClassesDir = (Test / productDirectories).value.head
         Map(javaModuleName.value -> Seq(testClassesDir))
@@ -1881,21 +1847,6 @@ lazy val `runtime-test-instruments` =
       inConfig(Compile)(truffleRunOptionsSettings),
       truffleDslSuppressWarnsSetting,
       instrumentationSettings,
-      javaModuleName := "org.enso.runtime.test",
-      modulePath := {
-        JPMSUtils.filterModulesFromUpdate(
-          update.value,
-          GraalVM.modules ++ Seq(
-            "org.graalvm.sdk"     % "polyglot-tck"            % graalMavenPackagesVersion,
-            "org.graalvm.truffle" % "truffle-tck"             % graalMavenPackagesVersion,
-            "org.graalvm.truffle" % "truffle-tck-common"      % graalMavenPackagesVersion,
-            "org.graalvm.truffle" % "truffle-tck-tests"       % graalMavenPackagesVersion,
-            "org.netbeans.api"    % "org-openide-util-lookup" % netbeansApiVersion
-          ),
-          streams.value.log,
-          shouldContainAll = true
-        )
-      },
       libraryDependencies ++= GraalVM.modules,
       libraryDependencies ++= Seq(
         "org.graalvm.sdk"     % "polyglot-tck"            % graalMavenPackagesVersion,
@@ -1903,7 +1854,17 @@ lazy val `runtime-test-instruments` =
         "org.graalvm.truffle" % "truffle-tck-common"      % graalMavenPackagesVersion,
         "org.graalvm.truffle" % "truffle-tck-tests"       % graalMavenPackagesVersion,
         "org.netbeans.api"    % "org-openide-util-lookup" % netbeansApiVersion % "provided"
-      )
+      ),
+      javaModuleName := "org.enso.runtime.test",
+      moduleDependencies := {
+        GraalVM.modules ++ Seq(
+          "org.graalvm.sdk"     % "polyglot-tck"            % graalMavenPackagesVersion,
+          "org.graalvm.truffle" % "truffle-tck"             % graalMavenPackagesVersion,
+          "org.graalvm.truffle" % "truffle-tck-common"      % graalMavenPackagesVersion,
+          "org.graalvm.truffle" % "truffle-tck-tests"       % graalMavenPackagesVersion,
+          "org.netbeans.api"    % "org-openide-util-lookup" % netbeansApiVersion % "provided"
+        )
+      }
     )
 
 lazy val runtime = (project in file("engine/runtime"))
@@ -2025,40 +1986,20 @@ lazy val `runtime-integration-tests` =
         (`profiling-utils` / javaModuleName).value,
         (`ydoc-server` / javaModuleName).value
       ),
-      Test / modulePath := {
+      Test / moduleDependencies := {
-        val updateReport = (Test / update).value
+        GraalVM.modules ++ GraalVM.langsPkgs ++ GraalVM.insightPkgs ++ logbackPkg ++ helidon ++ Seq(
-        val requiredModIds =
+          "org.slf4j"           % "slf4j-api"                    % slf4jVersion,
-          GraalVM.modules ++ GraalVM.langsPkgs ++ GraalVM.insightPkgs ++ logbackPkg ++ helidon ++ Seq(
+          "org.netbeans.api"    % "org-openide-util-lookup"      % netbeansApiVersion,
-            "org.slf4j"           % "slf4j-api"                    % slf4jVersion,
+          "org.netbeans.api"    % "org-netbeans-modules-sampler" % netbeansApiVersion,
-            "org.netbeans.api"    % "org-openide-util-lookup"      % netbeansApiVersion,
+          "org.graalvm.sdk"     % "polyglot-tck"                 % graalMavenPackagesVersion,
-            "org.netbeans.api"    % "org-netbeans-modules-sampler" % netbeansApiVersion,
+          "org.graalvm.truffle" % "truffle-tck"                  % graalMavenPackagesVersion,
-            "org.graalvm.sdk"     % "polyglot-tck"                 % graalMavenPackagesVersion,
+          "org.graalvm.truffle" % "truffle-tck-common"           % graalMavenPackagesVersion,
-            "org.graalvm.truffle" % "truffle-tck"                  % graalMavenPackagesVersion,
+          "org.graalvm.truffle" % "truffle-tck-tests"            % graalMavenPackagesVersion,
-            "org.graalvm.truffle" % "truffle-tck-common"           % graalMavenPackagesVersion,
+          (`runtime-test-instruments` / projectID).value,
-            "org.graalvm.truffle" % "truffle-tck-tests"            % graalMavenPackagesVersion
+          (`runtime-fat-jar` / projectID).value,
-          )
+          (`ydoc-server` / projectID).value,
-        val requiredMods = JPMSUtils.filterModulesFromUpdate(
+          (`syntax-rust-definition` / projectID).value,
-          updateReport,
+          (`profiling-utils` / projectID).value
-          requiredModIds,
-          streams.value.log,
-          shouldContainAll = true
-        )
-        val runtimeTestInstrumentsMod =
-          (`runtime-test-instruments` / Compile / exportedProducts).value.head.data
-        val runtimeMod =
-          (`runtime-fat-jar` / Compile / exportedProducts).value.head.data
-        val ydocMod =
-          (`ydoc-server` / Compile / exportedProducts).value.head.data
-        val syntaxMod =
-          (`syntax-rust-definition` / Compile / exportedProducts).value.head.data
-        val profilingMod =
-          (`profiling-utils` / Compile / exportedProducts).value.head.data
-        requiredMods ++ Seq(
-          runtimeTestInstrumentsMod,
-          runtimeMod,
-          ydocMod,
-          syntaxMod,
-          profilingMod
         )
       },
       Test / patchModules := {
@@ -2158,34 +2099,19 @@ lazy val `runtime-benchmarks` =
         .dependsOn(`runtime-fat-jar` / assembly)
         .value,
       parallelExecution := false,
-      modulePath := {
+      moduleDependencies := {
-        val requiredModIds =
+        GraalVM.modules ++ GraalVM.langsPkgs ++ helidon ++ Seq(
-          GraalVM.modules ++ GraalVM.langsPkgs ++ helidon ++ Seq(
+          "org.slf4j"        % "slf4j-api"                    % slf4jVersion,
-            "org.slf4j"        % "slf4j-api"                    % slf4jVersion,
+          "org.slf4j"        % "slf4j-nop"                    % slf4jVersion,
-            "org.slf4j"        % "slf4j-nop"                    % slf4jVersion,
+          "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion,
-            "org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion
+          (`ydoc-server` / projectID).value,
-          )
+          (`syntax-rust-definition` / projectID).value,
-        val requiredMods = JPMSUtils.filterModulesFromUpdate(
+          (`profiling-utils` / projectID).value
-          (Compile / update).value,
-          requiredModIds,
-          streams.value.log,
-          shouldContainAll = true
-        )
-        val runtimeMod =
-          (`runtime-fat-jar` / assembly / assemblyOutputPath).value
-        val ydocMod =
-          (`ydoc-server` / Compile / exportedProducts).value.head.data
-        val syntaxMod =
-          (`syntax-rust-definition` / Compile / exportedProducts).value.head.data
-        val profilingMod =
-          (`profiling-utils` / Compile / exportedProducts).value.head.data
-        requiredMods ++ Seq(
-          runtimeMod,
-          ydocMod,
-          syntaxMod,
-          profilingMod
         )
       },
+      modulePath += {
+        (`runtime-fat-jar` / assembly / assemblyOutputPath).value
+      },
       addModules := {
         val runtimeModuleName = (`runtime-fat-jar` / javaModuleName).value
         Seq(runtimeModuleName)
@@ -2969,18 +2895,10 @@ lazy val `std-benchmarks` = (project in file("std-bits/benchmarks"))
       "-J-Dpolyglotimpl.DisableClassPathIsolation=true",
       "-J-Dpolyglot.engine.WarnInterpreterOnly=false"
     ),
-    modulePath := {
+    moduleDependencies := {
-      val allRuntimeMods = componentModulesPaths.value
+      componentModulesIds.value ++ Seq(
-      val otherModIds = Seq(
         "org.slf4j" % "slf4j-nop" % slf4jVersion
       )
-      val requiredMods = JPMSUtils.filterModulesFromUpdate(
-        (Compile / update).value,
-        otherModIds,
-        streams.value.log,
-        shouldContainAll = true
-      )
-      allRuntimeMods ++ requiredMods
     },
     addModules := {
       val runtimeModuleName = (`runtime-fat-jar` / javaModuleName).value

project/JPMSPlugin.scala

@@ -12,6 +12,22 @@ import java.io.File
   *
   * If this plugin is enabled, and no settings/tasks from this plugin are used, then the plugin will
   * not inject anything into `javaOptions` or `javacOptions`.
+  *
+  * == How to work with this plugin ==
+  * - Specify `moduleDependencies` with something like:
+  *  {{{
+  *  moduleDependencies := Seq(
+  *    "org.apache.commons" % "commons-lang3" % "3.11",
+  *  )
+  *  }}}
+  *  - Ensure that all the module dependencies were gathered by the plugin correctly by
+  *    `print modulePath`.
+  *    - If not, make sure that these dependencies are in `libraryDependencies`.
+  *      Debug this with `print dependencyClasspath`.
+  *
+  * == Caveats ==
+  * - This plugin cannot determine transitive dependencies of modules in `moduleDependencies`.
+  *   As opposed to `libraryDependencies` which automatically gatheres all the transitive dependencies.
   */
 object JPMSPlugin extends AutoPlugin {
   object autoImport {
@@ -20,6 +36,11 @@ object JPMSPlugin extends AutoPlugin {
     val addModules = settingKey[Seq[String]](
       "Module names that will be added to --add-modules option"
     )
+    val moduleDependencies = taskKey[Seq[ModuleID]](
+      "Modules dependencies that will be added to --module-path option. List all the sbt modules " +
+      "that should be added on module-path, including internal dependencies. To get ModuleID for a " +
+      "local dependency, use the `projectID` setting."
+    )
     val modulePath = taskKey[Seq[File]](
       "Directories (Jar archives or expanded Jar archives) that will be put into " +
       "--module-path option"
@@ -51,7 +72,19 @@ object JPMSPlugin extends AutoPlugin {
 
   override lazy val projectSettings: Seq[Setting[_]] = Seq(
     addModules := Seq.empty,
-    modulePath := Seq.empty,
+    moduleDependencies := Seq.empty,
+    // modulePath is set based on moduleDependencies
+    modulePath := {
+      val cp = JPMSUtils.filterModulesFromClasspath(
+        // Do not use fullClasspath here - it will result in an infinite recursion
+        // and sbt will not be able to detect the cycle.
+        (Compile / dependencyClasspath).value,
+        (Compile / moduleDependencies).value,
+        streams.value.log,
+        shouldContainAll = true
+      )
+      cp.map(_.data)
+    },
     patchModules := Map.empty,
     addExports := Map.empty,
     addReads := Map.empty,
@@ -75,6 +108,15 @@ object JPMSPlugin extends AutoPlugin {
         (Compile / addReads).value
       )
     },
+    Test / modulePath := {
+      val cp = JPMSUtils.filterModulesFromClasspath(
+        (Test / dependencyClasspath).value,
+        (Test / moduleDependencies).value,
+        streams.value.log,
+        shouldContainAll = true
+      )
+      cp.map(_.data)
+    },
     Test / javacOptions ++= {
       constructOptions(
         streams.value.log,
@@ -91,6 +133,15 @@ object JPMSPlugin extends AutoPlugin {
         (Test / addExports).value,
         (Test / addReads).value
       )
+    },
+    Runtime / modulePath := {
+      val cp = JPMSUtils.filterModulesFromClasspath(
+        (Runtime / dependencyClasspath).value,
+        (Runtime / moduleDependencies).value,
+        streams.value.log,
+        shouldContainAll = true
+      )
+      cp.map(_.data)
     }
   )
 

project/JPMSUtils.scala

@@ -70,9 +70,14 @@ object JPMSUtils {
     })
     if (shouldContainAll) {
       if (ret.size < distinctModules.size) {
-        log.error("Not all modules from classpath were found")
+        log.error("[JPMSUtils] Not all modules from classpath were found")
-        log.error(s"Returned (${ret.size}): $ret")
+        log.error(
-        log.error(s"Expected: (${distinctModules.size}): $distinctModules")
+          "[JPMSUtils] Ensure libraryDependencies and moduleDependencies are correct"
+        )
+        log.error(s"[JPMSUtils] Returned (${ret.size}): $ret")
+        log.error(
+          s"[JPMSUtils] Expected: (${distinctModules.size}): $distinctModules"
+        )
       }
     }
     ret
@@ -107,9 +112,14 @@ object JPMSUtils {
     )
     if (shouldContainAll) {
       if (foundFiles.size < distinctModules.size) {
-        log.error("Not all modules from update were found")
+        log.error("[JPMSUtils] Not all modules from update were found")
-        log.error(s"Returned (${foundFiles.size}): $foundFiles")
+        log.error(
-        log.error(s"Expected: (${distinctModules.size}): $distinctModules")
+          "[JPMSUtils] Ensure libraryDependencies and moduleDependencies are correct"
+        )
+        log.error(s"[JPMSUtils] Returned (${foundFiles.size}): $foundFiles")
+        log.error(
+          s"[JPMSUtils] Expected: (${distinctModules.size}): $distinctModules"
+        )
       }
     }
     foundFiles