From 37984bbde11e6bcd0a009d2bbdbe7a73f9605b05 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sat, 3 Aug 2024 08:58:53 +0300 Subject: [PATCH 1/3] unify path syncing logic for vendor and dist Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/dist.rs | 34 ++++++-------------- src/bootstrap/src/core/build_steps/vendor.rs | 32 ++++++++++++------ 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index c14709ffb63..cd7d602e859 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -19,6 +19,7 @@ use crate::core::build_steps::doc::DocumentationFormat; use crate::core::build_steps::tool::{self, Tool}; +use crate::core::build_steps::vendor::default_paths_to_vendor; use crate::core::build_steps::{compile, llvm}; use crate::core::builder::{Builder, Kind, RunConfig, ShouldRun, Step}; use crate::core::config::TargetSelection; @@ -1016,35 +1017,18 @@ fn run(self, builder: &Builder<'_>) -> GeneratedTarball { if builder.rust_info().is_managed_git_subrepository() || builder.rust_info().is_from_tarball() { - // FIXME: This code looks _very_ similar to what we have in `src/core/build_steps/vendor.rs` - // perhaps it should be removed in favor of making `dist` perform the `vendor` step? - builder.require_and_update_all_submodules(); // Vendor all Cargo dependencies let mut cmd = command(&builder.initial_cargo); - cmd.arg("vendor") - .arg("--versioned-dirs") - .arg("--sync") - .arg(builder.src.join("./src/tools/cargo/Cargo.toml")) - .arg("--sync") - .arg(builder.src.join("./src/tools/rust-analyzer/Cargo.toml")) - .arg("--sync") - .arg(builder.src.join("./compiler/rustc_codegen_cranelift/Cargo.toml")) - .arg("--sync") - .arg(builder.src.join("./compiler/rustc_codegen_gcc/Cargo.toml")) - .arg("--sync") - .arg(builder.src.join("./library/Cargo.toml")) - .arg("--sync") - .arg(builder.src.join("./src/bootstrap/Cargo.toml")) - .arg("--sync") - .arg(builder.src.join("./src/tools/opt-dist/Cargo.toml")) - .arg("--sync") - .arg(builder.src.join("./src/tools/rustc-perf/Cargo.toml")) - .arg("--sync") - .arg(builder.src.join("./src/tools/rustbook/Cargo.toml")) - // Will read the libstd Cargo.toml - // which uses the unstable `public-dependency` feature. + cmd.arg("vendor").arg("--versioned-dirs"); + + for p in default_paths_to_vendor(builder) { + cmd.arg("--sync").arg(p); + } + + cmd + // Will read the libstd Cargo.toml which uses the unstable `public-dependency` feature. .env("RUSTC_BOOTSTRAP", "1") .current_dir(plain_dst_src); diff --git a/src/bootstrap/src/core/build_steps/vendor.rs b/src/bootstrap/src/core/build_steps/vendor.rs index 33768465225..82a6b4d4f28 100644 --- a/src/bootstrap/src/core/build_steps/vendor.rs +++ b/src/bootstrap/src/core/build_steps/vendor.rs @@ -4,6 +4,26 @@ use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; use crate::utils::exec::command; +/// List of default paths used for vendoring for `x vendor` and dist tarballs. +pub fn default_paths_to_vendor(builder: &Builder<'_>) -> Vec { + let mut paths = vec![]; + for p in [ + "src/tools/cargo/Cargo.toml", + "src/tools/rust-analyzer/Cargo.toml", + "compiler/rustc_codegen_cranelift/Cargo.toml", + "compiler/rustc_codegen_gcc/Cargo.toml", + "library/Cargo.toml", + "src/bootstrap/Cargo.toml", + "src/tools/rustbook/Cargo.toml", + "src/tools/rustc-perf/Cargo.toml", + "src/tools/opt-dist/Cargo.toml", + ] { + paths.push(builder.src.join(p)); + } + + paths +} + #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub(crate) struct Vendor { sync_args: Vec, @@ -42,16 +62,8 @@ fn run(self, builder: &Builder<'_>) -> Self::Output { } // Sync these paths by default. - for p in [ - "src/tools/cargo/Cargo.toml", - "src/tools/rust-analyzer/Cargo.toml", - "compiler/rustc_codegen_cranelift/Cargo.toml", - "compiler/rustc_codegen_gcc/Cargo.toml", - "library/Cargo.toml", - "src/bootstrap/Cargo.toml", - "src/tools/rustbook/Cargo.toml", - ] { - cmd.arg("--sync").arg(builder.src.join(p)); + for p in default_paths_to_vendor(builder) { + cmd.arg("--sync").arg(p); } // Also sync explicitly requested paths. From bc0bc91e109fb21243f6bf15747c6fe2158d6a4b Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sat, 3 Aug 2024 09:00:16 +0300 Subject: [PATCH 2/3] remove redundant FIXMEs Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/tool.rs | 7 +------ src/bootstrap/src/core/builder.rs | 14 +++++--------- src/bootstrap/src/core/download.rs | 2 -- 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 4d573b107b5..ff8ca2ad74a 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -1091,18 +1091,13 @@ fn run(mut $sel, $builder: &Builder<'_>) -> PathBuf { } } -// NOTE: tools need to be also added to `Builder::get_step_descriptions` in `builder.rs` -// to make `./x.py build ` work. tool_extended!((self, builder), Cargofmt, "src/tools/rustfmt", "cargo-fmt", stable=true; CargoClippy, "src/tools/clippy", "cargo-clippy", stable=true; Clippy, "src/tools/clippy", "clippy-driver", stable=true, add_bins_to_sysroot = ["clippy-driver", "cargo-clippy"]; Miri, "src/tools/miri", "miri", stable=false, add_bins_to_sysroot = ["miri"]; CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri", stable=true, add_bins_to_sysroot = ["cargo-miri"]; - // FIXME: tool_std is not quite right, we shouldn't allow nightly features. - // But `builder.cargo` doesn't know how to handle ToolBootstrap in stages other than 0, - // and this is close enough for now. - Rls, "src/tools/rls", "rls", stable=true, tool_std=true; + Rls, "src/tools/rls", "rls", stable=true; Rustfmt, "src/tools/rustfmt", "rustfmt", stable=true, add_bins_to_sysroot = ["rustfmt", "cargo-fmt"]; ); diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 84c23c059e9..d6c6de8f80d 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -1449,15 +1449,11 @@ pub fn bare_cargo( assert_eq!(target, compiler.host); } - if self.config.rust_optimize.is_release() { - // FIXME: cargo bench/install do not accept `--release` - // and miri doesn't want it - match cmd_kind { - Kind::Bench | Kind::Install | Kind::Miri | Kind::MiriSetup | Kind::MiriTest => {} - _ => { - cargo.arg("--release"); - } - } + if self.config.rust_optimize.is_release() && + // cargo bench/install do not accept `--release` and miri doesn't want it + !matches!(cmd_kind, Kind::Bench | Kind::Install | Kind::Miri | Kind::MiriSetup | Kind::MiriTest) + { + cargo.arg("--release"); } // Remove make-related flags to ensure Cargo can correctly set things up diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 4d1aea3cd95..25daeeb721e 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -616,8 +616,6 @@ fn download_component( }; // For the beta compiler, put special effort into ensuring the checksums are valid. - // FIXME: maybe we should do this for download-rustc as well? but it would be a pain to update - // this on each and every nightly ... let checksum = if should_verify { let error = format!( "src/stage0 doesn't contain a checksum for {url}. \ From 8d7c374b2ec4f6cfd99f9c8e7cd5b2624909fa21 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sat, 3 Aug 2024 09:10:00 +0300 Subject: [PATCH 3/3] improve rustup check in `x setup` Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/setup.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index 8cd9ba5fd14..f7b26712cab 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -247,9 +247,11 @@ fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) { impl Step for Link { type Output = (); const DEFAULT: bool = true; + fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { run.alias("link") } + fn make_run(run: RunConfig<'_>) { if run.builder.config.dry_run() { return; @@ -262,21 +264,30 @@ fn make_run(run: RunConfig<'_>) { } fn run(self, builder: &Builder<'_>) -> Self::Output { let config = &builder.config; + if config.dry_run() { return; } + + if !rustup_installed(builder) { + println!("WARNING: `rustup` is not installed; Skipping `stage1` toolchain linking."); + return; + } + let stage_path = ["build", config.build.rustc_target_arg(), "stage1"].join(MAIN_SEPARATOR_STR); - if !rustup_installed(builder) { - eprintln!("`rustup` is not installed; cannot link `stage1` toolchain"); - } else if stage_dir_exists(&stage_path[..]) && !config.dry_run() { + + if stage_dir_exists(&stage_path[..]) && !config.dry_run() { attempt_toolchain_link(builder, &stage_path[..]); } } } fn rustup_installed(builder: &Builder<'_>) -> bool { - command("rustup").arg("--version").run_capture_stdout(builder).is_success() + let mut rustup = command("rustup"); + rustup.arg("--version"); + + rustup.allow_failure().run_always().run_capture_stdout(builder).is_success() } fn stage_dir_exists(stage_path: &str) -> bool {