Resolve more correctly and add --extern

This commit adds support for passing --extern to dependencies. It allows
multiple copies of the same named dependency in the system, as long as
they come from different repos.

src/cargo/core/manifest.rs

@@ -298,6 +298,13 @@ impl Manifest {
 }
 
 impl Target {
+    pub fn file_stem(&self) -> String {
+        match self.metadata {
+            Some(ref metadata) => format!("{}{}", self.name, metadata.extra_filename),
+            None => self.name.clone()
+        }
+    }
+
     pub fn lib_target(name: &str, crate_targets: Vec<LibKind>,
                       src_path: &Path, profile: &Profile,
                       metadata: &Metadata)

src/cargo/core/mod.rs

@@ -42,6 +42,7 @@ pub use self::dependency::{
 };
 
 pub use self::version_req::VersionReq;
+pub use self::resolver::Resolve;
 
 pub mod errors;
 pub mod source;

src/cargo/core/package_id.rs

@@ -1,7 +1,9 @@
 use semver;
 use url::Url;
+use std::hash::Hash;
 use std::fmt;
 use std::fmt::{Show,Formatter};
+use collections::hash;
 use serialize::{
     Encodable,
     Encoder,
@@ -60,6 +62,16 @@ pub struct PackageId {
     source_id: SourceId,
 }
 
+impl<S: hash::Writer> Hash<S> for PackageId {
+    fn hash(&self, state: &mut S) {
+        self.name.hash(state);
+        self.version.to_str().hash(state);
+        self.source_id.hash(state);
+    }
+}
+
+impl Eq for PackageId {}
+
 #[deriving(Clone, Show, PartialEq)]
 pub enum PackageIdError {
     InvalidVersion(String),
@@ -110,7 +122,7 @@ impl PackageId {
         let extra_filename = short_hash(
             &(self.name.as_slice(), self.version.to_str(), &self.source_id));
 
-        Metadata { metadata: metadata, extra_filename: extra_filename }
+        Metadata { metadata: metadata, extra_filename: format!("-{}", extra_filename) }
     }
 }
 

src/cargo/core/resolver.rs

@@ -1,59 +1,128 @@
 use std::collections::HashMap;
+use util::graph::{Nodes,Edges};
 
 use core::{
     Dependency,
     PackageId,
     Summary,
     Registry,
+    SourceId,
 };
 
-use util::{CargoResult, human, internal};
+use semver;
 
-/* TODO:
- * - The correct input here is not a registry. Resolves should be performable
- * on package summaries vs. the packages themselves.
- */
-pub fn resolve<R: Registry>(deps: &[Dependency],
-                            registry: &mut R) -> CargoResult<Vec<PackageId>> {
+use util::{CargoResult, Graph, human, internal};
+
+pub struct Resolve {
+    graph: Graph<PackageId>
+}
+
+impl Resolve {
+    fn new() -> Resolve {
+        Resolve { graph: Graph::new() }
+    }
+
+    pub fn iter<'a>(&'a self) -> Nodes<'a, PackageId> {
+        self.graph.iter()
+    }
+
+    pub fn deps<'a>(&'a self, pkg: &PackageId) -> Option<Edges<'a, PackageId>> {
+        self.graph.edges(pkg)
+    }
+}
+
+/*
+pub fn resolve<R: Registry>(deps: &[Dependency], registry: &mut R) -> CargoResult<Vec<PackageId>> {
+    Ok(try!(resolve2(deps, registry)).iter().map(|p| p.clone()).collect())
+}
+*/
+
+struct Context<'a, R> {
+    registry: &'a mut R,
+    resolve: Resolve,
+
+    // Eventually, we will have smarter logic for checking for conflicts in the resolve,
+    // but without the registry, conflicts should not exist in practice, so this is just
+    // a sanity check.
+    seen: HashMap<(String, SourceId), semver::Version>
+}
+
+impl<'a, R: Registry> Context<'a, R> {
+    fn new(registry: &'a mut R) -> Context<'a, R> {
+        Context {
+            registry: registry,
+            resolve: Resolve::new(),
+            seen: HashMap::new()
+        }
+    }
+}
+
+pub fn resolve<R: Registry>(deps: &[Dependency], registry: &mut R) -> CargoResult<Resolve> {
     log!(5, "resolve; deps={}", deps);
 
-    let mut remaining = Vec::from_slice(deps);
-    let mut resolve = HashMap::<String, Summary>::new();
+    let mut context = Context::new(registry);
+    try!(resolve_deps(None, deps, &mut context));
+    Ok(context.resolve)
+}
 
-    loop {
-        let curr = match remaining.pop() {
-            Some(curr) => curr,
-            None => {
-                let ret = resolve.values().map(|summary| {
-                    summary.get_package_id().clone()
-                }).collect();
-                log!(5, "resolve complete; ret={}", ret);
-                return Ok(ret);
+fn resolve_deps<'a, R: Registry>(parent: Option<&PackageId>,
+                                 deps: &[Dependency],
+                                 ctx: &mut Context<'a, R>)
+                                 -> CargoResult<()>
+{
+    if deps.is_empty() {
+        return Ok(());
+    }
+
+    for dep in deps.iter() {
+        let pkgs = try!(ctx.registry.query(dep));
+
+        if pkgs.is_empty() {
+            return Err(human(format!("No package named {} found", dep)));
+        }
+
+        if pkgs.len() > 1 {
+            return Err(internal(format!("At the moment, Cargo only supports a \
+                single source for a particular package name ({}).", dep)));
+        }
+
+        let summary = pkgs.get(0).clone();
+        let name = summary.get_name().to_str();
+        let source_id = summary.get_source_id().clone();
+        let version = summary.get_version().clone();
+
+        let found = {
+            let found = ctx.seen.find(&(name.clone(), source_id.clone()));
+
+            if found.is_some() {
+                if found == Some(&version) { continue; }
+                return Err(human(format!("Cargo found multiple copies of {} in {}. This \
+                                        is not currently supported",
+                                        summary.get_name(), summary.get_source_id())));
+            } else {
+                false
             }
         };
 
-        let opts = try!(registry.query(&curr));
-
-        if opts.len() == 0 {
-            return Err(human(format!("No package named {} found", curr.get_name())));
+        if !found {
+            ctx.seen.insert((name, source_id), version);
         }
 
-        if opts.len() > 1 {
-            return Err(internal(format!("At the moment, Cargo only supports a \
-                single source for a particular package name ({}).", curr.get_name())));
-        }
+        ctx.resolve.graph.add(summary.get_package_id().clone(), []);
 
-        let pkg = opts.get(0).clone();
-        resolve.insert(pkg.get_name().to_str(), pkg.clone());
+        let deps: Vec<Dependency> = summary.get_dependencies().iter()
+            .filter(|d| d.is_transitive())
+            .map(|d| d.clone())
+            .collect();
 
-        for dep in pkg.get_dependencies().iter() {
-            if !dep.is_transitive() { continue; }
+        try!(resolve_deps(Some(summary.get_package_id()), deps.as_slice(), ctx));
 
-            if !resolve.contains_key_equiv(&dep.get_name()) {
-                remaining.push(dep.clone());
-            }
-        }
+        parent.map(|parent| {
+            ctx.resolve.graph.link(parent.clone(), summary.get_package_id().clone());
+        });
     }
+
+    Ok(())
 }
 
 #[cfg(test)]
@@ -61,8 +130,13 @@ mod test {
     use hamcrest::{assert_that, equal_to, contains};
 
     use core::source::{SourceId, RegistryKind, Location, Remote};
-    use core::{Dependency, PackageId, Summary};
+    use core::{Dependency, PackageId, Summary, Registry};
     use super::resolve;
+    use util::CargoResult;
+
+    fn resolve<R: Registry>(deps: &[Dependency], registry: &mut R) -> CargoResult<Vec<PackageId>> {
+        Ok(try!(super::resolve(deps, registry)).iter().map(|p| p.clone()).collect())
+    }
 
     trait ToDep {
         fn to_dep(self) -> Dependency;

src/cargo/lib.rs

@@ -2,9 +2,11 @@
 #![crate_type="rlib"]
 
 #![feature(macro_rules, phase)]
+#![feature(default_type_params)]
 
 extern crate debug;
 extern crate term;
+extern crate collections;
 extern crate url;
 extern crate serialize;
 extern crate semver;

src/cargo/ops/cargo_compile.rs

@@ -24,7 +24,7 @@
 
 use std::os;
 use util::config::{Config, ConfigValue};
-use core::{MultiShell, Source, SourceId, PackageSet, Target, resolver};
+use core::{MultiShell, Source, SourceId, PackageSet, Target, PackageId, resolver};
 use core::registry::PackageRegistry;
 use ops;
 use sources::{PathSource};
@@ -57,7 +57,7 @@ pub fn compile(manifest_path: &Path, options: CompileOptions) -> CargoResult<()>
     let override_ids = try!(source_ids_from_config());
     let source_ids = package.get_source_ids();
 
-    let packages = {
+    let (packages, resolve) = {
         let mut config = try!(Config::new(shell, update, jobs));
 
         let mut registry =
@@ -66,9 +66,12 @@ pub fn compile(manifest_path: &Path, options: CompileOptions) -> CargoResult<()>
         let resolved =
             try!(resolver::resolve(package.get_dependencies(), &mut registry));
 
-        try!(registry.get(resolved.as_slice()).wrap({
+        let req: Vec<PackageId> = resolved.iter().map(|r| r.clone()).collect();
+        let packages = try!(registry.get(req.as_slice()).wrap({
             human("Unable to get packages from source")
-        }))
+        }));
+
+        (packages, resolved)
     };
 
     debug!("packages={}", packages);
@@ -78,8 +81,9 @@ pub fn compile(manifest_path: &Path, options: CompileOptions) -> CargoResult<()>
     }).collect::<Vec<&Target>>();
 
     let mut config = try!(Config::new(shell, update, jobs));
+
     try!(ops::compile_targets(env.as_slice(), targets.as_slice(), &package,
-         &PackageSet::new(packages.as_slice()), &mut config));
+         &PackageSet::new(packages.as_slice()), &resolve, &mut config));
 
     Ok(())
 }

src/cargo/ops/cargo_rustc.rs

@@ -6,7 +6,7 @@ use std::os::args;
 use std::str;
 use term::color::YELLOW;
 
-use core::{Package, PackageSet, Target};
+use core::{Package, PackageSet, Target, Resolve};
 use util;
 use util::{CargoResult, ChainError, ProcessBuilder, internal, human, CargoError};
 use util::{Config, TaskPool, DependencyQueue, Fresh, Dirty, Freshness};
@@ -18,6 +18,8 @@ struct Context<'a, 'b> {
     deps_dir: &'a Path,
     primary: bool,
     rustc_version: &'a str,
+    resolve: &'a Resolve,
+    package_set: &'a PackageSet,
     config: &'b mut Config<'b>
 }
 
@@ -42,9 +44,9 @@ fn uniq_target_dest<'a>(targets: &[&'a Target]) -> Option<&'a str> {
 }
 
 pub fn compile_targets<'a>(env: &str, targets: &[&Target], pkg: &Package,
