Auto merge of #2406 - alexcrichton:download-less, r=brson

Currently Cargo will download an entire resolution graph all at once when in fact most packages may not be relevant to a compilation. For example target-specific dependencies and dev-dependencies are unconditionally downloaded regardless of whether they're actually needed or not.

This commit alters the internals of Cargo to avoid downloading everything immediately and just switches to lazily downloading packages. This involved adding a new `LazyCell` primitive (similar to the one in use on crates.io) and also propagates `CargoResult` in a few more locations.

Overall this ended up being a pretty large refactoring so the commits are separated in bite-sized chunks as much as possible with the end goal being this PR itself.

Closes #2394

Author: bors

src/cargo/core/mod.rs

@@ -6,7 +6,7 @@ pub use self::package_id_spec::PackageIdSpec;
 pub use self::registry::Registry;
 pub use self::resolver::Resolve;
 pub use self::shell::{Shell, MultiShell, ShellConfig, Verbosity, ColorConfig};
-pub use self::source::{Source, SourceId, SourceMap, SourceSet, GitReference};
+pub use self::source::{Source, SourceId, SourceMap, GitReference};
 pub use self::summary::Summary;
 
 pub mod source;

src/cargo/core/package.rs

@@ -1,15 +1,16 @@
+use std::cell::{Ref, RefCell};
 use std::collections::HashMap;
 use std::fmt;
 use std::hash;
-use std::slice;
 use std::path::{Path, PathBuf};
+
 use semver::Version;
 
-use core::{Dependency, Manifest, PackageId, SourceId, Registry, Target, Summary, Metadata};
+use core::{Dependency, Manifest, PackageId, SourceId, Target};
+use core::{Summary, Metadata, SourceMap};
 use ops;
-use util::{CargoResult, graph, Config};
+use util::{CargoResult, Config, LazyCell, ChainError, internal, human};
 use rustc_serialize::{Encoder,Encodable};
-use core::source::Source;
 
 /// Information about a package that is available somewhere in the file system.
 ///
@@ -118,78 +119,46 @@ impl hash::Hash for Package {
     }
 }
 
-#[derive(PartialEq,Clone,Debug)]
-pub struct PackageSet {
-    packages: Vec<Package>,
+pub struct PackageSet<'cfg> {
+    packages: Vec<(PackageId, LazyCell<Package>)>,
+    sources: RefCell<SourceMap<'cfg>>,
 }
 
