From b5ff4bda948d899d9cd8347f3949c472e13ac4b2 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Sat, 1 Jun 2019 13:03:40 -0700 Subject: [PATCH 1/3] Optimize imports --- main/src/main/scala/sbt/Main.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/src/main/scala/sbt/Main.scala b/main/src/main/scala/sbt/Main.scala index a1a54f095..3447ce8ba 100644 --- a/main/src/main/scala/sbt/Main.scala +++ b/main/src/main/scala/sbt/Main.scala @@ -9,7 +9,7 @@ package sbt import java.io.{ File, IOException } import java.net.URI -import java.util.concurrent.{ Executors, ForkJoinPool } +import java.util.concurrent.ForkJoinPool import java.util.concurrent.atomic.AtomicBoolean import java.util.{ Locale, Properties } From df5f9ae3cb3c1e467cfb4fbb514cf94f59b1acf9 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Sat, 1 Jun 2019 13:07:43 -0700 Subject: [PATCH 2/3] Support commands in continuous I had previously not though there was much reason to support commands in continuous builds. This was primarily because there were a number of questions regarding semantics. Commands cannot have fileInputs specifically assigned to them because they don't have an associated scope. They also can arbitrarily modify state so what is the expectation when running ~foo where foo is a command that, for example, replaces itself. I settled on the following semantics: 1) Commands run in a continuous build cannot modify the sbt execution state which is to say that the state that is returned by continuous is the same that was passed in (unless a reload occurred or we exited the build with an exception) 2) Any global watchTriggers or fileInputs apply to a watched command. They automatically inherit any fileInputs that are queried when running tasks in a command. So, for example, ~+compile does what you'd expect. The implementation is fairly straightforward. If we can successfully parse a command, but we cannot parse a scopedKey from it, we assign it a private ScopedKey. When computing the watch settings for that key, we will select the global settings through delegation. This is how it picks up the global watchTriggers. To run the command, I had to rework the task evaluation portion because a command may return a state with additional commands to run. The cross build command works this way. We recursively run all of the commands starting with the original until we run out of commands to run. As part of this work, I was able to remove the three argument version of Command.processCommand that I'd previously added to support my old approach to evaluating commands. This was a nice bonus. I added scripted tests that check that global watchTriggers are picked up and that commands that delegate to a command that uses fileInputs automatically pick up those inputs during the watch. I also added a test that approximates the ~+compile use case and ensures that the failure semantics are what we expect and that the task runs for all defined scala versions. --- main/src/main/scala/sbt/MainLoop.scala | 11 +--- .../main/scala/sbt/internal/Continuous.scala | 56 ++++++++++++------- sbt/src/sbt-test/watch/commands/build.sbt | 56 +++++++++++++++++++ .../commands/src/main/scala-2.11/Foo.scala | 1 + .../commands/src/main/scala-2.12/Foo.scala | 1 + .../watch/commands/src/main/scala/Bar.scala | 1 + sbt/src/sbt-test/watch/commands/test | 6 ++ .../sbt/scriptedtest/ScriptedTests.scala | 4 +- 8 files changed, 105 insertions(+), 31 deletions(-) create mode 100644 sbt/src/sbt-test/watch/commands/build.sbt create mode 100644 sbt/src/sbt-test/watch/commands/src/main/scala-2.11/Foo.scala create mode 100644 sbt/src/sbt-test/watch/commands/src/main/scala-2.12/Foo.scala create mode 100644 sbt/src/sbt-test/watch/commands/src/main/scala/Bar.scala create mode 100644 sbt/src/sbt-test/watch/commands/test diff --git a/main/src/main/scala/sbt/MainLoop.scala b/main/src/main/scala/sbt/MainLoop.scala index 8b8c1763b..8e9558a1b 100644 --- a/main/src/main/scala/sbt/MainLoop.scala +++ b/main/src/main/scala/sbt/MainLoop.scala @@ -177,21 +177,14 @@ object MainLoop { } /** This is the main function State transfer function of the sbt command processing. */ - def processCommand(exec: Exec, state: State): State = - processCommand(exec, state, () => Command.process(exec.commandLine, state)) - - private[sbt] def processCommand( - exec: Exec, - state: State, - runCommand: () => State - ): State = { + def processCommand(exec: Exec, state: State): State = { val channelName = exec.source map (_.channelName) StandardMain.exchange publishEventMessage ExecStatusEvent("Processing", channelName, exec.execId, Vector()) try { def process(): State = { - val newState = runCommand() + val newState = Command.process(exec.commandLine, state) val doneEvent = ExecStatusEvent( "Done", channelName, diff --git a/main/src/main/scala/sbt/internal/Continuous.scala b/main/src/main/scala/sbt/internal/Continuous.scala index d87b7d710..a87060d34 100644 --- a/main/src/main/scala/sbt/internal/Continuous.scala +++ b/main/src/main/scala/sbt/internal/Continuous.scala @@ -188,7 +188,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { inputs.foreach(i => repository.register(i.glob)) val watchSettings = new WatchSettings(scopedKey) new Config( - scopedKey, + scopedKey.show, () => dynamicInputs.toSeq.sorted, watchSettings ) @@ -212,7 +212,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { val onFail = Command.command(failureCommandName)(identity) // This adds the "SbtContinuousWatchOnFail" onFailure handler which allows us to determine // whether or not the last task successfully ran. It is used in the makeTask method below. - val s = (FailureWall :: state).copy( + val s = state.copy( onFailure = Some(Exec(failureCommandName, None)), definedCommands = state.definedCommands :+ onFail ) @@ -226,22 +226,29 @@ private[sbt] object Continuous extends DeprecatedContinuous { * if they are not visible in the input graph due to the use of Def.taskDyn. */ def makeTask(cmd: String): (String, State, () => Boolean) = { - val newState = s.put(DynamicInputs, mutable.Set.empty[DynamicInput]) - val task = Parser - .parse(cmd, Command.combine(newState.definedCommands)(newState)) - .getOrElse( - throw new IllegalStateException( - "No longer able to parse command after transforming state" - ) - ) + val newState = s + .put(DynamicInputs, mutable.Set.empty[DynamicInput]) + .copy(remainingCommands = Exec(cmd, None, None) :: Exec(FailureWall, None, None) :: Nil) ( cmd, newState, () => { - MainLoop - .processCommand(Exec(cmd, None), newState, task) - .remainingCommands - .forall(_.commandLine != failureCommandName) + @tailrec + def impl(s: State): Boolean = { + s.remainingCommands match { + case exec :: rest => + val updatedState = MainLoop.processCommand(exec, s.copy(remainingCommands = rest)) + val remaining = + updatedState.remainingCommands.takeWhile(_.commandLine != FailureWall) + remaining match { + case Nil => + updatedState.remainingCommands.forall(_.commandLine != failureCommandName) + case _ => impl(updatedState) + } + case Nil => true + } + } + impl(newState) } ) } @@ -256,7 +263,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { // Convert the command strings to runnable tasks, which are represented by // () => Try[Boolean]. - val taskParser = Command.combine(s.definedCommands)(s) + val taskParser = s.combinedParser // This specified either the task corresponding to a command or the command itself if the // the command cannot be converted to a task. val (invalid, valid) = @@ -378,6 +385,9 @@ private[sbt] object Continuous extends DeprecatedContinuous { } finally repo.close() } + // This is defined so we can assign a task key to a command to parse the WatchSettings. + private[this] val globalWatchSettingKey = + taskKey[Unit]("Internal task key. Not actually used.").withRank(KeyRanks.Invisible) private def parseCommand(command: String, state: State): Seq[ScopedKey[_]] = { // Collect all of the scoped keys that are used to delegate the multi commands. These are // necessary to extract all of the transitive globs that we need to monitor during watch. @@ -391,9 +401,13 @@ private[sbt] object Continuous extends DeprecatedContinuous { val aliases = BasicCommands.allAliases(state) aliases.collectFirst { case (`command`, aliased) => aliased } match { case Some(aliased) => impl(aliased) - case _ => - val msg = s"Error attempting to extract scope from $command: $e." - throw new IllegalStateException(msg) + case None => + Parser.parse(command, state.combinedParser) match { + case Right(_) => globalWatchSettingKey.scopedKey :: Nil + case _ => + val msg = s"Error attempting to extract scope from $command: $e." + throw new IllegalStateException(msg) + } } case _ => Nil: Seq[ScopedKey[_]] } @@ -619,7 +633,7 @@ private[sbt] object Continuous extends DeprecatedContinuous { configs.map { config => // Create a logger with a scoped key prefix so that we can tell from which task there // were inputs that matched the event path. - val configLogger = logger.withPrefix(config.key.show) + val configLogger = logger.withPrefix(config.command) observers.addObserver { e => if (config.inputs().exists(_.glob.matches(e.path))) { configLogger.debug(s"Accepted event for ${e.path}") @@ -910,12 +924,12 @@ private[sbt] object Continuous extends DeprecatedContinuous { * Container class for all of the components we need to setup a watch for a particular task or * input task. * - * @param key the [[ScopedKey]] instance for the task we will watch + * @param command the name of the command/task to run with each iteration * @param inputs the transitive task inputs (see [[SettingsGraph]]) * @param watchSettings the [[WatchSettings]] instance for the task */ private final class Config private[internal] ( - val key: ScopedKey[_], + val command: String, val inputs: () => Seq[DynamicInput], val watchSettings: WatchSettings ) { diff --git a/sbt/src/sbt-test/watch/commands/build.sbt b/sbt/src/sbt-test/watch/commands/build.sbt new file mode 100644 index 000000000..bf3d80a7c --- /dev/null +++ b/sbt/src/sbt-test/watch/commands/build.sbt @@ -0,0 +1,56 @@ +import java.nio.file.Files + +import scala.collection.JavaConverters._ + +val foo = taskKey[Unit]("foo") +foo := { + val fooTxt = baseDirectory.value / "foo.txt" + val _ = println(s"foo inputs: ${(foo / allInputFiles).value}") + IO.write(fooTxt, "foo") + println(s"foo wrote to $foo") +} +foo / fileInputs += baseDirectory.value.toGlob / "foo.txt" + +Global / watchTriggers += baseDirectory.value.toGlob / "bar.txt" + +commands ++= Seq( + Command.command("eval-foo") { s => + Project.extract(s).runTask(foo, s) + s + }, + Command.command("write-bar") { s => + val bar = Project.extract(s).get(baseDirectory) / "bar.txt" + IO.write(bar, "bar") + println(s"write-bar wrote to $bar") + s + } +) + +watchOnFileInputEvent := { (_, _) => sbt.nio.Watch.CancelWatch } + +/* + * This test ensures that when watching a cross build that commands are run for multiple scala + * versions. It also checks that the fileInputs are automatically detected during task evaluation + * even though we can't directly inspect the fileInputs of the cross ('+') command. + */ +val expectFailure = taskKey[Unit]("expect failure") +expectFailure := { + val main = baseDirectory.value.toPath / "src" / "main" + val crossDir = main / (if (scalaVersion.value.startsWith("2.11")) "scala-2.11" else "scala-2.12") + (Compile / compile).result.value.toEither match { + case Left(_) => + Files.write(crossDir / "Foo.scala", "class Foo".getBytes) + throw new IllegalStateException("Compilation failed.") + case Right(_) => + if (!Files.walk(main).iterator.asScala.exists(_.getFileName.toString == "first.scala")) + Files.write(crossDir / "first.scala", "class first".getBytes) + else Files.write(crossDir / "second.scala", "class second".getBytes) + } +} +expectFailure / watchOnFileInputEvent := { (_, e) => + if (e.path.getFileName.toString == "second.scala") sbt.nio.Watch.CancelWatch + else sbt.nio.Watch.Trigger +} + + +crossScalaVersions := Seq("2.11.12", "2.12.8") diff --git a/sbt/src/sbt-test/watch/commands/src/main/scala-2.11/Foo.scala b/sbt/src/sbt-test/watch/commands/src/main/scala-2.11/Foo.scala new file mode 100644 index 000000000..cc04bc468 --- /dev/null +++ b/sbt/src/sbt-test/watch/commands/src/main/scala-2.11/Foo.scala @@ -0,0 +1 @@ +class Foo { \ No newline at end of file diff --git a/sbt/src/sbt-test/watch/commands/src/main/scala-2.12/Foo.scala b/sbt/src/sbt-test/watch/commands/src/main/scala-2.12/Foo.scala new file mode 100644 index 000000000..cc04bc468 --- /dev/null +++ b/sbt/src/sbt-test/watch/commands/src/main/scala-2.12/Foo.scala @@ -0,0 +1 @@ +class Foo { \ No newline at end of file diff --git a/sbt/src/sbt-test/watch/commands/src/main/scala/Bar.scala b/sbt/src/sbt-test/watch/commands/src/main/scala/Bar.scala new file mode 100644 index 000000000..4a643c4dd --- /dev/null +++ b/sbt/src/sbt-test/watch/commands/src/main/scala/Bar.scala @@ -0,0 +1 @@ +class Bar \ No newline at end of file diff --git a/sbt/src/sbt-test/watch/commands/test b/sbt/src/sbt-test/watch/commands/test new file mode 100644 index 000000000..2eeb6d22c --- /dev/null +++ b/sbt/src/sbt-test/watch/commands/test @@ -0,0 +1,6 @@ +> ~ eval-foo + +> ~ write-bar + +> set watchOnFileInputEvent := (expectFailure / watchOnFileInputEvent).value +> ~ +expectFailure 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 5fef5fad7..03d871e56 100644 --- a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala +++ b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala @@ -232,7 +232,9 @@ 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 "watch/commands" => + LauncherBased // java.lang.ClassNotFoundException: javax.tools.DiagnosticListener when run with java 11 and an old sbt launcher + 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 From 87e6832f67564e7548294668e4ba3f5dd3904ec7 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Sat, 1 Jun 2019 20:16:58 -0700 Subject: [PATCH 3/3] Make scripted watch/on-changes more robust I noticed that sometimes this test hangs in ci. I think it may be because sometimes the event for A got handled while the cache entry for B.scala was invalidated. Because the test runs ~compile, the next run of compile would then reset the cache entry for B.scala. The next run of watchOnIteration invalidtes B.scala again, but this time to the same hash, so it doesn't actually trigger a file input event. By changing the content of B.scala every time, we ensure that it should still cause a file event. I haven't confirmed that this will fix the sporadic hangs because I can't reproduce on my machine, but this change won't hurt. --- sbt/src/sbt-test/watch/on-change/build.sbt | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/sbt/src/sbt-test/watch/on-change/build.sbt b/sbt/src/sbt-test/watch/on-change/build.sbt index b1ccae48d..048ca05aa 100644 --- a/sbt/src/sbt-test/watch/on-change/build.sbt +++ b/sbt/src/sbt-test/watch/on-change/build.sbt @@ -1,8 +1,10 @@ import java.nio.file._ +import java.nio.file.attribute.FileTime + import sbt.nio.Keys._ import sbt.nio._ + import scala.concurrent.duration._ -import StandardCopyOption.{ REPLACE_EXISTING => replace } watchTriggeredMessage := { (i, path: Path, c) => val prev = watchTriggeredMessage.value @@ -16,14 +18,15 @@ watchOnIteration := { i: Int => val src = base.resolve("src").resolve("main").resolve("scala").resolve("sbt").resolve("test") val changes = base.resolve("changes") - Files.copy(changes.resolve("C.scala"), src.resolve("C.scala"), replace) - if (i < 5) { + def copy(fileName: String): Unit = { val content = - new String(Files.readAllBytes(changes.resolve("A.scala"))) + "\n" + ("//" * i) - Files.write(src.resolve("A.scala"), content.getBytes) - } else { - Files.copy(changes.resolve("B.scala"), src.resolve("B.scala"), replace) + new String(Files.readAllBytes(changes.resolve(fileName))) + "\n" + ("//" * i) + Files.write(src.resolve(fileName), content.getBytes) } + val c = src.resolve("C.scala") + Files.setLastModifiedTime(c, FileTime.fromMillis(Files.getLastModifiedTime(c).toMillis + 1111)) + if (i < 5) copy("A.scala") + else copy("B.scala") println(s"Waiting for changes...") Watch.Ignore }