Skip to content

Commit

Permalink
Auto merge of #8489 - arlosi:deterministic_metadata, r=alexcrichton
Browse files Browse the repository at this point in the history 
Make `cargo metadata` output deterministic

Uses BTreeMap instead of HashMap for the `cargo metadata` command, ensuring the output is sorted.

The change did not cause a measurable performance impact for running `cargo metadata` on `cargo` itself.

Fixes #8477
  • Loading branch information
@bors
bors committed Jul 16, 2020
2 parents 1bc6e45 + 58869e5 commit e37b35c
Show file tree
Hide file tree
Showing 5 changed files with 247 additions and 261 deletions.
26 changes: 5 additions & 21 deletions crates/cargo-test-support/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1444,7 +1444,7 @@ fn lines_match_works() {
/// You can use `[..]` wildcard in strings (useful for OS-dependent things such
/// as paths). You can use a `"{...}"` string literal as a wildcard for
/// arbitrary nested JSON (useful for parts of object emitted by other programs
/// (e.g., rustc) rather than Cargo itself). Arrays are sorted before comparison.
/// (e.g., rustc) rather than Cargo itself).
pub fn find_json_mismatch(expected: &Value, actual: &Value) -> Result<(), String> {
match find_json_mismatch_r(expected, actual) {
Some((expected_part, actual_part)) => Err(format!(
Expand Down Expand Up @@ -1472,26 +1472,10 @@ fn find_json_mismatch_r<'a>(
return Some((expected, actual));
}

let mut l = l.iter().collect::<Vec<_>>();
let mut r = r.iter().collect::<Vec<_>>();

l.retain(
|l| match r.iter().position(|r| find_json_mismatch_r(l, r).is_none()) {
Some(i) => {
r.remove(i);
false
}
None => true,
},
);

if !l.is_empty() {
assert!(!r.is_empty());
Some((l[0], r[0]))
} else {
assert_eq!(r.len(), 0);
None
}
l.iter()
.zip(r.iter())
.filter_map(|(l, r)| find_json_mismatch_r(l, r))
.next()
}
(&Object(ref l), &Object(ref r)) => {
let same_keys = l.len() == r.len() && l.keys().all(|k| r.contains_key(k));
Expand Down
18 changes: 10 additions & 8 deletions src/cargo/ops/cargo_output_metadata.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::util::interning::InternedString;
use crate::util::CargoResult;
use cargo_platform::Platform;
use serde::Serialize;
use std::collections::HashMap;
use std::collections::BTreeMap;
use std::path::PathBuf;

const VERSION: u32 = 1;
Expand Down Expand Up @@ -131,7 +131,7 @@ fn build_resolve_graph(
// Download all Packages. This is needed to serialize the information
// for every package. In theory this could honor target filtering,
// but that would be somewhat complex.
let mut package_map: HashMap<PackageId, Package> = ws_resolve
let package_map: BTreeMap<PackageId, Package> = ws_resolve
.pkg_set
.get_many(ws_resolve.pkg_set.package_ids())?
.into_iter()
Expand All @@ -141,7 +141,7 @@ fn build_resolve_graph(

// Start from the workspace roots, and recurse through filling out the
// map, filtering targets as necessary.
let mut node_map = HashMap::new();
let mut node_map = BTreeMap::new();
for member_pkg in ws.members() {
build_resolve_graph_r(
&mut node_map,
Expand All @@ -154,21 +154,22 @@ fn build_resolve_graph(
}
// Get a Vec of Packages.
let actual_packages = package_map
.drain()
.into_iter()
.filter_map(|(pkg_id, pkg)| node_map.get(&pkg_id).map(|_| pkg))
.collect();

let mr = MetadataResolve {
nodes: node_map.drain().map(|(_pkg_id, node)| node).collect(),
nodes: node_map.into_iter().map(|(_pkg_id, node)| node).collect(),
root: ws.current_opt().map(|pkg| pkg.package_id()),
};
Ok((actual_packages, mr))
}

fn build_resolve_graph_r(
node_map: &mut HashMap<PackageId, MetadataResolveNode>,
node_map: &mut BTreeMap<PackageId, MetadataResolveNode>,
pkg_id: PackageId,
resolve: &Resolve,
package_map: &HashMap<PackageId, Package>,
package_map: &BTreeMap<PackageId, Package>,
target_data: &RustcTargetData,
requested_kinds: &[CompileKind],
) {
Expand All @@ -190,7 +191,8 @@ fn build_resolve_graph_r(
}
})
.filter_map(|(dep_id, deps)| {
let dep_kinds: Vec<_> = deps.iter().map(DepKindInfo::from).collect();
let mut dep_kinds: Vec<_> = deps.iter().map(DepKindInfo::from).collect();
dep_kinds.sort();
package_map
.get(&dep_id)
.and_then(|pkg| pkg.targets().iter().find(|t| t.is_lib()))
Expand Down
138 changes: 69 additions & 69 deletions tests/testsuite/alt_registry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,41 @@ fn alt_reg_metadata() {
r#"
{
"packages": [
{
"name": "altdep",
"version": "0.0.1",
"id": "altdep 0.0.1 (registry+file:[..]/alternative-registry)",
"license": null,
"license_file": null,
"description": null,
"source": "registry+file:[..]/alternative-registry",
"dependencies": [
{
"name": "bar",
"source": "registry+https://github.com/rust-lang/crates.io-index",
"req": "^0.0.1",
"kind": null,
"rename": null,
"optional": false,
"uses_default_features": true,
"features": [],
"target": null,
"registry": null
}
],
"targets": "{...}",
"features": {},
"manifest_path": "[..]/altdep-0.0.1/Cargo.toml",
"metadata": null,
"publish": null,
"authors": [],
"categories": [],
"keywords": [],
"readme": null,
"repository": null,
"edition": "2015",
"links": null
},
{
"name": "altdep2",
"version": "0.0.1",
Expand All @@ -859,30 +894,17 @@ fn alt_reg_metadata() {
"links": null
},
{
"name": "altdep",
"name": "bar",
"version": "0.0.1",
"id": "altdep 0.0.1 (registry+file:[..]/alternative-registry)",
"id": "bar 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"license": null,
"license_file": null,
"description": null,
"source": "registry+file:[..]/alternative-registry",
"dependencies": [
{
"name": "bar",
"source": "registry+https://github.com/rust-lang/crates.io-index",
"req": "^0.0.1",
"kind": null,
"rename": null,
"optional": false,
"uses_default_features": true,
"features": [],
"target": null,
"registry": null
}
],
"source": "registry+https://github.com/rust-lang/crates.io-index",
"dependencies": [],
"targets": "{...}",
"features": {},
"manifest_path": "[..]/altdep-0.0.1/Cargo.toml",
"manifest_path": "[..]/bar-0.0.1/Cargo.toml",
"metadata": null,
"publish": null,
"authors": [],
Expand Down Expand Up @@ -974,28 +996,6 @@ fn alt_reg_metadata() {
"repository": null,
"edition": "2015",
"links": null
},
{
"name": "bar",
"version": "0.0.1",
"id": "bar 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"license": null,
"license_file": null,
"description": null,
"source": "registry+https://github.com/rust-lang/crates.io-index",
"dependencies": [],
"targets": "{...}",
"features": {},
"manifest_path": "[..]/bar-0.0.1/Cargo.toml",
"metadata": null,
"publish": null,
"authors": [],
"categories": [],
"keywords": [],
"readme": null,
"repository": null,
"edition": "2015",
"links": null
}
],
"workspace_members": [
Expand Down Expand Up @@ -1056,14 +1056,27 @@ fn unknown_registry() {
{
"packages": [
{
"name": "baz",
"name": "bar",
"version": "0.0.1",
"id": "baz 0.0.1 (registry+file://[..]/alternative-registry)",
"id": "bar 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"license": null,
"license_file": null,
"description": null,
"source": "registry+file://[..]/alternative-registry",
"dependencies": [],
"source": "registry+https://github.com/rust-lang/crates.io-index",
"dependencies": [
{
"name": "baz",
"source": "registry+file://[..]/alternative-registry",
"req": "^0.0.1",
"kind": null,
"rename": null,
"optional": false,
"uses_default_features": true,
"features": [],
"target": null,
"registry": "file:[..]/alternative-registry"
}
],
"targets": "{...}",
"features": {},
"manifest_path": "[..]",
Expand All @@ -1078,30 +1091,17 @@ fn unknown_registry() {
"links": null
},
{
"name": "foo",
"name": "baz",
"version": "0.0.1",
"id": "foo 0.0.1 (path+file://[..]/foo)",
"id": "baz 0.0.1 (registry+file://[..]/alternative-registry)",
"license": null,
"license_file": null,
"description": null,
"source": null,
"dependencies": [
{
"name": "bar",
"source": "registry+https://github.com/rust-lang/crates.io-index",
"req": "^0.0.1",
"kind": null,
"rename": null,
"optional": false,
"uses_default_features": true,
"features": [],
"target": null,
"registry": null
}
],
"source": "registry+file://[..]/alternative-registry",
"dependencies": [],
"targets": "{...}",
"features": {},
"manifest_path": "[..]/foo/Cargo.toml",
"manifest_path": "[..]",
"metadata": null,
"publish": null,
"authors": [],
Expand All @@ -1113,30 +1113,30 @@ fn unknown_registry() {
"links": null
},
{
"name": "bar",
"name": "foo",
"version": "0.0.1",
"id": "bar 0.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"id": "foo 0.0.1 (path+file://[..]/foo)",
"license": null,
"license_file": null,
"description": null,
"source": "registry+https://github.com/rust-lang/crates.io-index",
"source": null,
"dependencies": [
{
"name": "baz",
"source": "registry+file://[..]/alternative-registry",
"name": "bar",
"source": "registry+https://github.com/rust-lang/crates.io-index",
"req": "^0.0.1",
"kind": null,
"rename": null,
"optional": false,
"uses_default_features": true,
"features": [],
"target": null,
"registry": "file:[..]/alternative-registry"
"registry": null
}
],
"targets": "{...}",
"features": {},
"manifest_path": "[..]",
"manifest_path": "[..]/foo/Cargo.toml",
"metadata": null,
"publish": null,
"authors": [],
Expand Down
Loading

0 comments on commit e37b35c

Please sign in to comment.