Merge ab8e0aaef1 into b411ade704

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 any of its files are missing, checking
+cheaply (without unzipping) on the warm path.
+
+This addresses the directory-restoration half of [#9462][i9462].
+
+[i9462]: https://github.com/sbt/sbt/issues/9462

sbt-app/src/sbt-test/cache/compile-classes-restore/Main.scala

@@ -0,0 +1,2 @@
+@main def main(args: String*) =
+  println("hello from probe")

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}")
+}

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

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 "<dir missing>"
+  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}")
+}

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

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

@@ -342,6 +342,11 @@ object ActionCache:
     val json = Parser.parseFromFile(manifest.toFile()).get
     Converter.fromJsonUnsafe[Manifest](json)
 
+  private[sbt] def manifestFromString(manifest: String): Manifest =
+    import sbt.internal.util.codec.ManifestCodec.given
+    val json = Parser.parseUnsafe(manifest)
+    Converter.fromJsonUnsafe[Manifest](json)
+
   private val default2010Timestamp: Long = 1262304000000L
 
   /**

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

@@ -1,6 +1,7 @@
 package sbt.util
 
 import java.io.{ ByteArrayInputStream, IOException }
+import java.nio.charset.StandardCharsets
 import java.nio.ByteBuffer
 import java.nio.file.{
   Files,
@@ -11,6 +12,7 @@ import java.nio.file.{
   StandardCopyOption
 }
 import java.util.concurrent.atomic.AtomicBoolean
+import java.util.zip.ZipFile
 import sjsonnew.support.scalajson.unsafe.{ CompactPrinter, Converter, Parser }
 import sjsonnew.shaded.scalajson.ast.unsafe.JValue
 
@@ -339,7 +341,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())
@@ -357,6 +361,31 @@ case class DiskActionCacheStore(base: Path, converter: FileConverter)
     if path.toString().endsWith(ActionCache.dirZipExt) then unpackageDirZip(path, outputDirectory)
     else ()
 
+  /**
+   * Re-extract a dirzip whose directory or manifest-listed files are missing; presence is
+   * checked without unzipping.
+   */
+  private def afterFileUpToDate(
+      ref: HashedVirtualFileRef,
+      path: Path,
+      outputDirectory: Path
+  ): Unit =
+    if path.toString().endsWith(ActionCache.dirZipExt) && !dirZipContentsPresent(path) then
+      val _ = unpackageDirZip(path, outputDirectory)
+
+  private def dirZipContentsPresent(dirzip: Path): Boolean =
+    val dirPath = Paths.get(dirzip.toString.dropRight(ActionCache.dirZipExt.size))
+    try
+      Files.isDirectory(dirPath) &&
+        Using.resource(new ZipFile(dirzip.toFile())): zf =>
+          Option(zf.getEntry(ActionCache.manifestFileName)) match
+            case None        => false
+            case Some(entry) =>
+              val str = new String(zf.getInputStream(entry).readAllBytes(), StandardCharsets.UTF_8)
+              val m = ActionCache.manifestFromString(str)
+              m.outputFiles.forall(ref => Files.exists(converter.toPath(ref)))
+    catch case NonFatal(_) => false
+
   /**
    * Given a dirzip, unzip it in a temp directory, and sync each items to the outputDirectory.
    */

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)
 
@@ -507,6 +531,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 =