From af6d6190ed7226336562814d63884920932175b0 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 5 Dec 2023 11:40:53 +0100 Subject: [PATCH 01/47] logging-service-logback is JPMS module --- build.sbt | 41 ++++++++++++++ .../src/test/java/module-info.java | 5 ++ project/JPMSUtils.scala | 55 ++++++++++++++----- 3 files changed, 87 insertions(+), 14 deletions(-) create mode 100644 lib/scala/logging-service-logback/src/test/java/module-info.java diff --git a/build.sbt b/build.sbt index 58a89da1d176..4fde3df2ee77 100644 --- a/build.sbt +++ b/build.sbt @@ -767,6 +767,47 @@ lazy val `logging-service-logback` = project "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion % "provided" ) ++ logbackPkg ) + // JPMS settings + .settings( + moduleInfos := Seq( + AutomaticModule("org.enso.logging.test") + ), + // This compilation order is required for the module-info.java file to be properly compiled + compileOrder := CompileOrder.JavaThenScala, + Test / javacOptions ++= { + val requiredModulesOnMp = logbackPkg ++ Seq( + "org.slf4j" % "slf4j-api" % slf4jVersion, + ) + val foundModulesOnMp = JPMSUtils.filterModulesFromUpdate( + update.value, + requiredModulesOnMp, + streams.value.log, + shouldContainAll = true + ) + val modulesToPatch = JPMSUtils.filterModulesFromUpdate( + update.value, + Seq( + "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion + ), + streams.value.log, + shouldContainAll = true + ) + val internalProjectsToPatch: ListBuffer[File] = ListBuffer() + internalProjectsToPatch ++= (LocalProject("logging-config") / Compile / productDirectories).value + internalProjectsToPatch ++= (Compile / productDirectories).value + val patchStr = (modulesToPatch ++ internalProjectsToPatch) + .map(_.getAbsolutePath) + .mkString(File.pathSeparator) + + val thisModName = moduleInfos.value.head.moduleName + Seq( + "--module-path", + foundModulesOnMp.map(_.getAbsolutePath).mkString(File.pathSeparator), + "--patch-module", + s"$thisModName=$patchStr", + ) + }, + ) .dependsOn(`logging-config`) .dependsOn(`logging-service`) diff --git a/lib/scala/logging-service-logback/src/test/java/module-info.java b/lib/scala/logging-service-logback/src/test/java/module-info.java new file mode 100644 index 000000000000..bd2e77974a37 --- /dev/null +++ b/lib/scala/logging-service-logback/src/test/java/module-info.java @@ -0,0 +1,5 @@ +module org.enso.logging.test { + requires org.slf4j; + requires ch.qos.logback.classic; + provides org.slf4j.spi.SLF4JServiceProvider with org.enso.logger.TestLogProvider; +} diff --git a/project/JPMSUtils.scala b/project/JPMSUtils.scala index 694c42897dd9..a67742313481 100644 --- a/project/JPMSUtils.scala +++ b/project/JPMSUtils.scala @@ -1,21 +1,12 @@ import sbt.* import sbt.Keys.* -import sbtassembly.Assembly.{Dependency, JarEntry, Library, Project} -import sbtassembly.MergeStrategy - -import java.io.{File, FilenameFilter} -import sbtassembly.CustomMergeStrategy -import xsbti.compile.IncToolOptionsUtil import sbt.internal.inc.{CompileOutput, PlainVirtualFile} +import sbtassembly.Assembly.{Dependency, JarEntry, Project} +import sbtassembly.{CustomMergeStrategy, MergeStrategy} +import xsbti.compile.IncToolOptionsUtil -import java.nio.file.attribute.BasicFileAttributes -import java.nio.file.{ - FileVisitResult, - FileVisitor, - Files, - Path, - SimpleFileVisitor -} +import java.io.File +import java.nio.file.{Files, Path} /** Collection of utility methods dealing with JPMS modules. * The motivation comes from the update of GraalVM to @@ -73,6 +64,42 @@ object JPMSUtils { ret } + /** + * Filters all the requested modules from the given [[UpdateReport]]. + * + * @param updateReport The update report to filter. This is the result of `update.value`. + * @param modules The modules to filter from the update report. + * @param log The logger to use for logging. + * @param shouldContainAll If true, the method will log an error if not all modules were found. + * @return The list of files (Jar archives, directories, etc.) that were found in the update report. + */ + def filterModulesFromUpdate( + updateReport: UpdateReport, + modules: Seq[ModuleID], + log: sbt.util.Logger, + shouldContainAll: Boolean = false + ): Seq[File] = { + def shouldFilterModule(module: ModuleID): Boolean = { + modules.exists(m => + m.organization == module.organization && + m.name == module.name && + m.revision == module.revision + ) + } + + val foundFiles = updateReport.select( + module = shouldFilterModule + ) + if (shouldContainAll) { + if (foundFiles.size < modules.size) { + log.error("Not all modules from update were found") + log.error(s"Returned (${foundFiles.size}): $foundFiles") + log.error(s"Expected: (${modules.size}): $modules") + } + } + foundFiles + } + def filterTruffleAndGraalArtifacts( classPath: Def.Classpath ): Def.Classpath = { From 8f6157ec2c7975c307a1a39c27207f955bc492a3 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 5 Dec 2023 17:39:20 +0100 Subject: [PATCH 02/47] runtime uses truffle-compiler in tests --- build.sbt | 80 ++++++++++++++++++++++++++++++++++++++--- project/JPMSUtils.scala | 3 +- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/build.sbt b/build.sbt index 4fde3df2ee77..f9de50c524d5 100644 --- a/build.sbt +++ b/build.sbt @@ -1460,7 +1460,8 @@ lazy val runtime = (project in file("engine/runtime")) "junit" % "junit" % junitVersion % Test, "com.github.sbt" % "junit-interface" % junitIfVersion % Test, "org.hamcrest" % "hamcrest-all" % hamcrestVersion % Test, - "org.slf4j" % "slf4j-nop" % slf4jVersion % Benchmark + "org.slf4j" % "slf4j-nop" % slf4jVersion % Benchmark, + "org.slf4j" % "slf4j-api" % slf4jVersion % Test ), // Add all GraalVM packages with Runtime scope - we don't need them for compilation, // just provide them at runtime (in module-path). @@ -1473,9 +1474,80 @@ lazy val runtime = (project in file("engine/runtime")) GraalVM.toolsPkgs.map(_.withConfigurations(Some(Runtime.name))) necessaryModules ++ langs ++ tools }, - Test / javaOptions ++= testLogProviderOptions ++ Seq( - "-Dpolyglotimpl.DisableClassPathIsolation=true" - ), + // The javaOptions are created such that the `runtime/test` will run with truffle-compiler + // So we need to properly create --module-path option. + // Module `org.enso.runtime` is added to --module-path as a path to a directory with + // expanded classes, i.e., not a Jar archive. This means we also have to properly create + // --patch-module option. This is an optimization so that we don't have to assembly the + // `runtime.jar` fat jar for the tests. + Test / javaOptions ++= { + val runtimeModName = (LocalProject( + "runtime-with-instruments" + ) / moduleInfos).value.head.moduleName + val runtimeMod = (LocalProject( + "runtime-with-instruments" + ) / Compile / productDirectories).value + val graalMods = graalModulesPaths.value + val loggingMods = JPMSUtils.filterModulesFromUpdate( + (Test / update).value, + logbackPkg ++ Seq( + "org.slf4j" % "slf4j-api" % slf4jVersion + ), + streams.value.log, + shouldContainAll = true + ) + + /** All these modules will be in --patch-module cmdline option to java, which means that + * for the JVM, it will appear that all the classes contained in these sbt projects are contained + * in the `org.enso.runtime` module. In this way, we do not have to assembly the `runtime.jar` + * fat jar. + */ + val modulesToPatchIntoRuntime: ListBuffer[File] = ListBuffer() + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-with-instruments" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-instrument-common" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-instrument-id-execution" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-instrument-repl-debugger" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-instrument-runtime-server" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-language-epb" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-compiler" + ) / Compile / productDirectories).value + val patchStr = modulesToPatchIntoRuntime + .map(_.getAbsolutePath) + .mkString(File.pathSeparator) + val allModulesPaths: Seq[String] = + runtimeMod.map(_.getAbsolutePath) ++ + graalMods.map(_.data.getAbsolutePath) ++ + loggingMods.map(_.getAbsolutePath) + Seq( + // We can't use org.enso.logger.TestLogProvider here because it is not + // in a module, and it cannot be simple wrapped inside a module. + "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", + "--module-path", + allModulesPaths.mkString(File.pathSeparator), + "--add-modules", + runtimeModName, + "--patch-module", + s"$runtimeModName=$patchStr", + "--add-reads", + s"$runtimeModName=ALL-UNNAMED" + ) + }, + Test / unmanagedClasspath := (LocalProject( + "runtime-with-instruments" + ) / Compile / exportedProducts).value, Test / fork := true, Test / envVars ++= distributionEnvironmentOverrides ++ Map( "ENSO_TEST_DISABLE_IR_CACHE" -> "false" diff --git a/project/JPMSUtils.scala b/project/JPMSUtils.scala index a67742313481..bac2c67d7a5b 100644 --- a/project/JPMSUtils.scala +++ b/project/JPMSUtils.scala @@ -64,8 +64,7 @@ object JPMSUtils { ret } - /** - * Filters all the requested modules from the given [[UpdateReport]]. + /** Filters all the requested modules from the given [[UpdateReport]]. * * @param updateReport The update report to filter. This is the result of `update.value`. * @param modules The modules to filter from the update report. From 2f7521ec060b2c5fc1305ebd007eb1392bfc2713 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 5 Dec 2023 17:40:38 +0100 Subject: [PATCH 03/47] Revert "logging-service-logback is JPMS module" --- build.sbt | 89 +++++++++---------- .../src/test/java/module-info.java | 5 -- 2 files changed, 40 insertions(+), 54 deletions(-) delete mode 100644 lib/scala/logging-service-logback/src/test/java/module-info.java diff --git a/build.sbt b/build.sbt index f9de50c524d5..62385beaccbb 100644 --- a/build.sbt +++ b/build.sbt @@ -13,8 +13,11 @@ import src.main.scala.licenses.{ SBTDistributionComponent } import com.sandinh.javamodule.moduleinfo.JpmsModule +import com.sandinh.javamodule.moduleinfo.AutomaticModule +import sbt.librarymanagement.DependencyFilter import java.io.File +import scala.collection.mutable.ListBuffer // ============================================================================ // === Global Configuration =================================================== @@ -519,6 +522,24 @@ lazy val componentModulesPaths = ) } +lazy val graalModulesPaths = + taskKey[Def.Classpath]( + "Gathers all GraalVM modules (Jar archives that should be put on module-path" + + " as files" + ) +(ThisBuild / graalModulesPaths) := { + val runnerCp = (`engine-runner` / Runtime / fullClasspath).value + val runtimeCp = (LocalProject("runtime") / Runtime / fullClasspath).value + val fullCp = (runnerCp ++ runtimeCp).distinct + val log = streams.value.log + JPMSUtils.filterModulesFromClasspath( + fullCp, + GraalVM.modules, + log, + shouldContainAll = true + ) +} + // ============================================================================ // === Internal Libraries ===================================================== // ============================================================================ @@ -767,47 +788,6 @@ lazy val `logging-service-logback` = project "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion % "provided" ) ++ logbackPkg ) - // JPMS settings - .settings( - moduleInfos := Seq( - AutomaticModule("org.enso.logging.test") - ), - // This compilation order is required for the module-info.java file to be properly compiled - compileOrder := CompileOrder.JavaThenScala, - Test / javacOptions ++= { - val requiredModulesOnMp = logbackPkg ++ Seq( - "org.slf4j" % "slf4j-api" % slf4jVersion, - ) - val foundModulesOnMp = JPMSUtils.filterModulesFromUpdate( - update.value, - requiredModulesOnMp, - streams.value.log, - shouldContainAll = true - ) - val modulesToPatch = JPMSUtils.filterModulesFromUpdate( - update.value, - Seq( - "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion - ), - streams.value.log, - shouldContainAll = true - ) - val internalProjectsToPatch: ListBuffer[File] = ListBuffer() - internalProjectsToPatch ++= (LocalProject("logging-config") / Compile / productDirectories).value - internalProjectsToPatch ++= (Compile / productDirectories).value - val patchStr = (modulesToPatch ++ internalProjectsToPatch) - .map(_.getAbsolutePath) - .mkString(File.pathSeparator) - - val thisModName = moduleInfos.value.head.moduleName - Seq( - "--module-path", - foundModulesOnMp.map(_.getAbsolutePath).mkString(File.pathSeparator), - "--patch-module", - s"$thisModName=$patchStr", - ) - }, - ) .dependsOn(`logging-config`) .dependsOn(`logging-service`) @@ -1304,15 +1284,26 @@ lazy val `language-server` = (project in file("engine/language-server")) .settings( // These settings are needed by language-server tests that create a runtime context. Test / fork := true, - Test / javaOptions ++= testLogProviderOptions ++ Seq( - "-Dpolyglot.engine.WarnInterpreterOnly=false", - "-Dpolyglotimpl.DisableClassPathIsolation=true" - ), - // Append enso language on the class-path - Test / unmanagedClasspath := - (LocalProject( + Test / javaOptions ++= testLogProviderOptions ++ { + val runtimeJar = (LocalProject( "runtime-with-instruments" - ) / Compile / fullClasspath).value, + ) / assembly / assemblyOutputPath).value + val log = streams.value.log + val requiredModules = + (Test / internalDependencyClasspath).value ++ (Test / unmanagedClasspath).value + val allModules = requiredModules.map(_.data.getAbsolutePath) ++ Seq( + runtimeJar.getAbsolutePath + ) + Seq( + "--module-path", + allModules.mkString(File.pathSeparator), + "--add-modules", + // TODO: Use JPMSModule name + "org.enso.runtime" + ) + }, + // Append enso language on the class-path + Test / unmanagedClasspath := componentModulesPaths.value, Test / compile := (Test / compile) .dependsOn(LocalProject("enso") / updateLibraryManifests) .value, diff --git a/lib/scala/logging-service-logback/src/test/java/module-info.java b/lib/scala/logging-service-logback/src/test/java/module-info.java deleted file mode 100644 index bd2e77974a37..000000000000 --- a/lib/scala/logging-service-logback/src/test/java/module-info.java +++ /dev/null @@ -1,5 +0,0 @@ -module org.enso.logging.test { - requires org.slf4j; - requires ch.qos.logback.classic; - provides org.slf4j.spi.SLF4JServiceProvider with org.enso.logger.TestLogProvider; -} From 308f8e830cf9e48e2be0b38a353b595d48315e1b Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 5 Dec 2023 18:14:12 +0100 Subject: [PATCH 04/47] Refactor Test/options to a separate task --- build.sbt | 165 +++++++++++++++++++++------------------- project/JPMSUtils.scala | 18 ++--- 2 files changed, 97 insertions(+), 86 deletions(-) diff --git a/build.sbt b/build.sbt index 62385beaccbb..490c70f2a513 100644 --- a/build.sbt +++ b/build.sbt @@ -525,13 +525,13 @@ lazy val componentModulesPaths = lazy val graalModulesPaths = taskKey[Def.Classpath]( "Gathers all GraalVM modules (Jar archives that should be put on module-path" + - " as files" + " as files" ) (ThisBuild / graalModulesPaths) := { - val runnerCp = (`engine-runner` / Runtime / fullClasspath).value + val runnerCp = (`engine-runner` / Runtime / fullClasspath).value val runtimeCp = (LocalProject("runtime") / Runtime / fullClasspath).value - val fullCp = (runnerCp ++ runtimeCp).distinct - val log = streams.value.log + val fullCp = (runnerCp ++ runtimeCp).distinct + val log = streams.value.log JPMSUtils.filterModulesFromClasspath( fullCp, GraalVM.modules, @@ -540,6 +540,87 @@ lazy val graalModulesPaths = ) } +/** The javaOptions are created such that the `runtime/test` will run with truffle-compiler + * So we need to properly create --module-path option. + * Module `org.enso.runtime` is added to --module-path as a path to a directory with + * expanded classes, i.e., not a Jar archive. This means we also have to properly create + * --patch-module option. This is an optimization so that we don't have to assembly the + * `runtime.jar` fat jar for the tests. + */ +lazy val modulePathTestOptions = + taskKey[Seq[String]]( + "Provides javaOptions for running tests with --module-path set such that " + + "enso (`org.enso.runtime`) module and truffle-compiler and the rest of " + + "the GraalVM modules are on module-path. Also makes sure that the `runtime.jar` " + + "fat jar is not rebuilt unnecessarily" + ) +(ThisBuild / modulePathTestOptions) := { + val updateReport = (LocalProject("runtime") / Test / update).value + val runtimeModName = (LocalProject( + "runtime-with-instruments" + ) / moduleInfos).value.head.moduleName + val runtimeMod = (LocalProject( + "runtime-with-instruments" + ) / Compile / productDirectories).value + val graalMods = graalModulesPaths.value + val loggingMods = JPMSUtils.filterModulesFromUpdate( + updateReport, + logbackPkg ++ Seq( + "org.slf4j" % "slf4j-api" % slf4jVersion + ), + streams.value.log, + shouldContainAll = true + ) + + /** All these modules will be in --patch-module cmdline option to java, which means that + * for the JVM, it will appear that all the classes contained in these sbt projects are contained + * in the `org.enso.runtime` module. In this way, we do not have to assembly the `runtime.jar` + * fat jar. + */ + val modulesToPatchIntoRuntime: ListBuffer[File] = ListBuffer() + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-with-instruments" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-instrument-common" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-instrument-id-execution" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-instrument-repl-debugger" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-instrument-runtime-server" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-language-epb" + ) / Compile / productDirectories).value + modulesToPatchIntoRuntime ++= (LocalProject( + "runtime-compiler" + ) / Compile / productDirectories).value + val patchStr = modulesToPatchIntoRuntime + .map(_.getAbsolutePath) + .mkString(File.pathSeparator) + val allModulesPaths: Seq[String] = + runtimeMod.map(_.getAbsolutePath) ++ + graalMods.map(_.data.getAbsolutePath) ++ + loggingMods.map(_.getAbsolutePath) + Seq( + // We can't use org.enso.logger.TestLogProvider here because it is not + // in a module, and it cannot be simple wrapped inside a module. + "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", + "--module-path", + allModulesPaths.mkString(File.pathSeparator), + "--add-modules", + runtimeModName, + "--patch-module", + s"$runtimeModName=$patchStr", + "--add-reads", + s"$runtimeModName=ALL-UNNAMED" + ) +} + // ============================================================================ // === Internal Libraries ===================================================== // ============================================================================ @@ -1451,8 +1532,8 @@ lazy val runtime = (project in file("engine/runtime")) "junit" % "junit" % junitVersion % Test, "com.github.sbt" % "junit-interface" % junitIfVersion % Test, "org.hamcrest" % "hamcrest-all" % hamcrestVersion % Test, - "org.slf4j" % "slf4j-nop" % slf4jVersion % Benchmark, - "org.slf4j" % "slf4j-api" % slf4jVersion % Test + "org.slf4j" % "slf4j-nop" % slf4jVersion % Benchmark, + "org.slf4j" % "slf4j-api" % slf4jVersion % Test ), // Add all GraalVM packages with Runtime scope - we don't need them for compilation, // just provide them at runtime (in module-path). @@ -1465,77 +1546,7 @@ lazy val runtime = (project in file("engine/runtime")) GraalVM.toolsPkgs.map(_.withConfigurations(Some(Runtime.name))) necessaryModules ++ langs ++ tools }, - // The javaOptions are created such that the `runtime/test` will run with truffle-compiler - // So we need to properly create --module-path option. - // Module `org.enso.runtime` is added to --module-path as a path to a directory with - // expanded classes, i.e., not a Jar archive. This means we also have to properly create - // --patch-module option. This is an optimization so that we don't have to assembly the - // `runtime.jar` fat jar for the tests. - Test / javaOptions ++= { - val runtimeModName = (LocalProject( - "runtime-with-instruments" - ) / moduleInfos).value.head.moduleName - val runtimeMod = (LocalProject( - "runtime-with-instruments" - ) / Compile / productDirectories).value - val graalMods = graalModulesPaths.value - val loggingMods = JPMSUtils.filterModulesFromUpdate( - (Test / update).value, - logbackPkg ++ Seq( - "org.slf4j" % "slf4j-api" % slf4jVersion - ), - streams.value.log, - shouldContainAll = true - ) - - /** All these modules will be in --patch-module cmdline option to java, which means that - * for the JVM, it will appear that all the classes contained in these sbt projects are contained - * in the `org.enso.runtime` module. In this way, we do not have to assembly the `runtime.jar` - * fat jar. - */ - val modulesToPatchIntoRuntime: ListBuffer[File] = ListBuffer() - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-with-instruments" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-instrument-common" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-instrument-id-execution" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-instrument-repl-debugger" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-instrument-runtime-server" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-language-epb" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-compiler" - ) / Compile / productDirectories).value - val patchStr = modulesToPatchIntoRuntime - .map(_.getAbsolutePath) - .mkString(File.pathSeparator) - val allModulesPaths: Seq[String] = - runtimeMod.map(_.getAbsolutePath) ++ - graalMods.map(_.data.getAbsolutePath) ++ - loggingMods.map(_.getAbsolutePath) - Seq( - // We can't use org.enso.logger.TestLogProvider here because it is not - // in a module, and it cannot be simple wrapped inside a module. - "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", - "--module-path", - allModulesPaths.mkString(File.pathSeparator), - "--add-modules", - runtimeModName, - "--patch-module", - s"$runtimeModName=$patchStr", - "--add-reads", - s"$runtimeModName=ALL-UNNAMED" - ) - }, + Test / javaOptions := modulePathTestOptions.value, Test / unmanagedClasspath := (LocalProject( "runtime-with-instruments" ) / Compile / exportedProducts).value, diff --git a/project/JPMSUtils.scala b/project/JPMSUtils.scala index bac2c67d7a5b..d0891ec8264f 100644 --- a/project/JPMSUtils.scala +++ b/project/JPMSUtils.scala @@ -65,13 +65,13 @@ object JPMSUtils { } /** Filters all the requested modules from the given [[UpdateReport]]. - * - * @param updateReport The update report to filter. This is the result of `update.value`. - * @param modules The modules to filter from the update report. - * @param log The logger to use for logging. - * @param shouldContainAll If true, the method will log an error if not all modules were found. - * @return The list of files (Jar archives, directories, etc.) that were found in the update report. - */ + * + * @param updateReport The update report to filter. This is the result of `update.value`. + * @param modules The modules to filter from the update report. + * @param log The logger to use for logging. + * @param shouldContainAll If true, the method will log an error if not all modules were found. + * @return The list of files (Jar archives, directories, etc.) that were found in the update report. + */ def filterModulesFromUpdate( updateReport: UpdateReport, modules: Seq[ModuleID], @@ -81,8 +81,8 @@ object JPMSUtils { def shouldFilterModule(module: ModuleID): Boolean = { modules.exists(m => m.organization == module.organization && - m.name == module.name && - m.revision == module.revision + m.name == module.name && + m.revision == module.revision ) } From 5dc857237a86944333959a0c72ef30d46f4ac10e Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 5 Dec 2023 18:41:23 +0100 Subject: [PATCH 05/47] Provide a default logback-test logging config for tests --- build.sbt | 9 +++++++-- .../src/test/resources/logback-test.xml | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 lib/scala/logging-service-logback/src/test/resources/logback-test.xml diff --git a/build.sbt b/build.sbt index 490c70f2a513..4667e2819f3f 100644 --- a/build.sbt +++ b/build.sbt @@ -606,10 +606,15 @@ lazy val modulePathTestOptions = runtimeMod.map(_.getAbsolutePath) ++ graalMods.map(_.data.getAbsolutePath) ++ loggingMods.map(_.getAbsolutePath) + // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not + // in a module, and it cannot be simple wrapped inside a module. + // So we use plain ch.qos.logback with its configuration. + val testLogbackConf = (LocalProject( + "logging-service-logback" + ) / Test / sourceDirectory).value / "resources" / "logback-test.xml" Seq( - // We can't use org.enso.logger.TestLogProvider here because it is not - // in a module, and it cannot be simple wrapped inside a module. "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", + s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}", "--module-path", allModulesPaths.mkString(File.pathSeparator), "--add-modules", diff --git a/lib/scala/logging-service-logback/src/test/resources/logback-test.xml b/lib/scala/logging-service-logback/src/test/resources/logback-test.xml new file mode 100644 index 000000000000..1b879a1f3823 --- /dev/null +++ b/lib/scala/logging-service-logback/src/test/resources/logback-test.xml @@ -0,0 +1,19 @@ + + + + + + [%d{HH:mm:ss.SSS}] [%-5level] [%logger{36}] -%kvp- %msg%n + + + + + WARN + + + + + + + From b99c4a9a2e1baad396f0854b6d5c33c1819df637 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 5 Dec 2023 18:46:02 +0100 Subject: [PATCH 06/47] Do not add runtime-with-instruments to --patch-module --- build.sbt | 3 --- 1 file changed, 3 deletions(-) diff --git a/build.sbt b/build.sbt index 4667e2819f3f..e97ee23e3d1f 100644 --- a/build.sbt +++ b/build.sbt @@ -578,9 +578,6 @@ lazy val modulePathTestOptions = * fat jar. */ val modulesToPatchIntoRuntime: ListBuffer[File] = ListBuffer() - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-with-instruments" - ) / Compile / productDirectories).value modulesToPatchIntoRuntime ++= (LocalProject( "runtime-instrument-common" ) / Compile / productDirectories).value From 9357291e29409f1749270ed41b9b5e245131833b Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 5 Dec 2023 18:48:43 +0100 Subject: [PATCH 07/47] Refactor mutable ListBuffer to List --- build.sbt | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/build.sbt b/build.sbt index e97ee23e3d1f..e4d8d6a2dca6 100644 --- a/build.sbt +++ b/build.sbt @@ -577,25 +577,23 @@ lazy val modulePathTestOptions = * in the `org.enso.runtime` module. In this way, we do not have to assembly the `runtime.jar` * fat jar. */ - val modulesToPatchIntoRuntime: ListBuffer[File] = ListBuffer() - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-instrument-common" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-instrument-id-execution" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-instrument-repl-debugger" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-instrument-runtime-server" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-language-epb" - ) / Compile / productDirectories).value - modulesToPatchIntoRuntime ++= (LocalProject( - "runtime-compiler" - ) / Compile / productDirectories).value + val modulesToPatchIntoRuntime: Seq[File] = + (LocalProject( + "runtime-instrument-common" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-id-execution" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-repl-debugger" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-runtime-server" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-language-epb" + ) / Compile / productDirectories).value ++ + (LocalProject("runtime-compiler") / Compile / productDirectories).value val patchStr = modulesToPatchIntoRuntime .map(_.getAbsolutePath) .mkString(File.pathSeparator) From d3c5048fe888ef00521214999b6ce76dcf5e5727 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 6 Dec 2023 17:49:55 +0100 Subject: [PATCH 08/47] Introduce runtime-fat-jar project. This project bundles all the functionality for assembling the `runtime.jar` fat jar. --- build.sbt | 137 +++++++++++------- .../src/main/java/module-info.java | 0 project/JPMSUtils.scala | 100 ++++++++----- 3 files changed, 149 insertions(+), 88 deletions(-) rename engine/{runtime-with-instruments => runtime-fat-jar}/src/main/java/module-info.java (100%) diff --git a/build.sbt b/build.sbt index e4d8d6a2dca6..f25b8fbd6f8e 100644 --- a/build.sbt +++ b/build.sbt @@ -557,10 +557,10 @@ lazy val modulePathTestOptions = (ThisBuild / modulePathTestOptions) := { val updateReport = (LocalProject("runtime") / Test / update).value val runtimeModName = (LocalProject( - "runtime-with-instruments" + "runtime-fat-jar" ) / moduleInfos).value.head.moduleName val runtimeMod = (LocalProject( - "runtime-with-instruments" + "runtime-fat-jar" ) / Compile / productDirectories).value val graalMods = graalModulesPaths.value val loggingMods = JPMSUtils.filterModulesFromUpdate( @@ -621,6 +621,8 @@ lazy val modulePathTestOptions = ) } +lazy val compileModuleInfo = taskKey[Unit]("Compiles `module-info.java`") + // ============================================================================ // === Internal Libraries ===================================================== // ============================================================================ @@ -1047,7 +1049,7 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) // Append enso language on the class-path Test / unmanagedClasspath := (LocalProject( - "runtime-with-instruments" + "runtime-fat-jar" ) / Compile / fullClasspath).value, // In project-manager tests, we test installing projects and for that, we need // to launch engine-runner properly. For that, we need all the JARs that we @@ -1308,7 +1310,7 @@ lazy val `polyglot-api` = project // Append enso language on the class-path Test / unmanagedClasspath := (LocalProject( - "runtime-with-instruments" + "runtime-fat-jar" ) / Compile / fullClasspath).value, libraryDependencies ++= Seq( "org.graalvm.sdk" % "polyglot-tck" % graalMavenPackagesVersion % "provided", @@ -1367,7 +1369,7 @@ lazy val `language-server` = (project in file("engine/language-server")) Test / fork := true, Test / javaOptions ++= testLogProviderOptions ++ { val runtimeJar = (LocalProject( - "runtime-with-instruments" + "runtime-fat-jar" ) / assembly / assemblyOutputPath).value val log = streams.value.log val requiredModules = @@ -1547,8 +1549,11 @@ lazy val runtime = (project in file("engine/runtime")) necessaryModules ++ langs ++ tools }, Test / javaOptions := modulePathTestOptions.value, + Test / compile := (Test / compile) + .dependsOn(LocalProject("runtime-fat-jar") / Compile / compileModuleInfo) + .value, Test / unmanagedClasspath := (LocalProject( - "runtime-with-instruments" + "runtime-fat-jar" ) / Compile / exportedProducts).value, Test / fork := true, Test / envVars ++= distributionEnvironmentOverrides ++ Map( @@ -1587,7 +1592,7 @@ lazy val runtime = (project in file("engine/runtime")) // runtime.jar fat jar needs to be assembled as it is used in the // benchmarks. This dependency is here so that `runtime/bench` works // after clean build. - LocalProject("runtime-with-instruments") / assembly + LocalProject("runtime-fat-jar") / assembly ) .value, benchOnly := Def.inputTaskDyn { @@ -1723,53 +1728,46 @@ lazy val `runtime-instrument-runtime-server` = .dependsOn(runtime) .dependsOn(`runtime-instrument-common`) -lazy val `runtime-with-instruments` = - (project in file("engine/runtime-with-instruments")) - .configs(Benchmark) +/** A "meta" project that exists solely to provide logic for assembling the `runtime.jar` fat Jar. + * We do not want to put this task into any other existing project, as it internally copies some + * classes from other projects into the `classes` directory, therefore, pollutes the build. + * There is only one Java source in this project - `module-info.java`. During the assembling of the + * fat jar, all the classes from the dependent projects are copied into the `classes` directory of + * this project and then, a custom task is invoked to compile the `module-info.java`. + */ +lazy val `runtime-fat-jar` = + (project in file("engine/runtime-fat-jar")) .settings( - frgaalJavaCompilerSetting, - inConfig(Compile)(truffleRunOptionsSettings), - commands += WithDebugCommand.withDebug, - Test / javaOptions ++= testLogProviderOptions ++ Seq( - "-Dpolyglotimpl.DisableClassPathIsolation=true" - ), - Test / fork := true, - Test / envVars ++= distributionEnvironmentOverrides ++ Map( - "ENSO_TEST_DISABLE_IR_CACHE" -> "false" - ), - libraryDependencies ++= Seq( - "org.scalatest" %% "scalatest" % scalatestVersion % Test, - "org.graalvm.truffle" % "truffle-api" % graalMavenPackagesVersion % Test, - "org.graalvm.truffle" % "truffle-dsl-processor" % graalMavenPackagesVersion % Test, - "org.slf4j" % "slf4j-nop" % slf4jVersion % Benchmark - ), - // Note [Unmanaged Classpath] - Test / unmanagedClasspath += (baseDirectory.value / ".." / ".." / "app" / "gui" / "view" / "graph-editor" / "src" / "builtin" / "visualization" / "native" / "inc"), + Compile / compileModuleInfo := { + JPMSUtils.compileModuleInfo( + copyDepsFilter = ScopeFilter( + inProjects( + LocalProject("runtime"), + LocalProject("runtime-language-epb"), + LocalProject("runtime-instrument-common"), + LocalProject("runtime-instrument-id-execution"), + LocalProject("runtime-instrument-repl-debugger"), + LocalProject("runtime-instrument-runtime-server") + ), + inConfigurations(Compile) + ), + modulePath = JPMSUtils.componentModules + ) + } + .dependsOn(Compile / compile) + .value, // Filter module-info.java from the compilation excludeFilter := excludeFilter.value || "module-info.java", moduleInfos := Seq( JpmsModule("org.enso.runtime") - ) + ), + compileOrder := CompileOrder.JavaThenScala ) /** Assembling Uber Jar */ .settings( assembly := assembly - .dependsOn( - JPMSUtils.compileModuleInfo( - copyDepsFilter = ScopeFilter( - inProjects( - LocalProject("runtime"), - LocalProject("runtime-language-epb"), - LocalProject("runtime-instrument-common"), - LocalProject("runtime-instrument-id-execution"), - LocalProject("runtime-instrument-repl-debugger"), - LocalProject("runtime-instrument-runtime-server") - ), - inConfigurations(Compile) - ), - modulePath = JPMSUtils.componentModules - ) - ) + .dependsOn(Compile / compile) + .dependsOn(Compile / compileModuleInfo) .value, assembly / assemblyJarName := "runtime.jar", assembly / test := {}, @@ -1798,6 +1796,43 @@ lazy val `runtime-with-instruments` = case _ => MergeStrategy.first } ) + .dependsOn(`runtime-instrument-common`) + .dependsOn(`runtime-instrument-id-execution`) + .dependsOn(`runtime-instrument-repl-debugger`) + .dependsOn(`runtime-instrument-runtime-server`) + .dependsOn(`runtime-language-epb`) + .dependsOn(runtime) + +lazy val `runtime-with-instruments` = + (project in file("engine/runtime-with-instruments")) + .configs(Benchmark) + .settings( + frgaalJavaCompilerSetting, + inConfig(Compile)(truffleRunOptionsSettings), + commands += WithDebugCommand.withDebug, + Test / javaOptions ++= testLogProviderOptions ++ Seq( + "-Dpolyglotimpl.DisableClassPathIsolation=true" + ), + Test / fork := true, + Test / envVars ++= distributionEnvironmentOverrides ++ Map( + "ENSO_TEST_DISABLE_IR_CACHE" -> "false" + ), + libraryDependencies ++= Seq( + "org.scalatest" %% "scalatest" % scalatestVersion % Test, + "org.graalvm.truffle" % "truffle-api" % graalMavenPackagesVersion % Test, + "org.graalvm.truffle" % "truffle-dsl-processor" % graalMavenPackagesVersion % Test, + "org.slf4j" % "slf4j-nop" % slf4jVersion % Benchmark + ), + // Note [Unmanaged Classpath] + Test / unmanagedClasspath += (baseDirectory.value / ".." / ".." / "app" / "gui" / "view" / "graph-editor" / "src" / "builtin" / "visualization" / "native" / "inc"), + assembly := { + streams.value.log.warn( + s"This is an empty task, use `runtime-fat-jar/assembly` instead." + ) + // Return an empty file to satisfy the type checker (the assembly task expect File as a return type) + file("") + } + ) /** Benchmark settings */ .settings( inConfig(Benchmark)(Defaults.testSettings), @@ -1927,7 +1962,7 @@ lazy val `engine-runner` = project ) .settings( assembly := assembly - .dependsOn(`runtime-with-instruments` / assembly) + .dependsOn(`runtime-fat-jar` / assembly) .value, rebuildNativeImage := NativeImage @@ -2110,7 +2145,7 @@ lazy val `bench-processor` = (project in file("lib/scala/bench-processor")) ), // Append enso language on the class-path (Test / unmanagedClasspath) := - (LocalProject("runtime-with-instruments") / Compile / fullClasspath).value + (LocalProject("runtime-fat-jar") / Compile / fullClasspath).value ) .dependsOn(`polyglot-api`) .dependsOn(runtime) @@ -2149,11 +2184,11 @@ lazy val `std-benchmarks` = (project in file("std-bits/benchmarks")) (Benchmark / parallelExecution) := false, (Benchmark / run / fork) := true, (Benchmark / run / connectInput) := true, - // This ensures that the full class-path of runtime-with-instruments is put on + // This ensures that the full class-path of runtime-fat-jar is put on // class-path of the Java compiler (and thus the benchmark annotation processor). (Benchmark / compile / unmanagedClasspath) ++= (LocalProject( - "runtime-with-instruments" + "runtime-fat-jar" ) / Compile / fullClasspath).value, (Benchmark / compile / javacOptions) ++= Seq( "-s", @@ -2177,12 +2212,12 @@ lazy val `std-benchmarks` = (project in file("std-bits/benchmarks")) .map(_.data.getAbsolutePath) val runtimeJar = (LocalProject( - "runtime-with-instruments" + "runtime-fat-jar" ) / assembly / assemblyOutputPath).value.getAbsolutePath val allModulePaths = requiredModulesPaths ++ Seq(runtimeJar) val runtimeModuleName = (LocalProject( - "runtime-with-instruments" + "runtime-fat-jar" ) / moduleInfos).value.head.moduleName Seq( // To enable logging in benchmarks, add ch.qos.logback module on the modulePath diff --git a/engine/runtime-with-instruments/src/main/java/module-info.java b/engine/runtime-fat-jar/src/main/java/module-info.java similarity index 100% rename from engine/runtime-with-instruments/src/main/java/module-info.java rename to engine/runtime-fat-jar/src/main/java/module-info.java diff --git a/project/JPMSUtils.scala b/project/JPMSUtils.scala index d0891ec8264f..23a989b33ffe 100644 --- a/project/JPMSUtils.scala +++ b/project/JPMSUtils.scala @@ -176,45 +176,24 @@ object JPMSUtils { // copied to. val outputPath: Path = output.getSingleOutputAsPath .get() - - /** Copy classes into the target directory from all the dependencies */ - log.debug(s"Copying classes to $output") + // Class directories of all the dependencies. val sourceProducts = products.all(copyDepsFilter).value.flatten + log.info(s"Compiling $moduleInfo with javac") - if (!(outputPath.toFile.exists())) { - Files.createDirectory(outputPath) - } - - val outputLangProvider = - outputPath / "META-INF" / "services" / "com.oracle.truffle.api.provider.TruffleLanguageProvider" - sourceProducts.foreach { sourceProduct => - log.debug(s"Copying ${sourceProduct} to ${output}") - val sourceLangProvider = - sourceProduct / "META-INF" / "services" / "com.oracle.truffle.api.provider.TruffleLanguageProvider" - if ( - outputLangProvider.toFile.exists() && sourceLangProvider.exists() - ) { - log.debug( - s"Merging ${sourceLangProvider} into ${outputLangProvider}" + // Ensure that all source product directories exist. + sourceProducts.foreach(sourceProduct => { + if (!sourceProduct.exists()) { + log.error(s"Source product ${sourceProduct} does not exist") + log.error( + "This means that the Compile/compile task was probably not run in " + + "the corresponding project" ) - val sourceLines = IO.readLines(sourceLangProvider) - val destLines = IO.readLines(outputLangProvider.toFile) - val outLines = (sourceLines ++ destLines).distinct - IO.writeLines(outputLangProvider.toFile, outLines) + log.error("Run Compile/compile before this task") } - // Copy the rest of the directory - don't override META-INF. - IO.copyDirectory( - sourceProduct, - outputPath.toFile, - CopyOptions( - overwrite = false, - preserveLastModified = true, - preserveExecutable = true - ) - ) - } + }) + + copyClasses(sourceProducts, output, log) - log.info("Compiling module-info.java with javac") val baseJavacOpts = (Compile / javacOptions).value val fullCp = (Compile / fullClasspath).value val (mp, cp) = fullCp.partition(file => { @@ -233,8 +212,10 @@ object JPMSUtils { "--module-path", mp.map(_.data.getAbsolutePath).mkString(File.pathSeparator), "-d", - outputPath.toAbsolutePath().toString() + outputPath.toAbsolutePath.toString ) + log.debug(s"javac options: $allOpts") + val javaCompiler = (Compile / compile / compilers).value.javaTools.javac() val succ = javaCompiler.run( @@ -246,9 +227,54 @@ object JPMSUtils { log ) if (!succ) { - sys.error(s"Compilation of ${moduleInfo} failed") + log.error(s"Compilation of ${moduleInfo} failed") } } - .dependsOn(Compile / compile) + /** Copies all classes from all the dependencies `classes` directories into the target directory. + * @param sourceProducts From where the classes will be copied. + * @param output Target directory where all the classes from all the dependencies + * will be copied to. + * @param log + */ + private def copyClasses( + sourceProducts: Seq[File], + output: xsbti.compile.Output, + log: Logger + ): Unit = { + // TODO: Introduce some file timestamp checking - do not copy the classes every time + val outputPath: Path = output.getSingleOutputAsPath.get() + log.info(s"Copying classes from ${sourceProducts.size} projects to $output") + + if (!(outputPath.toFile.exists())) { + Files.createDirectory(outputPath) + } + + val outputLangProvider = + outputPath / "META-INF" / "services" / "com.oracle.truffle.api.provider.TruffleLanguageProvider" + sourceProducts.foreach { sourceProduct => + log.debug(s"Copying ${sourceProduct} to ${output}") + val sourceLangProvider = + sourceProduct / "META-INF" / "services" / "com.oracle.truffle.api.provider.TruffleLanguageProvider" + if (outputLangProvider.toFile.exists() && sourceLangProvider.exists()) { + log.debug( + s"Merging ${sourceLangProvider} into ${outputLangProvider}" + ) + val sourceLines = IO.readLines(sourceLangProvider) + val destLines = IO.readLines(outputLangProvider.toFile) + val outLines = (sourceLines ++ destLines).distinct + IO.writeLines(outputLangProvider.toFile, outLines) + } + // Copy the rest of the directory - don't override META-INF. + IO.copyDirectory( + sourceProduct, + outputPath.toFile, + CopyOptions( + overwrite = false, + preserveLastModified = true, + preserveExecutable = true + ) + ) + } + } } From 3e0c5318acefb87ede9bf53fb6d754cf1ab7a587 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 7 Dec 2023 12:47:55 +0100 Subject: [PATCH 09/47] compileModuleInfo task does not copy anything unless necessary --- project/JPMSUtils.scala | 107 ++++++++++++++++++++++++++-------------- 1 file changed, 70 insertions(+), 37 deletions(-) diff --git a/project/JPMSUtils.scala b/project/JPMSUtils.scala index 23a989b33ffe..4ea9da1f4c65 100644 --- a/project/JPMSUtils.scala +++ b/project/JPMSUtils.scala @@ -1,12 +1,22 @@ +import com.sandinh.javamodule.moduleinfo.ModuleInfoPlugin.autoImport.moduleInfos import sbt.* import sbt.Keys.* import sbt.internal.inc.{CompileOutput, PlainVirtualFile} +import sbt.util.CacheStore import sbtassembly.Assembly.{Dependency, JarEntry, Project} import sbtassembly.{CustomMergeStrategy, MergeStrategy} import xsbti.compile.IncToolOptionsUtil import java.io.File -import java.nio.file.{Files, Path} +import java.nio.file.attribute.BasicFileAttributes +import java.nio.file.{ + FileVisitOption, + FileVisitResult, + Files, + Path, + SimpleFileVisitor +} +import scala.collection.mutable /** Collection of utility methods dealing with JPMS modules. * The motivation comes from the update of GraalVM to @@ -177,10 +187,13 @@ object JPMSUtils { val outputPath: Path = output.getSingleOutputAsPath .get() // Class directories of all the dependencies. - val sourceProducts = products.all(copyDepsFilter).value.flatten + val sourceProducts = + productDirectories.all(copyDepsFilter).value.flatten log.info(s"Compiling $moduleInfo with javac") - // Ensure that all source product directories exist. + val moduleName = moduleInfos.value.head.moduleName + val cacheStore = streams.value.cacheStoreFactory + val repoRootDir = (LocalProject("enso") / baseDirectory).value sourceProducts.foreach(sourceProduct => { if (!sourceProduct.exists()) { log.error(s"Source product ${sourceProduct} does not exist") @@ -190,10 +203,11 @@ object JPMSUtils { ) log.error("Run Compile/compile before this task") } + val relPath = repoRootDir.toPath.relativize(sourceProduct.toPath) + val cache = cacheStore.make("cache-" + relPath.toString) + copyClasses(sourceProduct, output, cache, log) }) - copyClasses(sourceProducts, output, log) - val baseJavacOpts = (Compile / javacOptions).value val fullCp = (Compile / fullClasspath).value val (mp, cp) = fullCp.partition(file => { @@ -232,49 +246,68 @@ object JPMSUtils { } /** Copies all classes from all the dependencies `classes` directories into the target directory. - * @param sourceProducts From where the classes will be copied. + * @param sourceClassesDir Directory from where the classes will be copied. * @param output Target directory where all the classes from all the dependencies * will be copied to. * @param log */ private def copyClasses( - sourceProducts: Seq[File], + sourceClassesDir: File, output: xsbti.compile.Output, + cache: CacheStore, log: Logger ): Unit = { - // TODO: Introduce some file timestamp checking - do not copy the classes every time + require(sourceClassesDir.isDirectory) val outputPath: Path = output.getSingleOutputAsPath.get() - log.info(s"Copying classes from ${sourceProducts.size} projects to $output") - - if (!(outputPath.toFile.exists())) { - Files.createDirectory(outputPath) - } + val outputDir = outputPath.toFile + val filesToCopy = mutable.HashSet.empty[File] + val fileVisitor = new SimpleFileVisitor[Path] { + override def visitFile( + path: Path, + attrs: BasicFileAttributes + ): FileVisitResult = { + if (!path.toFile.isDirectory) { + filesToCopy.add(path.toFile) + } + FileVisitResult.CONTINUE + } - val outputLangProvider = - outputPath / "META-INF" / "services" / "com.oracle.truffle.api.provider.TruffleLanguageProvider" - sourceProducts.foreach { sourceProduct => - log.debug(s"Copying ${sourceProduct} to ${output}") - val sourceLangProvider = - sourceProduct / "META-INF" / "services" / "com.oracle.truffle.api.provider.TruffleLanguageProvider" - if (outputLangProvider.toFile.exists() && sourceLangProvider.exists()) { - log.debug( - s"Merging ${sourceLangProvider} into ${outputLangProvider}" - ) - val sourceLines = IO.readLines(sourceLangProvider) - val destLines = IO.readLines(outputLangProvider.toFile) - val outLines = (sourceLines ++ destLines).distinct - IO.writeLines(outputLangProvider.toFile, outLines) + override def preVisitDirectory( + dir: Path, + attrs: BasicFileAttributes + ): FileVisitResult = { + // We do not care about files in META-INF directory. Everything should be described + // in the `module-info.java`. + if (dir.getFileName.toString == "META-INF") { + FileVisitResult.SKIP_SUBTREE + } else { + FileVisitResult.CONTINUE + } } - // Copy the rest of the directory - don't override META-INF. - IO.copyDirectory( - sourceProduct, - outputPath.toFile, - CopyOptions( - overwrite = false, - preserveLastModified = true, - preserveExecutable = true - ) - ) + } + Files.walkFileTree(sourceClassesDir.toPath, fileVisitor) + if (!outputDir.exists()) { + IO.createDirectory(outputDir) + } + Tracked.diffInputs(cache, FileInfo.lastModified)(filesToCopy.toSet) { + changeReport => + for (f <- changeReport.removed) { + val relPath = sourceClassesDir.toPath.relativize(f.toPath) + val dest = outputDir.toPath.resolve(relPath) + IO.delete(dest.toFile) + } + for (f <- changeReport.modified -- changeReport.removed) { + val relPath = sourceClassesDir.toPath.relativize(f.toPath) + val dest = outputDir.toPath.resolve(relPath) + IO.copyFile(f, dest.toFile) + } + for (f <- changeReport.unmodified) { + val relPath = sourceClassesDir.toPath.relativize(f.toPath) + val dest = outputDir.toPath.resolve(relPath) + if (!dest.toFile.exists()) { + IO.copyFile(f, dest.toFile) + } + } } } } From 1c516be49377c164b98a430777d96b1ad6e38872 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 7 Dec 2023 15:29:33 +0100 Subject: [PATCH 10/47] JPMSUtils.filterModules filters only distinct modules --- project/JPMSUtils.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/project/JPMSUtils.scala b/project/JPMSUtils.scala index 89da8b74cce3..08d505f4d61b 100644 --- a/project/JPMSUtils.scala +++ b/project/JPMSUtils.scala @@ -80,7 +80,7 @@ object JPMSUtils { /** Filters all the requested modules from the given [[UpdateReport]]. * * @param updateReport The update report to filter. This is the result of `update.value`. - * @param modules The modules to filter from the update report. + * @param modules The modules to filter from the update report. Can be duplicated. * @param log The logger to use for logging. * @param shouldContainAll If true, the method will log an error if not all modules were found. * @return The list of files (Jar archives, directories, etc.) that were found in the update report. @@ -91,8 +91,10 @@ object JPMSUtils { log: sbt.util.Logger, shouldContainAll: Boolean = false ): Seq[File] = { + val distinctModules = modules.distinct + def shouldFilterModule(module: ModuleID): Boolean = { - modules.exists(m => + distinctModules.exists(m => m.organization == module.organization && m.name == module.name && m.revision == module.revision @@ -103,10 +105,10 @@ object JPMSUtils { module = shouldFilterModule ) if (shouldContainAll) { - if (foundFiles.size < modules.size) { + if (foundFiles.size < distinctModules.size) { log.error("Not all modules from update were found") log.error(s"Returned (${foundFiles.size}): $foundFiles") - log.error(s"Expected: (${modules.size}): $modules") + log.error(s"Expected: (${distinctModules.size}): $distinctModules") } } foundFiles From 0dd005627af65cb7e6babfa2bcedf86a2a52df31 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 7 Dec 2023 15:30:09 +0100 Subject: [PATCH 11/47] Make sure that all GraalVM modules (including truffle langs) are in --module-path in runtime/test --- build.sbt | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 1491418e3560..614c6bb532de 100644 --- a/build.sbt +++ b/build.sbt @@ -555,7 +555,7 @@ lazy val modulePathTestOptions = "fat jar is not rebuilt unnecessarily" ) (ThisBuild / modulePathTestOptions) := { - val updateReport = (LocalProject("runtime") / Test / update).value + val updateReport = (LocalProject("runtime-fat-jar") / Runtime / update).value val runtimeModName = (LocalProject( "runtime-fat-jar" ) / moduleInfos).value.head.moduleName @@ -563,6 +563,12 @@ lazy val modulePathTestOptions = "runtime-fat-jar" ) / Compile / productDirectories).value val graalMods = graalModulesPaths.value + val graalLangMods = JPMSUtils.filterModulesFromUpdate( + updateReport, + GraalVM.langsPkgs, + streams.value.log, + shouldContainAll = true + ) val loggingMods = JPMSUtils.filterModulesFromUpdate( updateReport, logbackPkg ++ Seq( @@ -600,6 +606,7 @@ lazy val modulePathTestOptions = val allModulesPaths: Seq[String] = runtimeMod.map(_.getAbsolutePath) ++ graalMods.map(_.data.getAbsolutePath) ++ + graalLangMods.map(_.getAbsolutePath) ++ loggingMods.map(_.getAbsolutePath) // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not // in a module, and it cannot be simple wrapped inside a module. @@ -1760,7 +1767,24 @@ lazy val `runtime-fat-jar` = moduleInfos := Seq( JpmsModule("org.enso.runtime") ), - compileOrder := CompileOrder.JavaThenScala + compileOrder := CompileOrder.JavaThenScala, + ) + /** + * The following libraryDependencies are provided in Runtime scope. + * Later, we will collect them into --module-path option. + * We don't collect them in Compile scope as it does not even make sense + * to run `compile` task in this project. + */ + .settings( + libraryDependencies ++= { + val graalMods = + GraalVM.modules.map(_.withConfigurations(Some(Runtime.name))) + val langMods = + GraalVM.langsPkgs.map(_.withConfigurations(Some(Runtime.name))) + val logbackMods = + logbackPkg.map(_.withConfigurations(Some(Runtime.name)) ) + graalMods ++ langMods ++ logbackMods + }, ) /** Assembling Uber Jar */ .settings( From 4a9c9629c68bbc58e000183d7683831b1d664770 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 8 Dec 2023 17:40:58 +0100 Subject: [PATCH 12/47] Create new module org.enso.interpreter.test This module will contain all the truffle instruments required for testing --- build.sbt | 77 ++++++++++++++++--- .../src/main/java/module-info.java | 7 ++ .../instrument}/CodeIdsTestInstrument.java | 57 +++++++++++--- .../CodeLocationsTestInstrument.java | 2 +- .../test/instrument/BuiltinTypesTest.scala | 4 + .../instrument/RuntimeAsyncCommandsTest.scala | 1 + .../instrument/RuntimeComponentsTest.scala | 4 + .../test/instrument/RuntimeErrorsTest.scala | 4 + .../RuntimeExecutionEnvironmentTest.scala | 5 ++ .../instrument/RuntimeInstrumentTest.scala | 4 + .../instrument/RuntimeRefactoringTest.scala | 4 + .../test/instrument/RuntimeServerTest.scala | 4 + .../test/instrument/RuntimeStdlibTest.scala | 4 + .../RuntimeSuggestionUpdatesTest.scala | 1 + .../RuntimeVisualizationsTest.scala | 4 + .../interpreter/test/InterpreterTest.scala | 8 +- .../InstrumentTestContext.scala | 2 +- .../RuntimeServerEmulator.scala | 2 +- .../TestEdition.scala | 2 +- .../TestMessages.scala | 2 +- .../TestRuntimeServerConnector.scala | 2 +- 21 files changed, 168 insertions(+), 32 deletions(-) create mode 100644 engine/runtime-test-instruments/src/main/java/module-info.java rename engine/{runtime/src/test/java/org/enso/interpreter/test => runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument}/CodeIdsTestInstrument.java (67%) rename engine/{runtime/src/test/java/org/enso/interpreter/test => runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument}/CodeLocationsTestInstrument.java (99%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instrument => instruments}/InstrumentTestContext.scala (98%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instrument => instruments}/RuntimeServerEmulator.scala (98%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instrument => instruments}/TestEdition.scala (95%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instrument => instruments}/TestMessages.scala (99%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instrument => instruments}/TestRuntimeServerConnector.scala (96%) diff --git a/build.sbt b/build.sbt index 614c6bb532de..2b829cedf7b2 100644 --- a/build.sbt +++ b/build.sbt @@ -562,6 +562,10 @@ lazy val modulePathTestOptions = val runtimeMod = (LocalProject( "runtime-fat-jar" ) / Compile / productDirectories).value + val runtimeTestInstrModName = + (`runtime-test-instruments` / moduleInfos).value.head.moduleName + val runtimeTestInstrMod = + (`runtime-test-instruments` / Compile / productDirectories).value val graalMods = graalModulesPaths.value val graalLangMods = JPMSUtils.filterModulesFromUpdate( updateReport, @@ -605,9 +609,14 @@ lazy val modulePathTestOptions = .mkString(File.pathSeparator) val allModulesPaths: Seq[String] = runtimeMod.map(_.getAbsolutePath) ++ + runtimeTestInstrMod.map(_.getAbsolutePath) ++ graalMods.map(_.data.getAbsolutePath) ++ graalLangMods.map(_.getAbsolutePath) ++ loggingMods.map(_.getAbsolutePath) + val modulesToAdd = Seq( + runtimeModName, + runtimeTestInstrModName + ) // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not // in a module, and it cannot be simple wrapped inside a module. // So we use plain ch.qos.logback with its configuration. @@ -620,11 +629,11 @@ lazy val modulePathTestOptions = "--module-path", allModulesPaths.mkString(File.pathSeparator), "--add-modules", - runtimeModName, + modulesToAdd.mkString(","), "--patch-module", s"$runtimeModName=$patchStr", "--add-reads", - s"$runtimeModName=ALL-UNNAMED" + s"$runtimeModName=ALL-UNNAMED,$runtimeTestInstrModName" ) } @@ -1503,6 +1512,40 @@ lazy val `runtime-language-epb` = ) .dependsOn(`polyglot-api`) +/** This project contains only Truffle instruments that are used in various tests in `runtime-*` + * projects. + */ +lazy val `runtime-test-instruments` = + (project in file("engine/runtime-test-instruments")) + .settings( + inConfig(Compile)(truffleRunOptionsSettings), + instrumentationSettings, + // Needed to compile module-info.java + compileOrder := xsbti.compile.CompileOrder.JavaThenScala, + moduleInfos := Seq( + JpmsModule("org.enso.runtime.test.instrument") + ), + libraryDependencies ++= { + GraalVM.modules.map(_.withConfigurations(Some(Compile.name))) + }, + Compile / javacOptions ++= { + val updateReport = (Compile / update).value + val graalVmMods = JPMSUtils.filterModulesFromUpdate( + updateReport, + GraalVM.modules, + streams.value.log, + shouldContainAll = true + ) + //val runtimeMod = (`runtime-fat-jar` / Compile / exportedProducts).value.head.data + //val allRequiredMods = graalVmMods ++ Seq(runtimeMod) + val allRequiredMods = graalVmMods + Seq( + "--module-path", + allRequiredMods.map(_.getAbsolutePath).mkString(File.pathSeparator) + ) + } + ) + lazy val runtime = (project in file("engine/runtime")) .configs(Benchmark) .settings( @@ -1555,7 +1598,17 @@ lazy val runtime = (project in file("engine/runtime")) GraalVM.toolsPkgs.map(_.withConfigurations(Some(Runtime.name))) necessaryModules ++ langs ++ tools }, - Test / javaOptions := modulePathTestOptions.value, + Test / javaOptions := { + val prevOpts = modulePathTestOptions.value + // Append the `test-classes` directory of the current project to the `--patch-module` option. + // Otherwise, the test frameworks would be unable to load some testing classes as they are + // in the same package as some implementation classes (split packages). + val patchIdx = prevOpts.indexOf("--patch-module") + 1 + val testClassesDir = (Test / productDirectories).value.head + val newPatchOpt = + prevOpts(patchIdx) + File.pathSeparator + testClassesDir.getAbsolutePath + prevOpts.updated(patchIdx, newPatchOpt) + }, Test / compile := (Test / compile) .dependsOn(LocalProject("runtime-fat-jar") / Compile / compileModuleInfo) .value, @@ -1639,6 +1692,7 @@ lazy val runtime = (project in file("engine/runtime")) .dependsOn(`connected-lock-manager`) .dependsOn(testkit % Test) .dependsOn(`logging-service-logback` % "test->test") + .dependsOn(`runtime-test-instruments` % "test->compile") lazy val `runtime-parser` = (project in file("engine/runtime-parser")) @@ -1767,14 +1821,13 @@ lazy val `runtime-fat-jar` = moduleInfos := Seq( JpmsModule("org.enso.runtime") ), - compileOrder := CompileOrder.JavaThenScala, + compileOrder := CompileOrder.JavaThenScala ) - /** - * The following libraryDependencies are provided in Runtime scope. - * Later, we will collect them into --module-path option. - * We don't collect them in Compile scope as it does not even make sense - * to run `compile` task in this project. - */ + /** The following libraryDependencies are provided in Runtime scope. + * Later, we will collect them into --module-path option. + * We don't collect them in Compile scope as it does not even make sense + * to run `compile` task in this project. + */ .settings( libraryDependencies ++= { val graalMods = @@ -1782,9 +1835,9 @@ lazy val `runtime-fat-jar` = val langMods = GraalVM.langsPkgs.map(_.withConfigurations(Some(Runtime.name))) val logbackMods = - logbackPkg.map(_.withConfigurations(Some(Runtime.name)) ) + logbackPkg.map(_.withConfigurations(Some(Runtime.name))) graalMods ++ langMods ++ logbackMods - }, + } ) /** Assembling Uber Jar */ .settings( diff --git a/engine/runtime-test-instruments/src/main/java/module-info.java b/engine/runtime-test-instruments/src/main/java/module-info.java new file mode 100644 index 000000000000..4eea2327a066 --- /dev/null +++ b/engine/runtime-test-instruments/src/main/java/module-info.java @@ -0,0 +1,7 @@ +module org.enso.runtime.test.instrument { + requires org.graalvm.truffle; + exports org.enso.interpreter.test.instrument; + provides com.oracle.truffle.api.instrumentation.provider.TruffleInstrumentProvider with + org.enso.interpreter.test.instrument.CodeLocationsTestInstrumentProvider, + org.enso.interpreter.test.instrument.CodeIdsTestInstrumentProvider; +} diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/CodeIdsTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeIdsTestInstrument.java similarity index 67% rename from engine/runtime/src/test/java/org/enso/interpreter/test/CodeIdsTestInstrument.java rename to engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeIdsTestInstrument.java index da5801309ad4..4d0556ce02c7 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/CodeIdsTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeIdsTestInstrument.java @@ -1,12 +1,12 @@ -package org.enso.interpreter.test; +package org.enso.interpreter.test.instrument; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.*; import com.oracle.truffle.api.nodes.Node; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.LinkedHashMap; import java.util.Map; -import org.enso.interpreter.node.ExpressionNode; -import org.enso.interpreter.runtime.control.TailCallException; import java.util.UUID; @@ -82,11 +82,34 @@ public ExecutionEventNode create(EventContext context) { return node; } + /** + * Uses reflection to get access to {@link org.enso.interpreter.node.ExpressionNode} and + * {@link org.enso.interpreter.runtime.control.TailCallException} classes. We need this because + * this instrument is not a part of the {@code org.enso.runtime} module, and thus cannot import these classes. + * It is part of {@code org.enso.runtime.test.instrument} module, which just provides few instruments + * for testing of the {@code org.enso.runtime} module. + * + * This is a hack to make the unit testing work and to remove the compile-time dependency on the + * {@code org.enso.runtime} module. + */ private final class IdEventNode extends ExecutionEventNode { private final EventContext context; + private final Class expressionNodeClass; + private final Method expressionNodeGetIdMethod; + private final Class tailCallExceptionClass; IdEventNode(EventContext context) { this.context = context; + try { + // Are there two ExpressionNode classes at this point? One loaded here and one loaded inside + // the runtime module? + this.expressionNodeClass = Class.forName("org.enso.interpreter.node.ExpressionNode"); + this.tailCallExceptionClass = + Class.forName("org.enso.interpreter.runtime.control.TailCallException"); + this.expressionNodeGetIdMethod = expressionNodeClass.getDeclaredMethod("getId"); + } catch (ClassNotFoundException | NoSuchMethodException e) { + throw new AssertionError(e); + } } @Override @@ -95,7 +118,6 @@ public void onEnter(VirtualFrame frame) {} /** * Checks if the node to be executed is the node this listener was created to observe. * - * @param context current execution context * @param frame current execution frame * @param result the result of executing the node */ @@ -105,11 +127,11 @@ public void onReturnValue(VirtualFrame frame, Object result) { return; } Node node = context.getInstrumentedNode(); - if (!(node instanceof ExpressionNode)) { + if (!expressionNodeClass.isAssignableFrom(node.getClass())) { return; } nodes.put(this, result); - UUID id = ((ExpressionNode) node).getId(); + UUID id = getIdFromExpressionNode(node); if (id == null || !id.equals(expectedId)) { return; } @@ -118,22 +140,32 @@ public void onReturnValue(VirtualFrame frame, Object result) { } } + private UUID getIdFromExpressionNode(Node expressionNode) { + assert expressionNodeClass.isAssignableFrom(expressionNode.getClass()); + Object res = null; + try { + res = expressionNodeGetIdMethod.invoke(expressionNode); + } catch (IllegalAccessException | InvocationTargetException e) { + throw new AssertionError(e); + } + return (UUID) res; + } + /** * Checks if the specified was called, if its execution triggered TCO. * - * @param context current execution context. * @param frame current execution frame. * @param exception the exception thrown from this node's execution. */ @Override public void onReturnExceptional(VirtualFrame frame, Throwable exception) { - if (!(exception instanceof TailCallException)) { + if (!(exception.getClass().equals(tailCallExceptionClass))) { return; } - if (!(context.getInstrumentedNode() instanceof ExpressionNode)) { + if (!(expressionNodeClass.isAssignableFrom(context.getInstrumentedNode().getClass()))) { return; } - UUID id = ((ExpressionNode) context.getInstrumentedNode()).getId(); + UUID id = getIdFromExpressionNode(context.getInstrumentedNode()); if (expectedResult == null) { successful = true; } @@ -143,8 +175,9 @@ public void onReturnExceptional(VirtualFrame frame, Throwable exception) { public String toString() { var sb = new StringBuilder(); sb.append(context.getInstrumentedNode().getClass().getSimpleName()); - if (context.getInstrumentedNode() instanceof ExpressionNode expr) { - sb.append("@").append(expr.getId()); + if (expressionNodeClass.isAssignableFrom(context.getInstrumentedNode().getClass())) { + UUID id = getIdFromExpressionNode(context.getInstrumentedNode()); + sb.append("@").append(id); } sb.append(" "); sb.append(context.getInstrumentedSourceSection()); diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeLocationsTestInstrument.java similarity index 99% rename from engine/runtime/src/test/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java rename to engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeLocationsTestInstrument.java index 776f57e8b83b..bf217c1ef7a8 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeLocationsTestInstrument.java @@ -1,4 +1,4 @@ -package org.enso.interpreter.test; +package org.enso.interpreter.test.instrument; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.EventBinding; diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/BuiltinTypesTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/BuiltinTypesTest.scala index e8e58d73665b..8c743db91ce0 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/BuiltinTypesTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/BuiltinTypesTest.scala @@ -2,6 +2,10 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata +import org.enso.interpreter.test.instruments.{ + InstrumentTestContext, + TestMessages +} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.graalvm.polyglot.Context diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeAsyncCommandsTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeAsyncCommandsTest.scala index 2fa8519021c8..2b3b9d43d370 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeAsyncCommandsTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeAsyncCommandsTest.scala @@ -1,6 +1,7 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.test.Metadata +import org.enso.interpreter.test.instruments.InstrumentTestContext import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.enso.text.{ContentVersion, Sha3_224VersionCalculator} diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala index 9d4e2fddc052..f589746215be 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala @@ -3,6 +3,10 @@ package org.enso.interpreter.test.instrument import org.enso.editions.LibraryName import org.enso.interpreter.runtime import org.enso.interpreter.test.Metadata +import org.enso.interpreter.test.instruments.{ + InstrumentTestContext, + TestEdition +} import org.enso.pkg.{ ComponentGroup, ComponentGroups, diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala index 97bb92fd34e6..cdcf6a5a4989 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala @@ -2,6 +2,10 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata +import org.enso.interpreter.test.instruments.{ + InstrumentTestContext, + TestMessages +} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.enso.text.editing.model diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala index f1ac5c801e57..3a39b64e00a1 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala @@ -3,6 +3,11 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.EnsoContext import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata +import org.enso.interpreter.test.instruments.{ + InstrumentTestContext, + TestEdition, + TestMessages +} import org.enso.pkg.{Package, PackageManager} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala index d2deefd54e19..b929cf7bdedb 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala @@ -2,6 +2,10 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.{Constants, ConstantsGen} import org.enso.interpreter.test.Metadata +import org.enso.interpreter.test.instruments.{ + InstrumentTestContext, + TestMessages +} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.enso.text.{ContentVersion, Sha3_224VersionCalculator} diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeRefactoringTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeRefactoringTest.scala index 84b271c9b8ad..83a1256a226f 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeRefactoringTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeRefactoringTest.scala @@ -3,6 +3,10 @@ package org.enso.interpreter.test.instrument import org.apache.commons.io.output.TeeOutputStream import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata +import org.enso.interpreter.test.instruments.{ + InstrumentTestContext, + TestMessages +} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.enso.text.{ContentBasedVersioning, Sha3_224VersionCalculator} diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala index 5de2a7d207ac..420d67e79967 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala @@ -17,6 +17,10 @@ import java.io.{ByteArrayOutputStream, File} import java.nio.file.{Files, Paths} import java.util.UUID import org.apache.commons.io.output.TeeOutputStream +import org.enso.interpreter.test.instruments.{ + InstrumentTestContext, + TestMessages +} @scala.annotation.nowarn("msg=multiarg infix syntax") class RuntimeServerTest diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeStdlibTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeStdlibTest.scala index c43b70aa9736..b3f02aa4bd2d 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeStdlibTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeStdlibTest.scala @@ -1,6 +1,10 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.test.Metadata +import org.enso.interpreter.test.instruments.{ + InstrumentTestContext, + TestEdition +} import org.enso.pkg.{Package, PackageManager, QualifiedName} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeSuggestionUpdatesTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeSuggestionUpdatesTest.scala index 30b7d10b861e..b80746dbef66 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeSuggestionUpdatesTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeSuggestionUpdatesTest.scala @@ -1,6 +1,7 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.ConstantsGen +import org.enso.interpreter.test.instruments.InstrumentTestContext import org.enso.polyglot._ import org.enso.polyglot.data.Tree import org.enso.polyglot.runtime.Runtime.Api diff --git a/engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala b/engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala index 3f30657174bc..e251bbb48977 100644 --- a/engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala +++ b/engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala @@ -2,6 +2,10 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata +import org.enso.interpreter.test.instruments.{ + InstrumentTestContext, + TestMessages +} import org.enso.pkg.QualifiedName import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala index d51b0ba510bb..a7701764854d 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala @@ -1,8 +1,12 @@ package org.enso.interpreter.test import com.oracle.truffle.api.instrumentation.EventBinding -import org.enso.interpreter.test.CodeIdsTestInstrument.IdEventListener -import org.enso.interpreter.test.CodeLocationsTestInstrument.LocationsEventListener +import org.enso.interpreter.test.instrument.CodeIdsTestInstrument.IdEventListener +import org.enso.interpreter.test.instrument.{ + CodeIdsTestInstrument, + CodeLocationsTestInstrument +} +import org.enso.interpreter.test.instrument.CodeLocationsTestInstrument.LocationsEventListener import org.enso.polyglot.debugger.{ DebugServerInfo, DebuggerSessionManagerEndpoint, diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/InstrumentTestContext.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/InstrumentTestContext.scala similarity index 98% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/InstrumentTestContext.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/InstrumentTestContext.scala index 605b01709148..bace4c6b0124 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/InstrumentTestContext.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/InstrumentTestContext.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instrument +package org.enso.interpreter.test.instruments import org.apache.commons.io.FileUtils import org.enso.distribution.locking.ThreadSafeFileLockManager diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerEmulator.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/RuntimeServerEmulator.scala similarity index 98% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerEmulator.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/RuntimeServerEmulator.scala index 710f68de64a4..66d8ec3684c7 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerEmulator.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/RuntimeServerEmulator.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instrument +package org.enso.interpreter.test.instruments import akka.actor.ActorSystem import org.enso.distribution.locking.ThreadSafeLockManager diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestEdition.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestEdition.scala similarity index 95% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestEdition.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestEdition.scala index 3ea48be6443a..e2f1bad9348e 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestEdition.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestEdition.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instrument +package org.enso.interpreter.test.instruments import nl.gn0s1s.bump.SemVer import org.enso.editions.{Editions, LibraryName} diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestMessages.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestMessages.scala similarity index 99% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestMessages.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestMessages.scala index 824c45cb92c7..01c54841109a 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestMessages.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestMessages.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instrument +package org.enso.interpreter.test.instruments import java.util.UUID diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestRuntimeServerConnector.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestRuntimeServerConnector.scala similarity index 96% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestRuntimeServerConnector.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestRuntimeServerConnector.scala index cbb3d1205e6f..03515a565366 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestRuntimeServerConnector.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestRuntimeServerConnector.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instrument +package org.enso.interpreter.test.instruments import akka.actor.{Actor, ActorRef, Props} import com.typesafe.scalalogging.LazyLogging From 81ec91e80efe306eee8b28b31d469777369e9fcd Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 8 Dec 2023 17:54:08 +0100 Subject: [PATCH 13/47] module-info.java is not compiled every time --- project/JPMSUtils.scala | 103 +++++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 38 deletions(-) diff --git a/project/JPMSUtils.scala b/project/JPMSUtils.scala index 08d505f4d61b..0e20f0bd6878 100644 --- a/project/JPMSUtils.scala +++ b/project/JPMSUtils.scala @@ -168,6 +168,9 @@ object JPMSUtils { * Note that sbt is not able to correctly handle `module-info.java` files when * compilation order is defined to mixed order. * + * Compilation of `module-info.java` is skipped iff none of all the classes from all the dependencies + * changed and if the `module-info.java` itself have not changed. + * * @param copyDepsFilter The filter of scopes of the projects from which the class files are first * copied into the `target` directory before `module-info.java` is compiled. * @param modulePath IDs of dependencies that should be put on the module path. The modules @@ -194,11 +197,11 @@ object JPMSUtils { // Class directories of all the dependencies. val sourceProducts = productDirectories.all(copyDepsFilter).value.flatten - log.info(s"Compiling $moduleInfo with javac") - val moduleName = moduleInfos.value.head.moduleName - val cacheStore = streams.value.cacheStoreFactory - val repoRootDir = (LocalProject("enso") / baseDirectory).value + val moduleName = moduleInfos.value.head.moduleName + val cacheStore = streams.value.cacheStoreFactory + val repoRootDir = (LocalProject("enso") / baseDirectory).value + var someDepChanged = false sourceProducts.foreach(sourceProduct => { if (!sourceProduct.exists()) { log.error(s"Source product ${sourceProduct} does not exist") @@ -208,45 +211,62 @@ object JPMSUtils { ) log.error("Run Compile/compile before this task") } - val relPath = repoRootDir.toPath.relativize(sourceProduct.toPath) - val cache = cacheStore.make("cache-" + relPath.toString) - copyClasses(sourceProduct, output, cache, log) + val relPath = repoRootDir.toPath.relativize(sourceProduct.toPath) + val cache = cacheStore.make("cache-" + relPath.toString) + val depChanged = copyClasses(sourceProduct, output, cache, log) + if (depChanged) { + someDepChanged = true + } }) val baseJavacOpts = (Compile / javacOptions).value val fullCp = (Compile / fullClasspath).value - val (mp, cp) = fullCp.partition(file => { - val moduleID = - file.metadata.get(AttributeKey[ModuleID]("moduleID")).get - modulePath.exists(mod => { - mod.organization == moduleID.organization && - mod.name == moduleID.name && - mod.revision == moduleID.revision - }) - }) - - val allOpts = baseJavacOpts ++ Seq( - "--class-path", - cp.map(_.data.getAbsolutePath).mkString(File.pathSeparator), - "--module-path", - mp.map(_.data.getAbsolutePath).mkString(File.pathSeparator), - "-d", - outputPath.toAbsolutePath.toString - ) - log.debug(s"javac options: $allOpts") - val javaCompiler = (Compile / compile / compilers).value.javaTools.javac() - val succ = javaCompiler.run( - Array(PlainVirtualFile(moduleInfo.toPath)), - allOpts.toArray, - output, - incToolOpts, - reporter, - log - ) - if (!succ) { - log.error(s"Compilation of ${moduleInfo} failed") + + // Skip module-info.java compilation if the source have not changed + // Force the compilation if some class file from one of the dependencies changed, + // just to be sure that we don't cause any weird compilation errors. + val moduleInfoCache = cacheStore.make("cache-module-info-" + moduleName) + Tracked.diffInputs(moduleInfoCache, FileInfo.lastModified)( + Set(moduleInfo) + ) { changeReport => + if ( + someDepChanged || changeReport.modified.nonEmpty || changeReport.added.nonEmpty + ) { + log.info(s"Compiling $moduleInfo with javac") + val (mp, cp) = fullCp.partition(file => { + val moduleID = + file.metadata.get(AttributeKey[ModuleID]("moduleID")).get + modulePath.exists(mod => { + mod.organization == moduleID.organization && + mod.name == moduleID.name && + mod.revision == moduleID.revision + }) + }) + + val allOpts = baseJavacOpts ++ Seq( + "--class-path", + cp.map(_.data.getAbsolutePath).mkString(File.pathSeparator), + "--module-path", + mp.map(_.data.getAbsolutePath).mkString(File.pathSeparator), + "-d", + outputPath.toAbsolutePath.toString + ) + log.debug(s"javac options: $allOpts") + + val succ = javaCompiler.run( + Array(PlainVirtualFile(moduleInfo.toPath)), + allOpts.toArray, + output, + incToolOpts, + reporter, + log + ) + if (!succ) { + log.error(s"Compilation of ${moduleInfo} failed") + } + } } } @@ -255,13 +275,15 @@ object JPMSUtils { * @param output Target directory where all the classes from all the dependencies * will be copied to. * @param log + * @return True iff some of the dependencies changed, i.e., if there is a modified class file, or + * some class file was added, or removed */ private def copyClasses( sourceClassesDir: File, output: xsbti.compile.Output, cache: CacheStore, log: Logger - ): Unit = { + ): Boolean = { require(sourceClassesDir.isDirectory) val outputPath: Path = output.getSingleOutputAsPath.get() val outputDir = outputPath.toFile @@ -294,25 +316,30 @@ object JPMSUtils { if (!outputDir.exists()) { IO.createDirectory(outputDir) } + var someDependencyChanged = false Tracked.diffInputs(cache, FileInfo.lastModified)(filesToCopy.toSet) { changeReport => for (f <- changeReport.removed) { val relPath = sourceClassesDir.toPath.relativize(f.toPath) val dest = outputDir.toPath.resolve(relPath) IO.delete(dest.toFile) + someDependencyChanged = true } for (f <- changeReport.modified -- changeReport.removed) { val relPath = sourceClassesDir.toPath.relativize(f.toPath) val dest = outputDir.toPath.resolve(relPath) IO.copyFile(f, dest.toFile) + someDependencyChanged = true } for (f <- changeReport.unmodified) { val relPath = sourceClassesDir.toPath.relativize(f.toPath) val dest = outputDir.toPath.resolve(relPath) if (!dest.toFile.exists()) { IO.copyFile(f, dest.toFile) + someDependencyChanged = true } } } + someDependencyChanged } } From ea70d95f6e04edcbe4668fb60522ec57611a1653 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 11 Dec 2023 12:47:48 +0100 Subject: [PATCH 14/47] Revert "Create new module org.enso.interpreter.test" This reverts commit 4a9c9629c68bbc58e000183d7683831b1d664770. --- build.sbt | 77 +++---------------- .../src/main/java/module-info.java | 7 -- .../test/instrument/BuiltinTypesTest.scala | 4 - .../instrument/RuntimeAsyncCommandsTest.scala | 1 - .../instrument/RuntimeComponentsTest.scala | 4 - .../test/instrument/RuntimeErrorsTest.scala | 4 - .../RuntimeExecutionEnvironmentTest.scala | 5 -- .../instrument/RuntimeInstrumentTest.scala | 4 - .../instrument/RuntimeRefactoringTest.scala | 4 - .../test/instrument/RuntimeServerTest.scala | 4 - .../test/instrument/RuntimeStdlibTest.scala | 4 - .../RuntimeSuggestionUpdatesTest.scala | 1 - .../RuntimeVisualizationsTest.scala | 4 - .../test}/CodeIdsTestInstrument.java | 57 +++----------- .../test}/CodeLocationsTestInstrument.java | 2 +- .../interpreter/test/InterpreterTest.scala | 8 +- .../InstrumentTestContext.scala | 2 +- .../RuntimeServerEmulator.scala | 2 +- .../TestEdition.scala | 2 +- .../TestMessages.scala | 2 +- .../TestRuntimeServerConnector.scala | 2 +- 21 files changed, 32 insertions(+), 168 deletions(-) delete mode 100644 engine/runtime-test-instruments/src/main/java/module-info.java rename engine/{runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument => runtime/src/test/java/org/enso/interpreter/test}/CodeIdsTestInstrument.java (67%) rename engine/{runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument => runtime/src/test/java/org/enso/interpreter/test}/CodeLocationsTestInstrument.java (99%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instruments => instrument}/InstrumentTestContext.scala (98%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instruments => instrument}/RuntimeServerEmulator.scala (98%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instruments => instrument}/TestEdition.scala (95%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instruments => instrument}/TestMessages.scala (99%) rename engine/runtime/src/test/scala/org/enso/interpreter/test/{instruments => instrument}/TestRuntimeServerConnector.scala (96%) diff --git a/build.sbt b/build.sbt index 2b829cedf7b2..614c6bb532de 100644 --- a/build.sbt +++ b/build.sbt @@ -562,10 +562,6 @@ lazy val modulePathTestOptions = val runtimeMod = (LocalProject( "runtime-fat-jar" ) / Compile / productDirectories).value - val runtimeTestInstrModName = - (`runtime-test-instruments` / moduleInfos).value.head.moduleName - val runtimeTestInstrMod = - (`runtime-test-instruments` / Compile / productDirectories).value val graalMods = graalModulesPaths.value val graalLangMods = JPMSUtils.filterModulesFromUpdate( updateReport, @@ -609,14 +605,9 @@ lazy val modulePathTestOptions = .mkString(File.pathSeparator) val allModulesPaths: Seq[String] = runtimeMod.map(_.getAbsolutePath) ++ - runtimeTestInstrMod.map(_.getAbsolutePath) ++ graalMods.map(_.data.getAbsolutePath) ++ graalLangMods.map(_.getAbsolutePath) ++ loggingMods.map(_.getAbsolutePath) - val modulesToAdd = Seq( - runtimeModName, - runtimeTestInstrModName - ) // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not // in a module, and it cannot be simple wrapped inside a module. // So we use plain ch.qos.logback with its configuration. @@ -629,11 +620,11 @@ lazy val modulePathTestOptions = "--module-path", allModulesPaths.mkString(File.pathSeparator), "--add-modules", - modulesToAdd.mkString(","), + runtimeModName, "--patch-module", s"$runtimeModName=$patchStr", "--add-reads", - s"$runtimeModName=ALL-UNNAMED,$runtimeTestInstrModName" + s"$runtimeModName=ALL-UNNAMED" ) } @@ -1512,40 +1503,6 @@ lazy val `runtime-language-epb` = ) .dependsOn(`polyglot-api`) -/** This project contains only Truffle instruments that are used in various tests in `runtime-*` - * projects. - */ -lazy val `runtime-test-instruments` = - (project in file("engine/runtime-test-instruments")) - .settings( - inConfig(Compile)(truffleRunOptionsSettings), - instrumentationSettings, - // Needed to compile module-info.java - compileOrder := xsbti.compile.CompileOrder.JavaThenScala, - moduleInfos := Seq( - JpmsModule("org.enso.runtime.test.instrument") - ), - libraryDependencies ++= { - GraalVM.modules.map(_.withConfigurations(Some(Compile.name))) - }, - Compile / javacOptions ++= { - val updateReport = (Compile / update).value - val graalVmMods = JPMSUtils.filterModulesFromUpdate( - updateReport, - GraalVM.modules, - streams.value.log, - shouldContainAll = true - ) - //val runtimeMod = (`runtime-fat-jar` / Compile / exportedProducts).value.head.data - //val allRequiredMods = graalVmMods ++ Seq(runtimeMod) - val allRequiredMods = graalVmMods - Seq( - "--module-path", - allRequiredMods.map(_.getAbsolutePath).mkString(File.pathSeparator) - ) - } - ) - lazy val runtime = (project in file("engine/runtime")) .configs(Benchmark) .settings( @@ -1598,17 +1555,7 @@ lazy val runtime = (project in file("engine/runtime")) GraalVM.toolsPkgs.map(_.withConfigurations(Some(Runtime.name))) necessaryModules ++ langs ++ tools }, - Test / javaOptions := { - val prevOpts = modulePathTestOptions.value - // Append the `test-classes` directory of the current project to the `--patch-module` option. - // Otherwise, the test frameworks would be unable to load some testing classes as they are - // in the same package as some implementation classes (split packages). - val patchIdx = prevOpts.indexOf("--patch-module") + 1 - val testClassesDir = (Test / productDirectories).value.head - val newPatchOpt = - prevOpts(patchIdx) + File.pathSeparator + testClassesDir.getAbsolutePath - prevOpts.updated(patchIdx, newPatchOpt) - }, + Test / javaOptions := modulePathTestOptions.value, Test / compile := (Test / compile) .dependsOn(LocalProject("runtime-fat-jar") / Compile / compileModuleInfo) .value, @@ -1692,7 +1639,6 @@ lazy val runtime = (project in file("engine/runtime")) .dependsOn(`connected-lock-manager`) .dependsOn(testkit % Test) .dependsOn(`logging-service-logback` % "test->test") - .dependsOn(`runtime-test-instruments` % "test->compile") lazy val `runtime-parser` = (project in file("engine/runtime-parser")) @@ -1821,13 +1767,14 @@ lazy val `runtime-fat-jar` = moduleInfos := Seq( JpmsModule("org.enso.runtime") ), - compileOrder := CompileOrder.JavaThenScala + compileOrder := CompileOrder.JavaThenScala, ) - /** The following libraryDependencies are provided in Runtime scope. - * Later, we will collect them into --module-path option. - * We don't collect them in Compile scope as it does not even make sense - * to run `compile` task in this project. - */ + /** + * The following libraryDependencies are provided in Runtime scope. + * Later, we will collect them into --module-path option. + * We don't collect them in Compile scope as it does not even make sense + * to run `compile` task in this project. + */ .settings( libraryDependencies ++= { val graalMods = @@ -1835,9 +1782,9 @@ lazy val `runtime-fat-jar` = val langMods = GraalVM.langsPkgs.map(_.withConfigurations(Some(Runtime.name))) val logbackMods = - logbackPkg.map(_.withConfigurations(Some(Runtime.name))) + logbackPkg.map(_.withConfigurations(Some(Runtime.name)) ) graalMods ++ langMods ++ logbackMods - } + }, ) /** Assembling Uber Jar */ .settings( diff --git a/engine/runtime-test-instruments/src/main/java/module-info.java b/engine/runtime-test-instruments/src/main/java/module-info.java deleted file mode 100644 index 4eea2327a066..000000000000 --- a/engine/runtime-test-instruments/src/main/java/module-info.java +++ /dev/null @@ -1,7 +0,0 @@ -module org.enso.runtime.test.instrument { - requires org.graalvm.truffle; - exports org.enso.interpreter.test.instrument; - provides com.oracle.truffle.api.instrumentation.provider.TruffleInstrumentProvider with - org.enso.interpreter.test.instrument.CodeLocationsTestInstrumentProvider, - org.enso.interpreter.test.instrument.CodeIdsTestInstrumentProvider; -} diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/BuiltinTypesTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/BuiltinTypesTest.scala index 8c743db91ce0..e8e58d73665b 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/BuiltinTypesTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/BuiltinTypesTest.scala @@ -2,10 +2,6 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata -import org.enso.interpreter.test.instruments.{ - InstrumentTestContext, - TestMessages -} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.graalvm.polyglot.Context diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeAsyncCommandsTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeAsyncCommandsTest.scala index 2b3b9d43d370..2fa8519021c8 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeAsyncCommandsTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeAsyncCommandsTest.scala @@ -1,7 +1,6 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.test.Metadata -import org.enso.interpreter.test.instruments.InstrumentTestContext import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.enso.text.{ContentVersion, Sha3_224VersionCalculator} diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala index f589746215be..9d4e2fddc052 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala @@ -3,10 +3,6 @@ package org.enso.interpreter.test.instrument import org.enso.editions.LibraryName import org.enso.interpreter.runtime import org.enso.interpreter.test.Metadata -import org.enso.interpreter.test.instruments.{ - InstrumentTestContext, - TestEdition -} import org.enso.pkg.{ ComponentGroup, ComponentGroups, diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala index cdcf6a5a4989..97bb92fd34e6 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala @@ -2,10 +2,6 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata -import org.enso.interpreter.test.instruments.{ - InstrumentTestContext, - TestMessages -} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.enso.text.editing.model diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala index 3a39b64e00a1..f1ac5c801e57 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala @@ -3,11 +3,6 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.EnsoContext import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata -import org.enso.interpreter.test.instruments.{ - InstrumentTestContext, - TestEdition, - TestMessages -} import org.enso.pkg.{Package, PackageManager} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala index b929cf7bdedb..d2deefd54e19 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala @@ -2,10 +2,6 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.{Constants, ConstantsGen} import org.enso.interpreter.test.Metadata -import org.enso.interpreter.test.instruments.{ - InstrumentTestContext, - TestMessages -} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.enso.text.{ContentVersion, Sha3_224VersionCalculator} diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeRefactoringTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeRefactoringTest.scala index 83a1256a226f..84b271c9b8ad 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeRefactoringTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeRefactoringTest.scala @@ -3,10 +3,6 @@ package org.enso.interpreter.test.instrument import org.apache.commons.io.output.TeeOutputStream import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata -import org.enso.interpreter.test.instruments.{ - InstrumentTestContext, - TestMessages -} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.enso.text.{ContentBasedVersioning, Sha3_224VersionCalculator} diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala index 420d67e79967..5de2a7d207ac 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala @@ -17,10 +17,6 @@ import java.io.{ByteArrayOutputStream, File} import java.nio.file.{Files, Paths} import java.util.UUID import org.apache.commons.io.output.TeeOutputStream -import org.enso.interpreter.test.instruments.{ - InstrumentTestContext, - TestMessages -} @scala.annotation.nowarn("msg=multiarg infix syntax") class RuntimeServerTest diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeStdlibTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeStdlibTest.scala index b3f02aa4bd2d..c43b70aa9736 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeStdlibTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeStdlibTest.scala @@ -1,10 +1,6 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.test.Metadata -import org.enso.interpreter.test.instruments.{ - InstrumentTestContext, - TestEdition -} import org.enso.pkg.{Package, PackageManager, QualifiedName} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeSuggestionUpdatesTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeSuggestionUpdatesTest.scala index b80746dbef66..30b7d10b861e 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeSuggestionUpdatesTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeSuggestionUpdatesTest.scala @@ -1,7 +1,6 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.ConstantsGen -import org.enso.interpreter.test.instruments.InstrumentTestContext import org.enso.polyglot._ import org.enso.polyglot.data.Tree import org.enso.polyglot.runtime.Runtime.Api diff --git a/engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala b/engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala index e251bbb48977..3f30657174bc 100644 --- a/engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala +++ b/engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala @@ -2,10 +2,6 @@ package org.enso.interpreter.test.instrument import org.enso.interpreter.runtime.`type`.ConstantsGen import org.enso.interpreter.test.Metadata -import org.enso.interpreter.test.instruments.{ - InstrumentTestContext, - TestMessages -} import org.enso.pkg.QualifiedName import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeIdsTestInstrument.java b/engine/runtime/src/test/java/org/enso/interpreter/test/CodeIdsTestInstrument.java similarity index 67% rename from engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeIdsTestInstrument.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/CodeIdsTestInstrument.java index 4d0556ce02c7..da5801309ad4 100644 --- a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeIdsTestInstrument.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/CodeIdsTestInstrument.java @@ -1,12 +1,12 @@ -package org.enso.interpreter.test.instrument; +package org.enso.interpreter.test; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.*; import com.oracle.truffle.api.nodes.Node; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.util.LinkedHashMap; import java.util.Map; +import org.enso.interpreter.node.ExpressionNode; +import org.enso.interpreter.runtime.control.TailCallException; import java.util.UUID; @@ -82,34 +82,11 @@ public ExecutionEventNode create(EventContext context) { return node; } - /** - * Uses reflection to get access to {@link org.enso.interpreter.node.ExpressionNode} and - * {@link org.enso.interpreter.runtime.control.TailCallException} classes. We need this because - * this instrument is not a part of the {@code org.enso.runtime} module, and thus cannot import these classes. - * It is part of {@code org.enso.runtime.test.instrument} module, which just provides few instruments - * for testing of the {@code org.enso.runtime} module. - * - * This is a hack to make the unit testing work and to remove the compile-time dependency on the - * {@code org.enso.runtime} module. - */ private final class IdEventNode extends ExecutionEventNode { private final EventContext context; - private final Class expressionNodeClass; - private final Method expressionNodeGetIdMethod; - private final Class tailCallExceptionClass; IdEventNode(EventContext context) { this.context = context; - try { - // Are there two ExpressionNode classes at this point? One loaded here and one loaded inside - // the runtime module? - this.expressionNodeClass = Class.forName("org.enso.interpreter.node.ExpressionNode"); - this.tailCallExceptionClass = - Class.forName("org.enso.interpreter.runtime.control.TailCallException"); - this.expressionNodeGetIdMethod = expressionNodeClass.getDeclaredMethod("getId"); - } catch (ClassNotFoundException | NoSuchMethodException e) { - throw new AssertionError(e); - } } @Override @@ -118,6 +95,7 @@ public void onEnter(VirtualFrame frame) {} /** * Checks if the node to be executed is the node this listener was created to observe. * + * @param context current execution context * @param frame current execution frame * @param result the result of executing the node */ @@ -127,11 +105,11 @@ public void onReturnValue(VirtualFrame frame, Object result) { return; } Node node = context.getInstrumentedNode(); - if (!expressionNodeClass.isAssignableFrom(node.getClass())) { + if (!(node instanceof ExpressionNode)) { return; } nodes.put(this, result); - UUID id = getIdFromExpressionNode(node); + UUID id = ((ExpressionNode) node).getId(); if (id == null || !id.equals(expectedId)) { return; } @@ -140,32 +118,22 @@ public void onReturnValue(VirtualFrame frame, Object result) { } } - private UUID getIdFromExpressionNode(Node expressionNode) { - assert expressionNodeClass.isAssignableFrom(expressionNode.getClass()); - Object res = null; - try { - res = expressionNodeGetIdMethod.invoke(expressionNode); - } catch (IllegalAccessException | InvocationTargetException e) { - throw new AssertionError(e); - } - return (UUID) res; - } - /** * Checks if the specified was called, if its execution triggered TCO. * + * @param context current execution context. * @param frame current execution frame. * @param exception the exception thrown from this node's execution. */ @Override public void onReturnExceptional(VirtualFrame frame, Throwable exception) { - if (!(exception.getClass().equals(tailCallExceptionClass))) { + if (!(exception instanceof TailCallException)) { return; } - if (!(expressionNodeClass.isAssignableFrom(context.getInstrumentedNode().getClass()))) { + if (!(context.getInstrumentedNode() instanceof ExpressionNode)) { return; } - UUID id = getIdFromExpressionNode(context.getInstrumentedNode()); + UUID id = ((ExpressionNode) context.getInstrumentedNode()).getId(); if (expectedResult == null) { successful = true; } @@ -175,9 +143,8 @@ public void onReturnExceptional(VirtualFrame frame, Throwable exception) { public String toString() { var sb = new StringBuilder(); sb.append(context.getInstrumentedNode().getClass().getSimpleName()); - if (expressionNodeClass.isAssignableFrom(context.getInstrumentedNode().getClass())) { - UUID id = getIdFromExpressionNode(context.getInstrumentedNode()); - sb.append("@").append(id); + if (context.getInstrumentedNode() instanceof ExpressionNode expr) { + sb.append("@").append(expr.getId()); } sb.append(" "); sb.append(context.getInstrumentedSourceSection()); diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeLocationsTestInstrument.java b/engine/runtime/src/test/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java similarity index 99% rename from engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeLocationsTestInstrument.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java index bf217c1ef7a8..776f57e8b83b 100644 --- a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instrument/CodeLocationsTestInstrument.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instrument; +package org.enso.interpreter.test; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.EventBinding; diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala index a7701764854d..d51b0ba510bb 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala @@ -1,12 +1,8 @@ package org.enso.interpreter.test import com.oracle.truffle.api.instrumentation.EventBinding -import org.enso.interpreter.test.instrument.CodeIdsTestInstrument.IdEventListener -import org.enso.interpreter.test.instrument.{ - CodeIdsTestInstrument, - CodeLocationsTestInstrument -} -import org.enso.interpreter.test.instrument.CodeLocationsTestInstrument.LocationsEventListener +import org.enso.interpreter.test.CodeIdsTestInstrument.IdEventListener +import org.enso.interpreter.test.CodeLocationsTestInstrument.LocationsEventListener import org.enso.polyglot.debugger.{ DebugServerInfo, DebuggerSessionManagerEndpoint, diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/InstrumentTestContext.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/InstrumentTestContext.scala similarity index 98% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/InstrumentTestContext.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/InstrumentTestContext.scala index bace4c6b0124..605b01709148 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/InstrumentTestContext.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/InstrumentTestContext.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instruments +package org.enso.interpreter.test.instrument import org.apache.commons.io.FileUtils import org.enso.distribution.locking.ThreadSafeFileLockManager diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/RuntimeServerEmulator.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerEmulator.scala similarity index 98% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/RuntimeServerEmulator.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerEmulator.scala index 66d8ec3684c7..710f68de64a4 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/RuntimeServerEmulator.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerEmulator.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instruments +package org.enso.interpreter.test.instrument import akka.actor.ActorSystem import org.enso.distribution.locking.ThreadSafeLockManager diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestEdition.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestEdition.scala similarity index 95% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestEdition.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestEdition.scala index e2f1bad9348e..3ea48be6443a 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestEdition.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestEdition.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instruments +package org.enso.interpreter.test.instrument import nl.gn0s1s.bump.SemVer import org.enso.editions.{Editions, LibraryName} diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestMessages.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestMessages.scala similarity index 99% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestMessages.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestMessages.scala index 01c54841109a..824c45cb92c7 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestMessages.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestMessages.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instruments +package org.enso.interpreter.test.instrument import java.util.UUID diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestRuntimeServerConnector.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestRuntimeServerConnector.scala similarity index 96% rename from engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestRuntimeServerConnector.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestRuntimeServerConnector.scala index 03515a565366..cbb3d1205e6f 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instruments/TestRuntimeServerConnector.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/TestRuntimeServerConnector.scala @@ -1,4 +1,4 @@ -package org.enso.interpreter.test.instruments +package org.enso.interpreter.test.instrument import akka.actor.{Actor, ActorRef, Props} import com.typesafe.scalalogging.LazyLogging From 1d42a25df740e12e7bd33763a6ccb4bb626c9174 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 12 Dec 2023 12:31:16 +0100 Subject: [PATCH 15/47] Add skeleton of JPMSPlugin auto plugin. --- project/JPMSPlugin.scala | 148 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 project/JPMSPlugin.scala diff --git a/project/JPMSPlugin.scala b/project/JPMSPlugin.scala new file mode 100644 index 000000000000..57097536b42e --- /dev/null +++ b/project/JPMSPlugin.scala @@ -0,0 +1,148 @@ +import sbt.* +import sbt.Keys.* +import sbt.internal.inc.{CompileOutput, PlainVirtualFile} +import sbt.util.CacheStore +import sbtassembly.Assembly.{Dependency, JarEntry, Project} +import sbtassembly.{CustomMergeStrategy, MergeStrategy} +import xsbti.compile.IncToolOptionsUtil + +import java.io.File + +/** An automatic plugin that handles everything related to JPMS modules. One needs to explicitly + * enable this plugin in a project with `.enablePlugins(JPMSPlugin)`. The keys and tasks provided by this plugin + * corresponds to the module-related options of `javac` and `java` commands. + * + * This plugin injects all the module-specific options to `javaOptions` and `javacOptions`, based on + * the settings of this plugin. + * + * 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`. + */ +object JPMSPlugin extends AutoPlugin { + object autoImport { + val javaModuleName = + settingKey[String]("The name of the Java (JPMS) module") + val addModules = settingKey[Seq[String]]( + "Module names that will be added to --add-modules option" + ) + val modulePath = taskKey[Seq[File]]( + "Directories (Jar archives or expanded Jar archives) that will be put into " + + "--module-path option" + ) + val patchModules = taskKey[Map[String, Seq[File]]]( + """ + |A map of module names to directories (Jar archives or expanded Jar archives) that will be + |put into --patch-module option. + |""".stripMargin + ) + val addExports = taskKey[Map[String, Seq[String]]]( + """ + |A map of module names to packages that will be put into --add-exports option. + |The format of `--add-exports` option is `module/package=target-module(,target-module)*` + |The key in the map is `module/package` and the value is a sequence of target modules + |""".stripMargin + ) + val compileModuleInfo = taskKey[Unit]("Compile module-info.java") + val modulePathTestOptions_ = taskKey[Seq[String]]( + "Assembles options for the JVM for running tests with all the required modules. " + + "Including truffle-compiler and org.enso.runtime modules and all their dependencies." + ) + } + + import autoImport.* + + override lazy val projectSettings: Seq[Setting[_]] = Seq( + addModules := Seq.empty, + modulePath := Seq.empty, + patchModules := Map.empty, + addExports := Map.empty, + compileModuleInfo := {}, + javacOptions ++= { + constructOptions( + modulePath.value, + addModules.value, + patchModules.value, + addExports.value, + streams.value.log + ) + }, + javaOptions ++= { + constructOptions( + modulePath.value, + addModules.value, + patchModules.value, + addExports.value, + streams.value.log + ) + } + ) + + private def constructOptions( + modulePath: Seq[File], + addModules: Seq[String], + patchModules: Map[String, Seq[File]], + addExports: Map[String, Seq[String]], + log: Logger + ): Seq[String] = { + val patchOpts: Seq[String] = patchModules.flatMap { + case (moduleName, dirsToPatch) => + ensureDirectoriesExist(dirsToPatch, log) + val patchStr = dirsToPatch + .map(_.getAbsolutePath) + .mkString(File.pathSeparator) + Seq( + "--patch-module", + s"$moduleName=$patchStr" + ) + }.toSeq + + ensureDirectoriesExist(modulePath, log) + + val addExportsOpts: Seq[String] = addExports.flatMap { + case (modPkgName, targetModules) => + if (!modPkgName.contains("/")) { + log.error(s"JPMSPlugin: Invalid module/package name: $modPkgName") + } + Seq( + "--add-exports", + modPkgName + "=" + targetModules.mkString(",") + ) + }.toSeq + + val modulePathOpts = if (modulePath.isEmpty) { + Seq.empty + } else { + Seq( + "--module-path", + modulePath.map(_.getAbsolutePath).mkString(File.pathSeparator) + ) + } + + val addModsOpts = if (addModules.isEmpty) { + Seq.empty + } else { + Seq( + "--add-modules", + addModules.mkString(",") + ) + } + + modulePathOpts ++ addModsOpts ++ patchOpts ++ addExportsOpts + } + + /** Java does not mandate that the directories specified in the module path or + * in --patch-module exist, but it is usefull to report at least warnings. + * @param dirs + * @param log + */ + private def ensureDirectoriesExist( + dirs: Seq[File], + log: Logger + ): Unit = { + dirs.foreach { dir => + if (!dir.exists()) { + log.warn(s"JPMSPlugin: Directory $dir does not exist.") + } + } + } +} From 2af02fb0be99f064138ec22b6a0feea70a766083 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 12 Dec 2023 12:31:53 +0100 Subject: [PATCH 16/47] Add runtime-test-instruments project. --- build.sbt | 23 +++++++++ .../src/main/java/module-info.java | 6 +++ .../test/CodeIdsTestInstrument.java | 49 +++++++++++++++---- .../test/CodeLocationsTestInstrument.java | 0 4 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 engine/runtime-test-instruments/src/main/java/module-info.java rename engine/{runtime/src/test => runtime-test-instruments/src/main}/java/org/enso/interpreter/test/CodeIdsTestInstrument.java (76%) rename engine/{runtime/src/test => runtime-test-instruments/src/main}/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java (100%) diff --git a/build.sbt b/build.sbt index 614c6bb532de..4999df995b6b 100644 --- a/build.sbt +++ b/build.sbt @@ -1503,6 +1503,29 @@ lazy val `runtime-language-epb` = ) .dependsOn(`polyglot-api`) +/** `runtime-test-instruments` project contains Truffle instruments that are used solely for testing. + * It is compiled into an explicit Java module. Note that this project cannot have compile-time dependency on `runtime` + * project, so if you need access to classes from `runtime`, you need to use reflection. + */ +lazy val `runtime-test-instruments` = + (project in file("engine/runtime-test-instruments")) + .enablePlugins(JPMSPlugin) + .settings( + inConfig(Compile)(truffleRunOptionsSettings), + truffleDslSuppressWarnsSetting, + instrumentationSettings, + javaModuleName := "org.enso.runtime.test", + modulePath := { + JPMSUtils.filterModulesFromUpdate( + update.value, + GraalVM.modules, + streams.value.log, + shouldContainAll = true + ) + }, + libraryDependencies ++= GraalVM.modules + ) + lazy val runtime = (project in file("engine/runtime")) .configs(Benchmark) .settings( diff --git a/engine/runtime-test-instruments/src/main/java/module-info.java b/engine/runtime-test-instruments/src/main/java/module-info.java new file mode 100644 index 000000000000..0ed2bce564d3 --- /dev/null +++ b/engine/runtime-test-instruments/src/main/java/module-info.java @@ -0,0 +1,6 @@ +module org.enso.runtime.test { + requires org.graalvm.truffle; + provides com.oracle.truffle.api.instrumentation.provider.TruffleInstrumentProvider with + org.enso.interpreter.test.CodeIdsTestInstrumentProvider, + org.enso.interpreter.test.CodeLocationsTestInstrumentProvider; +} diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/CodeIdsTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeIdsTestInstrument.java similarity index 76% rename from engine/runtime/src/test/java/org/enso/interpreter/test/CodeIdsTestInstrument.java rename to engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeIdsTestInstrument.java index da5801309ad4..2f84485f5590 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/CodeIdsTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeIdsTestInstrument.java @@ -3,10 +3,10 @@ import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.*; import com.oracle.truffle.api.nodes.Node; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.LinkedHashMap; import java.util.Map; -import org.enso.interpreter.node.ExpressionNode; -import org.enso.interpreter.runtime.control.TailCallException; import java.util.UUID; @@ -21,8 +21,24 @@ services = CodeIdsTestInstrument.class) public class CodeIdsTestInstrument extends TruffleInstrument { public static final String INSTRUMENT_ID = "ids-test"; + private static final Class exprNodeClass; + private static final Method exprNodeGetId; + private static final Class tailCallExceptionClass; private Env env; + /** + * Use reflection to call methods from `runtime` project. We cannot use `runtime` as a dependency. + */ + static { + try { + exprNodeClass = Class.forName("org.enso.interpreter.node.ExpressionNode"); + exprNodeGetId = exprNodeClass.getMethod("getId"); + tailCallExceptionClass = Class.forName("org.enso.interpreter.runtime.control.TailCallException"); + } catch (ClassNotFoundException | NoSuchMethodException e) { + throw new AssertionError(e); + } + } + /** * Initializes the instrument. Substitute for a constructor, called by the Truffle framework. * @@ -95,7 +111,6 @@ public void onEnter(VirtualFrame frame) {} /** * Checks if the node to be executed is the node this listener was created to observe. * - * @param context current execution context * @param frame current execution frame * @param result the result of executing the node */ @@ -105,11 +120,11 @@ public void onReturnValue(VirtualFrame frame, Object result) { return; } Node node = context.getInstrumentedNode(); - if (!(node instanceof ExpressionNode)) { + if (!(isInstanceOf(node, exprNodeClass))) { return; } nodes.put(this, result); - UUID id = ((ExpressionNode) node).getId(); + UUID id = exprNodeGetId(node); if (id == null || !id.equals(expectedId)) { return; } @@ -118,6 +133,19 @@ public void onReturnValue(VirtualFrame frame, Object result) { } } + private boolean isInstanceOf(Object obj, Class clazz) { + return clazz.isInstance(obj); + } + + private UUID exprNodeGetId(Object exprNode) { + assert isInstanceOf(exprNode, exprNodeClass); + try { + return (UUID) exprNodeGetId.invoke(exprNode); + } catch (IllegalAccessException | InvocationTargetException e) { + throw new AssertionError(e); + } + } + /** * Checks if the specified was called, if its execution triggered TCO. * @@ -127,13 +155,13 @@ public void onReturnValue(VirtualFrame frame, Object result) { */ @Override public void onReturnExceptional(VirtualFrame frame, Throwable exception) { - if (!(exception instanceof TailCallException)) { + if (!isInstanceOf(exception, tailCallExceptionClass)) { return; } - if (!(context.getInstrumentedNode() instanceof ExpressionNode)) { + if (!isInstanceOf(context.getInstrumentedNode(), exprNodeClass)) { return; } - UUID id = ((ExpressionNode) context.getInstrumentedNode()).getId(); + UUID id = exprNodeGetId(context.getInstrumentedNode()); if (expectedResult == null) { successful = true; } @@ -143,8 +171,9 @@ public void onReturnExceptional(VirtualFrame frame, Throwable exception) { public String toString() { var sb = new StringBuilder(); sb.append(context.getInstrumentedNode().getClass().getSimpleName()); - if (context.getInstrumentedNode() instanceof ExpressionNode expr) { - sb.append("@").append(expr.getId()); + if (isInstanceOf(context.getInstrumentedNode(), exprNodeClass)) { + UUID id = exprNodeGetId(context.getInstrumentedNode()); + sb.append("@").append(id); } sb.append(" "); sb.append(context.getInstrumentedSourceSection()); diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java similarity index 100% rename from engine/runtime/src/test/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java rename to engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java From 95381be20e3e73071130a59543c0e0194162b822 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 12 Dec 2023 13:23:53 +0100 Subject: [PATCH 17/47] Fix VectorTests in runtime --- build.sbt | 115 +++++++++++++++--- .../src/main/java/module-info.java | 4 +- .../CodeIdsTestInstrument.java | 2 +- .../CodeLocationsTestInstrument.java | 2 +- .../interpreter/test/InterpreterTest.scala | 8 +- project/JPMSPlugin.scala | 60 +++++++-- 6 files changed, 159 insertions(+), 32 deletions(-) rename engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/{ => instruments}/CodeIdsTestInstrument.java (99%) rename engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/{ => instruments}/CodeLocationsTestInstrument.java (99%) diff --git a/build.sbt b/build.sbt index 4999df995b6b..f94e75176470 100644 --- a/build.sbt +++ b/build.sbt @@ -16,6 +16,10 @@ import com.sandinh.javamodule.moduleinfo.JpmsModule import com.sandinh.javamodule.moduleinfo.AutomaticModule import sbt.librarymanagement.DependencyFilter +// This import is unnecessary, but bit adds a proper code completion features +// to IntelliJ. +import JPMSPlugin.autoImport.* + import java.io.File import scala.collection.mutable.ListBuffer @@ -1528,6 +1532,7 @@ lazy val `runtime-test-instruments` = lazy val runtime = (project in file("engine/runtime")) .configs(Benchmark) + .enablePlugins(JPMSPlugin) .settings( frgaalJavaCompilerSetting, truffleDslSuppressWarnsSetting, @@ -1577,14 +1582,90 @@ lazy val runtime = (project in file("engine/runtime")) val tools = GraalVM.toolsPkgs.map(_.withConfigurations(Some(Runtime.name))) necessaryModules ++ langs ++ tools - }, - Test / javaOptions := modulePathTestOptions.value, - Test / compile := (Test / compile) - .dependsOn(LocalProject("runtime-fat-jar") / Compile / compileModuleInfo) - .value, + } + ) + .settings( Test / unmanagedClasspath := (LocalProject( "runtime-fat-jar" ) / Compile / exportedProducts).value, + Test / addModules := Seq( + (`runtime-test-instruments` / javaModuleName).value, + (`runtime-fat-jar` / javaModuleName).value + ), + Test / modulePath := { + val updateReport = (Test / update).value + val requiredModIds = + GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ Seq( + "org.slf4j" % "slf4j-api" % slf4jVersion + ) + val requiredMods = JPMSUtils.filterModulesFromUpdate( + updateReport, + 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 + requiredMods ++ + Seq(runtimeTestInstrumentsMod) ++ + Seq(runtimeMod) + }, + Test / patchModules := { + + /** All these modules will be in --patch-module cmdline option to java, which means that + * for the JVM, it will appear that all the classes contained in these sbt projects are contained + * in the `org.enso.runtime` module. In this way, we do not have to assembly the `runtime.jar` + * fat jar. + */ + val modulesToPatchIntoRuntime: Seq[File] = + (LocalProject( + "runtime-instrument-common" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-id-execution" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-repl-debugger" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-runtime-server" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-language-epb" + ) / Compile / productDirectories).value ++ + (LocalProject("runtime-compiler") / Compile / productDirectories).value + // Patch test-classes into the runtime module. This is standard way to deal with the + // split package problem in unit tests. For example, Maven's surefire plugin does this. + val testClassesDir = (Test / productDirectories).value.head + Map( + (`runtime-fat-jar` / javaModuleName).value -> (modulesToPatchIntoRuntime ++ Seq( + testClassesDir + )) + ) + }, + Test / addReads := { + // We patched the test-classes into the runtime module. These classes access some stuff from + // uunamed module. Thus, let's add ALL-UNNAMED. + Map( + (`runtime-fat-jar` / javaModuleName).value -> Seq("ALL-UNNAMED") + ) + }, + Test / javaOptions ++= { + // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not + // in a module, and it cannot be simple wrapped inside a module. + // So we use plain ch.qos.logback with its configuration. + val testLogbackConf = (LocalProject( + "logging-service-logback" + ) / Test / sourceDirectory).value / "resources" / "logback-test.xml" + Seq( + "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", + s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}" + ) + } + ) + .settings( Test / fork := true, Test / envVars ++= distributionEnvironmentOverrides ++ Map( "ENSO_TEST_DISABLE_IR_CACHE" -> "false" @@ -1662,6 +1743,7 @@ lazy val runtime = (project in file("engine/runtime")) .dependsOn(`connected-lock-manager`) .dependsOn(testkit % Test) .dependsOn(`logging-service-logback` % "test->test") + .dependsOn(`runtime-test-instruments` % "test->compile") lazy val `runtime-parser` = (project in file("engine/runtime-parser")) @@ -1726,7 +1808,9 @@ lazy val `runtime-instrument-common` = ) .dependsOn(`refactoring-utils`) .dependsOn( - runtime % "compile->compile;test->test;runtime->runtime;bench->bench" + LocalProject( + "runtime" + ) % "compile->compile;test->test;runtime->runtime;bench->bench" ) lazy val `runtime-instrument-id-execution` = @@ -1766,6 +1850,7 @@ lazy val `runtime-instrument-runtime-server` = */ lazy val `runtime-fat-jar` = (project in file("engine/runtime-fat-jar")) + .enablePlugins(JPMSPlugin) .settings( Compile / compileModuleInfo := { JPMSUtils.compileModuleInfo( @@ -1787,17 +1872,17 @@ lazy val `runtime-fat-jar` = .value, // Filter module-info.java from the compilation excludeFilter := excludeFilter.value || "module-info.java", + javaModuleName := "org.enso.runtime", moduleInfos := Seq( JpmsModule("org.enso.runtime") ), - compileOrder := CompileOrder.JavaThenScala, + compileOrder := CompileOrder.JavaThenScala ) - /** - * The following libraryDependencies are provided in Runtime scope. - * Later, we will collect them into --module-path option. - * We don't collect them in Compile scope as it does not even make sense - * to run `compile` task in this project. - */ + /** The following libraryDependencies are provided in Runtime scope. + * Later, we will collect them into --module-path option. + * We don't collect them in Compile scope as it does not even make sense + * to run `compile` task in this project. + */ .settings( libraryDependencies ++= { val graalMods = @@ -1805,9 +1890,9 @@ lazy val `runtime-fat-jar` = val langMods = GraalVM.langsPkgs.map(_.withConfigurations(Some(Runtime.name))) val logbackMods = - logbackPkg.map(_.withConfigurations(Some(Runtime.name)) ) + logbackPkg.map(_.withConfigurations(Some(Runtime.name))) graalMods ++ langMods ++ logbackMods - }, + } ) /** Assembling Uber Jar */ .settings( diff --git a/engine/runtime-test-instruments/src/main/java/module-info.java b/engine/runtime-test-instruments/src/main/java/module-info.java index 0ed2bce564d3..2ec22db486c2 100644 --- a/engine/runtime-test-instruments/src/main/java/module-info.java +++ b/engine/runtime-test-instruments/src/main/java/module-info.java @@ -1,6 +1,6 @@ module org.enso.runtime.test { requires org.graalvm.truffle; provides com.oracle.truffle.api.instrumentation.provider.TruffleInstrumentProvider with - org.enso.interpreter.test.CodeIdsTestInstrumentProvider, - org.enso.interpreter.test.CodeLocationsTestInstrumentProvider; + org.enso.interpreter.test.instruments.CodeIdsTestInstrumentProvider, + org.enso.interpreter.test.instruments.CodeLocationsTestInstrumentProvider; } diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeIdsTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java similarity index 99% rename from engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeIdsTestInstrument.java rename to engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java index 2f84485f5590..a209cf3536ae 100644 --- a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeIdsTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java @@ -1,4 +1,4 @@ -package org.enso.interpreter.test; +package org.enso.interpreter.test.instruments; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.*; diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeLocationsTestInstrument.java similarity index 99% rename from engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java rename to engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeLocationsTestInstrument.java index 776f57e8b83b..e660782ce57a 100644 --- a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/CodeLocationsTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeLocationsTestInstrument.java @@ -1,4 +1,4 @@ -package org.enso.interpreter.test; +package org.enso.interpreter.test.instruments; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.EventBinding; diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala index d51b0ba510bb..9bebf47d09db 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/InterpreterTest.scala @@ -1,8 +1,12 @@ package org.enso.interpreter.test import com.oracle.truffle.api.instrumentation.EventBinding -import org.enso.interpreter.test.CodeIdsTestInstrument.IdEventListener -import org.enso.interpreter.test.CodeLocationsTestInstrument.LocationsEventListener +import org.enso.interpreter.test.instruments.CodeIdsTestInstrument.IdEventListener +import org.enso.interpreter.test.instruments.{ + CodeIdsTestInstrument, + CodeLocationsTestInstrument +} +import org.enso.interpreter.test.instruments.CodeLocationsTestInstrument.LocationsEventListener import org.enso.polyglot.debugger.{ DebugServerInfo, DebuggerSessionManagerEndpoint, diff --git a/project/JPMSPlugin.scala b/project/JPMSPlugin.scala index 57097536b42e..e18bdf7cf3de 100644 --- a/project/JPMSPlugin.scala +++ b/project/JPMSPlugin.scala @@ -42,6 +42,13 @@ object JPMSPlugin extends AutoPlugin { |The key in the map is `module/package` and the value is a sequence of target modules |""".stripMargin ) + val addReads = taskKey[Map[String, Seq[String]]]( + """ + |A map of module names to modules that will be put into --add-reads option. + |When a module A reads a module B, it means that it "depends" on it - it has the same + |effect as if module A would have `requires B` in its module-info.java file. + |""".stripMargin + ) val compileModuleInfo = taskKey[Unit]("Compile module-info.java") val modulePathTestOptions_ = taskKey[Seq[String]]( "Assembles options for the JVM for running tests with all the required modules. " + @@ -56,22 +63,45 @@ object JPMSPlugin extends AutoPlugin { modulePath := Seq.empty, patchModules := Map.empty, addExports := Map.empty, + addReads := Map.empty, compileModuleInfo := {}, - javacOptions ++= { + Compile / javacOptions ++= { + constructOptions( + (Compile / modulePath).value, + (Compile / addModules).value, + (Compile / patchModules).value, + (Compile / addExports).value, + (Compile / addReads).value, + streams.value.log + ) + }, + Compile / javaOptions ++= { constructOptions( - modulePath.value, - addModules.value, - patchModules.value, - addExports.value, + (Compile / modulePath).value, + (Compile / addModules).value, + (Compile / patchModules).value, + (Compile / addExports).value, + (Compile / addReads).value, streams.value.log ) }, - javaOptions ++= { + Test / javacOptions ++= { constructOptions( - modulePath.value, - addModules.value, - patchModules.value, - addExports.value, + (Test / modulePath).value, + (Test / addModules).value, + (Test / patchModules).value, + (Test / addExports).value, + (Test / addReads).value, + streams.value.log + ) + }, + Test / javaOptions ++= { + constructOptions( + (Test / modulePath).value, + (Test / addModules).value, + (Test / patchModules).value, + (Test / addExports).value, + (Test / addReads).value, streams.value.log ) } @@ -82,6 +112,7 @@ object JPMSPlugin extends AutoPlugin { addModules: Seq[String], patchModules: Map[String, Seq[File]], addExports: Map[String, Seq[String]], + addReads: Map[String, Seq[String]], log: Logger ): Seq[String] = { val patchOpts: Seq[String] = patchModules.flatMap { @@ -127,7 +158,14 @@ object JPMSPlugin extends AutoPlugin { ) } - modulePathOpts ++ addModsOpts ++ patchOpts ++ addExportsOpts + val addReadsOpts = addReads.flatMap { case (modName, targetModules) => + Seq( + "--add-reads", + modName + "=" + targetModules.mkString(",") + ) + }.toSeq + + modulePathOpts ++ addModsOpts ++ patchOpts ++ addExportsOpts ++ addReadsOpts } /** Java does not mandate that the directories specified in the module path or From 6997cff0b0942847c24fb354ddedcc334830b8ed Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 12 Dec 2023 15:31:25 +0100 Subject: [PATCH 18/47] Fix the rest of runtime tests --- build.sbt | 10 ++++++---- .../src/main/java/module-info.java | 3 +++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/build.sbt b/build.sbt index f94e75176470..dd2f7fd1982c 100644 --- a/build.sbt +++ b/build.sbt @@ -1613,7 +1613,6 @@ lazy val runtime = (project in file("engine/runtime")) Seq(runtimeMod) }, Test / patchModules := { - /** All these modules will be in --patch-module cmdline option to java, which means that * for the JVM, it will appear that all the classes contained in these sbt projects are contained * in the `org.enso.runtime` module. In this way, we do not have to assembly the `runtime.jar` @@ -1646,10 +1645,13 @@ lazy val runtime = (project in file("engine/runtime")) ) }, Test / addReads := { - // We patched the test-classes into the runtime module. These classes access some stuff from - // uunamed module. Thus, let's add ALL-UNNAMED. + val runtimeModName = (`runtime-fat-jar` / javaModuleName).value + val testInstrumentsModName = (`runtime-test-instruments` / javaModuleName).value Map( - (`runtime-fat-jar` / javaModuleName).value -> Seq("ALL-UNNAMED") + // We patched the test-classes into the runtime module. These classes access some stuff from + // unnamed module. Thus, let's add ALL-UNNAMED. + runtimeModName -> Seq("ALL-UNNAMED", testInstrumentsModName), + testInstrumentsModName -> Seq(runtimeModName) ) }, Test / javaOptions ++= { diff --git a/engine/runtime-test-instruments/src/main/java/module-info.java b/engine/runtime-test-instruments/src/main/java/module-info.java index 2ec22db486c2..a7e253a72219 100644 --- a/engine/runtime-test-instruments/src/main/java/module-info.java +++ b/engine/runtime-test-instruments/src/main/java/module-info.java @@ -1,5 +1,8 @@ module org.enso.runtime.test { requires org.graalvm.truffle; + + exports org.enso.interpreter.test.instruments; + provides com.oracle.truffle.api.instrumentation.provider.TruffleInstrumentProvider with org.enso.interpreter.test.instruments.CodeIdsTestInstrumentProvider, org.enso.interpreter.test.instruments.CodeLocationsTestInstrumentProvider; From ffd95c4b745eb6bea542f687716543b0314568be Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 12 Dec 2023 18:34:40 +0100 Subject: [PATCH 19/47] runtime-with-instruments/test run with truffle-compiler --- build.sbt | 93 ++++++++++++++++++- .../src/test/java/module-info.java | 6 ++ .../RuntimeProjectContextTest.scala | 1 - project/JPMSPlugin.scala | 42 ++++----- 4 files changed, 115 insertions(+), 27 deletions(-) create mode 100644 engine/runtime-with-instruments/src/test/java/module-info.java diff --git a/build.sbt b/build.sbt index dd2f7fd1982c..3981af6241e3 100644 --- a/build.sbt +++ b/build.sbt @@ -1944,14 +1944,12 @@ lazy val `runtime-fat-jar` = lazy val `runtime-with-instruments` = (project in file("engine/runtime-with-instruments")) + .enablePlugins(JPMSPlugin) .configs(Benchmark) .settings( frgaalJavaCompilerSetting, inConfig(Compile)(truffleRunOptionsSettings), commands += WithDebugCommand.withDebug, - Test / javaOptions ++= testLogProviderOptions ++ Seq( - "-Dpolyglotimpl.DisableClassPathIsolation=true" - ), Test / fork := true, Test / envVars ++= distributionEnvironmentOverrides ++ Map( "ENSO_TEST_DISABLE_IR_CACHE" -> "false" @@ -1972,6 +1970,95 @@ lazy val `runtime-with-instruments` = file("") } ) + /** + * JPMS related settings + */ + .settings( + Test / excludeFilter := "module-info.java", + Test / javaModuleName := "org.enso.runtime.with.instruments", + Test / modulePath := { + val runtimeMod = (`runtime-fat-jar` / Compile / productDirectories).value.head + val updateReport = (Test / update).value + val requiredModIds = + GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ Seq( + "org.slf4j" % "slf4j-api" % slf4jVersion + ) + val requiredMods = JPMSUtils.filterModulesFromUpdate( + updateReport, + requiredModIds, + streams.value.log, + shouldContainAll = true + ) + requiredMods ++ Seq( + runtimeMod + ) + }, + Test / addModules := { + val runtimeModName = (`runtime-fat-jar` / javaModuleName).value + Seq( + runtimeModName + ) + }, + Test / compileModuleInfo := Def.taskDyn { + JPMSUtils.compileModuleInfoInScope( + extraModulePath = (Test / modulePath).value, + scope = ScopeFilter(configurations = inConfigurations(Test)) + ) + } + .dependsOn(Test / compile) + .value, + (Test / patchModules) := { + val modulesToPatchIntoRuntime: Seq[File] = + (LocalProject( + "runtime-instrument-common" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-id-execution" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-repl-debugger" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-runtime-server" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-language-epb" + ) / Compile / productDirectories).value ++ + (LocalProject("runtime-compiler") / Compile / productDirectories).value + + val testClassesDir = (Test / productDirectories).value.head + val runtimeModName = (`runtime-fat-jar` / javaModuleName).value + Map( + runtimeModName -> modulesToPatchIntoRuntime + ) + }, + (Test / addReads) := { + val runtimeModName = (`runtime-fat-jar` / javaModuleName).value + Map( + runtimeModName -> Seq( + "ALL-UNNAMED" + ) + ) + }, + (Test / javaOptions) ++= { + // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not + // in a module, and it cannot be simple wrapped inside a module. + // So we use plain ch.qos.logback with its configuration. + val testLogbackConf = (LocalProject( + "logging-service-logback" + ) / Test / sourceDirectory).value / "resources" / "logback-test.xml" + Seq( + "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", + s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}" + ) + }, + Test / test := (Test / test) + .dependsOn(Test / compileModuleInfo) + .value, + Test / testOnly := (Test / testOnly) + .dependsOn(Test / compileModuleInfo) + .evaluated + ) /** Benchmark settings */ .settings( inConfig(Benchmark)(Defaults.testSettings), diff --git a/engine/runtime-with-instruments/src/test/java/module-info.java b/engine/runtime-with-instruments/src/test/java/module-info.java new file mode 100644 index 000000000000..aae6cb9dabaa --- /dev/null +++ b/engine/runtime-with-instruments/src/test/java/module-info.java @@ -0,0 +1,6 @@ +open module org.enso.runtime.with.instruments { + requires org.enso.runtime; + requires org.graalvm.truffle; + // TODO: Provides instrument + +} diff --git a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeProjectContextTest.scala b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeProjectContextTest.scala index 44efe061f755..ad4092a5f14a 100644 --- a/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeProjectContextTest.scala +++ b/engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeProjectContextTest.scala @@ -44,7 +44,6 @@ class RuntimeProjectContextTest .toFile .getAbsolutePath ) - .option("engine.WarnInterpreterOnly", "false") .option(RuntimeOptions.EDITION_OVERRIDE, "0.0.0-dev") .option(RuntimeOptions.LOG_LEVEL, Level.WARNING.getName) .logHandler(System.err) diff --git a/project/JPMSPlugin.scala b/project/JPMSPlugin.scala index e18bdf7cf3de..1f2772b0ecd9 100644 --- a/project/JPMSPlugin.scala +++ b/project/JPMSPlugin.scala @@ -12,7 +12,7 @@ import java.io.File * enable this plugin in a project with `.enablePlugins(JPMSPlugin)`. The keys and tasks provided by this plugin * corresponds to the module-related options of `javac` and `java` commands. * - * This plugin injects all the module-specific options to `javaOptions` and `javacOptions`, based on + * This plugin injects all the module-specific options to `javaOptions`, based on * the settings of this plugin. * * If this plugin is enabled, and no settings/tasks from this plugin are used, then the plugin will @@ -65,55 +65,51 @@ object JPMSPlugin extends AutoPlugin { addExports := Map.empty, addReads := Map.empty, compileModuleInfo := {}, + // javacOptions only inject --module-path and --add-modules, not the rest of the + // options. Compile / javacOptions ++= { constructOptions( - (Compile / modulePath).value, - (Compile / addModules).value, - (Compile / patchModules).value, - (Compile / addExports).value, - (Compile / addReads).value, - streams.value.log + streams.value.log, + modulePath = (Compile / modulePath).value, + addModules = (Compile / addModules).value ) }, Compile / javaOptions ++= { constructOptions( + streams.value.log, (Compile / modulePath).value, (Compile / addModules).value, (Compile / patchModules).value, (Compile / addExports).value, - (Compile / addReads).value, - streams.value.log + (Compile / addReads).value ) }, Test / javacOptions ++= { constructOptions( - (Test / modulePath).value, - (Test / addModules).value, - (Test / patchModules).value, - (Test / addExports).value, - (Test / addReads).value, - streams.value.log + streams.value.log, + modulePath = (Test / modulePath).value, + addModules = (Test / addModules).value ) }, Test / javaOptions ++= { constructOptions( + streams.value.log, (Test / modulePath).value, (Test / addModules).value, (Test / patchModules).value, (Test / addExports).value, - (Test / addReads).value, - streams.value.log + (Test / addReads).value ) } ) - private def constructOptions( + def constructOptions( + log: Logger, modulePath: Seq[File], - addModules: Seq[String], - patchModules: Map[String, Seq[File]], - addExports: Map[String, Seq[String]], - addReads: Map[String, Seq[String]], - log: Logger + addModules: Seq[String] = Seq.empty, + patchModules: Map[String, Seq[File]] = Map.empty, + addExports: Map[String, Seq[String]] = Map.empty, + addReads: Map[String, Seq[String]] = Map.empty ): Seq[String] = { val patchOpts: Seq[String] = patchModules.flatMap { case (moduleName, dirsToPatch) => From acb27fc638a2071e4a076e31ee468f10ab7d4312 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 12 Dec 2023 18:36:09 +0100 Subject: [PATCH 20/47] fmt --- build.sbt | 73 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/build.sbt b/build.sbt index 3981af6241e3..5a2259112caf 100644 --- a/build.sbt +++ b/build.sbt @@ -1613,6 +1613,7 @@ lazy val runtime = (project in file("engine/runtime")) Seq(runtimeMod) }, Test / patchModules := { + /** All these modules will be in --patch-module cmdline option to java, which means that * for the JVM, it will appear that all the classes contained in these sbt projects are contained * in the `org.enso.runtime` module. In this way, we do not have to assembly the `runtime.jar` @@ -1646,11 +1647,12 @@ lazy val runtime = (project in file("engine/runtime")) }, Test / addReads := { val runtimeModName = (`runtime-fat-jar` / javaModuleName).value - val testInstrumentsModName = (`runtime-test-instruments` / javaModuleName).value + val testInstrumentsModName = + (`runtime-test-instruments` / javaModuleName).value Map( // We patched the test-classes into the runtime module. These classes access some stuff from // unnamed module. Thus, let's add ALL-UNNAMED. - runtimeModName -> Seq("ALL-UNNAMED", testInstrumentsModName), + runtimeModName -> Seq("ALL-UNNAMED", testInstrumentsModName), testInstrumentsModName -> Seq(runtimeModName) ) }, @@ -1970,14 +1972,14 @@ lazy val `runtime-with-instruments` = file("") } ) - /** - * JPMS related settings - */ + /** JPMS related settings + */ .settings( Test / excludeFilter := "module-info.java", Test / javaModuleName := "org.enso.runtime.with.instruments", Test / modulePath := { - val runtimeMod = (`runtime-fat-jar` / Compile / productDirectories).value.head + val runtimeMod = + (`runtime-fat-jar` / Compile / productDirectories).value.head val updateReport = (Test / update).value val requiredModIds = GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ Seq( @@ -1999,46 +2001,49 @@ lazy val `runtime-with-instruments` = runtimeModName ) }, - Test / compileModuleInfo := Def.taskDyn { - JPMSUtils.compileModuleInfoInScope( - extraModulePath = (Test / modulePath).value, - scope = ScopeFilter(configurations = inConfigurations(Test)) - ) - } + Test / compileModuleInfo := Def + .taskDyn { + JPMSUtils.compileModuleInfoInScope( + extraModulePath = (Test / modulePath).value, + scope = ScopeFilter(configurations = inConfigurations(Test)) + ) + } .dependsOn(Test / compile) .value, (Test / patchModules) := { val modulesToPatchIntoRuntime: Seq[File] = - (LocalProject( - "runtime-instrument-common" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-instrument-id-execution" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-instrument-repl-debugger" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-instrument-runtime-server" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-language-epb" - ) / Compile / productDirectories).value ++ - (LocalProject("runtime-compiler") / Compile / productDirectories).value + (LocalProject( + "runtime-instrument-common" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-id-execution" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-repl-debugger" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-runtime-server" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-language-epb" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-compiler" + ) / Compile / productDirectories).value val testClassesDir = (Test / productDirectories).value.head - val runtimeModName = (`runtime-fat-jar` / javaModuleName).value + val runtimeModName = (`runtime-fat-jar` / javaModuleName).value Map( runtimeModName -> modulesToPatchIntoRuntime ) }, (Test / addReads) := { - val runtimeModName = (`runtime-fat-jar` / javaModuleName).value - Map( - runtimeModName -> Seq( - "ALL-UNNAMED" - ) + val runtimeModName = (`runtime-fat-jar` / javaModuleName).value + Map( + runtimeModName -> Seq( + "ALL-UNNAMED" ) + ) }, (Test / javaOptions) ++= { // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not From 78c40a14be5462152759bf6eb06cb1a633184661 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 12 Dec 2023 18:37:09 +0100 Subject: [PATCH 21/47] Fix some sbt issues with lazy val settings --- build.sbt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/build.sbt b/build.sbt index 5a2259112caf..0ee7a822c737 100644 --- a/build.sbt +++ b/build.sbt @@ -1824,7 +1824,7 @@ lazy val `runtime-instrument-id-execution` = inConfig(Compile)(truffleRunOptionsSettings), instrumentationSettings ) - .dependsOn(runtime) + .dependsOn(LocalProject("runtime")) .dependsOn(`runtime-instrument-common`) lazy val `runtime-instrument-repl-debugger` = @@ -1833,7 +1833,7 @@ lazy val `runtime-instrument-repl-debugger` = inConfig(Compile)(truffleRunOptionsSettings), instrumentationSettings ) - .dependsOn(runtime) + .dependsOn(LocalProject("runtime")) .dependsOn(`runtime-instrument-common`) lazy val `runtime-instrument-runtime-server` = @@ -1842,7 +1842,7 @@ lazy val `runtime-instrument-runtime-server` = inConfig(Compile)(truffleRunOptionsSettings), instrumentationSettings ) - .dependsOn(runtime) + .dependsOn(LocalProject("runtime")) .dependsOn(`runtime-instrument-common`) /** A "meta" project that exists solely to provide logic for assembling the `runtime.jar` fat Jar. @@ -1942,7 +1942,7 @@ lazy val `runtime-fat-jar` = .dependsOn(`runtime-instrument-repl-debugger`) .dependsOn(`runtime-instrument-runtime-server`) .dependsOn(`runtime-language-epb`) - .dependsOn(runtime) + .dependsOn(LocalProject("runtime")) lazy val `runtime-with-instruments` = (project in file("engine/runtime-with-instruments")) From b98526ee882e4c10ce56d90b6750fd6e3b1754a1 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 13 Dec 2023 17:44:20 +0100 Subject: [PATCH 22/47] Use service provider instead of reflection in runtime-test-instruments --- build.sbt | 12 ++++-- .../src/main/java/module-info.java | 1 + .../instruments/CodeIdsTestInstrument.java | 43 +++++-------------- .../test/instruments/RuntimeTestService.java | 14 ++++++ .../test/RuntimeTestServiceImpl.java | 32 ++++++++++++++ 5 files changed, 67 insertions(+), 35 deletions(-) create mode 100644 engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/RuntimeTestService.java create mode 100644 engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java diff --git a/build.sbt b/build.sbt index 0ee7a822c737..a63801b8a7ef 100644 --- a/build.sbt +++ b/build.sbt @@ -1522,12 +1522,17 @@ lazy val `runtime-test-instruments` = modulePath := { JPMSUtils.filterModulesFromUpdate( update.value, - GraalVM.modules, + GraalVM.modules ++ Seq( + "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion + ), streams.value.log, shouldContainAll = true ) }, - libraryDependencies ++= GraalVM.modules + libraryDependencies ++= GraalVM.modules, + libraryDependencies ++= Seq( + "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion % "provided", + ) ) lazy val runtime = (project in file("engine/runtime")) @@ -1596,7 +1601,8 @@ lazy val runtime = (project in file("engine/runtime")) val updateReport = (Test / update).value val requiredModIds = GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ Seq( - "org.slf4j" % "slf4j-api" % slf4jVersion + "org.slf4j" % "slf4j-api" % slf4jVersion, + "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion ) val requiredMods = JPMSUtils.filterModulesFromUpdate( updateReport, diff --git a/engine/runtime-test-instruments/src/main/java/module-info.java b/engine/runtime-test-instruments/src/main/java/module-info.java index a7e253a72219..14ca672e5eb7 100644 --- a/engine/runtime-test-instruments/src/main/java/module-info.java +++ b/engine/runtime-test-instruments/src/main/java/module-info.java @@ -1,5 +1,6 @@ module org.enso.runtime.test { requires org.graalvm.truffle; + requires org.openide.util.lookup.RELEASE180; exports org.enso.interpreter.test.instruments; diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java index a209cf3536ae..ad2d27662612 100644 --- a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java @@ -3,11 +3,11 @@ import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.*; import com.oracle.truffle.api.nodes.Node; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.util.LinkedHashMap; import java.util.Map; +import org.openide.util.Lookup; + import java.util.UUID; /** @@ -21,22 +21,11 @@ services = CodeIdsTestInstrument.class) public class CodeIdsTestInstrument extends TruffleInstrument { public static final String INSTRUMENT_ID = "ids-test"; - private static final Class exprNodeClass; - private static final Method exprNodeGetId; - private static final Class tailCallExceptionClass; + private static final RuntimeTestService runtimeTestService; private Env env; - /** - * Use reflection to call methods from `runtime` project. We cannot use `runtime` as a dependency. - */ static { - try { - exprNodeClass = Class.forName("org.enso.interpreter.node.ExpressionNode"); - exprNodeGetId = exprNodeClass.getMethod("getId"); - tailCallExceptionClass = Class.forName("org.enso.interpreter.runtime.control.TailCallException"); - } catch (ClassNotFoundException | NoSuchMethodException e) { - throw new AssertionError(e); - } + runtimeTestService = Lookup.getDefault().lookup(RuntimeTestService.class); } /** @@ -120,11 +109,11 @@ public void onReturnValue(VirtualFrame frame, Object result) { return; } Node node = context.getInstrumentedNode(); - if (!(isInstanceOf(node, exprNodeClass))) { + if (!runtimeTestService.isExpressionNode(node)) { return; } nodes.put(this, result); - UUID id = exprNodeGetId(node); + UUID id = runtimeTestService.getExpressionNodeID(node); if (id == null || !id.equals(expectedId)) { return; } @@ -137,31 +126,21 @@ private boolean isInstanceOf(Object obj, Class clazz) { return clazz.isInstance(obj); } - private UUID exprNodeGetId(Object exprNode) { - assert isInstanceOf(exprNode, exprNodeClass); - try { - return (UUID) exprNodeGetId.invoke(exprNode); - } catch (IllegalAccessException | InvocationTargetException e) { - throw new AssertionError(e); - } - } - /** * Checks if the specified was called, if its execution triggered TCO. * - * @param context current execution context. * @param frame current execution frame. * @param exception the exception thrown from this node's execution. */ @Override public void onReturnExceptional(VirtualFrame frame, Throwable exception) { - if (!isInstanceOf(exception, tailCallExceptionClass)) { + if (!runtimeTestService.isTailCallException(exception)) { return; } - if (!isInstanceOf(context.getInstrumentedNode(), exprNodeClass)) { + if (!runtimeTestService.isExpressionNode(context.getInstrumentedNode())) { return; } - UUID id = exprNodeGetId(context.getInstrumentedNode()); + UUID id = runtimeTestService.getExpressionNodeID(context.getInstrumentedNode()); if (expectedResult == null) { successful = true; } @@ -171,8 +150,8 @@ public void onReturnExceptional(VirtualFrame frame, Throwable exception) { public String toString() { var sb = new StringBuilder(); sb.append(context.getInstrumentedNode().getClass().getSimpleName()); - if (isInstanceOf(context.getInstrumentedNode(), exprNodeClass)) { - UUID id = exprNodeGetId(context.getInstrumentedNode()); + if (runtimeTestService.isExpressionNode(context.getInstrumentedNode())) { + UUID id = runtimeTestService.getExpressionNodeID(context.getInstrumentedNode()); sb.append("@").append(id); } sb.append(" "); diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/RuntimeTestService.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/RuntimeTestService.java new file mode 100644 index 000000000000..bcfb042a0305 --- /dev/null +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/RuntimeTestService.java @@ -0,0 +1,14 @@ +package org.enso.interpreter.test.instruments; + +import java.util.UUID; + +/** + * A service that provides information from the `runtime` project to the instruments in this project + * (`runtime-test-instruments`). Note that this project cannot have a compile time dependency on + * `runtime`, thus, we need to use a service provider mechanism. + */ +public interface RuntimeTestService { + UUID getExpressionNodeID(Object expressionNode); + boolean isExpressionNode(Object node); + boolean isTailCallException(Object obj); +} diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java b/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java new file mode 100644 index 000000000000..01aec41206e2 --- /dev/null +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java @@ -0,0 +1,32 @@ +package org.enso.interpreter.test; + +import java.util.UUID; +import org.enso.interpreter.node.ExpressionNode; +import org.enso.interpreter.runtime.control.TailCallException; +import org.enso.interpreter.test.instruments.RuntimeTestService; +import org.openide.util.lookup.ServiceProvider; + +@ServiceProvider( + service = RuntimeTestService.class +) +public class RuntimeTestServiceImpl implements RuntimeTestService { + + @Override + public UUID getExpressionNodeID(Object expressionNode) { + if (expressionNode instanceof ExpressionNode exprNode) { + return exprNode.getId(); + } else { + throw new IllegalArgumentException("Expected ExpressionNode, got " + expressionNode); + } + } + + @Override + public boolean isExpressionNode(Object node) { + return node instanceof ExpressionNode; + } + + @Override + public boolean isTailCallException(Object obj) { + return obj instanceof TailCallException; + } +} From 07f5fa4a13d2cf6ca17f50b1fa81cabf36e8b8a9 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 14 Dec 2023 13:04:11 +0100 Subject: [PATCH 23/47] Port NodeCountingTestInstrument into runtime-test-instruments --- .../src/main/java/module-info.java | 3 +- .../instruments/CodeIdsTestInstrument.java | 7 +- .../NodeCountingTestInstrument.java | 91 ++++--------------- .../instruments/service/FunctionCallInfo.java | 13 +++ .../{ => service}/RuntimeTestService.java | 10 +- .../AvoidIdInstrumentationTagTest.java | 3 +- .../instrument/IncrementalUpdatesTest.java | 2 +- .../WarningInstrumentationTest.java | 12 +-- .../test/RuntimeTestServiceImpl.java | 44 +++++++-- 9 files changed, 91 insertions(+), 94 deletions(-) rename engine/{runtime-with-instruments/src/test/java/org/enso/interpreter/test => runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments}/NodeCountingTestInstrument.java (59%) create mode 100644 engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/service/FunctionCallInfo.java rename engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/{ => service}/RuntimeTestService.java (50%) diff --git a/engine/runtime-test-instruments/src/main/java/module-info.java b/engine/runtime-test-instruments/src/main/java/module-info.java index 14ca672e5eb7..21946a837d95 100644 --- a/engine/runtime-test-instruments/src/main/java/module-info.java +++ b/engine/runtime-test-instruments/src/main/java/module-info.java @@ -3,7 +3,8 @@ requires org.openide.util.lookup.RELEASE180; exports org.enso.interpreter.test.instruments; - + exports org.enso.interpreter.test.instruments.service; + provides com.oracle.truffle.api.instrumentation.provider.TruffleInstrumentProvider with org.enso.interpreter.test.instruments.CodeIdsTestInstrumentProvider, org.enso.interpreter.test.instruments.CodeLocationsTestInstrumentProvider; diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java index ad2d27662612..8bc69cbee41b 100644 --- a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java @@ -6,6 +6,7 @@ import java.util.LinkedHashMap; import java.util.Map; +import org.enso.interpreter.test.instruments.service.RuntimeTestService; import org.openide.util.Lookup; import java.util.UUID; @@ -113,7 +114,7 @@ public void onReturnValue(VirtualFrame frame, Object result) { return; } nodes.put(this, result); - UUID id = runtimeTestService.getExpressionNodeID(node); + UUID id = runtimeTestService.getNodeID(node); if (id == null || !id.equals(expectedId)) { return; } @@ -140,7 +141,7 @@ public void onReturnExceptional(VirtualFrame frame, Throwable exception) { if (!runtimeTestService.isExpressionNode(context.getInstrumentedNode())) { return; } - UUID id = runtimeTestService.getExpressionNodeID(context.getInstrumentedNode()); + UUID id = runtimeTestService.getNodeID(context.getInstrumentedNode()); if (expectedResult == null) { successful = true; } @@ -151,7 +152,7 @@ public String toString() { var sb = new StringBuilder(); sb.append(context.getInstrumentedNode().getClass().getSimpleName()); if (runtimeTestService.isExpressionNode(context.getInstrumentedNode())) { - UUID id = runtimeTestService.getExpressionNodeID(context.getInstrumentedNode()); + UUID id = runtimeTestService.getNodeID(context.getInstrumentedNode()); sb.append("@").append(id); } sb.append(" "); diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/NodeCountingTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/NodeCountingTestInstrument.java similarity index 59% rename from engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/NodeCountingTestInstrument.java rename to engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/NodeCountingTestInstrument.java index 1f786b1c07af..3b4b6b9c97f1 100644 --- a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/NodeCountingTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/NodeCountingTestInstrument.java @@ -1,4 +1,4 @@ -package org.enso.interpreter.test; +package org.enso.interpreter.test.instruments; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.EventContext; @@ -7,20 +7,17 @@ import com.oracle.truffle.api.instrumentation.SourceSectionFilter; import com.oracle.truffle.api.instrumentation.TruffleInstrument; import com.oracle.truffle.api.nodes.Node; -import com.oracle.truffle.api.nodes.RootNode; import com.oracle.truffle.api.source.SourceSection; -import org.enso.interpreter.node.MethodRootNode; -import org.enso.interpreter.node.callable.FunctionCallInstrumentationNode; -import org.enso.pkg.QualifiedName; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Function; -import static org.junit.Assert.fail; +import org.enso.interpreter.test.instruments.service.FunctionCallInfo; +import org.enso.interpreter.test.instruments.service.RuntimeTestService; +import org.openide.util.Lookup; /** Testing instrument to control newly created nodes. */ @TruffleInstrument.Registration( @@ -28,12 +25,17 @@ services = NodeCountingTestInstrument.class) public class NodeCountingTestInstrument extends TruffleInstrument { public static final String INSTRUMENT_ID = "node-count-test"; + private static final RuntimeTestService runtimeTestService; private final Map all = new ConcurrentHashMap<>(); private Map> counter = new ConcurrentHashMap<>(); private final Map calls = new ConcurrentHashMap<>(); private Env env; + static { + runtimeTestService = Lookup.getDefault().lookup(RuntimeTestService.class); + } + @Override protected void onCreate(Env env) { env.registerService(this); @@ -75,10 +77,10 @@ public Map> assertNewNodes(String msg, int min, int max) { }; if (value < min) { - fail(dump.apply(msg + ". Minimal size should be " + min + ", but was: " + value + " in")); + throw new AssertionError(dump.apply(msg + ". Minimal size should be " + min + ", but was: " + value + " in")); } if (value > max) { - fail(dump.apply(msg + ". Maximal size should be " + max + ", but was: " + value + " in")); + throw new AssertionError(dump.apply(msg + ". Maximal size should be " + max + ", but was: " + value + " in")); } counter = new ConcurrentHashMap<>(); return prev; @@ -120,73 +122,16 @@ public NodeWrapper(EventContext context, Map calls) { public void onReturnValue(VirtualFrame frame, Object result) { Node node = context.getInstrumentedNode(); - if (node instanceof FunctionCallInstrumentationNode instrumentableNode - && result instanceof FunctionCallInstrumentationNode.FunctionCall functionCall) { - onFunctionReturn(instrumentableNode, functionCall); - } - } - - private void onFunctionReturn(FunctionCallInstrumentationNode node, FunctionCallInstrumentationNode.FunctionCall result) { - if (node.getId() != null) { - calls.put(node.getId(), new FunctionCallInfo(result)); - } - } - } - - public static class FunctionCallInfo { - - private final QualifiedName moduleName; - private final QualifiedName typeName; - private final String functionName; - - public FunctionCallInfo(FunctionCallInstrumentationNode.FunctionCall call) { - RootNode rootNode = call.getFunction().getCallTarget().getRootNode(); - if (rootNode instanceof MethodRootNode methodNode) { - moduleName = methodNode.getModuleScope().getModule().getName(); - typeName = methodNode.getType().getQualifiedName(); - functionName = methodNode.getMethodName(); - } else { - moduleName = null; - typeName = null; - functionName = rootNode.getName(); - } - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; + if (runtimeTestService.isFunctionCallInstrumentationNode(node) + && runtimeTestService.isFunctionCall(result)) { + UUID nodeId = runtimeTestService.getNodeID(node); + if (nodeId != null) { + var funcCallInfo = runtimeTestService.extractFunctionCallInfo(result); + calls.put(nodeId, funcCallInfo); + } } - FunctionCallInfo that = (FunctionCallInfo) o; - return Objects.equals(moduleName, that.moduleName) - && Objects.equals(typeName, that.typeName) - && Objects.equals(functionName, that.functionName); - } - - @Override - public int hashCode() { - return Objects.hash(moduleName, typeName, functionName); - } - - @Override - public String toString() { - return moduleName + "::" + typeName + "::" + functionName; - } - - public QualifiedName getModuleName() { - return moduleName; - } - - public QualifiedName getTypeName() { - return typeName; - } - public String getFunctionName() { - return functionName; } } } diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/service/FunctionCallInfo.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/service/FunctionCallInfo.java new file mode 100644 index 000000000000..d859d59a1e74 --- /dev/null +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/service/FunctionCallInfo.java @@ -0,0 +1,13 @@ +package org.enso.interpreter.test.instruments.service; + +public record FunctionCallInfo( + String moduleName, + String typeName, + String functionName +) { + + @Override + public String toString() { + return moduleName + "::" + typeName + "::" + functionName; + } +} diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/RuntimeTestService.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/service/RuntimeTestService.java similarity index 50% rename from engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/RuntimeTestService.java rename to engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/service/RuntimeTestService.java index bcfb042a0305..8672fc95cb46 100644 --- a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/RuntimeTestService.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/service/RuntimeTestService.java @@ -1,6 +1,9 @@ -package org.enso.interpreter.test.instruments; +package org.enso.interpreter.test.instruments.service; +import com.oracle.truffle.api.nodes.Node; +import com.oracle.truffle.api.nodes.RootNode; import java.util.UUID; +import org.enso.interpreter.test.instruments.service.FunctionCallInfo; /** * A service that provides information from the `runtime` project to the instruments in this project @@ -8,7 +11,10 @@ * `runtime`, thus, we need to use a service provider mechanism. */ public interface RuntimeTestService { - UUID getExpressionNodeID(Object expressionNode); + UUID getNodeID(Node node); boolean isExpressionNode(Object node); boolean isTailCallException(Object obj); + boolean isFunctionCallInstrumentationNode(Object node); + boolean isFunctionCall(Object obj); + FunctionCallInfo extractFunctionCallInfo(Object functionCall); } diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java index 0b261ee8fa52..2d3dd68bc7b8 100644 --- a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java +++ b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java @@ -3,7 +3,6 @@ import com.oracle.truffle.api.instrumentation.StandardTags; import com.oracle.truffle.api.nodes.RootNode; import com.oracle.truffle.api.source.SourceSection; -import java.io.OutputStream; import java.nio.file.Paths; import java.util.Map; import java.util.function.Predicate; @@ -12,7 +11,7 @@ import org.enso.interpreter.node.ClosureRootNode; import org.enso.interpreter.runtime.tag.AvoidIdInstrumentationTag; import org.enso.interpreter.runtime.tag.IdentifiedTag; -import org.enso.interpreter.test.NodeCountingTestInstrument; +import org.enso.interpreter.test.instruments.NodeCountingTestInstrument; import org.enso.polyglot.RuntimeOptions; import org.graalvm.polyglot.Context; import org.graalvm.polyglot.Language; diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java index fa564515e0fe..509d1f8e1d74 100644 --- a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java +++ b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java @@ -10,7 +10,7 @@ import org.enso.interpreter.node.expression.literal.LiteralNode; import org.enso.interpreter.runtime.type.ConstantsGen; import org.enso.interpreter.test.Metadata; -import org.enso.interpreter.test.NodeCountingTestInstrument; +import org.enso.interpreter.test.instruments.NodeCountingTestInstrument; import org.enso.interpreter.test.instrument.RuntimeServerTest.TestContext; import org.enso.polyglot.runtime.Runtime$Api$CreateContextRequest; import org.enso.polyglot.runtime.Runtime$Api$CreateContextResponse; diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java index 5e23f6cbd1d2..2d02c893d750 100644 --- a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java +++ b/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java @@ -5,12 +5,13 @@ import org.enso.interpreter.runtime.tag.AvoidIdInstrumentationTag; import org.enso.interpreter.runtime.tag.IdentifiedTag; import org.enso.interpreter.test.Metadata; -import org.enso.interpreter.test.NodeCountingTestInstrument; +import org.enso.interpreter.test.instruments.NodeCountingTestInstrument; import org.enso.polyglot.RuntimeOptions; import org.graalvm.polyglot.Context; import org.graalvm.polyglot.Language; import org.graalvm.polyglot.Source; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import org.graalvm.polyglot.io.IOAccess; import org.junit.After; @@ -18,7 +19,6 @@ import org.junit.Before; import org.junit.Test; -import java.io.OutputStream; import java.nio.file.Paths; import java.util.Map; import java.util.logging.Level; @@ -91,9 +91,9 @@ public void instrumentValueWithWarnings() throws Exception { var calls = instrument.registeredCalls(); assertEquals(calls.keySet().size(), 3); - assertEquals(calls.get(idOp1).getFunctionName(), "new"); - assertEquals(calls.get(idOp2).getFunctionName(), "attach"); - assertEquals(calls.get(idOp3).getTypeName().item(), "Table"); - assertEquals(calls.get(idOp3).getFunctionName(), "get"); + assertEquals(calls.get(idOp1).functionName(), "new"); + assertEquals(calls.get(idOp2).functionName(), "attach"); + assertTrue(calls.get(idOp3).typeName().contains("Table")); + assertEquals(calls.get(idOp3).functionName(), "get"); } } diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java b/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java index 01aec41206e2..6efbfc9bdb5a 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java @@ -1,23 +1,29 @@ package org.enso.interpreter.test; +import com.oracle.truffle.api.nodes.Node; +import com.oracle.truffle.api.nodes.RootNode; import java.util.UUID; import org.enso.interpreter.node.ExpressionNode; +import org.enso.interpreter.node.MethodRootNode; +import org.enso.interpreter.node.callable.FunctionCallInstrumentationNode; +import org.enso.interpreter.node.callable.FunctionCallInstrumentationNode.FunctionCall; import org.enso.interpreter.runtime.control.TailCallException; -import org.enso.interpreter.test.instruments.RuntimeTestService; +import org.enso.interpreter.test.instruments.service.FunctionCallInfo; +import org.enso.interpreter.test.instruments.service.RuntimeTestService; import org.openide.util.lookup.ServiceProvider; @ServiceProvider( service = RuntimeTestService.class ) public class RuntimeTestServiceImpl implements RuntimeTestService { - @Override - public UUID getExpressionNodeID(Object expressionNode) { - if (expressionNode instanceof ExpressionNode exprNode) { + public UUID getNodeID(Node node) { + if (node instanceof ExpressionNode exprNode) { return exprNode.getId(); - } else { - throw new IllegalArgumentException("Expected ExpressionNode, got " + expressionNode); + } else if (node instanceof FunctionCallInstrumentationNode funcNode){ + return funcNode.getId(); } + return null; } @Override @@ -29,4 +35,30 @@ public boolean isExpressionNode(Object node) { public boolean isTailCallException(Object obj) { return obj instanceof TailCallException; } + + @Override + public boolean isFunctionCallInstrumentationNode(Object node) { + return node instanceof FunctionCallInstrumentationNode; + } + + @Override + public boolean isFunctionCall(Object obj) { + return obj instanceof FunctionCall; + } + + @Override + public FunctionCallInfo extractFunctionCallInfo(Object functionCallObj) { + if (functionCallObj instanceof FunctionCall functionCall) { + RootNode rootNode = functionCall.getFunction().getCallTarget().getRootNode(); + if (rootNode instanceof MethodRootNode methodNode) { + String moduleName = methodNode.getModuleScope().getModule().getName().toString(); + String typeName = methodNode.getType().getQualifiedName().toString(); + String functionName = methodNode.getMethodName(); + return new FunctionCallInfo(moduleName, typeName, functionName); + } else { + return new FunctionCallInfo(null, null, rootNode.getName()); + } + } + return null; + } } From 05b221a08fee23b93b04177064056187f10577ec Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 14 Dec 2023 16:00:44 +0100 Subject: [PATCH 24/47] Merge runtime-with-instruments project into runtime/Test --- build.sbt | 143 +----------------- .../src/main/java/module-info.java | 3 +- .../src/test/java/module-info.java | 6 - .../test/FindExceptionMessageTest.java | 0 .../AvoidIdInstrumentationTagTest.java | 0 .../instrument/IncrementalUpdatesTest.java | 15 +- .../WarningInstrumentationTest.java | 0 .../src/test/resources/application.conf | 0 .../test/instrument/BuiltinTypesTest.scala | 0 .../test/instrument/ReplTest.scala | 0 .../instrument/RuntimeAsyncCommandsTest.scala | 0 .../instrument/RuntimeComponentsTest.scala | 10 +- .../test/instrument/RuntimeErrorsTest.scala | 0 .../RuntimeExecutionEnvironmentTest.scala | 0 .../instrument/RuntimeInstrumentTest.scala | 0 .../RuntimeProjectContextTest.scala | 2 +- .../instrument/RuntimeRefactoringTest.scala | 2 +- .../test/instrument/RuntimeServerTest.scala | 4 +- .../test/instrument/RuntimeStdlibTest.scala | 0 .../RuntimeSuggestionUpdatesTest.scala | 0 20 files changed, 17 insertions(+), 168 deletions(-) delete mode 100644 engine/runtime-with-instruments/src/test/java/module-info.java rename engine/{runtime-with-instruments => runtime}/src/test/java/org/enso/interpreter/test/FindExceptionMessageTest.java (100%) rename engine/{runtime-with-instruments => runtime}/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java (100%) rename engine/{runtime-with-instruments => runtime}/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java (95%) rename engine/{runtime-with-instruments => runtime}/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java (100%) rename engine/{runtime-with-instruments => runtime}/src/test/resources/application.conf (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/BuiltinTypesTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/ReplTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeAsyncCommandsTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala (98%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeExecutionEnvironmentTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeProjectContextTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeRefactoringTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeStdlibTest.scala (100%) rename engine/{runtime-with-instruments => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeSuggestionUpdatesTest.scala (100%) diff --git a/build.sbt b/build.sbt index a63801b8a7ef..bcc9de66c7fd 100644 --- a/build.sbt +++ b/build.sbt @@ -302,7 +302,6 @@ lazy val enso = (project in file(".")) `runtime-instrument-id-execution`, `runtime-instrument-repl-debugger`, `runtime-instrument-runtime-server`, - `runtime-with-instruments`, `runtime-with-polyglot`, `runtime-version-manager`, `runtime-version-manager-test`, @@ -1641,7 +1640,8 @@ lazy val runtime = (project in file("engine/runtime")) (LocalProject( "runtime-language-epb" ) / Compile / productDirectories).value ++ - (LocalProject("runtime-compiler") / Compile / productDirectories).value + (LocalProject("runtime-compiler") / Compile / productDirectories).value ++ + (LocalProject("refactoring-utils") / Compile / productDirectories).value // Patch test-classes into the runtime module. This is standard way to deal with the // split package problem in unit tests. For example, Maven's surefire plugin does this. val testClassesDir = (Test / productDirectories).value.head @@ -1950,144 +1950,6 @@ lazy val `runtime-fat-jar` = .dependsOn(`runtime-language-epb`) .dependsOn(LocalProject("runtime")) -lazy val `runtime-with-instruments` = - (project in file("engine/runtime-with-instruments")) - .enablePlugins(JPMSPlugin) - .configs(Benchmark) - .settings( - frgaalJavaCompilerSetting, - inConfig(Compile)(truffleRunOptionsSettings), - commands += WithDebugCommand.withDebug, - Test / fork := true, - Test / envVars ++= distributionEnvironmentOverrides ++ Map( - "ENSO_TEST_DISABLE_IR_CACHE" -> "false" - ), - libraryDependencies ++= Seq( - "org.scalatest" %% "scalatest" % scalatestVersion % Test, - "org.graalvm.truffle" % "truffle-api" % graalMavenPackagesVersion % Test, - "org.graalvm.truffle" % "truffle-dsl-processor" % graalMavenPackagesVersion % Test, - "org.slf4j" % "slf4j-nop" % slf4jVersion % Benchmark - ), - // Note [Unmanaged Classpath] - Test / unmanagedClasspath += (baseDirectory.value / ".." / ".." / "app" / "gui" / "view" / "graph-editor" / "src" / "builtin" / "visualization" / "native" / "inc"), - assembly := { - streams.value.log.warn( - s"This is an empty task, use `runtime-fat-jar/assembly` instead." - ) - // Return an empty file to satisfy the type checker (the assembly task expect File as a return type) - file("") - } - ) - /** JPMS related settings - */ - .settings( - Test / excludeFilter := "module-info.java", - Test / javaModuleName := "org.enso.runtime.with.instruments", - Test / modulePath := { - val runtimeMod = - (`runtime-fat-jar` / Compile / productDirectories).value.head - val updateReport = (Test / update).value - val requiredModIds = - GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ Seq( - "org.slf4j" % "slf4j-api" % slf4jVersion - ) - val requiredMods = JPMSUtils.filterModulesFromUpdate( - updateReport, - requiredModIds, - streams.value.log, - shouldContainAll = true - ) - requiredMods ++ Seq( - runtimeMod - ) - }, - Test / addModules := { - val runtimeModName = (`runtime-fat-jar` / javaModuleName).value - Seq( - runtimeModName - ) - }, - Test / compileModuleInfo := Def - .taskDyn { - JPMSUtils.compileModuleInfoInScope( - extraModulePath = (Test / modulePath).value, - scope = ScopeFilter(configurations = inConfigurations(Test)) - ) - } - .dependsOn(Test / compile) - .value, - (Test / patchModules) := { - val modulesToPatchIntoRuntime: Seq[File] = - (LocalProject( - "runtime-instrument-common" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-instrument-id-execution" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-instrument-repl-debugger" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-instrument-runtime-server" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-language-epb" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-compiler" - ) / Compile / productDirectories).value - - val testClassesDir = (Test / productDirectories).value.head - val runtimeModName = (`runtime-fat-jar` / javaModuleName).value - Map( - runtimeModName -> modulesToPatchIntoRuntime - ) - }, - (Test / addReads) := { - val runtimeModName = (`runtime-fat-jar` / javaModuleName).value - Map( - runtimeModName -> Seq( - "ALL-UNNAMED" - ) - ) - }, - (Test / javaOptions) ++= { - // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not - // in a module, and it cannot be simple wrapped inside a module. - // So we use plain ch.qos.logback with its configuration. - val testLogbackConf = (LocalProject( - "logging-service-logback" - ) / Test / sourceDirectory).value / "resources" / "logback-test.xml" - Seq( - "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", - s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}" - ) - }, - Test / test := (Test / test) - .dependsOn(Test / compileModuleInfo) - .value, - Test / testOnly := (Test / testOnly) - .dependsOn(Test / compileModuleInfo) - .evaluated - ) - /** Benchmark settings */ - .settings( - inConfig(Benchmark)(Defaults.testSettings), - Benchmark / javacOptions --= Seq( - "-source", - frgaalSourceLevel, - "--enable-preview" - ), - (Benchmark / javaOptions) := - (LocalProject("std-benchmarks") / Benchmark / run / javaOptions).value - ) - .dependsOn(runtime % "compile->compile;test->test;runtime->runtime") - .dependsOn(`runtime-instrument-common`) - .dependsOn(`runtime-instrument-id-execution`) - .dependsOn(`runtime-instrument-repl-debugger`) - .dependsOn(`runtime-instrument-runtime-server`) - .dependsOn(`runtime-language-epb`) - .dependsOn(`logging-service-logback` % "test->test") /* runtime-with-polyglot * ~~~~~~~~~~~~~~~~~~~~~ @@ -2124,7 +1986,6 @@ lazy val `runtime-with-polyglot` = (LocalProject("std-benchmarks") / Benchmark / run / javaOptions).value ) .dependsOn(runtime % "compile->compile;test->test;runtime->runtime") - .dependsOn(`runtime-with-instruments`) /* Note [Unmanaged Classpath] * ~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/engine/runtime-test-instruments/src/main/java/module-info.java b/engine/runtime-test-instruments/src/main/java/module-info.java index 21946a837d95..7d3cfa8e8649 100644 --- a/engine/runtime-test-instruments/src/main/java/module-info.java +++ b/engine/runtime-test-instruments/src/main/java/module-info.java @@ -7,5 +7,6 @@ provides com.oracle.truffle.api.instrumentation.provider.TruffleInstrumentProvider with org.enso.interpreter.test.instruments.CodeIdsTestInstrumentProvider, - org.enso.interpreter.test.instruments.CodeLocationsTestInstrumentProvider; + org.enso.interpreter.test.instruments.CodeLocationsTestInstrumentProvider, + org.enso.interpreter.test.instruments.NodeCountingTestInstrumentProvider; } diff --git a/engine/runtime-with-instruments/src/test/java/module-info.java b/engine/runtime-with-instruments/src/test/java/module-info.java deleted file mode 100644 index aae6cb9dabaa..000000000000 --- a/engine/runtime-with-instruments/src/test/java/module-info.java +++ /dev/null @@ -1,6 +0,0 @@ -open module org.enso.runtime.with.instruments { - requires org.enso.runtime; - requires org.graalvm.truffle; - // TODO: Provides instrument - -} diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/FindExceptionMessageTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/FindExceptionMessageTest.java similarity index 100% rename from engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/FindExceptionMessageTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/FindExceptionMessageTest.java diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java similarity index 100% rename from engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java diff --git a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java similarity index 95% rename from engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java index 509d1f8e1d74..7fac917233e6 100644 --- a/engine/runtime-with-instruments/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java @@ -35,6 +35,7 @@ import static org.junit.Assert.assertTrue; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import scala.Option; @@ -76,7 +77,7 @@ public void sendUpdatesWhenFunctionBodyIsChanged() { var m = context.languageContext().findModule(MODULE_NAME).orElse(null); assertNotNull("Module found", m); var numbers = m.getIr().preorder().filter((v1) -> v1 instanceof Literal.Number); - assertEquals("One number found: " + numbers, 1, numbers.size()); + Assert.assertEquals("One number found: " + numbers, 1, numbers.size()); if (numbers.head() instanceof Literal.Number n) { assertEquals("updated to 5", "5", n.value()); } @@ -96,10 +97,10 @@ public void sendUpdatesWhenWhenLineIsChangedByTextEdit() { public void sendMultipleUpdates() { sendUpdatesWhenFunctionBodyIsChangedBySettingValue("4", ConstantsGen.INTEGER, "4", "1000", "1000", LiteralNode.class); sendExpressionValue("1000", "333"); - assertEquals(List.newBuilder().addOne("333"), context.consumeOut()); + Assert.assertEquals(List.newBuilder().addOne("333"), context.consumeOut()); nodeCountingInstrument.assertNewNodes("No execution on 333, no nodes yet", 0, 0); sendExpressionValue("333", "22"); - assertEquals(List.newBuilder().addOne("22"), context.consumeOut()); + Assert.assertEquals(List.newBuilder().addOne("22"), context.consumeOut()); nodeCountingInstrument.assertNewNodes("No execution on 22, no nodes yet", 0, 0); } @@ -117,7 +118,7 @@ public void sendUpdatesWhenTextIsChangedBySettingValue() { public void sendNotANumberChange() { var result = sendUpdatesWhenFunctionBodyIsChangedBySettingValue("4", ConstantsGen.INTEGER, "4", "x", null, LiteralNode.class); assertTrue("Execution succeeds: " + result, result.head().payload() instanceof Runtime$Api$ExecutionComplete); - assertEquals("Error is printed as a result", + Assert.assertEquals("Error is printed as a result", List.newBuilder().addOne("(Error: Uninitialized value)"), context.consumeOut() ); } @@ -237,7 +238,7 @@ private static String extractPositions(String code, String chars, Map Date: Thu, 14 Dec 2023 16:37:28 +0100 Subject: [PATCH 25/47] Merge runtime-with-polyglot project into runtime/Test --- build.sbt | 37 ------------------- .../test/ConversionMethodTests.java | 0 .../test/ForeignMethodInvokeTest.java | 0 .../interpreter/test/InsightForEnsoTest.java | 0 .../enso/interpreter/test/JsInteropTest.java | 0 .../interpreter/test/MetaIsAPolyglotTest.java | 0 .../test/MetaObjectPolyglotTest.java | 0 .../test/NonStrictConversionMethodTests.java | 0 .../PolyglotFindExceptionMessageTest.java | 0 .../test/instrument/MockLogHandler.java | 0 .../VerifyJavaScriptIsAvailableTest.java | 0 .../RuntimeVisualizationsTest.scala | 0 12 files changed, 37 deletions(-) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/ConversionMethodTests.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/ForeignMethodInvokeTest.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/InsightForEnsoTest.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/JsInteropTest.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/MetaIsAPolyglotTest.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/MetaObjectPolyglotTest.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/NonStrictConversionMethodTests.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/PolyglotFindExceptionMessageTest.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/instrument/MockLogHandler.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/java/org/enso/interpreter/test/instrument/VerifyJavaScriptIsAvailableTest.java (100%) rename engine/{runtime-with-polyglot => runtime}/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala (100%) diff --git a/build.sbt b/build.sbt index bcc9de66c7fd..98047b3a0ea1 100644 --- a/build.sbt +++ b/build.sbt @@ -302,7 +302,6 @@ lazy val enso = (project in file(".")) `runtime-instrument-id-execution`, `runtime-instrument-repl-debugger`, `runtime-instrument-runtime-server`, - `runtime-with-polyglot`, `runtime-version-manager`, `runtime-version-manager-test`, editions, @@ -1951,42 +1950,6 @@ lazy val `runtime-fat-jar` = .dependsOn(LocalProject("runtime")) -/* runtime-with-polyglot - * ~~~~~~~~~~~~~~~~~~~~~ - * Unlike `runtime`, this project includes the truffle language JARs on the - * class-path. - */ - -lazy val `runtime-with-polyglot` = - (project in file("engine/runtime-with-polyglot")) - .configs(Benchmark) - .settings( - frgaalJavaCompilerSetting, - inConfig(Compile)(truffleRunOptionsNoAssertSettings), - inConfig(Benchmark)(Defaults.testSettings), - commands += WithDebugCommand.withDebug, - Benchmark / javacOptions --= Seq( - "-source", - frgaalSourceLevel, - "--enable-preview" - ), - Test / javaOptions ++= testLogProviderOptions ++ Seq( - "-Dpolyglotimpl.DisableClassPathIsolation=true" - ), - Test / fork := true, - Test / envVars ++= distributionEnvironmentOverrides ++ Map( - "ENSO_TEST_DISABLE_IR_CACHE" -> "false" - ), - libraryDependencies ++= GraalVM.langsPkgs ++ Seq( - "org.graalvm.polyglot" % "polyglot" % graalMavenPackagesVersion % "provided", - "org.graalvm.tools" % "insight-tool" % graalMavenPackagesVersion % "provided", - "org.scalatest" %% "scalatest" % scalatestVersion % Test - ), - (Benchmark / javaOptions) := - (LocalProject("std-benchmarks") / Benchmark / run / javaOptions).value - ) - .dependsOn(runtime % "compile->compile;test->test;runtime->runtime") - /* Note [Unmanaged Classpath] * ~~~~~~~~~~~~~~~~~~~~~~~~~~ * As the definition of the core primitives in `core_definition` is achieved diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/ConversionMethodTests.java b/engine/runtime/src/test/java/org/enso/interpreter/test/ConversionMethodTests.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/ConversionMethodTests.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/ConversionMethodTests.java diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/ForeignMethodInvokeTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/ForeignMethodInvokeTest.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/ForeignMethodInvokeTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/ForeignMethodInvokeTest.java diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/InsightForEnsoTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/InsightForEnsoTest.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/InsightForEnsoTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/InsightForEnsoTest.java diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/JsInteropTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/JsInteropTest.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/JsInteropTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/JsInteropTest.java diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/MetaIsAPolyglotTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/MetaIsAPolyglotTest.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/MetaIsAPolyglotTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/MetaIsAPolyglotTest.java diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/MetaObjectPolyglotTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/MetaObjectPolyglotTest.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/MetaObjectPolyglotTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/MetaObjectPolyglotTest.java diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/NonStrictConversionMethodTests.java b/engine/runtime/src/test/java/org/enso/interpreter/test/NonStrictConversionMethodTests.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/NonStrictConversionMethodTests.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/NonStrictConversionMethodTests.java diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/PolyglotFindExceptionMessageTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/PolyglotFindExceptionMessageTest.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/PolyglotFindExceptionMessageTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/PolyglotFindExceptionMessageTest.java diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/instrument/MockLogHandler.java b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/MockLogHandler.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/instrument/MockLogHandler.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/instrument/MockLogHandler.java diff --git a/engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/instrument/VerifyJavaScriptIsAvailableTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/VerifyJavaScriptIsAvailableTest.java similarity index 100% rename from engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/instrument/VerifyJavaScriptIsAvailableTest.java rename to engine/runtime/src/test/java/org/enso/interpreter/test/instrument/VerifyJavaScriptIsAvailableTest.java diff --git a/engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala similarity index 100% rename from engine/runtime-with-polyglot/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala rename to engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala From c9bf46f88fe1919d8b0b52f2e54d544100683d65 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 14 Dec 2023 16:47:45 +0100 Subject: [PATCH 26/47] Run runtime/test without -ea --- build.sbt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 98047b3a0ea1..665fc01805bb 100644 --- a/build.sbt +++ b/build.sbt @@ -1672,7 +1672,13 @@ lazy val runtime = (project in file("engine/runtime")) "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}" ) - } + }, + // TODO[pm] This is a workaround after `runtime-with-polyglot` was merged into `runtime`. + Test / javaOptions := { + val oldOpts = (Test / javaOptions).value + val newOpts = oldOpts.filterNot(_ == "-ea") + newOpts + }, ) .settings( Test / fork := true, From b68412eef95b3cf887c2475a7fb2a0f05ec9dc5f Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 14 Dec 2023 16:49:05 +0100 Subject: [PATCH 27/47] fmt --- .../test/instrument/RuntimeComponentsTest.scala | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala index e5d826eae04d..9d4e2fddc052 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeComponentsTest.scala @@ -3,7 +3,15 @@ package org.enso.interpreter.test.instrument import org.enso.editions.LibraryName import org.enso.interpreter.runtime import org.enso.interpreter.test.Metadata -import org.enso.pkg.{ComponentGroup, ComponentGroups, ExtendedComponentGroup, GroupName, GroupReference, Package, PackageManager} +import org.enso.pkg.{ + ComponentGroup, + ComponentGroups, + ExtendedComponentGroup, + GroupName, + GroupReference, + Package, + PackageManager +} import org.enso.polyglot._ import org.enso.polyglot.runtime.Runtime.Api import org.enso.testkit.OsSpec From 111cda64d2dacf8c6477bb47eb6469670097f18f Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 10:56:46 +0100 Subject: [PATCH 28/47] Include insight tool in runtime/Test --- build.sbt | 17 +++++++++-------- project/GraalVM.scala | 6 +++++- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/build.sbt b/build.sbt index 665fc01805bb..007cb76123ca 100644 --- a/build.sbt +++ b/build.sbt @@ -1521,7 +1521,7 @@ lazy val `runtime-test-instruments` = JPMSUtils.filterModulesFromUpdate( update.value, GraalVM.modules ++ Seq( - "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion + "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion ), streams.value.log, shouldContainAll = true @@ -1529,7 +1529,7 @@ lazy val `runtime-test-instruments` = }, libraryDependencies ++= GraalVM.modules, libraryDependencies ++= Seq( - "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion % "provided", + "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion % "provided" ) ) @@ -1598,9 +1598,9 @@ lazy val runtime = (project in file("engine/runtime")) Test / modulePath := { val updateReport = (Test / update).value val requiredModIds = - GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ Seq( - "org.slf4j" % "slf4j-api" % slf4jVersion, - "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion + GraalVM.modules ++ GraalVM.langsPkgs ++ GraalVM.insightPkgs ++ logbackPkg ++ Seq( + "org.slf4j" % "slf4j-api" % slf4jVersion, + "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion ) val requiredMods = JPMSUtils.filterModulesFromUpdate( updateReport, @@ -1639,7 +1639,9 @@ lazy val runtime = (project in file("engine/runtime")) (LocalProject( "runtime-language-epb" ) / Compile / productDirectories).value ++ - (LocalProject("runtime-compiler") / Compile / productDirectories).value ++ + (LocalProject( + "runtime-compiler" + ) / Compile / productDirectories).value ++ (LocalProject("refactoring-utils") / Compile / productDirectories).value // Patch test-classes into the runtime module. This is standard way to deal with the // split package problem in unit tests. For example, Maven's surefire plugin does this. @@ -1678,7 +1680,7 @@ lazy val runtime = (project in file("engine/runtime")) val oldOpts = (Test / javaOptions).value val newOpts = oldOpts.filterNot(_ == "-ea") newOpts - }, + } ) .settings( Test / fork := true, @@ -1955,7 +1957,6 @@ lazy val `runtime-fat-jar` = .dependsOn(`runtime-language-epb`) .dependsOn(LocalProject("runtime")) - /* Note [Unmanaged Classpath] * ~~~~~~~~~~~~~~~~~~~~~~~~~~ * As the definition of the core primitives in `core_definition` is achieved diff --git a/project/GraalVM.scala b/project/GraalVM.scala index 773408bc676e..1220d2a31373 100644 --- a/project/GraalVM.scala +++ b/project/GraalVM.scala @@ -83,7 +83,11 @@ object GraalVM { "org.graalvm.tools" % "dap-tool" % version ) - val toolsPkgs = chromeInspectorPkgs ++ debugAdapterProtocolPkgs + val insightPkgs = Seq( + "org.graalvm.tools" % "insight-tool" % version + ) + + val toolsPkgs = chromeInspectorPkgs ++ debugAdapterProtocolPkgs ++ insightPkgs val langsPkgs = jsPkgs ++ pythonPkgs From 256fb9cbe92ade60cf495ed3c9f788bff866081e Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 11:58:39 +0100 Subject: [PATCH 29/47] withDebug command has better error message --- project/WithDebugCommand.scala | 38 +++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/project/WithDebugCommand.scala b/project/WithDebugCommand.scala index 824160dc8491..f288467a9201 100644 --- a/project/WithDebugCommand.scala +++ b/project/WithDebugCommand.scala @@ -61,11 +61,13 @@ object WithDebugCommand { val (debugFlags, prefixedRunArgs) = args.span(_ != argSeparator) val runArgs = " " + prefixedRunArgs.drop(1).mkString(" ") - val taskKey = - if (debugFlags.contains(benchOnlyCommandName)) BenchTasks.benchOnly - else if (debugFlags.contains(runCommandName)) Compile / Keys.run - else if (debugFlags.contains(testOnlyCommandName)) Test / Keys.testOnly - else throw new IllegalArgumentException("Invalid command name.") + val taskKeyOpt = + if (debugFlags.contains(benchOnlyCommandName)) + Some(BenchTasks.benchOnly) + else if (debugFlags.contains(runCommandName)) Some(Compile / Keys.run) + else if (debugFlags.contains(testOnlyCommandName)) + Some(Test / Keys.testOnly) + else None val dumpGraphsOpts = if (debugFlags.contains(dumpGraphsOption)) truffleDumpGraphsOptions @@ -90,14 +92,22 @@ object WithDebugCommand { debuggerOpts ).flatten - val extracted = Project.extract(state) - val withJavaOpts = extracted.appendWithoutSession( - Seq(Compile / Keys.javaOptions ++= javaOpts), - state - ) - Project - .extract(withJavaOpts) - .runInputTask(taskKey, runArgs, withJavaOpts) - state + taskKeyOpt match { + case None => + state.log.error( + s"Invalid command name. Expected one of $benchOnlyCommandName, $runCommandName, or $testOnlyCommandName" + ) + state.fail + case Some(taskKey) => + val extracted = Project.extract(state) + val withJavaOpts = extracted.appendWithoutSession( + Seq(Compile / Keys.javaOptions ++= javaOpts), + state + ) + Project + .extract(withJavaOpts) + .runInputTask(taskKey, runArgs, withJavaOpts) + state + } } } From efa56f803e09ef5f58511c9d9f154645ebf58359 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 13:46:27 +0100 Subject: [PATCH 30/47] Remove BigNumberTest.averageOfMixedArrayOverDouble. The test is now failing on "Cannot convert big integer to double", which is now expected. --- .../enso/interpreter/test/BigNumberTest.java | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/BigNumberTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/BigNumberTest.java index 6a7c9c76caad..f6631ab7356c 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/BigNumberTest.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/BigNumberTest.java @@ -87,35 +87,6 @@ private Value evalCode(final String code, final String methodName) throws URISyn return powers; } - @Test - public void averageOfMixedArrayOverDouble() throws Exception { - boolean assertsOn = false; - assert assertsOn = true; - if (assertsOn) { - // skip this test when asserts are on - return; - } - var code = """ - from Standard.Base.Data.Vector import Vector - polyglot java import org.enso.example.TestClass - - powers n = - go x v b = if x > n then b.to_vector else - b.append v - @Tail_Call go x+1 v*2 b - go 1 1 Vector.new_builder - - avg n = TestClass.doubleArrayAverage (powers n) - """; - var fn = evalCode(code, "avg"); - var avg = fn.execute(200); - - assertTrue("Got a number back " + avg,avg.isNumber()); - assertFalse("It's not a long", avg.fitsInLong()); - assertTrue("It's a double", avg.fitsInDouble()); - assertEquals("It is big enough", Math.pow(2, 200) / 200, avg.asDouble(), 300); - } - @Test public void averageOfMixedArrayOverNumber() throws Exception { var code = """ From b64d2b47e002cd32b453733d8c91a4218365ec4b Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 13:48:30 +0100 Subject: [PATCH 31/47] Fix context dispose in MetaObjectTest --- .../src/test/java/org/enso/interpreter/test/MetaIsATest.java | 1 + .../src/test/java/org/enso/interpreter/test/MetaObjectTest.java | 1 + 2 files changed, 2 insertions(+) diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/MetaIsATest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/MetaIsATest.java index 1885812aeee5..0dd34aea3195 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/MetaIsATest.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/MetaIsATest.java @@ -44,6 +44,7 @@ public static void prepareCtx() throws Exception { public static void disposeCtx() { if (generator != null) { generator.dispose(); + generator = null; } ctx.close(); } diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/MetaObjectTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/MetaObjectTest.java index ea5c08202513..250a08d29b5c 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/MetaObjectTest.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/MetaObjectTest.java @@ -37,6 +37,7 @@ public static void prepareCtx() { public static void disposeCtx() { if (generator != null) { generator.dispose(); + generator = null; } ctx.close(); } From c3264d77667b715c8626fd72ef1d771f767ff59e Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 16:23:13 +0100 Subject: [PATCH 32/47] project-manager/test runs with truffle-compiler --- build.sbt | 68 ++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/build.sbt b/build.sbt index 007cb76123ca..8c99c8784830 100644 --- a/build.sbt +++ b/build.sbt @@ -992,6 +992,7 @@ lazy val `refactoring-utils` = project .dependsOn(testkit % Test) lazy val `project-manager` = (project in file("lib/scala/project-manager")) + .enablePlugins(JPMSPlugin) .settings( (Compile / mainClass) := Some("org.enso.projectmanager.boot.ProjectManager") ) @@ -1020,6 +1021,9 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) "org.typelevel" %% "kind-projector" % kindProjectorVersion cross CrossVersion.full ) ) + /** + * Fat jar assembly settings + */ .settings( assembly / assemblyJarName := "project-manager.jar", assembly / test := {}, @@ -1048,25 +1052,53 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) case "reference.conf" => MergeStrategy.concat case _ => MergeStrategy.first }, - Test / javaOptions ++= + ) + /** + * JPMS related settings for tests + */ + .settings( + Test / fork := true, + // These dependencies are here so that we can use them in `--module-path` later on. + libraryDependencies ++= { + val necessaryModules = + GraalVM.modules.map(_.withConfigurations(Some(Test.name))) ++ + GraalVM.langsPkgs.map(_.withConfigurations(Some(Test.name))) + necessaryModules + }, + Test / addModules := Seq( + (`runtime-fat-jar` / javaModuleName).value + ), + Test / modulePath := { + val updateReport = (Test / update).value + val requiredModIds = + GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ Seq( + "org.slf4j" % "slf4j-api" % slf4jVersion, + ) + val requiredMods = JPMSUtils.filterModulesFromUpdate( + updateReport, + requiredModIds, + streams.value.log, + shouldContainAll = true + ) + val runtimeMod = + (`runtime-fat-jar` / Compile / productDirectories).value.head + + requiredMods ++ Seq(runtimeMod) + }, + Test / javaOptions ++= { + // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not + // in a module, and it cannot be simple wrapped inside a module. + // So we use plain ch.qos.logback with its configuration. + val testLogbackConf = (LocalProject( + "logging-service-logback" + ) / Test / sourceDirectory).value / "resources" / "logback-test.xml" Seq( - "-Dpolyglot.engine.WarnInterpreterOnly=false", - "-Dpolyglotimpl.DisableClassPathIsolation=true", - s"-Dconfig.file=${sourceDirectory.value}/test/resources/application.conf", - "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider" - ), - // Append enso language on the class-path - Test / unmanagedClasspath := - (LocalProject( - "runtime-fat-jar" - ) / Compile / fullClasspath).value, - // In project-manager tests, we test installing projects and for that, we need - // to launch engine-runner properly. For that, we need all the JARs that we - // normally use in engine distribution. That is why there is dependency on - // `buildEngineDistributionNoIndex`. - (Test / test) := (Test / test) - .dependsOn(buildEngineDistributionNoIndex) - .value, + "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", + s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}" + ) + }, + ) + .settings( rebuildNativeImage := NativeImage .buildNativeImage( "project-manager", From 303d2722eec0c97563f98e2ce3ca9be6b2921b70 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 16:23:27 +0100 Subject: [PATCH 33/47] language-server/test runs with truffle-compiler --- build.sbt | 45 ++++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/build.sbt b/build.sbt index 8c99c8784830..28211aad4531 100644 --- a/build.sbt +++ b/build.sbt @@ -1370,6 +1370,7 @@ lazy val `polyglot-api` = project .dependsOn(testkit % Test) lazy val `language-server` = (project in file("engine/language-server")) + .enablePlugins(JPMSPlugin) .settings( commands += WithDebugCommand.withDebug, frgaalJavaCompilerSetting, @@ -1406,28 +1407,34 @@ lazy val `language-server` = (project in file("engine/language-server")) ) ) .settings( - // These settings are needed by language-server tests that create a runtime context. Test / fork := true, - Test / javaOptions ++= testLogProviderOptions ++ { - val runtimeJar = (LocalProject( - "runtime-fat-jar" - ) / assembly / assemblyOutputPath).value - val log = streams.value.log - val requiredModules = - (Test / internalDependencyClasspath).value ++ (Test / unmanagedClasspath).value - val allModules = requiredModules.map(_.data.getAbsolutePath) ++ Seq( - runtimeJar.getAbsolutePath - ) - Seq( - "--module-path", - allModules.mkString(File.pathSeparator), - "--add-modules", - // TODO: Use JPMSModule name - "org.enso.runtime" + // These dependencies are here so that we can use them in `--module-path` later on. + libraryDependencies ++= { + val necessaryModules = + GraalVM.modules.map(_.withConfigurations(Some(Test.name))) + necessaryModules + }, + Test / addModules := Seq( + (`runtime-fat-jar` / javaModuleName).value + ), + Test / modulePath := { + val updateReport = (Test / update).value + val requiredModIds = + GraalVM.modules ++ logbackPkg ++ Seq( + "org.slf4j" % "slf4j-api" % slf4jVersion, + ) + val requiredMods = JPMSUtils.filterModulesFromUpdate( + updateReport, + requiredModIds, + streams.value.log, + shouldContainAll = true ) + val runtimeMod = + (`runtime-fat-jar` / Compile / productDirectories).value.head + requiredMods ++ Seq(runtimeMod) }, - // Append enso language on the class-path - Test / unmanagedClasspath := componentModulesPaths.value, + ) + .settings( Test / compile := (Test / compile) .dependsOn(LocalProject("enso") / updateLibraryManifests) .value, From 0ac724905653e3346d9dc139b9f9a54aeb975bed Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 16:26:26 +0100 Subject: [PATCH 34/47] JPMSUtils.compileModuleInfo takes an extraModulePath optional argument --- project/JPMSUtils.scala | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/project/JPMSUtils.scala b/project/JPMSUtils.scala index 0e20f0bd6878..d7b513f824c3 100644 --- a/project/JPMSUtils.scala +++ b/project/JPMSUtils.scala @@ -2,6 +2,7 @@ import com.sandinh.javamodule.moduleinfo.ModuleInfoPlugin.autoImport.moduleInfos import sbt.* import sbt.Keys.* import sbt.internal.inc.{CompileOutput, PlainVirtualFile} +import JPMSPlugin.autoImport.javaModuleName import sbt.util.CacheStore import sbtassembly.Assembly.{Dependency, JarEntry, Project} import sbtassembly.{CustomMergeStrategy, MergeStrategy} @@ -176,12 +177,15 @@ object JPMSUtils { * @param modulePath IDs of dependencies that should be put on the module path. The modules * put into `modulePath` are filtered away from class-path, so that module-path * and class-path passed to the `javac` are exclusive. + * @param modulePathExtra More directories that should be added on `--module-path`. This parameter is of + * type [[File]], because this is how inter project dependencies are gathered. * * @see https://users.scala-lang.org/t/scala-jdk-11-and-jpms/6102/19 */ def compileModuleInfo( copyDepsFilter: ScopeFilter, - modulePath: Seq[ModuleID] = Seq() + modulePath: Seq[ModuleID] = Seq(), + modulePathExtra: Seq[File] = Seq() ): Def.Initialize[Task[Unit]] = Def .task { @@ -244,12 +248,13 @@ object JPMSUtils { mod.revision == moduleID.revision }) }) + val allDirsOnMp = mp.map(_.data) ++ modulePathExtra val allOpts = baseJavacOpts ++ Seq( "--class-path", cp.map(_.data.getAbsolutePath).mkString(File.pathSeparator), "--module-path", - mp.map(_.data.getAbsolutePath).mkString(File.pathSeparator), + allDirsOnMp.map(_.getAbsolutePath).mkString(File.pathSeparator), "-d", outputPath.toAbsolutePath.toString ) From b9e1c3d5c1dbaf1970cec3c90d57d2f4765a1483 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 17:00:03 +0100 Subject: [PATCH 35/47] Small fixes after merge --- .../test/instruments/NodeCountingTestInstrument.java | 2 -- .../org/enso/interpreter/test/RuntimeTestServiceImpl.java | 6 ++---- .../test/instrument/IncrementalUpdatesTest.java | 2 +- .../test/instrument/VerifyJavaScriptIsAvailableTest.java | 0 .../test/instrument/VerifyLanguageAvailabilityTest.java | 0 .../test/instrument/WarningInstrumentationTest.java | 8 +------- 6 files changed, 4 insertions(+), 14 deletions(-) delete mode 100644 engine/runtime/src/test/java/org/enso/interpreter/test/instrument/VerifyJavaScriptIsAvailableTest.java delete mode 100644 engine/runtime/src/test/java/org/enso/interpreter/test/instrument/VerifyLanguageAvailabilityTest.java diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/NodeCountingTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/NodeCountingTestInstrument.java index b93efba0ce5a..5382cabc71a0 100644 --- a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/NodeCountingTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/NodeCountingTestInstrument.java @@ -1,7 +1,5 @@ package org.enso.interpreter.test.instruments; -import static org.junit.Assert.fail; - import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.instrumentation.EventContext; import com.oracle.truffle.api.instrumentation.ExecutionEventNode; diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java b/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java index 6efbfc9bdb5a..0eb2e3edfb6b 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/RuntimeTestServiceImpl.java @@ -12,15 +12,13 @@ import org.enso.interpreter.test.instruments.service.RuntimeTestService; import org.openide.util.lookup.ServiceProvider; -@ServiceProvider( - service = RuntimeTestService.class -) +@ServiceProvider(service = RuntimeTestService.class) public class RuntimeTestServiceImpl implements RuntimeTestService { @Override public UUID getNodeID(Node node) { if (node instanceof ExpressionNode exprNode) { return exprNode.getId(); - } else if (node instanceof FunctionCallInstrumentationNode funcNode){ + } else if (node instanceof FunctionCallInstrumentationNode funcNode) { return funcNode.getId(); } return null; diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java index b4302a9ea876..642c6495ecd7 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/IncrementalUpdatesTest.java @@ -14,8 +14,8 @@ import org.enso.interpreter.node.expression.literal.LiteralNode; import org.enso.interpreter.runtime.type.ConstantsGen; import org.enso.interpreter.test.Metadata; -import org.enso.interpreter.test.instruments.NodeCountingTestInstrument; import org.enso.interpreter.test.instrument.RuntimeServerTest.TestContext; +import org.enso.interpreter.test.instruments.NodeCountingTestInstrument; import org.enso.polyglot.runtime.Runtime$Api$CreateContextRequest; import org.enso.polyglot.runtime.Runtime$Api$CreateContextResponse; import org.enso.polyglot.runtime.Runtime$Api$EditFileNotification; diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/VerifyJavaScriptIsAvailableTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/VerifyJavaScriptIsAvailableTest.java deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/VerifyLanguageAvailabilityTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/VerifyLanguageAvailabilityTest.java deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java index e88c42f849e6..d73bdcbe941a 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/instrument/WarningInstrumentationTest.java @@ -1,6 +1,7 @@ package org.enso.interpreter.test.instrument; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import com.oracle.truffle.api.instrumentation.SourceSectionFilter; import com.oracle.truffle.api.instrumentation.StandardTags; @@ -15,19 +16,12 @@ import org.graalvm.polyglot.Context; import org.graalvm.polyglot.Language; import org.graalvm.polyglot.Source; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - import org.graalvm.polyglot.io.IOAccess; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import java.nio.file.Paths; -import java.util.Map; -import java.util.logging.Level; - public class WarningInstrumentationTest { private Context context; From 9d5071f47cb0ba01727db62b047ac371907731ae Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 17:21:03 +0100 Subject: [PATCH 36/47] langauge-server/test uses enso TestLogProvider --- build.sbt | 1 + 1 file changed, 1 insertion(+) diff --git a/build.sbt b/build.sbt index 6ec3f5e805ce..010f22fb2b8b 100644 --- a/build.sbt +++ b/build.sbt @@ -1428,6 +1428,7 @@ lazy val `language-server` = (project in file("engine/language-server")) (`runtime-fat-jar` / Compile / productDirectories).value.head requiredMods ++ Seq(runtimeMod) }, + Test / javaOptions ++= testLogProviderOptions, ) .settings( Test / compile := (Test / compile) From 8c90c5e374285dd01a53d04cbc61838837b59b3b Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Dec 2023 17:21:21 +0100 Subject: [PATCH 37/47] Add necessary modules on modulePath to language-server/test --- build.sbt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 010f22fb2b8b..6ae9d4c05a71 100644 --- a/build.sbt +++ b/build.sbt @@ -1383,7 +1383,11 @@ lazy val `language-server` = (project in file("engine/language-server")) "org.scalatest" %% "scalatest" % scalatestVersion % Test, "org.scalacheck" %% "scalacheck" % scalacheckVersion % Test, "org.graalvm.sdk" % "polyglot-tck" % graalMavenPackagesVersion % "provided", - "org.eclipse.jgit" % "org.eclipse.jgit" % jgitVersion + "org.eclipse.jgit" % "org.eclipse.jgit" % jgitVersion, + "org.bouncycastle" % "bcutil-jdk18on" % "1.76" % Test, + "org.bouncycastle" % "bcpkix-jdk18on" % "1.76" % Test, + "org.bouncycastle" % "bcprov-jdk18on" % "1.76" % Test, + ), Test / testOptions += Tests .Argument(TestFrameworks.ScalaCheck, "-minSuccessfulTests", "1000"), @@ -1414,9 +1418,13 @@ lazy val `language-server` = (project in file("engine/language-server")) ), Test / modulePath := { val updateReport = (Test / update).value + // org.bouncycastle is a module required by `org.enso.runtime` module. val requiredModIds = GraalVM.modules ++ logbackPkg ++ Seq( "org.slf4j" % "slf4j-api" % slf4jVersion, + "org.bouncycastle" % "bcutil-jdk18on" % "1.76", + "org.bouncycastle" % "bcpkix-jdk18on" % "1.76", + "org.bouncycastle" % "bcprov-jdk18on" % "1.76", ) val requiredMods = JPMSUtils.filterModulesFromUpdate( updateReport, From 1ce30694e722cee5c55eb73ab08f64c8db561f09 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 10:45:43 +0100 Subject: [PATCH 38/47] Add --patch-module option to language-server/test --- build.sbt | 90 +++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 71 insertions(+), 19 deletions(-) diff --git a/build.sbt b/build.sbt index 6ae9d4c05a71..e5d8493096a7 100644 --- a/build.sbt +++ b/build.sbt @@ -1021,9 +1021,8 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) "org.typelevel" %% "kind-projector" % kindProjectorVersion cross CrossVersion.full ) ) - /** - * Fat jar assembly settings - */ + /** Fat jar assembly settings + */ .settings( assembly / assemblyJarName := "project-manager.jar", assembly / test := {}, @@ -1051,11 +1050,10 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) case "application.conf" => MergeStrategy.concat case "reference.conf" => MergeStrategy.concat case _ => MergeStrategy.first - }, + } ) - /** - * JPMS related settings for tests - */ + /** JPMS related settings for tests + */ .settings( Test / fork := true, // These dependencies are here so that we can use them in `--module-path` later on. @@ -1072,7 +1070,7 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) val updateReport = (Test / update).value val requiredModIds = GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ Seq( - "org.slf4j" % "slf4j-api" % slf4jVersion, + "org.slf4j" % "slf4j-api" % slf4jVersion ) val requiredMods = JPMSUtils.filterModulesFromUpdate( updateReport, @@ -1096,7 +1094,7 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}" ) - }, + } ) .settings( rebuildNativeImage := NativeImage @@ -1384,10 +1382,10 @@ lazy val `language-server` = (project in file("engine/language-server")) "org.scalacheck" %% "scalacheck" % scalacheckVersion % Test, "org.graalvm.sdk" % "polyglot-tck" % graalMavenPackagesVersion % "provided", "org.eclipse.jgit" % "org.eclipse.jgit" % jgitVersion, - "org.bouncycastle" % "bcutil-jdk18on" % "1.76" % Test, - "org.bouncycastle" % "bcpkix-jdk18on" % "1.76" % Test, - "org.bouncycastle" % "bcprov-jdk18on" % "1.76" % Test, - + "org.bouncycastle" % "bcutil-jdk18on" % "1.76" % Test, + "org.bouncycastle" % "bcpkix-jdk18on" % "1.76" % Test, + "org.bouncycastle" % "bcprov-jdk18on" % "1.76" % Test, + "org.apache.tika" % "tika-core" % tikaVersion % Test ), Test / testOptions += Tests .Argument(TestFrameworks.ScalaCheck, "-minSuccessfulTests", "1000"), @@ -1410,7 +1408,8 @@ lazy val `language-server` = (project in file("engine/language-server")) // These dependencies are here so that we can use them in `--module-path` later on. libraryDependencies ++= { val necessaryModules = - GraalVM.modules.map(_.withConfigurations(Some(Test.name))) + GraalVM.modules.map(_.withConfigurations(Some(Test.name))) ++ + GraalVM.langsPkgs.map(_.withConfigurations(Some(Test.name))) necessaryModules }, Test / addModules := Seq( @@ -1420,11 +1419,11 @@ lazy val `language-server` = (project in file("engine/language-server")) val updateReport = (Test / update).value // org.bouncycastle is a module required by `org.enso.runtime` module. val requiredModIds = - GraalVM.modules ++ logbackPkg ++ Seq( - "org.slf4j" % "slf4j-api" % slf4jVersion, - "org.bouncycastle" % "bcutil-jdk18on" % "1.76", - "org.bouncycastle" % "bcpkix-jdk18on" % "1.76", - "org.bouncycastle" % "bcprov-jdk18on" % "1.76", + GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ Seq( + "org.slf4j" % "slf4j-api" % slf4jVersion, + "org.bouncycastle" % "bcutil-jdk18on" % "1.76", + "org.bouncycastle" % "bcpkix-jdk18on" % "1.76", + "org.bouncycastle" % "bcprov-jdk18on" % "1.76" ) val requiredMods = JPMSUtils.filterModulesFromUpdate( updateReport, @@ -1437,6 +1436,59 @@ lazy val `language-server` = (project in file("engine/language-server")) requiredMods ++ Seq(runtimeMod) }, Test / javaOptions ++= testLogProviderOptions, + Test / patchModules := { + + /** All these modules will be in --patch-module cmdline option to java, which means that + * for the JVM, it will appear that all the classes contained in these sbt projects are contained + * in the `org.enso.runtime` module. In this way, we do not have to assembly the `runtime.jar` + * fat jar. + */ + val modulesToPatchIntoRuntime: Seq[File] = + (LocalProject( + "runtime-instrument-common" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-id-execution" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-repl-debugger" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-instrument-runtime-server" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-language-epb" + ) / Compile / productDirectories).value ++ + (LocalProject( + "runtime-compiler" + ) / Compile / productDirectories).value ++ + (LocalProject("runtime-parser") / Compile / productDirectories).value ++ + (LocalProject( + "interpreter-dsl" + ) / Compile / productDirectories).value ++ + // We have to patch the `runtime` project as well, as it contains BuiltinTypes.metadata in + // runtime/target/classes/META-INF directory + (LocalProject("runtime") / Compile / productDirectories).value ++ + (LocalProject( + "syntax-rust-definition" + ) / Compile / productDirectories).value + val extraModsToPatch = JPMSUtils.filterModulesFromUpdate( + (Test / update).value, + Seq( + "org.apache.tika" % "tika-core" % tikaVersion + ), + streams.value.log, + shouldContainAll = true + ) + Map( + (`runtime-fat-jar` / javaModuleName).value -> (modulesToPatchIntoRuntime ++ extraModsToPatch) + ) + }, + Test / addReads := { + Map( + (`runtime-fat-jar` / javaModuleName).value -> Seq("ALL-UNNAMED") + ) + } ) .settings( Test / compile := (Test / compile) From a342562c2dd98b2ab5c1bd22e1ccb8ba82f82582 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 10:55:43 +0100 Subject: [PATCH 39/47] runtime/test uses Enso TestLogProvider --- build.sbt | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/build.sbt b/build.sbt index e5d8493096a7..a51159c66011 100644 --- a/build.sbt +++ b/build.sbt @@ -1770,18 +1770,7 @@ lazy val runtime = (project in file("engine/runtime")) testInstrumentsModName -> Seq(runtimeModName) ) }, - Test / javaOptions ++= { - // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not - // in a module, and it cannot be simple wrapped inside a module. - // So we use plain ch.qos.logback with its configuration. - val testLogbackConf = (LocalProject( - "logging-service-logback" - ) / Test / sourceDirectory).value / "resources" / "logback-test.xml" - Seq( - "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", - s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}" - ) - }, + Test / javaOptions ++= testLogProviderOptions, // TODO[pm] This is a workaround after `runtime-with-polyglot` was merged into `runtime`. Test / javaOptions := { val oldOpts = (Test / javaOptions).value From 3d097eec065f9266bd2b5f1d5c661ac360a7fdcb Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 11:12:28 +0100 Subject: [PATCH 40/47] Fix DebuggingEnsoTest. Stepping over / into behavior is reverted to before 5a7ad6bfe4fe6c0a945e1afb7c3ec3e58b937293 --- .../enso/interpreter/test/DebuggingEnsoTest.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java index c2aa2a839414..c7a9ff8fdfc3 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java @@ -58,7 +58,7 @@ public void initContext() { .option( RuntimeOptions.LANGUAGE_HOME_OVERRIDE, Paths.get("../../distribution/component").toFile().getAbsolutePath()) - .option(RuntimeOptions.LOG_LEVEL, Level.FINEST.getName()) + .option(RuntimeOptions.LOG_LEVEL, Level.WARNING.getName()) .logHandler(System.err) .build(); @@ -504,9 +504,7 @@ public void testSteppingOver() { bar 42 # 6 end = 0 # 7 """); - // Steps into line 2 - declaration of the method, which is fine. - // (5, 6, 7) would be better. - List expectedLineNumbers = List.of(5, 6, 2, 7); + List expectedLineNumbers = List.of(5, 6, 7); Queue steps = createStepOverEvents(expectedLineNumbers.size()); testStepping(src, "foo", new Object[] {0}, steps, expectedLineNumbers); } @@ -514,12 +512,7 @@ public void testSteppingOver() { /** * Use some methods from Vector in stdlib. Stepping over methods from different modules might be * problematic. - * - *

