Make manifest serialization deterministic

Fixes #4326

`cargo package` (and so `cargo publish`) parses a crate’s `Cargo.toml`,
makes some modifications, and re-serializes it.
Because the `TomlManifest` struct uses `HashMap`
with its default `RandomState` hasher,
the maps’ iteration order changed on every run.

As a result, when using `cargo vendor`,
updating a dependency would generate a diff larger than necessary,
with non-significant order-changes obscuring significant changes.

This replaces some uses of `HashMap` with `BTreeMap`,
whose iteration order is deterministic (based on `Ord`).

diff --git a/Cargo.lock b/Cargo.lock
index 2feeeafad8556aeb13f6212f5afdba12bdec2f12..9314ace80d87e998a7fe888e21b68fd59dd80599 100644 (file)
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2,13 +2,13 @@
 name = "cargo"
 version = "0.23.0"
 dependencies = [
- "advapi32-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "atty 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "bufstream 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "cargotest 0.1.0",
  "core-foundation 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "crates-io 0.12.0",
  "crossbeam 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "crypto-hash 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "curl 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "docopt 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -30,7 +30,6 @@ dependencies = [
  "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "miow 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "num_cpus 1.6.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "openssl 0.9.19 (registry+https://github.com/rust-lang/crates.io-index)",
  "psapi-sys 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "same-file 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "scoped-tls 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -155,6 +154,22 @@ dependencies = [
  "cc 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "commoncrypto"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "commoncrypto-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
+[[package]]
+name = "commoncrypto-sys"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "libc 0.2.31 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "conv"
 version = "0.3.3"
@@ -202,6 +217,18 @@ name = "crossbeam"
 version = "0.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "crypto-hash"
+version = "0.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "advapi32-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "commoncrypto 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "hex 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "openssl 0.9.19 (registry+https://github.com/rust-lang/crates.io-index)",
+ "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "curl"
 version = "0.4.8"
@@ -1028,11 +1055,14 @@ dependencies = [
 "checksum cc 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7db2f146208d7e0fbee761b09cd65a7f51ccc38705d4e7262dad4d73b12a76b1"
 "checksum cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d4c819a1287eb618df47cc647173c5c4c66ba19d888a6e50d605672aed3140de"
 "checksum cmake 0.1.26 (registry+https://github.com/rust-lang/crates.io-index)" = "357c07e7a1fc95732793c1edb5901e1a1f305cfcf63a90eb12dbd22bdb6b789d"
+"checksum commoncrypto 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d056a8586ba25a1e4d61cb090900e495952c7886786fc55f909ab2f819b69007"
+"checksum commoncrypto-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1fed34f46747aa73dfaa578069fd8279d2818ade2b55f38f22a9401c7f4083e2"
 "checksum conv 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "78ff10625fd0ac447827aa30ea8b861fead473bb60aeb73af6c1c58caf0d1299"
 "checksum core-foundation 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "5909502e547762013619f4c4e01cc7393c20fe2d52d7fa471c1210adb2320dc7"
 "checksum core-foundation-sys 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "bc9fb3d6cb663e6fd7cf1c63f9b144ee2b1e4a78595a0451dd34bff85b9a3387"
 "checksum crossbeam 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)" = "0c5ea215664ca264da8a9d9c3be80d2eaf30923c259d03e870388eb927508f97"
 "checksum crossbeam 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8837ab96533202c5b610ed44bc7f4183e7957c1c8f56e8cc78bb098593c8ba0a"
+"checksum crypto-hash 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "34903878eec1694faf53cae8473a088df333181de421d4d3d48061d6559fe602"
 "checksum curl 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)" = "7034c534a1d7d22f7971d6088aa9d281d219ef724026c3428092500f41ae9c2c"
 "checksum curl-sys 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)" = "4bee31aa3a079d5f3ff9579ea4dcfb1b1a17a40886f5f467436d383e78134b55"
 "checksum custom_derive 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "ef8ae57c4978a2acd8b869ce6b9ca1dfe817bff704c220209fdef2c0b75a01b9"

diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs
index 06f66a6b08ec53ec46174f6961bd7fe8c82f42f7..d8c3710ed43074d370fd57ad8ae688789ee31f41 100644 (file)
--- a/src/cargo/core/manifest.rs
+++ b/src/cargo/core/manifest.rs
@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, BTreeMap};
 use std::fmt;
 use std::path::{PathBuf, Path};
 use std::rc::Rc;
@@ -75,7 +75,7 @@ pub struct ManifestMetadata {
     pub homepage: Option<String>,       // url
     pub repository: Option<String>,     // url
     pub documentation: Option<String>,  // url
-    pub badges: HashMap<String, HashMap<String, String>>,
+    pub badges: BTreeMap<String, BTreeMap<String, String>>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]

diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs
index afc047f88f6e3538894ae0d53719493b8a627e9a..dc54497ec114fed84fc532273a5a107817df088a 100644 (file)
--- a/src/cargo/core/package.rs
+++ b/src/cargo/core/package.rs
@@ -1,5 +1,5 @@
 use std::cell::{Ref, RefCell};
-use std::collections::HashMap;
+use std::collections::{HashMap, BTreeMap};
 use std::fmt;
 use std::hash;
 use std::path::{Path, PathBuf};
@@ -37,7 +37,7 @@ struct SerializedPackage<'a> {
     source: &'a SourceId,
     dependencies: &'a [Dependency],
     targets: &'a [Target],
-    features: &'a HashMap<String, Vec<String>>,
+    features: &'a BTreeMap<String, Vec<String>>,
     manifest_path: &'a str,
 }
 

diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs
index 9902311e66ba38989bf3327b2ffd847d420ab77f..734f73bd63b6f42ec67e2e74b32600900214779d 100644 (file)
--- a/src/cargo/core/summary.rs
+++ b/src/cargo/core/summary.rs
@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::mem;
 use std::rc::Rc;
 
@@ -20,14 +20,14 @@ pub struct Summary {
 struct Inner {
     package_id: PackageId,
     dependencies: Vec<Dependency>,
-    features: HashMap<String, Vec<String>>,
+    features: BTreeMap<String, Vec<String>>,
     checksum: Option<String>,
 }
 
 impl Summary {
     pub fn new(pkg_id: PackageId,
                dependencies: Vec<Dependency>,
-               features: HashMap<String, Vec<String>>) -> CargoResult<Summary> {
+               features: BTreeMap<String, Vec<String>>) -> CargoResult<Summary> {
         for dep in dependencies.iter() {
             if features.get(dep.name()).is_some() {
                 bail!("Features and dependencies cannot have the \
@@ -78,7 +78,7 @@ impl Summary {
     pub fn version(&self) -> &Version { self.package_id().version() }
     pub fn source_id(&self) -> &SourceId { self.package_id().source_id() }
     pub fn dependencies(&self) -> &[Dependency] { &self.inner.dependencies }
-    pub fn features(&self) -> &HashMap<String, Vec<String>> { &self.inner.features }
+    pub fn features(&self) -> &BTreeMap<String, Vec<String>> { &self.inner.features }
     pub fn checksum(&self) -> Option<&str> {
         self.inner.checksum.as_ref().map(|s| &s[..])
     }

diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs
index 187a64dbef6218f7588e682750d9881c88a44f48..c967e2ebcd3a824fa8a9834af3628361b0dd061f 100644 (file)
--- a/src/cargo/sources/registry/mod.rs
+++ b/src/cargo/sources/registry/mod.rs
@@ -159,7 +159,7 @@
 //! ```
 
 use std::borrow::Cow;
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::fmt;
 use std::fs::File;
 use std::path::{PathBuf, Path};
@@ -206,7 +206,7 @@ struct RegistryPackage<'a> {
     name: Cow<'a, str>,
     vers: Version,
     deps: DependencyList,
-    features: HashMap<String, Vec<String>>,
+    features: BTreeMap<String, Vec<String>>,
     cksum: String,
     yanked: Option<bool>,
 }

diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs
index e31d4527590663e1b487d122bb96246c430a396e..c2220ac2d6e7254c219739276b9d474769d8cb3f 100644 (file)
--- a/src/cargo/util/toml/mod.rs
+++ b/src/cargo/util/toml/mod.rs
@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet, BTreeSet};
+use std::collections::{HashMap, BTreeMap, HashSet, BTreeSet};
 use std::fmt;
 use std::fs;
 use std::path::{Path, PathBuf};
@@ -207,21 +207,21 @@ pub struct TomlManifest {
     example: Option<Vec<TomlExampleTarget>>,
     test: Option<Vec<TomlTestTarget>>,
     bench: Option<Vec<TomlTestTarget>>,
-    dependencies: Option<HashMap<String, TomlDependency>>,
+    dependencies: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "dev-dependencies")]
-    dev_dependencies: Option<HashMap<String, TomlDependency>>,
+    dev_dependencies: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "dev_dependencies")]
-    dev_dependencies2: Option<HashMap<String, TomlDependency>>,
+    dev_dependencies2: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "build-dependencies")]
-    build_dependencies: Option<HashMap<String, TomlDependency>>,
+    build_dependencies: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "build_dependencies")]
-    build_dependencies2: Option<HashMap<String, TomlDependency>>,
-    features: Option<HashMap<String, Vec<String>>>,
-    target: Option<HashMap<String, TomlPlatform>>,
-    replace: Option<HashMap<String, TomlDependency>>,
-    patch: Option<HashMap<String, HashMap<String, TomlDependency>>>,
+    build_dependencies2: Option<BTreeMap<String, TomlDependency>>,
+    features: Option<BTreeMap<String, Vec<String>>>,
+    target: Option<BTreeMap<String, TomlPlatform>>,
+    replace: Option<BTreeMap<String, TomlDependency>>,
+    patch: Option<BTreeMap<String, BTreeMap<String, TomlDependency>>>,
     workspace: Option<TomlWorkspace>,
