From bd4d04d13152d892afe215571c69600e59a65922 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Mon, 26 Aug 2019 14:36:23 -0700 Subject: [PATCH] Store compile file stamps for each scala version https://github.com/sbt/sbt/issues/4986 reported that +compile would always recompile everything in the project even when the sources hadn't changed. This was because the dependency classpath was changing between calls to compile, which caused the external hooks cache introduced in 32a6d0d5d76716c7e85f05ea66bc9cd639611918 to invalidate the scala library. To fix this, I cache the file stamps on a per scala version basis. I added a scripted test that checks that there is no recompilation in two consecutive calls to `+compile` in a multi scala version build. It failed prior to these changes. --- main/src/main/scala/sbt/Defaults.scala | 32 +++++++++++++------ main/src/main/scala/sbt/nio/Keys.scala | 6 ++++ .../actions/cross-incremental/build.sbt | 22 +++++++++++++ .../project/src/main/scala/Stamps.scala | 3 ++ .../cross-incremental/src/main/scala/A.scala | 3 ++ .../sbt-test/actions/cross-incremental/test | 7 ++++ .../sbt/scriptedtest/ScriptedTests.scala | 1 + 7 files changed, 64 insertions(+), 10 deletions(-) create mode 100644 sbt/src/sbt-test/actions/cross-incremental/build.sbt create mode 100644 sbt/src/sbt-test/actions/cross-incremental/project/src/main/scala/Stamps.scala create mode 100644 sbt/src/sbt-test/actions/cross-incremental/src/main/scala/A.scala create mode 100644 sbt/src/sbt-test/actions/cross-incremental/test diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 056bda08f..be0fb5a42 100755 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -47,6 +47,7 @@ import sbt.internal.server.{ LanguageServerReporter, ServerHandler } +import sbt.nio.FileStamp.Formats.seqPathFileStampJsonFormatter import sbt.internal.testing.TestLogger import sbt.internal.util.Attributed.data import sbt.internal.util.Types._ @@ -617,31 +618,42 @@ object Defaults extends BuildCommon { s"inc_compile$extra.zip" }, externalHooks := { - import sbt.nio.FileStamp.Formats.seqPathFileStampJsonFormatter + import sjsonnew.BasicJsonProtocol.mapFormat val currentInputs = (unmanagedSources / inputFileStamps).value ++ (managedSourcePaths / outputFileStamps).value - val previousInputs = (externalHooks / inputFileStamps).previous + val sv = scalaVersion.value + val previousInputs = compileSourceFileInputs.previous.flatMap(_.get(sv)) val inputChanges = previousInputs .map(sbt.nio.Settings.changedFiles(_, currentInputs)) .getOrElse(FileChanges.noPrevious(currentInputs.map(_._1))) val currentOutputs = (dependencyClasspathFiles / outputFileStamps).value - val previousOutputs = (externalHooks / outputFileStamps).previous + val previousOutputs = compileBinaryFileInputs.previous.flatMap(_.get(sv)) val outputChanges = previousOutputs .map(sbt.nio.Settings.changedFiles(_, currentOutputs)) .getOrElse(FileChanges.noPrevious(currentOutputs.map(_._1))) ExternalHooks.default.value(inputChanges, outputChanges, fileTreeView.value) }, - externalHooks / inputFileStamps := { + compileSourceFileInputs := { + import sjsonnew.BasicJsonProtocol.mapFormat compile.value // ensures the inputFileStamps previous value is only set if compile succeeds. - (unmanagedSources / inputFileStamps).value ++ (managedSourcePaths / outputFileStamps).value + val version = scalaVersion.value + val versions = crossScalaVersions.value.toSet + version + val prev: Map[String, Seq[(java.nio.file.Path, sbt.nio.FileStamp)]] = + compileSourceFileInputs.previous.map(_.filterKeys(versions)).getOrElse(Map.empty) + prev + (version -> + ((unmanagedSources / inputFileStamps).value ++ (managedSourcePaths / outputFileStamps).value)) }, - externalHooks / inputFileStamps := (externalHooks / inputFileStamps).triggeredBy(compile).value, - externalHooks / outputFileStamps := { + compileSourceFileInputs := compileSourceFileInputs.triggeredBy(compile).value, + compileBinaryFileInputs := { + import sjsonnew.BasicJsonProtocol.mapFormat compile.value // ensures the inputFileStamps previous value is only set if compile succeeds. - (dependencyClasspathFiles / outputFileStamps).value + val version = scalaVersion.value + val versions = crossScalaVersions.value.toSet + version + val prev: Map[String, Seq[(java.nio.file.Path, sbt.nio.FileStamp)]] = + compileBinaryFileInputs.previous.map(_.filterKeys(versions)).getOrElse(Map.empty) + prev + (version -> (dependencyClasspathFiles / outputFileStamps).value) }, - externalHooks / outputFileStamps := - (externalHooks / outputFileStamps).triggeredBy(compile).value, + compileBinaryFileInputs := compileBinaryFileInputs.triggeredBy(compile).value, incOptions := { incOptions.value.withExternalHooks(externalHooks.value) }, compileIncSetup := compileIncSetupTask.value, console := consoleTask.value, diff --git a/main/src/main/scala/sbt/nio/Keys.scala b/main/src/main/scala/sbt/nio/Keys.scala index 1f4d85fdf..24983771e 100644 --- a/main/src/main/scala/sbt/nio/Keys.scala +++ b/main/src/main/scala/sbt/nio/Keys.scala @@ -168,6 +168,12 @@ object Keys { private[sbt] val classpathFiles = taskKey[Seq[Path]]("The classpath for a task.").withRank(Invisible) private[sbt] val compileOutputs = taskKey[Seq[Path]]("Compilation outputs").withRank(Invisible) + private[sbt] val compileSourceFileInputs = + taskKey[Map[String, Seq[(Path, FileStamp)]]]("Source file stamps stored by scala version") + .withRank(Invisible) + private[sbt] val compileBinaryFileInputs = + taskKey[Map[String, Seq[(Path, FileStamp)]]]("Source file stamps stored by scala version") + .withRank(Invisible) private[this] val hasCheckedMetaBuildMsg = "Indicates whether or not we have called the checkBuildSources task. This is to avoid warning " + diff --git a/sbt/src/sbt-test/actions/cross-incremental/build.sbt b/sbt/src/sbt-test/actions/cross-incremental/build.sbt new file mode 100644 index 000000000..39c22fb65 --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-incremental/build.sbt @@ -0,0 +1,22 @@ +scalaVersion := "2.12.8" +crossScalaVersions := List("2.12.8", "2.13.0") + +val setLastModified = taskKey[Unit]("Sets the last modified time for classfiles") +setLastModified := { + val versions = crossScalaVersions.value + versions.map(_.split('.').take(2).mkString("scala-", ".", "")).foreach { v => + val f = target.value / v / "classes" / "A.class" + Stamps.value.put(f, IO.getModifiedTimeOrZero(f)) + } +} + +val checkLastModified = taskKey[Unit]("Checks the last modified time for classfiles") +checkLastModified := { + val versions = crossScalaVersions.value + versions.map(_.split('.').take(2).mkString("scala-", ".", "")).foreach { v => + val classFile = target.value / v / "classes" / "A.class" + val actual = IO.getModifiedTimeOrZero(classFile) + val previous = Stamps.value.get(classFile) + assert(actual == previous, s"$actual did not equal $previous for $classFile") + } +} diff --git a/sbt/src/sbt-test/actions/cross-incremental/project/src/main/scala/Stamps.scala b/sbt/src/sbt-test/actions/cross-incremental/project/src/main/scala/Stamps.scala new file mode 100644 index 000000000..e39d1a4fb --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-incremental/project/src/main/scala/Stamps.scala @@ -0,0 +1,3 @@ +object Stamps { + val value = new java.util.HashMap[java.io.File, Long] +} \ No newline at end of file diff --git a/sbt/src/sbt-test/actions/cross-incremental/src/main/scala/A.scala b/sbt/src/sbt-test/actions/cross-incremental/src/main/scala/A.scala new file mode 100644 index 000000000..3c8bfdd75 --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-incremental/src/main/scala/A.scala @@ -0,0 +1,3 @@ +object A { + def a = 1 +} \ No newline at end of file diff --git a/sbt/src/sbt-test/actions/cross-incremental/test b/sbt/src/sbt-test/actions/cross-incremental/test new file mode 100644 index 000000000..036f3dca0 --- /dev/null +++ b/sbt/src/sbt-test/actions/cross-incremental/test @@ -0,0 +1,7 @@ +> +compile + +> setLastModified + +> +compile + +> checkLastModified diff --git a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala index 3afdc0c82..637ebdce6 100644 --- a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala +++ b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala @@ -184,6 +184,7 @@ final class ScriptedTests( val (group, name) = testName s"$group/$name" match { case "actions/add-alias" => LauncherBased // sbt/Package$ + case "actions/cross-incremental" => LauncherBased // tbd case "actions/cross-multiproject" => LauncherBased // tbd case "actions/cross-multi-parser" => LauncherBased // java.lang.ClassNotFoundException: javax.tools.DiagnosticListener when run with java 11 and an old sbt launcher