From 380a031cc798c2cf7633a55b4e9d06fe56b00e98 Mon Sep 17 00:00:00 2001 From: BrianHotopp Date: Wed, 15 Jul 2026 21:14:20 -0400 Subject: [PATCH 01/12] [2.0.x] fix: Propagate -java-home to a server the thin client starts (#9448) The thin client (sbtn) parsed the launcher value flags (-java-home, -mem, -jvm-debug, -sbt-dir, ...) but then dropped them. The space form was consumed and discarded; the flag=value form fell through to the residual arguments and was forwarded to the server verbatim, where --java-home=/path was rejected as a command (`Not a valid command: --`). When the client had to start a server (none running, no --server), the consumed -java-home never reached the forked sbt launcher, so the server came up under the default JVM and, in CI where the intended JDK is only reachable via -java-home, failed to connect. parseArgs now captures the consumed launcher value flags (both `flag value` and `flag=value`) into Arguments.launcherValueArgs, and the cold-start fork re-passes them to the sbt launcher so the server runs under the requested JVM. The client tokenizes arguments by splitting on whitespace, which would otherwise fragment a value that contains spaces (a Windows path like C:\Program Files\Java); parseArgs tracks those split boundaries and rejoins a value flag's value. An empty flag= value and a dangling flag with no value are consumed but not propagated, since the launcher's require_arg would otherwise fail the fork. The fork command construction is extracted into a pure, package-visible serverCommand so a test can assert the propagated flag reaches the started server. The sbt-launch-jar path is unchanged: it invokes java directly, with no launcher to interpret the flag. Fixes #9418 Co-authored-by: Claude Opus 4.8 (1M context) --- .../sbt/internal/client/NetworkClient.scala | 97 +++++++++++++------ .../client/NetworkClientParseArgsTest.scala | 65 +++++++++++++ notes/2.0.0/thin-client-java-home.md | 11 +++ 3 files changed, 143 insertions(+), 30 deletions(-) create mode 100644 notes/2.0.0/thin-client-java-home.md diff --git a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala b/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala index 0db8ed66f..03cf6b165 100644 --- a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala +++ b/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala @@ -375,33 +375,19 @@ class NetworkClient( term.isSupershellEnabled ).mkString(",") - val cmd = arguments.sbtLaunchJar match { - case Some(lj) => - if (log) { - val sbtScript = if (Properties.isWin) "sbt.bat" else "sbt" - console.appendLog(Level.Warn, s"server is started using sbt-launch jar directly") - console.appendLog( - Level.Warn, - "this is not the recommended way: .sbtopts and .jvmopts files are not loaded and SBT_OPTS is ignored" - ) - console.appendLog( - Level.Warn, - s"either upgrade $sbtScript to its latest version or make sure it is accessible from $$PATH, and run 'sbt bspConfig'" - ) - } - val java = Option(Properties.javaHome) - .map { javaHome => - s"$javaHome/bin/java" - } - .getOrElse("java") - List(java) ++ arguments.sbtArguments.filterNot( - NetworkClient.emptyBuildFlags.contains - ) ++ - List("-jar", lj, DashDashDetachStdio, DashDashServer) - case _ => - List(arguments.sbtScript) ++ arguments.sbtArguments ++ - List(DashDashDetachStdio, DashDashServer) + if (log && arguments.sbtLaunchJar.isDefined) { + val sbtScript = if (Properties.isWin) "sbt.bat" else "sbt" + console.appendLog(Level.Warn, s"server is started using sbt-launch jar directly") + console.appendLog( + Level.Warn, + "this is not the recommended way: .sbtopts and .jvmopts files are not loaded and SBT_OPTS is ignored" + ) + console.appendLog( + Level.Warn, + s"either upgrade $sbtScript to its latest version or make sure it is accessible from $$PATH, and run 'sbt bspConfig'" + ) } + val cmd = NetworkClient.serverCommand(arguments) // https://github.com/sbt/sbt/issues/6271 val nohup = @@ -1202,6 +1188,7 @@ object NetworkClient { val sbtScript: String, val bsp: Boolean, val sbtLaunchJar: Option[String], + val launcherValueArgs: Seq[String] = Nil, ) { def withBaseDirectory(file: File): Arguments = new Arguments( @@ -1212,8 +1199,20 @@ object NetworkClient { sbtScript, bsp, sbtLaunchJar, + launcherValueArgs, ) } + private[client] def serverCommand(arguments: Arguments): List[String] = + arguments.sbtLaunchJar match { + case Some(lj) => + val java = + Option(Properties.javaHome).map(javaHome => s"$javaHome/bin/java").getOrElse("java") + List(java) ++ arguments.sbtArguments.filterNot(emptyBuildFlags.contains) ++ + List("-jar", lj, DashDashDetachStdio, DashDashServer) + case _ => + List(arguments.sbtScript) ++ arguments.launcherValueArgs ++ arguments.sbtArguments ++ + List(DashDashDetachStdio, DashDashServer) + } private[client] val completions = "--completions" private[client] val noTab = "--no-tab" private[client] val noStdErr = "--no-stderr" @@ -1291,6 +1290,8 @@ object NetworkClient { "--autostart=", "-autostart=", ) + private[client] val launcherValueEqPrefixes: Seq[String] = + launcherValueFlags.toSeq.map(_ + "=") private[client] def parseArgs(args: Array[String]): Arguments = { val defaultSbtScript = if (Properties.isWin) "sbt.bat" else "sbt" var sbtScript = Properties.propOrNone("sbt.script") @@ -1299,10 +1300,32 @@ object NetworkClient { val commandArgs = new mutable.ArrayBuffer[String] val sbtArguments = new mutable.ArrayBuffer[String] val completionArguments = new mutable.ArrayBuffer[String] + val launcherValueArgs = new mutable.ArrayBuffer[String] val SysProp = "-D([^=]+)=(.*)".r - val sanitized = args.flatMap { - case a if a.startsWith("\"") => Array(a) - case a => a.split(" ") + val sanitized = new mutable.ArrayBuffer[String] + val splitFromPrev = new mutable.ArrayBuffer[Boolean] + args.foreach { + case a if a.startsWith("\"") => + sanitized += a + splitFromPrev += false + case a => + var first = true + a.split(" ").foreach { part => + if (part.nonEmpty) { + sanitized += part + splitFromPrev += !first + first = false + } + } + } + def valueFrom(start: Int): (String, Int) = { + var last = start + val sb = new StringBuilder(sanitized(start)) + while (last + 1 < sanitized.length && splitFromPrev(last + 1)) { + last += 1 + sb.append(" ").append(sanitized(last)) + } + (sb.toString, last) } var i = 0 while (i < sanitized.length) { @@ -1337,7 +1360,20 @@ object NetworkClient { case a if a.startsWith("-autostart=") => System.setProperty("sbt.server.autostart", a.stripPrefix("-autostart=")) case a if launcherValueFlags.contains(a) => - if (i + 1 < sanitized.length) i += 1 + if (i + 1 < sanitized.length) { + launcherValueArgs += a + val (value, last) = valueFrom(i + 1) + launcherValueArgs += value + i = last + } + case a if launcherValueEqPrefixes.exists(p => a.startsWith(p)) => + val (full, last) = valueFrom(i) + i = last + val eq = full.indexOf('=') + if (eq < full.length - 1) { + launcherValueArgs += full.substring(0, eq) + launcherValueArgs += full.substring(eq + 1) + } case a if launcherNoValueFlags.contains(a) => () case a if launcherEqPrefixes.exists(p => a.startsWith(p)) => () case a if a.startsWith("-J") => () @@ -1364,6 +1400,7 @@ object NetworkClient { sbtScript.getOrElse(defaultSbtScript).replace("%20", " "), bsp, launchJar, + launcherValueArgs.toSeq, ) } diff --git a/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala b/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala index 4cb18c8b8..c6f1448b0 100644 --- a/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala +++ b/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala @@ -184,6 +184,7 @@ object NetworkClientParseArgsTest extends BasicTestSuite: assert(!result.sbtArguments.contains("-mem")) assert(!result.sbtArguments.contains("10000")) assert(result.sbtArguments.exists(_.contains("-Dfoo=bar"))) + assert(result.launcherValueArgs == Seq("-mem", "10000")) assert(result.commandArguments.contains("compile")) assert(result.commandArguments.contains("test")) @@ -196,7 +197,71 @@ object NetworkClientParseArgsTest extends BasicTestSuite: assert(!result.sbtArguments.contains("/jdk")) assert(!result.sbtArguments.exists(_.contains("color=never"))) assert(result.sbtArguments.exists(_.contains("-Dfoo=bar"))) + assert(result.launcherValueArgs == Seq("-java-home", "/jdk")) assert(result.commandArguments.contains("compile")) assert(result.commandArguments.size == 1) + // -- Launcher value flags are captured for a forked server (#9418) -- + + test("-java-home /path is captured in launcherValueArgs for propagation"): + val result = parse("-java-home", "/path/to/jdk", "compile") + assert(result.launcherValueArgs == Seq("-java-home", "/path/to/jdk")) + + test("--java-home=/path is consumed, not forwarded, and captured as flag + value"): + val result = parse("--java-home=/path/to/jdk", "compile") + assert(!result.sbtArguments.exists(_.contains("java-home"))) + assert(!result.commandArguments.exists(_.contains("java-home"))) + assert(result.launcherValueArgs == Seq("--java-home", "/path/to/jdk")) + assert(result.commandArguments.contains("compile")) + + test("--java-home=/path does not leak a bare -- into forwarded args"): + val result = parse("--java-home=/usr/lib/jvm/java-17", "scalafmtCheckAll") + assert(!result.sbtArguments.exists(_.startsWith("--java-home"))) + assert(result.commandArguments == Seq("scalafmtCheckAll")) + + test("-mem=10000 eq-form is consumed and captured as flag + value"): + val result = parse("-mem=10000", "compile") + assert(!result.sbtArguments.exists(_.contains("mem"))) + assert(result.launcherValueArgs == Seq("-mem", "10000")) + assert(result.commandArguments.contains("compile")) + + test("--java-home= with an empty value is consumed but not propagated"): + val result = parse("--java-home=", "compile") + assert(!result.sbtArguments.exists(_.contains("java-home"))) + assert(result.launcherValueArgs.isEmpty) + assert(result.commandArguments == Seq("compile")) + + test("-java-home with a spaced path keeps the path intact"): + val result = parse("-java-home", "C:\\Program Files\\Java\\jdk-17", "compile") + assert(result.launcherValueArgs == Seq("-java-home", "C:\\Program Files\\Java\\jdk-17")) + assert(result.commandArguments == Seq("compile")) + + test("--java-home= with a spaced path keeps the path intact"): + val result = parse("--java-home=C:\\Program Files\\Java\\jdk-17", "compile") + assert(result.launcherValueArgs == Seq("--java-home", "C:\\Program Files\\Java\\jdk-17")) + assert(result.commandArguments == Seq("compile")) + + test("a single arg joining a flag and its value is still split and captured"): + val result = parse("-mem 10000", "compile") + assert(result.launcherValueArgs == Seq("-mem", "10000")) + assert(result.commandArguments == Seq("compile")) + + test("serverCommand propagates -java-home to the forked server before --server"): + val args = parse("-java-home", "/opt/jdk17", "compile") + val cmd = NetworkClient.serverCommand(args) + val jh = cmd.indexOf("-java-home") + assert(jh >= 0, cmd.toString) + assert(cmd(jh + 1) == "/opt/jdk17", cmd.toString) + assert(jh < cmd.indexOf("--server"), cmd.toString) + + test("a value flag with no value is dropped, not propagated as a dangling flag"): + val result = parse("-java-home") + assert(result.launcherValueArgs.isEmpty) + assert(result.commandArguments.isEmpty) + + test("extra whitespace in a value does not leave a stray leading space"): + val result = parse("-mem", " 10000", "compile") + assert(result.launcherValueArgs == Seq("-mem", "10000")) + assert(result.commandArguments == Seq("compile")) + end NetworkClientParseArgsTest diff --git a/notes/2.0.0/thin-client-java-home.md b/notes/2.0.0/thin-client-java-home.md new file mode 100644 index 000000000..c0d8f6edb --- /dev/null +++ b/notes/2.0.0/thin-client-java-home.md @@ -0,0 +1,11 @@ +### The thin client honors `-java-home` when it starts a server + +Launcher value flags such as `-java-home` were parsed by the thin client but then +dropped: the `=` form (`--java-home=/path`) was forwarded to the server verbatim +and rejected as a command (`Not a valid command: --`), and the space form +(`-java-home /path`) was silently discarded when the client had to start a server, +so that server came up under the default JVM. Both forms are now consumed by the +client and re-passed to a server it starts, so the server runs under the requested +JVM, including values that contain spaces such as a Windows path +(`C:\Program Files\Java\...`). This also applies to the other launcher value flags +(`-mem`, `-jvm-debug`, `-sbt-dir`, ...), which were dropped the same way. From 8d2219e90652c007b1ad69eb24d43fcb206b5478 Mon Sep 17 00:00:00 2001 From: Stas Shevchenko Date: Thu, 23 Jul 2026 01:28:59 +0200 Subject: [PATCH 02/12] [2.x] fix: Fixes partial cache restoration (#9488) A genuinely broken restore now degrades to the onsite task instead of a silent cache hit with incomplete outputs. Related to #9349. Co-authored-by: sshevchenko --- .../src/main/scala/sbt/util/ActionCache.scala | 4 +- .../scala/sbt/util/ActionCacheStore.scala | 19 ++-- .../test/scala/sbt/util/ActionCacheTest.scala | 104 +++++++++++++++++- 3 files changed, 116 insertions(+), 11 deletions(-) diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 79f9c3cd2..143de2904 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala @@ -8,7 +8,7 @@ package sbt.util -import java.io.{ File, IOException, PrintWriter } +import java.io.{ File, PrintWriter } import java.nio.charset.StandardCharsets import java.nio.file.{ AtomicMoveNotSupportedException, @@ -147,7 +147,7 @@ object ActionCache: result case Left(e) => throw e catch - case e: IOException => + case NonFatal(e) => logger.debug(s"Skipping cache storage due to error: ${e.getMessage}") cacheEventLog.append(ActionCacheEvent.Error) result diff --git a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala index 1ce0ec9cf..6d8624a5e 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala @@ -285,14 +285,17 @@ case class DiskActionCacheStore(base: Path, converter: FileConverter) override def syncBlobs(refs: Seq[HashedVirtualFileRef], outputDirectory: Path): Seq[Path] = refs.flatMap: r => - try - val casFile = toCasFile(Digest(r)) - if isCompleteBlob(casFile, Digest(r)) then - // println(s"syncBlobs: $casFile exists for $r") - Some(syncFile(r, casFile, outputDirectory)) - else None - // Digest(r) can throw NoSuchFileException - catch case _: NoSuchFileException => None + // Only the blob-availability lookup may swallow NoSuchFileException (Digest(r) can throw it): + // an absent blob is a cache miss for that entry. A write failure from syncFile, however, must + // propagate so the caller can degrade to the onsite task (sbt/sbt#8890) instead of silently + // leaving the output tree incomplete (sbt/sbt#9349). + val casFileOpt = + try + val digest = Digest(r) + val casFile = toCasFile(digest) + if isCompleteBlob(casFile, digest) then Some(casFile) else None + catch case _: NoSuchFileException => None + casFileOpt.map(syncFile(r, _, outputDirectory)) def syncFile(ref: HashedVirtualFileRef, casFile: Path, outputDirectory: Path): Path = val d = Digest(ref) diff --git a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala index 03c858829..eccc0e487 100644 --- a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala +++ b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala @@ -444,6 +444,90 @@ object ActionCacheTest extends BasicTestSuite: assert(Files.size(zipPath) > 0L) finally pool.shutdown() + // See https://github.com/sbt/sbt/issues/9349. With a warm cache, deleting the output tree + // out-of-band must not break the next build: restore recreates the missing parent directories. + test("Restore recreates a deleted output directory (direct blob)"): + IO.withTemporaryDirectory: cacheDir => + IO.withTemporaryDirectory: outDir => + val conv = fileConverter + val cache = DiskActionCacheStore(cacheDir.toPath, conv) + val nested = outDir.toPath.resolve("classes/pkg/A.txt") + val blob = StringVirtualFile1(nested.toString, "compiled") + val refs = cache.putBlobs(Seq(blob)) + cache.syncBlobs(refs, outDir.toPath) + assert(Files.exists(nested), "first sync should create the file") + IO.delete(outDir.toPath.resolve("classes").toFile()) + assert(!Files.exists(nested.getParent)) + cache.syncBlobs(refs, outDir.toPath) + assert(Files.exists(nested), "restore must recreate the deleted parent + file") + + test("Restore recreates a deleted output directory on a cache hit (dirzip)"): + import sjsonnew.BasicJsonProtocol.* + IO.withTemporaryDirectory: cacheDir => + IO.withTemporaryDirectory: outDir => + val conv = binaryConverter + val cache = DiskActionCacheStore(cacheDir.toPath, conv) + val classesDir = outDir.toPath.resolve("classes") + val classFile = classesDir.resolve("pkg/A.class") + var called = 0 + val action: Unit => InternalActionResult[Int] = { _ => + called += 1 + Files.createDirectories(classFile.getParent) + Files.writeString(classFile, "compiled") + val dirzip = + ActionCache.packageDirectory( + VirtualFileRef.of(classesDir.toString), + conv, + outDir.toPath + ) + InternalActionResult(1, Seq(dirzip)) + } + val config = getCacheConfig(cache, outDir, converter = conv) + val v1 = ActionCache.cache((), Digest.zero, Digest.zero, tags, config)(action) + assert(v1 == 1) + assert(called == 1) + assert(Files.exists(classFile)) + IO.delete(outDir.toPath.toFile()) + assert(!Files.exists(classesDir)) + val v2 = ActionCache.cache((), Digest.zero, Digest.zero, tags, config)(action) + assert(v2 == 1) + assert( + Files.exists(classFile), + s"class file must be restored after output dir deletion (called=$called)" + ) + + // A genuinely broken restore (syncFile write fails) must degrade to the onsite task rather than + // being silently swallowed by syncBlobs, which would report a cache hit with incomplete outputs. + test("Restore degrades to onsite recompute when materialising throws NoSuchFileException"): + testBrokenRestoreDegrades(r => new NoSuchFileException(r.toString)) + + test("Restore degrades to onsite recompute when materialising throws a non-IOException"): + testBrokenRestoreDegrades(_ => new RuntimeException("disk exploded")) + + def testBrokenRestoreDegrades(mkError: HashedVirtualFileRef => Throwable): Unit = + import sjsonnew.BasicJsonProtocol.* + IO.withTemporaryDirectory: cacheDir => + IO.withTemporaryDirectory: outDir => + val broken = new DiskActionCacheStore(cacheDir.toPath, fileConverter): + override def syncFile( + ref: HashedVirtualFileRef, + casFile: Path, + outputDirectory: Path, + ): Path = throw mkError(ref) + var called = 0 + val action: ((Int, Int)) => InternalActionResult[Int] = { (a, b) => + called += 1 + val out = StringVirtualFile1(s"$outDir/a.txt", (a + b).toString) + InternalActionResult(a + b, Seq(out)) + } + val config = getCacheConfig(broken, outDir) + val v1 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v1 == 2) + assert(called == 1) + val v2 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v2 == 2) + assert(called == 2, s"broken restore must degrade to onsite recompute (called=$called)") + test("Changing cacheVersion invalidates the cache"): withDiskCache(testCacheVersionInvalidation) @@ -492,6 +576,7 @@ object ActionCacheTest extends BasicTestSuite: cache: ActionCacheStore, outputDir: File, cacheVersion: Long = 0L, + converter: FileConverter = fileConverter, ): BuildWideCacheConfiguration = val logger = new Logger: override def trace(t: => Throwable): Unit = () @@ -500,7 +585,7 @@ object ActionCacheTest extends BasicTestSuite: BuildWideCacheConfiguration( cache, outputDir.toPath(), - fileConverter, + converter, logger, CacheEventLog(), CacheImplicits.defaultLocalDigestCacheByteSize, @@ -512,4 +597,21 @@ object ActionCacheTest extends BasicTestSuite: override def toVirtualFile(path: Path): VirtualFile = val content = if Files.isRegularFile(path) then new String(Files.readAllBytes(path)) else "" StringVirtualFile1(path.toString, content) + + // A converter whose VirtualFiles read raw bytes off disk, so binary blobs (e.g. dirzips) survive + // the put/sync round-trip instead of being mangled by a String round-trip. + def binaryConverter = new FileConverter: + override def toPath(ref: VirtualFileRef): Path = Paths.get(ref.id) + override def toVirtualFile(path: Path): VirtualFile = DiskVirtualFile(path.toString) + + final class DiskVirtualFile(path: String) + extends xsbti.BasicVirtualFileRef(path) + with VirtualFile: + private def bytes: Array[Byte] = + if Files.isRegularFile(Paths.get(path)) then Files.readAllBytes(Paths.get(path)) + else Array.emptyByteArray + override def contentHash: Long = HashUtil.xxhash64(bytes) + override def sizeBytes: Long = bytes.length.toLong + override def contentHashStr: String = Digest.sha256Hash(bytes).contentHashStr + override def input: InputStream = new java.io.ByteArrayInputStream(bytes) end ActionCacheTest From 90ce3b99f8df159eaadb8fd5970debfc570b1038 Mon Sep 17 00:00:00 2001 From: BrianHotopp Date: Thu, 23 Jul 2026 18:06:17 -0400 Subject: [PATCH 03/12] [2.x] fix: Re-extract a declared output directory on cache hit when it is missing (#9473) A directory declared via Def.declareOutputDirectory is packaged as a sibling .sbtdir.zip, so deleting the directory leaves the zip behind. On a cache hit, syncFile's up-to-date short-circuit saw the zip in sync (same digest, already a CAS symlink) and returned without the unpack side effect, which only ran from the file-write path: the directory was never restored. For sbt's own compile, whose classes directory is declared this way, rm -rf of the classes directory with a warm cache meant run failed with ClassNotFoundException and no recompile; only deleting the zip as well (or the whole cache) recovered. The up-to-date branch now re-extracts when the extracted directory itself is missing: a single stat on the warm path, per review preference over a manifest-based per-file check. Partial deletions inside a still-existing directory are not repaired, consistent with treating target/ contents as sbt-managed. Refs #9462 (the directory-restoration half; the declareOutput-in-a-loop half is a separate macro-layer issue) Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: eugene yokota --- notes/2.0.0/dirzip-restore-on-hit.md | 15 +++++ .../cache/compile-classes-restore/Main.scala | 2 + .../cache/compile-classes-restore/build.sbt | 53 ++++++++++++++++ .../cache/compile-classes-restore/test | 18 ++++++ .../declare-output-dir-restore/build.sbt | 60 +++++++++++++++++++ .../cache/declare-output-dir-restore/test | 16 +++++ .../scala/sbt/util/ActionCacheStore.scala | 14 ++++- .../test/scala/sbt/util/ActionCacheTest.scala | 38 +++++++++++- 8 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 notes/2.0.0/dirzip-restore-on-hit.md create mode 100644 sbt-app/src/sbt-test/cache/compile-classes-restore/Main.scala create mode 100644 sbt-app/src/sbt-test/cache/compile-classes-restore/build.sbt create mode 100644 sbt-app/src/sbt-test/cache/compile-classes-restore/test create mode 100644 sbt-app/src/sbt-test/cache/declare-output-dir-restore/build.sbt create mode 100644 sbt-app/src/sbt-test/cache/declare-output-dir-restore/test diff --git a/notes/2.0.0/dirzip-restore-on-hit.md b/notes/2.0.0/dirzip-restore-on-hit.md new file mode 100644 index 000000000..975d0f48b --- /dev/null +++ b/notes/2.0.0/dirzip-restore-on-hit.md @@ -0,0 +1,15 @@ +### Directories declared with `Def.declareOutputDirectory` are restored on cache hits + +A directory declared as a cached task's output is packaged as a `.sbtdir.zip` +sibling of the directory. Deleting the directory leaves the sibling zip behind +(an `rm -rf` of the directory or of `classes/` does exactly this), which left +the cache convinced everything was in sync: on the next cache hit the task did not re-run, but the +directory was never re-extracted either. For sbt's own `compile`, whose classes +directory is declared this way, a deleted output directory plus a warm cache +meant `run` failed with `ClassNotFoundException` and no recompile. The cache now +re-extracts a declared directory when the directory itself is missing, at the +cost of a single stat on the warm path. + +This addresses the directory-restoration half of [#9462][i9462]. + +[i9462]: https://github.com/sbt/sbt/issues/9462 diff --git a/sbt-app/src/sbt-test/cache/compile-classes-restore/Main.scala b/sbt-app/src/sbt-test/cache/compile-classes-restore/Main.scala new file mode 100644 index 000000000..368364e2b --- /dev/null +++ b/sbt-app/src/sbt-test/cache/compile-classes-restore/Main.scala @@ -0,0 +1,2 @@ +@main def main(args: String*) = + println("hello from probe") diff --git a/sbt-app/src/sbt-test/cache/compile-classes-restore/build.sbt b/sbt-app/src/sbt-test/cache/compile-classes-restore/build.sbt new file mode 100644 index 000000000..3335d1b29 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/compile-classes-restore/build.sbt @@ -0,0 +1,53 @@ +import sbt.internal.util.CacheEventSummary + +val checkCompileHit = taskKey[Unit]("asserts the previous command took cache hits") +val delClasses = taskKey[Unit]("deletes the classes directory") +val delClassesZip = taskKey[Unit]("deletes the sibling classes.sbtdir.zip") +val checkClasses = taskKey[Unit]("asserts .class files exist") +val checkNoClasses = taskKey[Unit]("asserts no .class files exist") + +Global / localCacheDirectory := baseDirectory.value / "diskcache" + +// A distinct project id keeps this fixture's output paths from colliding with same-named +// sibling fixtures in scripted's shared batch directory. +lazy val compileClassesRestore = project + .in(file(".")) + .settings( + scalaVersion := "3.8.4" + ) + +delClasses := Def.uncached { + val dir = (Compile / classDirectory).value + IO.delete(dir) + streams.value.log.info(s"deleted $dir") +} + +delClassesZip := Def.uncached { + val dir = (Compile / classDirectory).value + val zip = new java.io.File(dir.getParentFile, dir.getName + ".sbtdir.zip") + streams.value.log.info(s"deleting $zip (exists=${zip.exists})") + IO.delete(zip) +} + +checkClasses := Def.uncached { + val dir = (Compile / classDirectory).value + val classes = (dir ** "*.class").get() + streams.value.log.info(s"classes under $dir: ${classes.mkString(", ")}") + assert(classes.nonEmpty, s"no class files under $dir") +} + +checkNoClasses := Def.uncached { + val dir = (Compile / classDirectory).value + val classes = (dir ** "*.class").get() + streams.value.log.info(s"classes under $dir: ${classes.mkString(", ")}") + assert(classes.isEmpty, s"unexpected class files under $dir: ${classes.mkString(", ")}") +} + +checkCompileHit := Def.uncached { + val config = Def.cacheConfiguration.value + val prev = config.cacheEventLog.previous match + case s: CacheEventSummary.Data => s + case _ => sys.error("empty event log") + streams.value.log.info(s"prev hitCount=${prev.hitCount} missCount=${prev.missCount}") + assert(prev.hitCount >= 1, s"expected cache hits but hitCount=${prev.hitCount}") +} diff --git a/sbt-app/src/sbt-test/cache/compile-classes-restore/test b/sbt-app/src/sbt-test/cache/compile-classes-restore/test new file mode 100644 index 000000000..1ddc3bc71 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/compile-classes-restore/test @@ -0,0 +1,18 @@ +# Regression for #9462: compile's classes dir (declared via Def.declareOutputDirectory) +# must be restored on a cache hit after the classes directory is deleted. +> compile +> checkClasses +> run +> delClasses +> checkNoClasses +> compile +> checkCompileHit +> checkClasses +> run + +# restoration also works when the sibling classes.sbtdir.zip is gone too +> delClasses +> delClassesZip +> compile +> checkClasses +> run diff --git a/sbt-app/src/sbt-test/cache/declare-output-dir-restore/build.sbt b/sbt-app/src/sbt-test/cache/declare-output-dir-restore/build.sbt new file mode 100644 index 000000000..2b3fa4f6c --- /dev/null +++ b/sbt-app/src/sbt-test/cache/declare-output-dir-restore/build.sbt @@ -0,0 +1,60 @@ +import sbt.internal.util.CacheEventSummary +import xsbti.HashedVirtualFileRef + +val declareDir = taskKey[HashedVirtualFileRef]("writes 2 files into a dir and declares the dir") +val checkFiles = taskKey[Unit]("asserts both files exist") +val checkGone = taskKey[Unit]("asserts the dir has no files") +val delDir = taskKey[Unit]("deletes the generated dir") +val delZip = taskKey[Unit]("deletes the sibling .sbtdir.zip") +val checkHit = taskKey[Unit]("asserts previous command was a pure cache hit") + +Global / localCacheDirectory := baseDirectory.value / "diskcache" + +lazy val declareOutputDirRestore = project.in(file(".")) + +declareDir := { + val log = streams.value.log + val dir = target.value / "gen-dir" + IO.createDirectory(dir) + IO.write(dir / "a.txt", "contents A") + IO.write(dir / "b.txt", "contents B") + log.info(s"COMPUTED declareDir (cache miss)") + val vf = fileConverter.value.toVirtualFile(dir.toPath) + Def.declareOutputDirectory(vf) +} + +checkFiles := Def.uncached { + val log = streams.value.log + val dir = target.value / "gen-dir" + val listing = if (dir.exists) (dir ** "*").get().mkString(", ") else "" + log.info(s"gen-dir listing: $listing") + assert((dir / "a.txt").exists, s"a.txt missing under $dir") + assert((dir / "b.txt").exists, s"b.txt missing under $dir") +} + +checkGone := Def.uncached { + val dir = target.value / "gen-dir" + assert(!(dir / "a.txt").exists && !(dir / "b.txt").exists, s"files still present under $dir") +} + +delDir := Def.uncached { + val dir = target.value / "gen-dir" + IO.delete(dir) + streams.value.log.info(s"deleted $dir") +} + +delZip := Def.uncached { + val zip = new java.io.File(target.value, "gen-dir.sbtdir.zip") + streams.value.log.info(s"deleting $zip (exists=${zip.exists})") + IO.delete(zip) +} + +checkHit := Def.uncached { + val config = Def.cacheConfiguration.value + val prev = config.cacheEventLog.previous match + case s: CacheEventSummary.Data => s + case _ => sys.error("empty event log") + streams.value.log.info(s"prev hitCount=${prev.hitCount} missCount=${prev.missCount}") + assert(prev.missCount == 0, s"expected pure hit but missCount=${prev.missCount}") + assert(prev.hitCount >= 1, s"expected a hit but hitCount=${prev.hitCount}") +} diff --git a/sbt-app/src/sbt-test/cache/declare-output-dir-restore/test b/sbt-app/src/sbt-test/cache/declare-output-dir-restore/test new file mode 100644 index 000000000..79b8154d3 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/declare-output-dir-restore/test @@ -0,0 +1,16 @@ +# Regression for #9462: a Def.declareOutputDirectory dir must be restored on a cache hit +# after the extracted directory is deleted (the sibling .sbtdir.zip survives an rm -rf). +> declareDir +> checkFiles +> delDir +> checkGone +> declareDir +> checkHit +> checkFiles + +# restoration also works when the sibling zip itself is gone +> delDir +> delZip +> declareDir +> checkHit +> checkFiles diff --git a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala index 6d8624a5e..a386f1a1c 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala @@ -343,7 +343,9 @@ case class DiskActionCacheStore(base: Path, converter: FileConverter) try // `!symlinkSupported` prevents unnecessary deletion of files and then copying them again // in #writeFileAndNotify on machines that don't support symlinks. - if Digest.sameDigest(p, d) && (!symlinkSupported.get() || Files.isSymbolicLink(p)) then p + if Digest.sameDigest(p, d) && (!symlinkSupported.get() || Files.isSymbolicLink(p)) then + afterFileUpToDate(ref, p, outputDirectory) + p else // println(s"- syncFile: $p has different digest") IO.delete(p.toFile()) @@ -361,6 +363,16 @@ case class DiskActionCacheStore(base: Path, converter: FileConverter) if path.toString().endsWith(ActionCache.dirZipExt) then unpackageDirZip(path, outputDirectory) else () + /** Re-extract a dirzip whose extracted directory is missing: one stat on the warm path. */ + private def afterFileUpToDate( + ref: HashedVirtualFileRef, + path: Path, + outputDirectory: Path + ): Unit = + if path.toString().endsWith(ActionCache.dirZipExt) then + val dirPath = Paths.get(path.toString.dropRight(ActionCache.dirZipExt.size)) + if !Files.isDirectory(dirPath) then Util.ignoreResult(unpackageDirZip(path, outputDirectory)) + /** * Given a dirzip, unzip it in a temp directory, and sync each items to the outputDirectory. */ diff --git a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala index eccc0e487..cb3f15987 100644 --- a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala +++ b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala @@ -1,6 +1,6 @@ package sbt.util -import java.io.{ IOException, InputStream } +import java.io.{ ByteArrayInputStream, IOException, InputStream } import java.nio.charset.StandardCharsets import java.nio.file.{ Files, NoSuchFileException, Path, Paths } import java.util.Optional @@ -88,6 +88,30 @@ object ActionCacheTest extends BasicTestSuite: index += 1 b + test("Disk cache re-extracts a dirzip whose directory was deleted"): + withDiskCache: cache => + IO.withTemporaryDirectory: tempDir => + val outputDirectory = tempDir.toPath() + val dir = tempDir / "gen-dir" + IO.write(dir / "a.txt", "contents A") + IO.write(dir / "b.txt", "contents B") + val zipVf = ActionCache.packageDirectory( + binaryFileConverter.toVirtualFile(dir.toPath()), + binaryFileConverter, + outputDirectory, + ) + val refs = cache.putBlobs(Seq(zipVf)) + assert(refs.size == 1) + + cache.syncBlobs(refs, outputDirectory) + assert((dir / "a.txt").exists && (dir / "b.txt").exists) + + IO.delete(dir) + assert(!dir.exists) + cache.syncBlobs(refs, outputDirectory) + assert((dir / "a.txt").exists, "a.txt not re-extracted after the directory was deleted") + assert((dir / "b.txt").exists, "b.txt not re-extracted after the directory was deleted") + test("In-memory cache can hold action value"): withInMemoryCache(testActionCacheBasic) @@ -592,6 +616,18 @@ object ActionCacheTest extends BasicTestSuite: cacheVersion, ) + // The String-based fileConverter mangles binary blobs (zips). + def binaryFileConverter = new FileConverter: + override def toPath(ref: VirtualFileRef): Path = Paths.get(ref.id) + override def toVirtualFile(path: Path): VirtualFile = + val bytes = + if Files.isRegularFile(path) then Files.readAllBytes(path) else Array.empty[Byte] + new xsbti.BasicVirtualFileRef(path.toString) with VirtualFile: + override def contentHash: Long = sbt.util.HashUtil.xxhash64(bytes) + override def sizeBytes: Long = bytes.length.toLong + override def contentHashStr: String = Digest.sha256Hash(bytes).contentHashStr + override def input: InputStream = new ByteArrayInputStream(bytes) + def fileConverter = new FileConverter: override def toPath(ref: VirtualFileRef): Path = Paths.get(ref.id) override def toVirtualFile(path: Path): VirtualFile = From c12181af7f5608a648d1a4481a257391b6ae8036 Mon Sep 17 00:00:00 2001 From: eugene yokota Date: Thu, 23 Jul 2026 20:16:54 -0400 Subject: [PATCH 04/12] [2.x] fix: Fixes common settings with extraProjects (#9495) **Problem** The presence of extraProjects broke common settings. **Solution** This fixes it by passing finalRoot.commonSettings. --- main/src/main/scala/sbt/internal/Load.scala | 2 +- .../extra-projects-key-aggregate/build.sbt | 25 +++++++++++++++++-- .../project/ExtraPlugin.scala | 16 +++++++++--- .../project/extra-projects-key-aggregate/test | 6 ++++- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/main/src/main/scala/sbt/internal/Load.scala b/main/src/main/scala/sbt/internal/Load.scala index ae8b16b9f..10db41e5c 100755 --- a/main/src/main/scala/sbt/internal/Load.scala +++ b/main/src/main/scala/sbt/internal/Load.scala @@ -1117,7 +1117,7 @@ private[sbt] object Load { val newProjects = rest ++ discovered ++ projectLevelExtra val newAcc = acc :+ finalRoot val newGenerated = generated ++ generatedConfigClassFiles - loadTransitive1(newProjects, newAcc, newGenerated, commonSettings) + loadTransitive1(newProjects, newAcc, newGenerated, finalRoot.commonSettings) } // Load all config files AND process the project at the root directory, if it exists. diff --git a/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/build.sbt b/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/build.sbt index 415a43679..412b9343f 100644 --- a/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/build.sbt +++ b/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/build.sbt @@ -6,11 +6,32 @@ * Licensed under Apache License 2.0 (see LICENSE) */ -val check = taskKey[Unit]("Repro for #4947: task at root when extraProjects creates auto root") +@transient +val check4947 = taskKey[Unit]("") + +@transient +val check5661 = taskKey[Unit]("") + +@transient +val check9493 = taskKey[Unit]("") + +organization := "com.example" val a = project val p = project .settings( name := "p", - check := () + // subproject-level task + check4947 := {}, + + check9493 := { + val o = organization.value + assert(o == "com.example", s"actual: $o") + } ) + +LocalProject("mc") / cantTouchThis := "foo" +LocalRootProject / check5661 := { + val actual = (LocalProject("mc") / cantTouchThis).value + assert(actual == "foo", s"actual: $actual") +} diff --git a/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/project/ExtraPlugin.scala b/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/project/ExtraPlugin.scala index 77ff44de2..4649e8d26 100644 --- a/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/project/ExtraPlugin.scala +++ b/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/project/ExtraPlugin.scala @@ -6,10 +6,18 @@ * Licensed under Apache License 2.0 (see LICENSE) */ -import sbt._, Keys._ +import sbt.*, Keys.* + +object ExtraPlugin extends AutoPlugin: + object autoImport: + val cantTouchThis = settingKey[String]("") + end autoImport + import autoImport.* -object ExtraPlugin extends AutoPlugin { override def trigger = allRequirements override def extraProjects: Seq[Project] = - Seq(Project("z", file("z")).settings(name := "z")) -} + Seq(Project("mc", file("mc")).settings( + name := "mc", + cantTouchThis := "can't touch this", + )) +end ExtraPlugin diff --git a/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/test b/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/test index 15675b169..5dd83bb78 100644 --- a/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/test +++ b/sbt-app/src/sbt-test/project/extra-projects-key-aggregate/test @@ -1 +1,5 @@ -> check +> check4947 + +> check5661 + +> p/check9493 From f026f09c8996bcfd7b9a86032a94ed096502a263 Mon Sep 17 00:00:00 2001 From: eugene yokota Date: Thu, 23 Jul 2026 22:30:16 -0400 Subject: [PATCH 05/12] [2.x] clean task cleans sona-staging (#9479) **Problem/Solution** 1. clean task cleans sona-staging directory. 2. cleanFull calls clean task. --- main/src/main/scala/sbt/Defaults.scala | 1 + main/src/main/scala/sbt/internal/Clean.scala | 2 ++ main/src/main/scala/sbt/plugins/IvyPlugin.scala | 10 ++++++++++ .../src/sbt-test/actions/clean-sona-staging/build.sbt | 1 + sbt-app/src/sbt-test/actions/clean-sona-staging/test | 7 +++++++ 5 files changed, 21 insertions(+) create mode 100644 sbt-app/src/sbt-test/actions/clean-sona-staging/build.sbt create mode 100644 sbt-app/src/sbt-test/actions/clean-sona-staging/test diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 1b3e823c4..e704dd593 100644 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -762,6 +762,7 @@ object Defaults extends BuildCommon { case NonFatal(_) => () } clean.value + (ThisBuild / publish / clean).value }, scalaCompilerBridgeBin := Def .ifS(Def.task { diff --git a/main/src/main/scala/sbt/internal/Clean.scala b/main/src/main/scala/sbt/internal/Clean.scala index 4b1ab49fc..c7c8f8fe4 100644 --- a/main/src/main/scala/sbt/internal/Clean.scala +++ b/main/src/main/scala/sbt/internal/Clean.scala @@ -218,6 +218,7 @@ private[sbt] object Clean { val h = Help.more(CleanFull, cleanFullDetailed) val expunge: State => State = (s: State) => + import UpperStateOps.* val outputDirectory = s .get(BasicKeys.rootOutputDirectory) .getOrElse(sys.error("outputDirectory has not been set")) @@ -225,6 +226,7 @@ private[sbt] object Clean { cacheStore.foreach: case d: DiskActionCacheStore => d.clear() case _ => () + val s2 = s.unsafeRunAggregated(LocalRootProject / clean) IO.delete(outputDirectory.toFile()) s Command.command(CleanFull, h)(expunge andThen clearCachesFun) diff --git a/main/src/main/scala/sbt/plugins/IvyPlugin.scala b/main/src/main/scala/sbt/plugins/IvyPlugin.scala index 2d4a31b58..7eb9fad43 100644 --- a/main/src/main/scala/sbt/plugins/IvyPlugin.scala +++ b/main/src/main/scala/sbt/plugins/IvyPlugin.scala @@ -10,6 +10,9 @@ package sbt package plugins import Def.Setting +import Keys.* +import sbt.io.IO +import sbt.io.syntax.* /** * Plugin that enables resolving artifacts via ivy. @@ -29,6 +32,13 @@ object IvyPlugin extends AutoPlugin { override lazy val globalSettings: Seq[Setting[?]] = Defaults.globalIvyCore + override lazy val buildSettings: Seq[Setting[?]] = + Seq( + publish / clean := Def.uncached { + IO.delete(stagingDirectory.value) + IO.delete((ThisBuild / baseDirectory).value / "target" / "sona-bundle") + }, + ) override lazy val projectSettings: Seq[Setting[?]] = Classpaths.ivyPublishSettings ++ Classpaths.ivyBaseSettings diff --git a/sbt-app/src/sbt-test/actions/clean-sona-staging/build.sbt b/sbt-app/src/sbt-test/actions/clean-sona-staging/build.sbt new file mode 100644 index 000000000..86008ba9c --- /dev/null +++ b/sbt-app/src/sbt-test/actions/clean-sona-staging/build.sbt @@ -0,0 +1 @@ +name := "clean-sona-staging" diff --git a/sbt-app/src/sbt-test/actions/clean-sona-staging/test b/sbt-app/src/sbt-test/actions/clean-sona-staging/test new file mode 100644 index 000000000..0dd44b027 --- /dev/null +++ b/sbt-app/src/sbt-test/actions/clean-sona-staging/test @@ -0,0 +1,7 @@ +$ touch target/sona-staging/dummy.txt +$ touch target/sona-bundle/dummy.txt + +> clean + +$ absent target/sona-staging/dummy.txt +$ absent target/sona-bundle/dummy.txt From c9e94107a9677e109225fd91727909af07ebb8d8 Mon Sep 17 00:00:00 2001 From: eugene yokota Date: Fri, 24 Jul 2026 01:42:06 -0400 Subject: [PATCH 06/12] [2.x] Allow opt-out of transient warning (#9437) **Problem** We want to opt out of the transient key warning. **Solution** This implements an optout via nowarn annotation. --- build.sbt | 4 +++ .../sbt/internal/util/appmacro/Cont.scala | 13 ++++++-- .../internal/util/appmacro/ContextUtil.scala | 30 +++++++++++++++++++ .../warning-optout/changes/bad1.sbt | 8 +++++ .../warning-optout/changes/bad2.sbt | 7 +++++ .../warning-optout/changes/good.sbt | 7 +++++ .../warning-optout/changes/good2.sbt | 7 +++++ .../warning-optout/project/plugins.sbt | 1 + .../sbt-test/project-load/warning-optout/test | 17 +++++++++++ 9 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 sbt-app/src/sbt-test/project-load/warning-optout/changes/bad1.sbt create mode 100644 sbt-app/src/sbt-test/project-load/warning-optout/changes/bad2.sbt create mode 100644 sbt-app/src/sbt-test/project-load/warning-optout/changes/good.sbt create mode 100644 sbt-app/src/sbt-test/project-load/warning-optout/changes/good2.sbt create mode 100644 sbt-app/src/sbt-test/project-load/warning-optout/project/plugins.sbt create mode 100644 sbt-app/src/sbt-test/project-load/warning-optout/test diff --git a/build.sbt b/build.sbt index fd539bbb5..1ac01bf92 100644 --- a/build.sbt +++ b/build.sbt @@ -674,6 +674,10 @@ lazy val coreMacrosProj = (project in file("core-macros")) name := "Core Macros", SettingKey[Boolean]("exportPipelining") := false, mimaSettings, + mimaBinaryIssueFilters ++= Seq( + exclude[ReversedMissingMethodProblem]("sbt.internal.util.appmacro.ContextUtil.*"), + exclude[DirectMissingMethodProblem]("sbt.internal.util.appmacro.ContextUtil#Input.*"), + ), ) // Fixes scope=Scope for Setting (core defined in collectionProj) to define the settings system used in build definitions diff --git a/core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala b/core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala index 079277e52..2e34d4b9c 100644 --- a/core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala +++ b/core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala @@ -306,7 +306,7 @@ trait Cont: .asExprOf[BuildWideCacheConfiguration] inputs.foreach: input => if !input.isCacheInput then - if !Cont.transientAllowSet(input.sym.name) then + if !Cont.transientAllowSet(input.sym.name) && !input.isWarnSuppressed then report.warning( s"transient key ${input.sym.name} is excluded from the cache input" ) @@ -430,6 +430,7 @@ trait Cont: val WrapOutputName = "wrapOutput_\u2603\u2603" val WrapOutputDirectoryName = "wrapOutputDirectory_\u2603\u2603" + var nowarnQuals: Set[Term] = Set.empty // Called when transforming the tree to add an input. // For `qual` of type F[A], and a `selection` qual.value. val record = [a] => @@ -470,12 +471,18 @@ trait Cont: }.asTerm) case None => oldTree case _ => - // todo cache opt-out attribute - inputBuf += Input(TypeRepr.of[a], qual, replacement, freshName("q")) + inputBuf += Input( + TypeRepr.of[a], + qual, + replacement, + freshName("q"), + isWarnSuppressed = nowarnQuals.contains(qual), + ) oldTree } val exprWithConfig = cacheConfigExprOpt.map(config => '{ $config; $expr }).getOrElse(expr) + nowarnQuals = collectNowarnQuals(exprWithConfig.asTerm) val body = transformWrappers(exprWithConfig.asTerm, record, Symbol.spliceOwner) val r = inputBuf.toList match case Nil => pure(body) diff --git a/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala b/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala index b3547d3d6..2b65e4f76 100644 --- a/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala +++ b/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala @@ -76,11 +76,13 @@ trait ContextUtil[C <: Quotes & scala.Singleton](val valStart: Int): private val cacheLevelSym = Symbol.requiredClass("sbt.util.cacheLevel") private val transientSym = Symbol.requiredClass("scala.transient") + private val nowarnAnnotSym = Symbol.requiredClass("scala.annotation.nowarn") final class Input( val tpe: TypeRepr, val qual: Term, val term: Term, val name: String, + val isWarnSuppressed: Boolean, ): override def toString: String = s"Input($tpe, $qual, $term, $name, $tags)" @@ -194,6 +196,34 @@ trait ContextUtil[C <: Quotes & scala.Singleton](val valStart: Int): def idTransform[F[_]]: TermTransform[F] = in => in + def collectNowarnQuals(tree: Term): Set[Term] = + val result = mutable.HashSet[Term]() + @tailrec def extractQual(t: Term): Unit = t match + case Inlined(_, _, inner) => extractQual(inner) + case Typed(inner, _) => extractQual(inner) + case Apply(TypeApply(Select(_, _), _ :: Nil), qual :: Nil) => result += qual + case Apply(TypeApply(Ident(_), _ :: Nil), qual :: Nil) => result += qual + case _ => () + def targetsTransient(s: String) = s.isEmpty || s.startsWith("msg=transient") + object scanner extends TreeTraverser: + override def traverseTree(t: Tree)(owner: Symbol): Unit = t match + case Typed(inner, tpt) => + tpt.tpe match + case AnnotatedType(_, annot) if annot.tpe.typeSymbol == nowarnAnnotSym => + val isUnfiltered = annot match + case Apply(_, Nil) => true + case Apply(_, Literal(StringConstant(s)) :: Nil) => targetsTransient(s) + case Apply(_, NamedArg(_, Literal(StringConstant(s))) :: Nil) => targetsTransient(s) + case Apply(_, _ :: Nil) => true + case _ => false + if isUnfiltered then extractQual(inner) + case _ => + super.traverseTree(t)(owner) + case _ => super.traverseTree(t)(owner) + end scanner + scanner.traverseTree(tree)(Symbol.spliceOwner) + result.toSet + def collectDefs(tree: Term, isWrapper: (String, TypeRepr, Term) => Boolean): Set[Symbol] = val defs = mutable.HashSet[Symbol]() object traverser extends TreeTraverser: diff --git a/sbt-app/src/sbt-test/project-load/warning-optout/changes/bad1.sbt b/sbt-app/src/sbt-test/project-load/warning-optout/changes/bad1.sbt new file mode 100644 index 000000000..0d8f17286 --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/warning-optout/changes/bad1.sbt @@ -0,0 +1,8 @@ +import scala.annotation.nowarn + +lazy val check = taskKey[Unit]("") +check := { + val a = (state.value: @nowarn) + val b = state.value + println("hi") +} diff --git a/sbt-app/src/sbt-test/project-load/warning-optout/changes/bad2.sbt b/sbt-app/src/sbt-test/project-load/warning-optout/changes/bad2.sbt new file mode 100644 index 000000000..7ac6cd872 --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/warning-optout/changes/bad2.sbt @@ -0,0 +1,7 @@ +import scala.annotation.nowarn + +lazy val check = taskKey[Unit]("") +check := { + val s = (state.value: @nowarn("msg=unused")) + println("hi") +} diff --git a/sbt-app/src/sbt-test/project-load/warning-optout/changes/good.sbt b/sbt-app/src/sbt-test/project-load/warning-optout/changes/good.sbt new file mode 100644 index 000000000..541f1dc69 --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/warning-optout/changes/good.sbt @@ -0,0 +1,7 @@ +import scala.annotation.nowarn + +lazy val check = taskKey[Unit]("") +check := { + val s = (state.value: @nowarn) + println("hi") +} diff --git a/sbt-app/src/sbt-test/project-load/warning-optout/changes/good2.sbt b/sbt-app/src/sbt-test/project-load/warning-optout/changes/good2.sbt new file mode 100644 index 000000000..931c1b7ea --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/warning-optout/changes/good2.sbt @@ -0,0 +1,7 @@ +import scala.annotation.nowarn + +lazy val check = taskKey[Unit]("") +check := { + val s = (state.value: @nowarn("msg=transient key")) + println("hi") +} diff --git a/sbt-app/src/sbt-test/project-load/warning-optout/project/plugins.sbt b/sbt-app/src/sbt-test/project-load/warning-optout/project/plugins.sbt new file mode 100644 index 000000000..7bd8d3d31 --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/warning-optout/project/plugins.sbt @@ -0,0 +1 @@ +Compile / scalacOptions += "-Werror" diff --git a/sbt-app/src/sbt-test/project-load/warning-optout/test b/sbt-app/src/sbt-test/project-load/warning-optout/test new file mode 100644 index 000000000..14b8b09ee --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/warning-optout/test @@ -0,0 +1,17 @@ +$ copy-file changes/good.sbt build.sbt + +> about + +$ copy-file changes/bad1.sbt build.sbt + +-> reload + +$ copy-file changes/bad2.sbt build.sbt + +-> reload + +$ copy-file changes/good2.sbt build.sbt + +> reload + +$ copy-file changes/good.sbt build.sbt From 66e0a3ed931c9edf7b8146c499826efebc8c1d88 Mon Sep 17 00:00:00 2001 From: BrianHotopp Date: Fri, 24 Jul 2026 02:55:33 -0400 Subject: [PATCH 07/12] [2.x] fix: Register every Def.declareOutput execution, not one per call site (#9492) The cached-task macro allocated one mutable slot per syntactic Def.declareOutput / Def.declareOutputDirectory call site and snapshotted the slots into the task's outputs after the body ran. A call inside a loop or .map over a runtime-determined list is a single syntactic site executed many times, so each iteration overwrote the same slot and only the last file was cached and restored on a cache hit. There was also no way for a conditional call site that did not execute to stay out of the outputs: its slot remained null. Declared outputs now accumulate in a per-task ListBuffer: the macro emits one buffer at the top of the cached body and rewrites each call site to ActionCache.registerOutput(vf, buffer), which appends and returns the value. Every execution registers, an unexecuted site contributes nothing, and the static multi-site shape is unchanged. Refs #9462 (the declareOutput-in-a-loop half) Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: eugene yokota --- build.sbt | 6 +- .../sbt/internal/util/appmacro/Cont.scala | 68 ++++++++++++------- .../internal/util/appmacro/ContextUtil.scala | 23 +------ notes/2.0.0/declare-output-in-loop.md | 13 ++++ .../cache/declare-output-loop/build.sbt | 66 ++++++++++++++++++ .../sbt-test/cache/declare-output-loop/test | 10 +++ .../src/main/scala/sbt/util/ActionCache.scala | 9 +++ 7 files changed, 148 insertions(+), 47 deletions(-) create mode 100644 notes/2.0.0/declare-output-in-loop.md create mode 100644 sbt-app/src/sbt-test/cache/declare-output-loop/build.sbt create mode 100644 sbt-app/src/sbt-test/cache/declare-output-loop/test diff --git a/build.sbt b/build.sbt index 1ac01bf92..16befd471 100644 --- a/build.sbt +++ b/build.sbt @@ -675,8 +675,12 @@ lazy val coreMacrosProj = (project in file("core-macros")) SettingKey[Boolean]("exportPipelining") := false, mimaSettings, mimaBinaryIssueFilters ++= Seq( - exclude[ReversedMissingMethodProblem]("sbt.internal.util.appmacro.ContextUtil.*"), + // macro-expansion internals; Output's per-call-site var codegen members were removed + ProblemFilters.exclude[DirectMissingMethodProblem]( + "sbt.internal.util.appmacro.ContextUtil#Output.*" + ), exclude[DirectMissingMethodProblem]("sbt.internal.util.appmacro.ContextUtil#Input.*"), + exclude[ReversedMissingMethodProblem]("sbt.internal.util.appmacro.ContextUtil.*"), ), ) diff --git a/core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala b/core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala index 2e34d4b9c..29ac4703a 100644 --- a/core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala +++ b/core-macros/src/main/scala/sbt/internal/util/appmacro/Cont.scala @@ -192,6 +192,16 @@ trait Cont: val inputBuf = ListBuffer[Input]() val outputBuf = ListBuffer[Output]() + lazy val outputAccSym: Symbol = + Symbol.newVal( + Symbol.spliceOwner, + freshName("outputs"), + TypeRepr.of[ListBuffer[VirtualFile]], + Flags.EmptyFlags, + Symbol.noSymbol + ) + def outputAccRef: Expr[ListBuffer[VirtualFile]] = + Ref(outputAccSym).asExprOf[ListBuffer[VirtualFile]] def unitExpr: Expr[Unit] = '{ () } @@ -405,28 +415,33 @@ trait Cont: } // This will generate following code for Def.declareOutput(...): - // var $o1: VirtualFile = null - // ActionCache.ActionResult({ + // val $outputs = ListBuffer.empty[VirtualFile] + // ActionCache.InternalActionResult({ // body... - // $o1 = out // Def.declareOutput(out) + // ActionCache.registerOutput(out, $outputs) // Def.declareOutput(out) // result - // }, List($o1)) + // }, $outputs.toList) def letOutput[A1: Type]( outputs: List[Output], cacheConfigExpr: Expr[BuildWideCacheConfiguration], )(body: Expr[A1]): Expr[ActionCache.InternalActionResult[A1]] = - Block( - outputs.map(_.toVarDef), + if outputs.isEmpty then '{ ActionCache.InternalActionResult( value = $body, - outputs = List(${ - Varargs[VirtualFile](outputs.map: out => - out.toRef.asExprOf[VirtualFile]) - }*), + outputs = Nil, ) - }.asTerm - ).asExprOf[ActionCache.InternalActionResult[A1]] + } + else + Block( + ValDef(outputAccSym, Some('{ ListBuffer.empty[VirtualFile] }.asTerm)) :: Nil, + '{ + ActionCache.InternalActionResult( + value = $body, + outputs = $outputAccRef.toList, + ) + }.asTerm + ).asExprOf[ActionCache.InternalActionResult[A1]] val WrapOutputName = "wrapOutput_\u2603\u2603" val WrapOutputDirectoryName = "wrapOutputDirectory_\u2603\u2603" @@ -442,12 +457,16 @@ trait Cont: val output = Output( tpe = TypeRepr.of[a], term = qual, - name = freshName("o"), - parent = Symbol.spliceOwner, - outputType = OutputType.File + outputType = OutputType.File, ) outputBuf += output - if cacheConfigExprOpt.isDefined then output.toAssign(output.term) + if cacheConfigExprOpt.isDefined then + '{ + ActionCache.registerOutput( + ${ output.term.asExprOf[VirtualFile] }, + $outputAccRef, + ) + }.asTerm else oldTree case WrapOutputDirectoryName => val output = Output( @@ -455,20 +474,21 @@ trait Cont: // which contains hash. tpe = TypeRepr.of[VirtualFile], term = qual, - name = freshName("o"), - parent = Symbol.spliceOwner, outputType = OutputType.Directory, ) outputBuf += output cacheConfigExprOpt match case Some(cacheConfigExpr) => - output.toAssign('{ - ActionCache.packageDirectory( - dir = ${ output.term.asExprOf[VirtualFileRef] }, - conv = $cacheConfigExpr.fileConverter, - outputDirectory = $cacheConfigExpr.outputDirectory, + '{ + ActionCache.registerOutput( + ActionCache.packageDirectory( + dir = ${ output.term.asExprOf[VirtualFileRef] }, + conv = $cacheConfigExpr.fileConverter, + outputDirectory = $cacheConfigExpr.outputDirectory, + ), + $outputAccRef, ) - }.asTerm) + }.asTerm case None => oldTree case _ => inputBuf += Input( diff --git a/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala b/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala index 2b65e4f76..2d1414553 100644 --- a/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala +++ b/core-macros/src/main/scala/sbt/internal/util/appmacro/ContextUtil.scala @@ -157,31 +157,10 @@ trait ContextUtil[C <: Quotes & scala.Singleton](val valStart: Int): final class Output( val tpe: TypeRepr, val term: Term, - val name: String, - val parent: Symbol, val outputType: OutputType, ): override def toString: String = - s"Output($tpe, $term, $name, $outputType)" - val placeholder: Symbol = - tpe.asType match - case '[a] => - Symbol.newVal( - parent, - name, - tpe, - Flags.Mutable, - Symbol.noSymbol - ) - def toVarDef: ValDef = - ValDef(placeholder, rhs = Some('{ null }.asTerm)) - def toAssign(value: Term): Term = - Block( - Assign(toRef, value) :: Nil, - toRef - ) - def toRef: Ref = Ref(placeholder) - def isFile: Boolean = outputType == OutputType.File + s"Output($tpe, $term, $outputType)" end Output def applyTuple(tupleTerm: Term, tpe: TypeRepr, idx: Int): Term = diff --git a/notes/2.0.0/declare-output-in-loop.md b/notes/2.0.0/declare-output-in-loop.md new file mode 100644 index 000000000..fb788352a --- /dev/null +++ b/notes/2.0.0/declare-output-in-loop.md @@ -0,0 +1,13 @@ +### Every `Def.declareOutput` call registers, including inside loops + +The cached-task macro allocated one slot per syntactic `Def.declareOutput` (or +`Def.declareOutputDirectory`) call site, so a call inside a loop or `.map` over a +runtime-determined list of files overwrote the same slot on every iteration and +only the last file was cached and restored. Declared outputs now accumulate per +execution, so a dynamic number of outputs declared from one call site all +survive a cache hit. A `declareOutput` in a conditional branch that is not taken +no longer contributes a null entry to the task's outputs either. + +This addresses the loop half of [#9462][i9462]. + +[i9462]: https://github.com/sbt/sbt/issues/9462 diff --git a/sbt-app/src/sbt-test/cache/declare-output-loop/build.sbt b/sbt-app/src/sbt-test/cache/declare-output-loop/build.sbt new file mode 100644 index 000000000..69248e35a --- /dev/null +++ b/sbt-app/src/sbt-test/cache/declare-output-loop/build.sbt @@ -0,0 +1,66 @@ +import sbt.internal.util.CacheEventSummary +import xsbti.HashedVirtualFileRef + +val declareLoop = taskKey[Seq[HashedVirtualFileRef]]("declares 3 files via .map over a runtime list") +val checkAll = taskKey[Unit]("asserts all 3 files exist") +val delFiles = taskKey[Unit]("deletes the 3 files") +val checkNone = taskKey[Unit]("asserts none of the 3 files exist") +val checkHit = taskKey[Unit]("asserts previous command was a pure cache hit") + +Global / localCacheDirectory := baseDirectory.value / "diskcache" + +lazy val declareOutputLoop = project.in(file(".")) + +declareLoop := { + val log = streams.value.log + val dir = target.value / "gen-multi" + IO.createDirectory(dir) + val files = List(dir / "a.txt", dir / "b.txt", dir / "c.txt") + IO.write(files(0), "AAA") + IO.write(files(1), "BBB") + IO.write(files(2), "CCC") + log.info(s"COMPUTED declareLoop (cache miss)") + if (sys.props.contains("never.set.property")) { + val ghost = fileConverter.value.toVirtualFile((dir / "never.txt").toPath) + val _ = Def.declareOutput(ghost) + } + files.map { f => + val vf = fileConverter.value.toVirtualFile(f.toPath) + Def.declareOutput(vf) + } +} + +def listing(dir: File): String = + if (dir.exists) (dir ** "*").get().mkString(", ") else "" + +checkAll := Def.uncached { + val dir = target.value / "gen-multi" + streams.value.log.info(s"gen-multi listing: ${listing(dir)}") + assert((dir / "a.txt").exists, s"a.txt missing under $dir") + assert((dir / "b.txt").exists, s"b.txt missing under $dir") + assert((dir / "c.txt").exists, s"c.txt missing under $dir") +} + +delFiles := Def.uncached { + val dir = target.value / "gen-multi" + IO.delete(Seq(dir / "a.txt", dir / "b.txt", dir / "c.txt")) + streams.value.log.info(s"deleted files under $dir") +} + +checkNone := Def.uncached { + val dir = target.value / "gen-multi" + assert( + !(dir / "a.txt").exists && !(dir / "b.txt").exists && !(dir / "c.txt").exists, + s"files still present under $dir" + ) +} + +checkHit := Def.uncached { + val config = Def.cacheConfiguration.value + val prev = config.cacheEventLog.previous match + case s: CacheEventSummary.Data => s + case _ => sys.error("empty event log") + streams.value.log.info(s"prev hitCount=${prev.hitCount} missCount=${prev.missCount}") + assert(prev.missCount == 0, s"expected pure hit but missCount=${prev.missCount}") + assert(prev.hitCount >= 1, s"expected a hit but hitCount=${prev.hitCount}") +} diff --git a/sbt-app/src/sbt-test/cache/declare-output-loop/test b/sbt-app/src/sbt-test/cache/declare-output-loop/test new file mode 100644 index 000000000..bba463933 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/declare-output-loop/test @@ -0,0 +1,10 @@ +# Regression for #9462: every execution of a Def.declareOutput call site must register, +# including calls inside a .map over a runtime-determined list. The task also contains a +# declareOutput in a never-taken branch, which must contribute nothing. +> declareLoop +> checkAll +> delFiles +> checkNone +> declareLoop +> checkHit +> checkAll diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 143de2904..292fdadff 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala @@ -33,6 +33,7 @@ import sbt.nio.file.syntax.* import sbt.util.CacheImplicits import scala.reflect.ClassTag import scala.annotation.{ meta, StaticAnnotation } +import scala.collection.mutable import scala.util.control.NonFatal import sjsonnew.{ HashWriter, JsonFormat } import sjsonnew.support.murmurhash.Hasher @@ -374,6 +375,14 @@ object ActionCache: Files.move(staging, destZip, StandardCopyOption.REPLACE_EXISTING) finally Files.deleteIfExists(staging) + /** Appends a declared output; called from code generated by the cached-task macro. */ + def registerOutput( + vf: VirtualFile, + outputs: mutable.ListBuffer[VirtualFile], + ): VirtualFile = + outputs += vf + vf + def packageDirectory( dir: VirtualFileRef, conv: FileConverter, From 67b8c54432440700f874b165f28a548f1f5d3008 Mon Sep 17 00:00:00 2001 From: BrianHotopp Date: Fri, 24 Jul 2026 13:51:33 -0400 Subject: [PATCH 08/12] [2.x] fix: Keep file I/O out of cache-write serialization (#9496) putBlobsIfNeeded reads each blob's hash and size once, up front, and returns only plain HashedVirtualFileRef values, so serializing an ActionResult (disk, in-memory, or remote store) performs no file I/O and nothing re-stats a blob after its CAS entry is written: a file vanishing once stored no longer prevents the write, and I/O errors on an output file surface upfront at blob storage time rather than mid-serialization. Fixes #9349 Co-authored-by: Claude Opus 4.8 (1M context) --- .../2.0.0/cache-write-serialization-no-io.md | 15 +++ .../scala/sbt/util/ActionCacheStore.scala | 10 +- .../test/scala/sbt/util/ActionCacheTest.scala | 104 ++++++++++++++++++ 3 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 notes/2.0.0/cache-write-serialization-no-io.md diff --git a/notes/2.0.0/cache-write-serialization-no-io.md b/notes/2.0.0/cache-write-serialization-no-io.md new file mode 100644 index 000000000..dba3cc9b3 --- /dev/null +++ b/notes/2.0.0/cache-write-serialization-no-io.md @@ -0,0 +1,15 @@ +### Cache-write serialization no longer performs file I/O + +Serializing a task's result for the action cache re-read the size of every +referenced file at write time, so a file vanishing between the task completing +and the cache write (for example when two overlapping evaluations of the same +task race the jar-to-CAS-symlink swap) made an otherwise successful task's +cache write throw an intermittent `sjsonnew.SerializationException: error +while writing the field outputFiles`. Each stored output reference is now +materialized once, before its blob is stored, so the cache write succeeds even +if the file vanishes afterwards, and I/O errors on an output file surface +upfront at blob storage time rather than mid-serialization. + +This addresses [#9349][i9349]. + +[i9349]: https://github.com/sbt/sbt/issues/9349 diff --git a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala index a386f1a1c..fca60e0d1 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala @@ -65,16 +65,20 @@ end ActionCacheStore trait AbstractActionCacheStore extends ActionCacheStore: def putBlobsIfNeeded(blobs: Seq[VirtualFile]): Seq[HashedVirtualFileRef] = + // Read each blob's hash and size once, up front, and return only these plain value refs: + // serializing the ActionResult afterwards does no file I/O, and nothing re-stats a blob + // after its CAS entry is written. + val materialized: Seq[(VirtualFile, HashedVirtualFileRef)] = blobs.map: blob => + blob -> HashedVirtualFileRef.of(blob.id, blob.contentHashStr, blob.sizeBytes) val found = findBlobs(blobs).toSet val missing = blobs.flatMap: blob => val ref: HashedVirtualFileRef = blob if found.contains(ref) then None else Some(blob) val combined = putBlobs(missing).toSet ++ found - blobs.flatMap: blob => + materialized.flatMap: (blob, plain) => val ref: HashedVirtualFileRef = blob - if combined.contains(ref) then Some(ref) - else None + if combined.contains(ref) then Some(plain) else None def notFound: Throwable = new RuntimeException("not found") diff --git a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala index cb3f15987..1cf8fd32a 100644 --- a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala +++ b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala @@ -25,6 +25,8 @@ import xsbti.{ import ActionCache.InternalActionResult object ActionCacheTest extends BasicTestSuite: + final case class Unserializable(n: Int) + val tags = CacheLevelTag.all.toList test("findMissingFile extracts the path from a wrapped NoSuchFileException"): @@ -348,6 +350,108 @@ object ActionCacheTest extends BasicTestSuite: assert(v2 == 2) assert(called == 1, s"expected a success cache hit after the cure (called=$called)") + test("A file vanishing after its blob is stored no longer breaks the cache write"): + withDiskCache: cache => + import sjsonnew.BasicJsonProtocol.* + var called = 0 + IO.withTemporaryDirectory: tempDir => + val config = getCacheConfig(cache, tempDir) + val goodHash = Digest.sha256Hash("hello".getBytes(StandardCharsets.UTF_8)).contentHashStr + val casFile = cache.toCasFile(Digest(s"$goodHash/5")) + // Models the reported race: the backing file vanishes the moment its blob reaches the + // CAS (the concurrent winner's syncFile swap), so any later stat throws. + val vanishing = new xsbti.BasicVirtualFileRef(s"$tempDir/out.jar") with VirtualFile: + private def maybeThrow[A](a: A): A = + if Files.exists(casFile) then throw new NoSuchFileException(id) else a + override def contentHash: Long = 0L + override def sizeBytes: Long = maybeThrow(5L) + override def contentHashStr: String = maybeThrow(goodHash) + override def input: java.io.InputStream = + new ByteArrayInputStream("hello".getBytes(StandardCharsets.UTF_8)) + val action: ((Int, Int)) => InternalActionResult[Int] = { (a, b) => + called += 1 + InternalActionResult(a + b, Seq(vanishing)) + } + val v1 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v1 == 2) + assert(called == 1) + val v2 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v2 == 2) + assert(called == 1, s"expected a cache hit: the write must have succeeded (called=$called)") + + test("A file already missing at the cache write degrades to an uncached task"): + withDiskCache: cache => + import sjsonnew.BasicJsonProtocol.* + var called = 0 + IO.withTemporaryDirectory: tempDir => + val config = getCacheConfig(cache, tempDir) + val gone = new xsbti.BasicVirtualFileRef(s"$tempDir/out.jar") with VirtualFile: + override def contentHash: Long = throw new NoSuchFileException(id) + override def sizeBytes: Long = throw new NoSuchFileException(id) + override def contentHashStr: String = throw new NoSuchFileException(id) + override def input: java.io.InputStream = throw new NoSuchFileException(id) + val action: ((Int, Int)) => InternalActionResult[Int] = { (a, b) => + called += 1 + InternalActionResult(a + b, Seq(gone)) + } + val v1 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v1 == 2) + assert(called == 1) + val v2 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v2 == 2) + assert(called == 2, "a degraded cache write must mean a cache miss on the next run") + + test("put materializes output refs so serialization does no file I/O"): + withDiskCache: cache => + IO.withTemporaryDirectory: tempDir => + @volatile var vanished = false + val goodHash = Digest.sha256Hash("hello".getBytes(StandardCharsets.UTF_8)).contentHashStr + val ref = new xsbti.BasicVirtualFileRef(s"$tempDir/out.jar") with VirtualFile: + private def maybeThrow[A](a: A): A = + if vanished then throw new NoSuchFileException(id) else a + override def contentHash: Long = 0L + override def sizeBytes: Long = maybeThrow(5L) + override def contentHashStr: String = maybeThrow(goodHash) + override def input: java.io.InputStream = + new ByteArrayInputStream("hello".getBytes(StandardCharsets.UTF_8)) + val stored = + cache.put(UpdateActionResultRequest(Digest.dummy(42L), Vector(ref), exitCode = 0)) match + case Right(r) => r.outputFiles.head + case Left(e) => throw new AssertionError(s"put failed: $e", e) + vanished = true + assert(stored.id == ref.id) + assert( + stored.contentHashStr == goodHash && stored.sizeBytes == 5L, + "stored ref must not re-stat the file" + ) + + test("A successful task whose value fails to serialize returns it uncached"): + withDiskCache: cache => + var called = 0 + // Pins the value-serialization leg of the NonFatal recovery introduced in #9488, which + // had no test: a codec that always throws, standing in for any serialization failure + // during the cache write of a succeeded task. + given sjsonnew.JsonFormat[Unserializable] = new sjsonnew.JsonFormat[Unserializable]: + override def write[J](obj: Unserializable, builder: sjsonnew.Builder[J]): Unit = + sjsonnew.serializationError("Unserializable is unserializable") + override def read[J]( + jsOpt: Option[J], + unbuilder: sjsonnew.Unbuilder[J] + ): Unserializable = Unserializable(0) + import sjsonnew.BasicJsonProtocol.* + val action: ((Int, Int)) => InternalActionResult[Unserializable] = { (a, b) => + called += 1 + InternalActionResult(Unserializable(a + b), Nil) + } + IO.withTemporaryDirectory: tempDir => + val config = getCacheConfig(cache, tempDir) + val v1 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v1 == Unserializable(2)) + assert(called == 1) + val v2 = ActionCache.cache((1, 1), Digest.zero, Digest.zero, tags, config)(action) + assert(v2 == Unserializable(2)) + assert(called == 2, "a degraded cache write must mean a cache miss on the next run") + test("Cache falls back to recompute when syncBlobs throws FileNotFoundException"): withDiskCache(testSyncBlobsThrowsFallback) From 9b295a6d41d44434107a9448214aa1951ce47773 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Sat, 25 Jul 2026 15:59:09 -0400 Subject: [PATCH 09/12] [2.0.x] Zinc 2.0.3 --- project/Dependencies.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 0cfcb0b86..4422e8e67 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -12,8 +12,8 @@ object Dependencies { sys.env.get("BUILD_VERSION") orElse sys.props.get("sbt.build.version") // sbt modules - val ioVersion = nightlyVersion.getOrElse("1.12.1") - val zincVersion = nightlyVersion.getOrElse("2.0.2") + val ioVersion = nightlyVersion.getOrElse("1.12.2") + val zincVersion = nightlyVersion.getOrElse("2.0.3") private val sbtIO = "org.scala-sbt" %% "io" % ioVersion From fd4e7c4863913c12c16f59ecb38189655a8724ae Mon Sep 17 00:00:00 2001 From: eugene yokota Date: Fri, 24 Jul 2026 16:42:04 -0400 Subject: [PATCH 10/12] [2.x] fix: Skip checksum generation for asc file, take 2 (#9499) **Problem** Checksums are still generated for asc file. 1. localStaging is a file repo, which was not handled 2. It was checking Artifact name, not the file name **Solution** This fixes both. --- .../librarymanagement/ConvertResolver.scala | 32 ++++++++----------- .../ivy/publish-asc-no-checksum/build.sbt | 25 +++++++++++++++ .../project/plugins.sbt | 5 +++ .../sbt-test/ivy/publish-asc-no-checksum/test | 16 ++++++++++ 4 files changed, 60 insertions(+), 18 deletions(-) create mode 100644 sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/build.sbt create mode 100644 sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/project/plugins.sbt create mode 100644 sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/test diff --git a/lm-ivy/src/main/scala/sbt/internal/librarymanagement/ConvertResolver.scala b/lm-ivy/src/main/scala/sbt/internal/librarymanagement/ConvertResolver.scala index 81d559a36..5bb43b487 100644 --- a/lm-ivy/src/main/scala/sbt/internal/librarymanagement/ConvertResolver.scala +++ b/lm-ivy/src/main/scala/sbt/internal/librarymanagement/ConvertResolver.scala @@ -116,18 +116,13 @@ private[sbt] object ConvertResolver { checksum <- checksums if !ChecksumHelper.isKnownAlgorithm(checksum) } throw new IllegalArgumentException("Unknown checksum algorithm: " + checksum) - repository.put(artifact, src, dest, overwrite); - // Fix for sbt#1156 - Artifactory will auto-generate MD5/sha1 files, so - // we need to overwrite what it has. - if (!artifact.getName.endsWith(".asc")) { - for (checksum <- checksums) { - putChecksumMethod match { + repository.put(artifact, src, dest, overwrite) + if !dest.endsWith(".asc") then + for checksum <- checksums do + putChecksumMethod match case Some(method) => method.invoke(this, artifact, src, dest, true: java.lang.Boolean, checksum) case None => // TODO - issue warning? - } - } - } if (signerName != null) { putSignatureMethod match { case None => () @@ -219,15 +214,16 @@ private[sbt] object ConvertResolver { resolver } case repo: FileRepository => { - val resolver = new FileSystemResolver with DescriptorRequired { - // Workaround for #1156 - // Temporarily in sbt 0.13.x we deprecate overwriting - // in local files for non-changing revisions. - // This will be fully enforced in sbt 1.0. - setRepository(new WarnOnOverwriteFileRepo()) - override val managedChecksumsEnabled: Boolean = managedChecksums - override def getResource(resource: Resource, dest: File): Long = get(resource, dest) - } + val resolver = + new FileSystemResolver with ChecksumFriendlyURLResolver with DescriptorRequired { + // Workaround for #1156 + // Temporarily in sbt 0.13.x we deprecate overwriting + // in local files for non-changing revisions. + // This will be fully enforced in sbt 1.0. + setRepository(new WarnOnOverwriteFileRepo()) + override val managedChecksumsEnabled: Boolean = managedChecksums + override def getResource(resource: Resource, dest: File): Long = get(resource, dest) + } resolver.setName(repo.name) initializePatterns(resolver, repo.patterns, settings) import repo.configuration.{ isLocal, isTransactional } diff --git a/sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/build.sbt b/sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/build.sbt new file mode 100644 index 000000000..dfdfa9a65 --- /dev/null +++ b/sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/build.sbt @@ -0,0 +1,25 @@ +// `signTask` stands in for sbt-pgp's signing task: it just writes a fake signature file +// and publishes it as an extra artifact with an ".asc" extension. + +useIvy := true + +organization := "com.example" +name := "foo" +version := "1.0.0" +scalaVersion := "2.12.21" +autoScalaLibrary := false +crossPaths := false +Compile / packageDoc / publishArtifact := false +Compile / packageSrc / publishArtifact := false +publishTo := localStaging.value + +lazy val signTask = taskKey[HashedVirtualFileRef]("Emulates sbt-pgp's signing task") + +signTask := { + val conv = fileConverter.value + val out = target.value / "foo-1.0.0.jar.asc" + IO.write(out, "fake-signature") + conv.toVirtualFile(out.toPath) +} + +addArtifact(Artifact("foo", "asc", "jar.asc"), signTask) diff --git a/sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/project/plugins.sbt b/sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/project/plugins.sbt new file mode 100644 index 000000000..15c7fdd35 --- /dev/null +++ b/sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/project/plugins.sbt @@ -0,0 +1,5 @@ +libraryDependencies += Defaults.sbtPluginExtra( + "org.scala-sbt" % "sbt-ivy" % sbtVersion.value, + sbtVersion.value, + scalaVersion.value, +) diff --git a/sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/test b/sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/test new file mode 100644 index 000000000..fab6a6f32 --- /dev/null +++ b/sbt-app/src/sbt-test/ivy/publish-asc-no-checksum/test @@ -0,0 +1,16 @@ +# useIvy := true forces the Ivy-backed publisher (ConvertResolver), which is what generates +# checksums via the ChecksumFriendlyURLResolver shim. +> publish + +# ordinary artifacts and their checksums are published as usual +$ exists target/sona-staging/com/example/foo/1.0.0/foo-1.0.0.jar +$ exists target/sona-staging/com/example/foo/1.0.0/foo-1.0.0.jar.md5 +$ exists target/sona-staging/com/example/foo/1.0.0/foo-1.0.0.jar.sha1 +$ exists target/sona-staging/com/example/foo/1.0.0/foo-1.0.0.pom +$ exists target/sona-staging/com/example/foo/1.0.0/foo-1.0.0.pom.md5 +$ exists target/sona-staging/com/example/foo/1.0.0/foo-1.0.0.pom.sha1 + +# the .asc signature artifact is published, but must NOT get its own checksum files +$ exists target/sona-staging/com/example/foo/1.0.0/foo-1.0.0.jar.asc +-$ exists target/sona-staging/com/example/foo/1.0.0/foo-1.0.0.jar.asc.md5 +-$ exists target/sona-staging/com/example/foo/1.0.0/foo-1.0.0.jar.asc.sha1 From fa2f8b87b8d69ad0be625b76ac3e5f4f0325978f Mon Sep 17 00:00:00 2001 From: Kevin Lee Date: Fri, 24 Jul 2026 08:05:37 +1000 Subject: [PATCH 11/12] [2.0.x] fix: Fixes three test-classloader / jar-handle leaks in the sbt server JVM (#9485) 2 independent, pre-existing retention bugs kept the classloader of a finished in-process test run -- and the open jar handles it holds -- alive for the rest of the server session. - JUnitXmlTestsListener: testSuite is an InheritableThreadLocal, so threads spawned during a run (e.g. async-framework pool workers) inherit a copy of the suite reference that remove() cannot reach. - ClassLoaderCache: loaders evicted from delegate by clearExpiredLoaders are unreachable from the map and never enqueued on the ReferenceQueue, so neither clear()/close() nor the cleanup thread could ever close them. SuiteResult now documents the retention hazard on throwables. Adds deterministic, cross-platform tests for all three severed chains. --- .../internal/classpath/ClassLoaderCache.scala | 33 +++++++++++ .../sbt/internal/ClassLoaderCacheTest.scala | 27 +++++++++ .../scala/sbt/JUnitXmlTestsListener.scala | 55 ++++++++++++++++--- .../main/scala/sbt/TestReportListener.scala | 14 +++++ .../scala/sbt/JUnitXmlTestsListenerSpec.scala | 50 +++++++++++++++++ 5 files changed, 172 insertions(+), 7 deletions(-) diff --git a/main-command/src/main/scala/sbt/internal/classpath/ClassLoaderCache.scala b/main-command/src/main/scala/sbt/internal/classpath/ClassLoaderCache.scala index c77913911..1bc7cb979 100644 --- a/main-command/src/main/scala/sbt/internal/classpath/ClassLoaderCache.scala +++ b/main-command/src/main/scala/sbt/internal/classpath/ClassLoaderCache.scala @@ -77,12 +77,34 @@ private[sbt] class ClassLoaderCache( new java.util.concurrent.ConcurrentHashMap[Key, Reference[ClassLoader]]() private val referenceQueue = new ReferenceQueue[ClassLoader] + /* + * Loaders evicted from `delegate` by clearExpiredLoaders are no longer reachable from the + * map, so clear()/close() alone can never close them. Nor can the cleanup thread: once the + * entry is removed, the Reference object itself becomes unreachable, and an unreachable + * Reference is never enqueued on the ReferenceQueue. They would linger with open jar handles + * for the life of the JVM. On Windows those handles make the underlying jars undeletable + * (e.g. clearCaches cannot delete cas blobs the loaders still reference). Track evicted + * loaders weakly so clear() can close them deterministically; weak keys preserve the + * metaspace-pressure design above by adding no strong retention of their own. + */ + private val retired = + java.util.Collections.synchronizedMap(new java.util.WeakHashMap[ClassLoader, java.lang.Boolean]) + private def clearExpiredLoaders(): Unit = lock.synchronized { val clear = (k: Key, ref: Reference[ClassLoader]) => { ref.get() match { case w: WrappedLoader => w.invalidate() case _ => } + ref match { + case ClassLoaderReference(_, underlying) => + retired.put(underlying, java.lang.Boolean.TRUE) + case r => + r.get() match { + case null => + case loader => retired.put(loader, java.lang.Boolean.TRUE) + } + } delegate.remove(k) () } @@ -109,6 +131,7 @@ private[sbt] class ClassLoaderCache( referenceQueue.remove(1000) match { case ClassLoaderReference(key, classLoader) => close(classLoader) + retired.remove(classLoader) delegate.remove(key) () case _ => @@ -148,6 +171,10 @@ private[sbt] class ClassLoaderCache( * handle from being modified. On linux and mac, we probably leak some file descriptors but it's * fairly uncommon for sbt to run out of file descriptors. * + * Loaders evicted by clearExpiredLoaders (as opposed to by garbage collection) are a separate + * case: they are removed from `delegate`, so neither clear()/close() nor the reference-queue + * cleanup thread can reach them. Those are tracked weakly in `retired` and closed by + * clear()/close(), so their handles are released deterministically rather than never. */ private val metaspaceIsLimited = ManagementFactory.getMemoryPoolMXBeans.asScala @@ -230,6 +257,12 @@ private[sbt] class ClassLoaderCache( } } delegate.clear() + /* Also close loaders that were evicted from the delegate map but never closed (see + * `retired`); WeakHashMap iteration requires holding its monitor, so snapshot the keys + * first and close outside the lock. */ + val evicted = retired.synchronized(new java.util.ArrayList(retired.keySet()).asScala.toList) + evicted.foreach(close) + retired.clear() } /** diff --git a/main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala b/main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala index f2d0515a7..4a6154167 100644 --- a/main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala +++ b/main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala @@ -52,4 +52,31 @@ object ClassLoaderCacheTest extends BasicTestSuite: Predef.assert(cache.get(jarClassPath) == secondLoader) Predef.assert(cache.get(jarClassPath) != initLoader) + test("Loaders evicted by a newer timestamp should be closed by clear()"): + IO.withTemporaryDirectory: dir => + val entry = "leak-test-resource.txt" + val jar = dir.toPath.resolve("evicted.jar").toFile + Using.resource(new java.util.jar.JarOutputStream(new java.io.FileOutputStream(jar))): out => + out.putNextEntry(new java.util.zip.ZipEntry(entry)) + out.write("hello".getBytes("UTF-8")) + out.closeEntry() + + withCache: cache => + val classPath = jar :: Nil + val first = cache.get(classPath) + Predef.assert(first.getResource(entry) != null, "jar resource should load before eviction") + + // Bumping the timestamp makes a new Key, and addLoader calls clearExpiredLoaders after + // inserting it, so `first`'s entry is evicted from the delegate map here. Once evicted + // it is unreachable from the map, so only the `retired` set can still close it. + IO.setModifiedTimeOrFalse(jar, System.currentTimeMillis + 5000L) + val second = cache.get(classPath) + Predef.assert(first != second, "a newer timestamp should produce a new loader") + + cache.clear() + Predef.assert( + first.getResource(entry) == null, + "clear() should have closed the evicted loader, releasing its jar handle" + ) + end ClassLoaderCacheTest diff --git a/testing/src/main/scala/sbt/JUnitXmlTestsListener.scala b/testing/src/main/scala/sbt/JUnitXmlTestsListener.scala index bfb2adacd..c84ebef38 100644 --- a/testing/src/main/scala/sbt/JUnitXmlTestsListener.scala +++ b/testing/src/main/scala/sbt/JUnitXmlTestsListener.scala @@ -15,6 +15,7 @@ import java.time.format.DateTimeFormatter import java.time.temporal.ChronoUnit import java.util.Hashtable import java.util.concurrent.TimeUnit.NANOSECONDS +import java.util.concurrent.atomic.AtomicReference import scala.collection.mutable.ListBuffer import scala.util.Properties @@ -196,13 +197,31 @@ class JUnitXmlTestsListener(val targetDir: File, legacyTestReport: Boolean, logg } } + /** + * A mutable cell holding the suite that is currently running on a thread. + * + * The cell exists purely so the suite can be released deterministically. `testSuite` below + * is an [[InheritableThreadLocal]], so every thread created while a suite is running -- for + * example a pooled worker spawned by an async test framework -- receives a copy of the + * *reference* to this cell at construction time. `ThreadLocal.remove()` only clears the + * calling thread's entry, so those inherited copies would otherwise pin the `TestSuite`, its + * buffered events, and through them the test class loader (and its open jar handles) for the + * remaining life of the JVM. Clearing the cell severs the reference for the owning thread and + * every thread that inherited it at once. + */ + private final class SuiteRef(initial: Option[TestSuite]) { + private val ref = new AtomicReference(initial) + def current: Option[TestSuite] = ref.get() + def clear(): Unit = ref.set(None) + } + /** The currently running test suite */ - private val testSuite = new InheritableThreadLocal[Option[TestSuite]] { - override def initialValue(): Option[TestSuite] = None + private val testSuite = new InheritableThreadLocal[SuiteRef] { + override def initialValue(): SuiteRef = new SuiteRef(None) } private def withTestSuite[T](f: TestSuite => T): T = - testSuite.get().map(f).getOrElse(sys.error("no test suite")) + testSuite.get().current.map(f).getOrElse(sys.error("no test suite")) /** Creates the output Dir */ override def doInit(): Unit = { @@ -212,14 +231,29 @@ class JUnitXmlTestsListener(val targetDir: File, legacyTestReport: Boolean, logg /** * Starts a new, initially empty Suite with the given name. */ - override def startGroup(name: String): Unit = testSuite.set(Some(new TestSuite(name))) + override def startGroup(name: String): Unit = + testSuite.set(new SuiteRef(Some(new TestSuite(name)))) /** * Adds all details for the given even to the current suite. + * + * Events that arrive after the suite has been written are dropped. Test frameworks may call + * the event handler from threads they spawned during the run (see `TestFramework.TestRunner`), + * and such a thread can report after `writeSuite` has already emitted the XML. Before the + * suite was released those late events were appended to an already-written suite, so they + * were discarded in practice; dropping them here keeps that outcome without turning every + * late event into an error line via `TestFramework.safeForeach`. */ - override def testEvent(event: TestEvent): Unit = for (e <- event.detail) { - withTestSuite(_.addEvent(e)) - } + override def testEvent(event: TestEvent): Unit = + testSuite.get().current match { + case Some(suite) => for (e <- event.detail) suite.addEvent(e) + case None => + if (logger != null) { + logger.debug( + s"ignoring ${event.detail.size} test event(s) reported after the suite was written" + ) + } else () + } /** * called for each class or equivalent grouping We map one group to one Testsuite, so for each @@ -283,6 +317,13 @@ class JUnitXmlTestsListener(val targetDir: File, legacyTestReport: Boolean, logg } val testSuiteResult = withTestSuite(_.stop()) XML.save(file, testSuiteResult, "UTF-8", xmlDecl = true, null) + /* Order matters: `clear()` releases the suite for this thread *and* for every thread that + * inherited the cell, which `remove()` cannot reach. `remove()` then drops this thread's + * own entry. Without the `clear()` the suite -- and through its buffered events the test + * class loader with its open jar handles -- would stay reachable from pooled worker threads + * for the life of the JVM. + */ + testSuite.get().clear() testSuite.remove() } diff --git a/testing/src/main/scala/sbt/TestReportListener.scala b/testing/src/main/scala/sbt/TestReportListener.scala index 2b5a9ee60..3870e1786 100644 --- a/testing/src/main/scala/sbt/TestReportListener.scala +++ b/testing/src/main/scala/sbt/TestReportListener.scala @@ -44,6 +44,20 @@ trait TestsListener extends TestReportListener { /** * Provides the overall `result` of a group of tests (a suite) and test counts for each result type. + * + * @param throwables + * The exceptions thrown by the tests in this suite, as live objects. They are needed as objects + * rather than as rendered text because the ClassLoaderLayeringStrategy diagnostic in `Defaults` + * inspects them with `isInstanceOf` and walks `getCause` to recognise `NoClassDefFoundError` and + * friends. + * + * RETENTION HAZARD: a `Throwable`'s backtrace references the `Class` objects of every frame, and + * a `Class` strongly references its defining class loader. Holding a `SuiteResult` with a + * non-empty `throwables` therefore keeps the test class loader -- and every jar handle it has + * open -- alive. That is fine for the duration of the test task, which is where these are + * consumed, but anything that outlives the task must drop them first. `TestRecap.collect` does + * exactly that before stashing a copy on `State.attributes`; see the note on `TestRecap.recapKey`. + * On Windows a leaked handle makes the underlying jar undeletable (e.g. by `clearCaches`). */ private[sbt] final class SuiteResult( val result: TestResult, diff --git a/testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala b/testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala index 98074f666..8266a2387 100644 --- a/testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala +++ b/testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala @@ -102,4 +102,54 @@ object JUnitXmlTestsListenerSpec extends BasicTestSuite: tempDir.listFiles().foreach(_.delete()) tempDir.delete() + test("JUnitXmlTestsListener should release the suite from threads that inherited it"): + IO.withTemporaryDirectory: tempDir => + val listener = new JUnitXmlTestsListener(tempDir, false, null) + listener.doInit() + + def event(name: String) = new TEvent: + def fullyQualifiedName = s"InheritSuite.$name" + def duration() = 1L + def status = TStatus.Success + def fingerprint = null + def selector = new TestSelector(name) + def throwable = new OptionalThrowable() + + listener.startGroup("InheritSuite") + + // A thread created *while* the suite is set inherits the suite cell, standing in for a + // pooled worker spawned by an async test framework during the run. + val suiteWritten = new java.util.concurrent.CountDownLatch(1) + val childDone = new java.util.concurrent.CountDownLatch(1) + val endGroupOutcome = new AtomicReference[Option[Throwable]](None) + val testEventOutcome = new AtomicReference[Option[Throwable]](None) + val child = new Thread(() => + suiteWritten.await() + // The inherited cell must no longer reach a TestSuite, so the strict path fails... + endGroupOutcome.set( + scala.util.Try(listener.endGroup("InheritSuite", TestResult.Passed)).failed.toOption + ) + // ...while a late event is dropped rather than raised. + testEventOutcome.set( + scala.util.Try(listener.testEvent(sbt.TestEvent(Seq(event("late"))))).failed.toOption + ) + childDone.countDown() + ) + child.setDaemon(true) + child.start() + + listener.testEvent(sbt.TestEvent(Seq(event("testMethod")))) + listener.endGroup("InheritSuite", TestResult.Passed) + suiteWritten.countDown() + assert(childDone.await(30, java.util.concurrent.TimeUnit.SECONDS), "child thread timed out") + + assert( + endGroupOutcome.get().isDefined, + "a thread that inherited the suite could still reach it after writeSuite" + ) + assert( + testEventOutcome.get().isEmpty, + s"a late test event should be dropped, but threw: ${testEventOutcome.get()}" + ) + end JUnitXmlTestsListenerSpec From 752f5d42b1eafbf53e536f6d486a332a044706b8 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Sat, 25 Jul 2026 16:21:53 -0400 Subject: [PATCH 12/12] Add missing imports --- .../src/test/scala/sbt/internal/ClassLoaderCacheTest.scala | 1 + testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala | 1 + 2 files changed, 2 insertions(+) diff --git a/main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala b/main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala index 4a6154167..3d3962f11 100644 --- a/main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala +++ b/main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala @@ -13,6 +13,7 @@ import java.nio.file.Files import sbt.internal.classpath.ClassLoaderCache import sbt.io.IO +import scala.util.Using import verify.BasicTestSuite object ClassLoaderCacheTest extends BasicTestSuite: diff --git a/testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala b/testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala index 8266a2387..abb5c0337 100644 --- a/testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala +++ b/testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala @@ -14,6 +14,7 @@ import java.util.concurrent.atomic.AtomicReference import testing.{ Event as TEvent, OptionalThrowable, Status as TStatus, TestSelector } import util.{ AbstractLogger, Level, ControlEvent, LogEvent } import sbt.protocol.testing.TestResult +import sbt.io.IO import verify.BasicTestSuite object JUnitXmlTestsListenerSpec extends BasicTestSuite: