Auto merge of #2879 - QuiltOS:replace-bug, r=alexcrichton

Fix #2595 by skipping reflexive replacements

If you know of something better than `source_id == source_id`, I'd gladly change it.

src/cargo/core/resolver/mod.rs

@@ -773,45 +773,55 @@ impl<'a> Context<'a> {
              dep: &Dependency) -> CargoResult<Vec<Candidate>> {
         let summaries = try!(registry.query(dep));
         summaries.into_iter().map(Rc::new).map(|summary| {
-            let mut replace = None;
-            let mut matched_spec = None;
-            for &(ref spec, ref dep) in self.replacements.iter() {
-                if !spec.matches(summary.package_id()) {
-                    continue
-                }
-
-                if replace.is_some() {
-                    bail!("overlapping replacement specifications found:\n\n  \
-                           * {}\n  * {}\n\nboth specifications match: {}",
-                          matched_spec.unwrap(), spec, summary.package_id());
-                }
-
-                let mut summaries = try!(registry.query(dep)).into_iter();
-                let s = try!(summaries.next().chain_error(|| {
-                    human(format!("no matching package for override `{}` found\n\
-                                   location searched: {}\n\
-                                   version required: {}",
-                                  spec, dep.source_id(), dep.version_req()))
-                }));
-                let summaries = summaries.collect::<Vec<_>>();
-                if summaries.len() > 0 {
-                    let bullets = summaries.iter().map(|s| {
-                        format!("  * {}", s.package_id())
-                    }).collect::<Vec<_>>();
-                    bail!("the replacement specification `{}` matched \
-                           multiple packages:\n  * {}\n{}", spec,
-                          s.package_id(), bullets.join("\n"));
-                }
-
-                // The dependency should be hard-coded to have the same name and
-                // an exact version requirement, so both of these assertions
-                // should never fail.
-                assert_eq!(s.version(), summary.version());
-                assert_eq!(s.name(), summary.name());
+            // get around lack of non-lexical lifetimes
+            let summary2 = summary.clone();
+
+            let mut potential_matches = self.replacements.iter()
+                .filter(|&&(ref spec, _)| spec.matches(summary2.package_id()));
+
+            let &(ref spec, ref dep) = match potential_matches.next() {
+                None => return Ok(Candidate { summary: summary, replace: None }),
+                Some(replacement) => replacement,
+            };
+
+            let mut summaries = try!(registry.query(dep)).into_iter();
+            let s = try!(summaries.next().chain_error(|| {
+                human(format!("no matching package for override `{}` found\n\
+                               location searched: {}\n\
+                               version required: {}",
+                              spec, dep.source_id(), dep.version_req()))
+            }));
+            let summaries = summaries.collect::<Vec<_>>();
+            if summaries.len() > 0 {
+                let bullets = summaries.iter().map(|s| {
+                    format!("  * {}", s.package_id())
+                }).collect::<Vec<_>>();
+                bail!("the replacement specification `{}` matched \
+                       multiple packages:\n  * {}\n{}", spec, s.package_id(),
+                      bullets.join("\n"));
+            }
 
-                replace = Some(Rc::new(s));
-                matched_spec = Some(spec.clone());
+            // The dependency should be hard-coded to have the same name and an
+            // exact version requirement, so both of these assertions should
+            // never fail.
+            assert_eq!(s.version(), summary.version());
+            assert_eq!(s.name(), summary.name());
+
+            let replace = if s.source_id() == summary.source_id() {
+                debug!("Preventing\n{:?}\nfrom replacing\n{:?}", summary, s);
+                None
+            } else {
+                Some(Rc::new(s))
+            };
+            let matched_spec = spec.clone();
+
+            // Make sure no duplicates
+            if let Some(&(ref spec, _)) = potential_matches.next() {
+                bail!("overlapping replacement specifications found:\n\n  \
+                       * {}\n  * {}\n\nboth specifications match: {}",
+                      matched_spec, spec, summary.package_id());
             }
+
             Ok(Candidate { summary: summary, replace: replace })
         }).collect()
     }

tests/overrides.rs

@@ -617,3 +617,55 @@ fn update() {
 [UPDATING] git repository `[..]`
 "));
 }
+
+// local -> near -> far
+// near is overridden with itself
+#[test]
+fn no_override_self() {
+    let deps = git::repo(&paths::root().join("override"))
+
+        .file("far/Cargo.toml", r#"
+            [package]
+            name = "far"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("far/src/lib.rs", "")
+
+        .file("near/Cargo.toml", r#"
+            [package]
+            name = "near"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            far = { path = "../far" }
+        "#)
+        .file("near/src/lib.rs", r#"
+            #![no_std]
+            pub extern crate far;
+        "#);
+
+    deps.build();
+
+    let p = project("local")
+        .file("Cargo.toml", &format!(r#"
+            [package]
+            name = "local"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            near = {{ git = '{0}' }}
+
+            [replace]
+            "near:0.1.0" = {{ git = '{0}' }}
+        "#, deps.url()))
+        .file("src/lib.rs", r#"
+            #![no_std]
+            pub extern crate near;
+        "#);
+
+    assert_that(p.cargo_process("build").arg("--verbose"),
+                execs().with_status(0));
+}