Split runner out of client assembly (#5025)

Fixes #5002

This decouples the Mill executable and the Mill server classpath. 

* Previously the Mill server took on the Mill executable as part of the
classpath, with this change the Mill server classpath is resolved from
Maven Central and the Mill client may not even be a jar anymore (e.g. if
it is a graal native image)

* Reduces the native executable download size from ~108mb to ~49mb, with
the difference being the runner assembly jar we previously prepended to
the executable that now gets resolved from maven central.

* Removes the messy `MILL_CLASSPATH` environment variable handling that
was necessary to reliably get the classpath of the client JVM across all
different execution modes (local, packaged, native)

* Introduces a `cachedComputedValue` helper in our client Java code to
make it easy to wrap arbitrary expensive computations, and used to to
add some caching to the `yamlHeader` processing

Author: lihaoyi

core/constants/src/mill/constants/Util.java

@@ -1,6 +1,7 @@
 package mill.constants;
 
 import java.io.Console;
+import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.nio.charset.StandardCharsets;
@@ -76,14 +77,18 @@ public static boolean hasConsole() {
     return hasConsole0;
   }
 
-  public static String readYamlHeader(java.nio.file.Path buildFile) throws java.io.IOException {
-    java.util.List<String> lines = java.nio.file.Files.readAllLines(buildFile);
-    String yamlString = lines.stream()
-        .takeWhile(line -> line.startsWith("//|"))
-        .map(line -> line.substring(4)) // Remove the `//|` prefix
-        .collect(java.util.stream.Collectors.joining("\n"));
-
-    return yamlString;
+  public static String readYamlHeader(java.nio.file.Path buildFile) {
+    try {
+      java.util.List<String> lines = java.nio.file.Files.readAllLines(buildFile);
+      String yamlString = lines.stream()
+          .takeWhile(line -> line.startsWith("//|"))
+          .map(line -> line.substring(4)) // Remove the `//|` prefix
+          .collect(java.util.stream.Collectors.joining("\n"));
+
+      return yamlString;
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
   }
 
   private static String envInterpolatorPattern0 = "(\\$|[A-Z_][A-Z0-9_]*)";

core/util/src/mill/util/Jvm.scala

@@ -679,7 +679,12 @@ object Jvm {
 
     val testOverridesClassloaders = System.getenv("MILL_LOCAL_TEST_OVERRIDE_CLASSPATH") match {
       case null => Nil
-      case cp => Seq(new URLClassLoader(Array(os.Path(cp).toNIO.toUri.toURL)))
+      case cp =>
+        cp.split(';').map { s =>
+          val url = os.Path(s).toNIO.toUri.toURL
+          mill.constants.DebugLog.println("MILL_LOCAL_TEST_OVERRIDE_CLASSPATH " + url)
+          new java.net.URLClassLoader(Array(url))
+        }.toList
     }
 
     try {

developer.adoc

@@ -125,7 +125,7 @@ You can reproduce these tests manually using `dist.installLocal`:
 
 [source,console]
 ----
-> ./mill dist.installLocal && (cd example/basic/1-simple && ../../../mill-assembly.jar run --text hello)
+> ./mill dist.installLocal && (cd example/javalib/basic/1-simple && ../../../../mill-assembly.jar run --text hello)
 ----
 
 You can also use `dist.native.installLocal` for a Graal Native Image executable,

dist/package.mill

@@ -15,7 +15,6 @@ trait DistModule extends Module {
   lazy val allPublishModules = build.moduleInternal.modules.collect {
     case m: build.MillPublishJavaModule => m
   }
-  def moduleDeps = Seq(build.runner)
 
   def executableRaw: T[PathRef]
 
@@ -73,8 +72,7 @@ trait DistModule extends Module {
 
 object `package` extends MillJavaModule with DistModule {
 
-  // disable scalafix here because it crashes when a module has no sources
-  def moduleDeps = Seq(build.runner)
+  def moduleDeps = Seq(build.runner.client)
 
   def transitiveLocalTestOverrides = Map(
     Task.traverse(allPublishModules)(_.localTestOverride)(): _*
@@ -91,10 +89,7 @@ object `package` extends MillJavaModule with DistModule {
     val isWin = scala.util.Properties.isWin
     val outputPath = Task.dest / (if (isWin) "run.bat" else "run")
 
-    val launcherForkArgs = testArgs() ++
-      Seq(
-        "-DMILL_CLASSPATH=" + runClasspath().map(_.path.toString).mkString(",")
-      )
+    val launcherForkArgs = testArgs()
     val (millArgs, otherArgs) =
       launcherForkArgs.partition(arg =>
         arg.startsWith("-DMILL") && !arg.startsWith("-DMILL_VERSION")
@@ -138,9 +133,6 @@ object `package` extends MillJavaModule with DistModule {
     "-Djna.nosys=true"
   )
 
-  def forkShellArgs = Seq("-DMILL_CLASSPATH=$0")
-  def forkCmdArgs = Seq(""""-DMILL_CLASSPATH=%~dpnx0"""")
-
   def mainClass = Some("mill.runner.client.MillClientMain")
 
   def executableRaw = Task {
@@ -260,26 +252,18 @@ object `package` extends MillJavaModule with DistModule {
       s"${build.dist.artifactName()}-native-${artifactOsSuffix()}-${artifactCpuSuffix()}"
     }
 
-    def nativeImageClasspath = build.runner.client.runClasspath()
+    // Use assembly jar as the upstream ivy classpath rather than using runClasspath
+    // directly to try and avoid native image command length problems on windows
+    def nativeImageClasspath =
+      Seq(build.runner.client.resolvedIvyAssembly().pathRef) ++
+        build.runner.client.upstreamLocalAssemblyClasspath() ++
+        build.runner.client.localClasspath()
 
     def localBinName = "mill-native"
 
     def cacheBinarySuffix = "-native"
 
-    def executableRaw = Task {
-      val previous = nativeImage().path
-      val executable = Task.dest / previous.baseName
-
-      Using(os.write.outputStream(executable)) { out =>
-        out.write(os.read.bytes(previous))
-        out.write(System.lineSeparator.getBytes)
-        out.write(os.read.bytes(assembly().path))
-      }
-
-      if (!mill.constants.Util.isWindows) os.perms.set(executable, "rwxrwxrwx")
-
-      PathRef(executable)
-    }
+    def executableRaw = nativeImage()
 
     def nativeImageOptions = Seq(
       "--no-fallback",

example/extending/plugins/7-writing-mill-plugins/build.mill

@@ -19,17 +19,20 @@ object myplugin extends ScalaModule with PublishModule {
   // Testing Config, with necessary setup for unit/integration/example tests
   object test extends ScalaTests with TestModule.Utest {
     def mvnDeps = Seq(mvn"com.lihaoyi::mill-testkit:$millVersion")
-    def forkEnv = Map("MILL_EXECUTABLE_PATH" -> millExecutable.assembly().path.toString)
+    def forkEnv = Task {
+      val p = Task.dest / "mill/local-test-overrides" / s"com.lihaoyi-${myplugin.artifactId()}"
+      os.write(p, myplugin.localClasspath().map(_.path).mkString("\n"), createFolders = true)
+      Map(
+        "MILL_EXECUTABLE_PATH" -> millExecutable.assembly().path.toString,
+        "MILL_LOCAL_TEST_OVERRIDE_CLASSPATH" ->
+          (sys.env.get("MILL_LOCAL_TEST_OVERRIDE_CLASSPATH") ++ Seq(Task.dest)).mkString(";")
+      )
+    }
 
     // Create a Mill executable configured for testing our plugin
     object millExecutable extends JavaModule {
-      def mvnDeps = Seq(mvn"com.lihaoyi:mill-runner_3:$millVersion")
+      def mvnDeps = Seq(mvn"com.lihaoyi:mill-runner-client_3:$millVersion")
       def mainClass = Some("mill.runner.client.MillClientMain")
-      def resources = Task {
-        val p = Task.dest / "mill/local-test-overrides" / s"com.lihaoyi-${myplugin.artifactId()}"
-        os.write(p, myplugin.localClasspath().map(_.path).mkString("\n"), createFolders = true)
-        Seq(PathRef(Task.dest))
-      }
     }
   }
 

example/extending/plugins/7-writing-mill-plugins/myplugin/test/src/mill/testkit/ExampleTests.scala

@@ -6,6 +6,8 @@ object ExampleTests extends TestSuite {
 
   def tests: Tests = Tests {
     test("example") {
+//      pprint.log(sys.env("MILL_EXECUTABLE_PATH"))
+//      pprint.log(sys.env("MILL_LOCAL_TEST_OVERRIDE_CLASSPATH"))
       val resourceFolder = os.Path(sys.env("MILL_TEST_RESOURCE_DIR"))
       ExampleTester.run(
         clientServerMode = true,

example/extending/plugins/7-writing-mill-plugins/myplugin/test/src/mill/testkit/IntegrationTests.scala

@@ -9,7 +9,8 @@ object IntegrationTests extends TestSuite {
     println("initializing myplugin.IntegrationTest.tests")
 
     test("integration") {
-      pprint.log(sys.env("MILL_EXECUTABLE_PATH"))
+//      pprint.log(sys.env("MILL_EXECUTABLE_PATH"))
+//      pprint.log(sys.env("MILL_LOCAL_TEST_OVERRIDE_CLASSPATH"))
       val resourceFolder = os.Path(sys.env("MILL_TEST_RESOURCE_DIR"))
       val tester = new IntegrationTester(
         clientServerMode = true,

integration/invalidation/process-file-deleted-exit/src/ProcessFileDeletedExit.scala

@@ -41,10 +41,17 @@ object ProcessFileDeletedExit extends UtestIntegrationTestSuite {
         if (tester.clientServerMode) workspacePath / "out/mill-server"
         else workspacePath / "out/mill-no-server"
 
-      os.list(processRoot).map(p => os.remove(p / "processId"))
+      eventually {
+        os.walk(processRoot).exists(_.last == "processId")
+      }
+
+      os.list(processRoot).map { p =>
+        os.remove(p / "processId")
+      }
 
-      // Not sure why this is flaky on windows but disable it
-      if (!mill.constants.Util.isWindows) eventually { watchTerminated == true }
+      eventually {
+        watchTerminated == true
+      }
     }
   }
 }

runner/client/package.mill

@@ -0,0 +1,39 @@
+package build.runner.client
+
+import mill._
+import mill.contrib.buildinfo.BuildInfo
+
+object `package` extends build.MillPublishScalaModule with BuildInfo {
+  // Disable scalafix because it seems to misbehave and cause
+  // spurious errors when there are mixed Java/Scala sources
+  def fix(args: String*): Command[Unit] = Task.Command {}
+
+  def buildInfoPackageName = "mill.runner.client"
+
+  def moduleDeps = Seq(build.runner.server.client)
+
+  def mvnDeps = Agg(
+    build.Deps.nativeTerminal,
+    build.Deps.coursier,
+    build.Deps.coursierInterface,
+    build.Deps.coursierJvm,
+    build.Deps.logback,
+    build.Deps.snakeyamlEngine,
+    build.Deps.osLib
+  )
+
+  def buildInfoObjectName = "Versions"
+
+  def buildInfoMembers = Task {
+    val jlineNativeVersion = compileClasspath().map(_.path.last)
+      .find(name => name.startsWith("jline-native-") && name.endsWith(".jar"))
+      .map(_.stripPrefix("jline-native-").stripSuffix(".jar"))
+      .getOrElse {
+        sys.error("Cannot get jline-native version from compile class path")
+      }
+    Seq(
+      BuildInfo.Value("coursierJvmIndexVersion", build.Deps.coursierJvmIndexVersion),
+      BuildInfo.Value("jlineNativeVersion", jlineNativeVersion)
+    )
+  }
+}