-    badges: Option<HashMap<String, HashMap<String, String>>>,
+    badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
     #[serde(rename = "cargo-features")]
     cargo_features: Option<Vec<String>>,
 }
@@ -475,8 +475,8 @@ impl TomlManifest {
             cargo_features: self.cargo_features.clone(),
         };
 
-        fn map_deps(deps: Option<&HashMap<String, TomlDependency>>)
-                        -> Option<HashMap<String, TomlDependency>>
+        fn map_deps(deps: Option<&BTreeMap<String, TomlDependency>>)
+                        -> Option<BTreeMap<String, TomlDependency>>
         {
             let deps = match deps {
                 Some(deps) => deps,
@@ -557,7 +557,7 @@ impl TomlManifest {
 
             fn process_dependencies(
                 cx: &mut Context,
-                new_deps: Option<&HashMap<String, TomlDependency>>,
+                new_deps: Option<&BTreeMap<String, TomlDependency>>,
                 kind: Option<Kind>)
                 -> CargoResult<()>
             {
@@ -600,7 +600,7 @@ impl TomlManifest {
         }
 
         {
-            let mut names_sources = HashMap::new();
+            let mut names_sources = BTreeMap::new();
             for dep in &deps {
                 let name = dep.name();
                 let prev = names_sources.insert(name, dep.source_id());
@@ -616,7 +616,7 @@ impl TomlManifest {
         let include = project.include.clone().unwrap_or_default();
 
         let summary = Summary::new(pkgid, deps, me.features.clone()
-            .unwrap_or_else(HashMap::new))?;
+            .unwrap_or_else(BTreeMap::new))?;
         let metadata = ManifestMetadata {
             description: project.description.clone(),
             homepage: project.homepage.clone(),
@@ -985,15 +985,15 @@ impl ser::Serialize for PathValue {
 /// Corresponds to a `target` entry, but `TomlTarget` is already used.
 #[derive(Serialize, Deserialize, Debug)]
 struct TomlPlatform {
-    dependencies: Option<HashMap<String, TomlDependency>>,
+    dependencies: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "build-dependencies")]
-    build_dependencies: Option<HashMap<String, TomlDependency>>,
+    build_dependencies: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "build_dependencies")]
-    build_dependencies2: Option<HashMap<String, TomlDependency>>,
+    build_dependencies2: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "dev-dependencies")]
-    dev_dependencies: Option<HashMap<String, TomlDependency>>,
+    dev_dependencies: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "dev_dependencies")]
-    dev_dependencies2: Option<HashMap<String, TomlDependency>>,
+    dev_dependencies2: Option<BTreeMap<String, TomlDependency>>,
 }
 
 impl TomlTarget {

diff --git a/src/crates-io/lib.rs b/src/crates-io/lib.rs
index 6caa46cf716fea830130b18a1f2023eec8f4ec38..910d51a3730d25506403d3ac413abadb3ecf7e13 100644 (file)
--- a/src/crates-io/lib.rs
+++ b/src/crates-io/lib.rs
@@ -8,7 +8,7 @@ extern crate serde_json;
 #[macro_use]
 extern crate serde_derive;
 
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::fs::File;
 use std::io::prelude::*;
 use std::io::{self, Cursor};
@@ -76,7 +76,7 @@ pub struct NewCrate {
     pub name: String,
     pub vers: String,
     pub deps: Vec<NewCrateDependency>,
-    pub features: HashMap<String, Vec<String>>,
+    pub features: BTreeMap<String, Vec<String>>,
     pub authors: Vec<String>,
     pub description: Option<String>,
     pub documentation: Option<String>,
@@ -87,7 +87,7 @@ pub struct NewCrate {
     pub license: Option<String>,
     pub license_file: Option<String>,
     pub repository: Option<String>,
-    pub badges: HashMap<String, HashMap<String, String>>,
+    pub badges: BTreeMap<String, BTreeMap<String, String>>,
 }
 
 #[derive(Serialize)]

diff --git a/tests/package.rs b/tests/package.rs
index d827ffd866eb65e0d6b286b3be8b2b12299287f5..724e1ff26c48132e312746ac3cdb0a3f89a28928 100644 (file)
--- a/tests/package.rs
+++ b/tests/package.rs
@@ -700,6 +700,9 @@ to proceed despite this, pass the `--allow-dirty` flag
 
 #[test]
 fn generated_manifest() {
+    Package::new("abc", "1.0.0").publish();
+    Package::new("def", "1.0.0").publish();
+    Package::new("ghi", "1.0.0").publish();
     let p = project("foo")
         .file("Cargo.toml", r#"
             [project]
@@ -717,6 +720,9 @@ fn generated_manifest() {
 
             [dependencies]
             bar = { path = "bar", version = "0.1" }
+            def = "1.0"
+            ghi = "1.0"
+            abc = "1.0"
         "#)
         .file("src/main.rs", "")
         .file("bar/Cargo.toml", r#"
@@ -741,6 +747,8 @@ fn generated_manifest() {
                         .unwrap();
     let mut contents = String::new();
     entry.read_to_string(&mut contents).unwrap();
+    // BTreeMap makes the order of dependencies in the generated file deterministic
+    // by sorting alphabetically
     assert_that(&contents[..], equal_to(
 r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
 #
@@ -764,8 +772,17 @@ license = "MIT"
 
 [package.metadata]
 foo = "bar"
+[dependencies.abc]
+version = "1.0"
+
 [dependencies.bar]
 version = "0.1"
+
+[dependencies.def]
+version = "1.0"
+
+[dependencies.ghi]
+version = "1.0"
 "#));
 }
 

diff --git a/tests/resolve.rs b/tests/resolve.rs
index 03fa38eb6e326bad75c99f21258ca99c6ec59fa7..2b84106a657ac3b9289a7843bfcbada6c8a4673d 100644 (file)
--- a/tests/resolve.rs
+++ b/tests/resolve.rs
@@ -3,7 +3,7 @@
 extern crate hamcrest;
 extern crate cargo;
 
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 
 use hamcrest::{assert_that, equal_to, contains, not};
 
@@ -32,7 +32,7 @@ fn resolve(pkg: &PackageId, deps: Vec<Dependency>, registry: &[Summary])
         fn requires_precise(&self) -> bool { false }
     }
     let mut registry = MyRegistry(registry);
-    let summary = Summary::new(pkg.clone(), deps, HashMap::new()).unwrap();
+    let summary = Summary::new(pkg.clone(), deps, BTreeMap::new()).unwrap();
     let method = Method::Everything;
     let resolve = resolver::resolve(&[(summary, method)], &[], &mut registry, None)?;
     let res = resolve.iter().cloned().collect();
@@ -78,11 +78,11 @@ macro_rules! pkg {
     ($pkgid:expr => [$($deps:expr),+]) => ({
         let d: Vec<Dependency> = vec![$($deps.to_dep()),+];
 
-        Summary::new($pkgid.to_pkgid(), d, HashMap::new()).unwrap()
+        Summary::new($pkgid.to_pkgid(), d, BTreeMap::new()).unwrap()
     });
 
     ($pkgid:expr) => (
-        Summary::new($pkgid.to_pkgid(), Vec::new(), HashMap::new()).unwrap()
+        Summary::new($pkgid.to_pkgid(), Vec::new(), BTreeMap::new()).unwrap()
     )
 }
 
@@ -92,7 +92,7 @@ fn registry_loc() -> SourceId {
 }
 
 fn pkg(name: &str) -> Summary {
-    Summary::new(pkg_id(name), Vec::new(), HashMap::new()).unwrap()
+    Summary::new(pkg_id(name), Vec::new(), BTreeMap::new()).unwrap()
 }
 
 fn pkg_id(name: &str) -> PackageId {
@@ -108,7 +108,7 @@ fn pkg_id_loc(name: &str, loc: &str) -> PackageId {
 }
 
 fn pkg_loc(name: &str, loc: &str) -> Summary {
-    Summary::new(pkg_id_loc(name, loc), Vec::new(), HashMap::new()).unwrap()
+    Summary::new(pkg_id_loc(name, loc), Vec::new(), BTreeMap::new()).unwrap()
 }
 
 fn dep(name: &str) -> Dependency { dep_req(name, "1.0.0") }