Allow .mill-jvm-opts to interpolate environment variables, add `.mi…

…ll-opts` (#3841)

This PR extends `.mill-jvm-opts` to support interpolating JVM variables.
This provides a bit of flexibility in allowing `$PWD` and `$USER` to be
used when passing `-Dproperty=` to Mill. This could in theory be fleshed
out even further to run arbitrary shell snippets, but for now I this
should provide enough flexibility to meet the immediate needs.

Also added a `.mill-opts`, which is similar to `.mill-jvm-opts` but used
for Mill arguments rather than JVM arguments. This provides a way to
persistent required Mill configuration in a format that is similar to
what we already have today (CLI args). This would be a common place to
put things like `--jobs=0.5C` or `--allow-positional` or
`--no-build-lock`.

Updated the integration tests and documentation

related discussions

* #1226 
* #3840

build.mill

@@ -642,81 +642,27 @@ def launcherScript(
   Jvm.universalScript(
     shellCommands = {
       val jvmArgsStr = shellJvmArgs.mkString(" ")
-      def java(mainClass: String, passMillJvmOpts: Boolean) = {
-        val millJvmOpts = if (passMillJvmOpts) "$mill_jvm_opts" else ""
-        s"""exec "$$JAVACMD" $jvmArgsStr $$JAVA_OPTS $millJvmOpts -cp "${shellClassPath.mkString(
-            ":"
-          )}" $mainClass "$$@""""
-      }
+      val classpathStr = shellClassPath.mkString(":")
 
       s"""if [ -z "$$JAVA_HOME" ] ; then
          |  JAVACMD="java"
          |else
          |  JAVACMD="$$JAVA_HOME/bin/java"
          |fi
          |
-         |mill_jvm_opts=""
-         |init_mill_jvm_opts () {
-         |  if [ -z $$MILL_JVM_OPTS_PATH ] ; then
-         |    mill_jvm_opts_file=".mill-jvm-opts"
-         |  else
-         |    mill_jvm_opts_file=$$MILL_JVM_OPTS_PATH
-         |  fi
-         |
-         |  if [ -f "$$mill_jvm_opts_file" ] ; then
-         |    # We need to append a newline at the end to fix
-         |    # https://github.com/com-lihaoyi/mill/issues/2140
-         |    newline="
-         |"
-         |    mill_jvm_opts="$$(
-         |      echo "$$newline" | cat "$$mill_jvm_opts_file" - | (
-         |        while IFS= read line
-         |        do
-         |          mill_jvm_opts="$${mill_jvm_opts} $$(echo $$line | grep -v "^[[:space:]]*[#]")"
-         |        done
-         |        # we are in a sub-shell, so need to return it explicitly
-         |        echo "$${mill_jvm_opts}"
-         |      )
-         |    )"
-         |    mill_jvm_opts="$${mill_jvm_opts} -Dmill.jvm_opts_applied=true"
-         |  fi
-         |}
-         |
          |# Client-server mode doesn't seem to work on WSL, just disable it for now
          |# https://stackoverflow.com/a/43618657/871202
          |if grep -qEi "(Microsoft|WSL)" /proc/version > /dev/null 2> /dev/null ; then
-         |    init_mill_jvm_opts
          |    if [ -z $$COURSIER_CACHE ] ; then
          |      COURSIER_CACHE=.coursier
          |    fi
-         |    ${java(millMainClass, true)}
-         |else
-         |    if [ "$${1%"-i"*}" != "$$1" ] ; then # first arg starts with "-i"
-         |        init_mill_jvm_opts
-         |        ${java(millMainClass, true)}
-         |    else
-         |        case "$$1" in
-         |          -i | --interactive | --repl | --no-server | --bsp )
-         |            init_mill_jvm_opts
-         |            ${java(millMainClass, true)}
-         |            ;;
-         |          *)
-         |            ${java(millMainClass, false)}
-         |            ;;
-         |        esac
-         |    fi
          |fi
+         |exec "$$JAVACMD" $jvmArgsStr $$JAVA_OPTS -cp "$classpathStr" $millMainClass "$$@"
          |""".stripMargin
     },
     cmdCommands = {
       val jvmArgsStr = cmdJvmArgs.mkString(" ")
-      def java(mainClass: String, passMillJvmOpts: Boolean) = {
-        val millJvmOpts = if (passMillJvmOpts) "!mill_jvm_opts!" else ""
-        s""""%JAVACMD%" $jvmArgsStr %JAVA_OPTS% $millJvmOpts -cp "${cmdClassPath.mkString(
-            ";"
-          )}" $mainClass %*"""
-      }
-
+      val classpathStr = cmdClassPath.mkString(";")
       s"""setlocal EnableDelayedExpansion
          |set "JAVACMD=java.exe"
          |if not "%JAVA_HOME%"=="" set "JAVACMD=%JAVA_HOME%\\bin\\java.exe"
@@ -726,21 +672,8 @@ def launcherScript(
          |if "%1" == "--no-server" set _I_=true
          |if "%1" == "--bsp" set _I_=true
          |
-         |set "mill_jvm_opts="
-         |set "mill_jvm_opts_file=.mill-jvm-opts"
-         |if not "%MILL_JVM_OPTS_PATH%"=="" set "mill_jvm_opts_file=%MILL_JVM_OPTS_PATH%"
+         |"%JAVACMD%" $jvmArgsStr %JAVA_OPTS% -cp "$classpathStr" $millMainClass %*
          |
-         |if defined _I_ (
-         |  if exist %mill_jvm_opts_file% (
-         |    for /f "delims=" %%G in (%mill_jvm_opts_file%) do (
-         |      set line=%%G
-         |      if "!line:~0,2!"=="-X" set "mill_jvm_opts=!mill_jvm_opts! !line!"
-         |    )
-         |  )
-         |  ${java(millMainClass, true)}
-         |) else (
-         |  ${java(millMainClass, false)}
-         |)
          |endlocal
          |""".stripMargin
     }

docs/modules/ROOT/partials/Installation_IDE_Support.adoc

@@ -196,18 +196,44 @@ JVM options, one per line.
 For example, if your build requires a lot of memory and bigger stack size, your
 `.mill-jvm-opts` could look like this:
 
+_.mill-jvm-opts_
 ----
 -Xss10m
 -Xmx10G
 ----
 
-The file name for passing JVM options to the Mill launcher is configurable. If
-for some reason you don't want to use `.mill-jvm-opts` file name, add
-`MILL_JVM_OPTS_PATH` environment variable with any other file name.
+Note that `.mill-jvm-opts` requires each CLI token to be on a separate line, so
+`-Xss10m -Xmx10G` on a single line is not allowed (as it would pass `"-Xss10m -Xmx10G"`
+as a single token and fail argument parsing)
 
-NOTE: `.mill-jvm-opts` is for passing JVM options to the JVM running Mill itself.
-If you want to pass JVM options to the project that Mill is building and running,
-see the section on
+`.mill-jvm-opts` also supports environment variable interpolation, e.g.
+
+_.mill-jvm-opts_
+----
+# PWD on mac/linux
+-Dmy.jvm.property=$PWD
+----
+
+The file name `.mill-jvm-opts` can be overridden via the `MILL_JVM_OPTS_PATH` environment
+variable.
+
+== Custom Mill Options
+
+Mill also supports the `.mill-opts` file for passing a default set of command line
+options to Mill itself. For example, if your project's tasks are CPU heavy, you
+may want everyone using your project to run only 0.5 concurrent tasks per CPU. This
+can be done by setting `.mill-opts` to:
+
+_.mill-opts_
+----
+--jobs=0.5C
+----
+
+The file name `.mill-opts` can be overridden via the `MILL_OPTS_PATH` environment variable.
+
+NOTE: `.mill-jvm-opts` is for passing JVM options to the JVM running Mill,
+and `.mill-opts` is for passing options to Mill itself. If you want to pass JVM options
+to the project that Mill is building and running, see the section on
 xref:javalib/module-config.adoc#_compilation_execution_flags[Compilation and Execution Flags].
 
 == Other installation methods

integration/feature/mill-jvm-opts/resources/.mill-jvm-opts

@@ -1,4 +1,5 @@
 
 # comment after an empty line
--DPROPERTY_PROPERLY_SET_VIA_JVM_OPTS=value-from-file
+-Dproperty.properly.set.via.jvm.opts=value-from-file
+-Dproperty.with.interpolated.working.dir=value-with-$PWD
 -Xss120m

integration/feature/mill-jvm-opts/resources/.mill-jvm-opts-alternate

@@ -0,0 +1,5 @@
+
+# comment after an empty line
+-Dproperty.properly.set.via.jvm.opts=alternate-value-from-file
+-Dproperty.with.interpolated.working.dir=alternate-value-with-$PWD
+-Xss220m

integration/feature/mill-jvm-opts/resources/.mill-opts

@@ -0,0 +1,3 @@
+
+# comment after an empty line
+--jobs=17

integration/feature/mill-jvm-opts/resources/.mill-opts-alternate

@@ -0,0 +1,3 @@
+
+# comment after an empty line
+--jobs=29

integration/feature/mill-jvm-opts/resources/build.mill

@@ -4,15 +4,23 @@ import java.lang.management.ManagementFactory
 import scala.jdk.CollectionConverters._
 
 def checkJvmOpts() = Task.Command {
-  val prop = System.getProperty("PROPERTY_PROPERLY_SET_VIA_JVM_OPTS")
+  val prop = System.getProperty("property.properly.set.via.jvm.opts")
   if (prop != "value-from-file") sys.error("jvm-opts not correctly applied, value was: " + prop)
   val runtime = ManagementFactory.getRuntimeMXBean()
   val args = runtime.getInputArguments().asScala.toSet
-  if (!args.contains("-DPROPERTY_PROPERLY_SET_VIA_JVM_OPTS=value-from-file")) {
+  if (!args.contains("-Dproperty.properly.set.via.jvm.opts=value-from-file")) {
     sys.error("jvm-opts not correctly applied, args were: " + args.mkString)
   }
   if (!args.contains("-Xss120m")) {
     sys.error("jvm-opts not correctly applied, args were: " + args.mkString)
   }
   ()
 }
+
+def getEnvJvmOpts() = Task.Command {
+  System.getProperty("property.with.interpolated.working.dir")
+}
+
+def getNonJvmOpts() = Task.Command {
+  Task.ctx().asInstanceOf[mill.api.Ctx.Jobs].jobs
+}

integration/feature/mill-jvm-opts/src/MillJvmOptsTests.scala

@@ -1,14 +1,45 @@
 package mill.integration
 
+import mill.main.client.Util
 import mill.testkit.UtestIntegrationTestSuite
-
 import utest._
 
 object MillJvmOptsTests extends UtestIntegrationTestSuite {
   val tests: Tests = Tests {
-    test("JVM options from file .mill-jvm-opts are properly read") - integrationTest { tester =>
+    test("simple") - integrationTest { tester =>
+      import tester._
+      val res = eval("checkJvmOpts")
+      assert(res.isSuccess)
+    }
+    test("interpolatedEnvVars") - integrationTest { tester =>
+      if (!Util.isWindows) { // PWD does not exist on windows
+        import tester._
+        val res = eval(("show", "getEnvJvmOpts"))
+        val out = res.out
+        val expected = "\"value-with-" + tester.workspacePath + "\""
+        assert(out == expected)
+      }
+    }
+    test("alternate") - integrationTest { tester =>
+      if (!Util.isWindows) {
+        import tester._
+        val res = eval(
+          ("show", "getEnvJvmOpts"),
+          env = Map("MILL_JVM_OPTS_PATH" -> ".mill-jvm-opts-alternate")
+        )
+        assert(res.out == "\"alternate-value-with-" + tester.workspacePath + "\"")
+      }
+    }
+    test("nonJvmOpts") - integrationTest { tester =>
+      import tester._
+      val res = eval(("show", "getNonJvmOpts"))
+      assert(res.out == "17")
+    }
+    test("nonJvmOptsAlternate") - integrationTest { tester =>
       import tester._
-      assert(eval("checkJvmOpts").isSuccess)
+      val res =
+        eval(("show", "getNonJvmOpts"), env = Map("MILL_OPTS_PATH" -> ".mill-opts-alternate"))
+      assert(res.out == "29")
     }
   }
 }

main/client/src/mill/main/client/EnvVars.java

@@ -21,6 +21,7 @@ public class EnvVars {
 
 
     public static final String MILL_JVM_OPTS_PATH = "MILL_JVM_OPTS_PATH";
+    public static final String MILL_OPTS_PATH = "MILL_OPTS_PATH";
 
 
     /**

main/client/src/mill/main/client/Util.java

@@ -16,6 +16,8 @@
 import java.util.List;
 import java.util.LinkedList;
 import java.util.Scanner;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 public class Util {
     // use methods instead of constants to avoid inlining by compiler
@@ -139,14 +141,12 @@ static String sha1Hash(String path) throws NoSuchAlgorithmException {
      */
     public static List<String> readOptsFileLines(final File file) {
         final List<String> vmOptions = new LinkedList<>();
-        try (
-                final Scanner sc = new Scanner(file)
-        ) {
+        try (final Scanner sc = new Scanner(file)) {
             while (sc.hasNextLine()) {
                 String arg = sc.nextLine();
                 String trimmed = arg.trim();
                 if (!trimmed.isEmpty() && !trimmed.startsWith("#")) {
-                    vmOptions.add(arg);
+                    vmOptions.add(interpolateEnvVars(arg, System.getenv()));
                 }
             }
         } catch (FileNotFoundException e) {
@@ -155,4 +155,25 @@ public static List<String> readOptsFileLines(final File file) {
         return vmOptions;
     }
 
+    public static String interpolateEnvVars(String input, Map<String, String> env){
+        Matcher matcher = envInterpolatorPattern.matcher(input);
+        // StringBuilder to store the result after replacing
+        StringBuffer result = new StringBuffer();
+
+        while (matcher.find()) {
+            String match = matcher.group(1);
+            if (match.equals("$")) {
+                matcher.appendReplacement(result, "\\$");
+            } else {
+                String envVarValue = env.containsKey(match) ? env.get(match) : "";
+                matcher.appendReplacement(result, envVarValue);
+            }
+        }
+
+        matcher.appendTail(result); // Append the remaining part of the string
+        return result.toString();
+    }
+
+    static Pattern envInterpolatorPattern = Pattern.compile("\\$(\\$|[A-Z_][A-Z0-9_]*)");
+
 }

main/server/src/mill/main/server/Server.scala

@@ -74,7 +74,7 @@ abstract class Server[T](
     os.remove.all(socketPath)
 
     val relFile = socketPath.relativeTo(os.pwd).toNIO.toFile
-    serverLog("listening on socket " + relFile)
+    serverLog("listening on socket " + relFile + " " + os.pwd)
     // Use relative path because otherwise the full path might be too long for the socket API
     val addr = AFUNIXSocketAddress.of(relFile)
     AFUNIXServerSocket.bindOn(addr)

runner/client/src/mill/runner/client/MillClientMain.java

@@ -3,12 +3,16 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Arrays;
+import java.util.Collections;
+
 import mill.main.client.ServerLauncher;
 import mill.main.client.Util;
 import mill.main.client.lock.Locks;
 import mill.main.client.OutFiles;
 import mill.main.client.ServerCouldNotBeStarted;
 
+import static mill.runner.client.MillProcessLauncher.millOptsFile;
+
 /**
  * This is a Java implementation to speed up repetitive starts.
  * A Scala implementation would result in the JVM loading much more classes almost doubling the start-up times.
@@ -36,7 +40,14 @@ public static void main(String[] args) throws Exception {
             MillNoServerLauncher.runMain(args);
         } else try {
             // start in client-server mode
-            ServerLauncher launcher = new ServerLauncher(System.in, System.out, System.err, System.getenv(), args, null, -1){
+            java.util.List<String> optsArgs = Util.readOptsFileLines(millOptsFile());
+            Collections.addAll(optsArgs, args);
+
+            ServerLauncher launcher = new ServerLauncher(
+                System.in, System.out, System.err,
+                System.getenv(), optsArgs.toArray(new String[0]),
+                null, -1
+            ){
                 public void initServer(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception{
                     MillProcessLauncher.launchMillServer(serverDir, setJnaNoSys);
                 }

runner/client/src/mill/runner/client/MillProcessLauncher.java

@@ -29,6 +29,7 @@ static int launchMillNoServer(String[] args) throws Exception {
         l.addAll(millLaunchJvmCommand(setJnaNoSys));
         l.add("mill.runner.MillMain");
         l.add(processDir.toAbsolutePath().toString());
+        l.addAll(Util.readOptsFileLines(millOptsFile()));
         l.addAll(Arrays.asList(args));
 
         final ProcessBuilder builder = new ProcessBuilder()
@@ -91,6 +92,14 @@ static File millJvmOptsFile() {
         return new File(millJvmOptsPath).getAbsoluteFile();
     }
 
+    static File millOptsFile() {
+        String millJvmOptsPath = System.getenv(EnvVars.MILL_OPTS_PATH);
+        if (millJvmOptsPath == null || millJvmOptsPath.trim().equals("")) {
+            millJvmOptsPath = ".mill-opts";
+        }
+        return new File(millJvmOptsPath).getAbsoluteFile();
+    }
+
     static boolean millJvmOptsAlreadyApplied() {
         final String propAppliedProp = System.getProperty("mill.jvm_opts_applied");
         return propAppliedProp != null && propAppliedProp.equals("true");

testkit/src/mill/testkit/ExampleTester.scala

@@ -1,5 +1,6 @@
 package mill.testkit
 import mill.util.Util
+import mill.main.client.EnvVars.MILL_TEST_SUITE
 import utest._
 
 /**
@@ -114,7 +115,7 @@ class ExampleTester(
       stderr = os.Pipe,
       cwd = workspacePath,
       mergeErrIntoOut = true,
-      env = Map("MILL_TEST_SUITE" -> this.getClass().toString()),
+      env = Map(MILL_TEST_SUITE -> this.getClass().toString()),
       check = false
     )