Merge pull request #673 from alexarchambault/tweaking

Tweaking

Author: alexarchambault

.github/workflows/ci.yml

@@ -13,33 +13,6 @@ concurrency:
   cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
 
 jobs:
-  bloop-memory-footprint:
-    timeout-minutes: 120
-    runs-on: ${{ matrix.OS }}
-    strategy:
-      fail-fast: false
-      matrix:
-        OS: ["ubuntu-latest"]
-    steps:
-    - uses: actions/checkout@v2
-      with:
-        fetch-depth: 0
-        submodules: true
-    - uses: coursier/[email protected]
-    - uses: coursier/[email protected]
-      with:
-        jvm: "temurin:17"
-    - name: Java Version
-      run: java -version
-    - name: Java Home
-      run: echo "$JAVA_HOME"
-    - name: Build Scala CLI
-      run: ./mill copyJvmLauncher build
-    - name: Build Benchmark
-      run: java -jar ./build/scala-cli package --standalone gcbenchmark/gcbenchmark.scala -o gc
-    - name: Run Benchmark
-      run: ./gc $(realpath ./build/scala-cli)
-
   jvm-tests:
     timeout-minutes: 120
     runs-on: ${{ matrix.OS }}
@@ -269,6 +242,29 @@ jobs:
           exit 1
         )
 
+  bloop-memory-footprint:
+    timeout-minutes: 120
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        fetch-depth: 0
+        submodules: true
+    - uses: coursier/[email protected]
+    - uses: coursier/[email protected]
+      with:
+        jvm: "temurin:17"
+    - name: Java Version
+      run: java -version
+    - name: Java Home
+      run: echo "$JAVA_HOME"
+    - name: Build Scala CLI
+      run: ./mill copyJvmLauncher build
+    - name: Build Benchmark
+      run: java -jar ./build/scala-cli package --standalone gcbenchmark/gcbenchmark.scala -o gc
+    - name: Run Benchmark
+      run: ./gc $(realpath ./build/scala-cli)
+
   vc-redist:
     timeout-minutes: 15
     runs-on: windows-latest

build.sc

@@ -65,7 +65,8 @@ object integration extends Module {
       }
       def forkEnv = super.forkEnv() ++ Seq(
         "SCALA_CLI_TMP"   -> tmpDirBase().path.toString,
-        "SCALA_CLI_IMAGE" -> "scala-cli"
+        "SCALA_CLI_IMAGE" -> "scala-cli",
+        "CI"              -> "1"
       )
     }
   }
@@ -79,7 +80,8 @@ object integration extends Module {
       }
       def forkEnv = super.forkEnv() ++ Seq(
         "SCALA_CLI_TMP"   -> tmpDirBase().path.toString,
-        "SCALA_CLI_IMAGE" -> "scala-cli-slim"
+        "SCALA_CLI_IMAGE" -> "scala-cli-slim",
+        "CI"              -> "1"
       )
     }
   }
