Make `T.worker`s compute their signature based on their inputs, rather than the hash of their output, to allow for returning functions/classloaders/subprocesses/etc. which do not hash reliably between process restarts

2 files changed, 28 insertions, 19 deletions

diff --git a/core/src/mill/eval/Evaluator.scala b/core/src/mill/eval/Evaluator.scala
index 0e8709e0..880f496d 100644
--- a/core/src/mill/eval/Evaluator.scala
+++ b/core/src/mill/eval/Evaluator.scala
@@ -58,7 +58,7 @@ class Evaluator[T](val workspacePath: Path,
     }
 
     val evaluated = new Agg.Mutable[Task[_]]
-    val results = mutable.LinkedHashMap.empty[Task[_], Result[Any]]
+    val results = mutable.LinkedHashMap.empty[Task[_], (Result[Any], Int)]
 
     for (((terminal, group), i) <- sortedGroups.items().zipWithIndex){
       // Increment the counter message by 1 to go from 1/10 to 10/10 instead of 0/10 to 9/10
@@ -73,28 +73,28 @@ class Evaluator[T](val workspacePath: Path,
 
     val failing = new util.MultiBiMap.Mutable[Either[Task[_], Labelled[_]], Result.Failing]
     for((k, vs) <- sortedGroups.items()){
-      failing.addAll(k, vs.items.flatMap(results.get).collect{case f: Result.Failing => f})
+      failing.addAll(k, vs.items.flatMap(results.get).collect{case (f: Result.Failing, _) => f})
     }
     Evaluator.Results(
-      goals.indexed.map(results),
+      goals.indexed.map(results(_)._1),
       evaluated,
       transitive,
       failing,
-      results
+      results.map{case (k, (v, hash)) => (k, v)}
     )
   }
 
 
   def evaluateGroupCached(terminal: Either[Task[_], Labelled[_]],
                           group: Agg[Task[_]],
-                          results: collection.Map[Task[_], Result[Any]],
-                          counterMsg: String): (collection.Map[Task[_], Result[Any]], Seq[Task[_]]) = {
+                          results: collection.Map[Task[_], (Result[Any], Int)],
+                          counterMsg: String): (collection.Map[Task[_], (Result[Any], Int)], Seq[Task[_]]) = {
 
 
     val externalInputs = group.items.flatMap(_.inputs).filter(!group.contains(_))
 
     val inputsHash =
-      externalInputs.map(results).toVector.hashCode +
+      externalInputs.map(results(_)._2).toVector.hashCode +
       group.toIterator.map(_.sideHash).toVector.hashCode() +
       classLoaderSig.hashCode()
 
@@ -103,6 +103,7 @@ class Evaluator[T](val workspacePath: Path,
         evaluateGroup(
           group,
           results,
+          inputsHash,
           groupBasePath = None,
           paths = None,
           maybeTargetLabel = None,
@@ -126,15 +127,15 @@ class Evaluator[T](val workspacePath: Path,
 
         (workerCached, cached) match{
           case (Some(workerValue), _) =>
-            val newResults = mutable.LinkedHashMap.empty[Task[_], Result[Any]]
-            newResults(labelledNamedTask.target) = {
-              Result.Success(workerValue._2)
-            }
+            val newResults = mutable.LinkedHashMap.empty[Task[_], (Result[Any], Int)]
+            newResults(labelledNamedTask.target) =
+              (Result.Success(workerValue._2), inputsHash)
+
             (newResults, Nil)
 
           case (_, Some(parsed)) =>
-            val newResults = mutable.LinkedHashMap.empty[Task[_], Result[Any]]
-            newResults(labelledNamedTask.target) = parsed
+            val newResults = mutable.LinkedHashMap.empty[Task[_], (Result[Any], Int)]
+            newResults(labelledNamedTask.target) = (parsed, parsed.hashCode)
             (newResults, Nil)
 
           case _ =>
@@ -149,6 +150,7 @@ class Evaluator[T](val workspacePath: Path,
             val (newResults, newEvaluated) = evaluateGroup(
               group,
               results,
+              inputsHash,
               groupBasePath = Some(groupBasePath),
               paths = Some(paths),
               maybeTargetLabel = Some(msgParts.mkString),
@@ -156,7 +158,7 @@ class Evaluator[T](val workspacePath: Path,
             )
 
             newResults(labelledNamedTask.target) match{
-              case Result.Success(v) =>
+              case (Result.Success(v), _) =>
                 labelledNamedTask.target.asWorker match{
                   case Some(w) =>
                     workerCache(w.ctx.segments) = (inputsHash, v)
@@ -187,7 +189,8 @@ class Evaluator[T](val workspacePath: Path,
 
 
   def evaluateGroup(group: Agg[Task[_]],
-                    results: collection.Map[Task[_], Result[Any]],
+                    results: collection.Map[Task[_], (Result[Any], Int)],
+                    inputsHash: Int,
                     groupBasePath: Option[Path],
                     paths: Option[Evaluator.Paths],
                     maybeTargetLabel: Option[String],
@@ -195,7 +198,7 @@ class Evaluator[T](val workspacePath: Path,
 
 
     val newEvaluated = mutable.Buffer.empty[Task[_]]
-    val newResults = mutable.LinkedHashMap.empty[Task[_], Result[Any]]
+    val newResults = mutable.LinkedHashMap.empty[Task[_], (Result[Any], Int)]
 
     val nonEvaluatedTargets = group.indexed.filterNot(results.contains)
 
@@ -205,7 +208,7 @@ class Evaluator[T](val workspacePath: Path,
         item <- target.inputs.filterNot(group.contains)
       } yield results(item)
 
-      val logRun = inputResults.forall(_.isInstanceOf[Result.Success[_]])
+      val logRun = inputResults.forall(_._1.isInstanceOf[Result.Success[_]])
 
       if(logRun) { log.ticker(s"[$counterMsg] $targetLabel ") }
     }
@@ -217,7 +220,7 @@ class Evaluator[T](val workspacePath: Path,
       newEvaluated.append(target)
       val targetInputValues = target.inputs
         .map(x => newResults.getOrElse(x, results(x)))
-        .collect{ case Result.Success(v) => v }
+        .collect{ case (Result.Success(v), _) => v }
 
       val res =
         if (targetInputValues.length != target.inputs.length) Result.Skipped
@@ -251,7 +254,11 @@ class Evaluator[T](val workspacePath: Path,
           }
         }
 
-      newResults(target) = res
+      newResults(target) = (
+        res,
+        if (target.isInstanceOf[Worker[_]]) inputsHash
+        else res match{ case Result.Success(v) => v.hashCode() case _ => 0}
+      )
     }
 
     multiLogger.close()
diff --git a/core/test/src/mill/eval/TaskTests.scala b/core/test/src/mill/eval/TaskTests.scala
index 3c355fa4..3f265618 100644
--- a/core/test/src/mill/eval/TaskTests.scala
+++ b/core/test/src/mill/eval/TaskTests.scala
@@ -8,6 +8,8 @@ object TaskTests extends TestSuite{
   val tests = Tests{
     object build extends mill.util.TestUtil.BaseModule{
       var count = 0
+      // Explicitly instantiate `Function1` objects to make sure we get
+      // different instances each time
       def staticWorker = T.worker{
         new Function1[Int, Int] {
           def apply(v1: Int) = v1 + 1