-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Fix spurious error when running build --stage 2 compiler/rustc
#96859
Conversation
This comment has been minimized.
This comment has been minimized.
src/bootstrap/compile.rs
Outdated
}); | ||
// NOTE: `builder.compiler` has the side effect of running `Assemble` | ||
let target_compiler = Compiler { stage: run.builder.top_stage + 1, host: run.target }; | ||
if run.builder.top_stage >= 2 { |
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 would have expected full_bootstrap to play a role here, I think.
It also feels a little iffy that this is only changing make_run -- builder.compiler(...) invokes Assemble via ensure, which won't run this code, right?
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.
Correct, that's what I meant in the description by "if you don't want to change the stage numbering, then I'll need to fix the sysroot issue eventually anyway and it doesn't make sense to merge this PR". The clippy PR uses builder.compiler
and so won't be fixed by this change.
I am not going to put a ton more time into this PR until the clippy PR is unblocked, but I'll investigate full_bootstrap at that time, I hadn't thought about it.
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.
Someone commented on #90244 this was causing issues, so I've updated the PR. It now takes into account full-bootstrap; I've tested that and it works fine. I also tested that build --stage 2 compiler/rustc
now works:
$ x build --stage 2 compiler/rustc
Building rustbuild
Compiling bootstrap v0.0.0 (/home/jnelson/rust-lang/rust2/src/bootstrap)
Finished dev [unoptimized] target(s) in 12.80s
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Finished release [optimized] target(s) in 0.19s
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building stage0 compiler artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Finished release [optimized] target(s) in 0.32s
Copying stage0 rustc from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Assembling stage1 compiler (x86_64-unknown-linux-gnu)
Building stage1 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Finished release [optimized] target(s) in 0.20s
Copying stage1 std from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Uplifting stage1 std (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Copying stage3 std from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building stage1 compiler artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Finished release [optimized] target(s) in 0.31s
Copying stage1 rustc from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Uplifting stage1 rustc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Copying stage3 rustc from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Build completed successfully in 0:00:14
@rustbot label -S-waiting-on-author +S-blocked |
This comment has been minimized.
This comment has been minimized.
abd46ff
to
32a6c02
Compare
This comment has been minimized.
This comment has been minimized.
rust-lang#89759 introduced a panic: ``` Assembling stage3 compiler (x86_64-apple-darwin) thread 'main' panicked at 'fs::read(stamp) failed with No such file or directory (os error 2) ("/Users/user/rust2/build/x86_64-apple-darwin/stage2-rustc/x86_64-apple-darwin/release/.librustc.stamp")', src/bootstrap/lib.rs:1296:24 ``` This wasn't actually a bug in that PR - the problem was that `x build --stage 3` is broken, and has been for quite some time, even ignoring the stamp file error: ``` thread 'main' panicked at 'src.symlink_metadata() failed with No such file or directory (os error 2) ("failed to determine metadata of /home/jnelson/rust-lang/rust/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/rustc-main")', src/bootstrap/lib.rs:1414:24 ``` It needs to take into account whether the artifacts from stage1 are being reused, rather than blindly assuming rustc will be recompiled. Doing so is kind of annoying, because it requires knowing the target the compiler is being built for. Instead, just revert to the old behavior of `build --stage 2 compiler/rustc`, which avoids trying to create the sysroot in the first place.
The job Click to see the possible cause of the failure (guessed by this bot)
|
// NOTE: Anywhere in bootstrap that calls `builder.compiler` has the side effect of running | ||
// `Assemble`. In those cases, this workaround in `make_run` doesn't help, we'll still hit a | ||
// panic (see #90224). In practice, that's probably ok: nothing currently uses the stage 2 | ||
// sysroot. |
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 a little confused why we put this in make_run, rather than in the main body of the Assemble step. That would seem to prevent this FIXME from being necessary.
@jyn514 FYI: when a PR is ready for review, send a message containing |
This change is important, and complicated enough I doubt someone else will pick it up, but I also don't really have the energy to work on it for the foreseeable future. |
closing in favor of #108288 |
#89759 introduced a panic:
This wasn't actually a bug in that PR - the problem was that
x build --stage 3
is broken, and has been for quite some time, even ignoring the stamp file error:
It needs to take into account whether the artifacts from stage1 are being reused, rather than blindly assuming rustc will be recompiled.
Doing so is kind of annoying, because it requires knowing the target the compiler is being built for.
Instead, just revert to the old behavior of
build --stage 2 compiler/rustc
, which avoids trying to create the sysroot in the first place.Note that this does not help with #96798 (comment), which really does need the sysroot available. I think requiring the stage 3 sysroot there is a bug - we should either use
test --stage 1
in CI, or change the stage numbering so thattest --stage 2
matches the behavior of other tools (i.e. it runs the clippy in the stage 2 toolchain, not the clippy built by the stage 2 toolchain).Fixes #90244.
r? @Mark-Simulacrum