@@ -414,7 +416,8 @@ trait CliIntegrationBase extends SbtModule with ScalaCliPublishModule with HasTe
     def forkEnv = super.forkEnv() ++ Seq(
       "SCALA_CLI"      -> testLauncher().path.toString,
       "SCALA_CLI_KIND" -> cliKind(),
-      "SCALA_CLI_TMP"  -> tmpDirBase().path.toString
+      "SCALA_CLI_TMP"  -> tmpDirBase().path.toString,
+      "CI"             -> "1"
     )
     def sources = T.sources {
       val name = mainArtifactName().stripPrefix(prefix)

modules/cli/src/main/java/scala/cli/internal/ScalaJSLinkerSubst.java

@@ -4,6 +4,7 @@
 import com.oracle.svm.core.annotate.TargetClass;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
+import org.scalajs.logging.Logger;
 import scala.build.internal.ScalaJsConfig;
 
 import java.nio.file.Path;
@@ -18,7 +19,8 @@ void link(
         String mainClassOrNull,
         boolean addTestInitializer,
         ScalaJsConfig config,
-        Path dest
+        Path dest,
+        Logger logger
     ) {
         throw new RuntimeException("Scala.JS linking unsupported on Windows");
     }

modules/cli/src/main/scala/scala/cli/commands/Package.scala

@@ -25,6 +25,7 @@ import scala.build.options.{PackageType, Platform}
 import scala.build.{Build, Inputs, Logger, Os}
 import scala.cli.CurrentParams
 import scala.cli.commands.OptionsHelper._
+import scala.cli.errors.ScalaJsLinkingError
 import scala.cli.internal.{ProcUtil, ScalaJsLinker}
 import scala.util.Properties
 
@@ -180,7 +181,7 @@ object Package extends ScalaCommand[PackageOptions] {
         assembly(build, destPath, value(mainClass), () => alreadyExistsCheck())
 
       case PackageType.Js =>
-        buildJs(build, destPath, value(mainClass), logger)
+        value(buildJs(build, destPath, value(mainClass), logger))
 
       case PackageType.Native =>
         buildNative(inputs, build, destPath, value(mainClass), logger)
@@ -413,9 +414,9 @@ object Package extends ScalaCommand[PackageOptions] {
     destPath: os.Path,
     mainClass: String,
     logger: Logger
-  ): Unit = {
+  ): Either[BuildException, Unit] = {
     val linkerConfig = build.options.scalaJsOptions.linkerConfig(logger)
-    linkJs(build, destPath, Some(mainClass), addTestInitializer = false, linkerConfig)
+    linkJs(build, destPath, Some(mainClass), addTestInitializer = false, linkerConfig, logger)
   }
 
   private def buildNative(
@@ -544,17 +545,31 @@ object Package extends ScalaCommand[PackageOptions] {
     dest: os.Path,
     mainClassOpt: Option[String],
     addTestInitializer: Boolean,
-    config: StandardConfig
-  ): Unit =
+    config: StandardConfig,
+    logger: Logger
+  ): Either[BuildException, Unit] =
     withLibraryJar(build, dest.last.toString.stripSuffix(".jar")) { mainJar =>
-      val classPath = mainJar +: build.artifacts.classPath
+      val classPath  = mainJar +: build.artifacts.classPath
+      val linkingDir = os.temp.dir(prefix = "scala-cli-js-linking")
       (new ScalaJsLinker).link(
         classPath.toArray,
         mainClassOpt.orNull,
         addTestInitializer,
         new ScalaJsConfig(config),
-        dest.toNIO
+        linkingDir.toNIO,
+        logger.scalaJsLogger
       )
+      val relMainJs = os.rel / "main.js"
+      val mainJs    = linkingDir / relMainJs
+      if (os.exists(mainJs)) {
+        os.copy(mainJs, dest, replaceExisting = true)
+        os.remove.all(linkingDir)
+        Right(())
+      }
+      else {
+        val found = os.walk(linkingDir).map(_.relativeTo(linkingDir))
+        Left(new ScalaJsLinkingError(relMainJs, found))
+      }
     }
 
   def buildNative(

modules/cli/src/main/scala/scala/cli/commands/Run.scala

@@ -130,7 +130,7 @@ object Run extends ScalaCommand[RunOptions] {
     val (finalMainClass, finalArgs) =
       if (jvmRunner) (Constants.runnerMainClass, mainClass +: verbosity +: args)
       else (mainClass, args)
-    runOnce(
+    val res = runOnce(
       root,
       projectName,
       build,
@@ -140,6 +140,7 @@ object Run extends ScalaCommand[RunOptions] {
       allowExecve,
       exitOnError
     )
+    value(res)
   }
 
   private def runOnce(
@@ -151,19 +152,22 @@ object Run extends ScalaCommand[RunOptions] {
     logger: Logger,
     allowExecve: Boolean,
     exitOnError: Boolean
-  ): Boolean = {
+  ): Either[BuildException, Boolean] = either {
 
     val retCode = build.options.platform.value match {
       case Platform.JS =>
         val linkerConfig = build.options.scalaJsOptions.linkerConfig(logger)
-        withLinkedJs(build, Some(mainClass), addTestInitializer = false, linkerConfig) { js =>
-          Runner.runJs(
-            js.toIO,
-            args,
-            logger,
-            allowExecve = allowExecve
-          )
-        }
+        val res =
+          withLinkedJs(build, Some(mainClass), addTestInitializer = false, linkerConfig, logger) {
+            js =>
+              Runner.runJs(
+                js.toIO,
+                args,
+                logger,
+                allowExecve = allowExecve
+              )
+          }
+        value(res)
       case Platform.Native =>
         withNativeLauncher(
           build,
@@ -207,11 +211,11 @@ object Run extends ScalaCommand[RunOptions] {
     build: Build.Successful,
     mainClassOpt: Option[String],
     addTestInitializer: Boolean,
-    config: StandardConfig
-  )(f: os.Path => T): T = {
+    config: StandardConfig,
+    logger: Logger
+  )(f: os.Path => T): Either[BuildException, T] = {
     val dest = os.temp(prefix = "main", suffix = ".js")
-    try {
-      Package.linkJs(build, dest, mainClassOpt, addTestInitializer, config)
+    try Package.linkJs(build, dest, mainClassOpt, addTestInitializer, config, logger).map { _ =>
       f(dest)
     }
     finally if (os.exists(dest)) os.remove(dest)

modules/cli/src/main/scala/scala/cli/commands/Test.scala

@@ -140,7 +140,7 @@ object Test extends ScalaCommand[TestOptions] {
       case Platform.JS =>
         val linkerConfig = build.options.scalaJsOptions.linkerConfig(logger)
         value {
-          Run.withLinkedJs(build, None, addTestInitializer = true, linkerConfig) { js =>
+          Run.withLinkedJs(build, None, addTestInitializer = true, linkerConfig, logger) { js =>
             Runner.testJs(
               build.fullClassPath,
               js.toIO,
@@ -149,7 +149,7 @@ object Test extends ScalaCommand[TestOptions] {
               testFrameworkOpt,
               logger
             )
-          }
+          }.flatMap(e => e)
         }
       case Platform.Native =>
         value {

modules/cli/src/main/scala/scala/cli/errors/ScalaJsLinkingError.scala

@@ -0,0 +1,11 @@
+package scala.cli.errors
+
+import scala.build.errors.BuildException
+
+final class ScalaJsLinkingError(
+  val expected: os.RelPath,
+  val foundFiles: Seq[os.RelPath]
+) extends BuildException(
+      s"Error: $expected not found after Scala.JS linking " +
+        (if (foundFiles.isEmpty) "(no files found)" else s"(found ${foundFiles.mkString(", ")})")
+    )

modules/cli/src/main/scala/scala/cli/internal/ScalaJsLinker.scala

@@ -1,14 +1,16 @@
 package scala.cli.internal
 
-import org.scalajs.linker.interface.{LinkerOutput, ModuleInitializer}
-import org.scalajs.linker.{PathIRContainer, PathOutputFile, StandardImpl}
-import org.scalajs.logging.{Level, ScalaConsoleLogger}
+import org.scalajs.linker.interface.ModuleInitializer
+import org.scalajs.linker.{PathIRContainer, PathOutputDirectory, StandardImpl}
+import org.scalajs.logging.Logger
 import org.scalajs.testing.adapter.{TestAdapterInitializer => TAI}
 
-import java.net.URI
 import java.nio.file.Path
 
 import scala.build.internal.ScalaJsConfig
+import scala.concurrent.Await
+import scala.concurrent.ExecutionContext.{global => ec}
+import scala.concurrent.duration.Duration
 
 final class ScalaJsLinker {
 
@@ -17,21 +19,15 @@ final class ScalaJsLinker {
     mainClassOrNull: String,
     addTestInitializer: Boolean,
     config: ScalaJsConfig,
-    dest: Path
+    linkingDir: Path,
+    logger: Logger
   ): Unit = {
 
     // adapted from https://github.com/scala-js/scala-js-cli/blob/729824848e25961a3d9a1cfe6ac0260745033148/src/main/scala/org/scalajs/cli/Scalajsld.scala#L158-L193
 
     val linker = StandardImpl.linker(config.config)
 
-    def relURI(f: Path) =
-      new URI(null, null, f.getFileName.toString, null)
-
-    val sm = dest.resolveSibling(dest.getFileName.toString + ".map")
-    val output = LinkerOutput(PathOutputFile(dest))
-      .withSourceMap(PathOutputFile(sm))
-      .withSourceMapURI(relURI(sm))
-      .withJSFileURI(relURI(dest))
+    val output = PathOutputDirectory(linkingDir)
 
     val cache = StandardImpl.irFileCache().newCache
 
@@ -46,11 +42,7 @@ final class ScalaJsLinker {
 
     val moduleInitializers = mainInitializers ++ testInitializers
 
-    val logger = new ScalaConsoleLogger(Level.Info)
-
-    import scala.concurrent.Await
-    import scala.concurrent.duration.Duration
-    import scala.concurrent.ExecutionContext.Implicits.global
+    implicit val ec0 = ec
     val futureResult = PathIRContainer
       .fromClasspath(classPath)
       .flatMap(containers => cache.cached(containers._1))

modules/integration/src/test/scala/scala/cli/integration/CompileTestDefinitions.scala

@@ -269,7 +269,7 @@ abstract class CompileTestDefinitions(val scalaVersionOpt: Option[String])
           )
         )
     }
-  if (TestUtil.canRunNative && actualScalaVersion.startsWith("2.12"))
+  if (actualScalaVersion.startsWith("2.12"))
     test("JVM options only for JVM platform") {
       val inputs = TestInputs(
         Seq(os.rel / "Main.scala" -> "//> using `java-opt` \"-Xss1g\"")

modules/integration/src/test/scala/scala/cli/integration/ExportMillTestDefinitions.scala

@@ -79,7 +79,7 @@ abstract class ExportMillTestDefinitions(val scalaVersionOpt: Option[String])
       simpleTest(ExportTestProjects.jsTest(actualScalaVersion))
     }
 
-  if (runExportTests && TestUtil.canRunNative && !actualScalaVersion.startsWith("3."))
+  if (runExportTests && !actualScalaVersion.startsWith("3."))
     test("Scala Native") {
       simpleTest(ExportTestProjects.nativeTest(actualScalaVersion))
     }