From cdf6b2510645a138aa63e48c0c7e03c24b1f86cd Mon Sep 17 00:00:00 2001 From: eugene yokota Date: Wed, 19 Aug 2026 13:05:17 -0400 Subject: [PATCH] [2.x] fix: Avoid rewriting unchanged plugin descriptors (#9612) (#9624) (#9628) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-authored-by: Codex --- .../scala/sbt/internal/PluginDiscovery.scala | 5 +- .../sbt/internal/PluginDiscoveryTest.scala | 55 ++++++++++++++++++ .../package/sbt-plugin-incremental/build.sbt | 30 ++++++++++ .../changes/AdditionalPlugin.scala | 5 ++ .../main/scala/example/ExamplePlugin.scala | 5 ++ .../package/sbt-plugin-incremental/test | 8 +++ .../sbt-test/plugins/sbt-assembly/build.sbt | 57 +++++++++++++++++++ .../changes/AdditionalPlugin.scala | 5 ++ .../plugins/sbt-assembly/project/plugins.sbt | 1 + .../main/scala/example/ExamplePlugin.scala | 5 ++ .../src/sbt-test/plugins/sbt-assembly/test | 11 ++++ 11 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 main/src/test/scala/sbt/internal/PluginDiscoveryTest.scala create mode 100644 sbt-app/src/sbt-test/package/sbt-plugin-incremental/build.sbt create mode 100644 sbt-app/src/sbt-test/package/sbt-plugin-incremental/changes/AdditionalPlugin.scala create mode 100644 sbt-app/src/sbt-test/package/sbt-plugin-incremental/src/main/scala/example/ExamplePlugin.scala create mode 100644 sbt-app/src/sbt-test/package/sbt-plugin-incremental/test create mode 100644 sbt-app/src/sbt-test/plugins/sbt-assembly/build.sbt create mode 100644 sbt-app/src/sbt-test/plugins/sbt-assembly/changes/AdditionalPlugin.scala create mode 100644 sbt-app/src/sbt-test/plugins/sbt-assembly/project/plugins.sbt create mode 100644 sbt-app/src/sbt-test/plugins/sbt-assembly/src/main/scala/example/ExamplePlugin.scala create mode 100644 sbt-app/src/sbt-test/plugins/sbt-assembly/test diff --git a/main/src/main/scala/sbt/internal/PluginDiscovery.scala b/main/src/main/scala/sbt/internal/PluginDiscovery.scala index 934a0a53d..134768678 100644 --- a/main/src/main/scala/sbt/internal/PluginDiscovery.scala +++ b/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) } } diff --git a/main/src/test/scala/sbt/internal/PluginDiscoveryTest.scala b/main/src/test/scala/sbt/internal/PluginDiscoveryTest.scala new file mode 100644 index 000000000..db7c67877 --- /dev/null +++ b/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 diff --git a/sbt-app/src/sbt-test/package/sbt-plugin-incremental/build.sbt b/sbt-app/src/sbt-test/package/sbt-plugin-incremental/build.sbt new file mode 100644 index 000000000..c9f76a6f0 --- /dev/null +++ b/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") +} diff --git a/sbt-app/src/sbt-test/package/sbt-plugin-incremental/changes/AdditionalPlugin.scala b/sbt-app/src/sbt-test/package/sbt-plugin-incremental/changes/AdditionalPlugin.scala new file mode 100644 index 000000000..5a29e69ba --- /dev/null +++ b/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 diff --git a/sbt-app/src/sbt-test/package/sbt-plugin-incremental/src/main/scala/example/ExamplePlugin.scala b/sbt-app/src/sbt-test/package/sbt-plugin-incremental/src/main/scala/example/ExamplePlugin.scala new file mode 100644 index 000000000..9f65abd01 --- /dev/null +++ b/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 diff --git a/sbt-app/src/sbt-test/package/sbt-plugin-incremental/test b/sbt-app/src/sbt-test/package/sbt-plugin-incremental/test new file mode 100644 index 000000000..0e4f1acb5 --- /dev/null +++ b/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 diff --git a/sbt-app/src/sbt-test/plugins/sbt-assembly/build.sbt b/sbt-app/src/sbt-test/plugins/sbt-assembly/build.sbt new file mode 100644 index 000000000..6db649d51 --- /dev/null +++ b/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") +} diff --git a/sbt-app/src/sbt-test/plugins/sbt-assembly/changes/AdditionalPlugin.scala b/sbt-app/src/sbt-test/plugins/sbt-assembly/changes/AdditionalPlugin.scala new file mode 100644 index 000000000..5a29e69ba --- /dev/null +++ b/sbt-app/src/sbt-test/plugins/sbt-assembly/changes/AdditionalPlugin.scala @@ -0,0 +1,5 @@ +package example + +import sbt.AutoPlugin + +object AdditionalPlugin extends AutoPlugin diff --git a/sbt-app/src/sbt-test/plugins/sbt-assembly/project/plugins.sbt b/sbt-app/src/sbt-test/plugins/sbt-assembly/project/plugins.sbt new file mode 100644 index 000000000..a6be0fd1e --- /dev/null +++ b/sbt-app/src/sbt-test/plugins/sbt-assembly/project/plugins.sbt @@ -0,0 +1 @@ +addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "2.4.2") diff --git a/sbt-app/src/sbt-test/plugins/sbt-assembly/src/main/scala/example/ExamplePlugin.scala b/sbt-app/src/sbt-test/plugins/sbt-assembly/src/main/scala/example/ExamplePlugin.scala new file mode 100644 index 000000000..9f65abd01 --- /dev/null +++ b/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 diff --git a/sbt-app/src/sbt-test/plugins/sbt-assembly/test b/sbt-app/src/sbt-test/plugins/sbt-assembly/test new file mode 100644 index 000000000..2d6a4b2b4 --- /dev/null +++ b/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