Buffer unix and lock windows to prevent message interleaving #35975
Conversation
|
cc @aturon |
|
Also worth a mention that this does allow interleaving of error messages themselves. For example, this is still valid output: (notice: the multiple "aborting" at the end) This is desirable as it ensures we're still probably streaming out errors as they happen rather than buffering multiple messages and emitting them as one group. |
|
Can we also deduplicate the messages on unix? This came up on IRC today, so it's great that part of the problem is being fixed! |
|
@durka - those are from multiple compiles running simultaneously, not from a single compilation. |
|
Oh right. I guess what I'm asking for would be another buffer, in cargo. |
|
|
||
| //! Bindings to acquire a global named lock. | ||
| //! | ||
| //! This is intended to be used to synchronize multiple compiler processes to |
There was a problem hiding this comment.
This comment should probably be updated.
There was a problem hiding this comment.
How are you thinking it should be updated?
Update: helps if I actually keep reading past your comment ;) Yes, I'll update that.
| @@ -724,7 +724,10 @@ impl EmitterWriter { | |||
| } | |||
| match write!(&mut self.dst, "\n") { | |||
| Err(e) => panic!("failed to emit error: {}", e), | |||
| _ => () | |||
| _ => match self.dst.flush() { | |||
There was a problem hiding this comment.
Is this flush necessary? There's a flush below as well it looks lke.
There was a problem hiding this comment.
This flush allows us to print the last "\n" at the end of the errors printing:
error[E0101]: cannot determine a type for this expression: unconstrained type
--> src/test/compile-fail/E0101.rs:12:13
|
12 | let x = |_| {};
| ^^^^^^ cannot resolve type of expression
error: aborting due to previous error
jturner-23759: rust jturner$
vs
error[E0101]: cannot determine a type for this expression: unconstrained type
--> src/test/compile-fail/E0101.rs:12:13
|
12 | let x = |_| {};
| ^^^^^^ cannot resolve type of expression
error: aborting due to previous error
jturner-23759: rust jturner$
sophiajt
force-pushed
the
error_buffering
branch
from
9024f11 to
a65b201
Compare
|
Commented and code updated based on recommendations |
Buffer unix and lock windows to prevent message interleaving
When cargo does a build on multiple processes, multiple crates may error at the same time. If this happens, currently you'll see interleaving of the error messages, which makes for an unreadable message.
Example:
```
--> --> src/bin/multithread-unix.rs:16:35src/bin/singlethread.rs:16:24
||
1616 | | Server::new(&addr).workers(8). Server::new(&addr).serveserve(|r: Request| {(|r: Request| {
| | ^^^^^^^^^^ expected struct `std::io::Error`, found () expected struct `std::io::Error`, found ()
||
= = notenote: expected type `std::io::Error`: expected type `std::io::Error`
= = notenote: found type `()`: found type `()`
= = notenote: required because of the requirements on the impl of `futures_minihttp::Service<futures_minihttp::Request, futures_minihttp::Response>` for `[closure@src/bin/multithread-unix.rs:16:41: 22:6]`: required because of the requirements on the impl of `futures_minihttp::Service<futures_minihttp::Request, futures_minihttp::Response>` for `[closure@src/bin/singlethread.rs:16:30: 22:6]`
error: aborting due to previous error
error: aborting due to previous error
```
This patch uses two techniques to prevent this interleaving. On Unix systems, since they use the text-based ANSI protocol for coloring, we can safely buffer up whole messages before we emit. This PR does this buffering, and emits the whole message at once.
On Windows, term must use the Windows terminal API to color the output. This disallows us from using the same buffering technique. Instead, here we grab a Windows mutex (thanks to @alexcrichton for the lock code). This lock only works in Windows and will hold a mutex for the duration of a message output.
r? @nikomatsakis
|
💔 Test failed - auto-win-msvc-64-opt-rustbuild |
|
@bors retry Looks like it may be something race-y going on with the tests/directories. |
When cargo does a build on multiple processes, multiple crates may error at the same time. If this happens, currently you'll see interleaving of the error messages, which makes for an unreadable message.
Example:
This patch uses two techniques to prevent this interleaving. On Unix systems, since they use the text-based ANSI protocol for coloring, we can safely buffer up whole messages before we emit. This PR does this buffering, and emits the whole message at once.
On Windows, term must use the Windows terminal API to color the output. This disallows us from using the same buffering technique. Instead, here we grab a Windows mutex (thanks to @alexcrichton for the lock code). This lock only works in Windows and will hold a mutex for the duration of a message output.
r? @nikomatsakis