From b50f995b2aa78908f75e4f4540dad6f9c552de67 Mon Sep 17 00:00:00 2001
From: boxdot <d@zerovolt.org>
Date: Mon, 28 May 2018 19:02:42 +0200
Subject: [PATCH] Clean error msg when lockfile contains duplicate packages.

---
 src/cargo/ops/lockfile.rs            | 35 ++++++++++++++---
 tests/testsuite/generate_lockfile.rs | 58 +++++++++++++++++++++++++++-
 2 files changed, 87 insertions(+), 6 deletions(-)

diff --git a/src/cargo/ops/lockfile.rs b/src/cargo/ops/lockfile.rs
index d90e5f408..c3b648c2e 100644
--- a/src/cargo/ops/lockfile.rs
+++ b/src/cargo/ops/lockfile.rs
@@ -1,12 +1,13 @@
+use std::collections::HashSet;
 use std::io::prelude::*;
 
 use toml;
 
-use core::{resolver, Resolve, Workspace};
 use core::resolver::WorkspaceResolve;
-use util::Filesystem;
-use util::errors::{CargoResult, CargoResultExt};
+use core::{resolver, Resolve, Workspace};
+use util::errors::{CargoResult, CargoResultExt, Internal};
 use util::toml as cargo_toml;
+use util::Filesystem;
 
 pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult<Option<Resolve>> {
     if !ws.root().join("Cargo.lock").exists() {
@@ -25,11 +26,33 @@ pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult<Option<Resolve>> {
             let resolve: toml::Value = cargo_toml::parse(&s, f.path(), ws.config())?;
             let v: resolver::EncodableResolve = resolve.try_into()?;
             Ok(Some(v.into_resolve(ws)?))
-        })()
-            .chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
+        })().chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
     Ok(resolve)
 }
 
+fn duplicate_pkg_names(resolve: &Resolve) -> Vec<&'static str> {
+    let mut unique_names = HashSet::new();
+    let mut result = HashSet::new();
+    for pkg_id in resolve.iter() {
+        if !unique_names.insert(pkg_id.name()) {
+            result.insert(pkg_id.name().as_str());
+        }
+    }
+    result.into_iter().collect()
+}
+
+fn check_duplicate_pkg_names(resolve: &Resolve) -> Result<(), Internal> {
+    let names = duplicate_pkg_names(resolve);
+    if names.is_empty() {
+        Ok(())
+    } else {
+        Err(Internal::new(format_err!(
+            "dependencies contain duplicate package(s): {}",
+            names.join(", ")
+        )))
+    }
+}
+
 pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()> {
     // Load the original lockfile if it exists.
     let ws_root = Filesystem::new(ws.root().to_path_buf());
@@ -40,6 +63,8 @@ pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()>
         Ok(s)
     });
 
+    check_duplicate_pkg_names(resolve).chain_err(|| format!("failed to generate lock file"))?;
+
     let toml = toml::Value::try_from(WorkspaceResolve { ws, resolve }).unwrap();
 
     let mut out = String::new();
diff --git a/tests/testsuite/generate_lockfile.rs b/tests/testsuite/generate_lockfile.rs
index 6018904f0..398722a13 100644
--- a/tests/testsuite/generate_lockfile.rs
+++ b/tests/testsuite/generate_lockfile.rs
@@ -1,8 +1,8 @@
 use std::fs::{self, File};
 use std::io::prelude::*;
 
-use cargotest::support::{execs, project};
 use cargotest::support::registry::Package;
+use cargotest::support::{execs, project, ProjectBuilder, paths};
 use cargotest::ChannelChanger;
 use hamcrest::{assert_that, existing_file, is_not};
 
@@ -250,3 +250,59 @@ fn cargo_update_generate_lockfile() {
     assert_that(p.cargo("update"), execs().with_status(0).with_stdout(""));
     assert_that(&lockfile, existing_file());
 }
+
+#[test]
+fn duplicate_entries_in_lockfile() {
+    let _a = ProjectBuilder::new("a", paths::root().join("a"))
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "a"
+            authors = []
+            version = "0.0.1"
+
+            [dependencies]
+            common = {path="common"}
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    let common_toml = r#"
+        [package]
+        name = "common"
+        authors = []
+        version = "0.0.1"
+        "#;
+
+    let _common_in_a = ProjectBuilder::new("common", paths::root().join("a/common"))
+        .file("Cargo.toml", common_toml)
+        .file("src/lib.rs", "")
+        .build();
+
+    let b = ProjectBuilder::new("common", paths::root().join("b"))
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "b"
+            authors = []
+            version = "0.0.1"
+
+            [dependencies]
+            common = {path="common"}
+            a = {path="../a"}
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    let _common_in_b = ProjectBuilder::new("common", paths::root().join("b/common"))
+        .file("Cargo.toml", common_toml)
+        .file("src/lib.rs", "")
+        .build();
+
+    // should fail due to a duplicate package `common` in the lockfile
+    assert_that(b.cargo("build"), execs().with_status(101));
+}
-- 
2.30.2