Remove hacks in the Hash impl of PackageId

src/bin/read_manifest.rs

@@ -1,9 +1,8 @@
 use std::env;
 
-use cargo::core::{Package, Source};
+use cargo::core::Package;
 use cargo::util::{CliResult, Config};
 use cargo::util::important_paths::{find_root_manifest_for_wd};
-use cargo::sources::{PathSource};
 
 #[derive(RustcDecodable)]
 pub struct Options {
@@ -32,9 +31,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<Package>>
 
     let root = try!(find_root_manifest_for_wd(options.flag_manifest_path, config.cwd()));
 
-    let mut source = try!(PathSource::for_path(root.parent().unwrap(), config));
-    try!(source.update());
-
-    let pkg = try!(source.root_package());
+    let pkg = try!(Package::for_path(&root, config));
     Ok(Some(pkg))
 }

src/cargo/core/package.rs

@@ -87,7 +87,7 @@ impl Package {
     }
 
     pub fn generate_metadata(&self) -> Metadata {
-        self.package_id().generate_metadata(self.root())
+        self.package_id().generate_metadata()
     }
 }
 
@@ -107,15 +107,7 @@ impl Eq for Package {}
 
 impl hash::Hash for Package {
     fn hash<H: hash::Hasher>(&self, into: &mut H) {
-        // We want to be sure that a path-based package showing up at the same
-        // location always has the same hash. To that effect we don't hash the
-        // vanilla package ID if we're a path, but instead feed in our own root
-        // path.
-        if self.package_id().source_id().is_path() {
-            (0, self.root(), self.name(), self.package_id().version()).hash(into)
-        } else {
-            (1, self.package_id()).hash(into)
-        }
+        self.package_id().hash(into)
     }
 }
 

src/cargo/core/package_id.rs

@@ -3,7 +3,6 @@ use std::error::Error;
 use std::fmt::{self, Formatter};
 use std::hash::Hash;
 use std::hash;
-use std::path::Path;
 use std::sync::Arc;
 
 use regex::Regex;
