Introduce Task.Command(exclusive = true) and convert `Task.Persiste…

…nt` to `Task(persistent = true)` (#3617)

fixes #3566. Mostly a
straightforward implementation of what was discussed.

We use `Task.Command(exclusive = true)` to make
`console`/`repl`/`clean`/etc. run serially, all other commands should
run in parallel. That includes most `test` commands. The name
`exclusive` is taken from the Bazel action/target tag with the same
meaning: that the tagged action runs alone with no other actions in
parallel

`Task.Persistent` was changed to `Task(persistent = true)` for
consistency, and also for other reasons: the new syntax is more
composable, e.g. a user can more easily choose whether they want
`persistent = true` or `persistent = false` based on a computed
`Boolean` value, and Mill can in future extend it such that we can have
`Task(exclusive = true, persistent = true)`, `Task.Command(exclusive =
true, persistent = true)`, or other such combinations perhaps with even
more flags (e.g. Bazel has a pretty long list
https://bazel.build/reference/be/common-definitions#common.tags).

To make overload resolution work correctly, I make the first parameter
of the `(exclusive = true)` or `(persistent = true)` parameter list
`dummy: NamedParameterOnlyDummy.type = NamedParameterOnlyDummy`. This
ensures that there is no normal value the user could pass in
positionally that would select that overload of `def Command` or `def
apply`, and the only way to select it is by passing in `exclusive =
true` or `persistent = true` as a named parameter

Tested manually by adding `println`s and making sure that
`leafSerialCommands` no longer contains test tasks when I run tests in
`example/scalalib/basic/1-simple`.

contrib/playlib/src/mill/playlib/RouterModule.scala

@@ -60,7 +60,7 @@ trait RouterModule extends ScalaModule with Version {
 
   protected val routeCompilerWorker: RouteCompilerWorkerModule = RouteCompilerWorkerModule
 
-  def compileRouter: T[CompilationResult] = Task.Persistent {
+  def compileRouter: T[CompilationResult] = Task(persistent = true) {
     T.log.debug(s"compiling play routes with ${playVersion()} worker")
     routeCompilerWorker.routeCompilerWorker().compile(
       routerClasspath = playRouterToolsClasspath(),

contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala

@@ -126,7 +126,7 @@ trait ScalaPBModule extends ScalaModule {
     )
   }
 
-  def compileScalaPB: T[PathRef] = Task.Persistent {
+  def compileScalaPB: T[PathRef] = Task(persistent = true) {
     ScalaPBWorkerApi.scalaPBWorker()
       .compile(
         scalaPBClasspath(),

contrib/scoverage/src/mill/contrib/scoverage/ScoverageModule.scala

@@ -146,7 +146,7 @@ trait ScoverageModule extends ScalaModule { outer: ScalaModule =>
      * The persistent data dir used to store scoverage coverage data.
      * Use to store coverage data at compile-time and by the various report targets.
      */
-    def data: T[PathRef] = Task.Persistent {
+    def data: T[PathRef] = Task(persistent = true) {
       // via the persistent target, we ensure, the dest dir doesn't get cleared
       PathRef(T.dest)
     }

contrib/twirllib/src/mill/twirllib/TwirlModule.scala

@@ -88,7 +88,7 @@ trait TwirlModule extends mill.Module { twirlModule =>
 
   def twirlInclusiveDot: Boolean = false
 
-  def compileTwirl: T[mill.scalalib.api.CompilationResult] = Task.Persistent {
+  def compileTwirl: T[mill.scalalib.api.CompilationResult] = Task(persistent = true) {
     TwirlWorkerApi.twirlWorker
       .compile(
         twirlClasspath(),

example/depth/tasks/5-persistent-targets/build.mill

@@ -12,7 +12,7 @@ import java.util.zip.GZIPOutputStream
 
 def data = Task.Source(millSourcePath / "data")
 
-def compressedData = Task.Persistent{
+def compressedData = Task(persistent = true) {
   println("Evaluating compressedData")
   os.makeDir.all(Task.dest / "cache")
   os.remove.all(Task.dest / "compressed")

main/define/src/mill/define/NamedParameterOnlyDummy.scala

@@ -0,0 +1,6 @@
+package mill.define
+
+/**
+ * Dummy class used to mark parameters that come after it as named only parameters
+ */
+class NamedParameterOnlyDummy private[mill] ()

main/define/src/mill/define/Task.scala

@@ -45,22 +45,6 @@ abstract class Task[+T] extends Task.Ops[T] with Applyable[Task, T] {
 
 object Task extends TaskBase {
 
-  /**
-   * [[PersistentImpl]] are a flavor of [[TargetImpl]], normally defined using
-   * the `Task.Persistent{...}` syntax. The main difference is that while
-   * [[TargetImpl]] deletes the `T.dest` folder in between runs,
-   * [[PersistentImpl]] preserves it. This lets the user make use of files on
-   * disk that persistent between runs of the task, e.g. to implement their own
-   * fine-grained caching beyond what Mill provides by default.
-   *
-   * Note that the user defining a `Task.Persistent` task is taking on the
-   * responsibility of ensuring that their implementation is idempotent, i.e.
-   * that it computes the same result whether or not there is data in `T.dest`.
-   * Violating that invariant can result in confusing mis-behaviors
-   */
-  def Persistent[T](t: Result[T])(implicit rw: RW[T], ctx: mill.define.Ctx): Target[T] =
-    macro Target.Internal.persistentImpl[T]
-
   /**
    * A specialization of [[InputImpl]] defined via `Task.Sources`, [[SourcesImpl]]
    * uses [[PathRef]]s to compute a signature for a set of source files and
@@ -122,6 +106,18 @@ object Task extends TaskBase {
       cls: EnclosingClass
   ): Command[T] = macro Target.Internal.commandImpl[T]
 
+  def Command(
+      t: NamedParameterOnlyDummy = new NamedParameterOnlyDummy,
+      exclusive: Boolean = false
+  ): CommandFactory = new CommandFactory(exclusive)
+  class CommandFactory private[mill] (val exclusive: Boolean) extends TaskBase.TraverseCtxHolder {
+    def apply[T](t: Result[T])(implicit
+        w: W[T],
+        ctx: mill.define.Ctx,
+        cls: EnclosingClass
+    ): Command[T] = macro Target.Internal.serialCommandImpl[T]
+  }
+
   /**
    * [[Worker]] is a [[NamedTask]] that lives entirely in-memory, defined using
    * `Task.Worker{...}`. The value returned by `Task.Worker{...}` is long-lived,
@@ -160,6 +156,30 @@ object Task extends TaskBase {
   def apply[T](t: Result[T])(implicit rw: RW[T], ctx: mill.define.Ctx): Target[T] =
     macro Target.Internal.targetResultImpl[T]
 
+  /**
+   * Persistent tasks are defined using
+   * the `Task(persistent = true){...}` syntax. The main difference is that while
+   * [[TargetImpl]] deletes the `T.dest` folder in between runs,
+   * [[PersistentImpl]] preserves it. This lets the user make use of files on
+   * disk that persistent between runs of the task, e.g. to implement their own
+   * fine-grained caching beyond what Mill provides by default.
+   *
+   * Note that the user defining a `Task(persistent = true)` task is taking on the
+   * responsibility of ensuring that their implementation is idempotent, i.e.
+   * that it computes the same result whether or not there is data in `T.dest`.
+   * Violating that invariant can result in confusing mis-behaviors
+   */
+  def apply(
+      t: NamedParameterOnlyDummy = new NamedParameterOnlyDummy,
+      persistent: Boolean = false
+  ): ApplyFactory = new ApplyFactory(persistent)
+  class ApplyFactory private[mill] (val persistent: Boolean) extends TaskBase.TraverseCtxHolder {
+    def apply[T](t: Result[T])(implicit
+        rw: RW[T],
+        ctx: mill.define.Ctx
+    ): Target[T] = macro Target.Internal.persistentTargetResultImpl[T]
+  }
+
   abstract class Ops[+T] { this: Task[T] =>
     def map[V](f: T => V): Task[V] = new Task.Mapped(this, f)
     def filter(f: T => Boolean): Task[T] = this
@@ -238,7 +258,7 @@ trait NamedTask[+T] extends Task[T] {
 trait Target[+T] extends NamedTask[T]
 
 object Target extends TaskBase {
-  @deprecated("Use Task.Persistent instead", "Mill after 0.12.0-RC1")
+  @deprecated("Use Task(persistent = true){...} instead", "Mill after 0.12.0-RC1")
   def persistent[T](t: Result[T])(implicit rw: RW[T], ctx: mill.define.Ctx): Target[T] =
     macro Target.Internal.persistentImpl[T]
 
@@ -362,6 +382,28 @@ object Target extends TaskBase {
         )
       )
     }
+    def persistentTargetResultImpl[T: c.WeakTypeTag](c: Context)(t: c.Expr[Result[T]])(
+        rw: c.Expr[RW[T]],
+        ctx: c.Expr[mill.define.Ctx]
+    ): c.Expr[Target[T]] = {
+      import c.universe._
+
+      val taskIsPrivate = isPrivateTargetOption(c)
+
+      mill.moduledefs.Cacher.impl0[Target[T]](c)(
+        reify {
+          val s1 = Applicative.impl0[Task, T, mill.api.Ctx](c)(t.tree).splice
+          val c1 = ctx.splice
+          val r1 = rw.splice
+          val t1 = taskIsPrivate.splice
+          if (c.prefix.splice.asInstanceOf[Task.ApplyFactory].persistent) {
+            new PersistentImpl[T](s1, c1, r1, t1)
+          } else {
+            new TargetImpl[T](s1, c1, r1, t1)
+          }
+        }
+      )
+    }
 
     def targetTaskImpl[T: c.WeakTypeTag](c: Context)(t: c.Expr[Task[T]])(
         rw: c.Expr[RW[T]],
@@ -521,6 +563,27 @@ object Target extends TaskBase {
       )
     }
 
+    def serialCommandImpl[T: c.WeakTypeTag](c: Context)(t: c.Expr[T])(
+        w: c.Expr[W[T]],
+        ctx: c.Expr[mill.define.Ctx],
+        cls: c.Expr[EnclosingClass]
+    ): c.Expr[Command[T]] = {
+      import c.universe._
+
+      val taskIsPrivate = isPrivateTargetOption(c)
+
+      reify(
+        new Command[T](
+          Applicative.impl[Task, T, mill.api.Ctx](c)(t).splice,
+          ctx.splice,
+          w.splice,
+          cls.splice.value,
+          taskIsPrivate.splice,
+          exclusive = c.prefix.splice.asInstanceOf[Task.CommandFactory].exclusive
+        )
+      )
+    }
+
     def workerImpl1[T: c.WeakTypeTag](c: Context)(t: c.Expr[Task[T]])(ctx: c.Expr[mill.define.Ctx])
         : c.Expr[Worker[T]] = {
       import c.universe._
@@ -581,7 +644,8 @@ object Target extends TaskBase {
  * define the tasks, while methods like `T.`[[dest]], `T.`[[log]] or
  * `T.`[[env]] provide the core APIs that are provided to a task implementation
  */
-class TaskBase extends Applicative.Applyer[Task, Task, Result, mill.api.Ctx] {
+class TaskBase extends Applicative.Applyer[Task, Task, Result, mill.api.Ctx]
+    with TaskBase.TraverseCtxHolder {
 
   /**
    * `T.dest` is a unique `os.Path` (e.g. `out/classFiles.dest/` or `out/run.dest/`)
@@ -656,16 +720,20 @@ class TaskBase extends Applicative.Applyer[Task, Task, Result, mill.api.Ctx] {
   def traverse[T, V](source: Seq[T])(f: T => Task[V]): Task[Seq[V]] = {
     new Task.Sequence[V](source.map(f))
   }
+}
 
-  /**
-   * A variant of [[traverse]] that also provides the [[mill.api.Ctx]] to the
-   * function [[f]]
-   */
-  def traverseCtx[I, R](xs: Seq[Task[I]])(f: (IndexedSeq[I], mill.api.Ctx) => Result[R])
-      : Task[R] = {
-    new Task.TraverseCtx[I, R](xs, f)
-  }
+object TaskBase {
+  trait TraverseCtxHolder {
 
+    /**
+     * A variant of [[traverse]] that also provides the [[mill.api.Ctx]] to the
+     * function [[f]]
+     */
+    def traverseCtx[I, R](xs: Seq[Task[I]])(f: (IndexedSeq[I], mill.api.Ctx) => Result[R])
+        : Task[R] = {
+      new Task.TraverseCtx[I, R](xs, f)
+    }
+  }
 }
 
 class TargetImpl[+T](
@@ -693,8 +761,16 @@ class Command[+T](
     val ctx0: mill.define.Ctx,
     val writer: W[_],
     val cls: Class[_],
-    val isPrivate: Option[Boolean]
+    val isPrivate: Option[Boolean],
+    val exclusive: Boolean
 ) extends NamedTask[T] {
+  def this(
+      t: Task[T],
+      ctx0: mill.define.Ctx,
+      writer: W[_],
+      cls: Class[_],
+      isPrivate: Option[Boolean]
+  ) = this(t, ctx0, writer, cls, isPrivate, false)
   override def asCommand: Some[Command[T]] = Some(this)
   // FIXME: deprecated return type: Change to Option
   override def writerOpt: Some[W[_]] = Some(writer)

main/define/test/src/mill/define/MacroErrorTests.scala

@@ -69,7 +69,7 @@ object MacroErrorTests extends TestSuite {
       test("persistent") {
         val e = compileError("""
           object foo extends TestBaseModule{
-            def a() = Task.Persistent{1}
+            def a() = Task(persistent = true){1}
           }
           mill.define.Discover[foo.type]
         """)

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

@@ -180,8 +180,13 @@ private[mill] trait EvaluatorCore extends GroupEvaluator {
     val (_, tasksTransitive0) = Plan.plan(Agg.from(tasks0.map(_.task)))
 
     val tasksTransitive = tasksTransitive0.toSet
-    val (tasks, leafCommands) = terminals0.partition {
-      case Terminal.Labelled(t, _) if tasksTransitive.contains(t) => true
+    val (tasks, leafSerialCommands) = terminals0.partition {
+      case Terminal.Labelled(t, _) =>
+        if (tasksTransitive.contains(t)) true
+        else t match {
+          case t: Command[_] => !t.exclusive
+          case _ => false
+        }
       case _ => !serialCommandExec
     }
 
@@ -194,7 +199,7 @@ private[mill] trait EvaluatorCore extends GroupEvaluator {
       if (sys.env.contains(EnvVars.MILL_TEST_SUITE)) _ => ""
       else contextLoggerMsg0
     )(ec)
-    evaluateTerminals(leafCommands, _ => "")(ExecutionContexts.RunNow)
+    evaluateTerminals(leafSerialCommands, _ => "")(ExecutionContexts.RunNow)
 
     logger.clearAllTickers()
     val finishedOptsMap = terminals0

main/eval/src/mill/eval/Terminal.scala

@@ -5,7 +5,7 @@ import mill.define.{NamedTask, Segment, Segments}
 /**
  * A terminal or terminal target is some important work unit, that in most cases has a name (Right[Labelled])
  * or was directly called by the user (Left[Task]).
- * It's a T, Task.Worker, Task.Input, Task.Source, Task.Sources, Task.Persistent
+ * It's a Task, Task.Worker, Task.Input, Task.Source, Task.Sources, Task.Command
  */
 sealed trait Terminal {
   def render: String

main/eval/test/src/mill/eval/TaskTests.scala

@@ -70,7 +70,7 @@ trait TaskTests extends TestSuite {
     def taskInput = Task { input() }
     def taskNoInput = Task { task() }
 
-    def persistent = Task.Persistent {
+    def persistent = Task(persistent = true) {
       input() // force re-computation
       os.makeDir.all(T.dest)
       os.write.append(T.dest / "count", "hello\n")