[2.x] fix: Don't re-extract a dirzip whose digest already matches (#9555)

syncFile treated an output as up to date only when its digest matched and the
path was already a symlink. packageDirectory installs the archive with
Files.move, so on the write path it is always a regular file and the second
conjunct never held. Every compile therefore fell through to IO.delete +
writeFileAndNotify -> afterFileWrite -> unpackageDirZip, which inflated the
archive it had just written into a temp directory, digest-compared every file in
the output tree, found all of them unchanged, and deleted the temp directory.

The cost is proportional to output size and is paid on every compile of every
module. Measured at ~450 ms for a 2557-file, 33 MB classes directory (11 MB
archive), against a 4.2 s warm single-file edit.

A matching digest already means the local content is the blob's, whether or not
the path is a symlink, so take the up-to-date path in that case. Where the path
is not yet a symlink, still relink it to the CAS to keep deduplication, but
reach afterFileUpToDate rather than afterFileWrite. The symlink-creation
fallback is factored into linkOrCopy and shared with writeFileAndNotify.

For non-dirzip outputs both hooks are no-ops, and that branch already did a
delete-and-relink, so their behaviour is unchanged. The behaviour given up is
that a file inside the extracted directory which diverged from the manifest is
no longer silently restored while the archive digest still matches; on a cache
hit with a differing digest the full unpackage runs as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

@@ -318,9 +318,8 @@ case class DiskActionCacheStore(base: Path, converter: FileConverter)
     // See https://github.com/sbt/sbt/issues/7656
     // On Windows, the program has be running under the Administrator privileges or the
     // user enable Developer Mode on Windows 10+ to create symbolic links.
-    def writeFileAndNotify(outPath: Path): Path =
-      Option(outPath.getParent()).foreach(parent => IO.createDirectory(parent.toFile()))
-      val result = Retry:
+    def linkOrCopy(outPath: Path): Path =
+      Retry:
         if Files.exists(outPath) then IO.delete(outPath.toFile())
         if symlinkSupported.get() && Files.exists(casFile) then
           try Files.createSymbolicLink(outPath, casFile)
@@ -339,6 +338,9 @@ case class DiskActionCacheStore(base: Path, converter: FileConverter)
                 symlinkSupported.set(false)
               copyFile(outPath)
         else copyFile(outPath)
+    def writeFileAndNotify(outPath: Path): Path =
+      Option(outPath.getParent()).foreach(parent => IO.createDirectory(parent.toFile()))
+      val result = linkOrCopy(outPath)
       afterFileWrite(ref, result, outputDirectory)
       result
     val resolvedPath = converter.toPath(ref) match
@@ -350,11 +352,11 @@ case class DiskActionCacheStore(base: Path, converter: FileConverter)
         writeFileAndNotify(p)
       case p =>
         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
-            afterFileUpToDate(ref, p, outputDirectory)
-            p
+          if Digest.sameDigest(p, d) then
+            val result =
+              if symlinkSupported.get() && !Files.isSymbolicLink(p) then linkOrCopy(p) else p
+            afterFileUpToDate(ref, result, outputDirectory)
+            result
           else
             // println(s"- syncFile: $p has different digest")
             IO.delete(p.toFile())

util-cache/src/test/scala/sbt/util/ActionCacheTest.scala

@@ -115,6 +115,63 @@ object ActionCacheTest extends BasicTestSuite:
         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("Disk cache does not re-extract a dirzip whose archive digest already matches"):
+    withDiskCache: cache =>
+      IO.withTemporaryDirectory: tempDir =>
+        val outputDirectory = tempDir.toPath()
+        val dir = tempDir / "gen-dir"
+        IO.write(dir / "a.txt", "contents A")
+        val zipVf = ActionCache.packageDirectory(
+          binaryFileConverter.toVirtualFile(dir.toPath()),
+          binaryFileConverter,
+          outputDirectory,
+        )
+        val refs = cache.putBlobs(Seq(zipVf))
+
+        // packageDirectory leaves a regular file whose digest already matches the blob, so the
+        // extracted tree is in sync by construction and syncing must not unpackage it again.
+        IO.write(dir / "a.txt", "diverged")
+        cache.syncBlobs(refs, outputDirectory)
+        assert(IO.read(dir / "a.txt") == "diverged")
+
+  test("Disk cache relinks a digest-matching dirzip to the CAS"):
+    withDiskCache: cache =>
+      IO.withTemporaryDirectory: tempDir =>
+        val outputDirectory = tempDir.toPath()
+        val dir = tempDir / "gen-dir"
+        IO.write(dir / "a.txt", "contents A")
+        val zipVf = ActionCache.packageDirectory(
+          binaryFileConverter.toVirtualFile(dir.toPath()),
+          binaryFileConverter,
+          outputDirectory,
+        )
+        val refs = cache.putBlobs(Seq(zipVf))
+        val zipPath = Paths.get(dir.toString + ActionCache.dirZipExt)
+        assert(!Files.isSymbolicLink(zipPath), "packageDirectory should leave a regular file")
+
+        cache.syncBlobs(refs, outputDirectory)
+        assert(Files.isSymbolicLink(zipPath), "digest-matching archive was not relinked to the CAS")
+
+  test("Disk cache re-extracts a dirzip whose archive digest differs"):
+    withDiskCache: cache =>
+      IO.withTemporaryDirectory: tempDir =>
+        val outputDirectory = tempDir.toPath()
+        val dir = tempDir / "gen-dir"
+        IO.write(dir / "a.txt", "contents A")
+        val zipVf = ActionCache.packageDirectory(
+          binaryFileConverter.toVirtualFile(dir.toPath()),
+          binaryFileConverter,
+          outputDirectory,
+        )
+        val refs = cache.putBlobs(Seq(zipVf))
+
+        IO.write(Paths.get(dir.toString + ActionCache.dirZipExt).toFile(), "not an archive")
+        IO.write(dir / "a.txt", "diverged")
+        IO.write(dir / "stray.txt", "stray")
+        cache.syncBlobs(refs, outputDirectory)
+        assert(IO.read(dir / "a.txt") == "contents A", "diverged file was not restored")
+        assert(!(dir / "stray.txt").exists, "stray file was not removed")
+
   test("In-memory cache can hold action value"):
     withInMemoryCache(testActionCacheBasic)