Skip to content

Commit 640487c

Browse files
alexcrichtonalexcrichton
committed
Avoid updating the registry too frequently
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.
1 parent 33e0017 commit 640487c

File tree

4 files changed

+109
-26
lines changed

4 files changed

+109
-26
lines changed

‎src/cargo/ops/cargo_generate_lockfile.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ pub fn update_lockfile(manifest_path: &Path,
4949
let mut config = try!(Config::new(opts.shell, None, None));
5050
let mut registry = PackageRegistry::new(&mut config);
5151
let mut to_avoid = HashSet::new();
52-
let mut previous = Some(&previous_resolve);
5352

5453
match opts.to_update {
5554
Some(name) => {
@@ -69,13 +68,13 @@ pub fn update_lockfile(manifest_path: &Path,
6968
}
7069
}
7170
}
72-
None => { previous = None; }
71+
None => to_avoid.extend(previous_resolve.iter()),
7372
}
7473

7574
let resolve = try!(ops::resolve_with_previous(&mut registry,
7675
&package,
7776
resolver::ResolveEverything,
78-
previous,
77+
Some(&previous_resolve),
7978
Some(&to_avoid)));
8079
try!(ops::write_pkg_lockfile(&package, &resolve));
8180
return Ok(());

‎src/cargo/ops/resolve.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
8787

8888
let mut resolved = try!(resolver::resolve(&summary, method, registry));
8989
match previous {
90-
Some(r) => resolved.copy_metadata(previous),
90+
Some(r) => resolved.copy_metadata(r),
9191
None => {}
9292
}
9393
return Ok(resolved);

‎src/cargo/sources/registry.rs

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ pub struct RegistrySource<'a, 'b:'a> {
189189
sources: Vec<PathSource>,
190190
hashes: HashMap<(String, String), String>, // (name, vers) => cksum
191191
cache: HashMap<String, Vec<(Summary, bool)>>,
192+
updated: bool,
192193
}
193194

194195
#[deriving(Decodable)]
@@ -239,6 +240,7 @@ impl<'a, 'b> RegistrySource<'a, 'b> {
239240
sources: Vec::new(),
240241
hashes: HashMap::new(),
241242
cache: HashMap::new(),
243+
updated: false,
242244
}
243245
}
244246

@@ -420,20 +422,11 @@ impl<'a, 'b> RegistrySource<'a, 'b> {
420422
.default_features(default_features)
421423
.features(features))
422424
}
423-
}
424425

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

435-
impl<'a, 'b> Source for RegistrySource<'a, 'b> {
436-
fn update(&mut self) -> CargoResult<()> {
437430
try!(self.config.shell().status("Updating",
438431
format!("registry `{}`", self.source_id.get_url())));
439432
let repo = try!(self.open());
@@ -451,6 +444,41 @@ impl<'a, 'b> Source for RegistrySource<'a, 'b> {
451444
log!(5, "[{}] updating to rev {}", self.source_id, oid);
452445
let object = try!(repo.find_object(oid, None));
453446
try!(repo.reset(&object, git2::Hard, None, None));
447+
self.updated = true;
448+
self.cache.clear();
449+
Ok(())
450+
}
451+
}
452+
453+
impl<'a, 'b> Registry for RegistrySource<'a, 'b> {
454+
fn query(&mut self, dep: &Dependency) -> CargoResult<Vec<Summary>> {
455+
// If this is a precise dependency, then it came from a lockfile and in
456+
// theory the registry is known to contain this version. If, however, we
457+
// come back with no summaries, then our registry may need to be
458+
// updated, so we fall back to performing a lazy update.
459+
if dep.get_source_id().get_precise().is_some() &&
460+
try!(self.summaries(dep.get_name())).len() == 0 {
461+
try!(self.do_update());
462+
}
463+
464+
let summaries = try!(self.summaries(dep.get_name()));
465+
let mut summaries = summaries.iter().filter(|&&(_, yanked)| {
466+
dep.get_source_id().get_precise().is_some() || !yanked
467+
}).map(|&(ref s, _)| s.clone()).collect::<Vec<_>>();
468+
summaries.query(dep)
469+
}
470+
}
471+
472+
impl<'a, 'b> Source for RegistrySource<'a, 'b> {
473+
fn update(&mut self) -> CargoResult<()> {
474+
// If we have an imprecise version then we don't know what we're going
475+
// to look for, so we always atempt to perform an update here.
476+
//
477+
// If we have a precise version, then we'll update lazily during the
478+
// querying phase.
479+
if self.source_id.get_precise().is_none() {
480+
try!(self.do_update());
481+
}
454482
Ok(())
455483
}
456484

‎tests/test_cargo_registry.rs

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::io::{fs, File};
22

33
use support::{project, execs, cargo_dir};
44
use support::{UPDATING, DOWNLOADING, COMPILING, PACKAGING, VERIFYING};
5-
use support::paths::PathExt;
5+
use support::paths::{mod, PathExt};
66
use support::registry as r;
77

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

251251
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
252-
execs().with_status(0).with_stdout(format!("\
253-
{updating} registry `[..]`
254-
", updating = UPDATING).as_slice()));
252+
execs().with_status(0).with_stdout(""));
255253
})
256254

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

289287
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
290-
execs().with_status(0).with_stdout(format!("\
291-
{updating} registry `[..]`
292-
", updating = UPDATING).as_slice()));
288+
execs().with_status(0).with_stdout(""));
293289
})
294290

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

