Skip to content

Commit 00a5cb1

Browse files
borsbors
authored
Auto merge of #3220 - alexcrichton:bad, r=brson
Load [replace] sections from lock files 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
2 parents cb524b7 + 155dee5 commit 00a5cb1

File tree

5 files changed

+126
-2
lines changed

5 files changed

+126
-2
lines changed

‎src/cargo/core/registry.rs

+18-2
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,10 @@ impl<'cfg> PackageRegistry<'cfg> {
165165
}
166166

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

231+
trace!("locking summary of {}", summary.package_id());
232+
227233
// Lock the summary's id if possible
228234
let summary = match pair {
229235
Some(&(ref precise, _)) => summary.override_id(precise.clone()),
230236
None => summary,
231237
};
232238
summary.map_dependencies(|dep| {
239+
trace!("\t{}/{}/{}", dep.name(), dep.version_req(),
240+
dep.source_id());
241+
233242
// If we've got a known set of overrides for this summary, then
234243
// one of a few cases can arise:
235244
//
@@ -252,6 +261,7 @@ impl<'cfg> PackageRegistry<'cfg> {
252261
if let Some(&(_, ref locked_deps)) = pair {
253262
let locked = locked_deps.iter().find(|id| dep.matches_id(id));
254263
if let Some(locked) = locked {
264+
trace!("\tfirst hit on {}", locked);
255265
return dep.lock_to(locked)
256266
}
257267
}
@@ -265,8 +275,14 @@ impl<'cfg> PackageRegistry<'cfg> {
265275
vec.iter().find(|&&(ref id, _)| dep.matches_id(id))
266276
});
267277
match v {
268-
Some(&(ref id, _)) => dep.lock_to(id),
269-
None => dep
278+
Some(&(ref id, _)) => {
279+
trace!("\tsecond hit on {}", id);
280+
dep.lock_to(id)
281+
}
282+
None => {
283+
trace!("\tremaining unlocked");
284+
dep
285+
}
270286
}
271287
})
272288
}

‎src/cargo/core/resolver/encode.rs

+2
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,10 @@ fn build_path_deps(ws: &Workspace) -> HashMap<String, SourceId> {
185185
fn build(pkg: &Package,
186186
config: &Config,
187187
ret: &mut HashMap<String, SourceId>) {
188+
let replace = pkg.manifest().replace();
188189
let deps = pkg.dependencies()
189190
.iter()
191+
.chain(replace.iter().map(|p| &p.1))
190192
.filter(|d| !ret.contains_key(d.name()))
191193
.map(|d| d.source_id())
192194
.filter(|id| id.is_path())

‎src/cargo/core/resolver/mod.rs

+5
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,7 @@ impl<'a> Context<'a> {
864864
None => return Ok(Candidate { summary: summary, replace: None }),
865865
Some(replacement) => replacement,
866866
};
867+
debug!("found an override for {} {}", dep.name(), dep.version_req());
867868

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

907+
for dep in summary.dependencies() {
908+
debug!("\t{} => {}", dep.name(), dep.version_req());
909+
}
910+
906911
Ok(Candidate { summary: summary, replace: replace })
907912
}).collect()
908913
}

‎src/cargo/ops/resolve.rs

+1
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
7777
// to the previously resolved version if the dependency listed
7878
// still matches the locked version.
7979
if let Some(r) = previous {
80+
trace!("previous: {:?}", r);
8081
for node in r.iter().filter(|p| keep(p, to_avoid, &to_avoid_sources)) {
8182
let deps = r.deps_not_replaced(node)
8283
.filter(|p| keep(p, to_avoid, &to_avoid_sources))

‎tests/overrides.rs

+100
Original file line numberDiff line numberDiff line change
@@ -775,3 +775,103 @@ http://doc.crates.io/specifying-dependencies.html#overriding-dependencies
775775
[FINISHED] [..]
776776
"));
777777
}
778+
779+
#[test]
780+
fn override_an_override() {
781+
Package::new("chrono", "0.2.0").dep("serde", "< 0.9").publish();
782+
Package::new("serde", "0.7.0")
783+
.file("src/lib.rs", "pub fn serde07() {}")
784+
.publish();
785+
Package::new("serde", "0.8.0")
786+
.file("src/lib.rs", "pub fn serde08() {}")
787+
.publish();
788+
789+
let p = project("local")
790+
.file("Cargo.toml", r#"
791+
[package]
792+
name = "local"
793+
version = "0.0.1"
794+
authors = []
795+
796+
[dependencies]
797+
chrono = "0.2"
798+
serde = "0.8"
799+
800+
[replace]
801+
"chrono:0.2.0" = { path = "chrono" }
802+
"serde:0.8.0" = { path = "serde" }
803+
"#)
804+
.file("Cargo.lock", r#"
805+
[root]
806+
name = "local"
807+
version = "0.0.1"
808+
dependencies = [
809+
"chrono 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
810+
"serde 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
811+
]
812+
813+
[[package]]
814+
name = "chrono"
815+
version = "0.2.0"
816+
source = "registry+https://github.com/rust-lang/crates.io-index"
817+
replace = "chrono 0.2.0"
818+
819+
[[package]]
820+
name = "chrono"
821+
version = "0.2.0"
822+
dependencies = [
823+
"serde 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
824+
]
825+
826+
[[package]]
827+
name = "serde"
828+
version = "0.7.0"
829+
source = "registry+https://github.com/rust-lang/crates.io-index"
830+
831+
[[package]]
832+
name = "serde"
833+
version = "0.8.0"
834+
source = "registry+https://github.com/rust-lang/crates.io-index"
835+
replace = "serde 0.8.0"
836+
837+
[[package]]
838+
name = "serde"
839+
version = "0.8.0"
840+
"#)
841+
.file("src/lib.rs", "
842+
extern crate chrono;
843+
extern crate serde;
844+
845+
pub fn local() {
846+
chrono::chrono();
847+
serde::serde08_override();
848+
}
849+
")
850+
.file("chrono/Cargo.toml", r#"
851+
[package]
852+
name = "chrono"
853+
version = "0.2.0"
854+
authors = []
855+
856+
[dependencies]
857+
serde = "< 0.9"
858+
"#)
859+
.file("chrono/src/lib.rs", "
860+
extern crate serde;
861+
pub fn chrono() {
862+
serde::serde07();
863+
}
864+
")
865+
.file("serde/Cargo.toml", r#"
866+
[package]
867+
name = "serde"
868+
version = "0.8.0"
869+
authors = []
870+
"#)
871+
.file("serde/src/lib.rs", "
872+
pub fn serde08_override() {}
873+
");
874+
875+
assert_that(p.cargo_process("build").arg("-v"),
876+
execs().with_status(0));
877+
}

0 commit comments

Comments
 (0)