From 9cdeb7120ed7bdb9291e24060a16a268eeb9cfcf Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Sat, 12 Jan 2019 15:44:09 -0800 Subject: [PATCH] Add StateTransform class This commit cleans up the approach for transforming the sbt state upon completion of a task returning State. I add a new approach where a task can return an instance of StateTransform, which is just a wrapper around State. I then update EvaluateTask to apply this stateTransform rather than the (optional) state transformation that may be stored in the Task info parameter. By requiring that the user return StateTransform rather than State directly, we ensure that no existing tasks that depend on the state transformation function embedded in the Task info break. In sbt 2, I could see the possibility of making this automatic (and probably removing the state transformation function via attribute). The problem with using the transformState attribute key is that it is applied non-deterministically. This means that if you decorate a task returning State, then the state transformation may or may not be correctly applied. I tracked this non-determinism down to the stateTransform method in EvaluateTask. It iterates through the task result map and chains all of the defined transformState attribute values. Because the result is a map, this order is not specified. This chaining is arguably a bad design because State => State does not imply commutivity. Indeed, the problem here was that my state transformation functions were constant functions, which are obviously non-commutative. I believe that this logic likely written under the assumption that there would be no more than one of these tranformations in a given result map. --- main-settings/src/main/scala/sbt/Def.scala | 2 +- main/src/main/scala/sbt/EvaluateTask.scala | 10 +++++-- main/src/main/scala/sbt/Keys.scala | 2 +- main/src/main/scala/sbt/StateTransform.scala | 20 +++++++++++++ .../main/scala/sbt/internal/Continuous.scala | 16 ++++------ .../sbt-test/watch/on-termination/build.sbt | 3 ++ .../watch/on-termination/project/Build.scala | 29 +++++++++++++++++++ sbt/src/sbt-test/watch/on-termination/test | 7 +++++ 8 files changed, 75 insertions(+), 14 deletions(-) create mode 100644 main/src/main/scala/sbt/StateTransform.scala create mode 100644 sbt/src/sbt-test/watch/on-termination/build.sbt create mode 100644 sbt/src/sbt-test/watch/on-termination/project/Build.scala create mode 100644 sbt/src/sbt-test/watch/on-termination/test diff --git a/main-settings/src/main/scala/sbt/Def.scala b/main-settings/src/main/scala/sbt/Def.scala index 1451b5ffc..a42255f4e 100644 --- a/main-settings/src/main/scala/sbt/Def.scala +++ b/main-settings/src/main/scala/sbt/Def.scala @@ -14,7 +14,7 @@ import sbt.KeyRanks.{ DTask, Invisible } import sbt.Scope.{ GlobalScope, ThisScope } import sbt.internal.util.Types.const import sbt.internal.util.complete.Parser -import sbt.internal.util.{ AttributeKey, Attributed, ConsoleAppender, Init } +import sbt.internal.util._ import sbt.util.Show /** A concrete settings system that uses `sbt.Scope` for the scope type. */ diff --git a/main/src/main/scala/sbt/EvaluateTask.scala b/main/src/main/scala/sbt/EvaluateTask.scala index ada880ef8..ca08ee093 100644 --- a/main/src/main/scala/sbt/EvaluateTask.scala +++ b/main/src/main/scala/sbt/EvaluateTask.scala @@ -14,6 +14,7 @@ import sbt.Def.{ ScopedKey, Setting, dummyState } import sbt.Keys.{ TaskProgress => _, name => _, _ } import sbt.Project.richInitializeTask import sbt.Scope.Global +import sbt.internal.Aggregation.KeyValue import sbt.internal.TaskName._ import sbt.internal.TransitiveGlobs._ import sbt.internal.util._ @@ -479,8 +480,13 @@ object EvaluateTask { results: RMap[Task, Result], state: State, root: Task[T] - ): (State, Result[T]) = - (stateTransform(results)(state), results(root)) + ): (State, Result[T]) = { + val newState = results(root) match { + case Value(KeyValue(_, st: StateTransform) :: Nil) => st.state + case _ => stateTransform(results)(state) + } + (newState, results(root)) + } def stateTransform(results: RMap[Task, Result]): State => State = Function.chain( results.toTypedSeq flatMap { diff --git a/main/src/main/scala/sbt/Keys.scala b/main/src/main/scala/sbt/Keys.scala index e25f962cf..977cc928a 100644 --- a/main/src/main/scala/sbt/Keys.scala +++ b/main/src/main/scala/sbt/Keys.scala @@ -116,7 +116,7 @@ object Keys { val watchService = settingKey[() => WatchService]("Service to use to monitor file system changes.").withRank(BMinusSetting).withRank(DSetting) val watchStartMessage = settingKey[Int => Option[String]]("The message to show when triggered execution waits for sources to change. The parameter is the current watch iteration count.").withRank(DSetting) // The watchTasks key should really be named watch, but that is already taken by the deprecated watch key. I'd be surprised if there are any plugins that use it so I think we should consider breaking binary compatibility to rename this task. - val watchTasks = InputKey[State]("watch", "Watch a task (or multiple tasks) and rebuild when its file inputs change or user input is received. The semantics are more or less the same as the `~` command except that it cannot transform the state on exit. This means that it cannot be used to reload the build.").withRank(DSetting) + val watchTasks = InputKey[StateTransform]("watch", "Watch a task (or multiple tasks) and rebuild when its file inputs change or user input is received. The semantics are more or less the same as the `~` command except that it cannot transform the state on exit. This means that it cannot be used to reload the build.").withRank(DSetting) val watchTrackMetaBuild = settingKey[Boolean]("Toggles whether or not changing the build files (e.g. **/*.sbt, project/**/(*.scala | *.java)) should automatically trigger a project reload").withRank(DSetting) val watchTriggeredMessage = settingKey[(Int, Event[FileAttributes]) => Option[String]]("The message to show before triggered execution executes an action after sources change. The parameters are the path that triggered the build and the current watch iteration count.").withRank(DSetting) diff --git a/main/src/main/scala/sbt/StateTransform.scala b/main/src/main/scala/sbt/StateTransform.scala new file mode 100644 index 000000000..abea411d5 --- /dev/null +++ b/main/src/main/scala/sbt/StateTransform.scala @@ -0,0 +1,20 @@ +/* + * sbt + * Copyright 2011 - 2018, Lightbend, Inc. + * Copyright 2008 - 2010, Mark Harrah + * Licensed under Apache License 2.0 (see LICENSE) + */ + +package sbt + +final class StateTransform(val state: State) { + override def equals(o: Any): Boolean = o match { + case that: StateTransform => this.state == that.state + case _ => false + } + override def hashCode: Int = state.hashCode + override def toString: String = s"StateTransform($state)" +} +object StateTransform { + def apply(state: State): State = state +} diff --git a/main/src/main/scala/sbt/internal/Continuous.scala b/main/src/main/scala/sbt/internal/Continuous.scala index ce6eaf86b..613c3c3a7 100644 --- a/main/src/main/scala/sbt/internal/Continuous.scala +++ b/main/src/main/scala/sbt/internal/Continuous.scala @@ -23,10 +23,9 @@ import sbt.Scope.Global import sbt.internal.FileManagement.FileTreeRepositoryOps import sbt.internal.LabeledFunctions._ import sbt.internal.io.WatchState -import sbt.internal.util.Types.const import sbt.internal.util.complete.Parser._ import sbt.internal.util.complete.{ Parser, Parsers } -import sbt.internal.util.{ AttributeKey, AttributeMap, Util } +import sbt.internal.util.{ AttributeKey, Util } import sbt.io._ import sbt.util.{ Level, _ } @@ -122,16 +121,13 @@ object Continuous extends DeprecatedContinuous { * we have to modify the Task.info to apply the state transformation after the task completes. * @return the [[InputTask]] */ - private[sbt] def continuousTask: Def.Initialize[InputTask[State]] = + private[sbt] def continuousTask: Def.Initialize[InputTask[StateTransform]] = Def.inputTask { val (initialCount, command) = continuousParser.parsed - runToTermination(Keys.state.value, command, initialCount, isCommand = false) - }(_.mapTask { t => - val postTransform = t.info.postTransform { - case (state: State, am: AttributeMap) => am.put(Keys.transformState, const(state)) - } - Task(postTransform, t.work) - }) + new StateTransform( + runToTermination(Keys.state.value, command, initialCount, isCommand = false) + ) + } private[this] val DupedSystemIn = AttributeKey[DupedInputStream]( diff --git a/sbt/src/sbt-test/watch/on-termination/build.sbt b/sbt/src/sbt-test/watch/on-termination/build.sbt new file mode 100644 index 000000000..3e1169f6d --- /dev/null +++ b/sbt/src/sbt-test/watch/on-termination/build.sbt @@ -0,0 +1,3 @@ +import sbt.watch.task.Build + +val root = Build.root diff --git a/sbt/src/sbt-test/watch/on-termination/project/Build.scala b/sbt/src/sbt-test/watch/on-termination/project/Build.scala new file mode 100644 index 000000000..31751e291 --- /dev/null +++ b/sbt/src/sbt-test/watch/on-termination/project/Build.scala @@ -0,0 +1,29 @@ +package sbt.watch.task + +import sbt._ +import Keys._ + +object Build { + val reloadFile = settingKey[File]("file to toggle whether or not to reload") + val setStringValue = inputKey[Unit]("set a global string to a value") + val checkStringValue = inputKey[Unit]("check the value of a global") + def setStringValueImpl: Def.Initialize[InputTask[Unit]] = Def.inputTask { + val Seq(stringFile, string) = Def.spaceDelimited().parsed.map(_.trim) + IO.write(file(stringFile), string) + } + def checkStringValueImpl: Def.Initialize[InputTask[Unit]] = Def.inputTask { + val Seq(stringFile, string) = Def.spaceDelimited().parsed + assert(IO.read(file(stringFile)) == string) + } + lazy val root = (project in file(".")).settings( + reloadFile := baseDirectory.value / "reload", + setStringValue / watchTriggers += baseDirectory.value * "foo.txt", + setStringValue := setStringValueImpl.evaluated, + checkStringValue := checkStringValueImpl.evaluated, + watchOnTriggerEvent := { (_, _) => Watch.CancelWatch }, + watchTasks := Def.inputTask { + val prev = watchTasks.evaluated + new StateTransform(prev.state.fail) + }.evaluated + ) +} \ No newline at end of file diff --git a/sbt/src/sbt-test/watch/on-termination/test b/sbt/src/sbt-test/watch/on-termination/test new file mode 100644 index 000000000..6da243996 --- /dev/null +++ b/sbt/src/sbt-test/watch/on-termination/test @@ -0,0 +1,7 @@ +# This tests that we can override the state transformation in the watch task +# In the build, watchOnEvent should return CancelWatch which should be successful, but we +# override watchTasks to fail the state instead + +-> watch root / setStringValue foo.txt bar + +> checkStringValue foo.txt bar