-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Improve context of bootstrap errors in CI #145565
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
Conversation
|
thanks Jakub <3 |
9941402
to
c8d9fb2
Compare
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.
I'm on board with commits 1 and 2. Commit 3 scares me a bit, it feels a bit too magical... I would prefer if you reverted that, yes. You can r=me on commits 1 and 2.
@rustbot author |
c8d9fb2
to
a1f5bbe
Compare
Ok. I think that even without the "steptrace", we should have enough information now. @bors r=jieyouxu |
Rollup of 19 pull requests Successful merges: - #140956 (`impl PartialEq<{str,String}> for {Path,PathBuf}`) - #141744 (Stabilize `ip_from`) - #142681 (Remove the `#[no_sanitize]` attribute in favor of `#[sanitize(xyz = "on|off")]`) - #142871 (Trivial improve doc for transpose ) - #144252 (Do not copy .rmeta files into the sysroot of the build compiler during check of rustc/std) - #144476 (rustdoc-search: search backend with partitioned suffix tree) - #144567 (Fix RISC-V Test Failures in ./x test for Multiple Codegen Cases) - #144804 (Don't warn on never to any `as` casts as unreachable) - #144960 ([RTE-513] Ignore sleep_until test on SGX) - #145013 (overhaul `&mut` suggestions in borrowck errors) - #145041 (rework GAT borrowck limitation error) - #145243 (take attr style into account in diagnostics) - #145405 (cleanup: use run_in_tmpdir in run-make/rustdoc-scrape-examples-paths) - #145432 (cg_llvm: Small cleanups to `owned_target_machine`) - #145484 (Remove `LlvmArchiveBuilder` and supporting code/bindings) - #145557 (Fix uplifting in `Assemble` step) - #145563 (Remove the `From` derive macro from prelude) - #145565 (Improve context of bootstrap errors in CI) - #145584 (interpret: avoid forcing all integer newtypes into memory during clear_provenance) Failed merges: - #145359 (Fix bug where `rustdoc-js` tester would not pick the right `search.js` file if there is more than one) - #145573 (Add an experimental unsafe(force_target_feature) attribute.) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #145565 - Kobzol:bootstrap-ci-print-error, r=jieyouxu Improve context of bootstrap errors in CI Inspired by https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/printing.20test.20suite.20name.20by.20default/with/534920583, this PR attempts to improve the context displayed when a bootstrap invocation fails in CI. Since #145261, we now see the latest started step when a failure occurs. However, we can go further. 1) The first commit prints the actual executed bootstrap invocation command arguments when bootstrap ends. Since CI jobs often run multiple bootstrap commands, this makes it easier to figure out which one of them failed (before it was annoying having to search for that in CI logs). Because bootstrap doesn't really use `Result`s much, and most of them time it ends with the `detail_exit` function, which YOLOs `std::process::exit(...)`, I added the print there. 2) Adds `#[track_caller]` to a few bootstrap Cargo builder functions. This makes the log that we print when a command fails more accurate: ``` 2025-08-16T18:18:51.6998201Z Command ... failed ... 2025-08-16T18:18:51.7003653Z Created at: src/bootstrap/src/core/builder/cargo.rs:423:33 2025-08-16T18:18:51.7004032Z Executed at: src/bootstrap/src/core/build_steps/doc.rs:933:26 ``` Before, the `cargo.rs:XYZ` location wasn't very useful. 3) Is the most wild thing (I'll revert if you find it too magical). We store the step stack of the currently active `Builder` instance in a global variable, and when bootstrap exits with a failure, we print the stack, to make it easier to find out what was happening when a failure occurred. We could print an actual captured `Backtrace`, but I think that would be too much information in the common case. We now pass `RUST_BACKTRACE=1` on CI, so if bootstrap actually crashes unexpectedly, we would see the stacktrace. The end of the bootsrap failure log in CI now looks like this now: ``` Bootstrap failed while executing `x build library` ---BOOTSTRAP step stack start--- Assemble { target_compiler: Compiler { stage: 1, host: x86_64-unknown-linux-gnu, forced_compiler: false } } Rustc { target: x86_64-unknown-linux-gnu, build_compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, crates: [] } ---BOOTSTRAP step stack end--- ``` r? `@jieyouxu`
Inspired by https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/printing.20test.20suite.20name.20by.20default/with/534920583, this PR attempts to improve the context displayed when a bootstrap invocation fails in CI.
Since #145261, we now see the latest started step when a failure occurs. However, we can go further.
Result
s much, and most of them time it ends with thedetail_exit
function, which YOLOsstd::process::exit(...)
, I added the print there.#[track_caller]
to a few bootstrap Cargo builder functions. This makes the log that we print when a command fails more accurate:Before, the
cargo.rs:XYZ
location wasn't very useful.3) Is the most wild thing (I'll revert if you find it too magical). We store the step stack of the currently active
Builder
instance in a global variable, and when bootstrap exits with a failure, we print the stack, to make it easier to find out what was happening when a failure occurred. We could print an actual capturedBacktrace
, but I think that would be too much information in the common case. We now passRUST_BACKTRACE=1
on CI, so if bootstrap actually crashes unexpectedly, we would see the stacktrace.The end of the bootsrap failure log in CI now looks like this now:
r? @jieyouxu