fix: sort npm packages in snapshot area for more determinism (#196)

denoland · Aug 19, 2024 · 2e134b4 · 2e134b4
1 parent b74b3c6
commit 2e134b4
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 53 deletions.
diff --git a/src/snapshots/eszip__v2__tests__npm_packages.snap b/src/snapshots/eszip__v2__tests__npm_packages.snap
@@ -138,7 +138,7 @@ expression: bytes
     0,
     0,
     0,
-    4,
+    3,
     0,
     0,
     0,
@@ -159,7 +159,7 @@ expression: bytes
     0,
     0,
     0,
-    0,
+    5,
     0,
     0,
     0,
@@ -178,53 +178,14 @@ expression: bytes
     0,
     0,
     0,
-    0,
+    5,
     0,
     0,
     0,
     158,
     0,
     0,
     0,
-    13,
-    112,
-    97,
-    99,
-    107,
-    97,
-    103,
-    101,
-    64,
-    49,
-    46,
-    50,
-    46,
-    50,
-    0,
-    0,
-    0,
-    2,
-    0,
-    0,
-    0,
-    1,
-    97,
-    0,
-    0,
-    0,
-    1,
-    0,
-    0,
-    0,
-    1,
-    98,
-    0,
-    0,
-    0,
-    2,
-    0,
-    0,
-    0,
     7,
     97,
     64,
@@ -277,7 +238,7 @@ expression: bytes
     0,
     0,
     0,
-    3,
+    2,
     0,
     0,
     0,
@@ -316,7 +277,7 @@ expression: bytes
     0,
     0,
     0,
-    5,
+    4,
     0,
     0,
     0,
@@ -340,7 +301,46 @@ expression: bytes
     0,
     0,
     0,
-    4,
+    3,
+    0,
+    0,
+    0,
+    13,
+    112,
+    97,
+    99,
+    107,
+    97,
+    103,
+    101,
+    64,
+    49,
+    46,
+    50,
+    46,
+    50,
+    0,
+    0,
+    0,
+    2,
+    0,
+    0,
+    0,
+    1,
+    97,
+    0,
+    0,
+    0,
+    0,
+    0,
+    0,
+    0,
+    1,
+    98,
+    0,
+    0,
+    0,
+    1,
     0,
     0,
     0,

diff --git a/src/v2.rs b/src/v2.rs
@@ -1156,7 +1156,8 @@ impl EszipV2 {
 
     // add npm snapshot entries to the header and fill the npm bytes
     if let Some(npm_snapshot) = self.npm_snapshot {
-      let npm_snapshot = npm_snapshot.into_serialized();
+      let mut npm_snapshot = npm_snapshot.into_serialized();
+      npm_snapshot.packages.sort_by(|a, b| a.id.cmp(&b.id)); // determinism
       let ids_to_eszip_ids = npm_snapshot
         .packages
         .iter()
@@ -1165,7 +1166,7 @@ impl EszipV2 {
         .collect::<HashMap<_, _>>();
 
       let mut root_packages: Vec<_> =
-        npm_snapshot.root_packages.into_iter().collect();
+        npm_snapshot.root_packages.iter().collect();
       root_packages.sort();
       for (req, id) in root_packages {
         append_string(&mut modules_header, &req.to_string());
@@ -1178,11 +1179,7 @@ impl EszipV2 {
         append_string(&mut npm_bytes, &pkg.id.as_serialized());
         let deps_len = pkg.dependencies.len() as u32;
         npm_bytes.extend_from_slice(&deps_len.to_be_bytes());
-        let mut deps: Vec<_> = pkg
-          .dependencies
-          .iter()
-          .map(|(a, b)| (a.clone(), b.clone()))
-          .collect();
+        let mut deps: Vec<_> = pkg.dependencies.iter().collect();
         deps.sort();
         for (req, id) in deps {
           append_string(&mut npm_bytes, &req.to_string());
@@ -2898,7 +2895,12 @@ mod tests {
         .await
         .unwrap();
     let snapshot = eszip.take_npm_snapshot().unwrap();
-    assert_eq!(snapshot.as_serialized(), original_snapshot.as_serialized());
+    assert_eq!(snapshot.into_serialized(), {
+      let mut original = original_snapshot.into_serialized();
+      // this will be sorted for determinism
+      original.packages.sort_by(|a, b| a.id.cmp(&b.id));
+      original
+    });
 
     // ensure the eszip still works otherwise
     fut.await.unwrap();