-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable verbose bootstrap command failure logging by default #128874
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
labels
Aug 9, 2024
onur-ozkan
approved these changes
Aug 9, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Kobzol
force-pushed
the
cmd-verbose-logging
branch
from
August 9, 2024 12:59
2d04e2d
to
a380d5e
Compare
@bors r+ |
bors
added
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Aug 9, 2024
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 9, 2024
…-ozkan Disable verbose bootstrap command failure logging by default One of my recent bootstrap command refactoring PRs enabled verbose logging of command failures by default. While this is great for debugging bootstrap, in many situations it's just too verbose and prevents the user from seeing the actual printed stdout/stderr, which usually contains much more useful information. This PR reverts that logic, and only prints a detailed error when `-v` is passed to bootstrap. r? `@onur-ozkan`
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 9, 2024
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#128742 (miri: make vtable addresses not globally unique) - rust-lang#128815 (Add `Steal::is_stolen()`) - rust-lang#128859 (Fix the name of signal 19 in library/std/src/sys/pal/unix/process/process_unix/tests.rs for mips/sparc linux) - rust-lang#128864 (Use `SourceMap::end_point` instead of `- BytePos(1)` in arg removal suggestion) - rust-lang#128865 (Ensure let stmt compound assignment removal suggestion respect codepoint boundaries) - rust-lang#128874 (Disable verbose bootstrap command failure logging by default) r? `@ghost` `@rustbot` modify labels: rollup
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 9, 2024
…-ozkan Disable verbose bootstrap command failure logging by default One of my recent bootstrap command refactoring PRs enabled verbose logging of command failures by default. While this is great for debugging bootstrap, in many situations it's just too verbose and prevents the user from seeing the actual printed stdout/stderr, which usually contains much more useful information. This PR reverts that logic, and only prints a detailed error when `-v` is passed to bootstrap. r? ``@onur-ozkan``
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
Aug 9, 2024
…-ozkan Disable verbose bootstrap command failure logging by default One of my recent bootstrap command refactoring PRs enabled verbose logging of command failures by default. While this is great for debugging bootstrap, in many situations it's just too verbose and prevents the user from seeing the actual printed stdout/stderr, which usually contains much more useful information. This PR reverts that logic, and only prints a detailed error when `-v` is passed to bootstrap. r? ```@onur-ozkan```
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 9, 2024
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#128815 (Add `Steal::is_stolen()`) - rust-lang#128817 (VxWorks code refactored ) - rust-lang#128822 (add `builder-config` into tarball sources) - rust-lang#128838 (rustdoc: do not run doctests with invalid langstrings) - rust-lang#128852 (use stable sort to sort multipart diagnostics) - rust-lang#128859 (Fix the name of signal 19 in library/std/src/sys/pal/unix/process/process_unix/tests.rs for mips/sparc linux) - rust-lang#128864 (Use `SourceMap::end_point` instead of `- BytePos(1)` in arg removal suggestion) - rust-lang#128865 (Ensure let stmt compound assignment removal suggestion respect codepoint boundaries) - rust-lang#128874 (Disable verbose bootstrap command failure logging by default) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 9, 2024
Rollup merge of rust-lang#128874 - Kobzol:cmd-verbose-logging, r=onur-ozkan Disable verbose bootstrap command failure logging by default One of my recent bootstrap command refactoring PRs enabled verbose logging of command failures by default. While this is great for debugging bootstrap, in many situations it's just too verbose and prevents the user from seeing the actual printed stdout/stderr, which usually contains much more useful information. This PR reverts that logic, and only prints a detailed error when `-v` is passed to bootstrap. r? ````@onur-ozkan````
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 9, 2024
…llaumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#128815 (Add `Steal::is_stolen()`) - rust-lang#128822 (add `builder-config` into tarball sources) - rust-lang#128838 (rustdoc: do not run doctests with invalid langstrings) - rust-lang#128852 (use stable sort to sort multipart diagnostics) - rust-lang#128859 (Fix the name of signal 19 in library/std/src/sys/pal/unix/process/process_unix/tests.rs for mips/sparc linux) - rust-lang#128864 (Use `SourceMap::end_point` instead of `- BytePos(1)` in arg removal suggestion) - rust-lang#128865 (Ensure let stmt compound assignment removal suggestion respect codepoint boundaries) - rust-lang#128874 (Disable verbose bootstrap command failure logging by default) r? `@ghost` `@rustbot` modify labels: rollup
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 15, 2024
…onur-ozkan Print more verbose error for commands that capture output rust-lang#128874 made bootstrap command errors less verbose without `-v`. However, in some cases it's too extreme. If a command fails, it now outputs just `Command has failed. Rerun with -v to see more details.`, without providing any context. I think that I found a reasonable heuristic to figure out when we should print a more verbose error. When the command doesn't capture output, its stdout/stderr is printed, therefore the user sees context about the error. However, when the command captures its output, the user won't see any error message in the output, which is not great. So only in that case, bootstrap now prints a slightly more verbose output (and also prints the captured output). r? `@onur-ozkan`
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 15, 2024
Rollup merge of rust-lang#129096 - Kobzol:bootstrap-cmd-verbosity, r=onur-ozkan Print more verbose error for commands that capture output rust-lang#128874 made bootstrap command errors less verbose without `-v`. However, in some cases it's too extreme. If a command fails, it now outputs just `Command has failed. Rerun with -v to see more details.`, without providing any context. I think that I found a reasonable heuristic to figure out when we should print a more verbose error. When the command doesn't capture output, its stdout/stderr is printed, therefore the user sees context about the error. However, when the command captures its output, the user won't see any error message in the output, which is not great. So only in that case, bootstrap now prints a slightly more verbose output (and also prints the captured output). r? `@onur-ozkan`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One of my recent bootstrap command refactoring PRs enabled verbose logging of command failures by default. While this is great for debugging bootstrap, in many situations it's just too verbose and prevents the user from seeing the actual printed stdout/stderr, which usually contains much more useful information.
This PR reverts that logic, and only prints a detailed error when
-v
is passed to bootstrap.r? @onur-ozkan