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.
This commit is contained in:
Ethan Atkins 2019-05-30 14:43:13 -07:00
parent 63956827e0
commit 525bf8fa3d
4 changed files with 70 additions and 44 deletions

View File

@ -255,6 +255,7 @@ object Defaults extends BuildCommon {
buildStructure := Project.structure(state.value), buildStructure := Project.structure(state.value),
settingsData := buildStructure.value.data, settingsData := buildStructure.value.data,
aggregate in checkBuildSources :== false, aggregate in checkBuildSources :== false,
aggregate in checkBuildSources / changedInputFiles := false,
checkBuildSources / Continuous.dynamicInputs := None, checkBuildSources / Continuous.dynamicInputs := None,
checkBuildSources / fileInputs := CheckBuildSources.buildSourceFileInputs.value, checkBuildSources / fileInputs := CheckBuildSources.buildSourceFileInputs.value,
checkBuildSources := CheckBuildSources.needReloadImpl.value, checkBuildSources := CheckBuildSources.needReloadImpl.value,

View File

@ -19,7 +19,6 @@ import sbt.internal.TaskName._
import sbt.internal._ import sbt.internal._
import sbt.internal.util._ import sbt.internal.util._
import sbt.librarymanagement.{ Resolver, UpdateReport } import sbt.librarymanagement.{ Resolver, UpdateReport }
import sbt.nio.Keys.IgnoreSourceChanges
import sbt.std.Transform.DummyTaskMap import sbt.std.Transform.DummyTaskMap
import sbt.util.{ Logger, Show } import sbt.util.{ Logger, Show }
@ -355,7 +354,7 @@ object EvaluateTask {
val msgString = (msg.toList ++ ex.toList.map(ErrorHandling.reducedToString)).mkString("\n\t") val msgString = (msg.toList ++ ex.toList.map(ErrorHandling.reducedToString)).mkString("\n\t")
val log = getStreams(key, streams).log val log = getStreams(key, streams).log
val display = contextDisplay(state, ConsoleAppender.formatEnabledInEnv) 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 Some(t: Task[_]) => transformNode(t).isEmpty
case _ => true case _ => true
} }
def run[R](s: State, toRun: Task[R], doShutdown: Boolean) = { def run() = {
val x = new Execute[Task]( val x = new Execute[Task](
Execute.config(config.checkCycles, overwriteNode), Execute.config(config.checkCycles, overwriteNode),
triggers, triggers,
@ -448,12 +447,12 @@ object EvaluateTask {
)(taskToNode) )(taskToNode)
val (newState, result) = val (newState, result) =
try { try {
val results = x.runKeep(toRun)(service) val results = x.runKeep(root)(service)
storeValuesForPrevious(results, s, streams) storeValuesForPrevious(results, state, streams)
applyResults(results, s, toRun) applyResults(results, state, root)
} catch { case inc: Incomplete => (s, Inc(inc)) } finally if (doShutdown) shutdown() } catch { case inc: Incomplete => (state, Inc(inc)) } finally shutdown()
val replaced = transformInc(result) val replaced = transformInc(result)
logIncResult(replaced, s, streams) logIncResult(replaced, state, streams)
(newState, replaced) (newState, replaced)
} }
object runningEngine extends RunningTaskEngine { object runningEngine extends RunningTaskEngine {
@ -468,24 +467,8 @@ object EvaluateTask {
val strat = config.cancelStrategy val strat = config.cancelStrategy
val cancelState = strat.onTaskEngineStart(runningEngine) val cancelState = strat.onTaskEngineStart(runningEngine)
config.progressReporter.initial() config.progressReporter.initial()
try { try run()
(state.get(stateBuildStructure), state.get(sessionSettings)) match { finally {
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 {
strat.onTaskEngineFinish(cancelState) strat.onTaskEngineFinish(cancelState)
currentlyRunningEngine.set(null) currentlyRunningEngine.set(null)
lastEvaluatedState.set(SafeState(state)) lastEvaluatedState.set(SafeState(state))

View File

@ -11,8 +11,9 @@ import java.io.PrintWriter
import java.util.Properties import java.util.Properties
import jline.TerminalFactory import jline.TerminalFactory
import sbt.internal.ShutdownHooks import sbt.internal.{ Aggregation, ShutdownHooks }
import sbt.internal.langserver.ErrorCodes import sbt.internal.langserver.ErrorCodes
import sbt.internal.util.complete.Parser
import sbt.internal.util.{ ErrorHandling, GlobalLogBacking } import sbt.internal.util.{ ErrorHandling, GlobalLogBacking }
import sbt.io.{ IO, Using } import sbt.io.{ IO, Using }
import sbt.protocol._ import sbt.protocol._
@ -189,23 +190,47 @@ object MainLoop {
ExecStatusEvent("Processing", channelName, exec.execId, Vector()) ExecStatusEvent("Processing", channelName, exec.execId, Vector())
try { try {
val newState = runCommand() def process(): State = {
val doneEvent = ExecStatusEvent( val newState = runCommand()
"Done", val doneEvent = ExecStatusEvent(
channelName, "Done",
exec.execId, channelName,
newState.remainingCommands.toVector map (_.commandLine), exec.execId,
exitCode(newState, state), newState.remainingCommands.toVector map (_.commandLine),
) exitCode(newState, state),
if (doneEvent.execId.isDefined) { // send back a response or error )
import sbt.protocol.codec.JsonProtocol._ if (doneEvent.execId.isDefined) { // send back a response or error
StandardMain.exchange publishEvent doneEvent import sbt.protocol.codec.JsonProtocol._
} else { // send back a notification StandardMain.exchange publishEvent doneEvent
StandardMain.exchange publishEventMessage 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 { } catch {
case err: Throwable => case err: Throwable =>
err.printStackTrace()
val errorEvent = ExecStatusEvent( val errorEvent = ExecStatusEvent(
"Error", "Error",
channelName, channelName,

View File

@ -9,14 +9,17 @@ package sbt
package internal package internal
import java.text.DateFormat import java.text.DateFormat
import Def.ScopedKey import Def.ScopedKey
import Keys.{ showSuccess, showTiming, timingFormat } import Keys.{ showSuccess, showTiming, timingFormat }
import sbt.internal.util.complete.Parser 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 sbt.util.{ Logger, Show }
import Parser.{ failure, seq, success } import Parser.{ failure, seq, success }
import std.Transform.DummyTaskMap import std.Transform.DummyTaskMap
import scala.annotation.tailrec
sealed trait Aggregation sealed trait Aggregation
object Aggregation { object Aggregation {
final case class ShowConfig( final case class ShowConfig(
@ -76,7 +79,8 @@ object Aggregation {
results.toEither.right.foreach { r => results.toEither.right.foreach { r =>
if (show.taskValues) printSettings(r, show.print) 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]( def timedRun[T](
@ -110,8 +114,19 @@ object Aggregation {
)(implicit display: Show[ScopedKey[_]]): State = { )(implicit display: Show[ScopedKey[_]]): State = {
val complete = timedRun[T](s, ts, extra) val complete = timedRun[T](s, ts, extra)
showRun(complete, show) 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 { 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 val remaining = s.currentCommand.toList ::: s.remainingCommands
complete.state.copy(remainingCommands = Exec("reload", None, None) :: remaining) complete.state.copy(remainingCommands = Exec("reload", None, None) :: remaining)
case Inc(i) => complete.state.handleError(i) case Inc(i) => complete.state.handleError(i)
@ -289,4 +304,6 @@ object Aggregation {
def aggregationEnabled(key: ScopedKey[_], data: Settings[Scope]): Boolean = def aggregationEnabled(key: ScopedKey[_], data: Settings[Scope]): Boolean =
Keys.aggregate in Scope.fillTaskAxis(key.scope, key.key) get data getOrElse true Keys.aggregate in Scope.fillTaskAxis(key.scope, key.key) get data getOrElse true
private[sbt] val suppressShow =
AttributeKey[Boolean]("suppress-aggregation-show", Int.MaxValue)
} }