Add proper error message when circular task dependency is detected (#6044)

This provides a nicer error that is easier to understand than the
`StackOverflow` that was reported before

Author: lihaoyi

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,

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() }

integration/failure/circular-tasks/src/CircularTasksTests.scala

@@ -0,0 +1,23 @@
+package mill.integration
+
+import mill.testkit.UtestIntegrationTestSuite
+
+import utest._
+
+object CircularTasksTests extends UtestIntegrationTestSuite {
+  def captureOutErr = true
+  val tests: Tests = Tests {
+    test - integrationTest { tester =>
+      // Ensure that resolve works even of the modules containing the resolved
+      // tasks are broken
+      val res = tester.eval(("module.nested.taskA"))
+      assert(res.err.contains(
+        """Circular task dependency detected:
+          |module.nested.taskA
+          |depends on: module.taskB
+          |depends on: taskC
+          |depends on: module.nested.taskA""".stripMargin.replace("\r\n", "\n")
+      ))
+    }
+  }
+}