In eb688c9ecd, we started buffering output
to the remote client to reduce flickering. This was causing problems
with the output for the thin client in batch mode. With the delay, it
was possible for the client to exit before all of its output had been
displayed.
Bonus: only display aggregation error message if terminal has success
enabled (the thin client displays its own timing message so the message
in aggregation ended up being a duplicate).
The more we call flush, the more likely progress status is to blink. To
reduce the amount of calls to flush, we can instead batch all of the
bytes that are going to be written and wriite them all at once. This
change made progress noticeably less blinky in the zinc project running
the latest sbt snapshot (which frankly was almost siezure inducing when
running publishLocal)
Printing a new line was not great ux. You might see something like:
[info] set current project to project (in build file:project)
sbt:project>
[info] new client connected: network-1
sbt:project>
instead of initially
[info] set current project to project (in build file:project)
sbt:project>
and then after the client connects:
[info] set current project to project (in build file:project)
[info] new client connected: network-1
sbt:project>
This reverts commit b1dcf031a5.
I found that b1dcf031a5 had some
unintended consequences that seemed to mess up the prompt state. The
real problem that it was trying to address was that the prompt was being
interleaved with log messages in some scenarios. There was a different
way to fix that in ProgressState that was both simpler and more
reliable.
I noticed that when reloading the build, that certain errors are logged
by sbt to System.err. These were not shown to a thin client because we
weren't forwarding System.err. This change remedies that.
System.err is handled more simply than System.out. We do not put
System.err through the progress state because generally System.err is
tends to be unbuffered. I had hesitated to add System.err to the
Terminal interface at all to give users an escape hatch but I couldn't
get project loading to work well with the thin client without it.
It is useful to store a buffer of the lines written to each terminal. We
can use those lines to replay the terminal log lines to a different
client. This is particularly nice when a remote client connects to sbt
while it's booting. We can show the remote client all the lines
displayed by the console prior to the client connecting.
This commit upgrades sbt to using jline3. The advantage to jline3 is
that it has a significantly better tab completion engine that is more
similar to what you get from zsh or fish.
The diff is bigger than I'd hoped because there are a number of
behaviors that are different in jline3 vs jline2 in how the library
consumes input streams and implements various features. I also was
unable to remove jline2 because we need it for older versions of the
scala console to work correctly with the thin client. As a result, the
changes are largely additive.
A good amount of this commit was in adding more protocol so that the
remote client can forward its jline3 terminal information to the server.
There were a number of minor changes that I made that either fixed
outstanding ui bugs from #5620 or regressions due to differences between
jline3 and jline2.
The number one thing that caused problems is that the jline3 LineReader
insists on using a NonBlockingInputStream. The implementation ofo
NonBlockingInputStream seems buggy. Moreover, sbt internally uses a
non blocking input stream for system in so jline is adding non blocking
to an already non blocking stream, which is frustrating.
A long term solution might be to consider insourcing LineReader.java
from jline3 and just adapting it to use an sbt terminal rather than
fighting with the jline3 api. This would also have the advantage of not
conflicting with other versions of jline3. Even if we don't, we may want to
shade jline3 if that is possible.
If there is no system console available, then there is no point in
making an ask user thread. An ask user thread can only be created when
the terminal prompt is in the Prompt.Running or Prompt.Loading state.
The console channel will now set itself to be in the Prompt.NoPrompt
state if it detects that there is no System.console available.
The motivation for this change is that jline was printing a lot of extra
text during scripted and server tests. Whenever a jline3 linereader is
closed, it prints a newline so the logs were filled with unnecessary
newlines.
In the situation where sbt was started in server mode and a client is
running a `~` command and a project reload is triggered by a change to
a build source, the console terminal looks like
sbt:foo>
[info] received remote command: ~compile
sbt:foo>
[info] welcome to sbt 1.4.0-SNAPSHOT (Azul Systems, Inc. Java 1.8.0_252)
sbt:foo>
[info] loading global plugins from ~/.sbt/1.0/plugins
sbt:foo>
[info] loading settings for project foo-build from metals.sbt ...
sbt:foo>
[info] loading project definition from
~/foo/project
sbt:foo>
[info] loading settings for project root from build.sbt ...
sbt:foo>
[info] loading settings for project macros from build.sbt ...
sbt:foo>
[info] loading settings for project main from build.sbt ...
sbt:foo>
[info] set current project to foo (in build file:~/foo)
sbt:foo>
This change fixes that by unprompting all channels during project
loading and reprompting them when it completes.
In a few places, I used this pattern in an attempt to debug some ui
issues. It is incorrect because it doesn't use System.lineSeparator
and is also pointless.
When running reboot at the console, the first character that the user
enters after the reboot has completed is lost. This is because it isn't
possible to interrupt System.in and we have a thread that is blocking on
reads to System.in in WriteableInputStream. That thread cannot be
shutdown during normal sbt shutdown while it is reading. When sbt next
starts up (in the same jvm), the previous thread gets the byte but has
nowhere to write it so the byte is lost. This commit fixes that behavior
by ensuring that we only poll from System.in when there is actually a
downstream consumer.
The behavior of reboot is still a little wonky if the user issues a
reboot from a network client and then tries to input commands at the
console. In that case, sbt will have been polling System.in in the ask
user thread prior to the reboot and the ask user thread will be
uninterruptible for the reason described above so the first byte will
again by swallowed by the previous sbt instance. This use case is
sufficiently pathological that it doesn't feel worth the effort to fix.
As annoying as it is, it doesn't break the sbt session. The user will
either submit an invalid command with the missing leading character or
notice the character is missing, possibly think they missed the key,
type backspace a few times and re-type the command.
On windows, it is sometimes possible to leak an sbt process if two
processes are started simultaneously by a remote client at the same
time. When this happens, the second process is unable to create a
server because of the first process and it also has no io streams
because the the client detaches its streams. We can detect this
in the shell command and prevent the process from persisting as a
zombie.
One issue with the remote client approach is that it is possible for
multiple clients to start multiple servers concurrently. I encountered
this in testing where in one tmux pane I'd start an sbt server and in
another I might run sbtc before the server had finished loading. This
can actually cause java processes to leak because the second process is
unable to start a server but it doesn't necessarily die after the client
that spawned it exits. This commit prevents this scenario by creating a
server socket before it loads the build and closes once the build is
complete. The client can then receive output bytes and forward input to
the booting server.
The socket that is created during boot is always a local socket, either
a UnixDomainServerSocket or a Win32NamedPipeServerSocket. At the moment,
I don't see any reason to support TCP. This socket also has no impact at
all on the normal sbt server that is started up after the project has
loaded.
The socket is hardcoded to be located at the relative path
project/target/$SOCK_NAME or the named pipe $SOCK_NAME where SOCK_NAME
is a farm hash of the absolute path of the project base directory. There
is no portfile json since there is no need since we don't support TCP.
After the socket is created it listens for clients to whom it relays
input to the console's input stream and relays the process output back
to the client. See the javadoc in BootServerSocket.java for further
details.
The process for forking the server is also a bit more complicated after
this change because the client will read the process output and error
streams until the socket is created and thereafter will only read output
from the socket, not the process.
Supershell does not work correctly when the sbt server is started by the
remote client on windows because it incorrectly calculates the terminal
dimensions. To work around this, we can pass in the dimensions from the
remote client as an environment variable. I tried to do this as a system
property but had all kinds of problems with windows stripping delimeters
from the command. It was much easier to get working with an environment
variable and should really only be set by the sbtc client anyway.
It seemed like a good idea to avoid printing the progress lines if they
were the same as last time. Unfortunately there is a scenario where the
user has multiple clients open and while one of the clients is running a
command, the user presses enter in the inactive client. When this
happens the message that warns that the server is running a different
command gets overwritten. Always printing keeps the message visible.
JLine will spam the console with error messages if certain commands get
interrupted. This is very noticeable and annoying in tab completions.
We can stop doing work in background as well since we've silenced the
jline logging.
This commit makes it possible for a remote client to cancel a running
task initiated by a different client by typing `cancel` into the shell.
It can be useful if the remote client has run something blocking like
console.
The console task can't safely be interrupted, so instead we write some
newlines filed by ctrl+d to exit the console.
This commit makes it possible for the sbt server to render the same ui
to multiple clients. The network client ui should look nearly identical
to the console ui except for the log messages about the experimental
client.
The way that it works is that it associates a ui thread with each
terminal. Whenever a command starts or completes, callbacks are invoked
on the various channels to update their ui state. For example, if there
are two clients and one of them runs compile, then the prompt is changed
from AskUser to Running for the terminal that initiated the command
while the other client remains in the AskUser state. Whenever the client
changes uses ui states, the existing thread is terminated if it is
running and a new thread is begun.
The UITask formalizes this process. It is based on the AskUser class
from older versions of sbt. In fact, there is an AskUserTask which is
very similar. It uses jline to read input from the terminal (which could
be a network terminal). When it gets a line, it submits it to the
CommandExchange and exits. Once the next command is run (which may or
may not be the command it submitted), the ui state will be reset.
The debug, info, warn and error commands should work with the multi
client ui. When run, they set the log level globally, not just for the
client that set the level.
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.
Rather than going through the console appender logging to make
TaskProgress work, we can instead use the CommandExchange. This will be
useful in future commits where there are multiple terminals that all
need to receive progress. By organizing the TaskProgress this way, we
can store a separate progress state for each terminal and update the
progress for all of the active terminals. We also can set the current
running command in command exchange which will be useful in future
commits to show what command is currently running.
This commit also reworks TaskProgress to always kill its thread when
there are no active tasks. It will start a new thread as soon as there
is another active task.
This commit adds a number of functions for stripping ansi escape
characters and/or finding the position of the cursor in a line that may
contain colors and moves. The motivation for EscHelpers.cursorPosition
is that when printing progress lines, we need to know the visual
dimensions of the last line printed to the prompt. The
EscHelpers.stripColorsAndMoves can be used to remove all ansi escape
sequences. Finally EscHelpers.stripMoves leaves colors but strips out
all other escape sequences. This is so we can reprint the terminal
prompt during supershell. If we didn't strip out the escape sequences,
we could inadvertently blow away everything below the cursor in cases
where we actually want the lines below the cursor to persist.
This file somehow got stuck in the repo although it wasn't actually
used. In fact, it fails to compile at all because
sbt.internal.util.TaskProgress is defined in main, not util-logging. I
noticed this because metals wasn't working well because it was failing
to compile util-logging because of this file. I think the file was
checked in by accident in e28e052b5b.
In order to make supershell work with println, this commit introduces a
virtual System.out to sbt. While sbt is running, we override the default
java.lang.System.out, java.lang.System.in, scala.Console.out and
scala.Console.in (unless the property `sbt.io.virtual` is set to
something other than true). When using virtual io, we buffer all of the
bytes that are written to System.out and Console.out until flush is
called. When flushing the output, we check if there are any progress
lines. If so, we interleave them with the new lines to print.
The flushing happens on a background thread so it should hopefully not
impede task progress.
This commit also adds logic for handling progress when the cursor is not
all the way to the left. We now track all of the bytes that have been
written since the last new line. Supershell will then calculate the
cursor position from those bytes* and move the cursor back to the
correct position. The motivation for this was to make the run command
work with supershell even when multiple main classes were specified.
* This might not be completely reliable if the string contains ansi
cursor movement characters.
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.
Fixes https://github.com/sbt/sbt/issues/5063
This fixes "sbt new" on Ubuntu by restoring the terminal state after supershell querying for the terminal width.
A StringBuilder is a mutable data structure to create a String.
When the String is created, the new String does not share any
storage with the StringBuilder. Thus, we can keep a same
StringBuilder, and reuse its internal storage between different
iterations.
I incorrectly included the DeleteLine in the progress line length and
this could cause certain progress lines to be incorrectly reported as
multi line when they actually fit on a single terminal line.
On the off chance that in some configurations the terminal width is set
to zero, avoid an exception by returning 0 for terminal lines. It is
likely that supershell will not work well if terminal width is zero, but
that's better than a potential crash (I think the crash would be in the
progress background thread, so I'm not sure how bad it would be, but
still its good to avoid).
Sometimes if the progress lines are wider than the terminal width,
the supershell blank zone can expand indefinitely because be do not move
the cursor far enough up to properly re-fill the blank zone.
It takes about a second to load scala.reflect.runtime.universe. If we
lazy load here, we can load scala.relect.runtime.universe in the
background to speed up the sbt start up time. See
0ebb7a5662.
The previous implementation of supershell log line interlacing with
regular line interlacing relied on state in a global object. A somewhat
better approach is for each appender to hold a reference to a state
object. Every time tasks run, new appenders are created, so the state
should always reflect the current progress state.
eugene yokota
Ethan Atkins
Diego E. Alonso-Blas