show combined with --watch does not watch files if evaluation fails
#909
Milestone
Comments
|
My suspicion is, that this is true for all evaluator command. |
|
In evaluator commands, we pass in an evaluator, which can be used to run arbitrary targets. We can do this even many times before returning. Those evaluations are completely independent of the outer evaluation, which is used to run the evaluator command (in this case |
lihaoyi
added a commit
that referenced
this issue
May 4, 2023
Fixes #838 and also fixes #909 We add the ability to re-run `T.input{...}` blocks during the polling loop, just like we re-stat the `os.Paths` reported by `T.source` and `T.sources`. This involves some complexity, since `T.input{...}` blocks are not just a single lambda but instead a wrapper for a part of the `Task` graph that feeds into it. We capture the body of `evaluateGroup` in a lambda so we can call it later # Major Changes 1. Modify `Evaluator.Results#results` from a `Map[Task, Result]` to `Map[Task, TaskResult]`, where `TaskResult[T]` is a wrapper of `Result[T]` that also contains a `recalcOpt: Option[() => Result[T]]` callback 2. Wrap the bulk of `Evaluator#evaluateGroup` in a function, that we evaluate once to populate `TaskResult#result` and store in a callback to populate `TaskResult#recalcOpt` 3. Update `RunScript.evaluateNamed` to handle `InputImpl`s, in addition to the current handling of `SourceImpl`s and `SourcesImpl`s, and translate the `recalcOpt` callback into a `Watchable.Value` 4. The `Watchable.Value` is then propagated to `MillBuildBootstrap.evaluateWithWatches`, which ends up feeding it into `Watching.scala` where it is processed the same way as the `Watchable.Value`s we get from `interp.watchValue` 5. Overhauled the implementation of `show`; rather than returning a `Watched` wrapper, we now call back to `interp.evalWatch0` to register the things we need to watch. I also consolidated the common code in `show` and `showNamed` into one shared helper. 6. I wrapped most of the `Any`s or `_`s in `Evaluator` with a new `case class Val(value: Any)` wrapper. This should considerably increase type safety: e.g. you can no longer pass an `Option[Val]` where a `Val` was expected, unless you manually wrap it in `Val(...)` which is hard to do accidentally. This makes it much easier to ensure the various data structures inside `Evaluator` are passed around correctly and not mis-used # Minor Changes 1. Cleaned up `RunScripts` considerably, now there's only two methods left and no copy-pasty code 2. Removed some `T.input`s from `MillBuildRootModule`: `def unmanagedClasspath` can be a `T.sources`, and `def parseBuildFiles` is now a normal target that depends on `allBuildFiles` which is a `T.sources`. That should improve the `--watch` status message in the terminal and reduce the amount of work that needs to be done every 100ms while polling for changes # Testing Tested manually with the following `build.sc`; verified with `-w bar` that `bar` re-runs every 2s, and the script re-evaluates every 10s ```scala import mill._ interp.watchValue(System.currentTimeMillis() / 10000) println("Setting up build.sc") def foo = T.input{ System.currentTimeMillis() / 2000 } def bar = T{ foo() + " seconds since the epoch" } ``` Also added `integration/feature/watch-source-input/` suite that starts up a `--watch` in a background thread, makes changes to the files on disk, and ensures the `build.sc` and tasks within it re-evaluate as expected. # Notes 1. I'm not sure if the new way `show` registers watches is better than the old one. The old way with `Watched` can be made to work even in failure cases if we pass it as the optional second parameter to `Result.Failure` 2. The `Val` change is not strictly necessary, and is pretty invasive. But it was very hard to make the necessary changes to `Evaluator` without it due to bugs passing the wrong `Any`s around. So I think it's a reasonable time to introduce `Val` to support future work inside `Evaluator` with increased compiler support and type safety 3. The `recalc` logic would be a lot simpler if `input`s couldn't have upstream tasks, since we could just re-run the block rather than re-evaluating the task group. I had some thoughts about restricting `T.input`s to allow this, but couldn't convince myself that was the right thing to do, so left the `T.input` semantics in place and just made it work by capturing a callback that can evaluate the entire task group
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Seems like we're not properly propagating the watched files out of
def showin the case of failureThe text was updated successfully, but these errors were encountered: