Sort crate dependencies before adding to index (#6317)

Also ensure that the sorting of dependencies uses all fields.

Author: arlosi

cargo-registry-index/lib.rs

@@ -175,7 +175,26 @@ impl Ord for Dependency {
         // misinterpreted. With this manual `Ord` implementation we ensure that
         // `normal` dependencies are always first when multiple with the same
         // `name` exist.
-        (&self.name, self.kind, &self.req).cmp(&(&other.name, other.kind, &other.req))
+        (
+            &self.name,
+            self.kind,
+            &self.req,
+            self.optional,
+            self.default_features,
+            &self.target,
+            &self.package,
+            &self.features,
+        )
+            .cmp(&(
+                &other.name,
+                other.kind,
+                &other.req,
+                other.optional,
+                other.default_features,
+                &other.target,
+                &other.package,
+                &other.features,
+            ))
     }
 }
 

src/controllers/krate/publish.rs

@@ -409,8 +409,9 @@ pub fn add_dependencies(
         })
         .collect::<Result<Vec<_>, _>>()?;
 
-    let (git_deps, new_dependencies): (Vec<_>, Vec<_>) =
+    let (mut git_deps, new_dependencies): (Vec<_>, Vec<_>) =
         git_and_new_dependencies.into_iter().unzip();
+    git_deps.sort();
 
     insert_into(dependencies)
         .values(&new_dependencies)

src/tests/http-data/krate_publish_new_krate_sorts_deps.json

@@ -0,0 +1,62 @@
+[
+  {
+    "request": {
+      "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/two-deps/two-deps-1.0.0.crate",
+      "method": "PUT",
+      "headers": [
+        [
+          "accept",
+          "*/*"
+        ],
+        [
+          "accept-encoding",
+          "gzip"
+        ],
+        [
+          "content-length",
+          "35"
+        ],
+        [
+          "content-type",
+          "application/gzip"
+        ]
+      ],
+      "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA="
+    },
+    "response": {
+      "status": 200,
+      "headers": [],
+      "body": ""
+    }
+  },
+  {
+    "request": {
+      "uri": "http://alexcrichton-test.s3.amazonaws.com/tw/o-/two-deps",
+      "method": "PUT",
+      "headers": [
+        [
+          "accept",
+          "*/*"
+        ],
+        [
+          "accept-encoding",
+          "gzip"
+        ],
+        [
+          "content-length",
+          "376"
+        ],
+        [
+          "content-type",
+          "text/plain"
+        ]
+      ],
+      "body": "eyJuYW1lIjoidHdvLWRlcHMiLCJ2ZXJzIjoiMS4wLjAiLCJkZXBzIjpbeyJuYW1lIjoiZGVwLWEiLCJyZXEiOiI+IDAiLCJmZWF0dXJlcyI6W10sIm9wdGlvbmFsIjpmYWxzZSwiZGVmYXVsdF9mZWF0dXJlcyI6dHJ1ZSwidGFyZ2V0IjpudWxsLCJraW5kIjoibm9ybWFsIn0seyJuYW1lIjoiZGVwLWIiLCJyZXEiOiI+IDAiLCJmZWF0dXJlcyI6W10sIm9wdGlvbmFsIjpmYWxzZSwiZGVmYXVsdF9mZWF0dXJlcyI6dHJ1ZSwidGFyZ2V0IjpudWxsLCJraW5kIjoibm9ybWFsIn1dLCJja3N1bSI6ImFjYjU2MDRiMTI2YWM4OTRjMWViMTFjNDU3NWJmMjA3MmZlYTYxMjMyYTg4OGU0NTM3NzBjNzlkN2VkNTY0MTkiLCJmZWF0dXJlcyI6e30sInlhbmtlZCI6ZmFsc2V9Cg=="
+    },
+    "response": {
+      "status": 200,
+      "headers": [],
+      "body": ""
+    }
+  }
+]

src/tests/krate/publish.rs

@@ -995,3 +995,31 @@ fn features_version_2() {
     )]);
     assert_eq!(crates[0].features2, Some(features2));
 }
+
+#[test]
+fn new_krate_sorts_deps() {
+    let (app, _, user, token) = TestApp::full().with_token();
+
+    app.db(|conn| {
+        // Insert crates directly into the database so that two-deps can depend on it
+        CrateBuilder::new("dep-a", user.as_model().id).expect_build(conn);
+        CrateBuilder::new("dep-b", user.as_model().id).expect_build(conn);
+    });
+
+    let dep_a = DependencyBuilder::new("dep-a");
+    let dep_b = DependencyBuilder::new("dep-b");
+
+    // Add the deps in reverse order to ensure they get sorted.
+    let crate_to_publish = PublishBuilder::new("two-deps")
+        .version("1.0.0")
+        .dependency(dep_b)
+        .dependency(dep_a);
+    token.publish_crate(crate_to_publish).good();
+
+    let crates = app.crates_from_index_head("two-deps");
+    assert!(crates.len() == 1);
+    let deps = &crates[0].deps;
+    assert!(deps.len() == 2);
+    assert_eq!(deps[0].name, "dep-a");
+    assert_eq!(deps[1].name, "dep-b");
+}