@@ -136,13 +135,8 @@ impl PackageId {
     pub fn version(&self) -> &semver::Version { &self.inner.version }
     pub fn source_id(&self) -> &SourceId { &self.inner.source_id }
 
-    pub fn generate_metadata(&self, source_root: &Path) -> Metadata {
-        // See comments in Package::hash for why we have this test
-        let metadata = if self.inner.source_id.is_path() {
-            short_hash(&(0, &self.inner.name, &self.inner.version, source_root))
-        } else {
-            short_hash(&(1, self))
-        };
+    pub fn generate_metadata(&self) -> Metadata {
+        let metadata = short_hash(self);
         let extra_filename = format!("-{}", metadata);
 
         Metadata { metadata: metadata, extra_filename: extra_filename }

src/cargo/core/registry.rs

@@ -143,11 +143,9 @@ impl<'cfg> PackageRegistry<'cfg> {
         self.source_ids.insert(id.clone(), (id.clone(), kind));
     }
 
-    pub fn add_overrides(&mut self, ids: Vec<SourceId>) -> CargoResult<()> {
-        for id in ids.iter() {
-            try!(self.load(id, Kind::Override));
-        }
-        Ok(())
+    pub fn add_override(&mut self, id: &SourceId, source: Box<Source + 'cfg>) {
+        self.add_source(id, source, Kind::Override);
+        self.overrides.push(id.clone());
     }
 
     pub fn register_lock(&mut self, id: PackageId, deps: Vec<PackageId>) {

src/cargo/core/resolver/encode.rs

@@ -3,8 +3,8 @@ use std::collections::{HashMap, BTreeMap};
 use regex::Regex;
 use rustc_serialize::{Encodable, Encoder, Decodable, Decoder};
 
-use core::{PackageId, SourceId};
-use util::{CargoResult, Graph};
+use core::{Package, PackageId, SourceId};
+use util::{CargoResult, Graph, Config};
 
 use super::Resolve;
 
@@ -18,66 +18,113 @@ pub struct EncodableResolve {
 pub type Metadata = BTreeMap<String, String>;
 
 impl EncodableResolve {
-    pub fn to_resolve(&self, default: &SourceId) -> CargoResult<Resolve> {
+    pub fn to_resolve(&self, root: &Package, config: &Config)
+                      -> CargoResult<Resolve> {
+        let mut path_deps = HashMap::new();
+        try!(build_path_deps(root, &mut path_deps, config));
+        let default = root.package_id().source_id();
+
         let mut g = Graph::new();
         let mut tmp = HashMap::new();
 
         let packages = Vec::new();
         let packages = self.package.as_ref().unwrap_or(&packages);
 
+        let root = try!(to_package_id(&self.root.name,
+                                      &self.root.version,
+                                      self.root.source.as_ref(),
+                                      default, &path_deps));
+        let ids = try!(packages.iter().map(|p| {
+            to_package_id(&p.name, &p.version, p.source.as_ref(),
+                          default, &path_deps)
+        }).collect::<CargoResult<Vec<_>>>());
+
         {
-            let mut register_pkg = |pkg: &EncodableDependency|
-                                    -> CargoResult<()> {
-                let pkgid = try!(pkg.to_package_id(default));
+            let mut register_pkg = |pkgid: &PackageId| {
                 let precise = pkgid.source_id().precise()
                                    .map(|s| s.to_string());
                 assert!(tmp.insert(pkgid.clone(), precise).is_none(),
                         "a package was referenced twice in the lockfile");
-                g.add(try!(pkg.to_package_id(default)), &[]);
-                Ok(())
+                g.add(pkgid.clone(), &[]);
             };
 
-            try!(register_pkg(&self.root));
-            for pkg in packages.iter() {
-                try!(register_pkg(pkg));
+            register_pkg(&root);
+            for id in ids.iter() {
+                register_pkg(id);
             }
         }
 
         {
-            let mut add_dependencies = |pkg: &EncodableDependency|
+            let mut add_dependencies = |id: &PackageId, pkg: &EncodableDependency|
                                         -> CargoResult<()> {
-                let package_id = try!(pkg.to_package_id(default));
-
                 let deps = match pkg.dependencies {
                     Some(ref deps) => deps,
                     None => return Ok(()),
                 };
                 for edge in deps.iter() {
-                    let to_depend_on = try!(edge.to_package_id(default));
+                    let to_depend_on = try!(to_package_id(&edge.name,
+                                                          &edge.version,
+                                                          edge.source.as_ref(),
+                                                          default,
+                                                          &path_deps));
                     let precise_pkgid =
                         tmp.get(&to_depend_on)
                            .map(|p| to_depend_on.with_precise(p.clone()))
                            .unwrap_or(to_depend_on.clone());
-                    g.link(package_id.clone(), precise_pkgid);
+                    g.link(id.clone(), precise_pkgid);
                 }
                 Ok(())
             };
 
-            try!(add_dependencies(&self.root));
-            for pkg in packages.iter() {
-                try!(add_dependencies(pkg));
+            try!(add_dependencies(&root, &self.root));
+            for (id, pkg) in ids.iter().zip(packages) {
+                try!(add_dependencies(id, pkg));
             }
         }
 
         Ok(Resolve {
             graph: g,
-            root: try!(self.root.to_package_id(default)),
+            root: root,
             features: HashMap::new(),
             metadata: self.metadata.clone(),
         })
     }
 }
 
+fn build_path_deps(root: &Package,
+                   map: &mut HashMap<String, SourceId>,
+                   config: &Config)
+                   -> CargoResult<()> {
+    assert!(root.package_id().source_id().is_path());
+
+    let deps = root.dependencies()
+                   .iter()
+                   .map(|d| d.source_id())
+                   .filter(|id| id.is_path())
+                   .filter_map(|id| id.url().to_file_path().ok())
+                   .map(|path| path.join("Cargo.toml"))
+                   .filter_map(|path| Package::for_path(&path, config).ok());
+    for pkg in deps {
+        let source_id = pkg.package_id().source_id();
+        if map.insert(pkg.name().to_string(), source_id.clone()).is_none() {
+            try!(build_path_deps(&pkg, map, config));
+        }
+    }
+
+    Ok(())
+}
+
+fn to_package_id(name: &str,
+                 version: &str,
+                 source: Option<&SourceId>,
+                 default_source: &SourceId,
+                 path_sources: &HashMap<String, SourceId>)
+                 -> CargoResult<PackageId> {
+    let source = source.or(path_sources.get(name)).unwrap_or(default_source);
+    PackageId::new(name, version, source)
+}
+
+
 #[derive(RustcEncodable, RustcDecodable, Debug, PartialOrd, Ord, PartialEq, Eq)]
 pub struct EncodableDependency {
     name: String,
@@ -86,15 +133,6 @@ pub struct EncodableDependency {
     dependencies: Option<Vec<EncodablePackageId>>
 }
 
-impl EncodableDependency {
-    fn to_package_id(&self, default_source: &SourceId) -> CargoResult<PackageId> {
-        PackageId::new(
-            &self.name,
-            &self.version,
-            self.source.as_ref().unwrap_or(default_source))
-    }
-}
-
 #[derive(Debug, PartialOrd, Ord, PartialEq, Eq)]
 pub struct EncodablePackageId {
     name: String,
@@ -134,15 +172,6 @@ impl Decodable for EncodablePackageId {
     }
 }
 
-impl EncodablePackageId {
-    fn to_package_id(&self, default_source: &SourceId) -> CargoResult<PackageId> {
-        PackageId::new(
-            &self.name,
-            &self.version,
-            self.source.as_ref().unwrap_or(default_source))
-    }
-}
-
 impl Encodable for Resolve {
     fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
         let mut ids: Vec<&PackageId> = self.graph.iter().collect();
@@ -151,28 +180,26 @@ impl Encodable for Resolve {
         let encodable = ids.iter().filter_map(|&id| {
             if self.root == *id { return None; }
 
-            Some(encodable_resolve_node(id, &self.root, &self.graph))
+            Some(encodable_resolve_node(id, &self.graph))
         }).collect::<Vec<EncodableDependency>>();
 
         EncodableResolve {
             package: Some(encodable),
-            root: encodable_resolve_node(&self.root, &self.root, &self.graph),
+            root: encodable_resolve_node(&self.root, &self.graph),
             metadata: self.metadata.clone(),
         }.encode(s)
     }
 }
 
-fn encodable_resolve_node(id: &PackageId, root: &PackageId,
-                          graph: &Graph<PackageId>) -> EncodableDependency {
+fn encodable_resolve_node(id: &PackageId, graph: &Graph<PackageId>)
+                          -> EncodableDependency {
     let deps = graph.edges(id).map(|edge| {
-        let mut deps = edge.map(|e| {
-            encodable_package_id(e, root)
-        }).collect::<Vec<EncodablePackageId>>();
+        let mut deps = edge.map(encodable_package_id).collect::<Vec<_>>();
         deps.sort();
         deps
     });
 
-    let source = if id.source_id() == root.source_id() {
+    let source = if id.source_id().is_path() {
         None
     } else {
         Some(id.source_id().clone())
@@ -186,8 +213,8 @@ fn encodable_resolve_node(id: &PackageId, root: &PackageId,
     }
 }
 
-fn encodable_package_id(id: &PackageId, root: &PackageId) -> EncodablePackageId {
-    let source = if id.source_id() == root.source_id() {
+fn encodable_package_id(id: &PackageId) -> EncodablePackageId {
+    let source = if id.source_id().is_path() {
         None
     } else {
         Some(id.source_id().with_precise(None))

src/cargo/ops/cargo_compile.rs

@@ -32,8 +32,9 @@ use core::{Source, SourceId, PackageSet, Package, Target};
 use core::{Profile, TargetKind, Profiles};
 use core::resolver::{Method, Resolve};
 use ops::{self, BuildOutput, ExecEngine};
+use sources::PathSource;
 use util::config::Config;
-use util::{CargoResult, internal, ChainError, profile};
+use util::{CargoResult, profile};
 
 /// Contains information about how a package should be compiled.
 pub struct CompileOptions<'a> {
@@ -104,8 +105,6 @@ pub fn resolve_dependencies<'a>(root_package: &Package,
                                 no_default_features: bool)
                                 -> CargoResult<(PackageSet<'a>, Resolve)> {
 
-    let override_ids = try!(source_ids_from_config(config, root_package.root()));
-
     let mut registry = PackageRegistry::new(config);
 
     if let Some(source) = source {
@@ -114,14 +113,14 @@ pub fn resolve_dependencies<'a>(root_package: &Package,
 
     // First, resolve the root_package's *listed* dependencies, as well as
     // downloading and updating all remotes and such.
-    let resolve = try!(ops::resolve_pkg(&mut registry, root_package));
+    let resolve = try!(ops::resolve_pkg(&mut registry, root_package, config));
 
     // Second, resolve with precisely what we're doing. Filter out
     // transitive dependencies if necessary, specify features, handle
     // overrides, etc.
     let _p = profile::start("resolving w/ overrides...");
 
-    try!(registry.add_overrides(override_ids));
+    try!(add_overrides(&mut registry, root_package.root(), config));
 
     let method = Method::Required{
         dev_deps: true, // TODO: remove this option?
@@ -383,20 +382,14 @@ fn generate_targets<'a>(pkg: &'a Package,
 
 /// Read the `paths` configuration variable to discover all path overrides that
 /// have been configured.
-fn source_ids_from_config(config: &Config, cur_path: &Path)
-                          -> CargoResult<Vec<SourceId>> {
-
-    let configs = try!(config.values());
-    debug!("loaded config; configs={:?}", configs);
-    let config_paths = match configs.get("paths") {
-        Some(cfg) => cfg,
-        None => return Ok(Vec::new())
+fn add_overrides<'a>(registry: &mut PackageRegistry<'a>,
+                     cur_path: &Path,
+                     config: &'a Config) -> CargoResult<()> {
+    let paths = match try!(config.get_list("paths")) {
+        Some(list) => list,
+        None => return Ok(())
     };
-    let paths = try!(config_paths.list().chain_error(|| {
-        internal("invalid configuration for the key `paths`")
-    }));
-
-    paths.iter().map(|&(ref s, ref p)| {
+    let paths = paths.val.iter().map(|&(ref s, ref p)| {
         // The path listed next to the string is the config file in which the
         // key was located, so we want to pop off the `.cargo/config` component
         // to get the directory containing the `.cargo` folder.
@@ -405,7 +398,15 @@ fn source_ids_from_config(config: &Config, cur_path: &Path)
         // Make sure we don't override the local package, even if it's in the
         // list of override paths.
         cur_path != &**p
-    }).map(|p| SourceId::for_path(&p)).collect()
+    });
+
+    for path in paths {
+        let id = try!(SourceId::for_path(&path));
+        let mut source = PathSource::new_recursive(&path, &id, config);
+        try!(source.update());
+        registry.add_override(&id, Box::new(source));
+    }
+    Ok(())
 }
 
 /// Parse all config files to learn about build configuration. Currently

src/cargo/ops/cargo_fetch.rs

@@ -11,7 +11,7 @@ pub fn fetch<'a>(manifest_path: &Path,
                  -> CargoResult<(Resolve, PackageSet<'a>)> {
     let package = try!(Package::for_path(manifest_path, config));
     let mut registry = PackageRegistry::new(config);
-    let resolve = try!(ops::resolve_pkg(&mut registry, &package));
+    let resolve = try!(ops::resolve_pkg(&mut registry, &package, config));
     let packages = get_resolved_packages(&resolve, registry);
     for id in resolve.iter() {
         try!(packages.get(id));