diff --git a/build.sbt b/build.sbt index fd539bbb5..16befd471 100644 --- a/build.sbt +++ b/build.sbt @@ -674,6 +674,14 @@ lazy val coreMacrosProj = (project in file("core-macros")) name := "Core Macros", SettingKey[Boolean]("exportPipelining") := false, mimaSettings, + mimaBinaryIssueFilters ++= Seq( + // 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.*"), + ), ) // 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..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] = '{ () } @@ -306,7 +316,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" ) @@ -405,31 +415,37 @@ 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" + 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] => @@ -441,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( @@ -454,28 +474,35 @@ 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 _ => - // 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..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 @@ -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)" @@ -155,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 = @@ -194,6 +175,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/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/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/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/ClassLoaderCacheTest.scala b/main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala index f2d0515a7..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: @@ -52,4 +53,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/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/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/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/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/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/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/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/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. 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 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 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/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/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 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 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 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..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: @@ -102,4 +103,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 diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 79f9c3cd2..292fdadff 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, @@ -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 @@ -147,7 +148,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 @@ -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, diff --git a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala index 1ce0ec9cf..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") @@ -285,14 +289,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) @@ -340,7 +347,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()) @@ -358,6 +367,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 03c858829..1cf8fd32a 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 @@ -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"): @@ -88,6 +90,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) @@ -324,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) @@ -444,6 +572,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 +704,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,16 +713,45 @@ object ActionCacheTest extends BasicTestSuite: BuildWideCacheConfiguration( cache, outputDir.toPath(), - fileConverter, + converter, logger, CacheEventLog(), CacheImplicits.defaultLocalDigestCacheByteSize, 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 = 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