From 9d8296fe491471fee358de8b31b9eeb5631e9b6b Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Wed, 29 May 2019 19:49:39 -0700 Subject: [PATCH] Fix indefinite recompilation @olegych reported in #4721 that play projects would get stuck in a strange loop where modifying any source file would cause that source file to always be recompiled every time a build was triggered regardless of whether or not it was modified. This was because the play project sets custom watchSources (using the legacy api) that overlap with the fileInputs. There were two parts to this fix: 1) When detecting an event, find if any of the dynamic inputs that cover the glob use a hash. If so, these are file inputs so we want to update the hash for the path, not the last modified time. 2) Only write hashes into the persistent file stamp cache. Computing the last modified time is much cheaper than the hash so it makes sense to avoid ever caching last modified times. I wrote a scripted test that fails if Continuous writes a last modified time into the file stamp cache instead of a hash. I also verified manually that a sample play project no longer exhibits the weird recompilation behavior. Fixes #4721 --- .../main/scala/sbt/internal/Continuous.scala | 37 +++++++++++++------ sbt/src/sbt-test/watch/overlapping/build.sbt | 26 +++++++++++++ .../sbt-test/watch/overlapping/files/foo.txt | 1 + .../watch/overlapping/project/Stamps.scala | 16 ++++++++ sbt/src/sbt-test/watch/overlapping/test | 1 + 5 files changed, 70 insertions(+), 11 deletions(-) create mode 100644 sbt/src/sbt-test/watch/overlapping/build.sbt create mode 100644 sbt/src/sbt-test/watch/overlapping/files/foo.txt create mode 100644 sbt/src/sbt-test/watch/overlapping/project/Stamps.scala create mode 100644 sbt/src/sbt-test/watch/overlapping/test diff --git a/main/src/main/scala/sbt/internal/Continuous.scala b/main/src/main/scala/sbt/internal/Continuous.scala index 080925606..41c59ba1e 100644 --- a/main/src/main/scala/sbt/internal/Continuous.scala +++ b/main/src/main/scala/sbt/internal/Continuous.scala @@ -27,6 +27,7 @@ import sbt.internal.nio._ import sbt.internal.util.complete.Parser._ import sbt.internal.util.complete.{ Parser, Parsers } import sbt.internal.util.{ AttributeKey, JLine, Util } +import sbt.nio.FileStamper.LastModified import sbt.nio.Keys.{ fileInputs, _ } import sbt.nio.Watch.{ Creation, Deletion, Update } import sbt.nio.file.FileAttributes @@ -531,7 +532,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { val onEvent: Event => Seq[(Watch.Event, Watch.Action)] = event => { val path = event.path - def watchEvent(stamper: FileStamper, forceTrigger: Boolean): Option[Watch.Event] = { + def watchEvent(forceTrigger: Boolean): Option[Watch.Event] = { if (!event.exists) { Some(Deletion(event)) fileStampCache.remove(event.path) match { @@ -539,7 +540,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { case _ => Some(Deletion(event)) } } else { - fileStampCache.update(path, stamper) match { + fileStampCache.update(path, FileStamper.Hash) match { case (None, Some(_)) => Some(Creation(event)) case (Some(_), None) => Some(Deletion(event)) case (Some(p), Some(c)) => @@ -563,23 +564,37 @@ private[sbt] object Continuous extends DeprecatedContinuous { } if (buildGlobs.exists(_.matches(path))) { - watchEvent(FileStamper.Hash, forceTrigger = false).map(e => e -> Watch.Reload).toSeq + watchEvent(forceTrigger = false).map(e => e -> Watch.Reload).toSeq } else { configs .flatMap { config => config .inputs() - .collectFirst { - case d if d.glob.matches(path) => (d.forceTrigger, true, d.fileStamper) - } - .flatMap { - case (forceTrigger, accepted, stamper) => - if (accepted) { - watchEvent(stamper, forceTrigger).flatMap { e => + .filter(_.glob.matches(path)) + .sortBy(_.fileStamper match { + case FileStamper.Hash => -1 + case FileStamper.LastModified => 0 + }) + .headOption + .flatMap { d => + val forceTrigger = d.forceTrigger + d.fileStamper match { + // We do not update the file stamp cache because we only want hashes in that + // cache or else it can mess up the external hooks that use that cache. + case LastModified => + logger.debug(s"Trigger path detected $path") + val watchEvent = + if (!event.exists) Deletion(event) + else if (fileStampCache.get(path).isDefined) Creation(event) + else Update(event) + val action = config.watchSettings.onFileInputEvent(count.get(), watchEvent) + Some(watchEvent -> action) + case _ => + watchEvent(forceTrigger).flatMap { e => val action = config.watchSettings.onFileInputEvent(count.get(), e) if (action != Watch.Ignore) Some(e -> action) else None } - } else None + } } } match { case events if events.isEmpty => Nil diff --git a/sbt/src/sbt-test/watch/overlapping/build.sbt b/sbt/src/sbt-test/watch/overlapping/build.sbt new file mode 100644 index 000000000..73eab1fe7 --- /dev/null +++ b/sbt/src/sbt-test/watch/overlapping/build.sbt @@ -0,0 +1,26 @@ +import java.nio.file.Files +import java.nio.file.attribute.FileTime + +import sbt.nio.Watch + +import scala.concurrent.duration._ + +val foo = taskKey[Unit]("foo.txt") +foo / watchForceTriggerOnAnyChange := true +foo / fileInputs := baseDirectory.value.toGlob / "files" / "foo.txt" :: Nil +foo / watchTriggers := baseDirectory.value.toGlob / ** / "foo.txt" :: Nil +foo := { + (foo / allInputFiles).value.foreach { p => + Files.setLastModifiedTime(p, FileTime.fromMillis(Files.getLastModifiedTime(p).toMillis + 3000)) + } + sbt.nio.Stamps.check(foo).value +} + +watchAntiEntropy := 0.seconds +watchOnFileInputEvent := { (count, event: Watch.Event) => + assert(event.path.getFileName.toString == "foo.txt") + if (new String(Files.readAllBytes(event.path)) == "foo") { + if (count < 3) Watch.Trigger + else Watch.CancelWatch + } else new Watch.HandleError(new IllegalStateException("Wrong stamp was set")) +} \ No newline at end of file diff --git a/sbt/src/sbt-test/watch/overlapping/files/foo.txt b/sbt/src/sbt-test/watch/overlapping/files/foo.txt new file mode 100644 index 000000000..191028156 --- /dev/null +++ b/sbt/src/sbt-test/watch/overlapping/files/foo.txt @@ -0,0 +1 @@ +foo \ No newline at end of file diff --git a/sbt/src/sbt-test/watch/overlapping/project/Stamps.scala b/sbt/src/sbt-test/watch/overlapping/project/Stamps.scala new file mode 100644 index 000000000..b8389d791 --- /dev/null +++ b/sbt/src/sbt-test/watch/overlapping/project/Stamps.scala @@ -0,0 +1,16 @@ +package sbt +package nio + +import java.nio.file._ +import sbt.nio.Keys._ + +object Stamps { + def check(key: TaskKey[_]): Def.Initialize[Task[Unit]] = Def.task { + (key / inputFileStamps).value.map { + case (p, FileStamp.Hash(_)) => + case (p, _) => + Files.write(p, "fail".getBytes) + throw new IllegalStateException("Got incorrect stamp: $s") + } + } +} \ No newline at end of file diff --git a/sbt/src/sbt-test/watch/overlapping/test b/sbt/src/sbt-test/watch/overlapping/test new file mode 100644 index 000000000..e51dd3f49 --- /dev/null +++ b/sbt/src/sbt-test/watch/overlapping/test @@ -0,0 +1 @@ +> ~foo \ No newline at end of file