diff --git a/integration/feature/output-directory/resources/build.mill b/integration/feature/output-directory/resources/build.mill index e068bbd11cb..21d1473b74c 100644 --- a/integration/feature/output-directory/resources/build.mill +++ b/integration/feature/output-directory/resources/build.mill @@ -11,10 +11,15 @@ object `package` extends RootModule with ScalaModule { } def blockWhileExists(path: os.Path) = Task.Command[String] { - if (!os.exists(path)) - os.write(path, Array.emptyByteArray) - while (os.exists(path)) - Thread.sleep(100L) + os.write(path, Array.emptyByteArray) + + while (os.exists(path)) Thread.sleep(100L) "Blocking command done" } + + def writeMarker(path: os.Path) = Task.Command[String] { + os.write(path, Array.emptyByteArray) + + "Write marker done" + } } diff --git a/integration/feature/output-directory/src/OutputDirectoryLockTests.scala b/integration/feature/output-directory/src/OutputDirectoryLockTests.scala index 25b5a15d078..bcfce43b82f 100644 --- a/integration/feature/output-directory/src/OutputDirectoryLockTests.scala +++ b/integration/feature/output-directory/src/OutputDirectoryLockTests.scala @@ -2,91 +2,78 @@ package mill.integration import mill.testkit.UtestIntegrationTestSuite import utest._ +import utest.asserts.{RetryInterval, RetryMax} -import java.io.ByteArrayOutputStream -import java.util.concurrent.{CountDownLatch, Executors} - -import scala.concurrent.duration.Duration +import java.util.concurrent.Executors +import scala.concurrent.duration.{Duration, DurationInt} import scala.concurrent.{Await, ExecutionContext, Future} object OutputDirectoryLockTests extends UtestIntegrationTestSuite { private val pool = Executors.newCachedThreadPool() - private val ec = ExecutionContext.fromExecutorService(pool) + private implicit val ec = ExecutionContext.fromExecutorService(pool) override def utestAfterAll(): Unit = { pool.shutdown() } - + implicit val retryMax: RetryMax = RetryMax(30000.millis) + implicit val retryInterval: RetryInterval = RetryInterval(50.millis) def tests: Tests = Tests { test("basic") - integrationTest { tester => import tester._ val signalFile = workspacePath / "do-wait" - System.err.println("Spawning blocking task") - val blocksFuture = - Future(eval(("show", "blockWhileExists", "--path", signalFile), check = true))(ec) - 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 + // Kick off blocking task in background + Future { + eval(("show", "blockWhileExists", "--path", signalFile), check = true) } + // Wait for blocking task to write signal file, to indicate it has begun + eventually { os.exists(signalFile) } + val testCommand: os.Shellable = ("show", "hello") val testMessage = "Hello from hello task" - System.err.println("Evaluating task without lock") + // --no-build-lock allows commands to complete despite background blocker 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)") + // --no-wait-for-build-lock causes commands fail due to background blocker 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 + assert( + noWaitRes + .err + .contains( + s"Another Mill process is running 'show blockWhileExists --path $signalFile', failing" + ) + ) + + // By default, we wait until the background blocking task completes + val waitingLogFile = workspacePath / "waitingLogFile" + val waitingCompleteFile = workspacePath / "waitingCompleteFile" 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() - }, + ("show", "writeMarker", "--path", waitingCompleteFile), + stderr = waitingLogFile, check = true ) - }(ec) - 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")) + // Ensure we see the waiting message + eventually { + os.read(waitingLogFile) + .contains( + s"Another Mill process is running 'show blockWhileExists --path $signalFile', waiting for it to be done..." + ) + } - System.err.println("Waiting task should be free to proceed") + // Even after task starts waiting on blocking task, it is not complete + assert(!futureWaitingRes.isCompleted) + assert(!os.exists(waitingCompleteFile)) + // Terminate blocking task, make sure waiting task now completes + os.remove(signalFile) val waitingRes = Await.result(futureWaitingRes, Duration.Inf) - assert(waitingRes.out.contains(testMessage)) + assert(os.exists(waitingCompleteFile)) + assert(waitingRes.out == "\"Write marker done\"") } } } diff --git a/main/api/src/mill/api/Logger.scala b/main/api/src/mill/api/Logger.scala index f50543dac32..430b6352f99 100644 --- a/main/api/src/mill/api/Logger.scala +++ b/main/api/src/mill/api/Logger.scala @@ -1,7 +1,6 @@ package mill.api import java.io.{InputStream, PrintStream} -import mill.main.client.lock.{Lock, Locked} /** * The standard logging interface of the Mill build tool. @@ -85,19 +84,6 @@ trait Logger extends AutoCloseable { finally removePromptLine() } - def waitForLock(lock: Lock, waitingAllowed: Boolean): Locked = { - val tryLocked = lock.tryLock() - if (tryLocked.isLocked()) - tryLocked - 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") - } - } - def withOutStream(outStream: PrintStream): Logger = this private[mill] def logPrefixKey: Seq[String] = Nil } diff --git a/main/client/src/mill/main/client/OutFiles.java b/main/client/src/mill/main/client/OutFiles.java index 0af23e233a1..b69b93b535c 100644 --- a/main/client/src/mill/main/client/OutFiles.java +++ b/main/client/src/mill/main/client/OutFiles.java @@ -62,4 +62,9 @@ public class OutFiles { */ final public static String millLock = "mill-lock"; + /** + * Any active Mill command that is currently run, for debugging purposes + */ + final public static String millActiveCommand = "mill-active-command"; + } diff --git a/main/client/src/mill/main/client/lock/DummyLock.java b/main/client/src/mill/main/client/lock/DummyLock.java deleted file mode 100644 index ede8224323d..00000000000 --- a/main/client/src/mill/main/client/lock/DummyLock.java +++ /dev/null @@ -1,22 +0,0 @@ -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 deleted file mode 100644 index 34ad7b5ea15..00000000000 --- a/main/client/src/mill/main/client/lock/DummyTryLocked.java +++ /dev/null @@ -1,11 +0,0 @@ -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 3870bc07a14..d7f65ee2b85 100644 --- a/main/client/src/mill/main/client/lock/Lock.java +++ b/main/client/src/mill/main/client/lock/Lock.java @@ -23,9 +23,4 @@ public static Lock file(String path) throws Exception { 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 429a394f18a..afd41f14f95 100644 --- a/runner/src/mill/runner/MillCliConfig.scala +++ b/runner/src/mill/runner/MillCliConfig.scala @@ -140,7 +140,7 @@ case class MillCliConfig( @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.""" + """Do not wait for an exclusive lock on the Mill output directory to evaluate tasks / commands.""" ) noWaitForBuildLock: Flag = Flag() ) diff --git a/runner/src/mill/runner/MillMain.scala b/runner/src/mill/runner/MillMain.scala index 817e126af7c..606c4ca2fd0 100644 --- a/runner/src/mill/runner/MillMain.scala +++ b/runner/src/mill/runner/MillMain.scala @@ -10,7 +10,7 @@ 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 mill.util.{Colors, PrintLogger, PromptLogger} import java.lang.reflect.InvocationTargetException import scala.util.control.NonFatal @@ -212,9 +212,7 @@ object MillMain { .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) { @@ -228,44 +226,46 @@ object MillMain { evaluate = (prevState: Option[RunnerState]) => { adjustJvmProperties(userSpecifiedProperties, initialSystemProperties) - val logger = getLogger( - streams, - config, - mainInteractive, - enableTicker = - config.ticker - .orElse(config.enableTicker) - .orElse(Option.when(config.disableTicker.value)(false)), - printLoggerState, - serverDir, - colored = colored, - colors = colors - ) - Using.resources( - logger, - logger.waitForLock( - outLock, - waitingAllowed = !config.noWaitForBuildLock.value + withOutLock( + config.noBuildLock.value || bspContext.isDefined, + config.noWaitForBuildLock.value, + out, + targetsAndParams, + streams + ) { + val logger = getLogger( + streams, + config, + mainInteractive, + enableTicker = + config.ticker + .orElse(config.enableTicker) + .orElse(Option.when(config.disableTicker.value)(false)), + printLoggerState, + serverDir, + colored = colored, + colors = colors ) - ) { (_, _) => - new MillBuildBootstrap( - projectRoot = WorkspaceRoot.workspaceRoot, - output = out, - home = config.home, - keepGoing = config.keepGoing.value, - imports = config.imports, - env = env, - threadCount = threadCount, - targetsAndParams = targetsAndParams, - prevRunnerState = prevState.getOrElse(stateCache), - logger = logger, - disableCallgraph = config.disableCallgraph.value, - needBuildSc = needBuildSc(config), - requestedMetaLevel = config.metaLevel, - config.allowPositional.value, - systemExit = systemExit, - streams0 = streams0 - ).evaluate() + Using.resource(logger) { _ => + try new MillBuildBootstrap( + projectRoot = WorkspaceRoot.workspaceRoot, + output = out, + home = config.home, + keepGoing = config.keepGoing.value, + imports = config.imports, + env = env, + threadCount = threadCount, + targetsAndParams = targetsAndParams, + prevRunnerState = prevState.getOrElse(stateCache), + logger = logger, + disableCallgraph = config.disableCallgraph.value, + needBuildSc = needBuildSc(config), + requestedMetaLevel = config.metaLevel, + config.allowPositional.value, + systemExit = systemExit, + streams0 = streams0 + ).evaluate() + } } }, colors = colors @@ -416,4 +416,44 @@ object MillMain { for (k <- systemPropertiesToUnset) System.clearProperty(k) for ((k, v) <- desiredProps) System.setProperty(k, v) } + + def withOutLock[T]( + noBuildLock: Boolean, + noWaitForBuildLock: Boolean, + out: os.Path, + targetsAndParams: Seq[String], + streams: SystemStreams + )(t: => T): T = { + if (noBuildLock) t + else { + val outLock = Lock.file((out / OutFiles.millLock).toString) + + def activeTaskString = + try { + os.read(out / OutFiles.millActiveCommand) + } catch { + case e => "" + } + + def activeTaskPrefix = s"Another Mill process is running '$activeTaskString'," + Using.resource { + val tryLocked = outLock.tryLock() + if (tryLocked.isLocked()) tryLocked + else if (noWaitForBuildLock) { + throw new Exception(s"$activeTaskPrefix failing") + } else { + + streams.err.println( + s"$activeTaskPrefix waiting for it to be done..." + ) + outLock.lock() + } + } { _ => + os.write.over(out / OutFiles.millActiveCommand, targetsAndParams.mkString(" ")) + try t + finally os.remove.all(out / OutFiles.millActiveCommand) + } + } + } + }