Fix rustc-guide build failure ignoring no-fail-fast #63089
Conversation
kennytm
added
T-bootstrap
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
kennytm
force-pushed
the
use-try-run-for-linkchecker
branch
from
62196d1 to
20924d2
Compare
kennytm
removed
the
S-waiting-on-review
src/bootstrap/test.rs
Outdated
| builder.run(rustbook_cmd | ||
| .arg("linkcheck") | ||
| .arg(&src)); | ||
| try_run(builder, rustbook_cmd.arg("linkcheck").arg(&src)); |
There was a problem hiding this comment.
We'll definitely want try_run_quiet here as try_run eats the command's output (a horrible and non-obvious default, I know -- we should fix that).
There was a problem hiding this comment.
Wait so quiet makes it not eat the output? That's, like, the opposite of what the name indicates.^^
There was a problem hiding this comment.
Yep, try_run internally calls run_silent which will eat all the output unconditionally, I believe.
There was a problem hiding this comment.
The one in lib.rs does, yeah. The one in test.rs calls builder.run.
So Builder::try_run should maybe just be renamed to try_run_silent, like the free-standing function it calls?
There was a problem hiding this comment.
Fun times, there's both free-standing run_silent (called by Builder::run) and run_suppressed (called by Builder::run_quiet).
There was a problem hiding this comment.
Actually, @Mark-Simulacrum are you sure about this? try_run_silent calls Command::status which says that
By default, stdin, stdout and stderr are inherited from the parent.
I have no idea why it is called "silent". There is no comment explaining that.
In contrast, run_suppressed (the helper behind Builder::run_quiet) calls Command::output, which does capture the output, and then throws it away (unless there was an error).
There was a problem hiding this comment.
See #63196: I think you are wrong and try_run does not eat the commends output. In contrast, try_run_quiet does eat (suppress) the output.
There was a problem hiding this comment.
Hm, you could definitely be right! I don't have a good handle on these functions beyond "too many and bad naming" :)
|
r=me with that and the azure config change reverted |
This caused clippy not being built on Linux previously.
kennytm
force-pushed
the
use-try-run-for-linkchecker
branch
from
20924d2 to
ebd1bf7
Compare
kennytm
changed the title
kennytm
added
S-waiting-on-review
|
@bors r=Mark-Simulacrum |
|
📌 Commit ebd1bf7 has been approved by |
bors
added
S-waiting-on-bors
…mulacrum Fix rustc-guide build failure ignoring no-fail-fast
|
☀️ Test successful - checks-azure |
build_helper: try less confusing method names build_helper's `*_silent` methods were likely called that way because they do not print the command being run to stdout. [In the original file this all makes sense](rust-lang@046e687#diff-5c3d6537a43ecae03014e118a7fe3321). But later it also gained `*_suppressed` methods and the difference between `silent` and `suppressed` is far from clear. So rename `run` (which prints the command being run) to `run_verbose`. Then we can call the methods that just run a command and show its output but nothing extra `run` and `try_run`. `run_verbose` (formerly `run`) is unused from what I can tell. Should I remove it? r? @alexcrichton Cc @Mark-Simulacrum Also see rust-lang#63089 (comment).
No description provided.