Skip to content

Commit

Permalink
Stop downloading all packages immediately
Browse files Browse the repository at this point in the history
Instead delay all downloads until just before they're needed. This should ensure
that we don't download any more packages than we really need to.
  • Loading branch information
alexcrichton committed Feb 26, 2016
1 parent 078b670 commit f994592
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 40 deletions.
12 changes: 7 additions & 5 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use semver::Version;
use core::{Dependency, Manifest, PackageId, SourceId, Target};
use core::{Summary, Metadata, SourceMap};
use ops;
use util::{CargoResult, Config, LazyCell, ChainError, internal};
use util::{CargoResult, Config, LazyCell, ChainError, internal, human};
use rustc_serialize::{Encoder,Encodable};

/// Information about a package that is available somewhere in the file system.
Expand Down Expand Up @@ -125,11 +125,11 @@ pub struct PackageSet<'cfg> {
}

impl<'cfg> PackageSet<'cfg> {
pub fn new(packages: Vec<Package>,
pub fn new(package_ids: &[PackageId],
sources: SourceMap<'cfg>) -> PackageSet<'cfg> {
PackageSet {
packages: packages.into_iter().map(|pkg| {
(pkg.package_id().clone(), LazyCell::new(Some(pkg)))
packages: package_ids.iter().map(|id| {
(id.clone(), LazyCell::new(None))
}).collect(),
sources: RefCell::new(sources),
}
Expand All @@ -151,7 +151,9 @@ impl<'cfg> PackageSet<'cfg> {
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));
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())
}
Expand Down
15 changes: 3 additions & 12 deletions src/cargo/core/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::{HashSet, HashMap};

use core::{Source, SourceId, SourceMap, Summary, Dependency, PackageId, Package};
use core::PackageSet;
use util::{CargoResult, ChainError, Config, human, internal, profile};
use util::{CargoResult, ChainError, Config, human, profile};

/// Source of information about a group of packages.
///
Expand Down Expand Up @@ -85,18 +85,9 @@ impl<'cfg> PackageRegistry<'cfg> {
}
}

pub fn get(mut self, package_ids: &[PackageId])
-> CargoResult<PackageSet<'cfg>> {
pub fn get(self, package_ids: &[PackageId]) -> PackageSet<'cfg> {
trace!("getting packages; sources={}", self.sources.len());

let pkgs = try!(package_ids.iter().map(|id| {
let src = try!(self.sources.get_mut(id.source_id()).chain_error(|| {
internal(format!("failed to find a source listed for `{}`", id))
}));
src.download(id)
}).collect::<CargoResult<Vec<_>>>());

Ok(PackageSet::new(pkgs, self.sources))
PackageSet::new(package_ids, self.sources)
}

fn ensure_loaded(&mut self, namespace: &SourceId, kind: Kind) -> CargoResult<()> {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ 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,
registry));
let packages = ops::get_resolved_packages(&resolved_with_overrides,
registry);

Ok((packages, resolved_with_overrides))
}
Expand Down
16 changes: 8 additions & 8 deletions src/cargo/ops/cargo_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::path::Path;
use core::registry::PackageRegistry;
use core::{Package, PackageId, Resolve, PackageSet};
use ops;
use util::{CargoResult, Config, human, ChainError};
use util::{CargoResult, Config};

/// Executes `cargo fetch`.
pub fn fetch<'a>(manifest_path: &Path,
Expand All @@ -12,16 +12,16 @@ pub fn fetch<'a>(manifest_path: &Path,
let package = try!(Package::for_path(manifest_path, config));
let mut registry = PackageRegistry::new(config);
let resolve = try!(ops::resolve_pkg(&mut registry, &package));
get_resolved_packages(&resolve, registry).map(move |packages| {
(resolve, packages)
})
let packages = get_resolved_packages(&resolve, registry);
for id in resolve.iter() {
try!(packages.get(id));
}
Ok((resolve, packages))
}

pub fn get_resolved_packages<'a>(resolve: &Resolve,
registry: PackageRegistry<'a>)
-> CargoResult<PackageSet<'a>> {
-> PackageSet<'a> {
let ids: Vec<PackageId> = resolve.iter().cloned().collect();
registry.get(&ids).chain_error(|| {
human("unable to get packages from source")
})
registry.get(&ids)
}
58 changes: 45 additions & 13 deletions tests/test_cargo_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ test!(login_with_no_cargo_dir {
});

test!(bad_license_file {
Package::new("foo", "1.0.0").publish();
let p = project("all")
.file("Cargo.toml", r#"
[project]
Expand Down Expand Up @@ -957,21 +958,52 @@ test!(update_same_prefix_oh_my_how_was_this_a_bug {
execs().with_status(0));
});

test!(use_semver {
let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "bar"
version = "0.5.0"
authors = []
test!(use_semver {
let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "bar"
version = "0.5.0"
authors = []
[dependencies]
foo = "1.2.3-alpha.0"
"#)
.file("src/main.rs", "fn main() {}");
p.build();
[dependencies]
foo = "1.2.3-alpha.0"
"#)
.file("src/main.rs", "fn main() {}");
p.build();

Package::new("foo", "1.2.3-alpha.0").publish();
Package::new("foo", "1.2.3-alpha.0").publish();

assert_that(p.cargo("build"), execs().with_status(0));
});

test!(only_download_relevant {
let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "bar"
version = "0.5.0"
authors = []
[target.foo.dependencies]
foo = "*"
[dev-dependencies]
bar = "*"
[dependencies]
baz = "*"
"#)
.file("src/main.rs", "fn main() {}");
p.build();

Package::new("foo", "0.1.0").publish();
Package::new("bar", "0.1.0").publish();
Package::new("baz", "0.1.0").publish();

assert_that(p.cargo("build"),
execs().with_status(0).with_stdout(&format!("\
{updating} registry `[..]`
{downloading} baz v0.1.0 ([..])
{compiling} baz v0.1.0 ([..])
{compiling} bar v0.5.0 ([..])
", downloading = DOWNLOADING, compiling = COMPILING, updating = UPDATING)));
});

0 comments on commit f994592

Please sign in to comment.