Use OS-Lib's pwdDynamicFunction to allow sandboxing of os.pwd, wr…

…ite sandboxing doc page, introduce `RunModule#runner` (#3479)

Also did some misc cleanup: `synchronizedEval` is now unnecessary since
#3243 added a coarse grained
lock around BSP, prettied up the `#threadId` prefixes, remove empty
`.dest/` folders for tidyness

`RunModule#runner` continues the pattern estabilished with
`CoursierModule#defaultResolver`: a convenient way to bundle up the
various tasks related to running a modules code in a way that lets
downstream callers pass things to `run` while being able to override the
default configs (e.g. `forkEnv`, `forkArgs`, etc.) and without the
hassle of wrapping things in `T.task`s. If this pattern works out, it
would serve as a great alternative to the "pass tasks as arguments to
command" pattern we've used in the past. This makes the
`11-module-run-task` example much nicer

Also more CI optimizations

.github/workflows/actions.yml

@@ -34,13 +34,38 @@ jobs:
       java-version: 11
       millargs: __.compile
       populate_cache: true
+
   build-windows:
     uses: ./.github/workflows/run-mill-action.yml
     with:
       os: windows-latest
       java-version: 11
       millargs: __.compile
       populate_cache: true
+
+  test-docs:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with: { fetch-depth: 0 }
+
+      - run: ./mill -i docs.githubPages
+
+  compiler-bridge:
+    needs: build-linux
+    uses: ./.github/workflows/run-mill-action.yml
+    with:
+      java-version: '8'
+      millargs: bridge.__.publishLocal
+      env-bridge-versions: 'essential'
+
+  lint-autofix:
+    needs: build-linux
+    uses: ./.github/workflows/run-mill-action.yml
+    with:
+      java-version: '11'
+      buildcmd: ./mill -i mill.scalalib.scalafmt.ScalafmtModule/checkFormatAll __.sources + __.mimaReportBinaryIssues + __.fix --check
+
   itest:
     needs: build-linux
     strategy:
@@ -99,31 +124,11 @@ jobs:
           - java-version: 17
             millargs: "'integration.invalidation.__.server.testCached'"
 
-          # Check docsite compiles
-          - java-version: 11
-            millargs: docs.githubPages
-
-
     uses: ./.github/workflows/run-mill-action.yml
     with:
       java-version: ${{ matrix.java-version }}
       millargs: ${{ matrix.millargs }}
 
-  compiler-bridge:
-    needs: build-linux
-    uses: ./.github/workflows/run-mill-action.yml
-    with:
-      java-version: '8'
-      millargs: bridge.__.publishLocal
-      env-bridge-versions: 'essential'
-
-  format-scalafix-bincompat:
-    needs: build-linux
-    uses: ./.github/workflows/run-mill-action.yml
-    with:
-      java-version: '11'
-      buildcmd: ./mill -i mill.scalalib.scalafmt.ScalafmtModule/checkFormatAll __.sources + __.mimaReportBinaryIssues + __.fix --check
-
   windows:
     needs: build-windows
     strategy:
@@ -152,7 +157,7 @@ jobs:
   publish-sonatype:
     # when in master repo, publish all tags and manual runs on main
     if: github.repository == 'com-lihaoyi/mill' && (startsWith( github.ref, 'refs/tags/') || (github.ref == 'refs/heads/main' && github.event_name == 'workflow_dispatch' ) )
-    needs: [linux, windows, compiler-bridge, format-scalafix-bincompat, itest]
+    needs: [linux, windows, compiler-bridge, lint-autofix, itest, test-docs]
 
     runs-on: ubuntu-latest
 
@@ -171,8 +176,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
-        with:
-          fetch-depth: 0
+        with: {fetch-depth: 0}
 
       - uses: coursier/cache-action@v6
 
@@ -194,8 +198,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
-        with:
-          fetch-depth: 0
+        with: {fetch-depth: 0}
 
       - uses: coursier/cache-action@v6
 

.github/workflows/run-mill-action.yml

@@ -29,8 +29,7 @@ on:
         type: string
 
 jobs:
-  build:
-
+  run:
     runs-on: ${{ inputs.os }}
     continue-on-error: ${{ inputs.continue-on-error }}
     timeout-minutes: ${{ inputs.timeout-minutes }}
@@ -39,8 +38,6 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
-        with:
-          fetch-depth: 0
         if: ${{ inputs.populate_cache }}
 
       - uses: actions/download-artifact@v4

build.mill

@@ -149,7 +149,7 @@ object Deps {
   val commonsIO = ivy"commons-io:commons-io:2.16.1"
   val lambdaTest = ivy"de.tototec:de.tobiasroeser.lambdatest:0.8.0"
   val log4j2Core = ivy"org.apache.logging.log4j:log4j-core:2.23.1"
-  val osLib = ivy"com.lihaoyi::os-lib:0.10.5"
+  val osLib = ivy"com.lihaoyi::os-lib:0.10.7-M1"
   val pprint = ivy"com.lihaoyi::pprint:0.9.0"
   val mainargs = ivy"com.lihaoyi::mainargs:0.7.4"
   val millModuledefsVersion = "0.11.0-M2"

docs/modules/ROOT/nav.adoc

@@ -39,6 +39,7 @@
 * xref:Structuring_Large_Builds.adoc[]
 
 * xref:The_Mill_Evaluation_Model.adoc[]
+* xref:Mill_Sandboxing.adoc[]
 
 // This section talks about Mill plugins. While it could theoretically fit in
 // either section above, it is probably an important enough topic it is worth

docs/modules/ROOT/pages/Mill_Sandboxing.adoc

@@ -0,0 +1,20 @@
+= Mill Sandboxing
+
+== Task Sandboxing
+
+include::example/depth/sandbox/1-task.adoc[]
+
+== Test Sandboxing
+
+include::example/depth/sandbox/2-test.adoc[]
+
+== Limitations
+
+Mill's approach to filesystem sandboxing is designed to avoid accidental interference
+between different Mill tasks. It is not designed to block intentional misbehavior, and
+tasks are always able to traverse the filesystem and do whatever they want. Furthermore,
+Mill's redirection of `os.pwd` does not apply to `java.io` or `java.nio` APIs, which are
+outside of Mill's control.
+
+However, by setting `os.pwd` to safe sandbox folders, we hope to minimize the cases where
+someone accidentally causes issues with their build by doing the wrong thing.

docs/modules/ROOT/pages/Tasks.adoc

@@ -80,6 +80,6 @@ xref:Scala_Builtin_Commands.adoc#_show[show].
 If you want a way to run Mill commands and programmatically manipulate the
 tasks and outputs, you do so with your own evaluator command.
 
-=== Using ScalaModule.run as a task
+== Using ScalaModule.run as a task
 
 include::example/depth/tasks/11-module-run-task.adoc[]

example/depth/sandbox/1-task/build.mill

@@ -0,0 +1,75 @@
+// In order to help manage your build, Mill performs some rudimentary filesystem
+// sandboxing to keep different tasks and modules from interfering with each other.
+// This tries to ensure your tasks only read and write from their designated `.dest/`
+// folders, which are unique to each task and thus guaranteed not to collide with
+// the filesystem operations of other tasks that may be occurring in parallel.
+//
+//
+// === `T.dest`
+// The standard way of working with a task's `.dest/` folder is through the `T.dest`
+// property. This is available within any task, and gives you access to the
+// `out/<module-names>/<task-name>.dest/` folder to use. The `.dest/` folder for
+// each task is lazily initialized when `T.dest` is referenced and used:
+
+package build
+import mill._
+
+object foo extends Module{
+  def tDestTask = T { println(T.dest.toString) }
+}
+
+/** Usage
+> ./mill foo.tDestTask
+.../out/foo/tDestTask.dest
+*/
+
+
+// === Task `os.pwd` redirection
+// Mill also redirects the `os.pwd` property from https://github.com/com-lihaoyi/os-lib[OS-Lib],
+// such that that also points towards a running task's own `.dest/` folder
+
+def osPwdTask = T { println(os.pwd.toString) }
+
+/** Usage
+> ./mill osPwdTask
+.../out/osPwdTask.dest
+*/
+
+// The redirection of `os.pwd` applies to `os.proc`, `os.call`, and `os.spawn` methods
+// as well. In the example below, we can see the `python3` subprocess we spawn prints
+// its `os.getcwd()`, which is our `osProcTask.dest/` sandbox folder:
+
+def osProcTask = T {
+  println(os.call(("python3", "-c", "import os; print(os.getcwd())"), cwd = T.dest).out.trim())
+}
+
+/** Usage
+> ./mill osProcTask
+.../out/osProcTask.dest
+*/
+
+// === Non-task `os.pwd` redirection
+//
+// Lastly, there is the possibily of calling `os.pwd` outside of a task. When outside of
+// a task there is no `.dest/` folder associated, so instead Mill will redirect `os.pwd`
+// towards an empty `sandbox/` folder in `out/mill-worker.../`:
+
+val externalPwd = os.pwd
+def externalPwdTask = T { println(externalPwd.toString) }
+
+/** Usage
+> ./mill externalPwdTask
+.../out/mill-worker-.../sandbox/sandbox
+*/
+
+
+// === Limitations of Mill's Sandboxing
+//
+// Mill's approach to filesystem sandboxing is designed to avoid accidental interference
+// between different Mill tasks. It is not designed to block intentional misbehavior, and
+// tasks are always able to traverse the filesystem and do whatever they want. Furthermore,
+// Mill's redirection of `os.pwd` does not apply to `java.io` or `java.nio` APIs, which are
+// outside of Mill's control.
+//
+// However, by setting `os.pwd` to safe sandbox folders, we hope to minimize the cases where
+// someone accidentally causes issues with their build by doing the wrong thing.

example/depth/sandbox/2-test/bar/src/bar/Bar.java

@@ -0,0 +1,7 @@
+package bar;
+
+public class Bar {
+    public static String generateHtml(String text) {
+        return "<p>" + text + "</p>";
+    }
+}

example/depth/sandbox/2-test/bar/test/src/bar/BarTests.java

@@ -0,0 +1,15 @@
+package bar;
+
+import static org.junit.Assert.assertEquals;
+import org.junit.Test;
+import java.nio.file.*;
+
+public class BarTests {
+    @Test
+    public void simple() throws Exception {
+        String result = Bar.generateHtml("world");
+        Path path = Paths.get("generated.html");
+        Files.write(path, result.getBytes());
+        assertEquals("<p>world</p>", new String(Files.readAllBytes(path)));
+    }
+}

example/depth/sandbox/2-test/build.mill

@@ -0,0 +1,71 @@
+// Mill also creates sandbox folders for test suites to run in. Consider the
+// following build with two modules `foo` and `bar`, and their test suites
+// `foo.test` and `bar.test`:
+
+package build
+import mill._, javalib._
+
+trait MyModule extends JavaModule{
+  object test extends JavaTests with TestModule.Junit4
+}
+
+object foo extends MyModule{
+  def moduleDeps = Seq(bar)
+}
+
+object bar extends MyModule
+
+// For the sake of the example, both test modules contain tests that exercise the
+// logic in their corresponding non-test module, but also do some basic filesystem
+// operations at the same time, writing out a `generated.html` file and then reading it:
+
+/** See Also: foo/src/foo/Foo.java */
+/** See Also: foo/test/src/foo/FooTests.java */
+/** See Also: bar/src/bar/Bar.java */
+/** See Also: bar/test/src/bar/BarTests.java */
+
+// Both test suites can be run via
+
+/** Usage
+> ./mill __.test
+*/
+
+// Without sandboxing, due to the tests running in parallel, there is a race condition:
+// it's possible that `FooTests` may write the file, `BarTests` write over it, before
+// `FooTests` reads the output from `BarTests`. That would cause non-deterministic
+// flaky failures in your test suite that can be very difficult to debug and resolve.
+//
+// With Mill's test sandboxing, each test runs in a separate folder: the `.dest` folder
+// of the respective task and module. For example:
+//
+// - `foo.test` runs in `out/foo/test/test.dest/`
+// - `bar.test` runs in `out/bar/test/test.dest/`
+//
+// As a result, each test's `generated.html` file is written to its own dedicated
+// working directory, without colliding with each other on disk:
+
+/** Usage
+
+> find . | grep generated.html
+.../out/foo/test/test.dest/sandbox/generated.html
+.../out/bar/test/test.dest/sandbox/generated.html
+
+> cat out/foo/test/test.dest/sandbox/generated.html
+<h1>hello</h1>
+
+> cat out/bar/test/test.dest/sandbox/generated.html
+<p>world</p>
+
+*/
+
+// As each test suite runs in a different working directory by default, naive usage
+// reading and writing to the filesystem does not cause tests to interefere with
+// one another, which helps keep tests stable and deterministic even when run in
+// parallel
+//
+// Like Mill's Task sandboxing, Mill's Test sandboxing does not guard against
+// intentional misbehavior: tests can still walk the filesystem from the
+// sandbox folder via `..` or from the root folder `/` or home folder `~/`.
+// Nevertheless, it should add some simple guardrails to prevent many common
+// causes of inter-test interference, letting your test suite run in parallel both
+// quickly and reliably

example/depth/sandbox/2-test/foo/src/foo/Foo.java

@@ -0,0 +1,8 @@
+package foo;
+
+public class Foo {
+    public static String generateHtml(String text) {
+        return "<h1>" + text + "</h1>";
+    }
+}
+

example/depth/sandbox/2-test/foo/test/src/foo/FooTests.java

@@ -0,0 +1,15 @@
+package foo;
+
+import static org.junit.Assert.assertEquals;
+import org.junit.Test;
+import java.nio.file.*;
+
+public class FooTests {
+    @Test
+    public void simple() throws Exception {
+        String result = Foo.generateHtml("hello");
+        Path path = Paths.get("generated.html");
+        Files.write(path, result.getBytes());
+        assertEquals("<h1>hello</h1>", new String(Files.readAllBytes(path)));
+    }
+}

example/depth/tasks/11-module-run-task/bar/src/Bar.scala

@@ -1,9 +1,8 @@
 package bar
 object Bar {
   def main(args: Array[String]) = {
-    val Array(destStr, sourceStrs @ _*) = args
-    val dest = os.Path(destStr)
-    for(sourceStr <- sourceStrs){
+    val dest = os.pwd
+    for(sourceStr <- args){
       val sourcePath = os.Path(sourceStr)
       for(p <- os.walk(sourcePath) if p.ext == "scala"){
         val text = os.read(p)