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
This commit is contained in:
Ethan Atkins 2019-05-29 19:49:39 -07:00
parent b9fed2abfb
commit 9d8296fe49
5 changed files with 70 additions and 11 deletions

View File

@ -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

View File

@ -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"))
}

View File

@ -0,0 +1 @@
foo

View File

@ -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")
}
}
}

View File

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