-
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
Follow-up work for download-rustc
#81930
Comments
While working on this, I found that the handling of Note that this has local changes enabling the constant for clippy: diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs
index 5c874f69bd9..c82b78c1c71 100644
--- a/src/bootstrap/tool.rs
+++ b/src/bootstrap/tool.rs
@@ -564,6 +564,7 @@ pub struct Cargo {
impl Step for Cargo {
type Output = PathBuf;
const DEFAULT: bool = true;
+ const ENABLE_DOWNLOAD_RUSTC: bool = true;
const ONLY_HOSTS: bool = true;
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -647,6 +648,7 @@ pub struct $name {
impl Step for $name {
type Output = Option<PathBuf>;
const DEFAULT: bool = true; // Overwritten below
+ const ENABLE_DOWNLOAD_RUSTC: bool = true;
const ONLY_HOSTS: bool = true;
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
|
lol I also found that this completely ignores all test suites, making it pretty useless:
|
This was introduced as part of the MVP for `download-rustc`. Unfortunately, it doesn't work very well: - Steps are ignored by default, which makes it easy to leave out a step that should be built. For example, the MVP forgot to enable any tests, so it was *only* possible to build locally. - It didn't work correctly even when it was enabled: calling `builder.ensure()` would completely ignore the constant and rebuild the step anyway. This has no obvious fix since `ensure()` has to return a `Step::Output`. Instead, this handles `download-rustc` in `impl Step for Rustc` and `impl Step for Std`, which to my knowledge are the only build steps that don't first go through `impl Step for Sysroot` (`Rustc` is used for the `rustc-dev` component). See rust-lang#79540 (comment) and rust-lang#81930 for further context. Here are some example runs with these changes and `download-rustc` enabled: ``` $ x.py build src/tools/clippy Building stage1 tool clippy-driver (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 1m 09s Building stage1 tool cargo-clippy (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.11s $ x.py test src/tools/clippy Updating only changed submodules Submodules updated in 0.01 seconds Finished dev [unoptimized + debuginfo] target(s) in 0.09s Building stage1 tool clippy-driver (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.09s Building rustdoc for stage1 (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.28s Finished release [optimized] target(s) in 15.26s Running build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/release/deps/clippy_driver-8b407b140e0aa91c test result: ok. 592 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out $ x.py build src/tools/rustdoc Building rustdoc for stage1 (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 41.28s Build completed successfully in 0:00:41 $ x.py test src/test/rustdoc-ui Building stage0 tool compiletest (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.12s Building rustdoc for stage1 (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.10s test result: ok. 105 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.15s $ x.py build compiler/rustc Finished dev [unoptimized + debuginfo] target(s) in 0.09s Build completed successfully in 0:00:00 ``` Note a few things: - Clippy depends on stage1 rustc-dev artifacts, but rustc didn't have to be recompiled. Instead, the artifacts were copied automatically. - All steps are always enabled. There is no danger of forgetting a step, since only the entrypoints have to handle `download-rustc`. - Building the compiler (`compiler/rustc`) automatically does no work.
I don't think this should be enabled by default until this is addressed as well so that it won't use the CI rustc if you've made local changes to rustc. Otherwise it would be really frustrating for people making changes to rustc and rustdoc (et al.). |
@camelid that's why it has a warning. Do you think it should force you to disable the option? That seems like it could be frustrating. |
I definitely don't think it should silently use the in-tree compiler if you make changes, it should always do what you specify in config.toml. |
I was thinking that there could be a |
Ah ok, that seems useful. I don't think it needs to block turning this on by default, though. |
Won't this be annoying to new rustc contributors who have to dig around in their settings? |
@camelid I found the confusion. When I say "by-default", I mean that I want to add a new |
Ah, I had a feeling we were talking past each other :) Sounds good to me, as long as the warning is helpful enough that if people start doing compiler contributions they don't get stuck. |
…imulacrum Remove `ENABLE_DOWNLOAD_RUSTC` constant `ENABLE_DOWNLOAD_RUSTC` was introduced as part of the MVP for `download-rustc` as a way not to rebuild artifacts that have already been downloaded. Unfortunately, it doesn't work very well: - Steps are ignored by default, which makes it easy to leave out a step that should be built. For example, the MVP forgot to enable any tests, so it was only possible to *build* locally. - It didn't work correctly even when it was enabled: calling `builder.ensure()` would completely ignore the constant and rebuild the step anyway. This has no obvious fix since `ensure()` has to return a `Step::Output`. Instead, this handles `download-rustc` in `impl Step for Rustc` and `impl Step for Std`, which to my knowledge are the only build steps that don't first go through `impl Step for Sysroot` (`Rustc` is used for the `rustc-dev` component). See rust-lang#79540 (comment) and rust-lang#81930 for further context. Here are some example runs with these changes and `download-rustc` enabled: ``` $ x.py build src/tools/clippy Building stage1 tool clippy-driver (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 1m 09s Building stage1 tool cargo-clippy (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.11s $ x.py test src/tools/clippy Finished dev [unoptimized + debuginfo] target(s) in 0.09s Building stage1 tool clippy-driver (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.09s Building rustdoc for stage1 (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.28s Finished release [optimized] target(s) in 15.26s Running build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/release/deps/clippy_driver-8b407b140e0aa91c test result: ok. 592 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out $ x.py build src/tools/rustdoc Building rustdoc for stage1 (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 41.28s Build completed successfully in 0:00:41 $ x.py test src/test/rustdoc-ui Building stage0 tool compiletest (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.12s Building rustdoc for stage1 (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.10s test result: ok. 105 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.15s $ x.py build compiler/rustc Finished dev [unoptimized + debuginfo] target(s) in 0.09s Build completed successfully in 0:00:00 ``` Note a few things: - Clippy depends on stage1 rustc-dev artifacts, but rustc didn't have to be recompiled. Instead, the artifacts were copied automatically. - All steps are always enabled. There is no danger of forgetting a step, since only the entrypoints have to handle `download-rustc`. - Building the compiler (`compiler/rustc`) automatically does no work. Helps with rust-lang#81930. r? `@Mark-Simulacrum`
…lacrum Always compile rustdoc with debug logging enabled when `download-rustc` is set Previously, logging at DEBUG or below would always be silenced, because rustc compiles tracing with the `static_max_level_info` feature. That makes sense for release artifacts, but not for developing rustdoc. Instead, this compiles two different versions of tracing: one in the release artifacts, distributed in the sysroot, and a new version compiled by rustdoc. Since `rustc_driver` is always linked to the version of sysroot, this copy/pastes `init_env_logging` into rustdoc. To avoid compiling an unnecessary version of tracing when `download-rustc` isn't set, this adds a new `using-ci-artifacts` feature for rustdoc and passes that feature in bootstrap. Addresses rust-lang#81930. This builds on rust-lang#79540. r? `@Mark-Simulacrum`
Err it looks like |
Add `x.py setup tools` which enables `download-rustc` by default Helps with rust-lang#81930. I know I said in that issue that I should fix that rebasing rebuilds bootstrap, but the compile time improvement is so good I think it's ok to leave that fix for later (I still plan to work on it). I think all the outright bugs have been fixed :) This builds on rust-lang#83368 so I can set the option to `if-unchanged`. r? `@Mark-Simulacrum`
Add `x.py setup tools` which enables `download-rustc` by default Helps with rust-lang#81930. I know I said in that issue that I should fix that rebasing rebuilds bootstrap, but the compile time improvement is so good I think it's ok to leave that fix for later (I still plan to work on it). I think all the outright bugs have been fixed :) This builds on rust-lang#83368 so I can set the option to `if-unchanged`. r? ``@Mark-Simulacrum``
Add `x.py setup tools` which enables `download-rustc` by default Helps with rust-lang#81930. I know I said in that issue that I should fix that rebasing rebuilds bootstrap, but the compile time improvement is so good I think it's ok to leave that fix for later (I still plan to work on it). I think all the outright bugs have been fixed :) This builds on rust-lang#83368 so I can set the option to `if-unchanged`. r? ```@Mark-Simulacrum```
Oh huh, this is an unexpected side-effect: since #82739, building compiletest also requires building stage0 std first. I think using master libstd is intentional since #59264? I don't think it necessarily needs to change, maybe just documented somewhere? Anyway, that fact that just works without build errors makes me a lot more confident we're choosing the right abstractions in bootstrap :) |
Reposting from this Zulip discussion as requested by I just ran into something confusing with I'm guessing this is because rustdoc built with I imagine it could be even more confusing for new contributors since we recommend using stage1 builds, but then when they use Is there any way to solve this problem? I imagine there are multiple possible solutions, some better than others. |
@bjorn3 suggests deleting the |
Great idea! That would be very helpful. |
Done in 973ff03 |
I'm going to cross this off the list - right now download-ci-llvm gives a useful error first and I think it's very rare to want to download rustc but not LLVM.
|
Fix `x check --stage 1` when download-rustc is enabled Helps with rust-lang#81930
…ertlarsan68 Fix `x test ui --target foo` when download-rustc is enabled Previously, we would never build the target std, only the host std: ``` ; x t tests/ui/attributes --target wasm32-unknown-unknown Building bootstrap Finished dev [unoptimized] target(s) in 0.02s Building stage0 library artifacts (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.08s Building tool compiletest (stage0) Finished release [optimized] target(s) in 0.09s Check compiletest suite=ui mode=ui (x86_64-unknown-linux-gnu -> wasm32-unknown-unknown) thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { depth: 0, inner: Io { path: Some("/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/wasm32-unknown-unknown/lib"), err: Os { code: 2, kind: NotFound, message: "No such file or directory" } } }', src/tools/compiletest/src/main.rs:842:31 ``` Helps with rust-lang#81930.
…ertlarsan68 Fix `x test ui --target foo` when download-rustc is enabled Previously, we would never build the target std, only the host std: ``` ; x t tests/ui/attributes --target wasm32-unknown-unknown Building bootstrap Finished dev [unoptimized] target(s) in 0.02s Building stage0 library artifacts (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.08s Building tool compiletest (stage0) Finished release [optimized] target(s) in 0.09s Check compiletest suite=ui mode=ui (x86_64-unknown-linux-gnu -> wasm32-unknown-unknown) thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { depth: 0, inner: Io { path: Some("/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/wasm32-unknown-unknown/lib"), err: Os { code: 2, kind: NotFound, message: "No such file or directory" } } }', src/tools/compiletest/src/main.rs:842:31 ``` Helps with rust-lang#81930.
…ertlarsan68 Fix `x test ui --target foo` when download-rustc is enabled Previously, we would never build the target std, only the host std: ``` ; x t tests/ui/attributes --target wasm32-unknown-unknown Building bootstrap Finished dev [unoptimized] target(s) in 0.02s Building stage0 library artifacts (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.08s Building tool compiletest (stage0) Finished release [optimized] target(s) in 0.09s Check compiletest suite=ui mode=ui (x86_64-unknown-linux-gnu -> wasm32-unknown-unknown) thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { depth: 0, inner: Io { path: Some("/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/wasm32-unknown-unknown/lib"), err: Os { code: 2, kind: NotFound, message: "No such file or directory" } } }', src/tools/compiletest/src/main.rs:842:31 ``` Helps with rust-lang#81930.
…ertlarsan68 Fix `x test ui --target foo` when download-rustc is enabled Previously, we would never build the target std, only the host std: ``` ; x t tests/ui/attributes --target wasm32-unknown-unknown Building bootstrap Finished dev [unoptimized] target(s) in 0.02s Building stage0 library artifacts (x86_64-unknown-linux-gnu) Finished release [optimized] target(s) in 0.08s Building tool compiletest (stage0) Finished release [optimized] target(s) in 0.09s Check compiletest suite=ui mode=ui (x86_64-unknown-linux-gnu -> wasm32-unknown-unknown) thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { depth: 0, inner: Io { path: Some("/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/wasm32-unknown-unknown/lib"), err: Os { code: 2, kind: NotFound, message: "No such file or directory" } } }', src/tools/compiletest/src/main.rs:842:31 ``` Helps with rust-lang#81930.
…lacrum download-rustc: Give a better error message if artifacts can't be dowloaded It should be very rare in practice to happen; people would need to both have `download-ci-llvm` disabled and `download-rustc` enabled. I think it may be more common if we start turning this on by default, though. Helps with rust-lang#81930. Before: ``` downloading https://ci-artifacts.rust-lang.org/rustc-builds/bf5cad8e775fb326465e5c1b98693e5d259da156/rust-std-nightly-x86_64-unknown-linux-gnu.tar.xz curl: (22) The requested URL returned error: 404 ``` After: ``` downloading https://ci-artifacts.rust-lang.org/rustc-builds/bf5cad8e775fb326465e5c1b98693e5d259da156/rust-std-nightly-x86_64-unknown-linux-gnu.tar.xz curl: (22) The requested URL returned error: 404 error: failed to download pre-built rustc from CI note: old builds get deleted after a certain time help: if trying to compile an old commit of rustc, disable `download-rustc` in config.toml: [rust] download-rustc = false ```
this is now fixed well enough that |
These are all the features for
download-rustc
that didn't land in the initial MVP (#79540).Required before this is made a default
x.py setup tools
will compile rustbuild twice. Allow building rustdoc without first building rustc (MVP) #79540 (comment), Use the beta compiler for building bootstrap tools whendownload-rustc
is set #82739debug
andtrace
logging). Always compile rustdoc with debug logging enabled whendownload-rustc
is set #81932ENABLE_DOWNLOAD_RUSTC
constant #82480git log --author=bors
sometimes breaks. This should usegit merge-base
instead, or use some other workaround that doesn't break. Allow building rustdoc without first building rustc (MVP) #79540 (comment), Fix commit detected when usingdownload-rustc
#82740src/version
is modified #83350Nice to have
download-rustc = "if-unchanged"
which rebuilds rustc instead of warning whencompiler/
has been modified. Follow-up work fordownload-rustc
#81930 (comment), Adddownload-rustc = "if-unchanged"
#83368x.py check --stage 0
doesn't work. Follow-up work fordownload-rustc
#81930 (comment), Use the beta compiler for building bootstrap tools whendownload-rustc
is set #82739x.py check --stage 1 src/tools/rustdoc
doesn't work. Follow-up work fordownload-rustc
#81930 (comment), fixed in Fixx check --stage 1
when download-rustc is enabled #110121x.py setup tools
to enable this conveniently (it doesn't make sense to use this for compiler developers). This shouldn't be added until people have used this feature for a little bit and it works reasonably well. Addx.py setup tools
which enablesdownload-rustc
by default #83370x test ui --target foo
when download-rustc is enabled #110113CombineThis doesn't seem useful.download-rustc
anddownload-ci-llvm
into the same option. This needs quite a bit more design work. Allow building rustdoc without first building rustc (MVP) #79540 (comment), Unifydownload-*
settings in config.toml #83369The text was updated successfully, but these errors were encountered: