Merge pull request #7539 from adpi2/sbt2-check-cached-file

[2.x] Report error if output file of a cached task is not in the output directory

.github/workflows/ci.yml

@@ -40,7 +40,7 @@ jobs:
             java: 8
             distribution: adopt
             jobtype: 7
-          - os: macos-latest
+          - os: macos-12
             java: 8
             distribution: adopt
             jobtype: 8

core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala

@@ -32,11 +32,8 @@ trait Cont:
     def contMapN[A: Type, F[_], Effect[_]: Type](
         tree: Expr[A],
         applicativeExpr: Expr[Applicative[F]],
-        cacheConfigExpr: Option[Expr[BuildWideCacheConfiguration]],
+        cacheConfigExpr: Option[Expr[BuildWideCacheConfiguration]]
-    )(using
+    )(using iftpe: Type[F], eatpe: Type[Effect[A]]): Expr[F[Effect[A]]] =
-        iftpe: Type[F],
-        eatpe: Type[Effect[A]],
-    ): Expr[F[Effect[A]]] =
       contMapN[A, F, Effect](tree, applicativeExpr, cacheConfigExpr, conv.idTransform)
 
     /**
@@ -176,29 +173,12 @@ trait Cont:
       val inputBuf = ListBuffer[Input]()
       val outputBuf = ListBuffer[Output]()
 
-      def makeApp(body: Term, inputs: List[Input]): Expr[F[Effect[A]]] = inputs match
-        case Nil      => pure(body)
-        case x :: Nil => genMap(body, x)
-        case xs       => genMapN(body, xs)
       def unitExpr: Expr[Unit] = '{ () }
 
       // no inputs, so construct F[A] via Instance.pure or pure+flatten
       def pure(body: Term): Expr[F[Effect[A]]] =
         val tags = CacheLevelTag.all.toList
         def pure0[A1: Type](body: Expr[A1]): Expr[F[A1]] =
-          cacheConfigExprOpt match
-            case Some(cacheConfigExpr) =>
-              '{
-                $applicativeExpr.pure[A1] { () =>
-                  ${
-                    callActionCache[A1, Unit](outputBuf.toList, cacheConfigExpr, tags)(
-                      body = body,
-                      input = unitExpr,
-                    )
-                  }
-                }
-              }
-            case None =>
           '{
             $applicativeExpr.pure[A1] { () => $body }
           }
@@ -236,27 +216,20 @@ trait Cont:
                   val substitute = [x] =>
                     (name: String, tpe: Type[x], qual: Term, replace: Term) =>
                       given t: Type[x] = tpe
-                      convert[x](name, qual) transform { (tree: Term) =>
+                      convert[x](name, qual).transform { _ => typed[x](Ref(param.symbol)) }
-                        typed[x](Ref(param.symbol))
-                    }
                   val modifiedBody =
                     transformWrappers(body.asTerm.changeOwner(sym), substitute, sym).asExprOf[A1]
                   cacheConfigExprOpt match
                     case Some(cacheConfigExpr) =>
-                      if input.isCacheInput then
+                      val modifiedCacheConfigExpr =
-                        callActionCache(outputBuf.toList, cacheConfigExpr, input.tags)(
+                        transformWrappers(cacheConfigExpr.asTerm.changeOwner(sym), substitute, sym)
-                          body = modifiedBody,
+                          .asExprOf[BuildWideCacheConfiguration]
-                          input = Ref(param.symbol).asExprOf[a],
+                      val tags = CacheLevelTag.all.toList
-                        ).asTerm.changeOwner(sym)
+                      callActionCache(outputBuf.toList, modifiedCacheConfigExpr, tags)(
-                      else
-                        callActionCache[A1, Unit](
-                          outputBuf.toList,
-                          cacheConfigExpr,
-                          input.tags,
-                        )(
                         body = modifiedBody,
                         input = unitExpr,
-                        ).asTerm.changeOwner(sym)
+                      ).asTerm
+                        .changeOwner(sym)
                     case None => modifiedBody.asTerm
                 }
               ).asExprOf[a => A1]
@@ -300,6 +273,9 @@ trait Cont:
                     transformWrappers(body.asTerm.changeOwner(sym), substitute, sym).asExprOf[A1]
                   cacheConfigExprOpt match
                     case Some(cacheConfigExpr) =>
+                      val modifiedCacheConfigExpr =
+                        transformWrappers(cacheConfigExpr.asTerm.changeOwner(sym), substitute, sym)
+                          .asExprOf[BuildWideCacheConfiguration]
                       if inputs.exists(_.isCacheInput) then
                         val tags = inputs
                           .filter(_.isCacheInput)
@@ -312,13 +288,13 @@ trait Cont:
                         )
                         br.cacheInputTupleTypeRepr.asType match
                           case '[cacheInputTpe] =>
-                            callActionCache(outputBuf.toList, cacheConfigExpr, tags)(
+                            callActionCache(outputBuf.toList, modifiedCacheConfigExpr, tags)(
                               body = modifiedBody,
                               input = br.cacheInputExpr(p0).asExprOf[cacheInputTpe],
                             ).asTerm.changeOwner(sym)
                       else
                         val tags = CacheLevelTag.all.toList
-                        callActionCache[A1, Unit](outputBuf.toList, cacheConfigExpr, tags)(
+                        callActionCache(outputBuf.toList, cacheConfigExpr, tags)(
                           body = modifiedBody,
                           input = unitExpr,
                         ).asTerm.changeOwner(sym)
@@ -409,7 +385,11 @@ trait Cont:
               else oldTree
             end if
         }
-      val tx = transformWrappers(expr.asTerm, record, Symbol.spliceOwner)
+      val exprWithConfig =
-      val tr = makeApp(tx, inputBuf.toList)
+        cacheConfigExprOpt.map(config => '{ $config; $expr }).getOrElse(expr)
-      tr
+      val body = transformWrappers(exprWithConfig.asTerm, record, Symbol.spliceOwner)
+      inputBuf.toList match
+        case Nil      => pure(body)
+        case x :: Nil => genMap(body, x)
+        case xs       => genMapN(body, xs)
 end Cont

core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala

@@ -71,7 +71,6 @@ trait ContextUtil[C <: Quotes & scala.Singleton](val valStart: Int):
   ):
     override def toString: String =
       s"Input($tpe, $qual, $term, $name, $tags)"
-
     def isCacheInput: Boolean = tags.nonEmpty
     lazy val tags = extractTags(qual)
     private def extractTags(tree: Term): List[CacheLevelTag] =

main-command/src/main/scala/sbt/BasicKeys.scala

@@ -17,7 +17,7 @@ import sbt.librarymanagement.ModuleID
 import sbt.util.{ ActionCacheStore, Level }
 import scala.annotation.nowarn
 import scala.concurrent.duration.FiniteDuration
-import xsbti.VirtualFile
+import xsbti.{ FileConverter, VirtualFile }
 
 object BasicKeys {
   val historyPath = AttributeKey[Option[File]](
@@ -119,6 +119,12 @@ object BasicKeys {
       10000
     )
 
+  val fileConverter = AttributeKey[FileConverter](
+    "fileConverter",
+    "The file converter used to convert between Path and VirtualFile",
+    10000
+  )
+
   // Unlike other BasicKeys, this is not used directly as a setting key,
   // and severLog / logLevel is used instead.
   private[sbt] val serverLogLevel =

main-settings/src/main/scala/sbt/Def.scala

@@ -7,7 +7,6 @@
 
 package sbt
 
-import java.nio.file.Path
 import java.net.URI
 
 import scala.annotation.tailrec
@@ -17,7 +16,7 @@ import sbt.Scope.{ GlobalScope, ThisScope }
 import sbt.internal.util.Types.const
 import sbt.internal.util.complete.Parser
 import sbt.internal.util.{ Terminal => ITerminal, * }
-import sbt.util.{ ActionCacheStore, BuildWideCacheConfiguration, InMemoryActionCacheStore }
+import sbt.util.{ ActionCacheStore, AggregateActionCacheStore, BuildWideCacheConfiguration, cacheLevel , DiskActionCacheStore }
 import Util._
 import sbt.util.Show
 import xsbti.{ HashedVirtualFileRef, VirtualFile }
@@ -229,17 +228,40 @@ object Def extends Init[Scope] with TaskMacroExtra with InitializeImplicits:
 
   import language.experimental.macros
 
+  private[sbt] val isDummyTask = AttributeKey[Boolean](
+    "is-dummy-task",
+    "Internal: used to identify dummy tasks. sbt injects values for these tasks at the start of task execution.",
+    Invisible
+  )
+
+  private[sbt] val (stateKey: TaskKey[State], dummyState: Task[State]) =
+    dummy[State]("state", "Current build state.")
+
+  private[sbt] val (streamsManagerKey, dummyStreamsManager) =
+    Def.dummy[std.Streams[ScopedKey[?]]](
+      "streams-manager",
+      "Streams manager, which provides streams for different contexts."
+    )
+
   // These are here, as opposed to RemoteCache, since we need them from TaskMacro etc
-  private[sbt] var _cacheStore: ActionCacheStore = InMemoryActionCacheStore()
-  def cacheStore: ActionCacheStore = _cacheStore
-  private[sbt] var _outputDirectory: Option[Path] = None
   private[sbt] val cacheEventLog: CacheEventLog = CacheEventLog()
-  def cacheConfiguration: BuildWideCacheConfiguration =
+  @cacheLevel(include = Array.empty)
+  val cacheConfiguration: Initialize[Task[BuildWideCacheConfiguration]] = Def.task {
+    val state = stateKey.value
+    val outputDirectory = state.get(BasicKeys.rootOutputDirectory)
+    val cacheStore = state
+      .get(BasicKeys.cacheStores)
+      .collect { case xs if xs.nonEmpty => AggregateActionCacheStore(xs) }
+      .getOrElse(DiskActionCacheStore(state.baseDir.toPath.resolve("target/bootcache")))
+    val fileConverter = state.get(BasicKeys.fileConverter)
     BuildWideCacheConfiguration(
-      _cacheStore,
+      cacheStore,
-      _outputDirectory.getOrElse(sys.error("outputDirectory has not been set")),
+      outputDirectory.getOrElse(sys.error("outputDirectory has not been set")),
+      fileConverter.getOrElse(sys.error("outputDirectory has not been set")),
+      state.log,
       cacheEventLog,
     )
+  }
 
   inline def cachedTask[A1: JsonFormat](inline a1: A1): Def.Initialize[Task[A1]] =
     ${ TaskMacro.taskMacroImpl[A1]('a1, cached = true) }
@@ -401,9 +423,9 @@ object Def extends Init[Scope] with TaskMacroExtra with InitializeImplicits:
     (TaskKey[A](name, description, DTask), dummyTask(name))
 
   private[sbt] def dummyTask[T](name: String): Task[T] = {
-    import std.TaskExtra.{ task => newTask, toTaskInfo }
+    import TaskExtra.toTaskInfo
-    val base: Task[T] = newTask(
+    val base: Task[T] = TaskExtra.task(
-      sys.error("Dummy task '" + name + "' did not get converted to a full task.")
+      sys.error(s"Dummy task '$name' did not get converted to a full task.")
     )
       .named(name)
     base.copy(info = base.info.set(isDummyTask, true))
@@ -411,21 +433,6 @@ object Def extends Init[Scope] with TaskMacroExtra with InitializeImplicits:
 
   private[sbt] def isDummy(t: Task[_]): Boolean =
     t.info.attributes.get(isDummyTask) getOrElse false
-
-  private[sbt] val isDummyTask = AttributeKey[Boolean](
-    "is-dummy-task",
-    "Internal: used to identify dummy tasks.  sbt injects values for these tasks at the start of task execution.",
-    Invisible
-  )
-
-  private[sbt] val (stateKey: TaskKey[State], dummyState: Task[State]) =
-    dummy[State]("state", "Current build state.")
-
-  private[sbt] val (streamsManagerKey, dummyStreamsManager) =
-    Def.dummy[std.Streams[ScopedKey[?]]](
-      "streams-manager",
-      "Streams manager, which provides streams for different contexts."
-    )
 end Def
 
 // these need to be mixed into the sbt package object

main-settings/src/main/scala/sbt/std/TaskMacro.scala

@@ -23,6 +23,7 @@ import sbt.internal.util.{ LinePosition, NoPosition, SourcePosition }
 import language.experimental.macros
 import scala.quoted.*
 import sjsonnew.JsonFormat
+import sbt.util.BuildWideCacheConfiguration
 
 object TaskMacro:
   final val AssignInitName = "set"
@@ -55,10 +56,17 @@ object TaskMacro:
       case '{ if ($cond) then $thenp else $elsep } => taskIfImpl[A1](t, cached)
       case _ =>
         val convert1 = new FullConvert(qctx, 0)
-        val cacheConfigExpr =
+        if cached then
-          if cached then Some('{ Def.cacheConfiguration })
+          convert1.contMapN[A1, F, Id](
-          else None
+            t,
-        convert1.contMapN[A1, F, Id](t, convert1.appExpr, cacheConfigExpr)
+            convert1.appExpr,
+            Some('{
+              InputWrapper.`wrapInitTask_\u2603\u2603`[BuildWideCacheConfiguration](
+                Def.cacheConfiguration
+              )
+            })
+          )
+        else convert1.contMapN[A1, F, Id](t, convert1.appExpr, None)
 
   def taskIfImpl[A1: Type](expr: Expr[A1], cached: Boolean)(using
       qctx: Quotes

main/src/main/scala/sbt/Keys.scala

@@ -37,7 +37,7 @@ import sbt.librarymanagement.ivy.{ Credentials, IvyConfiguration, IvyPaths, Upda
 import sbt.nio.file.Glob
 import sbt.testing.Framework
 import sbt.util.{ cacheLevel, ActionCacheStore, Level, Logger, LoggerContext }
-import xsbti.{ FileConverter, HashedVirtualFileRef, VirtualFile, VirtualFileRef }
+import xsbti.{ HashedVirtualFileRef, VirtualFile, VirtualFileRef }
 import xsbti.compile._
 import xsbti.compile.analysis.ReadStamps
 
@@ -283,7 +283,7 @@ object Keys {
   private[sbt] val externalHooks = taskKey[ExternalHooks]("The external hooks used by zinc.")
   val auxiliaryClassFiles = taskKey[Seq[AuxiliaryClassFiles]]("The auxiliary class files that must be managed by Zinc (for instance the TASTy files)")
   @cacheLevel(include = Array.empty)
-  val fileConverter = settingKey[FileConverter]("The file converter used to convert between Path and VirtualFile")
+  val fileConverter = SettingKey(BasicKeys.fileConverter)
   val allowMachinePath = settingKey[Boolean]("Allow machine-specific paths during conversion.")
   val reportAbsolutePath = settingKey[Boolean]("Report absolute paths during compilation.")
   val rootPaths = settingKey[Map[String, NioPath]]("The root paths used to abstract machine-specific paths.")

main/src/main/scala/sbt/Main.scala

@@ -951,7 +951,6 @@ object BuiltinCommands {
   def doLoadProject(s0: State, action: LoadAction): State = {
     welcomeBanner(s0)
     checkSBTVersionChanged(s0)
-    RemoteCache.initializeRemoteCache(s0)
     val (s1, base) = Project.loadAction(SessionVar.clear(s0), action)
     IO.createDirectory(base)
     val s2 = if (s1 has Keys.stateCompilerCache) s1 else registerCompilerCache(s1)
@@ -974,7 +973,6 @@ object BuiltinCommands {
       st => setupGlobalFileTreeRepository(addCacheStoreFactoryFactory(st))
     )
     val s4 = s3.put(Keys.useLog4J.key, Project.extract(s3).get(Keys.useLog4J))
-    RemoteCache.initializeRemoteCache(s4)
     addSuperShellParams(CheckBuildSources.init(LintUnused.lintUnusedFunc(s4)))
   }
 

main/src/main/scala/sbt/ProjectExtra.scala

@@ -57,6 +57,7 @@ import scala.annotation.targetName
 import scala.concurrent.{ Await, TimeoutException }
 import scala.concurrent.duration.*
 import ClasspathDep.*
+import xsbti.FileConverter
 
 /*
 sealed trait Project extends ProjectDefinition[ProjectReference] with CompositeProject {
@@ -322,6 +323,7 @@ trait ProjectExtra extends Scoped.Syntax:
       val hs: Option[Seq[ServerHandler]] = get(ThisBuild / fullServerHandlers)
       val caches: Option[Seq[ActionCacheStore]] = get(cacheStores)
       val rod: Option[NioPath] = get(rootOutputDirectory)
+      val fileConverter: Option[FileConverter] = get(Keys.fileConverter)
       val commandDefs = allCommands.distinct.flatten[Command].map(_ tag (projectCommand, true))
       val newDefinedCommands = commandDefs ++ BasicCommands.removeTagged(
         s.definedCommands,
@@ -349,6 +351,7 @@ trait ProjectExtra extends Scoped.Syntax:
           .setCond(fullServerHandlers.key, hs)
           .setCond(cacheStores.key, caches)
           .setCond(rootOutputDirectory.key, rod)
+          .setCond(BasicKeys.fileConverter, fileConverter)
       s.copy(
         attributes = newAttrs,
         definedCommands = newDefinedCommands

main/src/main/scala/sbt/RemoteCache.scala

@@ -22,7 +22,7 @@ import sbt.ProjectExtra.*
 import sbt.ScopeFilter.Make._
 import sbt.SlashSyntax0._
 import sbt.coursierint.LMCoursier
-import sbt.internal.inc.{ MappedFileConverter, HashUtil, JarUtils }
+import sbt.internal.inc.{ HashUtil, JarUtils }
 import sbt.internal.librarymanagement._
 import sbt.internal.remotecache._
 import sbt.io.IO
@@ -34,7 +34,7 @@ import sbt.nio.FileStamp
 import sbt.nio.Keys.{ inputFileStamps, outputFileStamps }
 import sbt.std.TaskExtra._
 import sbt.util.InterfaceUtil.toOption
-import sbt.util.{ ActionCacheStore, AggregateActionCacheStore, DiskActionCacheStore, Logger }
+import sbt.util.{ ActionCacheStore, DiskActionCacheStore, Logger }
 import sjsonnew.JsonFormat
 import xsbti.{ FileConverter, HashedVirtualFileRef, VirtualFileRef }
 import xsbti.compile.CompileAnalysis
@@ -50,27 +50,6 @@ object RemoteCache {
   private[sbt] val analysisStore: mutable.Map[HashedVirtualFileRef, CompileAnalysis] =
     mutable.Map.empty
 
-  // TODO: figure out a good timing to initialize cache
-  // currently this is called twice so metabuild can call compile with a minimal setting
-  private[sbt] def initializeRemoteCache(s: State): Unit =
-    val outDir =
-      s.get(BasicKeys.rootOutputDirectory).getOrElse((s.baseDir / "target" / "out").toPath)
-    Def._outputDirectory = Some(outDir)
-    def defaultCache =
-      val fileConverter = s
-        .get(Keys.fileConverter.key)
-        .getOrElse {
-          MappedFileConverter(
-            Defaults.getRootPaths(outDir, s.configuration),
-            allowMachinePath = true
-          )
-        }
-      DiskActionCacheStore((s.baseDir / "target" / "bootcache").toPath, fileConverter)
-    Def._cacheStore = s
-      .get(BasicKeys.cacheStores)
-      .collect { case xs if xs.nonEmpty => AggregateActionCacheStore(xs) }
-      .getOrElse(defaultCache)
-
   private[sbt] def artifactToStr(art: Artifact): String = {
     import LibraryManagementCodec._
     import sjsonnew.support.scalajson.unsafe._
@@ -110,7 +89,7 @@ object RemoteCache {
     },
     cacheStores := {
       List(
-        DiskActionCacheStore(localCacheDirectory.value.toPath(), fileConverter.value)
+        DiskActionCacheStore(localCacheDirectory.value.toPath())
       )
     },
     remoteCache := SysProp.remoteCache,

main/src/main/scala/sbt/internal/Aggregation.scala

@@ -98,13 +98,12 @@ object Aggregation {
     val roots = ts.map { case KeyValue(k, _) => k }
     val config = extractedTaskConfig(extracted, structure, s)
     val start = System.currentTimeMillis
-    val cacheEventLog = Def.cacheConfiguration.cacheEventLog
+    Def.cacheEventLog.clear()
-    cacheEventLog.clear()
     val (newS, result) = withStreams(structure, s): str =>
       val transform = nodeView(s, str, roots, extra)
       runTask(toRun, s, str, structure.index.triggers, config)(using transform)
     val stop = System.currentTimeMillis
-    val cacheSummary = cacheEventLog.summary
+    val cacheSummary = Def.cacheEventLog.summary
     Complete(start, stop, result, cacheSummary, newS)
 
   def runTasks[A1](

util-cache/src/main/scala/sbt/internal/util/CacheEventLog.scala

@@ -3,19 +3,25 @@ package internal
 package util
 
 import scala.collection.concurrent.TrieMap
+import xsbti.VirtualFileRef
 
 enum ActionCacheEvent:
   case Found(storeName: String)
-  case NotFound
+  case OnsiteTask
+  case Error
 end ActionCacheEvent
 
+case class ActionCacheError(outputFiles: Seq[VirtualFileRef])
+
 class CacheEventLog:
   private val acEvents = TrieMap.empty[ActionCacheEvent, Long]
+
   def append(event: ActionCacheEvent): Unit =
     acEvents.updateWith(event) {
       case None        => Some(1L)
       case Some(count) => Some(count + 1L)
     }
+
   def clear(): Unit =
     acEvents.clear()
 
@@ -23,21 +29,25 @@ class CacheEventLog:
     if acEvents.isEmpty then ""
     else
       val total = acEvents.values.sum
-      val hit = acEvents.view.collect { case (k @ ActionCacheEvent.Found(_), v) =>
+      val hits = acEvents.view.collect { case (ActionCacheEvent.Found(id), v) => (id, v) }.toMap
-        (k, v)
+      val hitCount = hits.values.sum
-      }.toMap
-      val hitCount = hit.values.sum
       val missCount = total - hitCount
       val hitRate = (hitCount.toDouble / total.toDouble * 100.0).floor.toInt
-      val hitDescs = hit.toSeq.map {
+      val hitDescs = hits.toSeq.map {
-        case (ActionCacheEvent.Found(id), 1) => s"1 $id cache hit"
+        case (id, 1) => s"1 $id cache hit"
-        case (ActionCacheEvent.Found(id), v) => s"$v $id cache hits"
+        case (id, v) => s"$v $id cache hits"
       }.sorted
-      val missDescs = missCount match
+      val missDesc = acEvents
-        case 0 => Nil
+        .get(ActionCacheEvent.OnsiteTask)
-        case 1 => Seq(s"$missCount onsite task")
+        .map:
-        case _ => Seq(s"$missCount onsite tasks")
+          case 1 => s"1 onsite task"
-      val descs = hitDescs ++ missDescs
+          case _ => s"$missCount onsite tasks"
-      val descsSummary = descs.mkString(", ", ", ", "")
+      val errorDesc = acEvents
-      s"cache $hitRate%$descsSummary"
+        .get(ActionCacheEvent.Error)
+        .map:
+          case 1      => s"1 error"
+          case errors => s"$errors errors"
+      val descs = hitDescs ++ missDesc ++ errorDesc
+      val descsSummary = descs.mkString(", ")
+      s"cache $hitRate%, $descsSummary"
 end CacheEventLog

util-cache/src/main/scala/sbt/util/ActionCache.scala

@@ -7,7 +7,7 @@ import scala.annotation.{ meta, StaticAnnotation }
 import sjsonnew.{ HashWriter, JsonFormat }
 import sjsonnew.support.murmurhash.Hasher
 import sjsonnew.support.scalajson.unsafe.{ CompactPrinter, Converter, Parser }
-import xsbti.VirtualFile
+import xsbti.{ FileConverter, VirtualFile }
 import java.nio.charset.StandardCharsets
 import java.nio.file.Path
 import scala.quoted.{ Expr, FromExpr, ToExpr, Quotes }
@@ -37,30 +37,47 @@ object ActionCache:
   )(
       config: BuildWideCacheConfiguration
   ): O =
-    val store = config.store
+    import config.*
-    val cacheEventLog = config.cacheEventLog
     val input =
       Digest.sha256Hash(codeContentHash, extraHash, Digest.dummy(Hasher.hashUnsafe[I](key)))
-    val valuePath = config.outputDirectory.resolve(s"value/${input}.json").toString
+    val valuePath = s"value/${input}.json"
+
     def organicTask: O =
-      cacheEventLog.append(ActionCacheEvent.NotFound)
       // run action(...) and combine the newResult with outputs
-      val (newResult, outputs) = action(key)
+      val (result, outputs) =
-      val json = Converter.toJsonUnsafe(newResult)
+        try action(key)
-      val valueFile = StringVirtualFile1(valuePath, CompactPrinter(json))
+        catch
+          case e: Exception =>
+            cacheEventLog.append(ActionCacheEvent.Error)
+            throw e
+      val json = Converter.toJsonUnsafe(result)
+      val uncacheableOutputs =
+        outputs.filter(f => !fileConverter.toPath(f).startsWith(outputDirectory))
+      if uncacheableOutputs.nonEmpty then
+        cacheEventLog.append(ActionCacheEvent.Error)
+        logger.error(
+          s"Cannot cache task because its output files are outside the output directory: \n" +
+            uncacheableOutputs.mkString("  - ", "\n  - ", "")
+        )
+        result
+      else
+        cacheEventLog.append(ActionCacheEvent.OnsiteTask)
+        val valueFile = StringVirtualFile1(s"value/${input}.json", CompactPrinter(json))
         val newOutputs = Vector(valueFile) ++ outputs.toVector
         store.put(UpdateActionResultRequest(input, newOutputs, exitCode = 0)) match
-        case Right(result) =>
+          case Right(cachedResult) =>
-          store.syncBlobs(result.outputFiles, config.outputDirectory)
+            store.syncBlobs(cachedResult.outputFiles, config.outputDirectory)
-          newResult
+            result
           case Left(e) => throw e
+
     def valueFromStr(str: String, origin: Option[String]): O =
       cacheEventLog.append(ActionCacheEvent.Found(origin.getOrElse("unknown")))
       val json = Parser.parseUnsafe(str)
       Converter.fromJsonUnsafe[O](json)
-    store.get(
+
+    val getRequest =
       GetActionResultRequest(input, inlineStdout = false, inlineStderr = false, Vector(valuePath))
-    ) match
+    store.get(getRequest) match
       case Right(result) =>
         // some protocol can embed values into the result
         result.contents.headOption match
@@ -78,6 +95,8 @@ end ActionCache
 class BuildWideCacheConfiguration(
     val store: ActionCacheStore,
     val outputDirectory: Path,
+    val fileConverter: FileConverter,
+    val logger: Logger,
     val cacheEventLog: CacheEventLog,
 ):
   override def toString(): String =

util-cache/src/main/scala/sbt/util/ActionCacheStore.scala

@@ -13,7 +13,7 @@ import sbt.io.IO
 import sbt.io.syntax.*
 import sbt.internal.util.StringVirtualFile1
 import sbt.internal.util.codec.ActionResultCodec.given
-import xsbti.{ FileConverter, HashedVirtualFileRef, PathBasedFile, VirtualFile }
+import xsbti.{ HashedVirtualFileRef, PathBasedFile, VirtualFile }
 import java.io.InputStream
 
 /**
@@ -166,8 +166,7 @@ class InMemoryActionCacheStore extends AbstractActionCacheStore:
     underlying.toString()
 end InMemoryActionCacheStore
 
-class DiskActionCacheStore(base: Path, fileConverter: FileConverter)
+class DiskActionCacheStore(base: Path) extends AbstractActionCacheStore:
-    extends AbstractActionCacheStore:
   lazy val casBase: Path = {
     val dir = base.resolve("cas")
     IO.createDirectory(dir.toFile)
@@ -242,10 +241,13 @@ class DiskActionCacheStore(base: Path, fileConverter: FileConverter)
       else None
 
   override def syncBlobs(refs: Seq[HashedVirtualFileRef], outputDirectory: Path): Seq[Path] =
-    refs.flatMap: ref =>
+    refs.flatMap: r =>
-      val casFile = toCasFile(Digest(ref))
+      val casFile = toCasFile(Digest(r))
       if casFile.toFile().exists then
-        val outPath = fileConverter.toPath(ref)
+        val shortPath =
+          if r.id.startsWith("${OUT}/") then r.id.drop(7)
+          else r.id
+        val outPath = outputDirectory.resolve(shortPath)
         Files.createDirectories(outPath.getParent())
         if outPath.toFile().exists() then IO.delete(outPath.toFile())
         Some(Files.createSymbolicLink(outPath, casFile))

util-cache/src/test/scala/sbt/util/ActionCacheTest.scala

@@ -5,13 +5,12 @@ import sbt.internal.util.StringVirtualFile1
 import sbt.io.IO
 import sbt.io.syntax.*
 import verify.BasicTestSuite
-import xsbti.FileConverter
 import xsbti.VirtualFile
+import xsbti.FileConverter
 import xsbti.VirtualFileRef
-
-import java.nio.file.Files
 import java.nio.file.Path
 import java.nio.file.Paths
+import java.nio.file.Files
 
 object ActionCacheTest extends BasicTestSuite:
   val tags = CacheLevelTag.all.toList
@@ -20,10 +19,10 @@ object ActionCacheTest extends BasicTestSuite:
     withDiskCache(testHoldBlob)
 
   def testHoldBlob(cache: ActionCacheStore): Unit =
-    IO.withTemporaryDirectory: tempDir =>
+    val in = StringVirtualFile1("a.txt", "foo")
-      val in = StringVirtualFile1(s"$tempDir/a.txt", "foo")
     val hashRefs = cache.putBlobs(in :: Nil)
     assert(hashRefs.size == 1)
+    IO.withTemporaryDirectory: tempDir =>
       val actual = cache.syncBlobs(hashRefs, tempDir.toPath()).head
       assert(actual.getFileName().toString() == "a.txt")
 
@@ -41,12 +40,12 @@ object ActionCacheTest extends BasicTestSuite:
       (a + b, Nil)
     }
     IO.withTemporaryDirectory: (tempDir) =>
-      val config = BuildWideCacheConfiguration(cache, tempDir.toPath(), CacheEventLog())
+      val config = getCacheConfig(cache, tempDir)
       val v1 =
-        ActionCache.cache[(Int, Int), Int]((1, 1), Digest.zero, Digest.zero, tags)(action)(config)
+        ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags)(action)(config)
       assert(v1 == 2)
       val v2 =
-        ActionCache.cache[(Int, Int), Int]((1, 1), Digest.zero, Digest.zero, tags)(action)(config)
+        ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags)(action)(config)
       assert(v2 == 2)
       // check that the action has been invoked only once
       assert(called == 1)
@@ -55,17 +54,17 @@ object ActionCacheTest extends BasicTestSuite:
     withDiskCache(testActionCacheWithBlob)
 
   def testActionCacheWithBlob(cache: ActionCacheStore): Unit =
-    IO.withTemporaryDirectory: (tempDir) =>
     import sjsonnew.BasicJsonProtocol.*
+    IO.withTemporaryDirectory: (tempDir) =>
       var called = 0
       val action: ((Int, Int)) => (Int, Seq[VirtualFile]) = { case (a, b) =>
         called += 1
         val out = StringVirtualFile1(s"$tempDir/a.txt", (a + b).toString)
         (a + b, Seq(out))
       }
-      val config = BuildWideCacheConfiguration(cache, tempDir.toPath(), CacheEventLog())
+      val config = getCacheConfig(cache, tempDir)
       val v1 =
-        ActionCache.cache[(Int, Int), Int]((1, 1), Digest.zero, Digest.zero, tags)(action)(config)
+        ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags)(action)(config)
       assert(v1 == 2)
       // ActionResult only contains the reference to the files.
       // To retrieve them, separately call readBlobs or syncBlobs.
@@ -75,7 +74,7 @@ object ActionCacheTest extends BasicTestSuite:
       assert(content == "2")
 
       val v2 =
-        ActionCache.cache[(Int, Int), Int]((1, 1), Digest.zero, Digest.zero, tags)(action)(config)
+        ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags)(action)(config)
       assert(v2 == 2)
       // check that the action has been invoked only once
       assert(called == 1)
@@ -88,12 +87,19 @@ object ActionCacheTest extends BasicTestSuite:
     IO.withTemporaryDirectory(
       { tempDir0 =>
         val tempDir = tempDir0.toPath
-        val cache = DiskActionCacheStore(tempDir, fileConverter)
+        val cache = DiskActionCacheStore(tempDir)
         f(cache)
       },
       keepDirectory = false
     )
 
+  def getCacheConfig(cache: ActionCacheStore, outputDir: File): BuildWideCacheConfiguration =
+    val logger = new Logger:
+      override def trace(t: => Throwable): Unit = ()
+      override def success(message: => String): Unit = ()
+      override def log(level: Level.Value, message: => String): Unit = ()
+    BuildWideCacheConfiguration(cache, outputDir.toPath(), fileConverter, logger, CacheEventLog())
+
   def fileConverter = new FileConverter:
     override def toPath(ref: VirtualFileRef): Path = Paths.get(ref.id)
     override def toVirtualFile(path: Path): VirtualFile =

util-cache/src/test/scala/sbt/util/CacheEventLogTest.scala

@@ -25,20 +25,38 @@ object CacheEventLogTest extends BasicTestSuite:
     assertEquals(logger.summary, expectedSummary)
   }
 
-  test("summary of 1 disk, 1 miss event") {
+  test("summary of 1 disk, 1 onsite task") {
     val logger = CacheEventLog()
     logger.append(ActionCacheEvent.Found("disk"))
-    logger.append(ActionCacheEvent.NotFound)
+    logger.append(ActionCacheEvent.OnsiteTask)
     val expectedSummary = "cache 50%, 1 disk cache hit, 1 onsite task"
     assertEquals(logger.summary, expectedSummary)
   }
 
-  test("summary of 1 disk, 2 remote, 1 miss event") {
+  test("summary of 1 disk, 1 onsite task, 1 error") {
+    val logger = CacheEventLog()
+    logger.append(ActionCacheEvent.Found("disk"))
+    logger.append(ActionCacheEvent.OnsiteTask)
+    logger.append(ActionCacheEvent.Error)
+    val expectedSummary = "cache 33%, 1 disk cache hit, 1 onsite task, 1 error"
+    assertEquals(logger.summary, expectedSummary)
+  }
+
+  test("summary of 1 disk, 2 errors") {
+    val logger = CacheEventLog()
+    logger.append(ActionCacheEvent.Found("disk"))
+    logger.append(ActionCacheEvent.Error)
+    logger.append(ActionCacheEvent.Error)
+    val expectedSummary = "cache 33%, 1 disk cache hit, 2 errors"
+    assertEquals(logger.summary, expectedSummary)
+  }
+
+  test("summary of 1 disk, 2 remote, 1 onsite task") {
     val logger = CacheEventLog()
     logger.append(ActionCacheEvent.Found("disk"))
     logger.append(ActionCacheEvent.Found("remote"))
     logger.append(ActionCacheEvent.Found("remote"))
-    logger.append(ActionCacheEvent.NotFound)
+    logger.append(ActionCacheEvent.OnsiteTask)
     val expectedSummary = "cache 75%, 1 disk cache hit, 2 remote cache hits, 1 onsite task"
     assertEquals(logger.summary, expectedSummary)
   }