Skip to content

Make cargo a workspace #109133

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

Merged
merged 6 commits into from
Apr 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
1,817 changes: 38 additions & 1,779 deletions Cargo.lock

Large diffs are not rendered by default.

10 changes: 0 additions & 10 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ members = [
"src/tools/remote-test-server",
"src/tools/rust-installer",
"src/tools/rust-demangler",
"src/tools/cargo",
"src/tools/cargo/crates/credential/cargo-credential-1password",
"src/tools/cargo/crates/credential/cargo-credential-macos-keychain",
"src/tools/cargo/crates/credential/cargo-credential-wincred",
"src/tools/cargo/crates/mdman",
# "src/tools/cargo/crates/resolver-tests",
"src/tools/rustdoc",
"src/tools/rls",
"src/tools/rustfmt",
Expand Down Expand Up @@ -106,10 +100,6 @@ miniz_oxide.debug = 0
object.debug = 0

[patch.crates-io]
# See comments in `src/tools/rustc-workspace-hack/README.md` for what's going on
# here
rustc-workspace-hack = { path = 'src/tools/rustc-workspace-hack' }

# See comments in `library/rustc-std-workspace-core/README.md` for what's going on
# here
rustc-std-workspace-core = { path = 'library/rustc-std-workspace-core' }
Expand Down
3 changes: 2 additions & 1 deletion src/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,8 @@ def check_vendored_status(self):
if self.use_vendored_sources:
vendor_dir = os.path.join(self.rust_root, 'vendor')
if not os.path.exists(vendor_dir):
sync_dirs = "--sync ./src/tools/rust-analyzer/Cargo.toml " \
sync_dirs = "--sync ./src/tools/cargo/Cargo.toml " \
"--sync ./src/tools/rust-analyzer/Cargo.toml " \
"--sync ./compiler/rustc_codegen_cranelift/Cargo.toml " \
"--sync ./src/bootstrap/Cargo.toml "
print('error: vendoring required, but vendor directory does not exist.')
Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,11 +996,14 @@ impl Step for PlainSourceTarball {
// If we're building from git sources, we need to vendor a complete distribution.
if builder.rust_info().is_managed_git_subrepository() {
// Ensure we have the submodules checked out.
builder.update_submodule(Path::new("src/tools/cargo"));
builder.update_submodule(Path::new("src/tools/rust-analyzer"));

// Vendor all Cargo dependencies
let mut cmd = Command::new(&builder.initial_cargo);
cmd.arg("vendor")
.arg("--sync")
.arg(builder.src.join("./src/tools/cargo/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./src/tools/rust-analyzer/Cargo.toml"))
.arg("--sync")
Expand Down
3 changes: 0 additions & 3 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,6 @@ pub struct Build {
ci_env: CiEnv,
delayed_failures: RefCell<Vec<String>>,
prerelease_version: Cell<Option<u32>>,
tool_artifacts:
RefCell<HashMap<TargetSelection, HashMap<String, (&'static str, PathBuf, Vec<String>)>>>,

#[cfg(feature = "build-metrics")]
metrics: metrics::BuildMetrics,
Expand Down Expand Up @@ -458,7 +456,6 @@ impl Build {
ci_env: CiEnv::current(),
delayed_failures: RefCell::new(Vec::new()),
prerelease_version: Cell::new(None),
tool_artifacts: Default::default(),

#[cfg(feature = "build-metrics")]
metrics: metrics::BuildMetrics::init(),
Expand Down
59 changes: 44 additions & 15 deletions src/bootstrap/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,38 +7,35 @@ use crate::cache::INTERNER;
use crate::util::output;
use crate::{Build, Crate};

#[derive(Deserialize)]
/// For more information, see the output of
/// <https://doc.rust-lang.org/nightly/cargo/commands/cargo-metadata.html>
#[derive(Debug, Deserialize)]
struct Output {
packages: Vec<Package>,
}

#[derive(Deserialize)]
/// For more information, see the output of
/// <https://doc.rust-lang.org/nightly/cargo/commands/cargo-metadata.html>
#[derive(Debug, Deserialize)]
struct Package {
name: String,
source: Option<String>,
manifest_path: String,
dependencies: Vec<Dependency>,
}

#[derive(Deserialize)]
/// For more information, see the output of
/// <https://doc.rust-lang.org/nightly/cargo/commands/cargo-metadata.html>
#[derive(Debug, Deserialize)]
struct Dependency {
name: String,
source: Option<String>,
}

/// Collects and stores package metadata of each workspace members into `build`,
/// by executing `cargo metadata` commands.
pub fn build(build: &mut Build) {
// Run `cargo metadata` to figure out what crates we're testing.
let mut cargo = Command::new(&build.initial_cargo);
cargo
.arg("metadata")
.arg("--format-version")
.arg("1")
.arg("--no-deps")
.arg("--manifest-path")
.arg(build.src.join("Cargo.toml"));
let output = output(&mut cargo);
let output: Output = serde_json::from_str(&output).unwrap();
for package in output.packages {
for package in workspace_members(build) {
if package.source.is_none() {
let name = INTERNER.intern_string(package.name);
let mut path = PathBuf::from(package.manifest_path);
Expand All @@ -57,3 +54,35 @@ 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> {
let cmd_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
};

// 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();

// 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)
}
135 changes: 5 additions & 130 deletions src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::collections::HashSet;
use std::env;
use std::fs;
use std::path::PathBuf;
Expand Down Expand Up @@ -120,135 +119,9 @@ impl Step for ToolBuild {
&self.target,
);
builder.info(&msg);
let mut duplicates = Vec::new();
let is_expected = compile::stream_cargo(builder, cargo, vec![], &mut |msg| {
// Only care about big things like the RLS/Cargo for now
match tool {
"rls" | "cargo" | "clippy-driver" | "miri" | "rustfmt" => {}

_ => return,
}
let (id, features, filenames) = match msg {
compile::CargoMessage::CompilerArtifact {
package_id,
features,
filenames,
target: _,
} => (package_id, features, filenames),
_ => return,
};
let features = features.iter().map(|s| s.to_string()).collect::<Vec<_>>();

for path in filenames {
let val = (tool, PathBuf::from(&*path), features.clone());
// we're only interested in deduplicating rlibs for now
if val.1.extension().and_then(|s| s.to_str()) != Some("rlib") {
continue;
}

// Don't worry about compiles that turn out to be host
// dependencies or build scripts. To skip these we look for
// anything that goes in `.../release/deps` but *doesn't* go in
// `$target/release/deps`. This ensure that outputs in
// `$target/release` are still considered candidates for
// deduplication.
if let Some(parent) = val.1.parent() {
if parent.ends_with("release/deps") {
let maybe_target = parent
.parent()
.and_then(|p| p.parent())
.and_then(|p| p.file_name())
.and_then(|p| p.to_str())
.unwrap();
if maybe_target != &*target.triple {
continue;
}
}
}

// Record that we've built an artifact for `id`, and if one was
// already listed then we need to see if we reused the same
// artifact or produced a duplicate.
let mut artifacts = builder.tool_artifacts.borrow_mut();
let prev_artifacts = artifacts.entry(target).or_default();
let prev = match prev_artifacts.get(&*id) {
Some(prev) => prev,
None => {
prev_artifacts.insert(id.to_string(), val);
continue;
}
};
if prev.1 == val.1 {
return; // same path, same artifact
}

// If the paths are different and one of them *isn't* inside of
// `release/deps`, then it means it's probably in
// `$target/release`, or it's some final artifact like
// `libcargo.rlib`. In these situations Cargo probably just
// copied it up from `$target/release/deps/libcargo-xxxx.rlib`,
// so if the features are equal we can just skip it.
let prev_no_hash = prev.1.parent().unwrap().ends_with("release/deps");
let val_no_hash = val.1.parent().unwrap().ends_with("release/deps");
if prev.2 == val.2 || !prev_no_hash || !val_no_hash {
return;
}

// ... and otherwise this looks like we duplicated some sort of
// compilation, so record it to generate an error later.
duplicates.push((id.to_string(), val, prev.clone()));
}
});

if is_expected && !duplicates.is_empty() {
eprintln!(
"duplicate artifacts found when compiling a tool, this \
typically means that something was recompiled because \
a transitive dependency has different features activated \
than in a previous build:\n"
);
let (same, different): (Vec<_>, Vec<_>) =
duplicates.into_iter().partition(|(_, cur, prev)| cur.2 == prev.2);
if !same.is_empty() {
eprintln!(
"the following dependencies are duplicated although they \
have the same features enabled:"
);
for (id, cur, prev) in same {
eprintln!(" {}", id);
// same features
eprintln!(" `{}` ({:?})\n `{}` ({:?})", cur.0, cur.1, prev.0, prev.1);
}
}
if !different.is_empty() {
eprintln!("the following dependencies have different features:");
for (id, cur, prev) in different {
eprintln!(" {}", id);
let cur_features: HashSet<_> = cur.2.into_iter().collect();
let prev_features: HashSet<_> = prev.2.into_iter().collect();
eprintln!(
" `{}` additionally enabled features {:?} at {:?}",
cur.0,
&cur_features - &prev_features,
cur.1
);
eprintln!(
" `{}` additionally enabled features {:?} at {:?}",
prev.0,
&prev_features - &cur_features,
prev.1
);
}
}
eprintln!();
eprintln!(
"to fix this you will probably want to edit the local \
src/tools/rustc-workspace-hack/Cargo.toml crate, as \
that will update the dependency graph to ensure that \
these crates all share the same feature set"
);
panic!("tools should not compile multiple copies of the same crate");
}
let mut cargo = Command::from(cargo);
let is_expected = builder.try_run_quiet(&mut cargo);

builder.save_toolstate(
tool,
Expand Down Expand Up @@ -299,7 +172,9 @@ pub fn prepare_tool_cargo(
|| path.ends_with("rustfmt")
{
cargo.env("LIBZ_SYS_STATIC", "1");
features.push("rustc-workspace-hack/all-static".to_string());
}
if path.ends_with("cargo") {
features.push("all-static".to_string());
}
}

Expand Down
4 changes: 0 additions & 4 deletions src/tools/rls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,3 @@ license = "Apache-2.0/MIT"
[dependencies]
serde = { version = "1.0.143", features = ["derive"] }
serde_json = "1.0.83"
# A noop dependency that changes in the Rust repository, it's a bit of a hack.
# See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`
# for more information.
rustc-workspace-hack = "1.0.0"
Loading