-                           deps: &PackageSet,
-                           config: &'a mut Config<'a>) -> CargoResult<()> {
-
+                           deps: &PackageSet, resolve: &'a Resolve,
+                           config: &'a mut Config<'a>) -> CargoResult<()>
+{
     if targets.is_empty() {
         return Ok(());
     }
@@ -74,6 +76,8 @@ pub fn compile_targets<'a>(env: &str, targets: &[&Target], pkg: &Package,
         deps_dir: &deps_target_dir,
         primary: false,
         rustc_version: rustc_version.as_slice(),
+        resolve: resolve,
+        package_set: deps,
         config: config
     };
 
@@ -135,7 +139,7 @@ fn compile(targets: &[&Target], pkg: &Package,
     // After the custom command has run, execute rustc for all targets of our
     // package.
     for &target in targets.iter() {
-        cmds.push(rustc(&pkg.get_root(), target, cx));
+        cmds.push(rustc(pkg, target, cx));
     }
 
     cmds.push(proc() {
@@ -207,15 +211,16 @@ fn compile_custom(pkg: &Package, cmd: &str,
     proc() p.exec_with_output().map(|_| ()).map_err(|e| e.mark_human())
 }
 
-fn rustc(root: &Path, target: &Target, cx: &mut Context) -> Job {
+fn rustc(package: &Package, target: &Target, cx: &mut Context) -> Job {
     let crate_types = target.rustc_crate_types();
+    let root = package.get_root();
 
     log!(5, "root={}; target={}; crate_types={}; dest={}; deps={}; verbose={}",
          root.display(), target, crate_types, cx.dest.display(),
          cx.deps_dir.display(), cx.primary);
 
     let primary = cx.primary;
-    let rustc = prepare_rustc(root, target, crate_types, cx);
+    let rustc = prepare_rustc(package, target, crate_types, cx);
 
     log!(5, "command={}", rustc);
 
@@ -232,12 +237,14 @@ fn rustc(root: &Path, target: &Target, cx: &mut Context) -> Job {
     }
 }
 
-fn prepare_rustc(root: &Path, target: &Target, crate_types: Vec<&str>,
-                 cx: &Context) -> ProcessBuilder {
+fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>,
+                 cx: &Context) -> ProcessBuilder
+{
+    let root = package.get_root();
     let mut args = Vec::new();
 
     build_base_args(&mut args, target, crate_types, cx);
-    build_deps_args(&mut args, cx);
+    build_deps_args(&mut args, package, cx);
 
     util::process("rustc")
         .cwd(root.clone())
@@ -286,7 +293,7 @@ fn build_base_args(into: &mut Args,
             into.push(format!("metadata={}", m.metadata));
 
             into.push("-C".to_str());
-            into.push(format!("extra-filename=-{}", m.extra_filename));
+            into.push(format!("extra-filename={}", m.extra_filename));
         }
         None => {}
     }
@@ -300,11 +307,36 @@ fn build_base_args(into: &mut Args,
     }
 }
 
-fn build_deps_args(dst: &mut Args, cx: &Context) {
+fn build_deps_args(dst: &mut Args, package: &Package, cx: &Context) {
     dst.push("-L".to_str());
     dst.push(cx.dest.display().to_str());
     dst.push("-L".to_str());
     dst.push(cx.deps_dir.display().to_str());
+
+    for target in dep_targets(package, cx).iter() {
+        dst.push("--extern".to_str());
+        dst.push(format!("{}={}/lib{}.rlib",
+                 target.get_name(),
+                 cx.deps_dir.display(),
+                 target.file_stem()));
+    }
+}
+
+fn dep_targets(pkg: &Package, cx: &Context) -> Vec<Target> {
+    match cx.resolve.deps(pkg.get_package_id()) {
+        None => vec!(),
+        Some(deps) => deps
+            .map(|pkg_id| {
+                cx.package_set.iter()
+                  .find(|pkg| pkg_id == pkg.get_package_id())
+                  .expect("Should have found package")
+            })
+            .filter_map(|pkg| {
+                pkg.get_targets().iter().find(|&t| t.is_lib() && t.get_profile().is_compile())
+            })
+            .map(|t| t.clone())
+            .collect()
+    }
 }
 
 /// Execute all jobs necessary to build the dependency graph.

src/cargo/util/graph.rs

@@ -1,8 +1,9 @@
 use std::hash::Hash;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
+use std::collections::hashmap::{Keys, SetItems};
 
 pub struct Graph<N> {
-    nodes: HashMap<N, Vec<N>>
+    nodes: HashMap<N, HashSet<N>>
 }
 
 enum Mark {
@@ -10,13 +11,26 @@ enum Mark {
     Done
 }
 
+pub type Nodes<'a, N> = Keys<'a, N, HashSet<N>>;
+pub type Edges<'a, N> = SetItems<'a, N>;
+
 impl<N: Eq + Hash + Clone> Graph<N> {
     pub fn new() -> Graph<N> {
         Graph { nodes: HashMap::new() }
     }
 
     pub fn add(&mut self, node: N, children: &[N]) {
-        self.nodes.insert(node, children.to_owned());
+        self.nodes.insert(node, children.iter().map(|n| n.clone()).collect());
+    }
+
+    pub fn link(&mut self, node: N, child: N) {
+        self.nodes
+            .find_or_insert_with(node, |_| HashSet::new())
+            .insert(child);
+    }
+
+    pub fn edges<'a>(&'a self, node: &N) -> Option<Edges<'a, N>> {
+        self.nodes.find(node).map(|set| set.iter())
     }
 
     pub fn sort(&self) -> Option<Vec<N>> {
@@ -44,4 +58,8 @@ impl<N: Eq + Hash + Clone> Graph<N> {
         dst.push(node.clone());
         marks.insert(node.clone(), Done);
     }
+
+    pub fn iter<'a>(&'a self) -> Nodes<'a, N> {
+        self.nodes.keys()
+    }
 }

src/cargo/util/mod.rs

@@ -8,6 +8,7 @@ pub use self::paths::realpath;
 pub use self::hex::{to_hex, short_hash};
 pub use self::pool::TaskPool;
 pub use self::dependency_queue::{DependencyQueue, Fresh, Dirty, Freshness};
+pub use self::graph::Graph;
 
 pub mod graph;
 pub mod process_builder;

tests/test_cargo_compile.rs

@@ -93,7 +93,7 @@ test!(cargo_compile_with_invalid_code {
 {filename}:1 invalid rust code!
              ^~~~~~~
 Could not execute process \
-`rustc {filename} --crate-name foo --crate-type bin -g -o {} -L {} -L {}` (status=101)\n",
+`rustc {filename} --crate-name foo --crate-type bin -o {} -L {} -L {}` (status=101)\n",
             target.join("foo").display(),
             target.display(),
             target.join("deps").display(),
@@ -973,7 +973,7 @@ test!(verbose_build {
     let hash = out.slice_from(out.find_str("extra-filename=").unwrap() + 15);
     let hash = hash.slice_to(17);
     assert_eq!(out, format!("\
-{} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib -g \
+{} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \
         -C metadata=test:-:0.0.0:-:file:{dir} \
         -C extra-filename={hash} \
         --out-dir {dir}{sep}target \