From 37dbfd2ec6b8be57d2f26c2483f45294c9ddc3b7 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Fri, 26 Sep 2014 21:45:51 -0700
Subject: [PATCH] Improve the error message for ambiguous specs

---
 src/cargo/core/resolver.rs           | 44 ++++++++++++++++-----
 tests/test_cargo_compile_git_deps.rs | 59 ++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 9 deletions(-)

diff --git a/src/cargo/core/resolver.rs b/src/cargo/core/resolver.rs
index f47aa7055..d33bfe618 100644
--- a/src/cargo/core/resolver.rs
+++ b/src/cargo/core/resolver.rs
@@ -1,5 +1,4 @@
-use std::collections::{HashMap, HashSet};
-use std::collections::hashmap::{Occupied, Vacant};
+use std::collections::hashmap::{HashMap, HashSet, Occupied, Vacant};
 use std::fmt;
 use semver;
 
@@ -222,17 +221,44 @@ impl Resolve {
             None => return Err(human(format!("package id specification `{}` \
                                               matched no packages", spec))),
         };
-        match ids.next() {
+        return match ids.next() {
             Some(other) => {
-                let mut msg = format!("Ambiguous package id specification: \
-                                       `{}`\nMatching packages:\n  {}\n  {}",
-                                      spec, ret, other);
-                for id in ids {
-                    msg = format!("{}\n  {}", msg, id);
-                }
+                let mut msg = format!("There are multiple `{}` packages in \
+                                       your project, and the specification \
+                                       `{}` is ambiguous.\n\
+                                       Please re-run this command \
+                                       with `-p <spec>` where `<spec>` is one \
+                                       of the following:",
+                                      spec.get_name(), spec);
+                let mut vec = vec![ret, other];
+                vec.extend(ids);
+                minimize(&mut msg, vec, &spec);
                 Err(human(msg))
             }
             None => Ok(ret)
+        };
+
+        fn minimize(msg: &mut String,
+                    ids: Vec<&PackageId>,
+                    spec: &PackageIdSpec) {
+            let mut version_cnt = HashMap::new();
+            for id in ids.iter() {
+                let slot = match version_cnt.entry(id.get_version()) {
+                    Occupied(e) => e.into_mut(),
+                    Vacant(e) => e.set(0u),
+                };
+                *slot += 1;
+            }
+            for id in ids.iter() {
+                if version_cnt[id.get_version()] == 1 {
+                    msg.push_str(format!("\n  {}:{}", spec.get_name(),
+                                 id.get_version()).as_slice());
+                } else {
+                    msg.push_str(format!("\n  {}",
+                                         PackageIdSpec::from_package_id(*id))
+                                        .as_slice());
+                }
+            }
         }
     }
 
diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs
index 1ab1f0e85..ad805d85e 100644
--- a/tests/test_cargo_compile_git_deps.rs
+++ b/tests/test_cargo_compile_git_deps.rs
@@ -1328,3 +1328,62 @@ test!(warnings_in_git_dep {
                              COMPILING, p.url()))
         .with_stderr(""));
 })
+
+test!(update_ambiguous {
+    let foo1 = git_repo("foo1", |project| {
+        project.file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.5.0"
+            authors = ["wycats@example.com"]
+        "#)
+        .file("src/lib.rs", "")
+    }).assert();
+    let foo2 = git_repo("foo2", |project| {
+        project.file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.6.0"
+            authors = ["wycats@example.com"]
+        "#)
+        .file("src/lib.rs", "")
+    }).assert();
+    let bar = git_repo("bar", |project| {
+        project.file("Cargo.toml", format!(r#"
+            [package]
+            name = "bar"
+            version = "0.5.0"
+            authors = ["wycats@example.com"]
+
+            [dependencies.foo]
+            git = '{}'
+        "#, foo2.url()).as_slice())
+        .file("src/lib.rs", "")
+    }).assert();
+
+    let p = project("project")
+        .file("Cargo.toml", format!(r#"
+            [project]
+            name = "project"
+            version = "0.5.0"
+            authors = []
+            [dependencies.foo]
+            git = '{}'
+            [dependencies.bar]
+            git = '{}'
+        "#, foo1.url(), bar.url()).as_slice())
+        .file("src/main.rs", "fn main() {}");
+
+    assert_that(p.cargo_process("generate-lockfile"), execs().with_status(0));
+    assert_that(p.process(cargo_dir().join("cargo")).arg("update")
+                 .arg("-p").arg("foo"),
+                execs().with_status(101)
+                       .with_stderr("\
+There are multiple `foo` packages in your project, and the specification `foo` \
+is ambiguous.
+Please re-run this command with `-p <spec>` where `<spec>` is one of the \
+following:
+  foo:0.[..].0
+  foo:0.[..].0
+"));
+})