Skip to content

Rollup of 5 pull requests #110679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix x test --no-deps
- Use `cargo metadata` to determine whether a crate has a library
  package or not
- Collect metadata for all workspaces, not just the root workspace and
  cargo
- Don't pass `--lib` for crates without a library
- Use `run_cargo_test` for rust-installer
- Don't build documentation in `lint-docs` if `--no-doc` is passed
  • Loading branch information
jyn514 committed Apr 21, 2023
commit b3ee81a536353e9622ef7fcf246fdb63ea9f4067
1 change: 1 addition & 0 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ struct Crate {
name: Interned<String>,
deps: HashSet<Interned<String>>,
path: PathBuf,
has_lib: bool,
}

impl Crate {
Expand Down
42 changes: 26 additions & 16 deletions src/bootstrap/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use serde_derive::Deserialize;

use crate::cache::INTERNER;
use crate::util::output;
use crate::{Build, Crate};
use crate::{t, Build, Crate};

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

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

#[derive(Debug, Deserialize)]
struct Target {
kind: Vec<String>,
}

/// Collects and stores package metadata of each workspace members into `build`,
/// by executing `cargo metadata` commands.
pub fn build(build: &mut Build) {
Expand All @@ -46,11 +52,16 @@ pub fn build(build: &mut Build) {
.filter(|dep| dep.source.is_none())
.map(|dep| INTERNER.intern_string(dep.name))
.collect();
let krate = Crate { name, deps, path };
let has_lib = package.targets.iter().any(|t| t.kind.iter().any(|k| k == "lib"));
let krate = Crate { name, deps, path, has_lib };
let relative_path = krate.local_path(build);
build.crates.insert(name, krate);
let existing_path = build.crate_paths.insert(relative_path, name);
assert!(existing_path.is_none(), "multiple crates with the same path");
assert!(
existing_path.is_none(),
"multiple crates with the same path: {}",
existing_path.unwrap()
);
}
}
}
Expand All @@ -60,29 +71,28 @@ pub fn build(build: &mut Build) {
/// 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> {
let cmd_metadata = |manifest_path| {
let collect_metadata = |manifest_path| {
let mut cargo = Command::new(&build.initial_cargo);
cargo
.arg("metadata")
.arg("--format-version")
.arg("1")
.arg("--no-deps")
.arg("--manifest-path")
.arg(manifest_path);
cargo
.arg(build.src.join(manifest_path));
let metadata_output = output(&mut cargo);
let Output { packages, .. } = t!(serde_json::from_str(&metadata_output));
packages
};

// Collects `metadata.packages` from the root workspace.
let root_manifest_path = build.src.join("Cargo.toml");
let root_output = output(&mut cmd_metadata(&root_manifest_path));
let Output { packages, .. } = serde_json::from_str(&root_output).unwrap();

// Collects `metadata.packages` from src/tools/cargo separately.
let cargo_manifest_path = build.src.join("src/tools/cargo/Cargo.toml");
let cargo_output = output(&mut cmd_metadata(&cargo_manifest_path));
let Output { packages: cargo_packages, .. } = serde_json::from_str(&cargo_output).unwrap();
// 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)

packages.into_iter().chain(cargo_package).chain(ra_packages).chain(bootstrap_packages)
}
49 changes: 31 additions & 18 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ impl Step for CrateBootstrap {
path,
bootstrap_host,
));
run_cargo_test(cargo, &[], &[], compiler, bootstrap_host, builder);
let crate_name = path.rsplit_once('/').unwrap().1;
run_cargo_test(cargo, &[], &[], crate_name, compiler, bootstrap_host, builder);
}
}

Expand Down Expand Up @@ -152,7 +153,11 @@ You can skip linkcheck with --exclude src/tools/linkchecker"
SourceType::InTree,
&[],
);
run_cargo_test(cargo, &[], &[], compiler, bootstrap_host, builder);
run_cargo_test(cargo, &[], &[], "linkchecker", compiler, bootstrap_host, builder);

if builder.doc_tests == DocTests::No {
return;
}

// Build all the default documentation.
builder.default_doc(&[]);
Expand Down Expand Up @@ -307,7 +312,7 @@ impl Step for Cargo {
cargo.env("CARGO_TEST_DISABLE_NIGHTLY", "1");
cargo.env("PATH", &path_for_cargo(builder, compiler));

run_cargo_test(cargo, &[], &[], compiler, self.host, builder);
run_cargo_test(cargo, &[], &[], "cargo", compiler, self.host, builder);
}
}

Expand Down Expand Up @@ -364,7 +369,7 @@ impl Step for RustAnalyzer {
cargo.env("SKIP_SLOW_TESTS", "1");

cargo.add_rustc_lib_path(builder, compiler);
run_cargo_test(cargo, &[], &[], compiler, host, builder);
run_cargo_test(cargo, &[], &[], "rust-analyzer", compiler, host, builder);
}
}

Expand Down Expand Up @@ -413,7 +418,7 @@ impl Step for Rustfmt {

cargo.add_rustc_lib_path(builder, compiler);

run_cargo_test(cargo, &[], &[], compiler, host, builder);
run_cargo_test(cargo, &[], &[], "rustfmt", compiler, host, builder);
}
}

