From eac7fa98abe1d8fbf6163583e44be9a7111246bf Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 7 Oct 2024 21:25:24 +0200 Subject: [PATCH 1/6] Make Logger extends Closeable and close it via Using.resource Later commits add more resources to the Using.resource(s) call --- main/api/src/mill/api/Logger.scala | 4 ++-- runner/src/mill/runner/MillMain.scala | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/main/api/src/mill/api/Logger.scala b/main/api/src/mill/api/Logger.scala index 4b93616d14f..6d7f550d3cd 100644 --- a/main/api/src/mill/api/Logger.scala +++ b/main/api/src/mill/api/Logger.scala @@ -1,6 +1,6 @@ package mill.api -import java.io.{InputStream, PrintStream} +import java.io.{Closeable, InputStream, PrintStream} /** * The standard logging interface of the Mill build tool. @@ -24,7 +24,7 @@ import java.io.{InputStream, PrintStream} * but when `show` is used both are forwarded to stderr and stdout is only * used to display the final `show` output for easy piping. */ -trait Logger { +trait Logger extends Closeable { def colored: Boolean def systemStreams: SystemStreams diff --git a/runner/src/mill/runner/MillMain.scala b/runner/src/mill/runner/MillMain.scala index 3e1550ff026..fb699cb9b6e 100644 --- a/runner/src/mill/runner/MillMain.scala +++ b/runner/src/mill/runner/MillMain.scala @@ -13,6 +13,7 @@ import mill.util.{PromptLogger, PrintLogger, Colors} import java.lang.reflect.InvocationTargetException import scala.util.control.NonFatal +import scala.util.Using @internal object MillMain { @@ -235,7 +236,8 @@ object MillMain { colored = colored, colors = colors ) - try new MillBuildBootstrap( + Using.resource(logger) { _ => + new MillBuildBootstrap( projectRoot = WorkspaceRoot.workspaceRoot, output = os.Path(OutFiles.out, WorkspaceRoot.workspaceRoot), home = config.home, @@ -252,8 +254,6 @@ object MillMain { config.allowPositional.value, systemExit = systemExit ).evaluate() - finally { - logger.close() } }, colors = colors From c8165b4f02e5b77706398128fe6606a5a8aa3678 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 23 Sep 2024 16:43:02 +0200 Subject: [PATCH 2/6] Acquire a lock on the out dir in order to run tasks / commands --- main/api/src/mill/api/Logger.scala | 12 ++++++++++ .../client/src/mill/main/client/OutFiles.java | 4 ++++ .../src/mill/main/client/lock/DummyLock.java | 22 +++++++++++++++++++ .../mill/main/client/lock/DummyTryLocked.java | 11 ++++++++++ .../src/mill/main/client/lock/Lock.java | 13 +++++++++++ runner/src/mill/runner/MillCliConfig.scala | 8 ++++++- runner/src/mill/runner/MillMain.scala | 12 ++++++++-- 7 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 main/client/src/mill/main/client/lock/DummyLock.java create mode 100644 main/client/src/mill/main/client/lock/DummyTryLocked.java diff --git a/main/api/src/mill/api/Logger.scala b/main/api/src/mill/api/Logger.scala index 6d7f550d3cd..5fe2e9e253e 100644 --- a/main/api/src/mill/api/Logger.scala +++ b/main/api/src/mill/api/Logger.scala @@ -2,6 +2,8 @@ package mill.api import java.io.{Closeable, InputStream, PrintStream} +import mill.main.client.lock.{Lock, Locked} + /** * The standard logging interface of the Mill build tool. * @@ -79,4 +81,14 @@ trait Logger extends Closeable { try t finally removePromptLine() } + + def waitForLock(lock: Lock): Locked = { + val tryLocked = lock.tryLock() + if (tryLocked.isLocked()) + tryLocked + else { + info("Another Mill process is running tasks, waiting for it to be done...") + lock.lock() + } + } } diff --git a/main/client/src/mill/main/client/OutFiles.java b/main/client/src/mill/main/client/OutFiles.java index 04ffeecb4db..0af23e233a1 100644 --- a/main/client/src/mill/main/client/OutFiles.java +++ b/main/client/src/mill/main/client/OutFiles.java @@ -57,5 +57,9 @@ public class OutFiles { */ final public static String millNoServer = "mill-no-server"; + /** + * Lock file used for exclusive access to the Mill output directory + */ + final public static String millLock = "mill-lock"; } diff --git a/main/client/src/mill/main/client/lock/DummyLock.java b/main/client/src/mill/main/client/lock/DummyLock.java new file mode 100644 index 00000000000..ede8224323d --- /dev/null +++ b/main/client/src/mill/main/client/lock/DummyLock.java @@ -0,0 +1,22 @@ +package mill.main.client.lock; + +import java.util.concurrent.locks.ReentrantLock; + +class DummyLock extends Lock { + + public boolean probe() { + return true; + } + + public Locked lock() { + return new DummyTryLocked(); + } + + public TryLocked tryLock() { + return new DummyTryLocked(); + } + + @Override + public void close() throws Exception { + } +} diff --git a/main/client/src/mill/main/client/lock/DummyTryLocked.java b/main/client/src/mill/main/client/lock/DummyTryLocked.java new file mode 100644 index 00000000000..34ad7b5ea15 --- /dev/null +++ b/main/client/src/mill/main/client/lock/DummyTryLocked.java @@ -0,0 +1,11 @@ +package mill.main.client.lock; + +class DummyTryLocked implements TryLocked { + public DummyTryLocked() { + } + + public boolean isLocked(){ return true; } + + public void release() throws Exception { + } +} diff --git a/main/client/src/mill/main/client/lock/Lock.java b/main/client/src/mill/main/client/lock/Lock.java index 6d729c0ebd6..3870bc07a14 100644 --- a/main/client/src/mill/main/client/lock/Lock.java +++ b/main/client/src/mill/main/client/lock/Lock.java @@ -15,4 +15,17 @@ public void await() throws Exception { */ public abstract boolean probe() throws Exception; public void delete() throws Exception {} + + public static Lock file(String path) throws Exception { + return new FileLock(path); + } + + public static Lock memory() { + return new MemoryLock(); + } + + public static Lock dummy() { + return new DummyLock(); + } + } diff --git a/runner/src/mill/runner/MillCliConfig.scala b/runner/src/mill/runner/MillCliConfig.scala index 61d1ef9fac4..6608672ea66 100644 --- a/runner/src/mill/runner/MillCliConfig.scala +++ b/runner/src/mill/runner/MillCliConfig.scala @@ -130,7 +130,13 @@ case class MillCliConfig( status at the command line and falls back to the legacy ticker """ ) - disablePrompt: Flag = Flag() + disablePrompt: Flag = Flag(), + @arg( + hidden = true, + doc = + """Evaluate tasks / commands without acquiring an exclusive lock on the Mill output directory""" + ) + noBuildLock: Flag = Flag() ) import mainargs.ParserForClass diff --git a/runner/src/mill/runner/MillMain.scala b/runner/src/mill/runner/MillMain.scala index fb699cb9b6e..1d4867a9aea 100644 --- a/runner/src/mill/runner/MillMain.scala +++ b/runner/src/mill/runner/MillMain.scala @@ -9,6 +9,7 @@ import mill.api.{MillException, SystemStreams, WorkspaceRoot, internal} import mill.bsp.{BspContext, BspServerResult} import mill.main.BuildInfo import mill.main.client.{OutFiles, ServerFiles} +import mill.main.client.lock.Lock import mill.util.{PromptLogger, PrintLogger, Colors} import java.lang.reflect.InvocationTargetException @@ -210,6 +211,10 @@ object MillMain { .map(_ => Seq(bspCmd)) .getOrElse(config.leftoverArgs.value.toList) + val out = os.Path(OutFiles.out, WorkspaceRoot.workspaceRoot) + val outLock = + if (config.noBuildLock.value || bspContext.isDefined) Lock.dummy() + else Lock.file((out / OutFiles.millLock).toString) var repeatForBsp = true var loopRes: (Boolean, RunnerState) = (false, RunnerState.empty) while (repeatForBsp) { @@ -236,10 +241,13 @@ object MillMain { colored = colored, colors = colors ) - Using.resource(logger) { _ => + Using.resources( + logger, + logger.waitForLock(outLock) + ) { (_, _) => new MillBuildBootstrap( projectRoot = WorkspaceRoot.workspaceRoot, - output = os.Path(OutFiles.out, WorkspaceRoot.workspaceRoot), + output = out, home = config.home, keepGoing = config.keepGoing.value, imports = config.imports, From baef6a07abd36a2f42f5237608df3f388e321bdb Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 7 Oct 2024 17:54:18 +0200 Subject: [PATCH 3/6] Add --no-wait-for-build-lock option Mainly meant to be used in integration tests --- main/api/src/mill/api/Logger.scala | 7 +++++-- runner/src/mill/runner/MillCliConfig.scala | 8 +++++++- runner/src/mill/runner/MillMain.scala | 5 ++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/main/api/src/mill/api/Logger.scala b/main/api/src/mill/api/Logger.scala index 5fe2e9e253e..53e5dce1a51 100644 --- a/main/api/src/mill/api/Logger.scala +++ b/main/api/src/mill/api/Logger.scala @@ -82,13 +82,16 @@ trait Logger extends Closeable { finally removePromptLine() } - def waitForLock(lock: Lock): Locked = { + def waitForLock(lock: Lock, waitingAllowed: Boolean): Locked = { val tryLocked = lock.tryLock() if (tryLocked.isLocked()) tryLocked - else { + else if (waitingAllowed) { info("Another Mill process is running tasks, waiting for it to be done...") lock.lock() + } else { + error("Cannot proceed, another Mill process is running tasks") + throw new Exception("Cannot acquire lock on Mill output directory") } } } diff --git a/runner/src/mill/runner/MillCliConfig.scala b/runner/src/mill/runner/MillCliConfig.scala index 6608672ea66..429a394f18a 100644 --- a/runner/src/mill/runner/MillCliConfig.scala +++ b/runner/src/mill/runner/MillCliConfig.scala @@ -136,7 +136,13 @@ case class MillCliConfig( doc = """Evaluate tasks / commands without acquiring an exclusive lock on the Mill output directory""" ) - noBuildLock: Flag = Flag() + noBuildLock: Flag = Flag(), + @arg( + hidden = true, + doc = + """Do not wait for an exclusive lock on the Mill output directory to evaluate tasks / commands. Fail if waiting for a lock is needed.""" + ) + noWaitForBuildLock: Flag = Flag() ) import mainargs.ParserForClass diff --git a/runner/src/mill/runner/MillMain.scala b/runner/src/mill/runner/MillMain.scala index 1d4867a9aea..10f343d1518 100644 --- a/runner/src/mill/runner/MillMain.scala +++ b/runner/src/mill/runner/MillMain.scala @@ -243,7 +243,10 @@ object MillMain { ) Using.resources( logger, - logger.waitForLock(outLock) + logger.waitForLock( + outLock, + waitingAllowed = !config.noWaitForBuildLock.value + ) ) { (_, _) => new MillBuildBootstrap( projectRoot = WorkspaceRoot.workspaceRoot, From b41fe773f598f2608f30fe06ce6ff0a32606cdec Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 7 Oct 2024 19:08:40 +0200 Subject: [PATCH 4/6] Add out dir lock test --- .../output-directory/resources/build.mill | 12 +++ .../src/OutputDirectoryLockTests.scala | 82 +++++++++++++++++++ .../src/mill/testkit/IntegrationTester.scala | 48 +++++++++++ 3 files changed, 142 insertions(+) create mode 100644 integration/feature/output-directory/src/OutputDirectoryLockTests.scala diff --git a/integration/feature/output-directory/resources/build.mill b/integration/feature/output-directory/resources/build.mill index b5fae4e0f89..e068bbd11cb 100644 --- a/integration/feature/output-directory/resources/build.mill +++ b/integration/feature/output-directory/resources/build.mill @@ -5,4 +5,16 @@ import mill.scalalib._ object `package` extends RootModule with ScalaModule { def scalaVersion = scala.util.Properties.versionNumberString + + def hello = Task { + "Hello from hello task" + } + + def blockWhileExists(path: os.Path) = Task.Command[String] { + if (!os.exists(path)) + os.write(path, Array.emptyByteArray) + while (os.exists(path)) + Thread.sleep(100L) + "Blocking command done" + } } diff --git a/integration/feature/output-directory/src/OutputDirectoryLockTests.scala b/integration/feature/output-directory/src/OutputDirectoryLockTests.scala new file mode 100644 index 00000000000..81b278d0879 --- /dev/null +++ b/integration/feature/output-directory/src/OutputDirectoryLockTests.scala @@ -0,0 +1,82 @@ +package mill.integration + +import mill.testkit.UtestIntegrationTestSuite +import utest._ + +import java.io.ByteArrayOutputStream +import java.util.concurrent.CountDownLatch + +import scala.concurrent.Await +import scala.concurrent.duration.Duration + +object OutputDirectoryLockTests extends UtestIntegrationTestSuite { + + def tests: Tests = Tests { + test("basic") - integrationTest { tester => + import tester._ + val signalFile = workspacePath / "do-wait" + System.err.println("Spawning blocking task") + val blocksFuture = evalAsync(("show", "blockWhileExists", "--path", signalFile), check = true) + while (!os.exists(signalFile) && !blocksFuture.isCompleted) + Thread.sleep(100L) + if (os.exists(signalFile)) + System.err.println("Blocking task is running") + else { + System.err.println("Failed to run blocking task") + Predef.assert(blocksFuture.isCompleted) + blocksFuture.value.get.get + } + + val testCommand: os.Shellable = ("show", "hello") + val testMessage = "Hello from hello task" + + System.err.println("Evaluating task without lock") + val noLockRes = eval(("--no-build-lock", testCommand), check = true) + assert(noLockRes.out.contains(testMessage)) + + System.err.println("Evaluating task without waiting for lock (should fail)") + val noWaitRes = eval(("--no-wait-for-build-lock", testCommand)) + assert(noWaitRes.err.contains("Cannot proceed, another Mill process is running tasks")) + + System.err.println("Evaluating task waiting for the lock") + + val lock = new CountDownLatch(1) + val stderr = new ByteArrayOutputStream + var success = false + val futureWaitingRes = evalAsync( + testCommand, + stderr = os.ProcessOutput { + val expectedMessage = + "Another Mill process is running tasks, waiting for it to be done..." + + (bytes, len) => + stderr.write(bytes, 0, len) + val output = new String(stderr.toByteArray) + if (output.contains(expectedMessage)) + lock.countDown() + }, + check = true + ) + try { + lock.await() + success = true + } finally { + if (!success) { + System.err.println("Waiting task output:") + System.err.write(stderr.toByteArray) + } + } + + System.err.println("Task is waiting for the lock, unblocking it") + os.remove(signalFile) + + System.err.println("Blocking task should exit") + val blockingRes = Await.result(blocksFuture, Duration.Inf) + assert(blockingRes.out.contains("Blocking command done")) + + System.err.println("Waiting task should be free to proceed") + val waitingRes = Await.result(futureWaitingRes, Duration.Inf) + assert(waitingRes.out.contains(testMessage)) + } + } +} diff --git a/testkit/src/mill/testkit/IntegrationTester.scala b/testkit/src/mill/testkit/IntegrationTester.scala index 043b530a5c5..a82958b94ab 100644 --- a/testkit/src/mill/testkit/IntegrationTester.scala +++ b/testkit/src/mill/testkit/IntegrationTester.scala @@ -5,6 +5,11 @@ import mill.eval.Evaluator import mill.resolve.SelectMode import ujson.Value +import java.util.concurrent.atomic.AtomicInteger + +import scala.concurrent.{Future, Promise} +import scala.util.Try + /** * Helper meant for executing Mill integration tests, which runs Mill in a subprocess * against a folder with a `build.mill` and project files. Provides APIs such as [[eval]] @@ -91,6 +96,49 @@ object IntegrationTester { ) } + private val evalAsyncCounter = new AtomicInteger + def evalAsync( + cmd: os.Shellable, + env: Map[String, String] = millTestSuiteEnv, + cwd: os.Path = workspacePath, + stdin: os.ProcessInput = os.Pipe, + stdout: os.ProcessOutput = os.Pipe, + stderr: os.ProcessOutput = os.Pipe, + mergeErrIntoOut: Boolean = false, + timeout: Long = -1, + check: Boolean = false, + propagateEnv: Boolean = true, + timeoutGracePeriod: Long = 100 + ): Future[IntegrationTester.EvalResult] = { + + val promise = Promise[IntegrationTester.EvalResult]() + + val thread = new Thread(s"mill-test-background-eval-${evalAsyncCounter.incrementAndGet()}") { + setDaemon(true) + override def run(): Unit = + promise.complete { + Try { + eval( + cmd = cmd, + env = env, + cwd = cwd, + stdin = stdin, + stdout = stdout, + stderr = stderr, + mergeErrIntoOut = mergeErrIntoOut, + timeout = timeout, + check = check, + propagateEnv = propagateEnv, + timeoutGracePeriod = timeoutGracePeriod + ) + } + } + } + thread.start() + + promise.future + } + def millTestSuiteEnv: Map[String, String] = Map("MILL_TEST_SUITE" -> this.getClass().toString()) /** From c58bac71965e07ea2e59fcb6fe6fc7387799d132 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Tue, 8 Oct 2024 00:56:38 +0200 Subject: [PATCH 5/6] fixup Make Logger extends Closeable and close it via Using.resource --- main/api/src/mill/api/Logger.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main/api/src/mill/api/Logger.scala b/main/api/src/mill/api/Logger.scala index 53e5dce1a51..add24defd80 100644 --- a/main/api/src/mill/api/Logger.scala +++ b/main/api/src/mill/api/Logger.scala @@ -1,6 +1,6 @@ package mill.api -import java.io.{Closeable, InputStream, PrintStream} +import java.io.{InputStream, PrintStream} import mill.main.client.lock.{Lock, Locked} @@ -26,7 +26,7 @@ import mill.main.client.lock.{Lock, Locked} * but when `show` is used both are forwarded to stderr and stdout is only * used to display the final `show` output for easy piping. */ -trait Logger extends Closeable { +trait Logger extends AutoCloseable { def colored: Boolean def systemStreams: SystemStreams From 11e57d452377c1237b48ce297119bda482e51224 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Tue, 8 Oct 2024 10:11:45 +0200 Subject: [PATCH 6/6] . --- .../src/OutputDirectoryLockTests.scala | 44 ++++++++++------- .../src/mill/testkit/IntegrationTester.scala | 48 ------------------- 2 files changed, 27 insertions(+), 65 deletions(-) diff --git a/integration/feature/output-directory/src/OutputDirectoryLockTests.scala b/integration/feature/output-directory/src/OutputDirectoryLockTests.scala index 81b278d0879..25b5a15d078 100644 --- a/integration/feature/output-directory/src/OutputDirectoryLockTests.scala +++ b/integration/feature/output-directory/src/OutputDirectoryLockTests.scala @@ -4,19 +4,27 @@ import mill.testkit.UtestIntegrationTestSuite import utest._ import java.io.ByteArrayOutputStream -import java.util.concurrent.CountDownLatch +import java.util.concurrent.{CountDownLatch, Executors} -import scala.concurrent.Await import scala.concurrent.duration.Duration +import scala.concurrent.{Await, ExecutionContext, Future} object OutputDirectoryLockTests extends UtestIntegrationTestSuite { + private val pool = Executors.newCachedThreadPool() + private val ec = ExecutionContext.fromExecutorService(pool) + + override def utestAfterAll(): Unit = { + pool.shutdown() + } + def tests: Tests = Tests { test("basic") - integrationTest { tester => import tester._ val signalFile = workspacePath / "do-wait" System.err.println("Spawning blocking task") - val blocksFuture = evalAsync(("show", "blockWhileExists", "--path", signalFile), check = true) + val blocksFuture = + Future(eval(("show", "blockWhileExists", "--path", signalFile), check = true))(ec) while (!os.exists(signalFile) && !blocksFuture.isCompleted) Thread.sleep(100L) if (os.exists(signalFile)) @@ -43,20 +51,22 @@ object OutputDirectoryLockTests extends UtestIntegrationTestSuite { val lock = new CountDownLatch(1) val stderr = new ByteArrayOutputStream var success = false - val futureWaitingRes = evalAsync( - testCommand, - stderr = os.ProcessOutput { - val expectedMessage = - "Another Mill process is running tasks, waiting for it to be done..." - - (bytes, len) => - stderr.write(bytes, 0, len) - val output = new String(stderr.toByteArray) - if (output.contains(expectedMessage)) - lock.countDown() - }, - check = true - ) + val futureWaitingRes = Future { + eval( + testCommand, + stderr = os.ProcessOutput { + val expectedMessage = + "Another Mill process is running tasks, waiting for it to be done..." + + (bytes, len) => + stderr.write(bytes, 0, len) + val output = new String(stderr.toByteArray) + if (output.contains(expectedMessage)) + lock.countDown() + }, + check = true + ) + }(ec) try { lock.await() success = true diff --git a/testkit/src/mill/testkit/IntegrationTester.scala b/testkit/src/mill/testkit/IntegrationTester.scala index a82958b94ab..043b530a5c5 100644 --- a/testkit/src/mill/testkit/IntegrationTester.scala +++ b/testkit/src/mill/testkit/IntegrationTester.scala @@ -5,11 +5,6 @@ import mill.eval.Evaluator import mill.resolve.SelectMode import ujson.Value -import java.util.concurrent.atomic.AtomicInteger - -import scala.concurrent.{Future, Promise} -import scala.util.Try - /** * Helper meant for executing Mill integration tests, which runs Mill in a subprocess * against a folder with a `build.mill` and project files. Provides APIs such as [[eval]] @@ -96,49 +91,6 @@ object IntegrationTester { ) } - private val evalAsyncCounter = new AtomicInteger - def evalAsync( - cmd: os.Shellable, - env: Map[String, String] = millTestSuiteEnv, - cwd: os.Path = workspacePath, - stdin: os.ProcessInput = os.Pipe, - stdout: os.ProcessOutput = os.Pipe, - stderr: os.ProcessOutput = os.Pipe, - mergeErrIntoOut: Boolean = false, - timeout: Long = -1, - check: Boolean = false, - propagateEnv: Boolean = true, - timeoutGracePeriod: Long = 100 - ): Future[IntegrationTester.EvalResult] = { - - val promise = Promise[IntegrationTester.EvalResult]() - - val thread = new Thread(s"mill-test-background-eval-${evalAsyncCounter.incrementAndGet()}") { - setDaemon(true) - override def run(): Unit = - promise.complete { - Try { - eval( - cmd = cmd, - env = env, - cwd = cwd, - stdin = stdin, - stdout = stdout, - stderr = stderr, - mergeErrIntoOut = mergeErrIntoOut, - timeout = timeout, - check = check, - propagateEnv = propagateEnv, - timeoutGracePeriod = timeoutGracePeriod - ) - } - } - } - thread.start() - - promise.future - } - def millTestSuiteEnv: Map[String, String] = Map("MILL_TEST_SUITE" -> this.getClass().toString()) /**