Skip to content

Commit

Permalink
Auto merge of rust-lang#137215 - onur-ozkan:rustc-tool-build-stages, …
Browse files Browse the repository at this point in the history 
…r=jieyouxu,Kobzol

stabilize stage management for rustc tools

rust-lang#135990 got out of control due to excessive complexity. This PR aims to achieve the same goal with a simpler approach, likely through multiple smaller PRs. I will keep the other one read-only and open as a reference for future work.

This work stabilizes the staging logic for `ToolRustc` programs, so you no longer need to handle build and target compilers separately in steps. Previously, most tools didn't do this correctly, which was causing the compiler to be built twice (e.g., `x test cargo --stage 1` would compile the stage 2 compiler before, but now it only compiles the stage 1 compiler).

I also tried to document how we should write `ToolRustc` steps as they are quite different and require more attention than other tools.

Next goal is to stabilize how stages are handled for the rustc itself. Currently, `x build --stage 1` builds the stage 1 compiler which is fine, but `x build compiler --stage 1` builds stage 2 compiler.

~~for now, r? ghost~~
  • Loading branch information
@bors
bors committed Feb 23, 2025
2 parents bca5f37 + 76063a6 commit bb2cc59
Show file tree
Hide file tree
Showing 12 changed files with 340 additions and 263 deletions.
2 changes: 2 additions & 0 deletions src/bootstrap/defaults/config.tools.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ incremental = true
download-rustc = "if-unchanged"

