This commit reworks Watched to be more testable and extensible. It also
adds some small features. The previous implementation presented a number
of challenges:
1) It relied on external side effects to terminate the watch, which was
difficult to test
2) It exposed irrelevant implementation details to the user in the
methods that exposed the WatchState as a parameter.
3) It spun up two worker threads. One was to monitor System.in for user
input. The other was to poll the watch service for events and write
them to a queue. The user input thread actually broke '~console'
because nearly every console session will hit the <enter> key, which
would eventually cause the watch to stop when the user exited the
console.
To address (1), I add the shouldTerminate method to WatchConfig. This
takes the current watch iteration is input and if the function returns
true, the watch will stop.
To address (2), I replace the triggeredMessage and watchingMessage keys
with watchTriggeredMessage and watchStartMessage. The latter two keys
are functions that do not take the WatchState as parameters. Both
functions take the current iteration count as a parameter and the
watchTriggeredMessage also has a parameter for the path that triggered
the build.
To address (3), I stop using the sbt.internal.io.EventMonitor and
instead use the sbt.io.FileEventMonitor. The latter class is similar to
the former except that it's polling method accepts a duration, which may
be finite or infinite) and returns all of the events that occurred since
it was last polled. By adding the ability to poll for a finite amount of
time, we can interleave polling for events with polling System.in for
user input, all on the main thread. This eliminates the two extraneous
threads and fixes the '~console' use case I described before.
I also let the user configure the function that reads from System.in via
the watchHandleInput method. In fact, this method need not read from
System.in at all since it's just () => Watched.Action. The reason that
it isn't () => Boolean is that I'd like to leave open the option for the
ability to trigger a build via user input, not just terminating the
watch. My initial idea was to add the ability to type 'r' to re-build in
addition to <enter> to exit. This doesn't work without integrating
jline though because the input is buffered. Regardless, for testing
purposes, it gives us the ability to add a timeout to the watch by
making handleInput return true when a deadline expires.
The tests are a bit wonky because I still need to rely on side effects
in the logging methods to orchestrate the sequence of file events that
I'd like to test. While I could move some of this logic into a
background thread, there still needs to be coordination between the
state of the watch and the background thread. I think it's easier to
reason about when all of the work occurs on the same thread, even if it
makes these user provided functions impure.
I deprecated all of the previous watch related keys that are no longer
used with the new infrastructure. To avoid breaking existing builds, I
make the watchConfig task use the deprecated logging methods if they are
defined in the user's builds, but sbt will not longer set the default
values. For the vast majority of users, it should be straightforward to
migrate their builds to use the new keys. My hunch is that the of the
deprecated keys, only triggeredMessage is widely used (in conjunction
with the clear screen method) and it is dead simple to replace it with
watchTriggeredMessage.
Note: The FileTreeViewConfig class is not really necessary for this commit.
It will become more important in a subsequent commit which introduces an
optional global file system cache.
This commit makes watch event logging work in the '~' command. The
previous design of the command made this difficult, so there is a
significant re-design of the implementation of '~'. I believe that this
redesign will allow the feature to be maintained and improved more
easily moving forward. With the redesign, it is now possible to test the
business logic of the watch command (and I add a rudimentary test that I
will build upon in subsequent commits).
A bonus of this redesign is that now if the user tries to watch an
invalid command, the watch will immediately terminate with an error
rather than get stuck waiting for events when the task can never
possibly succeed.
The previous implementation of the '~' command makes it difficult
to dynamically control the implementation arguments because it is
implemented in the command project which makes it unable to depend on
any task keys that are defined in the build. It works around this by
putting all of it's configuration in the Watched attribute which is
stored globally. This would not have been necessary if the function had
been defined in the main project where it could just extract the value
of the watched task rather than relying on the global attribute value.
Moreover, because it cannot depend on tasks, it makes it nigh impossible
to use the logging framework within the '~' command.
Another issue with the previous implementation is that it's somewhat
difficult to reason about. The executeContinuously has effectively two
entry points: one for the first time the command is run and one for each
subsequent invocation when a new build is triggered. The successive
invocations are triggered by prepending commands to run to the previous
state. This is made recursive by prepending the initial command (that
was prefixed with '~'. Which branch we're in is determined by checking
for the existence of a temporary attribute, that we must ensure that we
remove when the build is stopped. This makes a lot of behavior non-local and
difficult for an outsider who is less familiar with sbt to understand.
Broadly, this refactor does two things:
1) Move the definition of continuous from BasicCommands to BuiltInCommands
2) Re-work the implementation to be executed in code rather than using
the sbt dsl.
The first part is simple. We just add an implementation of continuous to
BuiltInCommands and remove it from the list of BasicCommands. We need to
leave in the legacy implementation for binary compatibility. I also
moved all of the actual implementation logic into Watched, which makes
maintenance easier since most of the logic is in one place.
The second part is more complicated. Rather than rely on the sbt dsl
(e.g. `(ClearOnFailure :: next :: FailureWall :: repeat :: s)`) to
parse and run the command. We manually parse the command and generate a
task of type `() => State`. We don't actually need to do anything with
the generated state because we're going to return the original state at
the end of the command no matter what. With this task, we can then
create a tail recursive function that repeatedly executes the task until
the watch is terminated.
The parsing is handled in the Watch.command method (which is where I
moved the refactored BasicCommands.continuous implementation). The
actual task running and monitoring is handled in Watched.watch. This
method has no reference to the sbt state, which makes it testable. It sets
up an event monitor and then delegates the recursive monitoring to a
small nested function, Watched.watch.impl. One nice thing about this
approach is that it is very easy to reason about the life cycle of the
EventMonitor. The recursive call is within a try { } finally { } where
the monitor and stdin are guaranteed to be cleared at the end.
Adding support for a custom (and default) watch logger is trivial with
the new infrastructure and is done via the watchLogger TaskKey.
There was a small reporting race condition that was introduced by the
change to (2). Because the new implementation is able to bypass command
parsing for triggered builds, the watch message would usually end up
being printed before the task outcome was fully logged. To work around
this, I made the watch and triggered messages be logged rather than
printed directly to stdout. As a result, the only user visible result of
this change should be that instead of seeing:
"1. Waiting for source changes in project foo... (press enter to interrupt)",
users will now see:
"[info] 1. Waiting for source changes in project foo... (press enter to interrupt)".
There have been reports that often a new build will be triggered
immediately after the previous build even when none of the files have
been modified since the start of the last build. This can happen when,
for example, a program implements save with a rename. When that occurs,
a deletion watch event may trigger the build but the corresponding
creation event may be detected outside of the current 40ms window. By
bumping this value to 500ms, we hopefully prevent the majority of these
false triggers. For unusual workflows in which this longer quarantine
period is an issue, the setting can be overridden.
This change makes the temporary shared library that is created by
the swoval file-tree-views library to be extracted into the sbt global
base directory rather than the temp file. This way if there is a leak of
shared libraries, they can easily be found in ~/.sbt rather than in,
say, /tmp (or the osx/windows equivalent location). The extracted shared
library objects will be in the path ~/.sbt/swoval-jni. There is a
shutdown hook that removes them as well as a garbage collection process
that runs in the background whenever the swoval library is loaded, so
these shouldn't leak uncontrollably.
I had to turn off -Xfatal-warnings in commandProj because after updating
io, commandProj depends on the deprecated EventMonitor class. In #4335,
I stop using EventMonitor, but deprecate the Watched class which is both
defined and used (as an unused attribute key) in commandProj. I think we
can probably get rid of Watched in 1.4.x and certainly in a hypothetical
2.x, so hopefully we can restore -Xfatal-warnings sooner than later.
I also had to replace uses of IO.classLocationFile with
IO.classLocationPath to avoid compilation failures due to
-Xfatal-warnings.
Fixes#4362
This implements an instance of ExecuteProgress that is enabled by default to report progress on the shell.
Combined with the scroll up logger, this takes over the bottom lines of the terminal screen and display a count up clock of the currently executing task.
This implements a new sbt.color flag that takes always/auto/never/true/false value as a replacement of current sbt.log.format=true/false flag.
When neither flags are set, the default behavior is to enable color when the terminal supports ANSI and it detects an stdout console (as opposed to redirects).
Fixes https://github.com/sbt/sbt/issues/4284
andrea
Ethan Atkins
Eugene Yokota
Ignasi Marimon-Clos