Skip to content
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 2 commits into from
Aug 9, 2024

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Aug 9, 2024

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

@rustbot 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
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

src/bootstrap/src/lib.rs Outdated Show resolved Hide resolved
@onur-ozkan
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 9, 2024

📌 Commit a380d5e has been approved by onur-ozkan

It is now in the queue for this repository.

@bors 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
@bors bors merged commit cea3b42 into rust-lang:master Aug 9, 2024
6 checks passed
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````
@rustbot rustbot added this to the 1.82.0 milestone Aug 9, 2024
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
@Kobzol Kobzol deleted the cmd-verbose-logging branch August 9, 2024 19:59
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants