com-lihaoyi · lihaoyi · Sep 7, 2024 · Sep 5, 2024 · Sep 5, 2024 · Sep 6, 2024
diff --git a/.github/workflows/run-mill-action.yml b/.github/workflows/run-mill-action.yml
@@ -49,6 +49,8 @@ jobs:
           path: .
           name: ${{ inputs.os }}-artifact
 
+      # Need to fix cached artifact file permissions because github actions screws it up
+      # https://github.com/actions/upload-artifact/issues/38
       - name: chmod executable
         run: "chmod -R +x ."
 

diff --git a/example/javalib/web/1-hello-jetty/build.mill b/example/javalib/web/1-hello-jetty/build.mill
@@ -19,7 +19,7 @@ object `package` extends RootModule with JavaModule {
 > mill test
 ...HelloJettyTest.testHelloJetty finished...
 
-> mill runBackground; sleep 2 # give time for server to start
+> mill runBackground
 
 > curl http://localhost:8085
 ...<h1>Hello, World!</h1>...

diff --git a/example/javalib/web/2-hello-spring-boot/build.mill b/example/javalib/web/2-hello-spring-boot/build.mill
@@ -23,7 +23,7 @@ object `package` extends RootModule with JavaModule {
 > mill test
 ...com.example.HelloSpringBootTest#shouldReturnDefaultMessage() finished...
 
-> mill runBackground; sleep 15 # give time for server to start
+> mill runBackground
 
 > curl http://localhost:8086
 ...<h1>Hello, World!</h1>...

diff --git a/example/javalib/web/3-todo-spring-boot/build.mill b/example/javalib/web/3-todo-spring-boot/build.mill
@@ -61,7 +61,7 @@ object `package` extends RootModule with JavaModule {
 ...com.example.TodomvcIntegrationTests#homePageLoads() finished...
 ...com.example.TodomvcIntegrationTests#addNewTodoItem() finished...
 
-> mill test.runBackground; sleep 15 # give time for server to start
+> mill test.runBackground
 
 > curl http://localhost:8087
 ...<h1>todos</h1>...

diff --git a/example/javalib/web/4-hello-micronaut/build.mill b/example/javalib/web/4-hello-micronaut/build.mill
@@ -69,7 +69,7 @@ trait MicronautModule extends MavenModule{
 > mill test
 ...example.micronaut.HelloControllerTest#testHello()...
 
-> mill runBackground; sleep 5 # give time for server to start
+> mill runBackground
 
 > curl http://localhost:8088/hello
 ...Hello World...

diff --git a/example/javalib/web/5-todo-micronaut/build.mill b/example/javalib/web/5-todo-micronaut/build.mill
@@ -90,7 +90,7 @@ trait MicronautModule extends MavenModule{
 ...example.micronaut.TodoItemControllerTest...
 ...example.micronaut.HtmxWebJarsTest...
 
-> mill runBackground; sleep 5 # give time for server to start
+> mill runBackground
 
 > curl http://localhost:8088
  ...<h1>todos</h1>...

diff --git a/example/package.mill b/example/package.mill
@@ -216,6 +216,7 @@ object `package` extends RootModule with Module {
       )
       os.remove.all(T.dest / "merged" / ".mill-version")
 
+      os.remove.all(T.dest / "merged" / "build.sc")
       PathRef(T.dest / "merged")
     }
   }

diff --git a/example/scalalib/module/3-run-compile-deps/build.mill b/example/scalalib/module/3-run-compile-deps/build.mill
@@ -44,7 +44,7 @@ object bar extends ScalaModule {
 
 /** Usage
 
-> ./mill foo.runBackground; sleep 2 # give time for server to start
+> ./mill foo.runBackground
 
 > curl http://localhost:8079
 <html><body>Hello World!</body></html>

diff --git a/example/scalalib/web/1-todo-webapp/build.mill b/example/scalalib/web/1-todo-webapp/build.mill
@@ -27,7 +27,7 @@ object `package` extends RootModule with ScalaModule {
 > ./mill test
 + webapp.WebAppTests.simpleRequest...
 
-> ./mill runBackground; sleep 2 # give time for server to start
+> ./mill runBackground
 
 > curl http://localhost:8080
 ...What needs to be done...

diff --git a/example/scalalib/web/2-webapp-cache-busting/build.mill b/example/scalalib/web/2-webapp-cache-busting/build.mill
@@ -65,7 +65,7 @@ object `package` extends RootModule with ScalaModule {
 > ./mill test
 + webapp.WebAppTests.simpleRequest ...
 
-> ./mill runBackground; sleep 2 # give time for server to start
+> ./mill runBackground
 
 > curl http://localhost:8081
 ...What needs to be done...

diff --git a/example/scalalib/web/4-webapp-scalajs/build.mill b/example/scalalib/web/4-webapp-scalajs/build.mill
@@ -51,7 +51,7 @@ object `package` extends RootModule with ScalaModule {
 > ./mill test
 + webapp.WebAppTests.simpleRequest ...
 
-> ./mill runBackground; sleep 2 # give time for server to start
+> ./mill runBackground
 
 > curl http://localhost:8082
 ...What needs to be done...

diff --git a/example/scalalib/web/5-webapp-scalajs-shared/build.mill b/example/scalalib/web/5-webapp-scalajs-shared/build.mill
@@ -69,7 +69,7 @@ object `package` extends RootModule with AppScalaModule {
 > ./mill test
 + webapp.WebAppTests.simpleRequest ...
 
-> ./mill runBackground; sleep 2 # give time for server to start
+> ./mill runBackground
 
 > curl http://localhost:8083
 ...What needs to be done...

diff --git a/integration/feature/shutdown-exit-code/resources/build.mill b/integration/feature/shutdown-exit-code/resources/build.mill
diff --git a/integration/feature/shutdown-exit-code/src/ShutdownExitCodeTests.scala b/integration/feature/shutdown-exit-code/src/ShutdownExitCodeTests.scala
@@ -0,0 +1,22 @@
+package mill.integration
+
+import mill.testkit.UtestIntegrationTestSuite
+
+import utest._
+
+object ShutdownExitCodeTests extends UtestIntegrationTestSuite {
+  // Ensure that `shutdown` succeeds even if the prior command failed
+  val tests: Tests = Tests {
+    test("test") - integrationTest { tester =>
+      val result1 = tester.eval(("resolve", "_"))
+      assert(result1.isSuccess == true)
+      val result2 = tester.eval("shutdown")
+      assert(result2.isSuccess == true)
+
+      val result3 = tester.eval("doesnt-exit")
+      assert(result3.isSuccess == false)
+      val result4 = tester.eval("shutdown")
+      assert(result4.isSuccess == true)
+    }
+  }
+}
diff --git a/main/api/src/mill/api/Ctx.scala b/main/api/src/mill/api/Ctx.scala
@@ -117,14 +117,27 @@ class Ctx(
     val env: Map[String, String],
     val reporter: Int => Option[CompileProblemReporter],
     val testReporter: TestReporter,
-    val workspace: os.Path
+    val workspace: os.Path,
+    val systemExit: Int => Nothing
 ) extends Ctx.Dest
     with Ctx.Log
     with Ctx.Args
     with Ctx.Home
     with Ctx.Env
     with Ctx.Workspace {
 
+  def this(
+      args: IndexedSeq[_],
+      dest0: () => os.Path,
+      log: Logger,
+      home: os.Path,
+      env: Map[String, String],
+      reporter: Int => Option[CompileProblemReporter],
+      testReporter: TestReporter,
+      workspace: os.Path
+  ) = {
+    this(args, dest0, log, home, env, reporter, testReporter, workspace, i => ???)
+  }
   def dest: os.Path = dest0()
   def arg[T](index: Int): T = {
     if (index >= 0 && index < args.length) args(index).asInstanceOf[T]

diff --git a/main/eval/src/mill/eval/EvaluatorImpl.scala b/main/eval/src/mill/eval/EvaluatorImpl.scala
@@ -29,7 +29,8 @@ private[mill] case class EvaluatorImpl(
     scriptImportGraph: Map[os.Path, (Int, Seq[os.Path])] = Map.empty,
     methodCodeHashSignatures: Map[String, Int],
     override val disableCallgraphInvalidation: Boolean,
-    override val allowPositionalCommandArgs: Boolean
+    override val allowPositionalCommandArgs: Boolean,
+    val systemExit: Int => Nothing
 ) extends Evaluator with EvaluatorCore {
   import EvaluatorImpl._
 

diff --git a/main/eval/src/mill/eval/GroupEvaluator.scala b/main/eval/src/mill/eval/GroupEvaluator.scala
@@ -33,6 +33,7 @@ private[mill] trait GroupEvaluator {
   def scriptImportGraph: Map[os.Path, (Int, Seq[os.Path])]
   def methodCodeHashSignatures: Map[String, Int]
   def disableCallgraphInvalidation: Boolean
+  def systemExit: Int => Nothing
 
   lazy val constructorHashSignatures: Map[String, Seq[(String, Int)]] = methodCodeHashSignatures
     .toSeq
@@ -313,7 +314,8 @@ private[mill] trait GroupEvaluator {
               env = env,
               reporter = reporter,
               testReporter = testReporter,
-              workspace = workspace
+              workspace = workspace,
+              systemExit = systemExit
             ) with mill.api.Ctx.Jobs {
               override def jobs: Int = effectiveThreadCount
             }

diff --git a/main/server/src/mill/main/server/Server.scala b/main/server/src/mill/main/server/Server.scala
@@ -21,7 +21,8 @@ import scala.util.Try
 abstract class Server[T](
     serverDir: os.Path,
     acceptTimeoutMillis: Int,
-    locks: Locks
+    locks: Locks,
+    testLogEvenWhenServerIdWrong: Boolean = false
 ) {
 
   @volatile var running = true
@@ -31,7 +32,7 @@ abstract class Server[T](
 
   val serverId: String = java.lang.Long.toHexString(scala.util.Random.nextLong())
   def serverLog0(s: String): Unit = {
-    if (running) {
+    if (running && (testLogEvenWhenServerIdWrong || checkServerIdFile().isEmpty)) {
       os.write.append(serverDir / ServerFiles.serverLog, s"$s\n", createFolders = true)
     }
   }
@@ -91,30 +92,32 @@ abstract class Server[T](
 
   def watchServerIdFile(): Unit = {
     os.write.over(serverDir / ServerFiles.serverId, serverId)
+
     val serverIdThread = new Thread(
       () =>
-        while (
-          running && {
-            Thread.sleep(100)
-            Try(os.read(serverDir / ServerFiles.serverId)).toOption match {
-              case None =>
-                serverLog("serverId file missing")
-                exitServer()
-                false
-              case Some(s) =>
-                if (s == serverId) true
-                else {
-                  serverLog(s"serverId file contents $s does not match serverId $serverId")
-                  exitServer()
-                  false
-                }
-            }
+        while (running) {
+          checkServerIdFile() match {
+            case None => Thread.sleep(100)
+            case Some(msg) =>
+              serverLog(msg)
+              exitServer()
           }
-        ) (),
+        },
       "Server ID Checker Thread"
     )
     serverIdThread.start()
   }
+  def checkServerIdFile(): Option[String] = {
+    Try(os.read(serverDir / ServerFiles.serverId)) match {
+      case scala.util.Failure(e) => Some(s"serverId file missing")
+
+      case scala.util.Success(s) =>
+        Option.when(s != serverId) {
+          s"serverId file contents $s does not match serverId $serverId"
+        }
+    }
+
+  }
 
   def interruptWithTimeout[T](close: () => Unit, t: () => T): Option[T] = {
     @volatile var interrupt = true
@@ -194,7 +197,11 @@ abstract class Server[T](
               env.asScala.toMap,
               idle = _,
               userSpecifiedProperties.asScala.toMap,
-              initialSystemProperties
+              initialSystemProperties,
+              systemExit = exitCode => {
+                os.write.over(serverDir / ServerFiles.exitCode, exitCode.toString)
+                sys.exit(exitCode)
+              }
             )
 
             stateCache = newStateCache
@@ -243,7 +250,8 @@ abstract class Server[T](
       env: Map[String, String],
       setIdle: Boolean => Unit,
       userSpecifiedProperties: Map[String, String],
-      initialSystemProperties: Map[String, String]
+      initialSystemProperties: Map[String, String],
+      systemExit: Int => Nothing
   ): (Boolean, T)
 
 }

diff --git a/main/server/test/src/mill/main/server/ClientServerTests.scala b/main/server/test/src/mill/main/server/ClientServerTests.scala
@@ -16,8 +16,13 @@ import utest._
 object ClientServerTests extends TestSuite {
 
   val ENDL = System.lineSeparator()
-  class EchoServer(override val serverId: String, serverDir: os.Path, locks: Locks)
-      extends Server[Option[Int]](serverDir, 1000, locks) with Runnable {
+  class EchoServer(
+      override val serverId: String,
+      serverDir: os.Path,
+      locks: Locks,
+      testLogEvenWhenServerIdWrong: Boolean
+  ) extends Server[Option[Int]](serverDir, 1000, locks, testLogEvenWhenServerIdWrong)
+      with Runnable {
     override def exitServer() = {
       serverLog("exiting server")
       super.exitServer()
@@ -37,7 +42,8 @@ object ClientServerTests extends TestSuite {
         env: Map[String, String],
         setIdle: Boolean => Unit,
         systemProperties: Map[String, String],
-        initialSystemProperties: Map[String, String]
+        initialSystemProperties: Map[String, String],
+        systemExit: Int => Nothing
     ) = {
 
       val reader = new BufferedReader(new InputStreamReader(streams.in))
@@ -61,7 +67,7 @@ object ClientServerTests extends TestSuite {
     }
   }
 
-  class Tester {
+  class Tester(testLogEvenWhenServerIdWrong: Boolean) {
 
     var nextServerId: Int = 0
     val terminatedServers = collection.mutable.Set.empty[String]
@@ -91,7 +97,12 @@ object ClientServerTests extends TestSuite {
         def initServer(serverDir: String, b: Boolean, locks: Locks) = {
           val serverId = "server-" + nextServerId
           nextServerId += 1
-          new Thread(new EchoServer(serverId, os.Path(serverDir, os.pwd), locks)).start()
+          new Thread(new EchoServer(
+            serverId,
+            os.Path(serverDir, os.pwd),
+            locks,
+            testLogEvenWhenServerIdWrong
+          )).start()
         }
       }.acquireLocksAndRun(outDir.relativeTo(os.pwd).toString)
 
@@ -123,7 +134,9 @@ object ClientServerTests extends TestSuite {
   def tests = Tests {
 
     test("hello") - retry(3) {
-      val tester = new Tester
+      // Continue logging when out folder is deleted so we can see the logs
+      // and ensure the correct code path is taken as the server exits
+      val tester = new Tester(testLogEvenWhenServerIdWrong = true)
       val res1 = tester(args = Array("world"))
 
       assert(
@@ -154,16 +167,36 @@ object ClientServerTests extends TestSuite {
         )
 
         // Make sure if we delete the out dir, the server notices and exits
+        Thread.sleep(500)
         os.remove.all(res3.outDir)
         Thread.sleep(500)
 
         assert(res3.logsFor("serverId file missing") == Seq("server-1"))
         assert(res3.logsFor("exiting server") == Seq("server-1"))
       }
     }
+    test("dontLogWhenOutFolderDeleted") - retry(3) {
+      val tester = new Tester(testLogEvenWhenServerIdWrong = false)
+      val res1 = tester(args = Array("world"))
+
+      assert(
+        res1.out == s"helloworld$ENDL",
+        res1.err == s"HELLOworld$ENDL"
+      )
+
+      if (!Util.isWindows) {
+        // Make sure if we delete the `out/` folder, the server notices
+        // and exits and does not re-create the deleted `out/` folder
+        Thread.sleep(500)
+        os.remove.all(res1.outDir)
+        Thread.sleep(2000)
+
+        assert(!os.exists(res1.outDir))
+      }
+    }
 
     test("concurrency") {
-      val tester = new Tester
+      val tester = new Tester(testLogEvenWhenServerIdWrong = false)
       // Make sure concurrently running client commands results in multiple processes
       // being spawned, running in different folders
       import concurrent._
@@ -188,7 +221,7 @@ object ClientServerTests extends TestSuite {
     }
 
     test("clientLockReleasedOnFailure") {
-      val tester = new Tester
+      val tester = new Tester(testLogEvenWhenServerIdWrong = false)
       // When the client gets interrupted via Ctrl-C, we exit the server immediately. This
       // is because Mill ends up executing arbitrary JVM code, and there is no generic way
       // to interrupt such an execution. The two options are to leave the server running
@@ -214,7 +247,7 @@ object ClientServerTests extends TestSuite {
     }
 
     test("envVars") - retry(3) {
-      val tester = new Tester
+      val tester = new Tester(testLogEvenWhenServerIdWrong = false)
       // Make sure the simple "have the client start a server and
       // exchange one message" workflow works from end to end.