[2.x] fix: Avoid rewriting unchanged plugin descriptors (#9612) (#9624) (#9628)

Only rewrite generated plugin descriptors when their normalized lines change, preserving the descriptor timestamp and downstream cache inputs for unchanged plugin discovery.

Add unit coverage plus direct packageBin and assembly Scripted scenarios. The descriptor regressions fail with the former unconditional write and pass with this change; the packageBin fixture also preserves SBT 2’s existing cache-hit behavior.

Co-authored-by: Dmitrii Naumenko <dimanaumenko1994@mail.ru>
Co-authored-by: Codex <noreply@openai.com>

main/src/main/scala/sbt/internal/PluginDiscovery.scala

@@ -87,7 +87,10 @@ object PluginDiscovery:
       IO.delete(descriptor)
       None
     } else {
-      IO.writeLines(descriptor, names.distinct.sorted)
+      val lines = names.distinct.sorted
+      // Do not invalidate timestamp-based downstream caches when the descriptor content is unchanged.
+      if (!descriptor.exists || IO.readLines(descriptor) != lines)
+        IO.writeLines(descriptor, lines)
       Some(descriptor)
     }
   }

main/src/test/scala/sbt/internal/PluginDiscoveryTest.scala

@@ -0,0 +1,55 @@
+/*
+ * sbt
+ * Copyright 2023, Scala center
+ * Copyright 2011 - 2022, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt.internal
+
+import java.nio.file.Files
+import java.nio.file.attribute.FileTime
+
+import sbt.io.IO
+import verify.BasicTestSuite
+
+object PluginDiscoveryTest extends BasicTestSuite:
+  import PluginDiscovery.Paths.AutoPlugins
+
+  test("writeDescriptor preserves mtime when the descriptor content is unchanged"):
+    val directory = Files.createTempDirectory("plugin-discovery-test").toFile
+    try
+      val descriptor =
+        PluginDiscovery.writeDescriptor(Seq("example.B", "example.A"), directory, AutoPlugins).get
+      val expectedTime = FileTime.fromMillis(1234L)
+      Files.setLastModifiedTime(descriptor.toPath, expectedTime)
+
+      PluginDiscovery.writeDescriptor(
+        Seq("example.A", "example.B", "example.A"),
+        directory,
+        AutoPlugins
+      )
+
+      assert(Files.getLastModifiedTime(descriptor.toPath) == expectedTime)
+    finally IO.delete(directory)
+
+  test("writeDescriptor updates the descriptor when its content changes"):
+    val directory = Files.createTempDirectory("plugin-discovery-test").toFile
+    try
+      val descriptor = PluginDiscovery.writeDescriptor(Seq("example.A"), directory, AutoPlugins).get
+
+      PluginDiscovery.writeDescriptor(Seq("example.B"), directory, AutoPlugins)
+
+      assert(IO.readLines(descriptor) == Seq("example.B"))
+    finally IO.delete(directory)
+
+  test("writeDescriptor deletes the descriptor when no modules are discovered"):
+    val directory = Files.createTempDirectory("plugin-discovery-test").toFile
+    try
+      val descriptor = PluginDiscovery.writeDescriptor(Seq("example.A"), directory, AutoPlugins).get
+
+      assert(PluginDiscovery.writeDescriptor(Nil, directory, AutoPlugins).isEmpty)
+      assert(!descriptor.exists)
+    finally IO.delete(directory)
+end PluginDiscoveryTest

sbt-app/src/sbt-test/package/sbt-plugin-incremental/build.sbt

@@ -0,0 +1,30 @@
+import java.nio.file.Files
+import java.nio.file.Paths
+
+sbtPlugin := true
+
+val recordPackageBinMtime = taskKey[Unit]("Records the packageBin output mtime")
+val checkPackageBinMtime = taskKey[Unit]("Checks that packageBin did not rewrite its output")
+val checkPackageBinRebuilt = taskKey[Unit]("Checks that packageBin rebuilt its output")
+
+recordPackageBinMtime := Def.uncached {
+  val artifact = fileConverter.value.toPath((Compile / packageBin / artifactPath).value)
+  val recordedMtime = Paths.get(System.getProperty("java.io.tmpdir"), s"package-bin-mtime-${baseDirectory.value.getAbsolutePath.hashCode}")
+  Files.writeString(recordedMtime, Files.getLastModifiedTime(artifact).toString)
+}
+
+checkPackageBinMtime := Def.uncached {
+  val artifact = fileConverter.value.toPath((Compile / packageBin / artifactPath).value)
+  val recordedMtime = Paths.get(System.getProperty("java.io.tmpdir"), s"package-bin-mtime-${baseDirectory.value.getAbsolutePath.hashCode}")
+  val expected = Files.readString(recordedMtime)
+  val actual = Files.getLastModifiedTime(artifact)
+  assert(actual.toString == expected, s"packageBin rewrote $artifact: $actual")
+}
+
+checkPackageBinRebuilt := Def.uncached {
+  val artifact = fileConverter.value.toPath((Compile / packageBin / artifactPath).value)
+  val recordedMtime = Paths.get(System.getProperty("java.io.tmpdir"), s"package-bin-mtime-${baseDirectory.value.getAbsolutePath.hashCode}")
+  val expected = Files.readString(recordedMtime)
+  val actual = Files.getLastModifiedTime(artifact)
+  assert(actual.toString != expected, s"packageBin did not rebuild $artifact")
+}

sbt-app/src/sbt-test/package/sbt-plugin-incremental/changes/AdditionalPlugin.scala

@@ -0,0 +1,5 @@
+package example
+
+import sbt.AutoPlugin
+
+object AdditionalPlugin extends AutoPlugin

sbt-app/src/sbt-test/package/sbt-plugin-incremental/src/main/scala/example/ExamplePlugin.scala

@@ -0,0 +1,5 @@
+package example
+
+import sbt.AutoPlugin
+
+object ExamplePlugin extends AutoPlugin

sbt-app/src/sbt-test/package/sbt-plugin-incremental/test

@@ -0,0 +1,8 @@
+> packageBin
+> recordPackageBinMtime
+> packageBin
+> checkPackageBinMtime
+
+$ copy-file changes/AdditionalPlugin.scala src/main/scala/example/AdditionalPlugin.scala
+> packageBin
+> checkPackageBinRebuilt

sbt-app/src/sbt-test/plugins/sbt-assembly/build.sbt

@@ -0,0 +1,57 @@
+import java.nio.file.Files
+import java.nio.file.Paths
+import java.nio.file.attribute.FileTime
+
+import sbtassembly.AssemblyPlugin.autoImport._
+
+val recordDescriptorMtime = taskKey[Unit]("Records the generated plugin descriptor mtime")
+val checkDescriptorMtime = taskKey[Unit]("Checks that assembly did not rewrite the generated plugin descriptor")
+val checkDescriptorRebuilt = taskKey[Unit]("Checks that assembly rewrote the generated plugin descriptor")
+val expectedAssemblyMtime = FileTime.fromMillis(1234L)
+val setAssemblyMtime = taskKey[Unit]("Sets the assembly output mtime")
+val checkAssemblyMtime = taskKey[Unit]("Checks that assembly did not rewrite its output")
+val checkAssemblyRebuilt = taskKey[Unit]("Checks that assembly rebuilt its output")
+
+sbtPlugin := true
+
+recordDescriptorMtime := Def.uncached {
+  val descriptor = (Compile / resourceManaged).value.toPath.resolve("sbt").resolve("sbt.autoplugins")
+  val recordedMtime =
+    Paths.get(System.getProperty("java.io.tmpdir"), s"plugin-descriptor-mtime-${baseDirectory.value.getAbsolutePath.hashCode}")
+  Files.writeString(recordedMtime, Files.getLastModifiedTime(descriptor).toString)
+}
+
+checkDescriptorMtime := Def.uncached {
+  val descriptor = (Compile / resourceManaged).value.toPath.resolve("sbt").resolve("sbt.autoplugins")
+  val recordedMtime =
+    Paths.get(System.getProperty("java.io.tmpdir"), s"plugin-descriptor-mtime-${baseDirectory.value.getAbsolutePath.hashCode}")
+  val expected = Files.readString(recordedMtime)
+  val actual = Files.getLastModifiedTime(descriptor)
+  assert(actual.toString == expected, s"assembly rewrote $descriptor: $actual")
+}
+
+checkDescriptorRebuilt := Def.uncached {
+  val descriptor = (Compile / resourceManaged).value.toPath.resolve("sbt").resolve("sbt.autoplugins")
+  val recordedMtime =
+    Paths.get(System.getProperty("java.io.tmpdir"), s"plugin-descriptor-mtime-${baseDirectory.value.getAbsolutePath.hashCode}")
+  val expected = Files.readString(recordedMtime)
+  val actual = Files.getLastModifiedTime(descriptor)
+  assert(actual.toString != expected, s"assembly did not rewrite $descriptor")
+}
+
+setAssemblyMtime := Def.uncached {
+  val artifact = (assembly / assemblyOutputPath).value
+  Files.setLastModifiedTime(artifact.toPath, expectedAssemblyMtime)
+}
+
+checkAssemblyMtime := Def.uncached {
+  val artifact = (assembly / assemblyOutputPath).value
+  val actual = Files.getLastModifiedTime(artifact.toPath)
+  assert(actual == expectedAssemblyMtime, s"assembly rewrote $artifact: $actual")
+}
+
+checkAssemblyRebuilt := Def.uncached {
+  val artifact = (assembly / assemblyOutputPath).value
+  val actual = Files.getLastModifiedTime(artifact.toPath)
+  assert(actual != expectedAssemblyMtime, s"assembly did not rebuild $artifact")
+}

sbt-app/src/sbt-test/plugins/sbt-assembly/changes/AdditionalPlugin.scala

@@ -0,0 +1,5 @@
+package example
+
+import sbt.AutoPlugin
+
+object AdditionalPlugin extends AutoPlugin

sbt-app/src/sbt-test/plugins/sbt-assembly/project/plugins.sbt

@@ -0,0 +1 @@
+addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "2.4.2")

sbt-app/src/sbt-test/plugins/sbt-assembly/src/main/scala/example/ExamplePlugin.scala

@@ -0,0 +1,5 @@
+package example
+
+import sbt.AutoPlugin
+
+object ExamplePlugin extends AutoPlugin

sbt-app/src/sbt-test/plugins/sbt-assembly/test

@@ -0,0 +1,11 @@
+> assembly
+> recordDescriptorMtime
+> setAssemblyMtime
+> assembly
+> checkDescriptorMtime
+> checkAssemblyMtime
+
+$ copy-file changes/AdditionalPlugin.scala src/main/scala/example/AdditionalPlugin.scala
+> assembly
+> checkDescriptorRebuilt
+> checkAssemblyRebuilt