From 4d094c080f7b576e59b1b2a09f37d311b4fc980f Mon Sep 17 00:00:00 2001 From: Drew Pirrone-Brusse Date: Tue, 17 Sep 2019 12:55:12 -0400 Subject: [PATCH 1/3] Add a test to model the bug found in #7346 --- tests/testsuite/patch.rs | 57 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 8c1459c916c..ac462078efc 100644 --- a/tests/testsuite/patch.rs +++ b/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(); From e2010e8dfa5446c4cdb4d2a1dad7accd4e8ad503 Mon Sep 17 00:00:00 2001 From: Drew Pirrone-Brusse Date: Thu, 26 Sep 2019 18:32:28 -0400 Subject: [PATCH 2/3] Modify Context::flag_activated to return ActivateResult --- src/cargo/core/resolver/context.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 27b9a0585eb..05968309308 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -2,17 +2,15 @@ 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::errors::ActivateResult; use super::types::{ConflictMap, FeaturesSet, ResolveOpts}; pub use super::encode::Metadata; @@ -109,7 +107,7 @@ impl Context { summary: &Summary, opts: &ResolveOpts, parent: Option<(&Summary, &Dependency)>, - ) -> CargoResult { + ) -> ActivateResult { 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)); From f7bfd9dd18c5c4b6bcc4cb009ce8b6f9ed179480 Mon Sep 17 00:00:00 2001 From: Drew Pirrone-Brusse Date: Thu, 26 Sep 2019 18:32:49 -0400 Subject: [PATCH 3/3] Don't panic on a non-fatal error --- src/cargo/core/resolver/context.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 05968309308..991b746e3c2 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -11,7 +11,7 @@ use crate::util::Graph; use super::dep_cache::RegistryQueryer; use super::errors::ActivateResult; -use super::types::{ConflictMap, FeaturesSet, ResolveOpts}; +use super::types::{ConflictMap, ConflictReason, FeaturesSet, ResolveOpts}; pub use super::encode::Metadata; pub use super::encode::{EncodableDependency, EncodablePackageId, EncodableResolve}; @@ -151,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() + ); + } } }