Merge pull request #603 from mystor/package_id_changes

Stop depending on PackageId format for node ordering
mozilla · Apr 2, 2024 · a248d43 · a248d43
2 parents 9bf5bbe + ab08e66
commit a248d43
Show file tree

Hide file tree

Showing 13 changed files with 67 additions and 171 deletions.
diff --git a/src/resolver.rs b/src/resolver.rs
@@ -180,9 +180,12 @@ pub type PackageIdx = usize;
 
 #[derive(Debug, Clone, Serialize)]
 pub struct PackageNode<'a> {
-    #[serde(skip_serializing_if = "pkgid_unstable")]
+    #[serde(skip)]
     /// The PackageId that cargo uses to uniquely identify this package
     ///
+    /// This ID is not guaranteed to be stable across cargo versions, so is not
+    /// serialized into graph JSON.
+    ///
     /// Prefer using a [`DepGraph`] and its memoized [`PackageIdx`]'s.
     pub package_id: &'a PackageId,
     /// The name of the package
@@ -211,11 +214,6 @@ pub struct PackageNode<'a> {
     pub is_dev_only: bool,
 }
 
-/// Don't serialize path package ids, not stable across systems
-fn pkgid_unstable(pkgid: &PackageId) -> bool {
-    pkgid.repr.contains("(path+file:/")
-}
-
 /// The dependency graph in a form we can use more easily.
 #[derive(Debug, Clone)]
 pub struct DepGraph<'a> {
@@ -449,9 +447,13 @@ impl<'a> DepGraph<'a> {
             });
         }
 
-        // Sort the nodes by package_id to make the graph more stable and to make
-        // anything sorted by package_idx to also be approximately sorted by name and version.
-        nodes.sort_by_key(|k| k.package_id);
+        // Sort the nodes by package name and version to make the graph as
+        // stable as possible.  We avoid sorting by the package_id if possible,
+        // as for some packages it may not be stable (e.g. file:///), and the
+        // package_id format can also vary between cargo versions.
+        nodes.sort_by(|a, b| {
+            (a.name, &a.version, &a.package_id).cmp(&(b.name, &b.version, &b.package_id))
+        });
 
         // Populate the interners based on the new ordering
         for (idx, node) in nodes.iter_mut().enumerate() {

diff --git a/src/tests/snapshots/cargo_vet__tests__vet__builtin-complex-full-audited.json.snap b/src/tests/snapshots/cargo_vet__tests__vet__builtin-complex-full-audited.json.snap
@@ -7,11 +7,11 @@ expression: json
   "vetted_fully": [
     {
       "name": "third-core",
-      "version": "10.0.0"
+      "version": "5.0.0"
     },
     {
       "name": "third-core",
-      "version": "5.0.0"
+      "version": "10.0.0"
     },
     {
       "name": "thirdA",

diff --git a/src/tests/snapshots/cargo_vet__tests__vet__builtin-complex-inited.json.snap b/src/tests/snapshots/cargo_vet__tests__vet__builtin-complex-inited.json.snap
@@ -9,11 +9,11 @@ expression: json
   "vetted_with_exemptions": [
     {
       "name": "third-core",
-      "version": "10.0.0"
+      "version": "5.0.0"
     },
     {
       "name": "third-core",
-      "version": "5.0.0"
+      "version": "10.0.0"
     },
     {
       "name": "thirdA",

diff --git a/src/tests/snapshots/cargo_vet__tests__vet__builtin-complex-minimal-audited.json.snap b/src/tests/snapshots/cargo_vet__tests__vet__builtin-complex-minimal-audited.json.snap
@@ -7,11 +7,11 @@ expression: json
   "vetted_fully": [
     {
       "name": "third-core",
-      "version": "10.0.0"
+      "version": "5.0.0"
     },
     {
       "name": "third-core",
-      "version": "5.0.0"
+      "version": "10.0.0"
     },
     {
       "name": "thirdA",

diff --git a/src/tests/snapshots/cargo_vet__tests__vet__builtin-complex-no-unaudited.json.snap b/src/tests/snapshots/cargo_vet__tests__vet__builtin-complex-no-unaudited.json.snap
@@ -7,14 +7,14 @@ expression: json
   "failures": [
     {
       "name": "third-core",
-      "version": "10.0.0",
+      "version": "5.0.0",
       "missing_criteria": [
         "safe-to-deploy"
       ]
     },
     {
       "name": "third-core",
-      "version": "5.0.0",
+      "version": "10.0.0",
       "missing_criteria": [
         "safe-to-deploy"
       ]

diff --git a/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-unaudited-twins.json.snap b/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-unaudited-twins.json.snap
@@ -18,11 +18,11 @@ expression: json
   "vetted_with_exemptions": [
     {
       "name": "third-core",
-      "version": "10.0.0"
+      "version": "5.0.0"
     },
     {
       "name": "third-core",
-      "version": "5.0.0"
+      "version": "10.0.0"
     }
   ]
 }
diff --git a/src/tests/snapshots/cargo_vet__tests__vet__mock-complex-full-audited.json.snap b/src/tests/snapshots/cargo_vet__tests__vet__mock-complex-full-audited.json.snap
@@ -7,11 +7,11 @@ expression: json
   "vetted_fully": [
     {
       "name": "third-core",
-      "version": "10.0.0"
+      "version": "5.0.0"
     },
     {
       "name": "third-core",
-      "version": "5.0.0"
+      "version": "10.0.0"
     },
     {
       "name": "thirdA",

diff --git a/src/tests/snapshots/cargo_vet__tests__vet__mock-complex-inited.json.snap b/src/tests/snapshots/cargo_vet__tests__vet__mock-complex-inited.json.snap
@@ -9,11 +9,11 @@ expression: json
   "vetted_with_exemptions": [
     {
       "name": "third-core",
-      "version": "10.0.0"
+      "version": "5.0.0"
     },
     {
       "name": "third-core",
-      "version": "5.0.0"
+      "version": "10.0.0"
     },
     {
       "name": "thirdA",

diff --git a/src/tests/snapshots/cargo_vet__tests__vet__mock-complex-no-unaudited.json.snap b/src/tests/snapshots/cargo_vet__tests__vet__mock-complex-no-unaudited.json.snap
@@ -7,14 +7,14 @@ expression: json
   "failures": [
     {
       "name": "third-core",
-      "version": "10.0.0",
+      "version": "5.0.0",
       "missing_criteria": [
         "reviewed"
       ]
     },
     {
       "name": "third-core",
-      "version": "5.0.0",
+      "version": "10.0.0",
       "missing_criteria": [
         "reviewed"
       ]