Expand Down Expand Up @@ -461,7 +466,7 @@ impl Step for RustDemangler {
cargo.env("RUST_DEMANGLER_DRIVER_PATH", rust_demangler);
cargo.add_rustc_lib_path(builder, compiler);

run_cargo_test(cargo, &[], &[], compiler, host, builder);
run_cargo_test(cargo, &[], &[], "rust-demangler", compiler, host, builder);
}
}

Expand Down Expand Up @@ -598,7 +603,7 @@ impl Step for Miri {

// This can NOT be `run_cargo_test` since the Miri test runner
// does not understand the flags added by `add_flags_and_try_run_test`.
let mut cargo = prepare_cargo_test(cargo, &[], &[], compiler, target, builder);
let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", compiler, target, builder);
{
let _time = util::timeit(&builder);
builder.run(&mut cargo);
Expand Down Expand Up @@ -675,7 +680,7 @@ impl Step for CompiletestTest {
&[],
);
cargo.allow_features("test");
run_cargo_test(cargo, &[], &[], compiler, host, builder);
run_cargo_test(cargo, &[], &[], "compiletest", compiler, host, builder);
}
}

Expand Down Expand Up @@ -718,17 +723,13 @@ impl Step for Clippy {
&[],
);

if !builder.fail_fast {
cargo.arg("--no-fail-fast");
}

cargo.env("RUSTC_TEST_SUITE", builder.rustc(compiler));
cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(compiler));
let host_libs = builder.stage_out(compiler, Mode::ToolRustc).join(builder.cargo_dir());
cargo.env("HOST_LIBS", host_libs);

cargo.add_rustc_lib_path(builder, compiler);
let mut cargo = prepare_cargo_test(cargo, &[], &[], compiler, host, builder);
let mut cargo = prepare_cargo_test(cargo, &[], &[], "clippy", compiler, host, builder);

if builder.try_run(&mut cargo) {
// The tests succeeded; nothing to do.
Expand Down Expand Up @@ -2037,11 +2038,13 @@ fn run_cargo_test(
cargo: impl Into<Command>,
libtest_args: &[&str],
crates: &[Interned<String>],
primary_crate: &str,
compiler: Compiler,
target: TargetSelection,
builder: &Builder<'_>,
) -> bool {
let mut cargo = prepare_cargo_test(cargo, libtest_args, crates, compiler, target, builder);
let mut cargo =
prepare_cargo_test(cargo, libtest_args, crates, primary_crate, compiler, target, builder);
let _time = util::timeit(&builder);
add_flags_and_try_run_tests(builder, &mut cargo)
}
Expand All @@ -2051,6 +2054,7 @@ fn prepare_cargo_test(
cargo: impl Into<Command>,
libtest_args: &[&str],
crates: &[Interned<String>],
primary_crate: &str,
compiler: Compiler,
target: TargetSelection,
builder: &Builder<'_>,
Expand All @@ -2068,7 +2072,14 @@ fn prepare_cargo_test(
cargo.arg("--doc");
}
DocTests::No => {
cargo.args(&["--lib", "--bins", "--examples", "--tests", "--benches"]);
let krate = &builder
.crates
.get(&INTERNER.intern_str(primary_crate))
.unwrap_or_else(|| panic!("missing crate {primary_crate}"));
if krate.has_lib {
cargo.arg("--lib");
}
cargo.args(&["--bins", "--examples", "--tests", "--benches"]);
}
DocTests::Yes => {}
}
Expand Down Expand Up @@ -2181,7 +2192,7 @@ impl Step for Crate {
&compiler.host,
target
));
run_cargo_test(cargo, &[], &self.crates, compiler, target, builder);
run_cargo_test(cargo, &[], &self.crates, &self.crates[0], compiler, target, builder);
}
}

Expand Down Expand Up @@ -2280,6 +2291,7 @@ impl Step for CrateRustdoc {
cargo,
&[],
&[INTERNER.intern_str("rustdoc:0.0.0")],
"rustdoc",
compiler,
target,
builder,
Expand Down Expand Up @@ -2346,6 +2358,7 @@ impl Step for CrateRustdocJsonTypes {
cargo,
libtest_args,
&[INTERNER.intern_str("rustdoc-json-types")],
"rustdoc-json-types",
compiler,
target,
builder,
Expand Down Expand Up @@ -2505,7 +2518,7 @@ impl Step for Bootstrap {
}
// rustbuild tests are racy on directory creation so just run them one at a time.
// Since there's not many this shouldn't be a problem.
run_cargo_test(cmd, &["--test-threads=1"], &[], compiler, host, builder);
run_cargo_test(cmd, &["--test-threads=1"], &[], "bootstrap", compiler, host, builder);
}

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand Down Expand Up @@ -2618,7 +2631,7 @@ impl Step for RustInstaller {
SourceType::InTree,
&[],
);
try_run(builder, &mut cargo.into());
run_cargo_test(cargo, &[], &[], "installer", compiler, bootstrap_host, builder);

// We currently don't support running the test.sh script outside linux(?) environments.
// Eventually this should likely migrate to #[test]s in rust-installer proper rather than a
Expand Down