Auto merge of #7452 - pyrrho:bug7346/transitive_patches, r=Eh2406

Bug7346/transitive patches

Fixes #7346.

A cursory comparison between current stable and nightly shows that projects with this topology resolve similarly. If there are other behaviors I should test, I'd be happy to expand that section. This is a pretty focused change, though, so I'm not sure what else there is to break.

Sorry about the delay in putting this PR together. Good news is I know more than I did last week.

src/cargo/core/resolver/context.rs

@@ -2,18 +2,16 @@ use std::collections::HashMap;
 use std::num::NonZeroU64;
 use std::rc::Rc;
 
-// "ensure" seems to require "bail" be in scope (macro hygiene issue?).
-#[allow(unused_imports)]
-use failure::{bail, ensure};
+use failure::format_err;
 use log::debug;
 
 use crate::core::interning::InternedString;
 use crate::core::{Dependency, PackageId, SourceId, Summary};
-use crate::util::CargoResult;
 use crate::util::Graph;
 
 use super::dep_cache::RegistryQueryer;
-use super::types::{ConflictMap, FeaturesSet, ResolveOpts};
+use super::errors::ActivateResult;
+use super::types::{ConflictMap, ConflictReason, FeaturesSet, ResolveOpts};
 
 pub use super::encode::Metadata;
 pub use super::encode::{EncodableDependency, EncodablePackageId, EncodableResolve};
@@ -109,7 +107,7 @@ impl Context {
         summary: &Summary,
         opts: &ResolveOpts,
         parent: Option<(&Summary, &Dependency)>,
-    ) -> CargoResult<bool> {
+    ) -> ActivateResult<bool> {
         let id = summary.package_id();
         let age: ContextAge = self.age();
         match self.activations.entry(id.as_activations_key()) {
@@ -122,12 +120,15 @@ impl Context {
             }
             im_rc::hashmap::Entry::Vacant(v) => {
                 if let Some(link) = summary.links() {
-                    ensure!(
-                        self.links.insert(link, id).is_none(),
-                        "Attempting to resolve a dependency with more then one crate with the \
-                         links={}.\nThis will not build as is. Consider rebuilding the .lock file.",
-                        &*link
-                    );
+                    if self.links.insert(link, id).is_some() {
+                        return Err(format_err!(
+                            "Attempting to resolve a dependency with more then \
+                             one crate with links={}.\nThis will not build as \
+                             is. Consider rebuilding the .lock file.",
+                            &*link
+                        )
+                        .into());
+                    }
                 }
                 v.insert((summary.clone(), age));
 
@@ -150,7 +151,11 @@ impl Context {
                     if dep.source_id() != id.source_id() {
                         let key = (id.name(), dep.source_id(), id.version().into());
                         let prev = self.activations.insert(key, (summary.clone(), age));
-                        assert!(prev.is_none());
+                        if let Some((previous_summary, _)) = prev {
+                            return Err(
+                                (previous_summary.package_id(), ConflictReason::Semver).into()
+                            );
+                        }
                     }
                 }
 

tests/testsuite/patch.rs

@@ -735,6 +735,63 @@ fn transitive_new_major() {
         .run();
 }
 
+#[cargo_test]
+fn shared_by_transitive() {
+    Package::new("baz", "0.1.1").publish();
+
+    let baz = git::repo(&paths::root().join("override"))
+        .file("Cargo.toml", &basic_manifest("baz", "0.1.2"))
+        .file("src/lib.rs", "")
+        .build();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+                    [package]
+                    name = "foo"
+                    version = " 0.1.0"
+
+                    [dependencies]
+                    bar = {{ path = "bar" }}
+                    baz = "0.1"
+
+                    [patch.crates-io]
+                    baz = {{ git = "{}", version = "0.1" }}
+                "#,
+                baz.url(),
+            ),
+        )
+        .file("src/lib.rs", "")
+        .file(
+            "bar/Cargo.toml",
+            r#"
+                [package]
+                name = "bar"
+                version = "0.1.0"
+
+                [dependencies]
+                baz = "0.1.1"
+            "#,
+        )
+        .file("bar/src/lib.rs", "")
+        .build();
+
+    p.cargo("build")
+        .with_stderr(
+            "\
+[UPDATING] git repository `file://[..]`
+[UPDATING] `[ROOT][..]` index
+[COMPILING] baz v0.1.2 [..]
+[COMPILING] bar v0.1.0 [..]
+[COMPILING] foo v0.1.0 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+",
+        )
+        .run();
+}
+
 #[cargo_test]
 fn remove_patch() {
     Package::new("foo", "0.1.0").publish();