Fix: Avoid requiring -i to propagate output (#3)

* Avoid requiring `-i` to propagate output

* Return scala-cli stdout instead of printing it

Author: lolgab

README.md

@@ -78,7 +78,6 @@ Benefits:
 - more reliable incremental compilation
 
 Drawbacks:
-- when compiling things in non-interactive mode, the output of Scala CLI, that prints errors and warnings, is sometimes trapped - use of `./mill -i` is recommended, which slows Mill commands a bit
 - no-op incremental compilation (when no sources changed, and nothing new needs to be compiled) has a small but noticeable cost - it takes a small amount of time (maybe in the ~100s of ms), which adds up when running Mill tasks involving numerous modules
 
 Limitations:

src/ProcessUtils.scala

@@ -0,0 +1,86 @@
+package scala.cli.mill
+
+import mill.main.client.InputPumper
+import os.SubProcess
+import java.io.PipedInputStream
+
+// Adapted from Mill Jvm.scala https://github.com/com-lihaoyi/mill/blob/f96162ecb41a9dfbac0bc524b77e09093fd61029/main/src/mill/modules/Jvm.scala#L37
+// Changes:
+//   - return stdout instead of printing it
+//   - return Either[Unit, os.SubProcess.OutputStream] in runSubprocess instead of Unit and to receive os.Shellable* instead of Seq[String]
+//   - receive os.Shellable* instead of Seq[String]
+//   - avoid receiving env and cwd since we don't pass them
+object ProcessUtils {
+  /**
+   * Runs a generic subprocess and waits for it to terminate.
+   */
+  def runSubprocess(command: os.Shellable*): Either[Unit, os.SubProcess.OutputStream] = {
+    val process = spawnSubprocess(command)
+    val shutdownHook = new Thread("subprocess-shutdown") {
+      override def run(): Unit = {
+        System.err.println("Host JVM shutdown. Forcefully destroying subprocess ...")
+        process.destroy()
+      }
+    }
+    Runtime.getRuntime().addShutdownHook(shutdownHook)
+    try {
+      process.waitFor()
+    } catch {
+      case e: InterruptedException =>
+        System.err.println("Interrupted. Forcefully destroying subprocess ...")
+        process.destroy()
+        // rethrow
+        throw e
+    } finally {
+      Runtime.getRuntime().removeShutdownHook(shutdownHook)
+    }
+    if (process.exitCode() == 0) Right(process.stdout)
+    else Left(())
+  }
+
+  /**
+   * Spawns a generic subprocess, streaming the stdout and stderr to the
+   * console. If the System.out/System.err have been substituted, makes sure
+   * that the subprocess's stdout and stderr streams go to the subtituted
+   * streams
+   */
+  def spawnSubprocess(
+      command: os.Shellable*
+  ): SubProcess = {
+    // If System.in is fake, then we pump output manually rather than relying
+    // on `os.Inherit`. That is because `os.Inherit` does not follow changes
+    // to System.in/System.out/System.err, so the subprocess's streams get sent
+    // to the parent process's origin outputs even if we want to direct them
+    // elsewhere
+    if (System.in.isInstanceOf[PipedInputStream]) {
+      val process = os.proc(command).spawn(
+        stdin = os.Pipe,
+        stdout = os.Pipe,
+        stderr = os.Pipe
+      )
+
+      val sources = Seq(
+        (process.stderr, System.err, "spawnSubprocess.stderr", false, () => true),
+        (System.in, process.stdin, "spawnSubprocess.stdin", true, () => process.isAlive())
+      )
+
+      for ((std, dest, name, checkAvailable, runningCheck) <- sources) {
+        val t = new Thread(
+          new InputPumper(std, dest, checkAvailable, () => runningCheck()),
+          name
+        )
+        t.setDaemon(true)
+        t.start()
+      }
+
+      process
+    } else {
+      os.proc(command).spawn(
+        stdin = os.Inherit,
+        stdout = os.Pipe,
+        stderr = os.Inherit
+      )
+    }
+  }
+
+}

src/ScalaCliCompile.scala

@@ -8,6 +8,7 @@ import coursier.cache.{ArchiveCache, FileCache}
 import coursier.cache.loggers.{FallbackRefreshDisplay, ProgressBarRefreshDisplay, RefreshLogger}
 import coursier.util.Artifact
 import mill._
+import mill.api.Result
 import mill.scalalib.ScalaModule
 import mill.scalalib.api.CompilationResult
 
@@ -33,7 +34,7 @@ trait ScalaCliCompile extends ScalaModule {
         else
           new FallbackRefreshDisplay
       )
-      val cache = FileCache().withLogger(logger)
+      val cache    = FileCache().withLogger(logger)
       val artifact = Artifact(url).withChanging(compileScalaCliIsChanging)
       val archiveCache = ArchiveCache()
         .withCache(cache)
@@ -110,13 +111,13 @@ trait ScalaCliCompile extends ScalaModule {
               .filter(os.exists(_))
             val workspace = T.dest / "workspace"
             os.makeDir.all(workspace)
-            val classFilesDir =
-              if (sourceFiles.isEmpty) out / "classes"
+            val classFilesDirEither =
+              if (sourceFiles.isEmpty) Right(out / "classes")
               else {
                 def asOpt[T](opt: String, values: IterableOnce[T]): Seq[String] =
                   values.iterator.toList.flatMap(v => Seq(opt, v.toString))
 
-                val proc = os.proc(
+                val outputEither = ProcessUtils.runSubprocess(
                   cli,
                   extraScalaCliHeadOptions(),
                   Seq("compile", "--classpath"),
@@ -130,13 +131,17 @@ trait ScalaCliCompile extends ScalaModule {
                   sourceFiles
                 )
 
-                val compile = proc.call()
-                val out     = compile.out.trim()
-
-                os.Path(out.split(File.pathSeparator).head)
+                outputEither.map { output =>
+                  val out = output.trim()
+                  os.Path(out.split(File.pathSeparator).head)
+                }
               }
-
-            CompilationResult(out / "unused.txt", PathRef(classFilesDir))
+            classFilesDirEither match {
+              case Right(classFilesDir) =>
+                Result.Success(CompilationResult(out / "unused.txt", PathRef(classFilesDir)))
+              case Left(()) =>
+                Result.Failure("Compilation failed")
+            }
           }
       }
     else