From 23994e1d760e0c163ead0821262bd40ff90eb24a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 28 Dec 2023 12:43:04 +0100 Subject: [PATCH 1/4] Remove `--enable-missing-tools` usage in CI --- src/ci/run.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/ci/run.sh b/src/ci/run.sh index 5700172fd3ec4..e48fac1a08798 100755 --- a/src/ci/run.sh +++ b/src/ci/run.sh @@ -153,10 +153,6 @@ else fi fi -if [ "$RUST_RELEASE_CHANNEL" = "nightly" ] || [ "$DIST_REQUIRE_ALL_TOOLS" = "" ]; then - RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-missing-tools" -fi - # Unless we're using an older version of LLVM, check that all LLVM components # used by tests are available. if [ "$IS_NOT_LATEST_LLVM" = "" ]; then From 8763f7ae7d43e3c8d5b719cc0fd35296863f343f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 28 Dec 2023 12:43:37 +0100 Subject: [PATCH 2/4] Remove `--enable-missing-tools` from `configure.py` --- src/bootstrap/configure.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bootstrap/configure.py b/src/bootstrap/configure.py index 544a42d9ada1a..d34c19a47e3fb 100755 --- a/src/bootstrap/configure.py +++ b/src/bootstrap/configure.py @@ -55,7 +55,6 @@ def v(*args): o("full-tools", None, "enable all tools") o("lld", "rust.lld", "build lld") o("clang", "llvm.clang", "build clang") -o("missing-tools", "dist.missing-tools", "allow failures when building tools") o("use-libcxx", "llvm.use-libcxx", "build LLVM with libc++") o("control-flow-guard", "rust.control-flow-guard", "Enable Control Flow Guard") o("patch-binaries-for-nix", "build.patch-binaries-for-nix", "whether patch binaries for usage with Nix toolchains") From 0e7f9ec2cad5e9d75e0a7e140ca209f6d874d1ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 28 Dec 2023 20:23:12 +0100 Subject: [PATCH 3/4] Add change tracker entry --- src/bootstrap/src/core/config/config.rs | 2 +- src/bootstrap/src/utils/change_tracker.rs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index f1e1b89d9ba71..a981608f297fd 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -305,7 +305,7 @@ pub struct Config { pub save_toolstates: Option, pub print_step_timings: bool, pub print_step_rusage: bool, - pub missing_tools: bool, + pub missing_tools: bool, // FIXME: Deprecated field. Remove it at 2024. // Fallback musl-root for all targets pub musl_root: Option, diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 1eadc036b5e6e..25efa5079c873 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -101,4 +101,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ severity: ChangeSeverity::Warning, summary: "rust-analyzer-proc-macro-srv is no longer enabled by default. To build it, you must either enable it in the configuration or explicitly invoke it with x.py.", }, + ChangeInfo { + change_id: 119373, + severity: ChangeSeverity::Info, + summary: "The dist.missing-tools config option was deprecated, as it was unused. If you are using it, remove it from your config, it will be removed soon.", + }, ]; From fdeb8c502797af5e21cab4de1f4001deb2cd8901 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 28 Dec 2023 16:40:50 +0100 Subject: [PATCH 4/4] Remove `is_optional_tool` from `ToolBuild` --- src/bootstrap/src/core/build_steps/dist.rs | 36 ++---- src/bootstrap/src/core/build_steps/run.rs | 5 +- src/bootstrap/src/core/build_steps/test.rs | 41 +++---- src/bootstrap/src/core/build_steps/tool.rs | 136 +++++++++------------ src/bootstrap/src/core/builder.rs | 2 +- 5 files changed, 92 insertions(+), 128 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index 98e267713daf7..55408660cc4ae 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -1110,9 +1110,7 @@ impl Step for Rls { let compiler = self.compiler; let target = self.target; - let rls = builder - .ensure(tool::Rls { compiler, target, extra_features: Vec::new() }) - .expect("rls expected to build"); + let rls = builder.ensure(tool::Rls { compiler, target, extra_features: Vec::new() }); let mut tarball = Tarball::new(builder, "rls", &target.triple); tarball.set_overlay(OverlayKind::RLS); @@ -1154,9 +1152,7 @@ impl Step for RustAnalyzer { let compiler = self.compiler; let target = self.target; - let rust_analyzer = builder - .ensure(tool::RustAnalyzer { compiler, target }) - .expect("rust-analyzer always builds"); + let rust_analyzer = builder.ensure(tool::RustAnalyzer { compiler, target }); let mut tarball = Tarball::new(builder, "rust-analyzer", &target.triple); tarball.set_overlay(OverlayKind::RustAnalyzer); @@ -1201,12 +1197,9 @@ impl Step for Clippy { // Prepare the image directory // We expect clippy to build, because we've exited this step above if tool // state for clippy isn't testing. - let clippy = builder - .ensure(tool::Clippy { compiler, target, extra_features: Vec::new() }) - .expect("clippy expected to build - essential tool"); - let cargoclippy = builder - .ensure(tool::CargoClippy { compiler, target, extra_features: Vec::new() }) - .expect("clippy expected to build - essential tool"); + let clippy = builder.ensure(tool::Clippy { compiler, target, extra_features: Vec::new() }); + let cargoclippy = + builder.ensure(tool::CargoClippy { compiler, target, extra_features: Vec::new() }); let mut tarball = Tarball::new(builder, "clippy", &target.triple); tarball.set_overlay(OverlayKind::Clippy); @@ -1255,9 +1248,9 @@ impl Step for Miri { let compiler = self.compiler; let target = self.target; - let miri = builder.ensure(tool::Miri { compiler, target, extra_features: Vec::new() })?; + let miri = builder.ensure(tool::Miri { compiler, target, extra_features: Vec::new() }); let cargomiri = - builder.ensure(tool::CargoMiri { compiler, target, extra_features: Vec::new() })?; + builder.ensure(tool::CargoMiri { compiler, target, extra_features: Vec::new() }); let mut tarball = Tarball::new(builder, "miri", &target.triple); tarball.set_overlay(OverlayKind::Miri); @@ -1396,12 +1389,10 @@ impl Step for Rustfmt { let compiler = self.compiler; let target = self.target; - let rustfmt = builder - .ensure(tool::Rustfmt { compiler, target, extra_features: Vec::new() }) - .expect("rustfmt expected to build - essential tool"); - let cargofmt = builder - .ensure(tool::Cargofmt { compiler, target, extra_features: Vec::new() }) - .expect("cargo fmt expected to build - essential tool"); + let rustfmt = + builder.ensure(tool::Rustfmt { compiler, target, extra_features: Vec::new() }); + let cargofmt = + builder.ensure(tool::Cargofmt { compiler, target, extra_features: Vec::new() }); let mut tarball = Tarball::new(builder, "rustfmt", &target.triple); tarball.set_overlay(OverlayKind::Rustfmt); tarball.is_preview(true); @@ -1455,9 +1446,8 @@ impl Step for RustDemangler { return None; } - let rust_demangler = builder - .ensure(tool::RustDemangler { compiler, target, extra_features: Vec::new() }) - .expect("rust-demangler expected to build - in-tree tool"); + let rust_demangler = + builder.ensure(tool::RustDemangler { compiler, target, extra_features: Vec::new() }); // Prepare the image directory let mut tarball = Tarball::new(builder, "rust-demangler", &target.triple); diff --git a/src/bootstrap/src/core/build_steps/run.rs b/src/bootstrap/src/core/build_steps/run.rs index d1d6b7e869ecb..d9e0da14a70ba 100644 --- a/src/bootstrap/src/core/build_steps/run.rs +++ b/src/bootstrap/src/core/build_steps/run.rs @@ -148,9 +148,8 @@ impl Step for Miri { let target = self.target; let compiler = builder.compiler(stage, host); - let miri = builder - .ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() }) - .expect("in-tree tool"); + let miri = + builder.ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() }); let miri_sysroot = test::Miri::build_miri_sysroot(builder, compiler, &miri, target); // # Run miri. diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index d0f36f99342fe..92140b00da843 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -427,9 +427,7 @@ impl Step for Rustfmt { let host = self.host; let compiler = builder.compiler(stage, host); - builder - .ensure(tool::Rustfmt { compiler, target: self.host, extra_features: Vec::new() }) - .expect("in-tree tool"); + builder.ensure(tool::Rustfmt { compiler, target: self.host, extra_features: Vec::new() }); let mut cargo = tool::prepare_tool_cargo( builder, @@ -476,9 +474,11 @@ impl Step for RustDemangler { let host = self.host; let compiler = builder.compiler(stage, host); - let rust_demangler = builder - .ensure(tool::RustDemangler { compiler, target: self.host, extra_features: Vec::new() }) - .expect("in-tree tool"); + let rust_demangler = builder.ensure(tool::RustDemangler { + compiler, + target: self.host, + extra_features: Vec::new(), + }); let mut cargo = tool::prepare_tool_cargo( builder, compiler, @@ -609,12 +609,13 @@ impl Step for Miri { // Except if we are at stage 2, the bootstrap loop is complete and we can stick with our current stage. let compiler_std = builder.compiler(if stage < 2 { stage + 1 } else { stage }, host); - let miri = builder - .ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() }) - .expect("in-tree tool"); - let _cargo_miri = builder - .ensure(tool::CargoMiri { compiler, target: self.host, extra_features: Vec::new() }) - .expect("in-tree tool"); + let miri = + builder.ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() }); + let _cargo_miri = builder.ensure(tool::CargoMiri { + compiler, + target: self.host, + extra_features: Vec::new(), + }); // The stdlib we need might be at a different stage. And just asking for the // sysroot does not seem to populate it, so we do that first. builder.ensure(compile::Std::new(compiler_std, host)); @@ -788,9 +789,7 @@ impl Step for Clippy { let host = self.host; let compiler = builder.compiler(stage, host); - builder - .ensure(tool::Clippy { compiler, target: self.host, extra_features: Vec::new() }) - .expect("in-tree tool"); + builder.ensure(tool::Clippy { compiler, target: self.host, extra_features: Vec::new() }); let mut cargo = tool::prepare_tool_cargo( builder, compiler, @@ -1668,13 +1667,11 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the if mode == "coverage-run" { // The demangler doesn't need the current compiler, so we can avoid // unnecessary rebuilds by using the bootstrap compiler instead. - let rust_demangler = builder - .ensure(tool::RustDemangler { - compiler: compiler.with_stage(0), - target: compiler.host, - extra_features: Vec::new(), - }) - .expect("in-tree tool"); + let rust_demangler = builder.ensure(tool::RustDemangler { + compiler: compiler.with_stage(0), + target: compiler.host, + extra_features: Vec::new(), + }); cmd.arg("--rust-demangler-path").arg(rust_demangler); } diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 8e3941dbedaca..2cd42dd8081ca 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -27,7 +27,6 @@ struct ToolBuild { tool: &'static str, path: &'static str, mode: Mode, - is_optional_tool: bool, source_type: SourceType, extra_features: Vec, /// Nightly-only features that are allowed (comma-separated list). @@ -60,7 +59,7 @@ impl Builder<'_> { } impl Step for ToolBuild { - type Output = Option; + type Output = PathBuf; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { run.never() @@ -70,12 +69,11 @@ impl Step for ToolBuild { /// /// This will build the specified tool with the specified `host` compiler in /// `stage` into the normal cargo output directory. - fn run(self, builder: &Builder<'_>) -> Option { + fn run(self, builder: &Builder<'_>) -> PathBuf { let compiler = self.compiler; let target = self.target; let mut tool = self.tool; let path = self.path; - let is_optional_tool = self.is_optional_tool; match self.mode { Mode::ToolRustc => { @@ -109,20 +107,16 @@ impl Step for ToolBuild { ); let mut cargo = Command::from(cargo); - // we check this in `is_optional_tool` in a second - let is_expected = builder.run_cmd(BootstrapCommand::from(&mut cargo).allow_failure()); + // we check this below + let build_success = builder.run_cmd(BootstrapCommand::from(&mut cargo).allow_failure()); builder.save_toolstate( tool, - if is_expected { ToolState::TestFail } else { ToolState::BuildFail }, + if build_success { ToolState::TestFail } else { ToolState::BuildFail }, ); - if !is_expected { - if !is_optional_tool { - crate::exit!(1); - } else { - None - } + if !build_success { + crate::exit!(1); } else { // HACK(#82501): on Windows, the tools directory gets added to PATH when running tests, and // compiletest confuses HTML tidy with the in-tree tidy. Name the in-tree tidy something @@ -133,7 +127,7 @@ impl Step for ToolBuild { let cargo_out = builder.cargo_out(compiler, self.mode, target).join(exe(tool, target)); let bin = builder.tools_dir(compiler).join(exe(tool, target)); builder.copy(&cargo_out, &bin); - Some(bin) + bin } } } @@ -278,7 +272,6 @@ macro_rules! bootstrap_tool { Mode::ToolBootstrap }, path: $path, - is_optional_tool: false, source_type: if false $(|| $external)* { SourceType::Submodule } else { @@ -286,7 +279,7 @@ macro_rules! bootstrap_tool { }, extra_features: vec![], allow_features: concat!($($allow_features)*), - }).expect("expected to build -- essential tool") + }) } } )+ @@ -361,19 +354,16 @@ impl Step for ErrorIndex { } fn run(self, builder: &Builder<'_>) -> PathBuf { - builder - .ensure(ToolBuild { - compiler: self.compiler, - target: self.compiler.host, - tool: "error_index_generator", - mode: Mode::ToolRustc, - path: "src/tools/error_index_generator", - is_optional_tool: false, - source_type: SourceType::InTree, - extra_features: Vec::new(), - allow_features: "", - }) - .expect("expected to build -- essential tool") + builder.ensure(ToolBuild { + compiler: self.compiler, + target: self.compiler.host, + tool: "error_index_generator", + mode: Mode::ToolRustc, + path: "src/tools/error_index_generator", + source_type: SourceType::InTree, + extra_features: Vec::new(), + allow_features: "", + }) } } @@ -398,19 +388,16 @@ impl Step for RemoteTestServer { } fn run(self, builder: &Builder<'_>) -> PathBuf { - builder - .ensure(ToolBuild { - compiler: self.compiler, - target: self.target, - tool: "remote-test-server", - mode: Mode::ToolStd, - path: "src/tools/remote-test-server", - is_optional_tool: false, - source_type: SourceType::InTree, - extra_features: Vec::new(), - allow_features: "", - }) - .expect("expected to build -- essential tool") + builder.ensure(ToolBuild { + compiler: self.compiler, + target: self.target, + tool: "remote-test-server", + mode: Mode::ToolStd, + path: "src/tools/remote-test-server", + source_type: SourceType::InTree, + extra_features: Vec::new(), + allow_features: "", + }) } } @@ -557,19 +544,16 @@ impl Step for Cargo { } fn run(self, builder: &Builder<'_>) -> PathBuf { - let cargo_bin_path = builder - .ensure(ToolBuild { - compiler: self.compiler, - target: self.target, - tool: "cargo", - mode: Mode::ToolRustc, - path: "src/tools/cargo", - is_optional_tool: false, - source_type: SourceType::Submodule, - extra_features: Vec::new(), - allow_features: "", - }) - .expect("expected to build -- essential tool"); + let cargo_bin_path = builder.ensure(ToolBuild { + compiler: self.compiler, + target: self.target, + tool: "cargo", + mode: Mode::ToolRustc, + path: "src/tools/cargo", + source_type: SourceType::Submodule, + extra_features: Vec::new(), + allow_features: "", + }); cargo_bin_path } } @@ -588,19 +572,16 @@ impl Step for LldWrapper { } fn run(self, builder: &Builder<'_>) -> PathBuf { - let src_exe = builder - .ensure(ToolBuild { - compiler: self.compiler, - target: self.target, - tool: "lld-wrapper", - mode: Mode::ToolStd, - path: "src/tools/lld-wrapper", - is_optional_tool: false, - source_type: SourceType::InTree, - extra_features: Vec::new(), - allow_features: "", - }) - .expect("expected to build -- essential tool"); + let src_exe = builder.ensure(ToolBuild { + compiler: self.compiler, + target: self.target, + tool: "lld-wrapper", + mode: Mode::ToolStd, + path: "src/tools/lld-wrapper", + source_type: SourceType::InTree, + extra_features: Vec::new(), + allow_features: "", + }); src_exe } @@ -617,7 +598,7 @@ impl RustAnalyzer { } impl Step for RustAnalyzer { - type Output = Option; + type Output = PathBuf; const DEFAULT: bool = true; const ONLY_HOSTS: bool = true; @@ -640,7 +621,7 @@ impl Step for RustAnalyzer { }); } - fn run(self, builder: &Builder<'_>) -> Option { + fn run(self, builder: &Builder<'_>) -> PathBuf { builder.ensure(ToolBuild { compiler: self.compiler, target: self.target, @@ -648,7 +629,6 @@ impl Step for RustAnalyzer { mode: Mode::ToolRustc, path: "src/tools/rust-analyzer", extra_features: vec!["rust-analyzer/in-rust-tree".to_owned()], - is_optional_tool: false, source_type: SourceType::InTree, allow_features: RustAnalyzer::ALLOW_FEATURES, }) @@ -696,10 +676,9 @@ impl Step for RustAnalyzerProcMacroSrv { mode: Mode::ToolStd, path: "src/tools/rust-analyzer/crates/proc-macro-srv-cli", extra_features: vec!["sysroot-abi".to_owned()], - is_optional_tool: false, source_type: SourceType::InTree, allow_features: RustAnalyzer::ALLOW_FEATURES, - })?; + }); // Copy `rust-analyzer-proc-macro-srv` to `/libexec/` // so that r-a can use it. @@ -730,7 +709,7 @@ macro_rules! tool_extended { } impl Step for $name { - type Output = Option; + type Output = PathBuf; const DEFAULT: bool = true; // Overwritten below const ONLY_HOSTS: bool = true; @@ -761,7 +740,7 @@ macro_rules! tool_extended { } #[allow(unused_mut)] - fn run(mut $sel, $builder: &Builder<'_>) -> Option { + fn run(mut $sel, $builder: &Builder<'_>) -> PathBuf { let tool = $builder.ensure(ToolBuild { compiler: $sel.compiler, target: $sel.target, @@ -769,10 +748,9 @@ macro_rules! tool_extended { mode: if false $(|| $tool_std)? { Mode::ToolStd } else { Mode::ToolRustc }, path: $path, extra_features: $sel.extra_features, - is_optional_tool: true, source_type: SourceType::InTree, allow_features: concat!($($allow_features)*), - })?; + }); if (false $(|| !$add_bins_to_sysroot.is_empty())?) && $sel.compiler.stage > 0 { let bindir = $builder.sysroot($sel.compiler).join("bin"); @@ -789,9 +767,9 @@ macro_rules! tool_extended { })? let tool = bindir.join(exe($tool_name, $sel.compiler.host)); - Some(tool) + tool } else { - Some(tool) + tool } } } diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 753b41abaf489..e85753a351232 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -1198,7 +1198,7 @@ impl<'a> Builder<'a> { let mut dylib_path = helpers::dylib_path(); dylib_path.insert(0, self.sysroot(run_compiler).join("lib")); - let mut cmd = Command::new(cargo_clippy.unwrap()); + let mut cmd = Command::new(cargo_clippy); cmd.env(helpers::dylib_path_var(), env::join_paths(&dylib_path).unwrap()); cmd.env("PATH", path); cmd