Merge branch 'main' into tr-path-mapping-optional

Author: lefou

core/api/src/mill/api/internal/Cacher.scala

@@ -1,17 +1,42 @@
 package mill.api.internal
 
+import collection.mutable.LinkedHashSet
 import scala.collection.mutable
+import scala.util.DynamicVariable
 import scala.quoted.*
 
 trait Cacher extends mill.moduledefs.Cacher {
   private lazy val cacherLazyMap = mutable.Map.empty[sourcecode.Enclosing, Any]
 
   protected def cachedTask[T](t: => T)(using c: sourcecode.Enclosing): T = synchronized {
-    cacherLazyMap.getOrElseUpdate(c, t).asInstanceOf[T]
+    if (Cacher.taskEvaluationStack.value.contains((c, this))) {
+      sys.error(
+        "Circular task dependency detected:\n" +
+          (Cacher.taskEvaluationStack.value.toList ++ Seq((c, this)))
+            .map { case (c, o) =>
+              val taskName = c.value.split("\\.|#| ").filter(!_.startsWith("$anon")).last
+              o.toString match {
+                case "" => taskName
+                case s => s + "." + taskName
+              }
+            }
+            .mkString("\ndepends on: ")
+      )
+    }
+
+    try {
+      Cacher.taskEvaluationStack.value.add((c, this))
+      cacherLazyMap.getOrElseUpdate(c, t).asInstanceOf[T]
+    } finally {
+      Cacher.taskEvaluationStack.value.remove((c, this))
+    }
   }
 }
 
 private[mill] object Cacher {
+  // Use a LinkedHashSet for fast contains checking while preserving insertion order
+  private[mill] val taskEvaluationStack =
+    DynamicVariable[LinkedHashSet[(sourcecode.Enclosing, Any)]](LinkedHashSet())
   private[mill] def withMacroOwner[T](using Quotes)(op: quotes.reflect.Symbol => T): T = {
     import quotes.reflect.*
     // In Scala 3, the top level splice of a macro is owned by a symbol called "macro" with the macro flag set,

core/exec/src/mill/exec/Execution.scala

@@ -218,11 +218,11 @@ private[mill] case class Execution(
                       testReporter = testReporter,
                       logger = contextLogger,
                       deps = deps,
-                      classToTransitiveClasses,
-                      allTransitiveClassMethods,
-                      forkExecutionContext,
-                      exclusive,
-                      upstreamPathRefs
+                      classToTransitiveClasses = classToTransitiveClasses,
+                      allTransitiveClassMethods = allTransitiveClassMethods,
+                      executionContext = forkExecutionContext,
+                      exclusive = exclusive,
+                      upstreamPathRefs = upstreamPathRefs
                     )
 
                     // Count new failures - if there are upstream failures, tasks should be skipped, not failed

core/exec/src/mill/exec/GroupExecution.scala

@@ -45,6 +45,29 @@ trait GroupExecution {
   val effectiveThreadCount: Int =
     ec.map(_.getMaximumPoolSize).getOrElse(1)
 
+  private val envVarsForInterpolation = Seq(
+    "PWD" -> workspace.toString,
+    "PWD_URI" -> workspace.toURI.toString,
+    "MILL_VERSION" -> mill.constants.BuildInfo.millVersion,
+    "MILL_BIN_PLATFORM" -> mill.constants.BuildInfo.millBinPlatform
+  )
+
+  /** Recursively examine all `ujson.Str` values and replace '${VAR}' patterns. */
+  private def interpolateEnvVarsInJson(json: ujson.Value): ujson.Value = {
+    import scala.jdk.CollectionConverters.*
+    val envWithPwd = (env ++ envVarsForInterpolation).asJava
+
+    // recursively convert java data structure to ujson.Value
+    def rec(json: ujson.Value): ujson.Value = json match {
+      case ujson.Str(s) => mill.constants.Util.interpolateEnvVars(s, envWithPwd)
+      case ujson.Arr(xs) => ujson.Arr(xs.map(rec))
+      case ujson.Obj(kvs) => ujson.Obj.from(kvs.map((k, v) => (k, rec(v))))
+      case v => v
+    }
+
+    rec(json)
+  }
+
   // those result which are inputs but not contained in this terminal group
   def executeGroupCached(
       terminal: Task[?],
@@ -62,61 +85,46 @@ trait GroupExecution {
       upstreamPathRefs: Seq[PathRef]
   ): GroupExecution.Results = MappedRoots.withMillDefaults(outPath = outPath) {
 
-    val externalInputsHash = MurmurHash3.orderedHash(
-      group.flatMap(_.inputs).filter(!group.contains(_))
-        .flatMap(results(_).asSuccess.map(_.value._2))
-    )
-
-    val sideHashes = MurmurHash3.orderedHash(group.iterator.map(_.sideHash))
-
-    val scriptsHash = MurmurHash3.orderedHash(
-      group
-        .iterator
-        .collect { case namedTask: Task.Named[_] =>
-          CodeSigUtils.codeSigForTask(
-            namedTask,
-            classToTransitiveClasses,
-            allTransitiveClassMethods,
-            codeSignatures,
-            constructorHashSignatures
-          )
-        }
-        .flatten
-    )
+    val inputsHash = {
+      val externalInputsHash = MurmurHash3.orderedHash(
+        group.flatMap(_.inputs).filter(!group.contains(_))
+          .flatMap(results(_).asSuccess.map(_.value._2))
+      )
+      val sideHashes = MurmurHash3.orderedHash(group.iterator.map(_.sideHash))
+      val scriptsHash = MurmurHash3.orderedHash(
+        group
+          .iterator
+          .collect { case namedTask: Task.Named[_] =>
+            CodeSigUtils.codeSigForTask(
+              namedTask,
+              classToTransitiveClasses,
+              allTransitiveClassMethods,
+              codeSignatures,
+              constructorHashSignatures
+            )
+          }
+          .flatten
+      )
+      // the JVM running this code currently
+      val javaHomeHash = sys.props("java.home").hashCode
 
-    val javaHomeHash = sys.props("java.home").hashCode
-    val inputsHash =
       externalInputsHash + sideHashes + classLoaderSigHash + scriptsHash + javaHomeHash
+    }
 
     terminal match {
 
       case labelled: Task.Named[_] =>
 
-        // recursively convert java data structure to ujson.Value
-        val envWithPwd = env ++ Seq(
-          "PWD" -> workspace.toString,
-          "PWD_URI" -> workspace.toURI.toString,
-          "MILL_VERSION" -> mill.constants.BuildInfo.millVersion,
-          "MILL_BIN_PLATFORM" -> mill.constants.BuildInfo.millBinPlatform
-        )
-
         labelled.ctx.segments.last.value match {
+          // apply build override
           case single if labelled.ctx.enclosingModule.buildOverrides.contains(single) =>
-            val jsonData = labelled.ctx.enclosingModule.buildOverrides(single)
-
-            import scala.jdk.CollectionConverters.*
-            def rec(x: ujson.Value): ujson.Value = x match {
-              case ujson.Str(s) => mill.constants.Util.interpolateEnvVars(s, envWithPwd.asJava)
-              case ujson.Arr(xs) => ujson.Arr(xs.map(rec))
-              case ujson.Obj(kvs) => ujson.Obj.from(kvs.map((k, v) => (k, rec(v))))
-              case v => v
-            }
 
+            val jsonData = labelled.ctx.enclosingModule.buildOverrides(single)
             val (execRes, serializedPaths) =
               try {
                 val (resultData, serializedPaths) = PathRef.withSerializedPaths {
                   PathRef.currentOverrideModulePath.withValue(labelled.ctx.millSourcePath) {
-                    upickle.read[Any](rec(jsonData))(
+                    upickle.read[Any](interpolateEnvVarsInJson(jsonData))(
                       using labelled.readWriterOpt.get.asInstanceOf[upickle.Reader[Any]]
                     )
                   }
@@ -133,14 +141,16 @@ trait GroupExecution {
               }
 
             GroupExecution.Results(
-              Map(labelled -> execRes),
-              Nil,
+              newResults = Map(labelled -> execRes),
+              newEvaluated = Nil,
               cached = true,
-              inputsHash,
-              -1,
-              false,
-              serializedPaths
+              inputsHash = inputsHash,
+              previousInputsHash = -1,
+              valueHashChanged = false,
+              serializedPaths = serializedPaths
             )
+
+          // no build overrides
           case _ =>
             val out = if (!labelled.ctx.external) outPath else externalOutPath
             val paths = ExecutionPaths.resolve(out, labelled.ctx.segments)
@@ -149,18 +159,18 @@ trait GroupExecution {
             // `cached.isEmpty` means worker metadata file removed by user so recompute the worker
             val (multiLogger, fileLoggerOpt) = resolveLogger(Some(paths).map(_.log), logger)
             val upToDateWorker = loadUpToDateWorker(
-              logger,
-              inputsHash,
-              labelled,
-              cached.isEmpty,
-              deps,
-              Some(paths),
-              upstreamPathRefs,
-              exclusive,
-              multiLogger,
-              countMsg,
-              new GroupExecution.DestCreator(Some(paths)),
-              terminal
+              logger = logger,
+              inputsHash = inputsHash,
+              labelled = labelled,
+              forceDiscard = cached.isEmpty,
+              deps = deps,
+              paths = Some(paths),
+              upstreamPathRefs = upstreamPathRefs,
+              exclusive = exclusive,
+              multiLogger = multiLogger,
+              counterMsg = countMsg,
+              destCreator = new GroupExecution.DestCreator(Some(paths)),
+              terminal = terminal
             )
 
             val cachedValueAndHash =
@@ -176,13 +186,13 @@ trait GroupExecution {
                   Map(labelled -> res)
 
                 GroupExecution.Results(
-                  newResults,
-                  Nil,
+                  newResults = newResults,
+                  newEvaluated = Nil,
                   cached = true,
-                  inputsHash,
-                  -1,
+                  inputsHash = inputsHash,
+                  previousInputsHash = -1,
                   valueHashChanged = false,
-                  serializedPaths
+                  serializedPaths = serializedPaths
                 )
 
               case _ =>
@@ -224,13 +234,13 @@ trait GroupExecution {
                 }
 
                 GroupExecution.Results(
-                  newResults,
-                  newEvaluated.toSeq,
+                  newResults = newResults,
+                  newEvaluated = newEvaluated.toSeq,
                   cached = if (labelled.isInstanceOf[Task.Input[?]]) null else false,
-                  inputsHash,
-                  cached.map(_.inputHash).getOrElse(-1),
-                  !cached.map(_.valueHash).contains(valueHash),
-                  serializedPaths
+                  inputsHash = inputsHash,
+                  previousInputsHash = cached.map(_.inputHash).getOrElse(-1),
+                  valueHashChanged = !cached.map(_.valueHash).contains(valueHash),
+                  serializedPaths = serializedPaths
                 )
             }
         }
@@ -252,11 +262,11 @@ trait GroupExecution {
           terminal = terminal
         )
         GroupExecution.Results(
-          newResults,
-          newEvaluated.toSeq,
-          null,
-          inputsHash,
-          -1,
+          newResults = newResults,
+          newEvaluated = newEvaluated.toSeq,
+          cached = null,
+          inputsHash = inputsHash,
+          previousInputsHash = -1,
           valueHashChanged = false,
           serializedPaths = Nil
         )
@@ -312,20 +322,20 @@ trait GroupExecution {
           )
 
           GroupExecution.wrap(
-            workspace,
-            deps,
-            outPath,
-            paths,
-            upstreamPathRefs,
-            exclusive,
-            multiLogger,
-            logger,
-            exclusiveSystemStreams,
-            counterMsg,
-            destCreator,
-            getEvaluator().asInstanceOf[Evaluator],
-            terminal,
-            rootModule.getClass.getClassLoader
+            workspace = workspace,
+            deps = deps,
+            outPath = outPath,
+            paths = paths,
+            upstreamPathRefs = upstreamPathRefs,
+            exclusive = exclusive,
+            multiLogger = multiLogger,
+            logger = logger,
+            exclusiveSystemStreams = exclusiveSystemStreams,
+            counterMsg = counterMsg,
+            destCreator = destCreator,
+            evaluator = getEvaluator().asInstanceOf[Evaluator],
+            terminal = terminal,
+            classLoader = rootModule.getClass.getClassLoader
           ) {
             try {
               task.evaluate(args) match {
@@ -564,7 +574,7 @@ object GroupExecution {
     def onRead(path: os.ReadablePath): Unit = path match {
       case path: os.Path =>
         if (!isCommand && !isInput && mill.api.FilesystemCheckerEnabled.value) {
-          if (path.startsWith(workspace) && !validReadDests.exists(path.startsWith(_))) {
+          if (path.startsWith(workspace) && !validReadDests.exists(path.startsWith)) {
             sys.error(
               s"Reading from ${path.relativeTo(workspace)} not allowed during execution of `$terminal`.\n" +
                 "You can only read files referenced by `Task.Source` or `Task.Sources`, or within a `Task.Input"
@@ -576,7 +586,7 @@ object GroupExecution {
 
     def onWrite(path: os.Path): Unit = {
       if (!isCommand && mill.api.FilesystemCheckerEnabled.value) {
-        if (path.startsWith(workspace) && !validWriteDests.exists(path.startsWith(_))) {
+        if (path.startsWith(workspace) && !validWriteDests.exists(path.startsWith)) {
           sys.error(
             s"Writing to ${path.relativeTo(workspace)} not allowed during execution of `$terminal`.\n" +
               "Normal `Task`s can only write to files within their `Task.dest` folder, only `Task.Command`s can write to other arbitrary files."

example/package.mill

@@ -151,8 +151,8 @@ object `package` extends Module {
     val upstreamOpt = upstreamCross(this.moduleSegments.parts.dropRight(1).last)
       .flatMap(_.valuesToModules.get(List(crossValue)))
 
-    def testRepoRoot = Task {
-      os.copy(super.testRepoRoot().path, Task.dest, mergeFolders = true)
+    def testRepoSourceRoot = Task {
+      os.copy(super.testRepoSourceRoot().path, Task.dest, mergeFolders = true)
       for (suffix <- Seq("build.mill")) {
         for (lines <- buildScLines() if os.exists(Task.dest / suffix)) {
           os.write.over(Task.dest / suffix, lines.mkString("\n"))
@@ -177,10 +177,10 @@ object `package` extends Module {
       case None => Task { None }
       case Some(upstream) => Task {
           Some {
-            val upstreamRoot = upstream.testRepoRoot().path
+            val upstreamRoot = upstream.testRepoSourceRoot().path
             val suffix = Seq("build.mill").find(s => os.exists(upstreamRoot / s)).head
-            val upstreamLines = os.read.lines(upstream.testRepoRoot().path / suffix)
-            val lines = os.read.lines(super.testRepoRoot().path / suffix)
+            val upstreamLines = os.read.lines(upstream.testRepoSourceRoot().path / suffix)
+            val lines = os.read.lines(super.testRepoSourceRoot().path / suffix)
 
             import collection.mutable
             val groupedLines = mutable.Map.empty[String, mutable.Buffer[String]]
@@ -242,7 +242,7 @@ object `package` extends Module {
      * Parses a `build.mill` for specific comments and return the split-by-type content
      */
     def parsed: T[Seq[(String, String)]] = Task {
-      mill.testkit.ExampleParser(testRepoRoot().path)
+      mill.testkit.ExampleParser(testRepoSourceRoot().path)
     }
 
     def rendered = Task {

integration/failure/circular-tasks/resources/build.mill

@@ -0,0 +1,11 @@
+package build
+import mill._
+
+object module extends Module {
+
+  object nested extends Module {
+    def taskA: T[String] = Task { taskB() }
+  }
+  def taskB: T[String] = Task { taskC() }
+}
+def taskC: T[String] = Task { module.nested.taskA() }