Skip to content

Commit 87b1f89

Browse files
committed
Auto merge of #110576 - jyn514:unify-test-args, r=ozkanonur
bootstrap: Unify test argument handling Fixes #104198. Does *not* help with #80124 because I couldn't figure out a reasonable way to omit `--lib` only for `panic_abort` and not other `std` dependencies. - Remove unnecessary `test_kind` field and `TestKind` struct. These are just subsets of the existing `builder.kind` / `Kind` struct. - Add a new `run_cargo_test` function which handles passing arguments to cargo based on `builder.config` - Switch all Steps in `mod test` to `run_cargo_test` where possible - Combine several steps into one `CrateBootstrap` step. These tests all do the same thing, just with different crate names. - Fix `x test --no-doc`. This is much simpler after the refactors mentioned earlier, but I'm happy to split it into a separate PR if desired. Before, this would panic a lot because steps forgot to pass `--lib`.
2 parents eb62877 + 78a7093 commit 87b1f89

File tree

6 files changed

+215
-349
lines changed

6 files changed

+215
-349
lines changed

src/bootstrap/builder.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,14 @@ impl Kind {
634634
Kind::Suggest => "suggest",
635635
}
636636
}
637+
638+
pub fn test_description(&self) -> &'static str {
639+
match self {
640+
Kind::Test => "Testing",
641+
Kind::Bench => "Benchmarking",
642+
_ => panic!("not a test command: {}!", self.as_str()),
643+
}
644+
}
637645
}
638646

639647
impl<'a> Builder<'a> {
@@ -695,7 +703,6 @@ impl<'a> Builder<'a> {
695703
crate::toolstate::ToolStateCheck,
696704
test::ExpandYamlAnchors,
697705
test::Tidy,
698-
test::TidySelfTest,
699706
test::Ui,
700707
test::RunPassValgrind,
701708
test::MirOpt,
@@ -711,11 +718,9 @@ impl<'a> Builder<'a> {
711718
test::CrateLibrustc,
712719
test::CrateRustdoc,
713720
test::CrateRustdocJsonTypes,
714-
test::CrateJsonDocLint,
715-
test::SuggestTestsCrate,
721+
test::CrateBootstrap,
716722
test::Linkcheck,
717723
test::TierCheck,
718-
test::ReplacePlaceholderTest,
719724
test::Cargotest,
720725
test::Cargo,
721726
test::RustAnalyzer,

src/bootstrap/builder/tests.rs

-1
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,6 @@ mod dist {
578578
compiler: Compiler { host, stage: 0 },
579579
target: host,
580580
mode: Mode::Std,
581-
test_kind: test::TestKind::Test,
582581
crates: vec![INTERNER.intern_str("std")],
583582
},]
584583
);

src/bootstrap/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ struct Crate {
246246
name: Interned<String>,
247247
deps: HashSet<Interned<String>>,
248248
path: PathBuf,
249+
has_lib: bool,
249250
}
250251

251252
impl Crate {

src/bootstrap/metadata.rs

+26-16
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use serde_derive::Deserialize;
55

66
use crate::cache::INTERNER;
77
use crate::util::output;
8-
use crate::{Build, Crate};
8+
use crate::{t, Build, Crate};
99

1010
/// For more information, see the output of
1111
/// <https://doc.rust-lang.org/nightly/cargo/commands/cargo-metadata.html>
@@ -22,6 +22,7 @@ struct Package {
2222
source: Option<String>,
2323
manifest_path: String,
2424
dependencies: Vec<Dependency>,
25+
targets: Vec<Target>,
2526
}
2627

2728
/// For more information, see the output of
@@ -32,6 +33,11 @@ struct Dependency {
3233
source: Option<String>,
3334
}
3435

36+
#[derive(Debug, Deserialize)]
37+
struct Target {
38+
kind: Vec<String>,
39+
}
40+
3541
/// Collects and stores package metadata of each workspace members into `build`,
3642
/// by executing `cargo metadata` commands.
3743
pub fn build(build: &mut Build) {
@@ -46,11 +52,16 @@ pub fn build(build: &mut Build) {
4652
.filter(|dep| dep.source.is_none())
4753
.map(|dep| INTERNER.intern_string(dep.name))
4854
.collect();
49-
let krate = Crate { name, deps, path };
55+
let has_lib = package.targets.iter().any(|t| t.kind.iter().any(|k| k == "lib"));
56+
let krate = Crate { name, deps, path, has_lib };
5057
let relative_path = krate.local_path(build);
5158
build.crates.insert(name, krate);
5259
let existing_path = build.crate_paths.insert(relative_path, name);
53-
assert!(existing_path.is_none(), "multiple crates with the same path");
60+
assert!(
61+
existing_path.is_none(),
62+
"multiple crates with the same path: {}",
63+
existing_path.unwrap()
64+
);
5465
}
5566
}
5667
}
@@ -60,29 +71,28 @@ pub fn build(build: &mut Build) {
6071
/// Note that `src/tools/cargo` is no longer a workspace member but we still
6172
/// treat it as one here, by invoking an additional `cargo metadata` command.
6273
fn workspace_members(build: &Build) -> impl Iterator<Item = Package> {
63-
let cmd_metadata = |manifest_path| {
74+
let collect_metadata = |manifest_path| {
6475
let mut cargo = Command::new(&build.initial_cargo);
6576
cargo
6677
.arg("metadata")
6778
.arg("--format-version")
6879
.arg("1")
6980
.arg("--no-deps")
7081
.arg("--manifest-path")
71-
.arg(manifest_path);
72-
cargo
82+
.arg(build.src.join(manifest_path));
83+
let metadata_output = output(&mut cargo);
84+
let Output { packages, .. } = t!(serde_json::from_str(&metadata_output));
85+
packages
7386
};
7487

75-
// Collects `metadata.packages` from the root workspace.
76-
let root_manifest_path = build.src.join("Cargo.toml");
77-
let root_output = output(&mut cmd_metadata(&root_manifest_path));
78-
let Output { packages, .. } = serde_json::from_str(&root_output).unwrap();
79-
80-
// Collects `metadata.packages` from src/tools/cargo separately.
81-
let cargo_manifest_path = build.src.join("src/tools/cargo/Cargo.toml");
82-
let cargo_output = output(&mut cmd_metadata(&cargo_manifest_path));
83-
let Output { packages: cargo_packages, .. } = serde_json::from_str(&cargo_output).unwrap();
88+
// Collects `metadata.packages` from all workspaces.
89+
let packages = collect_metadata("Cargo.toml");
90+
let cargo_packages = collect_metadata("src/tools/cargo/Cargo.toml");
91+
let ra_packages = collect_metadata("src/tools/rust-analyzer/Cargo.toml");
92+
let bootstrap_packages = collect_metadata("src/bootstrap/Cargo.toml");
8493

8594
// We only care about the root package from `src/tool/cargo` workspace.
8695
let cargo_package = cargo_packages.into_iter().find(|pkg| pkg.name == "cargo").into_iter();
87-
packages.into_iter().chain(cargo_package)
96+
97+
packages.into_iter().chain(cargo_package).chain(ra_packages).chain(bootstrap_packages)
8898
}

0 commit comments

Comments
 (0)