TODO[pm] This test is ignored, because the current behavior of step over is that it first - * steps into the declaration (name) of the method that is being stepped over and then steps back. - * So there would be weird line numbers from std lib. */ - @Ignore @Test public void testSteppingOverUseStdLib() { Source src = @@ -562,7 +555,7 @@ public void testSteppingInto() { bar 42 # 4 end = 0 # 5 """); - List expectedLineNumbers = List.of(3, 4, 2, 1, 5); + List expectedLineNumbers = List.of(3, 4, 2, 1, 2, 4, 5); Queue steps = new ArrayDeque<>( Collections.nCopies(expectedLineNumbers.size(), (event) -> event.prepareStepInto(1))); @@ -581,7 +574,7 @@ public void testSteppingIntoMoreExpressionsOneLine() { bar (baz x) # 4 end = 0 # 5 """); - List expectedLineNumbers = List.of(3, 4, 1, 2, 5); + List expectedLineNumbers = List.of(3, 4, 1, 4, 2, 4, 5); Queue steps = new ArrayDeque<>( Collections.nCopies(expectedLineNumbers.size(), (event) -> event.prepareStepInto(1))); From 9ecbd6894131a1fc46c686ac5becbc982eff5c02 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 11:17:59 +0100 Subject: [PATCH 41/47] Make sure runtime/test is run with -ea --- build.sbt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/build.sbt b/build.sbt index a51159c66011..89af62ca7938 100644 --- a/build.sbt +++ b/build.sbt @@ -1771,12 +1771,6 @@ lazy val runtime = (project in file("engine/runtime")) ) }, Test / javaOptions ++= testLogProviderOptions, - // TODO[pm] This is a workaround after `runtime-with-polyglot` was merged into `runtime`. - Test / javaOptions := { - val oldOpts = (Test / javaOptions).value - val newOpts = oldOpts.filterNot(_ == "-ea") - newOpts - } ) .settings( Test / fork := true, From e8846cc1b9eb2cbcb03c69072731a8624556a0d8 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 11:18:39 +0100 Subject: [PATCH 42/47] Make sure runtime/test is run with -ea --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 89af62ca7938..10185d8990c0 100644 --- a/build.sbt +++ b/build.sbt @@ -1770,7 +1770,7 @@ lazy val runtime = (project in file("engine/runtime")) testInstrumentsModName -> Seq(runtimeModName) ) }, - Test / javaOptions ++= testLogProviderOptions, + Test / javaOptions ++= testLogProviderOptions ) .settings( Test / fork := true, From e580ed9255e63436c16e9a3a7edc3bd7f4c3845d Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 11:39:49 +0100 Subject: [PATCH 43/47] Remove dependency on com.sandinh.sbt-java-module-info plugin --- build.sbt | 9 ++------- project/JPMSUtils.scala | 10 +++++----- project/plugins.sbt | 1 - 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/build.sbt b/build.sbt index 10185d8990c0..cf5cbf4cdaf9 100644 --- a/build.sbt +++ b/build.sbt @@ -12,8 +12,6 @@ import src.main.scala.licenses.{ DistributionDescription, SBTDistributionComponent } -import com.sandinh.javamodule.moduleinfo.JpmsModule -import com.sandinh.javamodule.moduleinfo.AutomaticModule import sbt.librarymanagement.DependencyFilter // This import is unnecessary, but bit adds a proper code completion features @@ -560,7 +558,7 @@ lazy val modulePathTestOptions = val updateReport = (LocalProject("runtime-fat-jar") / Runtime / update).value val runtimeModName = (LocalProject( "runtime-fat-jar" - ) / moduleInfos).value.head.moduleName + ) / javaModuleName).value val runtimeMod = (LocalProject( "runtime-fat-jar" ) / Compile / productDirectories).value @@ -1980,9 +1978,6 @@ lazy val `runtime-fat-jar` = // Filter module-info.java from the compilation excludeFilter := excludeFilter.value || "module-info.java", javaModuleName := "org.enso.runtime", - moduleInfos := Seq( - JpmsModule("org.enso.runtime") - ), compileOrder := CompileOrder.JavaThenScala ) /** The following libraryDependencies are provided in Runtime scope. @@ -2376,7 +2371,7 @@ lazy val `std-benchmarks` = (project in file("std-bits/benchmarks")) val runtimeModuleName = (LocalProject( "runtime-fat-jar" - ) / moduleInfos).value.head.moduleName + ) / javaModuleName).value Seq( // To enable logging in benchmarks, add ch.qos.logback module on the modulePath "-Dslf4j.provider=org.slf4j.nop.NOPServiceProvider", diff --git a/project/JPMSUtils.scala b/project/JPMSUtils.scala index d7b513f824c3..155cf29dd536 100644 --- a/project/JPMSUtils.scala +++ b/project/JPMSUtils.scala @@ -1,8 +1,7 @@ -import com.sandinh.javamodule.moduleinfo.ModuleInfoPlugin.autoImport.moduleInfos +import JPMSPlugin.autoImport.javaModuleName import sbt.* import sbt.Keys.* import sbt.internal.inc.{CompileOutput, PlainVirtualFile} -import JPMSPlugin.autoImport.javaModuleName import sbt.util.CacheStore import sbtassembly.Assembly.{Dependency, JarEntry, Project} import sbtassembly.{CustomMergeStrategy, MergeStrategy} @@ -42,8 +41,9 @@ object JPMSUtils { ) /** Filters modules by their IDs from the given classpath. - * @param cp The classpath to filter - * @param modules These modules are looked for in the class path, can be duplicated. + * + * @param cp The classpath to filter + * @param modules These modules are looked for in the class path, can be duplicated. * @param shouldContainAll If true, the method will throw an exception if not all modules were found * in the classpath. * @return The classpath with only the provided modules searched by their IDs. @@ -202,7 +202,7 @@ object JPMSUtils { val sourceProducts = productDirectories.all(copyDepsFilter).value.flatten - val moduleName = moduleInfos.value.head.moduleName + val moduleName = javaModuleName.value val cacheStore = streams.value.cacheStoreFactory val repoRootDir = (LocalProject("enso") / baseDirectory).value var someDepChanged = false diff --git a/project/plugins.sbt b/project/plugins.sbt index 5a66b8fd113c..393ed4fdcf0b 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -1,4 +1,3 @@ -addSbtPlugin("com.sandinh" % "sbt-java-module-info" % "0.4.0") addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "2.1.3") addSbtPlugin("ch.epfl.scala" % "sbt-bloop" % "1.5.6") addSbtPlugin("com.github.sbt" % "sbt-license-report" % "1.5.0") From e7171e9284c85dec1b94e68867f99000060cb655 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 11:43:32 +0100 Subject: [PATCH 44/47] CLeanup some unused methods --- build.sbt | 106 ------------------ .../instruments/CodeIdsTestInstrument.java | 4 - 2 files changed, 110 deletions(-) diff --git a/build.sbt b/build.sbt index cf5cbf4cdaf9..6e6e7905d652 100644 --- a/build.sbt +++ b/build.sbt @@ -522,112 +522,6 @@ lazy val componentModulesPaths = ) } -lazy val graalModulesPaths = - taskKey[Def.Classpath]( - "Gathers all GraalVM modules (Jar archives that should be put on module-path" + - " as files" - ) -(ThisBuild / graalModulesPaths) := { - val runnerCp = (`engine-runner` / Runtime / fullClasspath).value - val runtimeCp = (LocalProject("runtime") / Runtime / fullClasspath).value - val fullCp = (runnerCp ++ runtimeCp).distinct - val log = streams.value.log - JPMSUtils.filterModulesFromClasspath( - fullCp, - GraalVM.modules, - log, - shouldContainAll = true - ) -} - -/** The javaOptions are created such that the `runtime/test` will run with truffle-compiler - * So we need to properly create --module-path option. - * Module `org.enso.runtime` is added to --module-path as a path to a directory with - * expanded classes, i.e., not a Jar archive. This means we also have to properly create - * --patch-module option. This is an optimization so that we don't have to assembly the - * `runtime.jar` fat jar for the tests. - */ -lazy val modulePathTestOptions = - taskKey[Seq[String]]( - "Provides javaOptions for running tests with --module-path set such that " + - "enso (`org.enso.runtime`) module and truffle-compiler and the rest of " + - "the GraalVM modules are on module-path. Also makes sure that the `runtime.jar` " + - "fat jar is not rebuilt unnecessarily" - ) -(ThisBuild / modulePathTestOptions) := { - val updateReport = (LocalProject("runtime-fat-jar") / Runtime / update).value - val runtimeModName = (LocalProject( - "runtime-fat-jar" - ) / javaModuleName).value - val runtimeMod = (LocalProject( - "runtime-fat-jar" - ) / Compile / productDirectories).value - val graalMods = graalModulesPaths.value - val graalLangMods = JPMSUtils.filterModulesFromUpdate( - updateReport, - GraalVM.langsPkgs, - streams.value.log, - shouldContainAll = true - ) - val loggingMods = JPMSUtils.filterModulesFromUpdate( - updateReport, - logbackPkg ++ Seq( - "org.slf4j" % "slf4j-api" % slf4jVersion - ), - streams.value.log, - shouldContainAll = true - ) - - /** All these modules will be in --patch-module cmdline option to java, which means that - * for the JVM, it will appear that all the classes contained in these sbt projects are contained - * in the `org.enso.runtime` module. In this way, we do not have to assembly the `runtime.jar` - * fat jar. - */ - val modulesToPatchIntoRuntime: Seq[File] = - (LocalProject( - "runtime-instrument-common" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-instrument-id-execution" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-instrument-repl-debugger" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-instrument-runtime-server" - ) / Compile / productDirectories).value ++ - (LocalProject( - "runtime-language-epb" - ) / Compile / productDirectories).value ++ - (LocalProject("runtime-compiler") / Compile / productDirectories).value - val patchStr = modulesToPatchIntoRuntime - .map(_.getAbsolutePath) - .mkString(File.pathSeparator) - val allModulesPaths: Seq[String] = - runtimeMod.map(_.getAbsolutePath) ++ - graalMods.map(_.data.getAbsolutePath) ++ - graalLangMods.map(_.getAbsolutePath) ++ - loggingMods.map(_.getAbsolutePath) - // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not - // in a module, and it cannot be simple wrapped inside a module. - // So we use plain ch.qos.logback with its configuration. - val testLogbackConf = (LocalProject( - "logging-service-logback" - ) / Test / sourceDirectory).value / "resources" / "logback-test.xml" - Seq( - "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", - s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}", - "--module-path", - allModulesPaths.mkString(File.pathSeparator), - "--add-modules", - runtimeModName, - "--patch-module", - s"$runtimeModName=$patchStr", - "--add-reads", - s"$runtimeModName=ALL-UNNAMED" - ) -} - lazy val compileModuleInfo = taskKey[Unit]("Compiles `module-info.java`") // ============================================================================ diff --git a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java index 449a610747b4..68b145492a3b 100644 --- a/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java +++ b/engine/runtime-test-instruments/src/main/java/org/enso/interpreter/test/instruments/CodeIdsTestInstrument.java @@ -122,10 +122,6 @@ public void onReturnValue(VirtualFrame frame, Object result) { } } - private boolean isInstanceOf(Object obj, Class clazz) { - return clazz.isInstance(obj); - } - /** * Checks if the specified was called, if its execution triggered TCO. * From e8ca9a9a1ea1b494aefb361e89446e7e67d7ffc7 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 11:44:23 +0100 Subject: [PATCH 45/47] fmt --- .../org/enso/interpreter/test/DebuggingEnsoTest.java | 1 - project/plugins.sbt | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java index c7a9ff8fdfc3..0db916227626 100644 --- a/engine/runtime/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java +++ b/engine/runtime/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java @@ -42,7 +42,6 @@ import org.junit.After; import org.junit.Assert; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; public class DebuggingEnsoTest { diff --git a/project/plugins.sbt b/project/plugins.sbt index 393ed4fdcf0b..bcc4fed8a8e0 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -1,8 +1,8 @@ -addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "2.1.3") -addSbtPlugin("ch.epfl.scala" % "sbt-bloop" % "1.5.6") -addSbtPlugin("com.github.sbt" % "sbt-license-report" % "1.5.0") -addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.0") -addSbtPlugin("com.simplytyped" % "sbt-antlr4" % "0.8.3") +addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "2.1.3") +addSbtPlugin("ch.epfl.scala" % "sbt-bloop" % "1.5.6") +addSbtPlugin("com.github.sbt" % "sbt-license-report" % "1.5.0") +addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.0") +addSbtPlugin("com.simplytyped" % "sbt-antlr4" % "0.8.3") libraryDependencies += "io.circe" %% "circe-yaml" % "0.14.2" libraryDependencies += "commons-io" % "commons-io" % "2.12.0" From ef71ecff8a85b5dc2c8bdb7c12ccfec0e3af172f Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 14:45:51 +0100 Subject: [PATCH 46/47] project-manager/test uses TestLogProvider --- build.sbt | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/build.sbt b/build.sbt index 6e6e7905d652..15d11a86db6f 100644 --- a/build.sbt +++ b/build.sbt @@ -975,18 +975,7 @@ lazy val `project-manager` = (project in file("lib/scala/project-manager")) requiredMods ++ Seq(runtimeMod) }, - Test / javaOptions ++= { - // We can't use org.enso.logger.TestLogProvider (or anything from our own logging framework here) because it is not - // in a module, and it cannot be simple wrapped inside a module. - // So we use plain ch.qos.logback with its configuration. - val testLogbackConf = (LocalProject( - "logging-service-logback" - ) / Test / sourceDirectory).value / "resources" / "logback-test.xml" - Seq( - "-Dslf4j.provider=ch.qos.logback.classic.spi.LogbackServiceProvider", - s"-Dlogback.configurationFile=${testLogbackConf.getAbsolutePath}" - ) - } + Test / javaOptions ++= testLogProviderOptions ) .settings( rebuildNativeImage := NativeImage From 9820863b384dffb5f2bc15bd398b32d357486a7b Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 18 Dec 2023 14:47:15 +0100 Subject: [PATCH 47/47] Remove unused logback-test.xml resource --- .../src/test/resources/logback-test.xml | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 lib/scala/logging-service-logback/src/test/resources/logback-test.xml diff --git a/lib/scala/logging-service-logback/src/test/resources/logback-test.xml b/lib/scala/logging-service-logback/src/test/resources/logback-test.xml deleted file mode 100644 index 1b879a1f3823..000000000000 --- a/lib/scala/logging-service-logback/src/test/resources/logback-test.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - [%d{HH:mm:ss.SSS}] [%-5level] [%logger{36}] -%kvp- %msg%n - - - - - WARN - - - - - - -