From fe6125d8d199c8a2d34b452d56a957f2455e9fc0 Mon Sep 17 00:00:00 2001 From: MkDev11 Date: Mon, 12 Jan 2026 16:03:05 -0500 Subject: [PATCH] [2.x] feat: Cache failed compilation to avoid repeated failures (#8490) When a compilation fails with CompileFailed, the failure is now cached so that subsequent builds with the same inputs don't re-run the failed compilation. This significantly improves the experience when using BSP clients like Metals that may trigger many compilations in a row. The implementation: - Adds CachedCompileFailure, CachedProblem, and CachedPosition types to serialize compilation failures - Modifies ActionCache.cache to catch CompileFailed exceptions and store them in the cache with exitCode=1 - On cache lookup, checks for cached failures first and re-throws the cached exception if found - Fixes DiskActionCacheStore.put to preserve exitCode from request - Adds unit test to verify cached failure behavior Fixes #7662 --- .../sbt/util/GenericFailure.scala | 55 +++++++ .../src/main/contraband/actionresult.contra | 11 ++ .../src/main/scala/sbt/util/ActionCache.scala | 142 ++++++++++++------ .../scala/sbt/util/ActionCacheStore.scala | 2 +- .../scala/sbt/util/CachedCompileFailure.scala | 109 ++++++++++++++ .../test/scala/sbt/util/ActionCacheTest.scala | 69 ++++++++- 6 files changed, 339 insertions(+), 49 deletions(-) create mode 100644 util-cache/src/main/contraband-scala/sbt/util/GenericFailure.scala create mode 100644 util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala diff --git a/util-cache/src/main/contraband-scala/sbt/util/GenericFailure.scala b/util-cache/src/main/contraband-scala/sbt/util/GenericFailure.scala new file mode 100644 index 000000000..0be5c13c4 --- /dev/null +++ b/util-cache/src/main/contraband-scala/sbt/util/GenericFailure.scala @@ -0,0 +1,55 @@ +/** + * This code is generated using [[https://www.scala-sbt.org/contraband]]. + */ + +// DO NOT EDIT MANUALLY +package sbt.util +/** + * A GenericFailure represents a cached task failure. + * This allows caching failures so that repeated builds don't re-run failed tasks. + * The kind field indicates the type of failure (e.g., "CompileFailed"). + * + * Fixes https://github.com/sbt/sbt/issues/7662 + */ +final class GenericFailure private ( + val kind: Option[String], + val message: Option[String], + val problems: Vector[xsbti.Problem]) extends Serializable { + + private def this() = this(None, None, Vector()) + + override def equals(o: Any): Boolean = this.eq(o.asInstanceOf[AnyRef]) || (o match { + case x: GenericFailure => (this.kind == x.kind) && (this.message == x.message) && (this.problems == x.problems) + case _ => false + }) + override def hashCode: Int = { + 37 * (37 * (37 * (37 * (17 + "sbt.util.GenericFailure".##) + kind.##) + message.##) + problems.##) + } + override def toString: String = { + "GenericFailure(" + kind + ", " + message + ", " + problems + ")" + } + private def copy(kind: Option[String] = kind, message: Option[String] = message, problems: Vector[xsbti.Problem] = problems): GenericFailure = { + new GenericFailure(kind, message, problems) + } + def withKind(kind: Option[String]): GenericFailure = { + copy(kind = kind) + } + def withKind(kind: String): GenericFailure = { + copy(kind = Option(kind)) + } + def withMessage(message: Option[String]): GenericFailure = { + copy(message = message) + } + def withMessage(message: String): GenericFailure = { + copy(message = Option(message)) + } + def withProblems(problems: Vector[xsbti.Problem]): GenericFailure = { + copy(problems = problems) + } +} +object GenericFailure { + + def apply(): GenericFailure = new GenericFailure() + def apply(kind: Option[String], message: Option[String], problems: Vector[xsbti.Problem]): GenericFailure = new GenericFailure(kind, message, problems) + def apply(kind: String, message: String, problems: Vector[xsbti.Problem]): GenericFailure = new GenericFailure(Option(kind), Option(message), problems) +} diff --git a/util-cache/src/main/contraband/actionresult.contra b/util-cache/src/main/contraband/actionresult.contra index 49b524683..714abc9ed 100644 --- a/util-cache/src/main/contraband/actionresult.contra +++ b/util-cache/src/main/contraband/actionresult.contra @@ -15,3 +15,14 @@ type ActionResult { contents: [java.nio.ByteBuffer] @since("0.4.0") isExecutable: [Boolean] @since("0.5.0") } + +## A GenericFailure represents a cached task failure. +## This allows caching failures so that repeated builds don't re-run failed tasks. +## The kind field indicates the type of failure (e.g., "CompileFailed"). +## +## Fixes https://github.com/sbt/sbt/issues/7662 +type GenericFailure @generateCodec(false) { + kind: String @since("0.1.0") + message: String @since("0.1.0") + problems: [xsbti.Problem] @since("0.1.0") +} diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 5ed680e5f..c540cf7e2 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala @@ -24,11 +24,13 @@ import sjsonnew.{ HashWriter, JsonFormat } import sjsonnew.support.murmurhash.Hasher import sjsonnew.support.scalajson.unsafe.{ CompactPrinter, Converter, Parser } import scala.quoted.{ Expr, FromExpr, ToExpr, Quotes } -import xsbti.{ FileConverter, HashedVirtualFileRef, VirtualFile, VirtualFileRef } +import xsbti.{ CompileFailed, FileConverter, HashedVirtualFileRef, VirtualFile, VirtualFileRef } object ActionCache: private[sbt] val dirZipExt = ".sbtdir.zip" private[sbt] val manifestFileName = "sbtdir_manifest.json" + private[sbt] val failureFileName = "failure.json" + private[sbt] val failureExitCode = 1 /** * This is a key function that drives remote caching. @@ -54,11 +56,28 @@ object ActionCache: action: I => InternalActionResult[O], ): O = import config.* + + def cacheFailure(e: CompileFailed): Nothing = + // Cache the failure so subsequent builds don't re-run failed compilation + // This fixes https://github.com/sbt/sbt/issues/7662 + // Use the same input digest as success, distinguished by exitCode + cacheEventLog.append(ActionCacheEvent.OnsiteTask) + val (input, valuePath) = mkInput(key, codeContentHash, extraHash) + val cachedFailure = CachedCompileFailure.fromException(e) + val json = Converter.toJsonUnsafe(cachedFailure) + val failureFile = StringVirtualFile1(valuePath, CompactPrinter(json)) + store.put( + UpdateActionResultRequest(input, Vector(failureFile), exitCode = failureExitCode) + ) + throw e + def organicTask: O = // run action(...) and combine the newResult with outputs val InternalActionResult(result, outputs) = try action(key): @unchecked catch + case e: CompileFailed => + cacheFailure(e) case e: Exception => cacheEventLog.append(ActionCacheEvent.Error) throw e @@ -83,11 +102,82 @@ object ActionCache: result case Left(e) => throw e - get(key, codeContentHash, extraHash, tags, config) match - case Some(value) => value - case None => organicTask + // Single cache lookup - use exitCode to distinguish success from failure + getWithFailure(key, codeContentHash, extraHash, tags, config) match + case Right(value) => value + case Left(Some(failure)) => + config.cacheEventLog.append(ActionCacheEvent.Found("cached-failure")) + // Replay problems to the logger so users see the cached errors/warnings + failure.replay(config.logger) + throw failure.toException + case Left(None) => organicTask end cache + /** + * Retrieves the cached value or failure with a single cache lookup. + * Returns Right(value) for cached success, Left(Some(failure)) for cached failure, + * or Left(None) for cache miss. + */ + private def getWithFailure[I: HashWriter, O: JsonFormat]( + key: I, + codeContentHash: Digest, + extraHash: Digest, + tags: List[CacheLevelTag], + config: BuildWideCacheConfiguration, + ): Either[Option[CachedCompileFailure], O] = + import config.store + def valueFromStr(str: String, origin: Option[String]): O = + config.cacheEventLog.append(ActionCacheEvent.Found(origin.getOrElse("unknown"))) + val json = Parser.parseUnsafe(str) + Converter.fromJsonUnsafe[O](json) + + def failureFromStr(str: String): CachedCompileFailure = + val json = Parser.parseUnsafe(str) + Converter.fromJsonUnsafe[CachedCompileFailure](json) + + // Optimization: Check if we can read directly from symlinked value file + val (input, valuePath) = mkInput(key, codeContentHash, extraHash) + val resolvedValuePath = config.fileConverter.toPath(VirtualFileRef.of(valuePath)) + + def readFromSymlink(): Option[Either[Option[CachedCompileFailure], O]] = + if java.nio.file.Files.isSymbolicLink(resolvedValuePath) && java.nio.file.Files + .exists(resolvedValuePath) + then + Exception.nonFatalCatch + .opt(IO.read(resolvedValuePath.toFile(), StandardCharsets.UTF_8)) + .flatMap: str => + // We still need to sync output files for side effects and check exitCode + findActionResult(key, codeContentHash, extraHash, config) match + case Right(result) => + store.syncBlobs(result.outputFiles, config.outputDirectory) + if result.exitCode.contains(failureExitCode) then + Some(Left(Some(failureFromStr(str)))) + else Some(Right(valueFromStr(str, Some("symlink")))) + case Left(_) => None + else None + + readFromSymlink() match + case Some(result) => result + case None => + findActionResult(key, codeContentHash, extraHash, config) match + case Right(result) => + // Check exitCode to determine if this is a cached failure + val isFailure = result.exitCode.contains(failureExitCode) + result.contents.headOption match + case Some(head) => + store.syncBlobs(result.outputFiles, config.outputDirectory) + val str = String(head.array(), StandardCharsets.UTF_8) + if isFailure then Left(Some(failureFromStr(str))) + else Right(valueFromStr(str, result.origin)) + case _ => + val paths = store.syncBlobs(result.outputFiles, config.outputDirectory) + if paths.isEmpty then Left(None) + else + val str = IO.read(paths.head.toFile()) + if isFailure then Left(Some(failureFromStr(str))) + else Right(valueFromStr(str, result.origin)) + case Left(_) => Left(None) + /** * Retrieves the cached value. */ @@ -98,47 +188,9 @@ object ActionCache: tags: List[CacheLevelTag], config: BuildWideCacheConfiguration, ): Option[O] = - import config.store - def valueFromStr(str: String, origin: Option[String]): O = - config.cacheEventLog.append(ActionCacheEvent.Found(origin.getOrElse("unknown"))) - val json = Parser.parseUnsafe(str) - Converter.fromJsonUnsafe[O](json) - - // Optimization: Check if we can read directly from symlinked value file - val (input, valuePath) = mkInput(key, codeContentHash, extraHash) - val resolvedValuePath = config.fileConverter.toPath(VirtualFileRef.of(valuePath)) - - def readFromSymlink(): Option[O] = - if java.nio.file.Files.isSymbolicLink(resolvedValuePath) && java.nio.file.Files - .exists(resolvedValuePath) - then - Exception.nonFatalCatch - .opt(IO.read(resolvedValuePath.toFile(), StandardCharsets.UTF_8)) - .map: str => - // We still need to sync output files for side effects - findActionResult(key, codeContentHash, extraHash, config) match - case Right(result) => - store.syncBlobs(result.outputFiles, config.outputDirectory) - case Left(_) => // Ignore if we can't find ActionResult - valueFromStr(str, Some("symlink")) - else None - - readFromSymlink() match - case Some(value) => Some(value) - case None => - findActionResult(key, codeContentHash, extraHash, config) match - case Right(result) => - // some protocol can embed values into the result - result.contents.headOption match - case Some(head) => - store.syncBlobs(result.outputFiles, config.outputDirectory) - val str = String(head.array(), StandardCharsets.UTF_8) - Some(valueFromStr(str, result.origin)) - case _ => - val paths = store.syncBlobs(result.outputFiles, config.outputDirectory) - if paths.isEmpty then None - else Some(valueFromStr(IO.read(paths.head.toFile()), result.origin)) - case Left(_) => None + getWithFailure(key, codeContentHash, extraHash, tags, config) match + case Right(value) => Some(value) + case Left(_) => None /** * Checks if the ActionResult exists in the cache. diff --git a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala index 4b85ae8d2..8e358fbdb 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala @@ -217,7 +217,7 @@ class DiskActionCacheStore(base: Path, converter: FileConverter) extends Abstrac try val acFile = acBase.toFile / request.actionDigest.toString.replace("/", "-") val refs = putBlobsIfNeeded(request.outputFiles).toVector - val v = ActionResult(refs, storeName) + val v = ActionResult(refs, Some(storeName), request.exitCode) val json = Converter.toJsonUnsafe(v) IO.write(acFile, CompactPrinter(json)) Right(v) diff --git a/util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala b/util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala new file mode 100644 index 000000000..fc55bd681 --- /dev/null +++ b/util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala @@ -0,0 +1,109 @@ +/* + * sbt + * Copyright 2023, Scala center + * Copyright 2011 - 2022, Lightbend, Inc. + * Copyright 2008 - 2010, Mark Harrah + * Licensed under Apache License 2.0 (see LICENSE) + */ + +package sbt.util + +import sjsonnew.{ Builder, JsonFormat, Unbuilder, deserializationError } +import sbt.internal.util.codec.{ ProblemFormats, SeverityFormats, PositionFormats } +import xsbti.{ CompileFailed, Problem, Severity } + +/** + * A wrapper around GenericFailure for CompileFailed exceptions. + * This allows caching compilation failures so that repeated builds + * don't re-run failed compilations unnecessarily. + * + * Fixes https://github.com/sbt/sbt/issues/7662 + */ +final case class CachedCompileFailure(underlying: GenericFailure): + def toException: CompileFailed = new CompileFailed: + override def arguments(): Array[String] = Array.empty + override def problems(): Array[Problem] = underlying.problems.toArray + override def getMessage(): String = underlying.message.getOrElse("") + + /** Replay problems to the logger so users see the cached errors/warnings. */ + def replay(logger: Logger): Unit = + underlying.problems.foreach: problem => + val msg = CachedCompileFailure.formatProblem(problem) + problem.severity match + case Severity.Error => logger.error(msg) + case Severity.Warn => logger.warn(msg) + case Severity.Info => logger.info(msg) +end CachedCompileFailure + +object CachedCompileFailure + extends ProblemFormats + with SeverityFormats + with PositionFormats + with sjsonnew.BasicJsonProtocol: + + private val CompileFailedKind = "CompileFailed" + + /** + * Format a problem for display. Uses the `rendered` field if available (Scala 3), + * otherwise constructs a message from position and message (Scala 2.13). + */ + private[util] def formatProblem(problem: Problem): String = + import sbt.util.InterfaceUtil.toOption + toOption(problem.rendered).getOrElse: + val pos = problem.position + val file = toOption(pos.sourcePath).getOrElse("unknown") + val line = toOption(pos.line).map(l => s":$l").getOrElse("") + val pointer = toOption(pos.pointer).map(p => s":$p").getOrElse("") + val lineContent = Option(pos.lineContent).filter(_.nonEmpty).map(c => s"\n$c").getOrElse("") + val pointerLine = toOption(pos.pointerSpace).map(s => s"\n$s^").getOrElse("") + s"$file$line$pointer: ${problem.message}$lineContent$pointerLine" + + /** + * Check if the problems contain enough information to be useful when replayed. + * For Scala 2.13, the `rendered` field is empty, so we check if position info exists. + */ + def hasSufficientInfo(e: CompileFailed): Boolean = + import sbt.util.InterfaceUtil.toOption + e.problems() + .forall: problem => + // Either has rendered text (Scala 3) or has position info (Scala 2.13) + toOption(problem.rendered).isDefined || + (toOption(problem.position.sourcePath).isDefined && problem.message.nonEmpty) + + def fromException(e: CompileFailed): CachedCompileFailure = + CachedCompileFailure( + GenericFailure( + kind = CompileFailedKind, + message = Option(e.getMessage).getOrElse(""), + problems = e.problems().toVector + ) + ) + + // Custom JsonFormat for GenericFailure since we disabled automatic codec generation + given JsonFormat[GenericFailure] = new JsonFormat[GenericFailure]: + override def read[J](jsOpt: Option[J], unbuilder: Unbuilder[J]): GenericFailure = + jsOpt match + case Some(js) => + unbuilder.beginObject(js) + val kind = unbuilder.readField[Option[String]]("kind") + val message = unbuilder.readField[Option[String]]("message") + val problems = unbuilder.readField[Vector[Problem]]("problems") + unbuilder.endObject() + GenericFailure(kind, message, problems) + case None => + deserializationError("Expected JsObject but found None") + + override def write[J](obj: GenericFailure, builder: Builder[J]): Unit = + builder.beginObject() + builder.addField("kind", obj.kind) + builder.addField("message", obj.message) + builder.addField("problems", obj.problems) + builder.endObject() + + given JsonFormat[CachedCompileFailure] = new JsonFormat[CachedCompileFailure]: + private val gf = summon[JsonFormat[GenericFailure]] + override def read[J](jsOpt: Option[J], unbuilder: Unbuilder[J]): CachedCompileFailure = + CachedCompileFailure(gf.read(jsOpt, unbuilder)) + override def write[J](obj: CachedCompileFailure, builder: Builder[J]): Unit = + gf.write(obj.underlying, builder) +end CachedCompileFailure diff --git a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala index ee6d878e5..1ebd5e586 100644 --- a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala +++ b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala @@ -5,12 +5,19 @@ import sbt.internal.util.StringVirtualFile1 import sbt.io.IO import sbt.io.syntax.* import verify.BasicTestSuite -import xsbti.VirtualFile -import xsbti.FileConverter -import xsbti.VirtualFileRef +import xsbti.{ + CompileFailed, + Problem, + Position, + Severity, + VirtualFile, + FileConverter, + VirtualFileRef +} import java.nio.file.Path import java.nio.file.Paths import java.nio.file.Files +import java.util.Optional import ActionCache.InternalActionResult object ActionCacheTest extends BasicTestSuite: @@ -83,6 +90,9 @@ object ActionCacheTest extends BasicTestSuite: test("Disk cache can recover gracefully from invalid JSON"): withDiskCache(testActionCacheInvalidJson) + test("Disk cache caches CompileFailed exceptions"): + withDiskCache(testCachedCompileFailure) + def testActionCacheInvalidJson(cache: DiskActionCacheStore): Unit = import sjsonnew.BasicJsonProtocol.* var called = 0 @@ -105,6 +115,59 @@ object ActionCacheTest extends BasicTestSuite: // check that the action has been invoked twice assert(called == 2) + def testCachedCompileFailure(cache: DiskActionCacheStore): Unit = + import sjsonnew.BasicJsonProtocol.* + var called = 0 + val testProblem = new Problem: + override def category(): String = "Test" + override def severity(): Severity = Severity.Error + override def message(): String = "Test error message" + override def position(): Position = new Position: + override def line(): Optional[Integer] = Optional.of(42) + override def lineContent(): String = "val x = 1" + override def offset(): Optional[Integer] = Optional.empty() + override def pointer(): Optional[Integer] = Optional.empty() + override def pointerSpace(): Optional[String] = Optional.empty() + override def sourcePath(): Optional[String] = Optional.of("/test/file.scala") + override def sourceFile(): Optional[java.io.File] = Optional.empty() + + val testException = new CompileFailed: + override def arguments(): Array[String] = Array.empty + override def problems(): Array[Problem] = Array(testProblem) + override def getMessage(): String = "Compilation failed" + + val action: ((Int, Int)) => InternalActionResult[Int] = { (a, b) => + called += 1 + throw testException + } + IO.withTemporaryDirectory: tempDir => + val config = getCacheConfig(cache, tempDir) + + // First call should throw and cache the failure + var caught1: CompileFailed = null + try + ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(false, "Expected CompileFailed to be thrown") + catch case e: CompileFailed => caught1 = e + + assert(caught1 != null) + assert(called == 1) + + // Second call should throw cached failure without calling action again + var caught2: CompileFailed = null + try + ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(false, "Expected CompileFailed to be thrown") + catch case e: CompileFailed => caught2 = e + + assert(caught2 != null) + // Action should NOT have been called again - failure was cached + assert(called == 1) + // Verify the cached exception has the same data + assert(caught2.problems().length == 1) + assert(caught2.problems()(0).message() == "Test error message") + assert(caught2.getMessage() == "Compilation failed") + def withInMemoryCache(f: InMemoryActionCacheStore => Unit): Unit = val cache = InMemoryActionCacheStore() f(cache)