diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 3f3f1cd80..116427c4d 100644 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/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) } diff --git a/sbt-app/src/sbt-test/cache/compile-repackage-skip/a/src/main/scala/A.scala b/sbt-app/src/sbt-test/cache/compile-repackage-skip/a/src/main/scala/A.scala new file mode 100644 index 000000000..838aa7852 --- /dev/null +++ b/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 diff --git a/sbt-app/src/sbt-test/cache/compile-repackage-skip/b/src/main/scala/B.scala b/sbt-app/src/sbt-test/cache/compile-repackage-skip/b/src/main/scala/B.scala new file mode 100644 index 000000000..4fc2fc35c --- /dev/null +++ b/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 diff --git a/sbt-app/src/sbt-test/cache/compile-repackage-skip/build.sbt b/sbt-app/src/sbt-test/cache/compile-repackage-skip/build.sbt new file mode 100644 index 000000000..cd76092f2 --- /dev/null +++ b/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("")}|${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") + }, + ) diff --git a/sbt-app/src/sbt-test/cache/compile-repackage-skip/changes/A2.scala b/sbt-app/src/sbt-test/cache/compile-repackage-skip/changes/A2.scala new file mode 100644 index 000000000..6e6a4c8e3 --- /dev/null +++ b/sbt-app/src/sbt-test/cache/compile-repackage-skip/changes/A2.scala @@ -0,0 +1,4 @@ +package example + +object A: + def v: Int = 2 diff --git a/sbt-app/src/sbt-test/cache/compile-repackage-skip/test b/sbt-app/src/sbt-test/cache/compile-repackage-skip/test new file mode 100644 index 000000000..3d291bb2c --- /dev/null +++ b/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 diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 99d1ae961..1e5132063 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/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