[build]
# cargo and clippy tests don't pass on stage 1
test-stage = 2
# Document with the in-tree rustdoc by default, since `download-rustc` makes it quick to compile.
doc-stage = 2
# Contributors working on tools will probably expect compiler docs to be generated, so they can figure out how to use the API.
Expand Down
26 changes: 13 additions & 13 deletions src/bootstrap/src/core/build_steps/compile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1983,13 +1983,14 @@ impl Step for Assemble {
let maybe_install_llvm_bitcode_linker = |compiler| {
if builder.config.llvm_bitcode_linker_enabled {
trace!("llvm-bitcode-linker enabled, installing");
let src_path = builder.ensure(crate::core::build_steps::tool::LlvmBitcodeLinker {
compiler,
target: target_compiler.host,
extra_features: vec![],
});
let llvm_bitcode_linker =
builder.ensure(crate::core::build_steps::tool::LlvmBitcodeLinker {
compiler,
target: target_compiler.host,
extra_features: vec![],
});
let tool_exe = exe("llvm-bitcode-linker", target_compiler.host);
builder.copy_link(&src_path, &libdir_bin.join(tool_exe));
builder.copy_link(&llvm_bitcode_linker.tool_path, &libdir_bin.join(tool_exe));
}
};

Expand Down Expand Up @@ -2181,14 +2182,13 @@ impl Step for Assemble {
// logic to create the final binary. This is used by the
// `wasm32-wasip2` target of Rust.
if builder.tool_enabled("wasm-component-ld") {
let wasm_component_ld_exe =
builder.ensure(crate::core::build_steps::tool::WasmComponentLd {
compiler: build_compiler,
target: target_compiler.host,
});
let wasm_component = builder.ensure(crate::core::build_steps::tool::WasmComponentLd {
compiler: build_compiler,
target: target_compiler.host,
});
builder.copy_link(
&wasm_component_ld_exe,
&libdir_bin.join(wasm_component_ld_exe.file_name().unwrap()),
&wasm_component.tool_path,
&libdir_bin.join(wasm_component.tool_path.file_name().unwrap()),
);
}

Expand Down
22 changes: 11 additions & 11 deletions src/bootstrap/src/core/build_steps/dist.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ impl Step for Rustc {
},
builder.kind,
) {
builder.install(&ra_proc_macro_srv, &image.join("libexec"), 0o755);
builder.install(&ra_proc_macro_srv.tool_path, &image.join("libexec"), 0o755);
}

let libdir_relative = builder.libdir_relative(compiler);
Expand Down Expand Up @@ -1145,7 +1145,7 @@ impl Step for Cargo {
let mut tarball = Tarball::new(builder, "cargo", &target.triple);
tarball.set_overlay(OverlayKind::Cargo);

tarball.add_file(cargo, "bin", 0o755);
tarball.add_file(cargo.tool_path, "bin", 0o755);
tarball.add_file(etc.join("_cargo"), "share/zsh/site-functions", 0o644);
tarball.add_renamed_file(etc.join("cargo.bashcomp.sh"), "etc/bash_completion.d", "cargo");
tarball.add_dir(etc.join("man"), "share/man/man1");
Expand Down Expand Up @@ -1191,7 +1191,7 @@ impl Step for Rls {
let mut tarball = Tarball::new(builder, "rls", &target.triple);
tarball.set_overlay(OverlayKind::Rls);
tarball.is_preview(true);
tarball.add_file(rls, "bin", 0o755);
tarball.add_file(rls.tool_path, "bin", 0o755);
tarball.add_legal_and_readme_to("share/doc/rls");
Some(tarball.generate())
}
Expand Down Expand Up @@ -1233,7 +1233,7 @@ impl Step for RustAnalyzer {
let mut tarball = Tarball::new(builder, "rust-analyzer", &target.triple);
tarball.set_overlay(OverlayKind::RustAnalyzer);
tarball.is_preview(true);
tarball.add_file(rust_analyzer, "bin", 0o755);
tarball.add_file(rust_analyzer.tool_path, "bin", 0o755);
tarball.add_legal_and_readme_to("share/doc/rust-analyzer");
Some(tarball.generate())
}
Expand Down Expand Up @@ -1279,8 +1279,8 @@ impl Step for Clippy {
let mut tarball = Tarball::new(builder, "clippy", &target.triple);
tarball.set_overlay(OverlayKind::Clippy);
tarball.is_preview(true);
tarball.add_file(clippy, "bin", 0o755);
tarball.add_file(cargoclippy, "bin", 0o755);
tarball.add_file(clippy.tool_path, "bin", 0o755);
tarball.add_file(cargoclippy.tool_path, "bin", 0o755);
tarball.add_legal_and_readme_to("share/doc/clippy");
Some(tarball.generate())
}
Expand Down Expand Up @@ -1329,8 +1329,8 @@ impl Step for Miri {
let mut tarball = Tarball::new(builder, "miri", &target.triple);
tarball.set_overlay(OverlayKind::Miri);
tarball.is_preview(true);
tarball.add_file(miri, "bin", 0o755);
tarball.add_file(cargomiri, "bin", 0o755);
tarball.add_file(miri.tool_path, "bin", 0o755);
tarball.add_file(cargomiri.tool_path, "bin", 0o755);
tarball.add_legal_and_readme_to("share/doc/miri");
Some(tarball.generate())
}
Expand Down Expand Up @@ -1460,8 +1460,8 @@ impl Step for Rustfmt {
let mut tarball = Tarball::new(builder, "rustfmt", &target.triple);
tarball.set_overlay(OverlayKind::Rustfmt);
tarball.is_preview(true);
tarball.add_file(rustfmt, "bin", 0o755);
tarball.add_file(cargofmt, "bin", 0o755);
tarball.add_file(rustfmt.tool_path, "bin", 0o755);
tarball.add_file(cargofmt.tool_path, "bin", 0o755);
tarball.add_legal_and_readme_to("share/doc/rustfmt");
Some(tarball.generate())
}
Expand Down Expand Up @@ -2283,7 +2283,7 @@ impl Step for LlvmBitcodeLinker {
tarball.set_overlay(OverlayKind::LlvmBitcodeLinker);
tarball.is_preview(true);

tarball.add_file(llbc_linker, self_contained_bin_dir, 0o755);
tarball.add_file(llbc_linker.tool_path, self_contained_bin_dir, 0o755);

Some(tarball.generate())
}
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/perf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ Consider setting `rust.debuginfo-level = 1` in `config.toml`."#);
let results_dir = rustc_perf_dir.join("results");
builder.create_dir(&results_dir);

let mut cmd = command(collector);
let mut cmd = command(collector.tool_path);

// We need to set the working directory to `src/tools/rustc-perf`, so that it can find the directory
// with compile-time benchmarks.
Expand Down
6 changes: 1 addition & 5 deletions src/bootstrap/src/core/build_steps/run.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,7 @@ impl Step for Miri {

// This compiler runs on the host, we'll just use it for the target.
let target_compiler = builder.compiler(stage, host);
// Similar to `compile::Assemble`, build with the previous stage's compiler. Otherwise
// we'd have stageN/bin/rustc and stageN/bin/rustdoc be effectively different stage
// compilers, which isn't what we want. Rustdoc should be linked in the same way as the
// rustc compiler it's paired with, so it must be built with the previous stage compiler.
let host_compiler = builder.compiler(stage - 1, host);
let host_compiler = tool::get_tool_rustc_compiler(builder, target_compiler);

// Get a target sysroot for Miri.
let miri_sysroot = test::Miri::build_miri_sysroot(builder, target_compiler, target);
Expand Down
66 changes: 35 additions & 31 deletions src/bootstrap/src/core/build_steps/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ impl Step for Cargotest {

let _time = helpers::timeit(builder);
let mut cmd = builder.tool_cmd(Tool::CargoTest);
cmd.arg(&cargo)
cmd.arg(&cargo.tool_path)
.arg(&out_dir)
.args(builder.config.test_args())
.env("RUSTC", builder.rustc(compiler))
Expand Down Expand Up @@ -298,9 +298,16 @@ impl Step for Cargo {

/// Runs `cargo test` for `cargo` packaged with Rust.
fn run(self, builder: &Builder<'_>) {
if self.stage < 2 {
eprintln!("WARNING: cargo tests on stage {} may not behave well.", self.stage);
eprintln!("HELP: consider using stage 2");
}

let compiler = builder.compiler(self.stage, self.host);

builder.ensure(tool::Cargo { compiler, target: self.host });
let cargo = builder.ensure(tool::Cargo { compiler, target: self.host });
let compiler = cargo.build_compiler;

let cargo = tool::prepare_tool_cargo(
builder,
compiler,
Expand Down Expand Up @@ -367,6 +374,7 @@ impl Step for RustAnalyzer {
let stage = self.stage;
let host = self.host;
let compiler = builder.compiler(stage, host);
let compiler = tool::get_tool_rustc_compiler(builder, compiler);

// We don't need to build the whole Rust Analyzer for the proc-macro-srv test suite,
// but we do need the standard library to be present.
Expand Down Expand Up @@ -427,7 +435,8 @@ impl Step for Rustfmt {
let host = self.host;
let compiler = builder.compiler(stage, host);

builder.ensure(tool::Rustfmt { compiler, target: self.host });
let tool_result = builder.ensure(tool::Rustfmt { compiler, target: self.host });
let compiler = tool_result.build_compiler;

let mut cargo = tool::prepare_tool_cargo(
builder,
Expand Down Expand Up @@ -522,16 +531,11 @@ impl Step for Miri {

// This compiler runs on the host, we'll just use it for the target.
let target_compiler = builder.compiler(stage, host);
// Similar to `compile::Assemble`, build with the previous stage's compiler. Otherwise
// we'd have stageN/bin/rustc and stageN/bin/rustdoc be effectively different stage
// compilers, which isn't what we want. Rustdoc should be linked in the same way as the
// rustc compiler it's paired with, so it must be built with the previous stage compiler.
let host_compiler = builder.compiler(stage - 1, host);

// Build our tools.
let miri = builder.ensure(tool::Miri { compiler: host_compiler, target: host });
let miri = builder.ensure(tool::Miri { compiler: target_compiler, target: host });
// the ui tests also assume cargo-miri has been built
builder.ensure(tool::CargoMiri { compiler: host_compiler, target: host });
builder.ensure(tool::CargoMiri { compiler: target_compiler, target: host });

// We also need sysroots, for Miri and for the host (the latter for build scripts).
// This is for the tests so everything is done with the target compiler.
Expand All @@ -542,7 +546,8 @@ impl Step for Miri {
// Miri has its own "target dir" for ui test dependencies. Make sure it gets cleared when
// the sysroot gets rebuilt, to avoid "found possibly newer version of crate `std`" errors.
if !builder.config.dry_run() {
let ui_test_dep_dir = builder.stage_out(host_compiler, Mode::ToolStd).join("miri_ui");
let ui_test_dep_dir =
builder.stage_out(miri.build_compiler, Mode::ToolStd).join("miri_ui");
// The mtime of `miri_sysroot` changes when the sysroot gets rebuilt (also see
// <https://github.com/RalfJung/rustc-build-sysroot/commit/10ebcf60b80fe2c3dc765af0ff19fdc0da4b7466>).
// We can hence use that directly as a signal to clear the ui test dir.
Expand All @@ -553,7 +558,7 @@ impl Step for Miri {
// This is with the Miri crate, so it uses the host compiler.
let mut cargo = tool::prepare_tool_cargo(
builder,
host_compiler,
miri.build_compiler,
Mode::ToolRustc,
host,
Kind::Test,
Expand All @@ -571,7 +576,7 @@ impl Step for Miri {
// miri tests need to know about the stage sysroot
cargo.env("MIRI_SYSROOT", &miri_sysroot);
cargo.env("MIRI_HOST_SYSROOT", &host_sysroot);
cargo.env("MIRI", &miri);
cargo.env("MIRI", &miri.tool_path);

// Set the target.
cargo.env("MIRI_TEST_TARGET", target.rustc_target_arg());
Expand Down Expand Up @@ -743,7 +748,13 @@ impl Step for Clippy {
let host = self.host;
let compiler = builder.compiler(stage, host);

builder.ensure(tool::Clippy { compiler, target: self.host });
if stage < 2 {
eprintln!("WARNING: clippy tests on stage {stage} may not behave well.");
eprintln!("HELP: consider using stage 2");
}

let tool_result = builder.ensure(tool::Clippy { compiler, target: self.host });
let compiler = tool_result.build_compiler;
let mut cargo = tool::prepare_tool_cargo(
builder,
compiler,
Expand Down Expand Up @@ -1728,18 +1739,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
// If we're using `--stage 0`, we should provide the bootstrap cargo.
builder.initial_cargo.clone()
} else {
// We need to properly build cargo using the suitable stage compiler.

let compiler = builder.download_rustc().then_some(compiler).unwrap_or_else(||
// HACK: currently tool stages are off-by-one compared to compiler stages, i.e. if
// you give `tool::Cargo` a stage 1 rustc, it will cause stage 2 rustc to be built
// and produce a cargo built with stage 2 rustc. To fix this, we need to chop off
// the compiler stage by 1 to align with expected `./x test run-make --stage N`
// behavior, i.e. we need to pass `N - 1` compiler stage to cargo. See also Miri
// which does a similar hack.
builder.compiler(builder.top_stage - 1, compiler.host));

builder.ensure(tool::Cargo { compiler, target: compiler.host })
builder.ensure(tool::Cargo { compiler, target: compiler.host }).tool_path
};

cmd.arg("--cargo-path").arg(cargo_path);
Expand All @@ -1760,9 +1760,10 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
// Use the beta compiler for jsondocck
let json_compiler = compiler.with_stage(0);
cmd.arg("--jsondocck-path")
.arg(builder.ensure(tool::JsonDocCk { compiler: json_compiler, target }));
cmd.arg("--jsondoclint-path")
.arg(builder.ensure(tool::JsonDocLint { compiler: json_compiler, target }));
.arg(builder.ensure(tool::JsonDocCk { compiler: json_compiler, target }).tool_path);
cmd.arg("--jsondoclint-path").arg(
builder.ensure(tool::JsonDocLint { compiler: json_compiler, target }).tool_path,
);
}

if matches!(mode, "coverage-map" | "coverage-run") {
Expand Down Expand Up @@ -2999,12 +3000,15 @@ impl Step for RemoteCopyLibs {

builder.info(&format!("REMOTE copy libs to emulator ({target})"));

let server = builder.ensure(tool::RemoteTestServer { compiler, target });
let remote_test_server = builder.ensure(tool::RemoteTestServer { compiler, target });

// Spawn the emulator and wait for it to come online
let tool = builder.tool_exe(Tool::RemoteTestClient);
let mut cmd = command(&tool);
cmd.arg("spawn-emulator").arg(target.triple).arg(&server).arg(builder.tempdir());
cmd.arg("spawn-emulator")
.arg(target.triple)
.arg(&remote_test_server.tool_path)
.arg(builder.tempdir());
if let Some(rootfs) = builder.qemu_rootfs(target) {
cmd.arg(rootfs);
}
Expand Down
Loading

0 comments on commit bb2cc59

Please sign in to comment.