From 11acf83159edc94edeeafd75eff458d9d584b822 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Fri, 14 Jun 2024 09:45:02 +0300 Subject: [PATCH 1/5] bootstrap: exclude cargo from package metadata Signed-off-by: onur-ozkan --- src/bootstrap/src/core/metadata.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/bootstrap/src/core/metadata.rs b/src/bootstrap/src/core/metadata.rs index 08a96407a6917..12867cb46b811 100644 --- a/src/bootstrap/src/core/metadata.rs +++ b/src/bootstrap/src/core/metadata.rs @@ -66,9 +66,6 @@ pub fn build(build: &mut Build) { } /// Invokes `cargo metadata` to get package metadata of each workspace member. -/// -/// Note that `src/tools/cargo` is no longer a workspace member but we still -/// treat it as one here, by invoking an additional `cargo metadata` command. fn workspace_members(build: &Build) -> impl Iterator { let collect_metadata = |manifest_path| { let mut cargo = Command::new(&build.initial_cargo); @@ -89,12 +86,8 @@ fn workspace_members(build: &Build) -> impl Iterator { // Collects `metadata.packages` from all workspaces. let packages = collect_metadata("Cargo.toml"); - let cargo_packages = collect_metadata("src/tools/cargo/Cargo.toml"); let ra_packages = collect_metadata("src/tools/rust-analyzer/Cargo.toml"); let bootstrap_packages = collect_metadata("src/bootstrap/Cargo.toml"); - // We only care about the root package from `src/tool/cargo` workspace. - let cargo_package = cargo_packages.into_iter().find(|pkg| pkg.name == "cargo").into_iter(); - - packages.into_iter().chain(cargo_package).chain(ra_packages).chain(bootstrap_packages) + packages.into_iter().chain(ra_packages).chain(bootstrap_packages) } From 457ac5d570f1897ac4c59b3db5f86f26a17d14bb Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Fri, 14 Jun 2024 09:45:59 +0300 Subject: [PATCH 2/5] don't fetch/sync cargo submodule by default Signed-off-by: onur-ozkan --- src/bootstrap/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index dfc30298c28d2..afba907ee92b4 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -469,8 +469,7 @@ impl Build { // Make sure we update these before gathering metadata so we don't get an error about missing // Cargo.toml files. - let rust_submodules = - ["src/tools/cargo", "src/doc/book", "library/backtrace", "library/stdarch"]; + let rust_submodules = ["src/doc/book", "library/backtrace", "library/stdarch"]; for s in rust_submodules { build.update_submodule(Path::new(s)); } From 8c3ebf7a4dced401e58f544f1af187f8c41d3472 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Fri, 14 Jun 2024 09:49:28 +0300 Subject: [PATCH 3/5] refactor `tool_doc` macro in bootstrap Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/doc.rs | 42 ++++++++++------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/doc.rs b/src/bootstrap/src/core/build_steps/doc.rs index 6748625f1323d..87c700dad063d 100644 --- a/src/bootstrap/src/core/build_steps/doc.rs +++ b/src/bootstrap/src/core/build_steps/doc.rs @@ -888,12 +888,11 @@ impl Step for Rustc { macro_rules! tool_doc { ( $tool: ident, - $should_run: literal, $path: literal, $(rustc_tool = $rustc_tool:literal, )? - $(in_tree = $in_tree:literal ,)? $(is_library = $is_library:expr,)? $(crates = $crates:expr)? + $(, submodule $(= $submodule:literal)? )? ) => { #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub struct $tool { @@ -907,7 +906,7 @@ macro_rules! tool_doc { fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { let builder = run.builder; - run.crate_or_deps($should_run).default_condition(builder.config.compiler_docs) + run.path($path).default_condition(builder.config.compiler_docs) } fn make_run(run: RunConfig<'_>) { @@ -921,6 +920,15 @@ macro_rules! tool_doc { /// we do not merge it with the other documentation from std, test and /// proc_macros. This is largely just a wrapper around `cargo doc`. fn run(self, builder: &Builder<'_>) { + let source_type = SourceType::InTree; + $( + let _ = source_type; // silence the "unused variable" warning + let source_type = SourceType::Submodule; + + let path = Path::new(submodule_helper!( $path, submodule $( = $submodule )? )); + builder.update_submodule(&path); + )? + let stage = builder.top_stage; let target = self.target; @@ -941,12 +949,6 @@ macro_rules! tool_doc { builder.ensure(compile::Rustc::new(compiler, target)); } - let source_type = if true $(&& $in_tree)? { - SourceType::InTree - } else { - SourceType::Submodule - }; - // Build cargo command. let mut cargo = prepare_tool_cargo( builder, @@ -1008,21 +1010,14 @@ macro_rules! tool_doc { } } -tool_doc!(Rustdoc, "rustdoc-tool", "src/tools/rustdoc", crates = ["rustdoc", "rustdoc-json-types"]); -tool_doc!( - Rustfmt, - "rustfmt-nightly", - "src/tools/rustfmt", - crates = ["rustfmt-nightly", "rustfmt-config_proc_macro"] -); -tool_doc!(Clippy, "clippy", "src/tools/clippy", crates = ["clippy_config", "clippy_utils"]); -tool_doc!(Miri, "miri", "src/tools/miri", crates = ["miri"]); +tool_doc!(Rustdoc, "src/tools/rustdoc", crates = ["rustdoc", "rustdoc-json-types"]); +tool_doc!(Rustfmt, "src/tools/rustfmt", crates = ["rustfmt-nightly", "rustfmt-config_proc_macro"]); +tool_doc!(Clippy, "src/tools/clippy", crates = ["clippy_config", "clippy_utils"]); +tool_doc!(Miri, "src/tools/miri", crates = ["miri"]); tool_doc!( Cargo, - "cargo", "src/tools/cargo", rustc_tool = false, - in_tree = false, crates = [ "cargo", "cargo-credential", @@ -1034,12 +1029,12 @@ tool_doc!( "crates-io", "mdman", "rustfix", - ] + ], + submodule = "src/tools/cargo" ); -tool_doc!(Tidy, "tidy", "src/tools/tidy", rustc_tool = false, crates = ["tidy"]); +tool_doc!(Tidy, "src/tools/tidy", rustc_tool = false, crates = ["tidy"]); tool_doc!( Bootstrap, - "bootstrap", "src/bootstrap", rustc_tool = false, is_library = true, @@ -1047,7 +1042,6 @@ tool_doc!( ); tool_doc!( RunMakeSupport, - "run_make_support", "src/tools/run-make-support", rustc_tool = false, is_library = true, From 51f6e68559bdef1a1b16b3d026552eab51b5f389 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Fri, 14 Jun 2024 12:32:34 +0300 Subject: [PATCH 4/5] handle cargo submodule in a lazy-load way Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/test.rs | 3 +++ src/bootstrap/src/core/build_steps/tool.rs | 2 ++ src/bootstrap/src/core/build_steps/vendor.rs | 5 ++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index efc09c41bf429..1ef5af7cc2daf 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2983,6 +2983,9 @@ impl Step for Bootstrap { let compiler = builder.compiler(0, host); let _guard = builder.msg(Kind::Test, 0, "bootstrap", host, host); + // Some tests require cargo submodule to be present. + builder.build.update_submodule(Path::new("src/tools/cargo")); + let mut check_bootstrap = Command::new(builder.python()); check_bootstrap .args(["-m", "unittest", "bootstrap_test.py"]) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 850c8bfe2f8b9..d909a39b60a61 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -654,6 +654,8 @@ impl Step for Cargo { } fn run(self, builder: &Builder<'_>) -> PathBuf { + builder.build.update_submodule(Path::new("src/tools/cargo")); + builder.ensure(ToolBuild { compiler: self.compiler, target: self.target, diff --git a/src/bootstrap/src/core/build_steps/vendor.rs b/src/bootstrap/src/core/build_steps/vendor.rs index 68f1b1bef3f39..e92ab57619b6a 100644 --- a/src/bootstrap/src/core/build_steps/vendor.rs +++ b/src/bootstrap/src/core/build_steps/vendor.rs @@ -1,5 +1,5 @@ use crate::core::builder::{Builder, RunConfig, ShouldRun, Step}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Command; #[derive(Debug, Clone, Hash, PartialEq, Eq)] @@ -34,6 +34,9 @@ impl Step for Vendor { cmd.arg("--versioned-dirs"); } + // cargo submodule must be present for `x vendor` to work. + builder.build.update_submodule(Path::new("src/tools/cargo")); + // Sync these paths by default. for p in [ "src/tools/cargo/Cargo.toml", From 3457ecc776e87550bc1b1bd4bb686bde8bd5c504 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Thu, 27 Jun 2024 13:06:10 +0300 Subject: [PATCH 5/5] remove unnecessary packages from `metadata::workspace_members` Currently bootstrap doesn't use any inner paths from rust-analyzer and bootstrap with `ShouldRun::create_or_deps`. Signed-off-by: onur-ozkan --- src/bootstrap/src/core/metadata.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/src/core/metadata.rs b/src/bootstrap/src/core/metadata.rs index 12867cb46b811..220eb5ba126e4 100644 --- a/src/bootstrap/src/core/metadata.rs +++ b/src/bootstrap/src/core/metadata.rs @@ -66,7 +66,10 @@ pub fn build(build: &mut Build) { } /// Invokes `cargo metadata` to get package metadata of each workspace member. -fn workspace_members(build: &Build) -> impl Iterator { +/// +/// This is used to resolve specific crate paths in `fn should_run` to compile +/// particular crate (e.g., `x build sysroot` to build library/sysroot). +fn workspace_members(build: &Build) -> Vec { let collect_metadata = |manifest_path| { let mut cargo = Command::new(&build.initial_cargo); cargo @@ -85,9 +88,5 @@ fn workspace_members(build: &Build) -> impl Iterator { }; // Collects `metadata.packages` from all workspaces. - let packages = collect_metadata("Cargo.toml"); - let ra_packages = collect_metadata("src/tools/rust-analyzer/Cargo.toml"); - let bootstrap_packages = collect_metadata("src/bootstrap/Cargo.toml"); - - packages.into_iter().chain(ra_packages).chain(bootstrap_packages) + collect_metadata("Cargo.toml") }