Skip to content

Commit

Permalink
Rollup merge of #126470 - onur-ozkan:optional-cargo-submodule, r=Kobzol
Browse files Browse the repository at this point in the history
make cargo submodule optional

Right now, we fetch the cargo submodule no matter what, even if the command we are running doesn't need it (e.g., `x build compiler library`). This PR changes that to only fetch the cargo submodule when it's necessary.

For more context, see the zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Why.20is.20cargo.20always.20checked.20out.3F
  • Loading branch information
matthiaskrgr authored Jun 28, 2024
2 parents 0262932 + 3457ecc commit d5ab931
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 39 deletions.
42 changes: 18 additions & 24 deletions src/bootstrap/src/core/build_steps/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<'_>) {
Expand All @@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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",
Expand All @@ -1034,20 +1029,19 @@ 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,
crates = ["bootstrap"]
);
tool_doc!(
RunMakeSupport,
"run_make_support",
"src/tools/run-make-support",
rustc_tool = false,
is_library = true,
Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand Down
2 changes: 2 additions & 0 deletions src/bootstrap/src/core/build_steps/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,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,
Expand Down
5 changes: 4 additions & 1 deletion src/bootstrap/src/core/build_steps/vendor.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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",
Expand Down
16 changes: 4 additions & 12 deletions src/bootstrap/src/core/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ 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<Item = Package> {
/// 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<Package> {
let collect_metadata = |manifest_path| {
let mut cargo = Command::new(&build.initial_cargo);
cargo
Expand All @@ -88,13 +88,5 @@ fn workspace_members(build: &Build) -> impl Iterator<Item = Package> {
};

// 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)
collect_metadata("Cargo.toml")
}
3 changes: 1 addition & 2 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down

0 comments on commit d5ab931

Please sign in to comment.