Skip to content

Commit

Permalink
Load [replace] sections from lock files
Browse files Browse the repository at this point in the history
This commit fixes a bug in Cargo where path-based [replace] dependencies were
accidentally not loaded from lock files. This meant that even with a lock
file some compilations could accidentally become nondeterministic. The fix here
is to just look at all path dependencies, even those specified through [replace]

Closes #3216
  • Loading branch information
alexcrichton committed Oct 21, 2016
1 parent 9a0801d commit 155dee5
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 2 deletions.
20 changes: 18 additions & 2 deletions src/cargo/core/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ impl<'cfg> PackageRegistry<'cfg> {
}

pub fn register_lock(&mut self, id: PackageId, deps: Vec<PackageId>) {
trace!("register_lock: {}", id);
for dep in deps.iter() {
trace!("\t-> {}", dep);
}
let sub_map = self.locked.entry(id.source_id().clone())
.or_insert(HashMap::new());
let sub_vec = sub_map.entry(id.name().to_string())
Expand Down Expand Up @@ -224,12 +228,17 @@ impl<'cfg> PackageRegistry<'cfg> {
vec.iter().find(|&&(ref id, _)| id == summary.package_id())
});

trace!("locking summary of {}", summary.package_id());

// Lock the summary's id if possible
let summary = match pair {
Some(&(ref precise, _)) => summary.override_id(precise.clone()),
None => summary,
};
summary.map_dependencies(|dep| {
trace!("\t{}/{}/{}", dep.name(), dep.version_req(),
dep.source_id());

// If we've got a known set of overrides for this summary, then
// one of a few cases can arise:
//
Expand All @@ -252,6 +261,7 @@ impl<'cfg> PackageRegistry<'cfg> {
if let Some(&(_, ref locked_deps)) = pair {
let locked = locked_deps.iter().find(|id| dep.matches_id(id));
if let Some(locked) = locked {
trace!("\tfirst hit on {}", locked);
return dep.lock_to(locked)
}
}
Expand All @@ -265,8 +275,14 @@ impl<'cfg> PackageRegistry<'cfg> {
vec.iter().find(|&&(ref id, _)| dep.matches_id(id))
});
match v {
Some(&(ref id, _)) => dep.lock_to(id),
None => dep
Some(&(ref id, _)) => {
trace!("\tsecond hit on {}", id);
dep.lock_to(id)
}
None => {
trace!("\tremaining unlocked");
dep
}
}
})
}
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/core/resolver/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,10 @@ fn build_path_deps(ws: &Workspace) -> HashMap<String, SourceId> {
fn build(pkg: &Package,
config: &Config,
ret: &mut HashMap<String, SourceId>) {
let replace = pkg.manifest().replace();
let deps = pkg.dependencies()
.iter()
.chain(replace.iter().map(|p| &p.1))
.filter(|d| !ret.contains_key(d.name()))
.map(|d| d.source_id())
.filter(|id| id.is_path())
Expand Down
5 changes: 5 additions & 0 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ impl<'a> Context<'a> {
None => return Ok(Candidate { summary: summary, replace: None }),
Some(replacement) => replacement,
};
debug!("found an override for {} {}", dep.name(), dep.version_req());

let mut summaries = try!(registry.query(dep)).into_iter();
let s = try!(summaries.next().chain_error(|| {
Expand Down Expand Up @@ -903,6 +904,10 @@ impl<'a> Context<'a> {
matched_spec, spec, summary.package_id());
}

for dep in summary.dependencies() {
debug!("\t{} => {}", dep.name(), dep.version_req());
}

Ok(Candidate { summary: summary, replace: replace })
}).collect()
}
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
// to the previously resolved version if the dependency listed
// still matches the locked version.
if let Some(r) = previous {
trace!("previous: {:?}", r);
for node in r.iter().filter(|p| keep(p, to_avoid, &to_avoid_sources)) {
let deps = r.deps_not_replaced(node)
.filter(|p| keep(p, to_avoid, &to_avoid_sources))
Expand Down
100 changes: 100 additions & 0 deletions tests/overrides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -775,3 +775,103 @@ http://doc.crates.io/specifying-dependencies.html#overriding-dependencies
[FINISHED] [..]
"));
}

#[test]
fn override_an_override() {
Package::new("chrono", "0.2.0").dep("serde", "< 0.9").publish();
Package::new("serde", "0.7.0")
.file("src/lib.rs", "pub fn serde07() {}")
.publish();
Package::new("serde", "0.8.0")
.file("src/lib.rs", "pub fn serde08() {}")
.publish();

let p = project("local")
.file("Cargo.toml", r#"
[package]
name = "local"
version = "0.0.1"
authors = []
[dependencies]
chrono = "0.2"
serde = "0.8"
[replace]
"chrono:0.2.0" = { path = "chrono" }
"serde:0.8.0" = { path = "serde" }
"#)
.file("Cargo.lock", r#"
[root]
name = "local"
version = "0.0.1"
dependencies = [
"chrono 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"serde 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
name = "chrono"
version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
replace = "chrono 0.2.0"
[[package]]
name = "chrono"
version = "0.2.0"
dependencies = [
"serde 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
name = "serde"
version = "0.7.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
name = "serde"
version = "0.8.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
replace = "serde 0.8.0"
[[package]]
name = "serde"
version = "0.8.0"
"#)
.file("src/lib.rs", "
extern crate chrono;
extern crate serde;
pub fn local() {
chrono::chrono();
serde::serde08_override();
}
")
.file("chrono/Cargo.toml", r#"
[package]
name = "chrono"
version = "0.2.0"
authors = []
[dependencies]
serde = "< 0.9"
"#)
.file("chrono/src/lib.rs", "
extern crate serde;
pub fn chrono() {
serde::serde07();
}
")
.file("serde/Cargo.toml", r#"
[package]
name = "serde"
version = "0.8.0"
authors = []
"#)
.file("serde/src/lib.rs", "
pub fn serde08_override() {}
");

assert_that(p.cargo_process("build").arg("-v"),
execs().with_status(0));
}

0 comments on commit 155dee5

Please sign in to comment.