fix(dist/manifest): sort keys when serializing Manifest

Author: rami3l

src/dist/manifest.rs

@@ -12,11 +12,12 @@
 //!
 //! Docs: <https://forge.rust-lang.org/infra/channel-layout.html>
 
-use std::collections::HashMap;
-use std::collections::hash_map::Entry;
-use std::fmt;
-use std::hash::{Hash, Hasher};
-use std::str::FromStr;
+use std::{
+    collections::{BTreeMap, btree_map::Entry},
+    fmt,
+    hash::{Hash, Hasher},
+    str::FromStr,
+};
 
 use anyhow::{Context, Result, anyhow, bail};
 use serde::{Deserialize, Serialize};
@@ -41,13 +42,13 @@ pub struct Manifest {
     pub(crate) manifest_version: ManifestVersion,
     pub date: String,
     #[serde(default, rename = "pkg")]
-    pub packages: HashMap<String, Package>,
+    pub packages: BTreeMap<String, Package>,
     #[serde(default)]
-    pub renames: HashMap<String, Renamed>,
+    pub renames: BTreeMap<String, Renamed>,
     #[serde(default, skip_serializing)]
-    pub reverse_renames: HashMap<String, String>,
+    pub reverse_renames: BTreeMap<String, String>,
     #[serde(default)]
-    pub profiles: HashMap<Profile, Vec<String>>,
+    pub profiles: BTreeMap<Profile, Vec<String>>,
 }
 
 impl Manifest {
@@ -94,12 +95,12 @@ pub struct Package {
 #[serde(from = "TargetsMap", into = "TargetsMap")]
 pub enum PackageTargets {
     Wildcard(TargetedPackage),
-    Targeted(HashMap<TargetTriple, TargetedPackage>),
+    Targeted(BTreeMap<TargetTriple, TargetedPackage>),
 }
 
 #[derive(Deserialize, Serialize)]
 #[serde(transparent)]
-struct TargetsMap(HashMap<TargetTriple, TargetedPackage>);
+struct TargetsMap(BTreeMap<TargetTriple, TargetedPackage>);
 
 impl From<TargetsMap> for PackageTargets {
     fn from(mut map: TargetsMap) -> Self {
@@ -115,7 +116,7 @@ impl From<PackageTargets> for TargetsMap {
     fn from(targets: PackageTargets) -> Self {
         match targets {
             PackageTargets::Wildcard(tpkg) => {
-                let mut map = HashMap::new();
+                let mut map = BTreeMap::new();
                 map.insert(TargetTriple::new("*"), tpkg);
                 Self(map)
             }
@@ -639,6 +640,9 @@ mod tests {
 
     // Example manifest from https://public.etherpad-mozilla.org/p/Rust-infra-work-week
     static EXAMPLE: &str = include_str!("manifest/tests/channel-rust-nightly-example.toml");
+    // Same manifest as above, but with the packages in a different order.
+    static EXAMPLE_REORDERED: &str =
+        include_str!("manifest/tests/channel-rust-nightly-example.toml");
     // From brson's live build-rust-manifest.py script
     static EXAMPLE2: &str = include_str!("manifest/tests/channel-rust-nightly-example2.toml");
 
@@ -742,4 +746,13 @@ date = "2015-10-10"
 
         assert!(Manifest::parse(&manifest).is_ok());
     }
+
+    // #4715
+    #[test]
+    fn manifest_serialized_with_sorted_keys() -> anyhow::Result<()> {
+        let manifest = Manifest::parse(EXAMPLE)?;
+        let manifest_reordered = Manifest::parse(EXAMPLE_REORDERED)?;
+        assert_eq!(manifest.stringify()?, manifest_reordered.stringify()?);
+        Ok(())
+    }
 }

src/dist/manifest/tests/channel-rust-nightly-example-reordered.toml

@@ -0,0 +1,59 @@
+manifest-version = "2"
+date = "2015-10-10"
+[pkg.rustc]
+version = "rustc 1.3.0 (9a92aaf19 2015-09-15)"
+[pkg.cargo.target.x86_64-unknown-linux-gnu]
+available = true
+url = "example.com"
+hash = "..."
+[pkg.rustc.target.x86_64-unknown-linux-gnu]
+available = true
+url = "example.com"
+hash = "..."
+[pkg.cargo]
+version = "cargo 0.4.0-nightly (553b363 2015-08-03)"
+[pkg.rust-std]
+version = "rustc 1.3.0 (9a92aaf19 2015-09-15)"
+[pkg.rust-std.target.x86_64-unknown-linux-gnu]
+available = true
+url = "example.com"
+hash = "..."
+[pkg.rust-std.target.x86_64-unknown-linux-musl]
+available = true
+url = "example.com"
+hash = "..."
+[pkg.rust-std.target.i686-unknown-linux-gnu]
+available = true
+url = "example.com"
+hash = "..."
+[pkg.rust-docs]
+version = "rustc 1.3.0 (9a92aaf19 2015-09-15)"
+[pkg.rust-docs.target.x86_64-unknown-linux-gnu]
+available = true
+hash = "..."
+url = "example.com"
+[pkg.rust]
+version = "rustc 1.3.0 (9a92aaf19 2015-09-15)"
+[pkg.rust.target.x86_64-unknown-linux-gnu]
+available = true
+url = "example.com"
+hash = "..."
+[[pkg.rust.target.x86_64-unknown-linux-gnu.components]]
+pkg = "rustc"
+target = "x86_64-unknown-linux-gnu"
+[[pkg.rust.target.x86_64-unknown-linux-gnu.components]]
+pkg = "rust-docs"
+target = "x86_64-unknown-linux-gnu"
+[[pkg.rust.target.x86_64-unknown-linux-gnu.components]]
+pkg = "cargo"
+target = "x86_64-unknown-linux-gnu"
+[[pkg.rust.target.x86_64-unknown-linux-gnu.components]]
+pkg = "rust-std"
+target = "x86_64-unknown-linux-gnu"
+# extensions are rust-std or rust-docs that aren't in the rust tarball's component list
+[[pkg.rust.target.x86_64-unknown-linux-gnu.extensions]]
+pkg = "rust-std"
+target = "x86_64-unknown-linux-musl"
+[[pkg.rust.target.x86_64-unknown-linux-gnu.extensions]]
+pkg = "rust-std"
+target = "i686-unknown-linux-gnu"

src/dist/mod.rs

@@ -725,7 +725,9 @@ impl TryFrom<&ToolchainName> for ToolchainDesc {
     }
 }
 
-#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, Hash, PartialEq, Serialize)]
+#[derive(
+    Clone, Copy, Debug, Default, Deserialize, Eq, Hash, PartialEq, Serialize, PartialOrd, Ord,
+)]
 #[serde(rename_all = "kebab-case")]
 pub enum Profile {
     Minimal,

src/test/dist.rs

@@ -1,7 +1,7 @@
 //! Tools for building and working with the filesystem of a mock Rust
 //! distribution server, with v1 and v2 manifests.
 
-use std::collections::HashMap;
+use std::collections::{BTreeMap, HashMap};
 use std::fs::{self, File};
 use std::io::{self, Read, Write};
 use std::path::{Path, PathBuf};
@@ -771,15 +771,15 @@ impl MockDistServer {
         let mut manifest = Manifest {
             manifest_version: ManifestVersion::V2,
             date: channel.date.clone(),
-            renames: HashMap::default(),
-            packages: HashMap::default(),
-            reverse_renames: HashMap::default(),
-            profiles: HashMap::default(),
+            renames: BTreeMap::default(),
+            packages: BTreeMap::default(),
+            reverse_renames: BTreeMap::default(),
+            profiles: BTreeMap::default(),
         };
 
         // [pkg.*]
         for package in &channel.packages {
-            let mut targets = HashMap::default();
+            let mut targets = BTreeMap::default();
 
             // [pkg.*.target.*]
             for target in &package.targets {