Merge pull request #4742 from eatkins/unmanaged-source-cache

Improve managed file management in watch

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

@@ -148,7 +148,6 @@ object Defaults extends BuildCommon {
   private[sbt] lazy val globalCore: Seq[Setting[_]] = globalDefaults(
     defaultTestTasks(test) ++ defaultTestTasks(testOnly) ++ defaultTestTasks(testQuick) ++ Seq(
       excludeFilter :== HiddenFileFilter,
-      pathToFileStamp :== sbt.nio.FileStamp.hash,
       fileInputs :== Nil,
       inputFileStamper :== sbt.nio.FileStamper.Hash,
       outputFileStamper :== sbt.nio.FileStamper.LastModified,
@@ -157,11 +156,9 @@ object Defaults extends BuildCommon {
       watchPersistFileStamps :== true,
       watchTriggers :== Nil,
       clean := { () },
-      sbt.nio.Keys.fileStampCache := {
-        state.value
-          .get(sbt.nio.Keys.persistentFileStampCache)
-          .getOrElse(new sbt.nio.FileStamp.Cache)
-      },
+      unmanagedFileStampCache :=
+        state.value.get(persistentFileStampCache).getOrElse(new sbt.nio.FileStamp.Cache),
+      managedFileStampCache := new sbt.nio.FileStamp.Cache,
     ) ++ globalIvyCore ++ globalJvmCore
   ) ++ globalSbtCore
 
@@ -420,9 +417,12 @@ object Defaults extends BuildCommon {
     unmanagedSources := (unmanagedSources / inputFileStamps).value.map(_._1.toFile),
     managedSourceDirectories := Seq(sourceManaged.value),
     managedSources := {
-      val stamper = sbt.nio.Keys.pathToFileStamp.value
+      val stamper = inputFileStamper.value
+      val cache = managedFileStampCache.value
       val res = generate(sourceGenerators).value
-      res.foreach(f => stamper(f.toPath))
+      res.foreach { f =>
+        cache.putIfAbsent(f.toPath, stamper)
+      }
       res
     },
     sourceGenerators :== Nil,
@@ -1647,7 +1647,7 @@ object Defaults extends BuildCommon {
       val contents = AnalysisContents.create(analysisResult.analysis(), analysisResult.setup())
       store.set(contents)
     }
-    val map = sbt.nio.Keys.fileStampCache.value
+    val map = managedFileStampCache.value
     val analysis = analysisResult.analysis
     import scala.collection.JavaConverters._
     analysis.readStamps.getAllProductStamps.asScala.foreach {

main/src/main/scala/sbt/internal/ExternalHooks.scala

@@ -28,21 +28,23 @@ import scala.collection.mutable
 private[sbt] object ExternalHooks {
   private val javaHome = Option(System.getProperty("java.home")).map(Paths.get(_))
   def default: Def.Initialize[sbt.Task[ExternalHooks]] = Def.task {
-    val cache = fileStampCache.value
+    val unmanagedCache = unmanagedFileStampCache.value
+    val managedCache = managedFileStampCache.value
     val cp = dependencyClasspath.value.map(_.data)
     cp.foreach { file =>
       val path = file.toPath
-      cache.getOrElseUpdate(path, FileStamper.LastModified)
+      managedCache.getOrElseUpdate(path, FileStamper.LastModified)
     }
     val classGlob = classDirectory.value.toGlob / RecursiveGlob / "*.class"
     fileTreeView.value.list(classGlob).foreach {
-      case (path, _) => cache.update(path, FileStamper.LastModified)
+      case (path, _) => managedCache.update(path, FileStamper.LastModified)
     }
-    apply((compileOptions in compile).value, cache)
+    apply((compileOptions in compile).value, unmanagedCache, managedCache)
   }
   private def apply(
       options: CompileOptions,
-      fileStampCache: FileStamp.Cache
+      unmanagedCache: FileStamp.Cache,
+      managedCache: FileStamp.Cache
   ): DefaultExternalHooks = {
     val lookup = new ExternalLookup {
       override def changedSources(previousAnalysis: CompileAnalysis): Option[Changes[File]] = Some {
@@ -57,7 +59,10 @@ private[sbt] object ExternalHooks {
             previousAnalysis.readStamps().getAllSourceStamps.asScala
           prevSources.foreach {
             case (file: File, s: Stamp) =>
-              fileStampCache.getOrElseUpdate(file.toPath, FileStamper.Hash) match {
+              val path = file.toPath
+              unmanagedCache
+                .get(path)
+                .orElse(managedCache.getOrElseUpdate(file.toPath, FileStamper.Hash)) match {
                 case None => getRemoved.add(file)
                 case Some(stamp) =>
                   if (equiv(stamp.stamp, s)) getUnmodified.add(file) else getChanged.add(file)
@@ -79,7 +84,7 @@ private[sbt] object ExternalHooks {
       override def changedBinaries(previousAnalysis: CompileAnalysis): Option[Set[File]] = {
         Some(previousAnalysis.readStamps.getAllBinaryStamps.asScala.flatMap {
           case (file, stamp) =>
-            fileStampCache.get(file.toPath) match {
+            managedCache.get(file.toPath) match {
               case Some(cachedStamp) if equiv(cachedStamp.stamp, stamp) => None
               case _ =>
                 javaHome match {
@@ -94,7 +99,7 @@ private[sbt] object ExternalHooks {
       override def removedProducts(previousAnalysis: CompileAnalysis): Option[Set[File]] = {
         Some(previousAnalysis.readStamps.getAllProductStamps.asScala.flatMap {
           case (file, stamp) =>
-            fileStampCache.get(file.toPath) match {
+            managedCache.get(file.toPath) match {
               case Some(s) if equiv(s.stamp, stamp) => None
               case _                                => Some(file)
             }

main/src/main/scala/sbt/nio/FileStamp.scala

@@ -9,6 +9,7 @@ package sbt.nio
 
 import java.io.{ File, IOException }
 import java.nio.file.{ Path, Paths }
+import java.util.concurrent.ConcurrentHashMap
 
 import sbt.internal.inc.{ EmptyStamp, Stamper, LastModified => IncLastModified }
 import sbt.io.IO
@@ -217,7 +218,7 @@ private[sbt] object FileStamp {
   }
 
   private[sbt] class Cache {
-    private[this] val underlying = new java.util.HashMap[Path, Either[FileStamp, FileStamp]]
+    private[this] val underlying = new ConcurrentHashMap[Path, Either[FileStamp, FileStamp]]
 
     /**
      * Invalidate the cache entry, but don't re-stamp the file until it's actually used
@@ -251,6 +252,14 @@ private[sbt] object FileStamp {
         case null => None
         case e    => e.value
       }
+
+    def putIfAbsent(key: Path, stamper: FileStamper): Unit = {
+      underlying.get(key) match {
+        case null => updateImpl(key, stamper)
+        case _    =>
+      }
+      ()
+    }
     def update(key: Path, stamper: FileStamper): (Option[FileStamp], Option[FileStamp]) = {
       underlying.get(key) match {
         case null => (None, updateImpl(key, stamper))

main/src/main/scala/sbt/nio/Keys.scala

@@ -142,11 +142,11 @@ object Keys {
   private[sbt] val allInputPathsAndAttributes =
     taskKey[Seq[(Path, FileAttributes)]]("Get all of the file inputs for a task")
       .withRank(Invisible)
-  private[sbt] val fileStampCache = taskKey[FileStamp.Cache](
-    "Map of file stamps that may be cleared between task evaluation runs."
+  private[sbt] val unmanagedFileStampCache = taskKey[FileStamp.Cache](
+    "Map of managed file stamps that may be cleared between task evaluation runs."
   ).withRank(Invisible)
-  private[sbt] val pathToFileStamp = taskKey[Path => Option[FileStamp]](
-    "A function that computes a file stamp for a path. It may have the side effect of updating a cache."
+  private[sbt] val managedFileStampCache = taskKey[FileStamp.Cache](
+    "Map of managed file stamps that may be cleared between task evaluation runs."
   ).withRank(Invisible)
   private[sbt] val classpathFiles =
     taskKey[Seq[Path]]("The classpath for a task.").withRank(Invisible)

main/src/main/scala/sbt/nio/Settings.scala

@@ -144,8 +144,7 @@ private[sbt] object Settings {
         (transitiveClasspathDependency in scopedKey.scope := { () }) :: Nil
       case changedOutputFiles.key =>
         changedFilesImpl(scopedKey, changedOutputFiles, outputFileStamps)
-      case pathToFileStamp.key => stamper(scopedKey) :: Nil
-      case _                   => Nil
+      case _ => Nil
     }
 
   /**
@@ -318,10 +317,12 @@ private[sbt] object Settings {
    */
   private[sbt] def fileStamps(scopedKey: Def.ScopedKey[_]): Def.Setting[_] =
     addTaskDefinition(Keys.inputFileStamps in scopedKey.scope := {
-      val stamper = (Keys.pathToFileStamp in scopedKey.scope).value
+      val cache = (unmanagedFileStampCache in scopedKey.scope).value
+      val stamper = (Keys.inputFileStamper in scopedKey.scope).value
       (Keys.allInputPathsAndAttributes in scopedKey.scope).value.flatMap {
-        case (p, a) if a.isRegularFile && !Files.isHidden(p) => stamper(p).map(p -> _)
-        case _                                               => None
+        case (p, a) if a.isRegularFile && !Files.isHidden(p) =>
+          cache.getOrElseUpdate(p, stamper).map(p -> _)
+        case _ => None
       }
     })
   private[this] def outputsAndStamps[T: JsonFormat: ToSeqPath](
@@ -348,17 +349,4 @@ private[sbt] object Settings {
       (allOutputFiles in scope).value.flatMap(p => stamper(p).map(p -> _))
     })
 
-  /**
-   * Returns a function from `Path` to [[FileStamp]] that can be used by tasks to retrieve
-   * the stamp for a file. It has the side effect of stamping the file if it has not already
-   * been stamped during the task evaluation.
-   *
-   * @return a task definition for a function from `Path` to [[FileStamp]].
-   */
-  private[this] def stamper(scopedKey: Def.ScopedKey[_]): Def.Setting[_] =
-    addTaskDefinition((Keys.pathToFileStamp in scopedKey.scope) := {
-      val attributeMap = Keys.fileStampCache.value
-      val stamper = (Keys.inputFileStamper in scopedKey.scope).value
-      path: Path => attributeMap.getOrElseUpdate(path, stamper)
-    })
 }

sbt/src/sbt-test/watch/managed/build.sbt

@@ -0,0 +1,23 @@
+import java.nio.file.Files
+
+import sbt.nio.Watch
+
+import scala.concurrent.duration._
+
+Compile / sourceGenerators += Def.task {
+  baseDirectory.value / "sources" / "Write.scala" :: Nil
+}.taskValue
+
+val runTest = taskKey[Unit]("run the test")
+runTest := Def.taskDyn {
+  val args = s" ${baseDirectory.value}"
+  (Runtime / run).toTask(args)
+}.value
+
+runTest / watchTriggers += baseDirectory.value.toGlob / "*.txt"
+watchAntiEntropy := 0.milliseconds
+watchOnFileInputEvent := { (count, e) =>
+  if (new String(Files.readAllBytes(e.path)) == "ok") Watch.CancelWatch
+  else if (count < 2) Watch.Trigger
+  else new Watch.HandleError(new IllegalStateException(s"Wrong event triggered the build: $e"))
+}

sbt/src/sbt-test/watch/managed/changes/Write.scala

@@ -0,0 +1,7 @@
+import java.nio.file._
+
+object Write {
+  def main(args: Array[String]): Unit = {
+    Files.write(Paths.get(args(0)).resolve("output.txt"), "ok".getBytes)
+  }
+}

sbt/src/sbt-test/watch/managed/sources/Write.scala

@@ -0,0 +1,12 @@
+import java.nio.file._
+object Write {
+  def main(args: Array[String]): Unit = {
+    val dir = Paths.get(args(0))
+    Files.write(
+      dir.resolve("sources").resolve("Write.scala"),
+      Files.readAllBytes(dir.resolve("changes").resolve("Write.scala")),
+    )
+    val output = dir.resolve("output.txt")
+    Files.write(output, "failure".getBytes)
+  }
+}

sbt/src/sbt-test/watch/managed/test

@@ -0,0 +1 @@
+> ~runTest

scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala

@@ -232,6 +232,7 @@ final class ScriptedTests(
       case "source-dependencies/linearization"           => LauncherBased // sbt/Package$
       case "source-dependencies/named"                   => LauncherBased // sbt/Package$
       case "source-dependencies/specialized"             => LauncherBased // sbt/Package$
+      case "watch/managed"                               => LauncherBased // sbt/Package$
       case "tests/test-cross" =>
         LauncherBased // the sbt metabuild classpath leaks into the test interface classloader in older versions of sbt
       case _ => RunFromSourceBased