[2.x] perf: Skip re-packaging the class directory when zinc recompiles nothing (#9609)

Reuse the sibling dirzip when zinc reports it wrote nothing. The output
is still declared, so the stored ActionResult stays complete and a later hit
restores the class directory exactly as before -- it is the same
HashedVirtualFileRef the packaging path would have produced, read off disk
rather than rebuilt.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

main/src/main/scala/sbt/Defaults.scala

@@ -2319,13 +2319,21 @@ object Defaults extends BuildCommon with DefExtra {
       val store = analysisStore(compileAnalysisFile.value.toPath(), c)
       // TODO - Should readAnalysis + saveAnalysis be scoped by the compile task too?
       val analysisResult = Retry.io(compileIncrementalTaskImpl(bspTask, s, ci, ping, projectId))
-      val analysisOut = c.toVirtualFile(setup.cachePath())
-      val contents = AnalysisContents.create(analysisResult.analysis(), analysisResult.setup())
-      store.set(contents)
-      Def.declareOutput(analysisOut)
       val dir = ci.options.classesDirectory
       val vfDir = c.toVirtualFile(dir)
-      val packedDir = Def.declareOutputDirectory(vfDir)
+      val dirZip = ActionCache.dirZipPath(dir)
+      // Zinc leaves the class directory alone when it invalidates nothing, so the zip the previous
+      // run left behind still describes it and re-packing only reproduces a blob the store has.
+      val packedDir =
+        if analysisResult.hasModified() || !Files.exists(dirZip) then
+          Def.declareOutputDirectory(vfDir)
+        else Def.declareOutput(c.toVirtualFile(dirZip))
+      val analysisOut = c.toVirtualFile(setup.cachePath())
+      val contents = AnalysisContents.create(analysisResult.analysis(), analysisResult.setup())
+      // Packaging precedes this write so that a run interrupted in between leaves a stale analysis,
+      // which forces a recompile, rather than a current analysis paired with an outdated zip.
+      store.set(contents)
+      Def.declareOutput(analysisOut)
       s.log.debug(s"wrote $vfDir")
       (analysisResult.hasModified(), vfDir: VirtualFileRef, packedDir: HashedVirtualFileRef)
     }

sbt-app/src/sbt-test/cache/compile-repackage-skip/a/src/main/scala/A.scala

@@ -0,0 +1,4 @@
+package example
+
+object A:
+  def v: Int = 1

sbt-app/src/sbt-test/cache/compile-repackage-skip/b/src/main/scala/B.scala

@@ -0,0 +1,4 @@
+package example
+
+object B:
+  def w: Int = A.v + 1

sbt-app/src/sbt-test/cache/compile-repackage-skip/build.sbt

@@ -0,0 +1,78 @@
+import java.nio.file.{ Files, LinkOption }
+import java.nio.file.attribute.BasicFileAttributes
+
+val recordIds = taskKey[Unit]("records the on-disk identity of the compile outputs")
+val checkAnalysisChanged = taskKey[Unit]("asserts the analysis file was rewritten")
+val checkAnalysisUnchanged = taskKey[Unit]("asserts the analysis file was left alone")
+val checkClassesZipUnchanged = taskKey[Unit]("asserts the class directory was not re-packaged")
+val checkNotModified = taskKey[Unit]("asserts zinc recompiled nothing")
+val delClassesZip = taskKey[Unit]("deletes the sibling classes.sbtdir.zip")
+val checkClasses = taskKey[Unit]("asserts class files are present")
+
+// A rewritten output gets a fresh inode when the action cache relinks it into the CAS, and a fresh
+// mtime where symlinks are unavailable; either one moving means the file was written again.
+def idOf(f: File): String = {
+  val attrs =
+    Files.readAttributes(f.toPath, classOf[BasicFileAttributes], LinkOption.NOFOLLOW_LINKS)
+  s"${Option(attrs.fileKey()).getOrElse("<no-file-key>")}|${attrs.lastModifiedTime().toMillis}"
+}
+
+lazy val classesZip = Def.task {
+  val dir = (Compile / classDirectory).value
+  new File(dir.getParentFile, dir.getName + ".sbtdir.zip")
+}
+
+lazy val idFile = Def.task { target.value / "recorded-ids.txt" }
+
+def recorded(kind: String) = Def.task {
+  IO.readLines(idFile.value)
+    .collectFirst { case s"$k=$v" if k == kind => v }
+    .getOrElse(sys.error(s"no recorded id for $kind"))
+}
+
+Global / localCacheDirectory := (ThisBuild / baseDirectory).value / "diskcache"
+ThisBuild / scalaVersion := "3.8.4"
+ThisBuild / exportJars := true
+
+lazy val a = project.in(file("a"))
+
+lazy val b = project
+  .in(file("b"))
+  .dependsOn(a)
+  .settings(
+    recordIds := Def.uncached {
+      IO.writeLines(
+        idFile.value,
+        Seq(
+          s"analysis=${idOf((Compile / compileAnalysisFile).value)}",
+          s"classesZip=${idOf(classesZip.value)}",
+        )
+      )
+    },
+    checkAnalysisChanged := Def.uncached {
+      val now = idOf((Compile / compileAnalysisFile).value)
+      val before = recorded("analysis").value
+      assert(now != before, s"analysis was not rewritten, so compile was a cache hit: $now")
+    },
+    checkAnalysisUnchanged := Def.uncached {
+      val now = idOf((Compile / compileAnalysisFile).value)
+      val before = recorded("analysis").value
+      assert(now == before, s"analysis was rewritten: $before -> $now")
+    },
+    checkClassesZipUnchanged := Def.uncached {
+      val now = idOf(classesZip.value)
+      val before = recorded("classesZip").value
+      assert(now == before, s"class directory was re-packaged: $before -> $now")
+    },
+    checkNotModified := Def.uncached {
+      val (hasModified, _, _) = (Compile / compileIncremental).value
+      assert(!hasModified, "zinc reported modified output, expected nothing to recompile")
+    },
+    delClassesZip := Def.uncached {
+      IO.delete(classesZip.value)
+    },
+    checkClasses := Def.uncached {
+      val classes = ((Compile / classDirectory).value ** "*.class").get()
+      assert(classes.nonEmpty, "no class files")
+    },
+  )

sbt-app/src/sbt-test/cache/compile-repackage-skip/changes/A2.scala

@@ -0,0 +1,4 @@
+package example
+
+object A:
+  def v: Int = 2

sbt-app/src/sbt-test/cache/compile-repackage-skip/test

@@ -0,0 +1,26 @@
+# An upstream method body change invalidates a downstream module's compile cache key without
+# invalidating any of its classes. Zinc then recompiles nothing, so its class directory must not
+# be packaged into the action cache again.
+
+> b/compile
+> b/recordIds
+
+# a no-op build: b's compile is a pure cache hit, nothing on disk moves
+> b/compile
+> b/checkAnalysisUnchanged
+> b/checkClassesZipUnchanged
+
+# the body change alters a's jar, so b's cached compile task re-runs -- rewriting b's analysis --
+# but zinc recompiles nothing in b
+$ copy-file changes/A2.scala a/src/main/scala/A.scala
+> b/compile
+> b/checkNotModified
+> b/checkAnalysisChanged
+> b/checkClassesZipUnchanged
+
+# the entry stored on that miss still has to describe b's class directory: dropping the sibling
+# zip forces the next hit to fetch it back out of the content-addressed store and re-extract it
+> b/recordIds
+> b/delClassesZip
+> b/compile
+> b/checkClasses

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

@@ -384,6 +384,10 @@ object ActionCache:
     outputs += vf
     vf
 
+  /** The zip `packageDirectory` writes for `dirPath`, as a sibling of the directory itself. */
+  def dirZipPath(dirPath: Path): Path =
+    Paths.get(dirPath.toString + dirZipExt)
+
   def packageDirectory(
       dir: VirtualFileRef,
       conv: FileConverter,
@@ -411,7 +415,7 @@ object ActionCache:
     IO.withTemporaryDirectory: tempDir =>
       val mPath = (tempDir / manifestFileName).toPath()
       makeManifest(mPath)
-      val zipPath = Paths.get(dirPath.toString + dirZipExt)
+      val zipPath = dirZipPath(dirPath)
       val rebase: Path => Seq[(File, String)] =
         (p: Path) =>
           p match