From b411ade704e7a8c794b466b35ba99abfe4d5e63c Mon Sep 17 00:00:00 2001 From: BrianHotopp Date: Sat, 18 Jul 2026 09:49:05 -0400 Subject: [PATCH] [2.x] fix: Don't cache environmental (position-less) compile failures (#9464) Failure caching assumes a CompileFailed is a function of the sources, which is true for source errors. But zinc also surfaces I/O write failures ("error writing X.class") as compiler problems, so an environmental failure (a concurrent target/ deletion, a permission blip) was cached under the same mechanism and replayed from the global action cache on every later build, even after the cause was gone. When the poisoned task is the metabuild compile this is self-sustaining and unrecoverable from inside sbt: project loading fails, so no task -- including clean -- can run, and only deleting the global cache by hand recovers. The replayed diagnostics also name files/permissions that no longer exist. Refs #9455 Co-authored-by: Claude Opus 4.8 (1M context) --- .../cache-environmental-compile-failure.md | 18 ++++ project/Scripted.scala | 8 +- .../cache/compile-io-failure/Main.scala | 2 + .../cache/compile-io-failure/build.sbt | 15 +++ .../sbt-test/cache/compile-io-failure/test | 8 ++ .../src/main/scala/sbt/util/ActionCache.scala | 13 ++- .../scala/sbt/util/CachedCompileFailure.scala | 14 +-- .../test/scala/sbt/util/ActionCacheTest.scala | 102 ++++++++++++++++++ 8 files changed, 165 insertions(+), 15 deletions(-) create mode 100644 notes/2.0.0/cache-environmental-compile-failure.md create mode 100644 sbt-app/src/sbt-test/cache/compile-io-failure/Main.scala create mode 100644 sbt-app/src/sbt-test/cache/compile-io-failure/build.sbt create mode 100644 sbt-app/src/sbt-test/cache/compile-io-failure/test diff --git a/notes/2.0.0/cache-environmental-compile-failure.md b/notes/2.0.0/cache-environmental-compile-failure.md new file mode 100644 index 000000000..6260e5073 --- /dev/null +++ b/notes/2.0.0/cache-environmental-compile-failure.md @@ -0,0 +1,18 @@ +### Environmental compile failures are no longer cached + +sbt caches a `CompileFailed` so that an unchanged, still-broken compile does not +re-run from scratch (#7662). But zinc also reports I/O write failures ("error +writing X.class") as compiler problems, so a one-off environmental failure (a +concurrent `target/` deletion, a permission blip) was cached under the same +mechanism and replayed on every later build, even after the cause was gone. When +it hit the metabuild compile the build stayed wedged across restarts, recoverable +only by deleting the global cache by hand. Compile failures whose errors carry no +source position are now treated as environmental and left uncached, so the next +build retries for real. A position-less failure already sitting in a cache +written by an earlier sbt is likewise no longer replayed: the task re-runs and a +success overwrites the stale entry, so previously wedged builds recover on +upgrade without deleting the cache. + +This addresses [#9455][i9455]. + +[i9455]: https://github.com/sbt/sbt/issues/9455 diff --git a/project/Scripted.scala b/project/Scripted.scala index ccb61129a..462a53778 100644 --- a/project/Scripted.scala +++ b/project/Scripted.scala @@ -40,9 +40,11 @@ object Scripted { case ("classloader-cache", "jni") => true // no native lib is built for windows case ("classloader-cache", "spark") => true // the test spark server is unable to bind to a local socket on Visual Studio 2019 - case ("nio", "make-clone") => true // uses gcc which isn't set up on all systems - case ("watch", "symlinks") => true // symlinks don't work the same on windows - case _ => false + case ("nio", "make-clone") => true // uses gcc which isn't set up on all systems + case ("watch", "symlinks") => true // symlinks don't work the same on windows + case ("cache", "compile-io-failure") => + true // a read-only dir doesn't block writes on windows, so the I/O failure won't reproduce + case _ => false } ) else NothingFilter diff --git a/sbt-app/src/sbt-test/cache/compile-io-failure/Main.scala b/sbt-app/src/sbt-test/cache/compile-io-failure/Main.scala new file mode 100644 index 000000000..f2a3fb3f0 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/compile-io-failure/Main.scala @@ -0,0 +1,2 @@ +object Main: + def main(args: Array[String]): Unit = () diff --git a/sbt-app/src/sbt-test/cache/compile-io-failure/build.sbt b/sbt-app/src/sbt-test/cache/compile-io-failure/build.sbt new file mode 100644 index 000000000..836e6a8d9 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/compile-io-failure/build.sbt @@ -0,0 +1,15 @@ +Global / localCacheDirectory := baseDirectory.value / "diskcache" +scalaVersion := "3.8.4" + +val denyClassDir = taskKey[Unit]("Remove write permission from the compile output dir") +denyClassDir := { + val dir = (Compile / classDirectory).value + IO.createDirectory(dir) + assert(dir.setWritable(false, false), s"could not clear write permission on $dir") +} + +val allowClassDir = taskKey[Unit]("Restore write permission on the compile output dir") +allowClassDir := { + val dir = (Compile / classDirectory).value + assert(dir.setWritable(true, false), s"could not restore write permission on $dir") +} diff --git a/sbt-app/src/sbt-test/cache/compile-io-failure/test b/sbt-app/src/sbt-test/cache/compile-io-failure/test new file mode 100644 index 000000000..2d486eb2c --- /dev/null +++ b/sbt-app/src/sbt-test/cache/compile-io-failure/test @@ -0,0 +1,8 @@ +# sbt/sbt#9455: an environmental I/O compile failure must not be cached and replayed forever. +# Make the compile output dir read-only so compile fails with an "error writing" I/O error. +> denyClassDir +-> compile +# Heal the environment. The sources are unchanged, so without the fix the cached environmental +# failure is replayed and this compile fails; with the fix the compile actually runs and succeeds. +> allowClassDir +> compile diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 376ed87d9..79f9c3cd2 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala @@ -99,7 +99,10 @@ object ActionCache: try action(key): @unchecked catch case e: CompileFailed => - cacheFailure(e) + if CachedCompileFailure.hasSufficientInfo(e) then cacheFailure(e) + else + cacheEventLog.append(ActionCacheEvent.OnsiteTask) + throw e case e: Exception => cacheEventLog.append(ActionCacheEvent.Error) logExec( @@ -171,7 +174,7 @@ object ActionCache: ) ) value - case Left(Some(failure)) => + case Left(Some(failure)) if CachedCompileFailure.hasSufficientInfo(failure.toException) => config.cacheEventLog.append(ActionCacheEvent.Found("cached-failure")) // Replay problems to the logger so users see the cached errors/warnings failure.replay(config.logger) @@ -179,7 +182,7 @@ object ActionCache: SpawnExec(input = spawnInput, cacheHit = true, exitCode = 1) ) throw failure.toException - case Left(None) => organicTask + case Left(_) => organicTask end cache /** @@ -297,7 +300,7 @@ object ActionCache: ) config.store.get(getRequest) - private inline def mkInput[I: HashWriter]( + private[sbt] inline def mkInput[I: HashWriter]( key: I, codeContentHash: Digest, extraHash: Digest, @@ -331,7 +334,7 @@ object ActionCache: case e: NoSuchFileException => Option(e.getFile) case _ => findMissingFile(t.getCause) - private inline def mkValuePath(inputDigest: Digest): String = + private[sbt] inline def mkValuePath(inputDigest: Digest): String = s"$${OUT}/value/${inputDigest}.json" def manifestFromFile(manifest: Path): Manifest = diff --git a/util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala b/util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala index fc55bd681..83fcfa4e6 100644 --- a/util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala +++ b/util-cache/src/main/scala/sbt/util/CachedCompileFailure.scala @@ -59,16 +59,16 @@ object CachedCompileFailure 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. + * Whether a compile failure carries enough source information to be safely cached and replayed. + * It is a function of the sources only when every error has a source position; a position-less + * error such as "error writing X.class" is an environmental I/O fault, and caching it replays a + * stale failure on every later build until the global cache is cleared by hand (sbt/sbt#9455). + * Such failures are treated as not cacheable. */ 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) + val errors = e.problems().filter(_.severity == Severity.Error) + errors.nonEmpty && errors.forall(p => toOption(p.position.sourcePath).isDefined) def fromException(e: CompileFailed): CachedCompileFailure = CachedCompileFailure( diff --git a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala index 5978aeee5..03c858829 100644 --- a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala +++ b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala @@ -222,6 +222,108 @@ object ActionCacheTest extends BasicTestSuite: assert(caught2.problems()(0).message() == "Test error message") assert(caught2.getMessage() == "Compilation failed") + test("Disk cache does not cache an environmental (position-less) CompileFailed"): + withDiskCache(testEnvironmentalCompileFailureNotCached) + + def testEnvironmentalCompileFailureNotCached(cache: DiskActionCacheStore): Unit = + import sjsonnew.BasicJsonProtocol.* + var called = 0 + // An I/O write failure surfaced by zinc as a compiler problem: an error with no source position. + val ioProblem = new Problem: + override def category(): String = "Test" + override def severity(): Severity = Severity.Error + override def message(): String = "error writing Meta$.class: AccessDeniedException" + override def position(): Position = new Position: + override def line(): Optional[Integer] = Optional.empty() + override def lineContent(): String = "" + 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.empty() + override def sourceFile(): Optional[java.io.File] = Optional.empty() + + val ioException = new CompileFailed: + override def arguments(): Array[String] = Array.empty + override def problems(): Array[Problem] = Array(ioProblem) + override def getMessage(): String = "Compilation failed" + + val action: ((Int, Int)) => InternalActionResult[Int] = { (_, _) => + called += 1 + throw ioException + } + IO.withTemporaryDirectory: tempDir => + val config = getCacheConfig(cache, tempDir) + + try + ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(false, "Expected CompileFailed to be thrown") + catch case _: CompileFailed => () + assert(called == 1) + + // The environment may have healed, so the second call must re-run the action rather than + // replay a stale cached failure. + try + ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(false, "Expected CompileFailed to be thrown") + catch case _: CompileFailed => () + assert( + called == 2, + s"environmental failure must not be cached; action should re-run (called=$called)" + ) + + test("A position-less failure cached by an older sbt is not replayed and is cured by a re-run"): + withDiskCache(testLegacyPoisonedFailureCured) + + def testLegacyPoisonedFailureCured(cache: DiskActionCacheStore): Unit = + import sjsonnew.BasicJsonProtocol.* + import sjsonnew.support.scalajson.unsafe.{ CompactPrinter, Converter } + var called = 0 + val ioProblem = new Problem: + override def category(): String = "Test" + override def severity(): Severity = Severity.Error + override def message(): String = "error writing Meta$.class: AccessDeniedException" + override def position(): Position = new Position: + override def line(): Optional[Integer] = Optional.empty() + override def lineContent(): String = "" + 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.empty() + override def sourceFile(): Optional[java.io.File] = Optional.empty() + val ioException = new CompileFailed: + override def arguments(): Array[String] = Array.empty + override def problems(): Array[Problem] = Array(ioProblem) + override def getMessage(): String = "Compilation failed" + + val action: ((Int, Int)) => InternalActionResult[Int] = { (a, b) => + called += 1 + InternalActionResult(a + b, Nil) + } + IO.withTemporaryDirectory: tempDir => + val config = getCacheConfig(cache, tempDir) + + // Write the poisoned entry exactly the way sbt cached failures before the gate existed. + val digest = ActionCache.mkInput((1, 1), Digest.zero, Digest.zero, config.cacheVersion) + val json = Converter.toJsonUnsafe(CachedCompileFailure.fromException(ioException)) + val failureFile = StringVirtualFile1(ActionCache.mkValuePath(digest), CompactPrinter(json)) + cache.put( + UpdateActionResultRequest( + digest, + Vector(failureFile), + exitCode = ActionCache.failureExitCode + ) + ) + + // The poison must not replay: the action runs and succeeds. + val v1 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v1 == 2) + assert(called == 1, s"poisoned failure must not replay; action should run (called=$called)") + + // The success overwrites the poison: the next call is a cache hit. + val v2 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v2 == 2) + assert(called == 1, s"expected a success cache hit after the cure (called=$called)") + test("Cache falls back to recompute when syncBlobs throws FileNotFoundException"): withDiskCache(testSyncBlobsThrowsFallback)