Restrict sources and inputs to not accept upstream tasks (#4524)

Fixes #4121

This PR makes `Task.Source`, `Task.Sources`, and `Task.Input` to not
accept any upstream tasks. Sources and Inputs thus must be the
upstream-most nodes in our task graph: although they still contain
values computed during the `resolution` phase, they no longer can
contain values computed during `execution`. This is a significant
simplification of Mill's internal build graph data model, and I expect
it to allow a ton of internal code to be streamlined and simplifierd

In practice, following this restriction doesn't seem to be too onerous:

1. You can no longer `override def sources = Task.Sources` with another
`Task.Sources` and call `super.sources() ++ Seq(PathRef(...))`. Instead,
you need to define a separate `def customSources = Task.Sources{}` and
do `override def sources = Task { super.sources() ++ customSources() }`

2. You can no longer `override def sources = Task.Sources {
super.sources().flatMap{...} }`. Instead, you need to have a separate
`def sourcesFolders: Seq[os.SubPath]` that you can override and
manipulate, before it gets fed into `Task.Sources(sourcesFolders*)` once
at the end.

The various places where we were previously chaining `Task.Sources` or
`Task.Input`s together inside Mill's own codebase have been updated to
use the new pattern. This was a relatively straightforward and
mechanical process. Only `MillBuildRootModule#scriptSources` took a bit
more refactoring, which I took as an opportunity to drop the `$file`
syntax to allow us to separate `.mill` file discovery and processing
(which previously had to be interleaved due to parsing and
de-referencing `import $file` statements)

There's some inconvenience, but overall this way of doing things is a
lot more correct: e.g.

1. Previously, if you did `override def Task.Sources {
super.sources().map{...} }`, Mill's task graph would depend on
`super.sources()` even if the final paths that end up getting returned
to `Task.Sources` do not include the originals due to being transformed
somehow.

2. With this PR, you can manipulate `def sourcesFolders` all you want
during inheritance using overrides, and only the final `sourceFolders`
that get passed to `Task.Sources` ends up being part of the dependency
graph.

3. The current `--watch` behavior expects that the paths returned by
`Source` and `Sources` are constant, which is not strictly true. It is
possible that each time you call `Source`/`Sources` you get a different
set of paths, but the current `--watch` behavior does not account for
that possibility. With this PR, we formalize the existing assumption
that `Source`/`Sources` paths are fixed at module initialization time,
and only the content can change.

One scenario where we do lose something is `private def pullAndHash =
Task.Input {`. This depends on upstream tasks to determine what to pull,
and then acts as a `Task.Input`: that means every time you run `./mill
foo` that depends on `pullAndHash` it will re-run, and if you run
`./mill -w foo` it will run `pullAndHash` continuously in a loop. While
whether or not that is good idea for this specific scenario is
questionable (dockerhub may [rate limit
you](https://medium.com/@PlanB./docker-hubs-new-rate-limits-what-it-means-for-kubernetes-users-2748e9ff632e)!)
it does demonstrate a use case that would not be possible if we merge
this PR to restrict `Task.Input`s to not allow upstream tasks

In general you can still do computations to determine the paths to your
`Task.Source` and `Task.Sources` folders, just that these computations
must happen during the module-initialization/task-resolution phase
rather than during the execution phase

This simplification allows significant cleanups

1. `mill.api.TaskResult` is no longer necessary, since we can trivially
check the `os.Path`s of `Task.Sources` for updates and the `() => T` of
`Task.Input`s for updates without needing to awkwardly capture things in
a lambda that we can re-run later

2. `selective.run` no longer needs to run a full evaluation cycle in
order to collect the hashes of all the `Task.Input`s and `Task.Sources`,
since now you can easily collect those tasks and get each return value
individually

There are probably other follow ups I can't think of off the top of my
head that will become apparent later

bsp/worker/src/mill/bsp/worker/MillBuildServer.scala

@@ -8,7 +8,7 @@ import mill.api.{ColorLogger, CompileProblemReporter, DummyTestReporter, Result,
 import mill.bsp.{BspServerResult, Constants}
 import mill.bsp.worker.Utils.{makeBuildTarget, outputPaths, sanitizeUri}
 import mill.define.Segment.Label
-import mill.define.{Args, Discover, ExecutionResults, ExternalModule, NamedTask, Task, TaskResult}
+import mill.define.{Args, Discover, ExecutionResults, ExternalModule, NamedTask, Task}
 import mill.eval.Evaluator
 import mill.main.MainModule
 import mill.runner.MillBuildRootModule
@@ -476,7 +476,7 @@ private class MillBuildServer(
         logger = new MillBspLogger(client, runTask.hashCode(), ev.baseLogger)
       )
       val response = runResult.results(runTask) match {
-        case r if r.result.asSuccess.isDefined => new RunResult(StatusCode.OK)
+        case r if r.asSuccess.isDefined => new RunResult(StatusCode.OK)
         case _ => new RunResult(StatusCode.ERROR)
       }
       params.getOriginId match {
@@ -603,9 +603,9 @@ private class MillBuildServer(
             if (cleanResult.failing.size > 0) (
               msg + s" Target ${compileTargetName} could not be cleaned. See message from mill: \n" +
                 (cleanResult.results(cleanTask) match {
-                  case TaskResult(ex: ExecResult.Exception, _) => ex.toString()
-                  case TaskResult(ExecResult.Skipped, _) => "Task was skipped"
-                  case TaskResult(ExecResult.Aborted, _) => "Task was aborted"
+                  case ex: ExecResult.Exception => ex.toString()
+                  case ExecResult.Skipped => "Task was skipped"
+                  case ExecResult.Aborted => "Task was aborted"
                   case _ => "could not retrieve the failure message"
                 }),
               false
@@ -672,7 +672,7 @@ private class MillBuildServer(
         .map { case ((ev, id), ts) =>
           val results = evaluate(ev, ts)
           val failures = results.results.collect {
-            case (_, TaskResult(res: ExecResult.Failing[_], _)) => res
+            case (_, res: ExecResult.Failing[_]) => res
           }
 
           def logError(errorMsg: String): Unit = {

bsp/worker/src/mill/bsp/worker/Utils.scala

@@ -110,7 +110,7 @@ private object Utils {
       results: ExecutionResults,
       task: mill.define.Task[?]
   ): StatusCode = {
-    results.results(task).result match {
+    results.results(task) match {
       case Success(_) => StatusCode.OK
       case Skipped => StatusCode.CANCELLED
       case _ => StatusCode.ERROR

ci/mill-bootstrap.patch

@@ -1,5 +1,5 @@
 diff --git a/build.mill b/build.mill
-index ce02c1bc1a0..df2271c63a5 100644
+index 36bf4e9a593..84fd76e6645 100644
 --- a/build.mill
 +++ b/build.mill
 @@ -1,16 +1,16 @@
@@ -47,16 +47,16 @@ index ce02c1bc1a0..df2271c63a5 100644
  }
 
  def millDownloadPrefix = Task {
-@@ -320,7 +320,7 @@ def millBinPlatform: T[String] = Task {
-   "0.11"
+@@ -319,7 +319,7 @@ def millBinPlatform: T[String] = Task {
+   }
  }
 
 -def baseDir = build.millSourcePath
 +def baseDir = build.moduleDir
 
  val essentialBridgeScalaVersions =
    Seq(Deps.scalaVersion, Deps.scala2Version, Deps.workerScalaVersion212)
-@@ -468,7 +468,7 @@ trait MillPublishJavaModule extends MillJavaModule with PublishModule {
+@@ -467,7 +467,7 @@ trait MillPublishJavaModule extends MillJavaModule with PublishModule {
  /**
   * Some custom scala settings and test convenience
   */
@@ -65,7 +65,7 @@ index ce02c1bc1a0..df2271c63a5 100644
    def scalaVersion = Deps.scalaVersion
    def scalapVersion: T[String] = Deps.scala2Version
    def scalafixScalaBinaryVersion = T {
-@@ -525,8 +525,8 @@ trait MillScalaModule extends ScalaModule with MillJavaModule with ScalafixModul
+@@ -524,8 +524,8 @@ trait MillScalaModule extends ScalaModule with MillJavaModule with ScalafixModul
      val binaryVersion = ZincWorkerUtil.scalaBinaryVersion(sv)
      val hasModuleDefs = binaryVersion == "2.13" || binaryVersion == "3"
      super.scalacPluginIvyDeps() ++
@@ -76,7 +76,7 @@ index ce02c1bc1a0..df2271c63a5 100644
    }
 
    def mandatoryIvyDeps = T {
-@@ -534,13 +534,13 @@ trait MillScalaModule extends ScalaModule with MillJavaModule with ScalafixModul
+@@ -533,13 +533,13 @@ trait MillScalaModule extends ScalaModule with MillJavaModule with ScalafixModul
      val binaryVersion = ZincWorkerUtil.scalaBinaryVersion(sv)
      val hasModuleDefs = binaryVersion == "2.13" || binaryVersion == "3"
      super.mandatoryIvyDeps() ++
@@ -92,7 +92,7 @@ index ce02c1bc1a0..df2271c63a5 100644
      def scalafixConfig = T { Some(T.workspace / ".scalafix.conf") }
      def forkArgs = super.forkArgs() ++ outer.testArgs()
      def moduleDeps = outer.testModuleDeps
-@@ -580,7 +580,8 @@ trait MillBaseTestsModule extends TestModule {
+@@ -579,7 +579,8 @@ trait MillBaseTestsModule extends TestModule {
  trait MillPublishScalaModule extends MillScalaModule with MillPublishJavaModule
 
  /** Publishable module which contains strictly handled API. */
@@ -102,7 +102,7 @@ index ce02c1bc1a0..df2271c63a5 100644
    import com.github.lolgab.mill.mima._
    override def mimaBinaryIssueFilters: T[Seq[ProblemFilter]] = Seq(
      // (5x) MIMA doesn't properly ignore things which are nested inside other private things
-@@ -710,7 +711,7 @@ trait MillStableScalaModule extends MillPublishScalaModule with Mima {
+@@ -709,7 +710,7 @@ trait MillStableScalaModule extends MillPublishScalaModule with Mima {
    def skipPreviousVersions: T[Seq[String]] = T {
      T.log.info("Skipping mima for previous versions (!!1000s of errors due to Scala 3)")
      mimaPreviousVersions() // T(Seq.empty[String])
@@ -112,7 +112,7 @@ index ce02c1bc1a0..df2271c63a5 100644
 
  object bridge extends Cross[BridgeModule](compilerBridgeScalaVersions)
 diff --git a/contrib/package.mill b/contrib/package.mill
-index 421b2955955..30c453234b9 100644
+index 78dac75644b..fa53e9c7525 100644
 --- a/contrib/package.mill
 +++ b/contrib/package.mill
 @@ -3,13 +3,12 @@ package build.contrib
@@ -143,23 +143,19 @@ index 421b2955955..30c453234b9 100644
    }
 
    object testng extends JavaModule with ContribModule {
-@@ -85,13 +84,13 @@ object `package` extends RootModule {
+@@ -85,9 +84,9 @@ object `package` extends RootModule {
      object worker extends Cross[WorkerModule](build.Deps.play.keys.toSeq)
      trait WorkerModule extends build.MillPublishScalaModule with Cross.Module[String] {
        def playBinary = crossValue
 -      def millSourcePath: os.Path = super.millSourcePath / playBinary
 +      def moduleDir: os.Path = super.moduleDir / playBinary
 
-       def sources = Task.Sources {
+-      def sharedSources = Task.Sources(millSourcePath / os.up / "src-shared")
++      def sharedSources = Task.Sources(moduleDir / os.up / "src-shared")
+       def sources = Task {
          // We want to avoid duplicating code as long as the Play APIs allow.
          // But if newer Play versions introduce incompatibilities,
-         // just remove the shared source dir for that worker and implement directly.
--        Seq(PathRef(millSourcePath / os.up / "src-shared")) ++ super.sources()
-+        Seq(PathRef(moduleDir / os.up / "src-shared")) ++ super.sources()
-       }
-
-       def scalaVersion = build.Deps.play(playBinary).scalaVersion
-@@ -139,7 +138,7 @@ object `package` extends RootModule {
+@@ -140,7 +139,7 @@ object `package` extends RootModule {
            build.Deps.scalacScoverage2Reporter,
            build.Deps.scalacScoverage2Domain,
            build.Deps.scalacScoverage2Serializer
@@ -551,10 +547,10 @@ index af00b40d3f0..e43343cabf1 100644
      override def moduleDeps = super[IntegrationTestModule].moduleDeps
      def forkEnv = super.forkEnv() ++ Seq(
 diff --git a/main/package.mill b/main/package.mill
-index 24a6babcdb7..1887035b5a8 100644
+index 29e5ad2d931..3b65ea7a78c 100644
 --- a/main/package.mill
 +++ b/main/package.mill
-@@ -82,7 +82,7 @@ object `package` extends RootModule with build.MillStableScalaModule with BuildI
+@@ -80,7 +80,7 @@ object `package` extends RootModule with build.MillStableScalaModule with BuildI
            Some(Task.ctx()),
            dist.coursierCacheCustomizer()
          )

contrib/bloop/src/mill/contrib/bloop/BloopImpl.scala

@@ -125,7 +125,7 @@ class BloopImpl(evs: () => Seq[Evaluator], wd: os.Path) extends ExternalModule {
    * that does not get invalidated upon source file change. Mainly called
    * from module#sources in bloopInstall
    */
-  def moduleSourceMap = Task.Input {
+  def moduleSourceMap = Task {
     val sources = Task.traverse(computeModules) { m =>
       m.allSources.map { paths =>
         name(m) -> paths.map(_.path)

contrib/docker/src/mill/contrib/docker/DockerModule.scala

@@ -147,7 +147,7 @@ trait DockerModule { outer: JavaModule =>
          |ENTRYPOINT [$quotedEntryPointArgs]""".stripMargin
     }
 
-    private def pullAndHash = Task.Input {
+    private def pullAndHash = Task {
       val env = dockerEnv()
       def imageHash() =
         os.proc(executable(), "images", "--no-trunc", "--quiet", baseImage())

contrib/package.mill

@@ -87,11 +87,12 @@ object `package` extends RootModule {
       def playBinary = crossValue
       def millSourcePath: os.Path = super.millSourcePath / playBinary
 
-      def sources = Task.Sources {
+      def sharedSources = Task.Sources(millSourcePath / os.up / "src-shared")
+      def sources = Task {
         // We want to avoid duplicating code as long as the Play APIs allow.
         // But if newer Play versions introduce incompatibilities,
         // just remove the shared source dir for that worker and implement directly.
-        Seq(PathRef(millSourcePath / os.up / "src-shared")) ++ super.sources()
+        sharedSources() ++ super.sources()
       }
 
       def scalaVersion = build.Deps.play(playBinary).scalaVersion

contrib/playlib/src/mill/playlib/Static.scala

@@ -13,7 +13,7 @@ trait Static extends ScalaModule {
   /**
    * project resources including configuration, webjars and static assets
    */
-  override def resources = Task.Sources {
+  override def resources = Task {
     super.resources() :+ webJarResources() :+ staticAssets()
   }
 

contrib/proguard/src/mill/contrib/proguard/Proguard.scala

@@ -64,7 +64,7 @@ trait Proguard extends ScalaModule {
    * Keep in sync with [[javaHome]].
    */
   def java9RtJar: T[Seq[PathRef]] = Task {
-    if (Util.isJava9OrAbove) Seq(PathRef(Task.home / Export.rtJarName))
+    if (Util.isJava9OrAbove) Seq(PathRef(os.home / Export.rtJarName))
     else Seq()
   }
 

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

@@ -83,7 +83,7 @@ trait ScalaPBModule extends ScalaModule {
     )
   }
 
-  def scalaPBIncludePath: T[Seq[PathRef]] = Task.Sources { Seq.empty[PathRef] }
+  def scalaPBIncludePath: T[Seq[PathRef]] = Task.Sources()
 
   private def scalaDepsPBIncludePath: Task[Seq[PathRef]] = scalaPBSearchDeps match {
     case true => Task.Anon { Seq(scalaPBUnpackProto()) }

core/api/src/mill/api/Ctx.scala

@@ -52,14 +52,6 @@ object Ctx {
     implicit def logToCtx(l: Logger): Log = new Log { def log = l }
   }
 
-  /**
-   * Access to some internal storage dir used by underlying ammonite.
-   * You should not need this in a buildscript.
-   */
-  trait Home {
-    def home: os.Path
-  }
-
   /** Access to the current system environment settings. */
   trait Env {
 
@@ -171,7 +163,6 @@ class Ctx(
     val args: IndexedSeq[?],
     dest0: () => os.Path,
     val log: Logger,
-    val home: os.Path,
     val env: Map[String, String],
     val reporter: Int => Option[CompileProblemReporter],
     val testReporter: TestReporter,
@@ -181,22 +172,9 @@ class Ctx(
 ) extends Ctx.Dest
     with Ctx.Log
     with Ctx.Args
-    with Ctx.Home
     with Ctx.Env
     with Ctx.Workspace {
 
-  def this(
-      args: IndexedSeq[?],
-      dest0: () => os.Path,
-      log: Logger,
-      home: os.Path,
-      env: Map[String, String],
-      reporter: Int => Option[CompileProblemReporter],
-      testReporter: TestReporter,
-      workspace: os.Path
-  ) = {
-    this(args, dest0, log, home, env, reporter, testReporter, workspace, _ => ???, null)
-  }
   def dest: os.Path = dest0()
   def arg[T](index: Int): T = {
     if (index >= 0 && index < args.length) args(index).asInstanceOf[T]

core/define/src/mill/define/ExecResults.scala

@@ -6,6 +6,6 @@ trait ExecutionResults {
   def rawValues: Seq[ExecResult[Val]]
   def evaluated: Seq[Task[?]]
   def failing: Map[Task[?], Seq[ExecResult.Failing[Val]]]
-  def results: Map[Task[?], TaskResult[Val]]
+  def results: Map[Task[?], ExecResult[Val]]
   def values: Seq[Val] = rawValues.collect { case ExecResult.Success(v) => v }
 }