From 61cdc640fef711336cb4e02d75acf2df42d3992f Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Mon, 23 Sep 2024 16:43:02 +0200 Subject: [PATCH] Don't run tasks concurrently via a global lock --- main/api/src/mill/api/Logger.scala | 12 ++++++++++++ .../client/src/mill/main/client/OutFiles.java | 5 +++++ .../src/mill/main/client/lock/Lock.java | 9 +++++++++ main/eval/src/mill/eval/Evaluator.scala | 2 ++ main/eval/src/mill/eval/EvaluatorCore.scala | 17 ++++++++++++++--- main/eval/src/mill/eval/EvaluatorImpl.scala | 2 ++ main/eval/src/mill/eval/GroupEvaluator.scala | 2 ++ .../src/mill/runner/MillBuildBootstrap.scala | 19 +++++++++++++------ runner/src/mill/runner/MillMain.scala | 6 +++++- testkit/src/mill/testkit/UnitTester.scala | 5 ++++- 10 files changed, 68 insertions(+), 11 deletions(-) diff --git a/main/api/src/mill/api/Logger.scala b/main/api/src/mill/api/Logger.scala index 3c6967558e9..c9053d158bf 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.{InputStream, PrintStream} +import mill.main.client.lock.{Lock, Locked} + /** * The standard logging interface of the Mill build tool. * @@ -53,4 +55,14 @@ trait Logger { def debugEnabled: Boolean = false def close(): Unit = () + + def waitForLock(lock: Lock): Locked = { + val tryLocked = lock.tryLock() + if (tryLocked.isLocked()) + tryLocked + else { + info("Waiting for lock...") + lock.lock() + } + } } diff --git a/main/client/src/mill/main/client/OutFiles.java b/main/client/src/mill/main/client/OutFiles.java index 872560cfda3..14595a7d699 100644 --- a/main/client/src/mill/main/client/OutFiles.java +++ b/main/client/src/mill/main/client/OutFiles.java @@ -53,4 +53,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/Lock.java b/main/client/src/mill/main/client/lock/Lock.java index 6d729c0ebd6..1169ccff5b6 100644 --- a/main/client/src/mill/main/client/lock/Lock.java +++ b/main/client/src/mill/main/client/lock/Lock.java @@ -15,4 +15,13 @@ 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(); + } + } diff --git a/main/eval/src/mill/eval/Evaluator.scala b/main/eval/src/mill/eval/Evaluator.scala index 1a1d2853cb5..5258d39c456 100644 --- a/main/eval/src/mill/eval/Evaluator.scala +++ b/main/eval/src/mill/eval/Evaluator.scala @@ -4,6 +4,7 @@ import mill.api.{CompileProblemReporter, DummyTestReporter, Result, TestReporter import mill.api.Strict.Agg import mill.define.{BaseModule, Segments, Task} import mill.eval.Evaluator.{Results, formatFailing} +import mill.main.client.lock.Lock import mill.util.{ColorLogger, MultiBiMap} import scala.annotation.nowarn @@ -19,6 +20,7 @@ trait Evaluator { def rootModule: BaseModule def effectiveThreadCount: Int def outPath: os.Path + def outPathLock: Lock def externalOutPath: os.Path def pathsResolver: EvaluatorPathsResolver def workerCache: collection.Map[Segments, (Int, Val)] diff --git a/main/eval/src/mill/eval/EvaluatorCore.scala b/main/eval/src/mill/eval/EvaluatorCore.scala index 777640d7eba..92364f6c4ee 100644 --- a/main/eval/src/mill/eval/EvaluatorCore.scala +++ b/main/eval/src/mill/eval/EvaluatorCore.scala @@ -12,6 +12,7 @@ import mill.util._ import java.util.concurrent.atomic.{AtomicBoolean, AtomicInteger} import scala.collection.mutable import scala.concurrent._ +import scala.util.Using /** * Core logic of evaluating tasks, without any user-facing helper methods @@ -32,8 +33,6 @@ private[mill] trait EvaluatorCore extends GroupEvaluator { logger: ColorLogger = baseLogger, serialCommandExec: Boolean = false ): Evaluator.Results = { - os.makeDir.all(outPath) - PathRef.validatedPaths.withValue(new PathRef.ValidatedPaths()) { val ec = if (effectiveThreadCount == 1) ExecutionContexts.RunNow @@ -43,7 +42,19 @@ private[mill] trait EvaluatorCore extends GroupEvaluator { if (effectiveThreadCount == 1) "" else s"#${if (effectiveThreadCount > 9) f"$threadId%02d" else threadId} " - try evaluate0(goals, logger, reporter, testReporter, ec, contextLoggerMsg, serialCommandExec) + try + Using.resource(logger.waitForLock(outPathLock)) { _ => + os.makeDir.all(outPath) + evaluate0( + goals, + logger, + reporter, + testReporter, + ec, + contextLoggerMsg, + serialCommandExec + ) + } finally ec.close() } } diff --git a/main/eval/src/mill/eval/EvaluatorImpl.scala b/main/eval/src/mill/eval/EvaluatorImpl.scala index 508bf7b1f5c..d6b63870c57 100644 --- a/main/eval/src/mill/eval/EvaluatorImpl.scala +++ b/main/eval/src/mill/eval/EvaluatorImpl.scala @@ -3,6 +3,7 @@ package mill.eval import mill.api.{CompileProblemReporter, Strict, TestReporter, Val} import mill.api.Strict.Agg import mill.define._ +import mill.main.client.lock.Lock import mill.util._ import scala.collection.mutable @@ -17,6 +18,7 @@ private[mill] case class EvaluatorImpl( home: os.Path, workspace: os.Path, outPath: os.Path, + outPathLock: Lock, externalOutPath: os.Path, override val rootModule: mill.define.BaseModule, baseLogger: ColorLogger, diff --git a/main/eval/src/mill/eval/GroupEvaluator.scala b/main/eval/src/mill/eval/GroupEvaluator.scala index 7d1529801ea..410a5742385 100644 --- a/main/eval/src/mill/eval/GroupEvaluator.scala +++ b/main/eval/src/mill/eval/GroupEvaluator.scala @@ -5,6 +5,7 @@ import mill.api.Strict.Agg import mill.api._ import mill.define._ import mill.eval.Evaluator.TaskResult +import mill.main.client.lock.Lock import mill.util._ import java.io.PrintStream @@ -23,6 +24,7 @@ private[mill] trait GroupEvaluator { def home: os.Path def workspace: os.Path def outPath: os.Path + def outPathLock: Lock def externalOutPath: os.Path def rootModule: mill.define.BaseModule def classLoaderSigHash: Int diff --git a/runner/src/mill/runner/MillBuildBootstrap.scala b/runner/src/mill/runner/MillBuildBootstrap.scala index ad84d5eb956..b2969fb3836 100644 --- a/runner/src/mill/runner/MillBuildBootstrap.scala +++ b/runner/src/mill/runner/MillBuildBootstrap.scala @@ -6,12 +6,15 @@ import mill.main.client.CodeGenConstants._ import mill.api.{PathRef, Val, internal} import mill.eval.Evaluator import mill.main.RunScript +import mill.main.client.lock.Lock import mill.resolve.SelectMode import mill.define.{BaseModule, Discover, Segments} import mill.main.client.OutFiles.{millBuild, millRunnerState} import java.net.URLClassLoader +import scala.util.Using + /** * Logic around bootstrapping Mill, creating a [[MillBuildRootModule.BootstrapModule]] * and compiling builds/meta-builds and classloading their [[RootModule]]s so we @@ -31,6 +34,7 @@ import java.net.URLClassLoader class MillBuildBootstrap( projectRoot: os.Path, output: os.Path, + outputLock: Lock, home: os.Path, keepGoing: Boolean, imports: Seq[String], @@ -53,12 +57,14 @@ class MillBuildBootstrap( def evaluate(): Watching.Result[RunnerState] = CliImports.withValue(imports) { val runnerState = evaluateRec(0) - for ((frame, depth) <- runnerState.frames.zipWithIndex) { - os.write.over( - recOut(output, depth) / millRunnerState, - upickle.default.write(frame.loggedData, indent = 4), - createFolders = true - ) + Using.resource(logger.waitForLock(outputLock)) { _ => + for ((frame, depth) <- runnerState.frames.zipWithIndex) { + os.write.over( + recOut(output, depth) / millRunnerState, + upickle.default.write(frame.loggedData, indent = 4), + createFolders = true + ) + } } Watching.Result( @@ -344,6 +350,7 @@ class MillBuildBootstrap( home, projectRoot, recOut(output, depth), + outputLock, recOut(output, depth), rootModule, PrefixLogger(logger, "", tickerContext = bootLogPrefix), diff --git a/runner/src/mill/runner/MillMain.scala b/runner/src/mill/runner/MillMain.scala index e48179b2702..3e23517f8f3 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 +import mill.main.client.lock.Lock import mill.util.PrintLogger import java.lang.reflect.InvocationTargetException @@ -215,6 +216,8 @@ object MillMain { .map(_ => Seq(bspCmd)) .getOrElse(config.leftoverArgs.value.toList) + val out = os.Path(OutFiles.out, WorkspaceRoot.workspaceRoot) + val outLock = Lock.file((out / OutFiles.millLock).toString) var repeatForBsp = true var loopRes: (Boolean, RunnerState) = (false, RunnerState.empty) while (repeatForBsp) { @@ -231,7 +234,8 @@ object MillMain { new MillBuildBootstrap( projectRoot = WorkspaceRoot.workspaceRoot, - output = os.Path(OutFiles.out, WorkspaceRoot.workspaceRoot), + output = out, + outputLock = outLock, home = config.home, keepGoing = config.keepGoing.value, imports = config.imports, diff --git a/testkit/src/mill/testkit/UnitTester.scala b/testkit/src/mill/testkit/UnitTester.scala index 1d921c67477..411e7728676 100644 --- a/testkit/src/mill/testkit/UnitTester.scala +++ b/testkit/src/mill/testkit/UnitTester.scala @@ -5,6 +5,7 @@ import mill.api.Result.OuterStack import mill.api.{DummyInputStream, Result, SystemStreams, Val} import mill.define.{InputImpl, TargetImpl} import mill.eval.{Evaluator, EvaluatorImpl} +import mill.main.client.lock.Lock import mill.resolve.{Resolve, SelectMode} import mill.util.PrintLogger import mill.api.Strict.Agg @@ -55,7 +56,8 @@ class UnitTester( env: Map[String, String], resetSourcePath: Boolean )(implicit fullName: sourcecode.FullName) extends AutoCloseable { - val outPath: os.Path = module.millSourcePath / "out" + val outPath = module.millSourcePath / "out" + val outPathLock = Lock.file((module.millSourcePath / "out/.mill-lock").toString) if (resetSourcePath) { os.remove.all(module.millSourcePath) @@ -90,6 +92,7 @@ class UnitTester( mill.api.Ctx.defaultHome, module.millSourcePath, outPath, + outPathLock, outPath, module, logger,