Skip to content

Commit

Permalink
Avoid updating the registry too frequently
Browse files Browse the repository at this point in the history
This logic is based off the precision of the registry source's id as well as the
dependencies being passed in. More info can be found in the comments.
  • Loading branch information
alexcrichton committed Oct 27, 2014
1 parent 33e0017 commit 640487c
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 26 deletions.
5 changes: 2 additions & 3 deletions src/cargo/ops/cargo_generate_lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ pub fn update_lockfile(manifest_path: &Path,
let mut config = try!(Config::new(opts.shell, None, None));
let mut registry = PackageRegistry::new(&mut config);
let mut to_avoid = HashSet::new();
let mut previous = Some(&previous_resolve);

match opts.to_update {
Some(name) => {
Expand All @@ -69,13 +68,13 @@ pub fn update_lockfile(manifest_path: &Path,
}
}
}
None => { previous = None; }
None => to_avoid.extend(previous_resolve.iter()),
}

let resolve = try!(ops::resolve_with_previous(&mut registry,
&package,
resolver::ResolveEverything,
previous,
Some(&previous_resolve),
Some(&to_avoid)));
try!(ops::write_pkg_lockfile(&package, &resolve));
return Ok(());
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,

let mut resolved = try!(resolver::resolve(&summary, method, registry));
match previous {
Some(r) => resolved.copy_metadata(previous),
Some(r) => resolved.copy_metadata(r),
None => {}
}
return Ok(resolved);
Expand Down
52 changes: 40 additions & 12 deletions src/cargo/sources/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ pub struct RegistrySource<'a, 'b:'a> {
sources: Vec<PathSource>,
hashes: HashMap<(String, String), String>, // (name, vers) => cksum
cache: HashMap<String, Vec<(Summary, bool)>>,
updated: bool,
}

#[deriving(Decodable)]
Expand Down Expand Up @@ -239,6 +240,7 @@ impl<'a, 'b> RegistrySource<'a, 'b> {
sources: Vec::new(),
hashes: HashMap::new(),
cache: HashMap::new(),
updated: false,
}
}

Expand Down Expand Up @@ -420,20 +422,11 @@ impl<'a, 'b> RegistrySource<'a, 'b> {
.default_features(default_features)
.features(features))
}
}

impl<'a, 'b> Registry for RegistrySource<'a, 'b> {
fn query(&mut self, dep: &Dependency) -> CargoResult<Vec<Summary>> {
let summaries = try!(self.summaries(dep.get_name()));
let mut summaries = summaries.iter().filter(|&&(_, yanked)| {
dep.get_source_id().get_precise().is_some() || !yanked
}).map(|&(ref s, _)| s.clone()).collect::<Vec<_>>();
summaries.query(dep)
}
}
/// Actually perform network operations to update the registry
fn do_update(&mut self) -> CargoResult<()> {
if self.updated { return Ok(()) }

impl<'a, 'b> Source for RegistrySource<'a, 'b> {
fn update(&mut self) -> CargoResult<()> {
try!(self.config.shell().status("Updating",
format!("registry `{}`", self.source_id.get_url())));
let repo = try!(self.open());
Expand All @@ -451,6 +444,41 @@ impl<'a, 'b> Source for RegistrySource<'a, 'b> {
log!(5, "[{}] updating to rev {}", self.source_id, oid);
let object = try!(repo.find_object(oid, None));
try!(repo.reset(&object, git2::Hard, None, None));
self.updated = true;
self.cache.clear();
Ok(())
}
}

impl<'a, 'b> Registry for RegistrySource<'a, 'b> {
fn query(&mut self, dep: &Dependency) -> CargoResult<Vec<Summary>> {
// If this is a precise dependency, then it came from a lockfile and in
// theory the registry is known to contain this version. If, however, we
// come back with no summaries, then our registry may need to be
// updated, so we fall back to performing a lazy update.
if dep.get_source_id().get_precise().is_some() &&
try!(self.summaries(dep.get_name())).len() == 0 {
try!(self.do_update());
}

let summaries = try!(self.summaries(dep.get_name()));
let mut summaries = summaries.iter().filter(|&&(_, yanked)| {
dep.get_source_id().get_precise().is_some() || !yanked
}).map(|&(ref s, _)| s.clone()).collect::<Vec<_>>();
summaries.query(dep)
}
}

impl<'a, 'b> Source for RegistrySource<'a, 'b> {
fn update(&mut self) -> CargoResult<()> {
// If we have an imprecise version then we don't know what we're going
// to look for, so we always atempt to perform an update here.
//
// If we have a precise version, then we'll update lazily during the
// querying phase.
if self.source_id.get_precise().is_none() {
try!(self.do_update());
}
Ok(())
}

Expand Down
76 changes: 66 additions & 10 deletions tests/test_cargo_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::io::{fs, File};

use support::{project, execs, cargo_dir};
use support::{UPDATING, DOWNLOADING, COMPILING, PACKAGING, VERIFYING};
use support::paths::PathExt;
use support::paths::{mod, PathExt};
use support::registry as r;

use hamcrest::assert_that;
Expand Down Expand Up @@ -249,9 +249,7 @@ test!(lockfile_locks {
r::mock_pkg("bar", "0.0.2", []);

assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0).with_stdout(format!("\
{updating} registry `[..]`
", updating = UPDATING).as_slice()));
execs().with_status(0).with_stdout(""));
})

test!(lockfile_locks_transitively {
Expand Down Expand Up @@ -287,9 +285,7 @@ test!(lockfile_locks_transitively {
r::mock_pkg("bar", "0.0.2", [("baz", "*")]);

assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0).with_stdout(format!("\
{updating} registry `[..]`
", updating = UPDATING).as_slice()));
execs().with_status(0).with_stdout(""));
})

test!(yanks_are_not_used {
Expand Down Expand Up @@ -373,9 +369,7 @@ test!(yanks_in_lockfiles_are_ok {
r::mock_pkg_yank("bar", "0.0.1", [], true);

assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0).with_stdout(format!("\
{updating} registry `[..]`
", updating = UPDATING).as_slice()));
execs().with_status(0).with_stdout(""));

assert_that(p.process(cargo_dir().join("cargo")).arg("update"),
execs().with_status(101).with_stderr("\
Expand All @@ -384,3 +378,65 @@ location searched: the package registry
version required: *
"));
})

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

r::mock_pkg("bar", "0.0.1", []);
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0));
p.root().move_into_the_past().unwrap();

fs::rmdir_recursive(&paths::home().join(".cargo/registry")).unwrap();
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0).with_stdout(format!("\
{updating} registry `[..]`
{downloading} bar v0.0.1 (the package registry)
", updating = UPDATING, downloading = DOWNLOADING).as_slice()));
})

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

r::mock_pkg("bar", "0.0.1", []);
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0));

r::mock_pkg("bar", "0.0.2", []);
fs::rmdir_recursive(&paths::home().join(".cargo/registry")).unwrap();
assert_that(p.process(cargo_dir().join("cargo")).arg("update")
.arg("-p").arg("bar"),
execs().with_status(0).with_stdout(format!("\
{updating} registry `[..]`
", updating = UPDATING).as_slice()));

assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0).with_stdout(format!("\
{downloading} [..] v0.0.2 (the package registry)
{compiling} bar v0.0.2 (the package registry)
{compiling} foo v0.0.1 ({dir})
", downloading = DOWNLOADING, compiling = COMPILING,
dir = p.url()).as_slice()));
})

0 comments on commit 640487c

Please sign in to comment.