From 525bf8fa3db57a201d8ea9c26cc873773dfebb71 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 30 May 2019 14:43:13 -0700 Subject: [PATCH] Move meta build source check into Command.processCommand We want to check the build sources before any command runs, not just tasks. To achieve this, I moved the logic for checking for build source changes to CommandProcess.processCommand. Also, @smarter had noticed that if a user modified a build file and then ran reload, a warning would be displayed about changed build sources even though they had just ran reload. This was because running reload didn't update the previous cache for checkBuildSources / fileInputStamps. I fixed that bug by running 'checkBuildSources / changedInputFiles' instead of 'checkBuildSources' when the user runs reload. I verified that after this change: - If I changed a build file and ran 'show version' a warning was printed before it displayed the version. If I also set global / onChangedBuildSource := ReloadOnSourceChanges, it automatically reloaded before displaying the version. - If I changed a build source and ran 'reload', followed by 'show version', no warnings were ever displayed. As an implementation detail, I had to add the Aggregation.suppressShow attribute key. We set this key to true before checking the build sources. Without this, log.success is called whenever we check the build sources which is both confusing and noisy. --- main/src/main/scala/sbt/Defaults.scala | 1 + main/src/main/scala/sbt/EvaluateTask.scala | 35 +++--------- main/src/main/scala/sbt/MainLoop.scala | 55 ++++++++++++++----- .../main/scala/sbt/internal/Aggregation.scala | 23 +++++++- 4 files changed, 70 insertions(+), 44 deletions(-) diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 7975d71e1..4b6e11db9 100755 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -255,6 +255,7 @@ object Defaults extends BuildCommon { buildStructure := Project.structure(state.value), settingsData := buildStructure.value.data, aggregate in checkBuildSources :== false, + aggregate in checkBuildSources / changedInputFiles := false, checkBuildSources / Continuous.dynamicInputs := None, checkBuildSources / fileInputs := CheckBuildSources.buildSourceFileInputs.value, checkBuildSources := CheckBuildSources.needReloadImpl.value, diff --git a/main/src/main/scala/sbt/EvaluateTask.scala b/main/src/main/scala/sbt/EvaluateTask.scala index e08838bd5..d83ea65cd 100644 --- a/main/src/main/scala/sbt/EvaluateTask.scala +++ b/main/src/main/scala/sbt/EvaluateTask.scala @@ -19,7 +19,6 @@ import sbt.internal.TaskName._ import sbt.internal._ import sbt.internal.util._ import sbt.librarymanagement.{ Resolver, UpdateReport } -import sbt.nio.Keys.IgnoreSourceChanges import sbt.std.Transform.DummyTaskMap import sbt.util.{ Logger, Show } @@ -355,7 +354,7 @@ object EvaluateTask { val msgString = (msg.toList ++ ex.toList.map(ErrorHandling.reducedToString)).mkString("\n\t") val log = getStreams(key, streams).log val display = contextDisplay(state, ConsoleAppender.formatEnabledInEnv) - log.error("(" + display.show(key) + ") " + msgString) + if (!ex.contains(Reload)) log.error("(" + display.show(key) + ") " + msgString) } } @@ -440,7 +439,7 @@ object EvaluateTask { case Some(t: Task[_]) => transformNode(t).isEmpty case _ => true } - def run[R](s: State, toRun: Task[R], doShutdown: Boolean) = { + def run() = { val x = new Execute[Task]( Execute.config(config.checkCycles, overwriteNode), triggers, @@ -448,12 +447,12 @@ object EvaluateTask { )(taskToNode) val (newState, result) = try { - val results = x.runKeep(toRun)(service) - storeValuesForPrevious(results, s, streams) - applyResults(results, s, toRun) - } catch { case inc: Incomplete => (s, Inc(inc)) } finally if (doShutdown) shutdown() + val results = x.runKeep(root)(service) + storeValuesForPrevious(results, state, streams) + applyResults(results, state, root) + } catch { case inc: Incomplete => (state, Inc(inc)) } finally shutdown() val replaced = transformInc(result) - logIncResult(replaced, s, streams) + logIncResult(replaced, state, streams) (newState, replaced) } object runningEngine extends RunningTaskEngine { @@ -468,24 +467,8 @@ object EvaluateTask { val strat = config.cancelStrategy val cancelState = strat.onTaskEngineStart(runningEngine) config.progressReporter.initial() - try { - (state.get(stateBuildStructure), state.get(sessionSettings)) match { - case (Some(structure), Some(settings)) => - val extracted: Extracted = Project.extract(settings, structure) - if (extracted.get(sbt.nio.Keys.onChangedBuildSource) == IgnoreSourceChanges) { - run(state, root, doShutdown = true) - } else { - run(state, extracted.get(sbt.nio.Keys.checkBuildSources), doShutdown = false) match { - case (newState, r) => - r.toEither match { - case Left(i) => (newState, Result.fromEither(Left(i))) - case _ => run(newState, root, doShutdown = true) - } - } - } - case _ => run(state, root, doShutdown = true) - } - } finally { + try run() + finally { strat.onTaskEngineFinish(cancelState) currentlyRunningEngine.set(null) lastEvaluatedState.set(SafeState(state)) diff --git a/main/src/main/scala/sbt/MainLoop.scala b/main/src/main/scala/sbt/MainLoop.scala index 623782346..8b8c1763b 100644 --- a/main/src/main/scala/sbt/MainLoop.scala +++ b/main/src/main/scala/sbt/MainLoop.scala @@ -11,8 +11,9 @@ import java.io.PrintWriter import java.util.Properties import jline.TerminalFactory -import sbt.internal.ShutdownHooks +import sbt.internal.{ Aggregation, ShutdownHooks } import sbt.internal.langserver.ErrorCodes +import sbt.internal.util.complete.Parser import sbt.internal.util.{ ErrorHandling, GlobalLogBacking } import sbt.io.{ IO, Using } import sbt.protocol._ @@ -189,23 +190,47 @@ object MainLoop { ExecStatusEvent("Processing", channelName, exec.execId, Vector()) try { - val newState = runCommand() - val doneEvent = ExecStatusEvent( - "Done", - channelName, - exec.execId, - newState.remainingCommands.toVector map (_.commandLine), - exitCode(newState, state), - ) - if (doneEvent.execId.isDefined) { // send back a response or error - import sbt.protocol.codec.JsonProtocol._ - StandardMain.exchange publishEvent doneEvent - } else { // send back a notification - StandardMain.exchange publishEventMessage doneEvent + def process(): State = { + val newState = runCommand() + val doneEvent = ExecStatusEvent( + "Done", + channelName, + exec.execId, + newState.remainingCommands.toVector map (_.commandLine), + exitCode(newState, state), + ) + if (doneEvent.execId.isDefined) { // send back a response or error + import sbt.protocol.codec.JsonProtocol._ + StandardMain.exchange publishEvent doneEvent + } else { // send back a notification + StandardMain.exchange publishEventMessage doneEvent + } + newState + } + val checkCommand = state.currentCommand match { + // If the user runs reload directly, we want to be sure that we update the previous + // cache for checkBuildSources / changedInputFiles but we don't want to display any + // warnings. Without filling the previous cache, it's possible for the user to run + // reload and be prompted with a warning in spite of reload having just run and no build + // sources having changed. + case Some(exec) if exec.commandLine == "reload" => "checkBuildSources / changedInputFiles" + case _ => "checkBuildSources" + } + Parser.parse( + checkCommand, + state.put(Aggregation.suppressShow, true).combinedParser + ) match { + case Right(cmd) => + cmd() match { + case s if s.remainingCommands.headOption.map(_.commandLine).contains("reload") => + s.remove(Aggregation.suppressShow) + case _ => process() + } + case Left(_) => process() } - newState } catch { case err: Throwable => + err.printStackTrace() val errorEvent = ExecStatusEvent( "Error", channelName, diff --git a/main/src/main/scala/sbt/internal/Aggregation.scala b/main/src/main/scala/sbt/internal/Aggregation.scala index c723cc326..4b7d43473 100644 --- a/main/src/main/scala/sbt/internal/Aggregation.scala +++ b/main/src/main/scala/sbt/internal/Aggregation.scala @@ -9,14 +9,17 @@ package sbt package internal import java.text.DateFormat + import Def.ScopedKey import Keys.{ showSuccess, showTiming, timingFormat } import sbt.internal.util.complete.Parser -import sbt.internal.util.{ Dag, HList, Settings, Util } +import sbt.internal.util.{ AttributeKey, Dag, HList, Settings, Util } import sbt.util.{ Logger, Show } import Parser.{ failure, seq, success } import std.Transform.DummyTaskMap +import scala.annotation.tailrec + sealed trait Aggregation object Aggregation { final case class ShowConfig( @@ -76,7 +79,8 @@ object Aggregation { results.toEither.right.foreach { r => if (show.taskValues) printSettings(r, show.print) } - if (show.success) printSuccess(start, stop, extracted, success, log) + if (show.success && !state.get(suppressShow).getOrElse(false)) + printSuccess(start, stop, extracted, success, log) } def timedRun[T]( @@ -110,8 +114,19 @@ object Aggregation { )(implicit display: Show[ScopedKey[_]]): State = { val complete = timedRun[T](s, ts, extra) showRun(complete, show) + @tailrec def findReload( + incomplete: Incomplete, + remaining: List[Incomplete], + visited: Set[Incomplete] + ): Boolean = { + incomplete.directCause.contains(Reload) || ((remaining ::: incomplete.causes.toList) + .filterNot(visited) match { + case Nil => false + case h :: tail => findReload(h, tail.filterNot(visited), visited + incomplete) + }) + } complete.results match { - case Inc(i) if i.directCause.contains(Reload) => + case Inc(i) if findReload(i, i.causes.toList, Set.empty) => val remaining = s.currentCommand.toList ::: s.remainingCommands complete.state.copy(remainingCommands = Exec("reload", None, None) :: remaining) case Inc(i) => complete.state.handleError(i) @@ -289,4 +304,6 @@ object Aggregation { def aggregationEnabled(key: ScopedKey[_], data: Settings[Scope]): Boolean = Keys.aggregate in Scope.fillTaskAxis(key.scope, key.key) get data getOrElse true + private[sbt] val suppressShow = + AttributeKey[Boolean]("suppress-aggregation-show", Int.MaxValue) }