Only allow one of each links attribute in resolver

src/cargo/core/resolver/mod.rs

@@ -23,7 +23,9 @@
 //! * Never try to activate a crate version which is incompatible. This means we
 //!   only try crates which will actually satisfy a dependency and we won't ever
 //!   try to activate a crate that's semver compatible with something else
-//!   activated (as we're only allowed to have one).
+//!   activated (as we're only allowed to have one) nor try to activate a crate
+//!   that has the same links attribute as something else
+//!   activated.
 //! * Always try to activate the highest version crate first. The default
 //!   dependency in Cargo (e.g. when you write `foo = "0.1.2"`) is
 //!   semver-compatible, so selecting the highest version possible will allow us
@@ -345,11 +347,12 @@ enum GraphNode {
 // possible.
 #[derive(Clone)]
 struct Context<'a> {
-    // TODO: Both this and the map below are super expensive to clone. We should
+    // TODO: Both this and the map two below are super expensive to clone. We should
     //       switch to persistent hash maps if we can at some point or otherwise
     //       make these much cheaper to clone in general.
     activations: Activations,
     resolve_features: HashMap<PackageId, HashSet<String>>,
+    links: HashSet<String>,
 
     // These are two cheaply-cloneable lists (O(1) clone) which are effectively
     // hash maps but are built up as "construction lists". We'll iterate these
@@ -374,9 +377,10 @@ pub fn resolve(summaries: &[(Summary, Method)],
     let cx = Context {
         resolve_graph: RcList::new(),
         resolve_features: HashMap::new(),
+        links: HashSet::new(),
         resolve_replacements: RcList::new(),
         activations: HashMap::new(),
-        replacements: replacements,
+        replacements,
         warnings: RcList::new(),
     };
     let _p = profile::start("resolving");
@@ -476,7 +480,7 @@ impl<T> RcVecIter<T> {
     fn new(vec: Rc<Vec<T>>) -> RcVecIter<T> {
         RcVecIter {
             rest: 0..vec.len(),
-            vec: vec,
+            vec,
         }
     }
 }
@@ -564,19 +568,21 @@ struct RemainingCandidates {
 }
 
 impl RemainingCandidates {
-    fn next(&mut self, prev_active: &[Summary]) -> Option<Candidate> {
+    fn next(&mut self, prev_active: &[Summary], links: &HashSet<String>) -> Option<Candidate> {
         // Filter the set of candidates based on the previously activated
         // versions for this dependency. We can actually use a version if it
         // precisely matches an activated version or if it is otherwise
         // incompatible with all other activated versions. Note that we
         // define "compatible" here in terms of the semver sense where if
         // the left-most nonzero digit is the same they're considered
-        // compatible.
+        // compatible unless we have a `*-sys` crate (defined by having a
+        // linked attribute) then we can only have one version.
         self.remaining.by_ref().map(|p| p.1).find(|b| {
-            prev_active.iter().any(|a| *a == b.summary) ||
-                prev_active.iter().all(|a| {
-                    !compatible(a.version(), b.summary.version())
-                })
+            prev_active.iter().any(|a| *a == b.summary)
+                || (b.summary.links().map(|l| !links.contains(l)).unwrap_or(true)
+                    && prev_active
+                        .iter()
+                        .all(|a| !compatible(a.version(), b.summary.version())))
         })
     }
 }
@@ -671,8 +677,8 @@ fn activate_deps_loop<'a>(mut cx: Context<'a>,
             let mut candidates = RemainingCandidates {
                 remaining: RcVecIter::new(Rc::clone(&candidates)),
             };
-            (candidates.next(prev_active),
-             candidates.clone().next(prev_active).is_some(),
+            (candidates.next(prev_active, &cx.links),
+             candidates.clone().next(prev_active, &cx.links).is_some(),
              candidates)
         };
 
@@ -682,7 +688,7 @@ fn activate_deps_loop<'a>(mut cx: Context<'a>,
         // 1. The version matches the dependency requirement listed for this
         //    package
         // 2. There are no activated versions for this package which are
-        //    semver-compatible, or there's an activated version which is
+        //    semver/links-compatible, or there's an activated version which is
         //    precisely equal to `candidate`.
         //
         // This means that we're going to attempt to activate each candidate in
@@ -697,7 +703,7 @@ fn activate_deps_loop<'a>(mut cx: Context<'a>,
                         cur,
                         context_backup: Context::clone(&cx),
                         deps_backup: <BinaryHeap<DepsFrame>>::clone(&remaining_deps),
-                        remaining_candidates: remaining_candidates,
+                        remaining_candidates,
                         parent: Summary::clone(&parent),
                         dep: Dependency::clone(&dep),
                         features: Rc::clone(&features),
@@ -758,8 +764,8 @@ fn find_candidate<'a>(backtrack_stack: &mut Vec<BacktrackFrame<'a>>,
     while let Some(mut frame) = backtrack_stack.pop() {
         let (next, has_another) = {
             let prev_active = frame.context_backup.prev_active(&frame.dep);
-            (frame.remaining_candidates.next(prev_active),
-             frame.remaining_candidates.clone().next(prev_active).is_some())
+            (frame.remaining_candidates.next(prev_active, &frame.context_backup.links),
+             frame.remaining_candidates.clone().next(prev_active, &frame.context_backup.links).is_some())
         };
         if let Some(candidate) = next {
             *cur = frame.cur;
@@ -1033,9 +1039,9 @@ fn build_requirements<'a, 'b: 'a>(s: &'a Summary, method: &'b Method)
 }
 
 impl<'a> Context<'a> {
-    // Activate this summary by inserting it into our list of known activations.
-    //
-    // Returns if this summary with the given method is already activated.
+    /// Activate this summary by inserting it into our list of known activations.
+    ///
+    /// Returns true if this summary with the given method is already activated.
     fn flag_activated(&mut self,
                       summary: &Summary,
                       method: &Method) -> bool {
@@ -1047,6 +1053,9 @@ impl<'a> Context<'a> {
                        .or_insert(Vec::new());
         if !prev.iter().any(|c| c == summary) {
             self.resolve_graph.push(GraphNode::Add(id.clone()));
+            if let Some(link) = summary.links() {
+                self.links.insert(link.to_owned());
+            }
             prev.push(summary.clone());
             return false
         }

src/cargo/core/summary.rs

@@ -22,12 +22,14 @@ struct Inner {
     dependencies: Vec<Dependency>,
     features: BTreeMap<String, Vec<String>>,
     checksum: Option<String>,
+    links: Option<String>,
 }
 
 impl Summary {
     pub fn new(pkg_id: PackageId,
                dependencies: Vec<Dependency>,
-               features: BTreeMap<String, Vec<String>>) -> CargoResult<Summary> {
+               features: BTreeMap<String, Vec<String>>,
+               links: Option<String>) -> CargoResult<Summary> {
         for dep in dependencies.iter() {
             if features.get(dep.name()).is_some() {
                 bail!("Features and dependencies cannot have the \
@@ -66,9 +68,10 @@ impl Summary {
         Ok(Summary {
             inner: Rc::new(Inner {
                 package_id: pkg_id,
-                dependencies: dependencies,
-                features: features,
+                dependencies,
+                features,
                 checksum: None,
+                links,
             }),
         })
     }
@@ -82,6 +85,9 @@ impl Summary {
     pub fn checksum(&self) -> Option<&str> {
         self.inner.checksum.as_ref().map(|s| &s[..])
     }
+    pub fn links(&self) -> Option<&str> {
+        self.inner.links.as_ref().map(|s| &s[..])
+    }
 
     pub fn override_id(mut self, id: PackageId) -> Summary {
         Rc::make_mut(&mut self.inner).package_id = id;
@@ -94,7 +100,7 @@ impl Summary {
     }
 
     pub fn map_dependencies<F>(mut self, f: F) -> Summary
-                               where F: FnMut(Dependency) -> Dependency {
+        where F: FnMut(Dependency) -> Dependency {
         {
             let slot = &mut Rc::make_mut(&mut self.inner).dependencies;
             let deps = mem::replace(slot, Vec::new());

src/cargo/sources/registry/index.rs

@@ -146,12 +146,12 @@ impl<'cfg> RegistryIndex<'cfg> {
     fn parse_registry_package(&mut self, line: &str)
                               -> CargoResult<(Summary, bool)> {
         let RegistryPackage {
-            name, vers, cksum, deps, features, yanked
+            name, vers, cksum, deps, features, yanked, links
         } = super::DEFAULT_ID.set(&self.source_id, || {
             serde_json::from_str::<RegistryPackage>(line)
         })?;
         let pkgid = PackageId::new(&name, &vers, &self.source_id)?;
-        let summary = Summary::new(pkgid, deps.inner, features)?;
+        let summary = Summary::new(pkgid, deps.inner, features, links)?;
         let summary = summary.set_checksum(cksum.clone());
         if self.hashes.contains_key(&name[..]) {
             self.hashes.get_mut(&name[..]).unwrap().insert(vers, cksum);

src/cargo/sources/registry/mod.rs

@@ -220,6 +220,8 @@ struct RegistryPackage<'a> {
     features: BTreeMap<String, Vec<String>>,
     cksum: String,
     yanked: Option<bool>,
+    #[serde(default)]
+    links: Option<String>,
 }
 
 struct DependencyList {

src/cargo/util/toml/mod.rs

@@ -672,7 +672,7 @@ impl TomlManifest {
         let include = project.include.clone().unwrap_or_default();
 
         let summary = Summary::new(pkgid, deps, me.features.clone()
-            .unwrap_or_else(BTreeMap::new))?;
+            .unwrap_or_else(BTreeMap::new), project.links.clone())?;
         let metadata = ManifestMetadata {
             description: project.description.clone(),
             homepage: project.homepage.clone(),

tests/build-script.rs

@@ -227,6 +227,7 @@ not have a custom build script
 
 #[test]
 fn links_duplicates() {
+    // this tests that the links_duplicates are caught at resolver time
     let p = project("foo")
         .file("Cargo.toml", r#"
             [project]
@@ -256,20 +257,15 @@ fn links_duplicates() {
     assert_that(p.cargo("build"),
                 execs().with_status(101)
                        .with_stderr("\
-[ERROR] multiple packages link to native library `a`, but a native library can \
-be linked only once
-
-package `foo v0.5.0 ([..])`
-links to native library `a`
-
-package `a-sys v0.5.0 ([..])`
-    ... which is depended on by `foo v0.5.0 ([..])`
-also links to native library `a`
+error: failed to select a version for `a-sys` (required by `foo`):
+all possible versions conflict with previously selected versions of `a-sys`
+  possible versions to select: 0.5.0
 "));
 }
 
 #[test]
 fn links_duplicates_deep_dependency() {
+    // this tests that the links_duplicates are caught at resolver time
     let p = project("foo")
         .file("Cargo.toml", r#"
             [project]
@@ -311,16 +307,9 @@ fn links_duplicates_deep_dependency() {
     assert_that(p.cargo("build"),
                 execs().with_status(101)
                        .with_stderr("\
-[ERROR] multiple packages link to native library `a`, but a native library can \
-be linked only once
-
-package `foo v0.5.0 ([..])`
-links to native library `a`
-
-package `a-sys v0.5.0 ([..])`
-    ... which is depended on by `a v0.5.0 ([..])`
-    ... which is depended on by `foo v0.5.0 ([..])`
-also links to native library `a`
+error: failed to select a version for `a-sys` (required by `a`):
+all possible versions conflict with previously selected versions of `a-sys`
+  possible versions to select: 0.5.0
 "));
 }
 
@@ -2741,6 +2730,7 @@ fn deterministic_rustc_dependency_flags() {
 
 #[test]
 fn links_duplicates_with_cycle() {
+    // this tests that the links_duplicates are caught at resolver time
     let p = project("foo")
         .file("Cargo.toml", r#"
             [project]
@@ -2783,17 +2773,9 @@ fn links_duplicates_with_cycle() {
     assert_that(p.cargo("build"),
                 execs().with_status(101)
                        .with_stderr("\
-[ERROR] multiple packages link to native library `a`, but a native library can \
-be linked only once
-
-package `foo v0.5.0 ([..])`
-    ... which is depended on by `b v0.5.0 ([..])`
-links to native library `a`
-
-package `a v0.5.0 (file://[..])`
-    ... which is depended on by `foo v0.5.0 ([..])`
-    ... which is depended on by `b v0.5.0 ([..])`
-also links to native library `a`
+error: failed to select a version for `a` (required by `foo`):
+all possible versions conflict with previously selected versions of `a`
+  possible versions to select: 0.5.0
 "));
 }
 

tests/resolve.rs

@@ -32,7 +32,7 @@ fn resolve(pkg: &PackageId, deps: Vec<Dependency>, registry: &[Summary])
         fn requires_precise(&self) -> bool { false }
     }
     let mut registry = MyRegistry(registry);
-    let summary = Summary::new(pkg.clone(), deps, BTreeMap::new()).unwrap();
+    let summary = Summary::new(pkg.clone(), deps, BTreeMap::new(), None).unwrap();
     let method = Method::Everything;
     let resolve = resolver::resolve(&[(summary, method)], &[], &mut registry, None, false)?;
     let res = resolve.iter().cloned().collect();
@@ -78,11 +78,11 @@ macro_rules! pkg {
     ($pkgid:expr => [$($deps:expr),+]) => ({
         let d: Vec<Dependency> = vec![$($deps.to_dep()),+];
 
-        Summary::new($pkgid.to_pkgid(), d, BTreeMap::new()).unwrap()
+        Summary::new($pkgid.to_pkgid(), d, BTreeMap::new(), None).unwrap()
     });
 
     ($pkgid:expr) => (
-        Summary::new($pkgid.to_pkgid(), Vec::new(), BTreeMap::new()).unwrap()
+        Summary::new($pkgid.to_pkgid(), Vec::new(), BTreeMap::new(), None).unwrap()
     )
 }
 
@@ -92,7 +92,7 @@ fn registry_loc() -> SourceId {
 }
 
 fn pkg(name: &str) -> Summary {
-    Summary::new(pkg_id(name), Vec::new(), BTreeMap::new()).unwrap()
+    Summary::new(pkg_id(name), Vec::new(), BTreeMap::new(), None).unwrap()
 }
 
 fn pkg_id(name: &str) -> PackageId {
@@ -108,7 +108,7 @@ fn pkg_id_loc(name: &str, loc: &str) -> PackageId {
 }
 
 fn pkg_loc(name: &str, loc: &str) -> Summary {
-    Summary::new(pkg_id_loc(name, loc), Vec::new(), BTreeMap::new()).unwrap()
+    Summary::new(pkg_id_loc(name, loc), Vec::new(), BTreeMap::new(), None).unwrap()
 }
 
 fn dep(name: &str) -> Dependency { dep_req(name, "1.0.0") }