Add nice error when non-exclusive task depends on exclusive task (#3887)

Fixes #3875

I placed a check to raise an error when a non-exclusive task depends on
an exclusive task.

We could place this check any time from task instantiation to just
before the `NoSuchElementException` gets thrown. I ended up putting it
as late as possible, such that the failure is localized, which allows
other un-related parts of the build to proceed despite the failure, as
well as allowing other errors in the build to be reported in parallel
(rather than having to re-run the build over and over after fixing each
one to see the next one)

Covered with an integration test that asserts on the error message. Best
reviewed with Ignore Whitespace

integration/failure/non-exclusive-depends-on-exclusive/resources/build.mill

@@ -0,0 +1,13 @@
+package build
+import mill._, mill.eval.Evaluator
+
+def foo = Task{1}
+def cleanClientWrong(ev: Evaluator) = Task.Command {
+  clean(ev, "foo")()
+  println("cleanClientWrong done")
+}
+
+def cleanClientRight(ev: Evaluator) = Task.Command(exclusive = true) {
+  clean(ev, "foo")()
+  println("cleanClientRight done")
+}

integration/failure/non-exclusive-depends-on-exclusive/src/NonExclusiveDependsOnExclusive.scala

@@ -0,0 +1,24 @@
+package mill.integration
+
+import mill.testkit.UtestIntegrationTestSuite
+
+import utest._
+
+object NonExclusiveDependsOnExclusive extends UtestIntegrationTestSuite {
+  val tests: Tests = Tests {
+    test("wrong") - integrationTest { tester =>
+      val res = tester.eval("cleanClientWrong")
+      assert(res.isSuccess == false)
+      assert(res.err.contains(
+        "Non-exclusive task cleanClientWrong cannot depend on exclusive task clean"
+      ))
+      assert(!res.out.contains("cleanClientWrong done"))
+    }
+    test("right") - integrationTest { tester =>
+      val res = tester.eval("cleanClientRight")
+      assert(res.isSuccess == true)
+      assert(res.out.contains("cleanClientRight done"))
+
+    }
+  }
+}

main/eval/src/mill/eval/EvaluatorCore.scala

@@ -99,94 +99,112 @@ private[mill] trait EvaluatorCore extends GroupEvaluator {
       // due to the topological order of traversal.
       for (terminal <- terminals) {
         val deps = interGroupDeps(terminal)
-        futures(terminal) = Future.sequence(deps.map(futures)).map { upstreamValues =>
-          val countMsg = mill.util.Util.leftPad(
-            count.getAndIncrement().toString,
-            terminals.length.toString.length,
-            '0'
-          )
-
-          val verboseKeySuffix = s"/${terminals0.size}"
-          logger.setPromptHeaderPrefix(s"$countMsg$verboseKeySuffix")
-          if (failed.get()) None
-          else {
-            val upstreamResults = upstreamValues
-              .iterator
-              .flatMap(_.iterator.flatMap(_.newResults))
-              .toMap
-
-            val startTime = System.nanoTime() / 1000
-            val targetLabel = terminal match {
-              case Terminal.Task(task) => None
-              case t: Terminal.Labelled[_] => Some(Terminal.printTerm(t))
-            }
-
-            val group = sortedGroups.lookupKey(terminal)
-
-            // should we log progress?
-            val logRun = targetLabel.isDefined && {
-              val inputResults = for {
-                target <- group.indexed.filterNot(upstreamResults.contains)
-                item <- target.inputs.filterNot(group.contains)
-              } yield upstreamResults(item).map(_._1)
-              inputResults.forall(_.result.isInstanceOf[Result.Success[_]])
-            }
+        def isExclusiveCommand(t: Task[_]) = t match {
+          case c: Command[_] if c.exclusive => true
+          case _ => false
+        }
 
-            val tickerPrefix = terminal.render.collect {
-              case targetLabel if logRun && logger.enableTicker => targetLabel
-            }
+        val group = sortedGroups.lookupKey(terminal)
+        val exclusiveDeps = deps.filter(d => isExclusiveCommand(d.task))
 
-            val contextLogger = new PrefixLogger(
-              logger0 = logger,
-              key0 = if (!logger.enableTicker) Nil else Seq(countMsg),
-              verboseKeySuffix = verboseKeySuffix,
-              message = tickerPrefix,
-              noPrefix = exclusive
+        if (!isExclusiveCommand(terminal.task) && exclusiveDeps.nonEmpty) {
+          val failure = Result.Failure(
+            s"Non-exclusive task ${terminal.render} cannot depend on exclusive task " +
+              exclusiveDeps.map(_.render).mkString(", ")
+          )
+          val taskResults =
+            group.map(t => (t, TaskResult[(Val, Int)](failure, () => failure))).toMap
+          futures(terminal) = Future.successful(
+            Some(GroupEvaluator.Results(taskResults, group.toSeq, false, -1, -1))
+          )
+        } else {
+          futures(terminal) = Future.sequence(deps.map(futures)).map { upstreamValues =>
+            val countMsg = mill.util.Util.leftPad(
+              count.getAndIncrement().toString,
+              terminals.length.toString.length,
+              '0'
             )
 
-            val res = evaluateGroupCached(
-              terminal = terminal,
-              group = sortedGroups.lookupKey(terminal),
-              results = upstreamResults,
-              countMsg = countMsg,
-              verboseKeySuffix = verboseKeySuffix,
-              zincProblemReporter = reporter,
-              testReporter = testReporter,
-              logger = contextLogger,
-              classToTransitiveClasses,
-              allTransitiveClassMethods,
-              forkExecutionContext,
-              exclusive
-            )
+            val verboseKeySuffix = s"/${terminals0.size}"
+            logger.setPromptHeaderPrefix(s"$countMsg$verboseKeySuffix")
+            if (failed.get()) None
+            else {
+              val upstreamResults = upstreamValues
+                .iterator
+                .flatMap(_.iterator.flatMap(_.newResults))
+                .toMap
+
+              val startTime = System.nanoTime() / 1000
+              val targetLabel = terminal match {
+                case Terminal.Task(task) => None
+                case t: Terminal.Labelled[_] => Some(Terminal.printTerm(t))
+              }
+
+              // should we log progress?
+              val logRun = targetLabel.isDefined && {
+                val inputResults = for {
+                  target <- group.indexed.filterNot(upstreamResults.contains)
+                  item <- target.inputs.filterNot(group.contains)
+                } yield upstreamResults(item).map(_._1)
+                inputResults.forall(_.result.isInstanceOf[Result.Success[_]])
+              }
+
+              val tickerPrefix = terminal.render.collect {
+                case targetLabel if logRun && logger.enableTicker => targetLabel
+              }
+
+              val contextLogger = new PrefixLogger(
+                logger0 = logger,
+                key0 = if (!logger.enableTicker) Nil else Seq(countMsg),
+                verboseKeySuffix = verboseKeySuffix,
+                message = tickerPrefix,
+                noPrefix = exclusive
+              )
 
-            if (failFast && res.newResults.values.exists(_.result.asSuccess.isEmpty))
-              failed.set(true)
+              val res = evaluateGroupCached(
+                terminal = terminal,
+                group = sortedGroups.lookupKey(terminal),
+                results = upstreamResults,
+                countMsg = countMsg,
+                verboseKeySuffix = verboseKeySuffix,
+                zincProblemReporter = reporter,
+                testReporter = testReporter,
+                logger = contextLogger,
+                classToTransitiveClasses,
+                allTransitiveClassMethods,
+                forkExecutionContext,
+                exclusive
+              )
 
-            val endTime = System.nanoTime() / 1000
+              if (failFast && res.newResults.values.exists(_.result.asSuccess.isEmpty))
+                failed.set(true)
 
-            val duration = endTime - startTime
+              val endTime = System.nanoTime() / 1000
 
-            chromeProfileLogger.log(
-              task = Terminal.printTerm(terminal),
-              cat = "job",
-              startTime = startTime,
-              duration = duration,
-              threadId = threadNumberer.getThreadId(Thread.currentThread()),
-              cached = res.cached
-            )
+              val duration = endTime - startTime
 
-            profileLogger.log(
-              ProfileLogger.Timing(
-                terminal.render,
-                (duration / 1000).toInt,
-                res.cached,
-                deps.map(_.render),
-                res.inputsHash,
-                res.previousInputsHash
+              chromeProfileLogger.log(
+                task = Terminal.printTerm(terminal),
+                cat = "job",
+                startTime = startTime,
+                duration = duration,
+                threadId = threadNumberer.getThreadId(Thread.currentThread()),
+                cached = res.cached
+              )
+
+              profileLogger.log(
+                ProfileLogger.Timing(
+                  terminal.render,
+                  (duration / 1000).toInt,
+                  res.cached,
+                  deps.map(_.render),
+                  res.inputsHash,
+                  res.previousInputsHash
+                )
               )
-            )
 
-            Some(res)
+              Some(res)
+            }
           }
         }
       }