In the previous version of the NetworkClient, there was no feedback
while the client was starting up. It was also possible that if the
server had exited abruptly and there was a dead active.json portfile
left over, that the client wouldn't be able to start the server.
This commit reworks things so that we launch the server with a java
process and we print out the stdout, stderr streams from the process. We
also forward the client's stdin in case the server couldn't be started
and the user wants to retry or print the stacktrace.
This commit adds support for remote clients to connect to the sbt server
and attach themselves as a virtual terminal. In order to make this work,
each connection must send a json rpc request to attach to the server.
When this is received, the server will periodically query the remote
client to get the terminal properties and capabilities that allow the
remote client to act as a jline terminal proxy. There is also support
for json messages with ids sbt/systemIn and sbt/systemOut that allow io
to be relayed from the remote terminal to the sbt server and back.
Certain commands such as `exit` should be evaluated immediately. To make
this work, we add the concept of a MaintenanceTask. The CommandExchange
has a background thread that reads MaintenanceTasks and evaluates them
on demand. This allows maintenance tasks to be evaluated even when sbt
is evaluating an exec. If it weren't done this way, when the user typed
exit while a different remote connection was running a command, they
wouldn't be able to exit until the command completed.
The ServerIntents in ServerHandler did not handle
JsonRpcResponseMessage because prior to this commit, sbt clients were
primarily making requests to the server. But now the server sends
requests to the client for the terminal properties and terminal
capabilities so it was necessary to add an onResponse handler to
ServerIntent.
I had to move the network channel publishBytes method to run on a
background thread because there were scenarios in which the client
socket would get blocked because the server was trying to write on the
same thread that the read the bytes from the client.
To make the console command work, it is necessary to hijack the
classloader for JLine. In MetaBuildLoader, we put a custom forked JLine
that has a setter for the TerminalFactory singleton. This allows us to
change the terminal that is used by JLine in ConsoleReader. Without this
hack, the scala console would not work for remote clients.
Neither The ConsoleAppender class nor the jni based ClientSocket can be
used in a graalvm native image. This commit reworks the NetworkClient so
that we can avoid those limitations. It also adds some additional
command line argument parsing and changes the value of the run method to
return Int rather than Unit for exit code support.
In order to support a multi-client sbt server ux, we need to factor
`Terminal` out into a class instead of a singleton. Each terminal provides
and outputstream and inputstream. In all of the places where we were
previously relying on the `Terminal` singleton we need to update the
code to use `Terminal.get`, which will redirect io to the terminal whose
command is currently running.
This commit does not implement the server side ui for network clients.
It is just preparatory work for the multi-client ui.
The Terminal implementations have thread safe access to the output
stream. For this reason, I had to remove the sychronization on the
ConsoleOut lockObject. There were code paths that led to deadlock when
synchronizing on the lockObject.
We had similar code for reading json frames from an input stream in
NetworkChannel and ServerConnection. I reworked and consolidated this
logic into a shared method in ReadJsonFromInputStream.
This commit also removes the ObjectMessage reporting methods that
weren't doing anything.
The collectAnalysis task an be a bit slow and delays client connections
from running commands. This commit adds an option to skip the analysis
if it isn't needed. The default behavior is left as it was.
Initial draft for bsp support.
This shows two communication pattern around BSP.
First, if the request can be handled with the build knowledge is readily available in `NetworkChannel` we can reply immediately. `BuildServerImpl#onBspBuildTargets` is an example for that.
Second, if the request requires `State`, then we can forward the parameter into a custom command, and reply back from a command. `BuildServerProtocol.bspBuildTargetSources` is an example of that since it needs to invoke tasks to generate sources.
It is better that sbt not expose the implementation detail that
LineReader is implemented by JLine. Other terminal related apis should
be handled by sbt.internal.util.Terminal.
Presently if a server command comes in while in the shell, the client
output can appear on the same line as the command prompt and the command
prompt will not appear again until the user hits enter. This is a
confusing ux. For example, if I start an sbt server and type
the partial command "comp" and then start up a client and run the clean
command followed by a compile, the output looks like:
[info] sbt server started at local:///Users/ethanatkins/.sbt/1.0/server/51cfad3281b3a8a1820a/sock
sbt:scala-compile> comp[info] new client connected: network-1
[success] Total time: 0 s, completed Dec 12, 2019, 7:23:24 PM
[success] Total time: 0 s, completed Dec 12, 2019, 7:23:27 PM
[success] Total time: 2 s, completed Dec 12, 2019, 7:23:31 PM
Now, if I type "ile\n", I get:
[info] sbt server started at local:///Users/ethanatkins/.sbt/1.0/server/51cfad3281b3a8a1820a/sock
ile
[success] Total time: 0 s, completed Dec 12, 2019, 7:23:34 PM
sbt:scala-compile>
Following the same set of inputs after this change, I get:
[info] sbt server started at local:///Users/ethanatkins/.sbt/1.0/server/51cfad3281b3a8a1820a/sock
sbt:scala-compile> comp
[info] new client connected: network-1
[success] Total time: 0 s, completed Dec 12, 2019, 7:25:58 PM
sbt:scala-compile> comp
[success] Total time: 0 s, completed Dec 12, 2019, 7:26:14 PM
sbt:scala-compile> comp
[success] Total time: 1 s, completed Dec 12, 2019, 7:26:17 PM
sbt:scala-compile> compile
[success] Total time: 0 s, completed Dec 12, 2019, 7:26:19 PM
sbt:scala-compile>
To implement this change, I added the redraw() method to LineReader
which is a wrapper around ConsoleReader.drawLine; ConsoleReader.flush().
We invoke LineReader.redraw whenever the ConsoleChannel receives a
ConsolePromptEvent and there is a running thread.
To prevent log lines from being appended to the prompt line, in the
CommandExchange we print a newline character whenever a new command is
received from the network or a network client connects and we believe
that there is an active prompt.
The ask user thread is a background thread so it's fine for it to block
on System.in. By blocking rather than polling, the cpu utilization of
sbt drops to 0 on idle. We have to explicitly handle <ctrl+d> if we
block though because the JLine console reader will return null both if
the input stream returns -1
This commit aims to centralize all of the terminal interactions
throughout sbt. It also seeks to hide the jline implementation details
and only expose the apis that sbt needs for interacting with the
terminal.
In general, we should be able to assume that the terminal is in
canonical (line buffered) mode with echo enabled. To switch to raw mode
or to enable/disable echo, there are apis: Terminal.withRawSystemIn and
Terminal.withEcho that take a thunk as parameter to ensure that the
terminal is reset back to the canonical mode afterwards.
To demonstrate [-Yno-lub](http://eed3si9n.com/stricter-scala-with-ynolub), this shows the code changes that removes lubing (Not all subprojects are done).
After I made the changes, I switched the Scala back to normal 2.12.10.
During refactoring, these warnings got out of date. I also added
scaladoc to the watchTriggeredMessage key.
Ref: https://github.com/sbt/sbt/issues/5051.
Sometimes turbo mode didn't work correctly for projects where resources
were modified. This was because it was possible for the resource
classloader to inadvertently evict the dependency classloader from the
classloader cache because they had the same file stamps. There were two
fixes:
1) remove expired entries from the cache based on the
(Parent, Classpath) pair rather than just classpath
2) do not close the classloaders during cache eviction. They may still
be in use when we evict them so we need to wait until they are
explicitly closed elsewhere or until the go out of scope and are
collected by the CleanupThread
I tested this change with a spark project in which I kept modifying the
resources. Prior to this change, I could get into a state where if I
modified the resources, the dependency layer would get evicted every
time so the benefits of turbo mode were not realized.
There have been numerous issues with the multi parser incorrectly
splitting commands like `alias foo = ; bar` into
`"alias foo =" :: "bar" :: Nil`. To fix this, I update the multi parser
implementation to accept a list of commands that cannot be part of a
multi command. For now, the only excluded command is "alias", but if
other issues come up, we can add more. I also thought about adding a
system property for excluding more commands but it didn't seem worth the
maintenance cost at this point.
In addition to adding a filter for the excluded commands, I also
reworked the multi parser so that I think its more clear (and should
hopefully have more predictable performance). I changed the cmdPart
parser to accept empty strings. Prior to this, the parser explicitly
handled the non-leading semicolon and leading semicolon cases
separately. With the relaxed cmdPart, we can handle both cases with a
single parser. We just have to strip any empty commands at the beginning
or end of the command list.
It was reported in https://github.com/sbt/sbt/issues/4890 that cosmetic
white space could cause problems for the paser. I tracked this down to
primarily being because of the
`val semi = token(OptSpace ~> ';' ~> OptSpace)` line. This would cause
excessive backtracking. I added a test for a multi line command with a
lot of cosmetic whitespace that was adapted from #4890 except that I
made it even more taxing by running adding 100 commands instead of the
roughly 10 in the report. Before the parser changes, the test would
more or less block indefinitely. I never saw it successfully complete.
After these changes, it completes in 30-50ms (which drops to about 2-3
ms if the number of commands is dropped from 100 to 3).
I verified manually in a different project that a number of different
multi command completions still worked. In particular, I tested that
`~foo/test; foo/tes` would expand to `~foo/test; foo/test` which is one
of the hardest cases to get right.
I also added a few extra test cases for the parser since I wasn't sure
what the impact of removing the OptSpace ~> from the semi parser would
be.
We tried to prevent users from doing something like running a multi
command "foo; bar" where foo is valid but bar is invalid so that we
wouldn't run foo only to discover bar was an invalid key. It isn't
possible to know in general if any command other than the first command
in a multi command is valid because it might update the state and add
the initially invalid command.
The validation caused the intellij plugin to not work with 1.3.0-RC3.
The recent changes to make the multi parser strict broke any multi
command, or alias, where the multi command contained a command or task
that was not yet defined, but was possibly added by reload. This was
reported as #4869. I had had to work around this issue in ScriptedTests
by running `reload` and `setUpScripted` separately instead of as a multi
command. This workaround doesn't work for aliasing boot, which has been
a recommended approach by Mark Harrah since 2011.
To fix this, I relax the strict parser. We don't require that the parser
be valid to create a multi command string. In the multiApplied state
transformation, however, we validate all of the commands up to 'reload'.
Since there is no way to validate any commands to the right of 'reload,
we optimistically allow those commands to run.
So long as there is no 'reload' in the multi commands, all of the
commands will be validated.
The multi parser had very poor performance if there were many commands.
Evaluating the expansion of something like "compile;" * 30 could cause
sbt to hang indefinitely. I believe this was due to excessive
backtracking due to the optional `(parser <~ semi.?).?` part of the
parser in the non-leading semicolon case.
I also reworked the implementation so that the multi command now has a
name. This allows us to partition the commands into multi and non-multi
commands more easily in State while still having multi in the command
list. With this change, builds and plugins can exclude the multi parser
if they wish.
Using the partitioned parsers, I removed the high/priority low priority
distinction. Instead, I made it so that the multi command will actually
check if the first command is a named command, like '~'. If it is, it
will pass the raw command argument with the named command stripped out
into the parser for the named command. If that is parseable, then we
directly apply the effect. Otherwise we prefix each multi command to the
state.
We run into issues if we naively split the command input on ';' and
treat each part as a separate command unless the ';' is inside of a
string because it is also valid to have ';'s inside of braced
expressions, e.g. `set foo := { val x = 1; x + 1 }`. There was no parser
for expressions enclosed in braces. I add one that should parse any
expression wrapped in braces so long as each opening brace is matched by a
closing brace. The parser returns the original expression. This allows
the multi parser to ignore ';' inside of '{...}'.
I had to rework the scripted tests to individually run 'reload' and
'setUpScripted' because the new parser rejects setUpScripted because it
isn't a valid command until reload has run.
It was reported in https://github.com/sbt/sbt/issues/4808 that compared
to 1.2.8, sbt 1.3.0-RC2 will truncate the command args of an input task
that contains semicolons. This is actually intentional, but not
completely robust. For sbt >= 1.3.0, we are making ';' syntactically
meaningful. This means that it always represents a command separator
_unless_ it is inside of a quoted string. To enforce this, the multi parser
will effectively split the input on ';', it will then validate that each
command that it extracted is valid. If not, it throws an exception. If
the input is not a multi command, then parsing fails with a normal
failure.
I removed the multi command from the state's defined commands and reworked
State.combinedParser to explicitly first try multi parsing and fall back
to the regular combined parser if it is a regular command. If the multi
parser throws an uncaught exception, parsing fails even if the regular
parser could have successfully parsed the command. The reason is so that
we do not ever allow the user to evaluate, say 'run a;b'. Otherwise the
behavior would be inconsitent when the user runs 'compile; run a;b'
There was an incomplete pattern match that assumed that the jars in the
scala provider included one with the name "scala-library.jar". In
practice, I think this is always true, but it's safer to have a fallback
case and it also removes the compiler warning.
At some point I noticed that projects with no scala sources in the build
loaded significantly faster than projects that had even a single scala
file -- no matter how simple that file was. This didn't really make
sense to me because *.sbt files _do_ have to be compiled. I finally
realized that classloading was a likely bottle neck because *.sbt
files are compiled on the sbt classpath while *.scala files are compiled
with a different classloader generated by the classloader cache. It then
occurred to me that we could pre-fill the classloader cache with the
scala layer of the sbt metabuild classloader.
I found that compared to 1.3.0-M5, a project with a simple scala file in
the project directory loaded about 2 seconds faster after this change.
Even if there are no scala sources in the build.sbt, there is a similar
performance improvement for running "sbt compile", which I found exited
2-3 seconds faster after this change.
Ethan Atkins
adpi2
Eugene Yokota
xuwei-k
Nafer Sanabria
Ethan Atkins