-impl PackageSet {
-    pub fn new(packages: &[Package]) -> PackageSet {
-        //assert!(packages.len() > 0,
-        //        "PackageSet must be created with at least one package")
-        PackageSet { packages: packages.to_vec() }
-    }
-
-    pub fn is_empty(&self) -> bool {
-        self.packages.is_empty()
-    }
-
-    pub fn len(&self) -> usize {
-        self.packages.len()
-    }
-
-    pub fn pop(&mut self) -> Package {
-        self.packages.pop().expect("PackageSet.pop: empty set")
-    }
-
-    /// Get a package by name out of the set
-    pub fn get(&self, name: &str) -> &Package {
-        self.packages.iter().find(|pkg| name == pkg.name())
-            .expect("PackageSet.get: empty set")
-    }
-
-    pub fn get_all(&self, names: &[&str]) -> Vec<&Package> {
-        names.iter().map(|name| self.get(*name) ).collect()
-    }
-
-    pub fn packages(&self) -> &[Package] { &self.packages }
-
-    // For now, assume that the package set contains only one package with a
-    // given name
-    pub fn sort(&self) -> Option<PackageSet> {
-        let mut graph = graph::Graph::new();
-
-        for pkg in self.packages.iter() {
-            let deps: Vec<&str> = pkg.dependencies().iter()
-                .map(|dep| dep.name())
-                .collect();
-
-            graph.add(pkg.name(), &deps);
+impl<'cfg> PackageSet<'cfg> {
+    pub fn new(package_ids: &[PackageId],
+               sources: SourceMap<'cfg>) -> PackageSet<'cfg> {
+        PackageSet {
+            packages: package_ids.iter().map(|id| {
+                (id.clone(), LazyCell::new(None))
+            }).collect(),
+            sources: RefCell::new(sources),
         }
-
-        let pkgs = match graph.sort() {
-            Some(pkgs) => pkgs,
-            None => return None,
-        };
-        let pkgs = pkgs.iter().map(|name| {
-            self.get(*name).clone()
-        }).collect();
-
-        Some(PackageSet {
-            packages: pkgs
-        })
     }
 
-    pub fn iter(&self) -> slice::Iter<Package> {
-        self.packages.iter()
+    pub fn package_ids<'a>(&'a self) -> Box<Iterator<Item=&'a PackageId> + 'a> {
+        Box::new(self.packages.iter().map(|&(ref p, _)| p))
     }
-}
 
-impl Registry for PackageSet {
-    fn query(&mut self, name: &Dependency) -> CargoResult<Vec<Summary>> {
-        Ok(self.packages.iter()
-            .filter(|pkg| name.name() == pkg.name())
-            .map(|pkg| pkg.summary().clone())
-            .collect())
+    pub fn get(&self, id: &PackageId) -> CargoResult<&Package> {
+        let slot = try!(self.packages.iter().find(|p| p.0 == *id).chain_error(|| {
+            internal(format!("couldn't find `{}` in package set", id))
+        }));
+        let slot = &slot.1;
+        if let Some(pkg) = slot.borrow() {
+            return Ok(pkg)
+        }
+        let mut sources = self.sources.borrow_mut();
+        let source = try!(sources.get_mut(id.source_id()).chain_error(|| {
+            internal(format!("couldn't find source for `{}`", id))
+        }));
+        let pkg = try!(source.download(id).chain_error(|| {
+            human("unable to get packages from source")
+        }));
+        assert!(slot.fill(pkg).is_ok());
+        Ok(slot.borrow().unwrap())
+    }
+
+    pub fn sources(&self) -> Ref<SourceMap<'cfg>> {
+        self.sources.borrow()
     }
 }

src/cargo/core/registry.rs

@@ -1,7 +1,7 @@
-use std::collections::HashSet;
-use std::collections::hash_map::HashMap;
+use std::collections::{HashSet, HashMap};
 
 use core::{Source, SourceId, SourceMap, Summary, Dependency, PackageId, Package};
+use core::PackageSet;
 use util::{CargoResult, ChainError, Config, human, profile};
 
 /// Source of information about a group of packages.
@@ -85,30 +85,9 @@ impl<'cfg> PackageRegistry<'cfg> {
         }
     }
 
-    pub fn get(&mut self, package_ids: &[PackageId]) -> CargoResult<Vec<Package>> {
+    pub fn get(self, package_ids: &[PackageId]) -> PackageSet<'cfg> {
         trace!("getting packages; sources={}", self.sources.len());
-
-        // TODO: Only call source with package ID if the package came from the
-        // source
-        let mut ret = Vec::new();
-
-        for (_, source) in self.sources.sources_mut() {
-            try!(source.download(package_ids));
-            let packages = try!(source.get(package_ids));
-
-            ret.extend(packages.into_iter());
-        }
-
-        // TODO: Return earlier if fail
-        assert!(package_ids.len() == ret.len(),
-                "could not get packages from registry; ids={:?}; ret={:?}",
-                package_ids, ret);
-
-        Ok(ret)
-    }
-
-    pub fn move_sources(self) -> SourceMap<'cfg> {
-        self.sources
+        PackageSet::new(package_ids, self.sources)
     }
 
     fn ensure_loaded(&mut self, namespace: &SourceId, kind: Kind) -> CargoResult<()> {

src/cargo/core/source.rs

@@ -9,7 +9,7 @@ use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
 
 use url::Url;
 
-use core::{Summary, Package, PackageId, Registry, Dependency};
+use core::{Package, PackageId, Registry};
 use sources::{PathSource, GitSource, RegistrySource};
 use sources::git;
 use util::{human, Config, CargoResult, ToUrl};
@@ -24,13 +24,7 @@ pub trait Source: Registry {
 
     /// The download method fetches the full package for each name and
     /// version specified.
-    fn download(&mut self, packages: &[PackageId]) -> CargoResult<()>;
-
-    /// The get method returns the Path of each specified package on the
-    /// local file system. It assumes that `download` was already called,
-    /// and that the packages are already locally available on the file
-    /// system.
-    fn get(&self, packages: &[PackageId]) -> CargoResult<Vec<Package>>;
+    fn download(&mut self, package: &PackageId) -> CargoResult<Package>;
 
     /// Generates a unique string which represents the fingerprint of the
     /// current state of the source.
@@ -443,65 +437,6 @@ impl<'a, 'src> Iterator for SourcesMut<'a, 'src> {
     }
 }
 
-/// List of `Source` implementors. `SourceSet` itself implements `Source`.
-pub struct SourceSet<'src> {
-    sources: Vec<Box<Source+'src>>
-}
-
-impl<'src> SourceSet<'src> {
-    pub fn new(sources: Vec<Box<Source+'src>>) -> SourceSet<'src> {
-        SourceSet { sources: sources }
-    }
-}
-
-impl<'src> Registry for SourceSet<'src> {
-    fn query(&mut self, name: &Dependency) -> CargoResult<Vec<Summary>> {
-        let mut ret = Vec::new();
-
-        for source in self.sources.iter_mut() {
-            ret.extend(try!(source.query(name)).into_iter());
-        }
-
-        Ok(ret)
-    }
-}
-
-impl<'src> Source for SourceSet<'src> {
-    fn update(&mut self) -> CargoResult<()> {
-        for source in self.sources.iter_mut() {
-            try!(source.update());
-        }
-
-        Ok(())
-    }
-
-    fn download(&mut self, packages: &[PackageId]) -> CargoResult<()> {
-        for source in self.sources.iter_mut() {
-            try!(source.download(packages));
-        }
-
-        Ok(())
-    }
-
-    fn get(&self, packages: &[PackageId]) -> CargoResult<Vec<Package>> {
-        let mut ret = Vec::new();
-
-        for source in self.sources.iter() {
-            ret.extend(try!(source.get(packages)).into_iter());
-        }
-
-        Ok(ret)
-    }
-
-    fn fingerprint(&self, id: &Package) -> CargoResult<String> {
-        let mut ret = String::new();
-        for source in self.sources.iter() {
-            ret.push_str(&try!(source.fingerprint(id))[..]);
-        }
-        Ok(ret)
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::{SourceId, Kind, GitReference};

src/cargo/ops/cargo_clean.rs

@@ -2,9 +2,8 @@ use std::default::Default;
 use std::fs;
 use std::path::Path;
 
-use core::{Package, PackageSet, Profiles};
-use core::source::{Source, SourceMap};
-use core::registry::PackageRegistry;
+use core::{Package, Profiles};
+use core::source::Source;
 use util::{CargoResult, human, ChainError, Config};
 use ops::{self, Layout, Context, BuildConfig, Kind, Unit};
 
@@ -26,41 +25,24 @@ pub fn clean(manifest_path: &Path, opts: &CleanOptions) -> CargoResult<()> {
         return rm_rf(&target_dir);
     }
 
-    // Load the lockfile (if one's available)
-    let lockfile = root.root().join("Cargo.lock");
-    let source_id = root.package_id().source_id();
-    let resolve = match try!(ops::load_lockfile(&lockfile, source_id)) {
-        Some(resolve) => resolve,
-        None => bail!("a Cargo.lock must exist before cleaning")
-    };
-
-    // Create a compilation context to have access to information like target
-    // filenames and such
-    let srcs = SourceMap::new();
-    let pkgs = PackageSet::new(&[]);
+    let (resolve, packages) = try!(ops::fetch(manifest_path, opts.config));
 
     let dest = if opts.release {"release"} else {"debug"};
     let host_layout = Layout::new(opts.config, &root, None, dest);
     let target_layout = opts.target.map(|target| {
         Layout::new(opts.config, &root, Some(target), dest)
     });
 
-    let cx = try!(Context::new(&resolve, &srcs, &pkgs, opts.config,
+    let cx = try!(Context::new(&resolve, &packages, opts.config,
                                host_layout, target_layout,
                                BuildConfig::default(),
                                root.manifest().profiles()));
 
-    let mut registry = PackageRegistry::new(opts.config);
-
     // resolve package specs and remove the corresponding packages
     for spec in opts.spec {
+        // Translate the spec to a Package
         let pkgid = try!(resolve.query(spec));
-
-        // Translate the PackageId to a Package
-        let pkg = {
-            try!(registry.add_sources(&[pkgid.source_id().clone()]));
-            (try!(registry.get(&[pkgid.clone()]))).into_iter().next().unwrap()
-        };
+        let pkg = try!(packages.get(&pkgid));
 
         // And finally, clean everything out!
         for target in pkg.targets() {

src/cargo/ops/cargo_compile.rs

@@ -28,7 +28,7 @@ use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use core::registry::PackageRegistry;
-use core::{Source, SourceId, SourceMap, PackageSet, Package, Target};
+use core::{Source, SourceId, PackageSet, Package, Target};
 use core::{Profile, TargetKind, Profiles};
 use core::resolver::{Method, Resolve};
 use ops::{self, BuildOutput, ExecEngine};
@@ -102,7 +102,7 @@ pub fn resolve_dependencies<'a>(root_package: &Package,
                                 source: Option<Box<Source + 'a>>,
                                 features: Vec<String>,
                                 no_default_features: bool)
-                                -> CargoResult<(Vec<Package>, Resolve, SourceMap<'a>)> {
+                                -> CargoResult<(PackageSet<'a>, Resolve)> {
 
     let override_ids = try!(source_ids_from_config(config, root_package.root()));
 
@@ -133,10 +133,10 @@ pub fn resolve_dependencies<'a>(root_package: &Package,
             try!(ops::resolve_with_previous(&mut registry, root_package,
                                             method, Some(&resolve), None));
 
-    let packages = try!(ops::get_resolved_packages(&resolved_with_overrides,
-                                                   &mut registry));
+    let packages = ops::get_resolved_packages(&resolved_with_overrides,
+                                              registry);
 
-    Ok((packages, resolved_with_overrides, registry.move_sources()))
+    Ok((packages, resolved_with_overrides))
 }
 
 pub fn compile_pkg<'a>(root_package: &Package,
@@ -158,7 +158,7 @@ pub fn compile_pkg<'a>(root_package: &Package,
         bail!("jobs must be at least 1")
     }
 
-    let (packages, resolve_with_overrides, sources) = {
+    let (packages, resolve_with_overrides) = {
         try!(resolve_dependencies(root_package, config, source, features,
                                   no_default_features))
     };
@@ -180,8 +180,9 @@ pub fn compile_pkg<'a>(root_package: &Package,
               invalid_spec.join(", "))
     }
 
-    let to_builds = packages.iter().filter(|p| pkgids.contains(&p.package_id()))
-                            .collect::<Vec<_>>();
+    let to_builds = try!(pkgids.iter().map(|id| {
+        packages.get(id)
+    }).collect::<CargoResult<Vec<_>>>());
 
     let mut general_targets = Vec::new();
     let mut package_targets = Vec::new();
@@ -245,9 +246,8 @@ pub fn compile_pkg<'a>(root_package: &Package,
         }
 
         try!(ops::compile_targets(&package_targets,
-                                  &PackageSet::new(&packages),
+                                  &packages,
                                   &resolve_with_overrides,
-                                  &sources,
                                   config,
                                   build_config,
                                   root_package.manifest().profiles(),