Fix decoding lock files with path dependencies

alexcrichton · alexcrichton · commit f28c7872e66e · 2016-03-03T13:35:46.000-08:00
With the previous changes a path dependency must have the precise path to it
listed in its package id. Currently when decoding a lockfile, however, all path
dependencies have the same package id, which unfortunately causes a mismatch.

This commit alters the decoding of a lockfile to perform some simple path
traversals to probe the filesystem to understand where path dependencies are and
set the right package id for the found packages.
diff --git a/src/cargo/core/resolver/encode.rs b/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))
diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs
@@ -113,7 +113,7 @@ 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
diff --git a/src/cargo/ops/cargo_fetch.rs b/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));
diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs
@@ -31,7 +31,8 @@ pub fn update_lockfile(manifest_path: &Path,
                        opts: &UpdateOptions) -> CargoResult<()> {
     let package = try!(Package::for_path(manifest_path, opts.config));
 
-    let previous_resolve = match try!(ops::load_pkg_lockfile(&package)) {
+    let previous_resolve = match try!(ops::load_pkg_lockfile(&package,
+                                                             opts.config)) {
         Some(resolve) => resolve,
         None => bail!("a Cargo.lock must exist before it is updated")
     };
diff --git a/src/cargo/ops/cargo_pkgid.rs b/src/cargo/ops/cargo_pkgid.rs
@@ -10,8 +10,7 @@ pub fn pkgid(manifest_path: &Path,
     let package = try!(Package::for_path(manifest_path, config));
 
     let lockfile = package.root().join("Cargo.lock");
-    let source_id = package.package_id().source_id();
-    let resolve = match try!(ops::load_lockfile(&lockfile, source_id)) {
+    let resolve = match try!(ops::load_lockfile(&lockfile, &package, config)) {
         Some(resolve) => resolve,
         None => bail!("a Cargo.lock must exist for this command"),
     };
diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs
@@ -1,11 +1,11 @@
 use std::collections::{HashMap, HashSet};
-use std::fs::{self, File};
+use std::fs;
 use std::io::prelude::*;
 use std::io;
 use std::path::{Path, PathBuf};
 
 use core::{Package, Manifest, SourceId, PackageId};
-use util::{self, CargoResult, human, Config, ChainError};
+use util::{self, paths, CargoResult, human, Config, ChainError};
 use util::important_paths::find_project_manifest_exact;
 use util::toml::{Layout, project_layout};
 
@@ -22,13 +22,11 @@ pub fn read_manifest(contents: &[u8], layout: Layout, source_id: &SourceId,
 pub fn read_package(path: &Path, source_id: &SourceId, config: &Config)
                     -> CargoResult<(Package, Vec<PathBuf>)> {
     trace!("read_package; path={}; source-id={}", path.display(), source_id);
-    let mut file = try!(File::open(path));
-    let mut data = Vec::new();
-    try!(file.read_to_end(&mut data));
+    let data = try!(paths::read(path));
 
     let layout = project_layout(path.parent().unwrap());
     let (manifest, nested) =
-        try!(read_manifest(&data, layout, source_id, config));
+        try!(read_manifest(data.as_bytes(), layout, source_id, config));
 
     Ok((Package::new(manifest, path), nested))
 }
diff --git a/src/cargo/ops/lockfile.rs b/src/cargo/ops/lockfile.rs
@@ -5,19 +5,20 @@ use std::path::Path;
 use rustc_serialize::{Encodable, Decodable};
 use toml::{self, Encoder, Value};
 
-use core::{Resolve, resolver, Package, SourceId};
-use util::{CargoResult, ChainError, human, paths};
+use core::{Resolve, resolver, Package};
+use util::{CargoResult, ChainError, human, paths, Config};
 use util::toml as cargo_toml;
 
-pub fn load_pkg_lockfile(pkg: &Package) -> CargoResult<Option<Resolve>> {
+pub fn load_pkg_lockfile(pkg: &Package, config: &Config)
+                         -> CargoResult<Option<Resolve>> {
     let lockfile = pkg.root().join("Cargo.lock");
-    let source_id = pkg.package_id().source_id();
-    load_lockfile(&lockfile, source_id).chain_error(|| {
+    load_lockfile(&lockfile, pkg, config).chain_error(|| {
         human(format!("failed to parse lock file at: {}", lockfile.display()))
     })
 }
 
-pub fn load_lockfile(path: &Path, sid: &SourceId) -> CargoResult<Option<Resolve>> {
+pub fn load_lockfile(path: &Path, pkg: &Package, config: &Config)
+                     -> CargoResult<Option<Resolve>> {
     // If there is no lockfile, return none.
     let mut f = match File::open(path) {
         Ok(f) => f,
@@ -30,7 +31,7 @@ pub fn load_lockfile(path: &Path, sid: &SourceId) -> CargoResult<Option<Resolve>
     let table = toml::Value::Table(try!(cargo_toml::parse(&s, path)));
     let mut d = toml::Decoder::new(table);
     let v: resolver::EncodableResolve = try!(Decodable::decode(&mut d));
-    Ok(Some(try!(v.to_resolve(sid))))
+    Ok(Some(try!(v.to_resolve(pkg, config))))
 }
 
 pub fn write_pkg_lockfile(pkg: &Package, resolve: &Resolve) -> CargoResult<()> {
diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
@@ -4,16 +4,18 @@ use core::{Package, PackageId, SourceId};
 use core::registry::PackageRegistry;
 use core::resolver::{self, Resolve, Method};
 use ops;
-use util::CargoResult;
+use util::{CargoResult, Config};
 
 /// Resolve all dependencies for the specified `package` using the previous
 /// lockfile as a guide if present.
 ///
 /// This function will also write the result of resolution as a new
 /// lockfile.
-pub fn resolve_pkg(registry: &mut PackageRegistry, package: &Package)
+pub fn resolve_pkg(registry: &mut PackageRegistry,
+                   package: &Package,
+                   config: &Config)
                    -> CargoResult<Resolve> {
-    let prev = try!(ops::load_pkg_lockfile(package));
+    let prev = try!(ops::load_pkg_lockfile(package, config));
     let resolve = try!(resolve_with_previous(registry, package,
                                              Method::Everything,
                                              prev.as_ref(), None));