375371
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
376-
execs().with_status(0).with_stdout(format!("\
377-
{updating} registry `[..]`
378-
", updating = UPDATING).as_slice()));
372+
execs().with_status(0).with_stdout(""));
379373

380374
assert_that(p.process(cargo_dir().join("cargo")).arg("update"),
381375
execs().with_status(101).with_stderr("\
@@ -384,3 +378,65 @@ location searched: the package registry
384378
version required: *
385379
"));
386380
})
381+
382+
test!(update_with_lockfile_if_packages_missing {
383+
let p = project("foo")
384+
.file("Cargo.toml", r#"
385+
[project]
386+
name = "foo"
387+
version = "0.0.1"
388+
authors = []
389+
390+
[dependencies]
391+
bar = "*"
392+
"#)
393+
.file("src/main.rs", "fn main() {}");
394+
p.build();
395+
396+
r::mock_pkg("bar", "0.0.1", []);
397+
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
398+
execs().with_status(0));
399+
p.root().move_into_the_past().unwrap();
400+
401+
fs::rmdir_recursive(&paths::home().join(".cargo/registry")).unwrap();
402+
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
403+
execs().with_status(0).with_stdout(format!("\
404+
{updating} registry `[..]`
405+
{downloading} bar v0.0.1 (the package registry)
406+
", updating = UPDATING, downloading = DOWNLOADING).as_slice()));
407+
})
408+
409+
test!(update_lockfile {
410+
let p = project("foo")
411+
.file("Cargo.toml", r#"
412+
[project]
413+
name = "foo"
414+
version = "0.0.1"
415+
authors = []
416+
417+
[dependencies]
418+
bar = "*"
419+
"#)
420+
.file("src/main.rs", "fn main() {}");
421+
p.build();
422+
423+
r::mock_pkg("bar", "0.0.1", []);
424+
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
425+
execs().with_status(0));
426+
427+
r::mock_pkg("bar", "0.0.2", []);
428+
fs::rmdir_recursive(&paths::home().join(".cargo/registry")).unwrap();
429+
assert_that(p.process(cargo_dir().join("cargo")).arg("update")
430+
.arg("-p").arg("bar"),
431+
execs().with_status(0).with_stdout(format!("\
432+
{updating} registry `[..]`
433+
", updating = UPDATING).as_slice()));
434+
435+
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
436+
execs().with_status(0).with_stdout(format!("\
437+
{downloading} [..] v0.0.2 (the package registry)
438+
{compiling} bar v0.0.2 (the package registry)
439+
{compiling} foo v0.0.1 ({dir})
440+
", downloading = DOWNLOADING, compiling = COMPILING,
441+
dir = p.url()).as_slice()));
442+
})

0 commit comments

Comments
 (0)