Skip to content

Commit

Permalink
Rollup merge of rust-lang#126470 - onur-ozkan:optional-cargo-submodul…
Browse files Browse the repository at this point in the history
…e, r=Kobzol

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 e0439d9 + 3457ecc commit b08a0cf
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 b08a0cf

Please sign in to comment.