From 47e3d167689260b34a22197e57927d9e69d17f97 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 11 Jul 2018 18:08:02 -0400 Subject: [PATCH 1/3] add a test for #5529 --- tests/testsuite/path.rs | 102 +++++++++++++++++++++++++++++++++------- 1 file changed, 86 insertions(+), 16 deletions(-) diff --git a/tests/testsuite/path.rs b/tests/testsuite/path.rs index e7ea0f024b0..43bba365a45 100644 --- a/tests/testsuite/path.rs +++ b/tests/testsuite/path.rs @@ -765,18 +765,19 @@ fn override_self() { let p = project("foo"); let root = p.root().clone(); - let p = p.file( - ".cargo/config", - &format!( - r#" - paths = ['{}'] - "#, - root.display() - ), - ).file( - "Cargo.toml", + let p = + p.file( + ".cargo/config", &format!( r#" + paths = ['{}'] + "#, + root.display() + ), + ).file( + "Cargo.toml", + &format!( + r#" [package] name = "foo" @@ -787,12 +788,12 @@ fn override_self() { path = '{}' "#, - bar.root().display() - ), - ) - .file("src/lib.rs", "") - .file("src/main.rs", "fn main() {}") - .build(); + bar.root().display() + ), + ) + .file("src/lib.rs", "") + .file("src/main.rs", "fn main() {}") + .build(); assert_that(p.cargo("build"), execs().with_status(0)); } @@ -1177,6 +1178,75 @@ Caused by: ); } +#[test] +fn cargo_update_downgrade_dependencies() { + // https://github.com/rust-lang/cargo/issues/5529 + + Package::new("x", "1.0.3").publish(); + Package::new("x", "1.0.2").publish(); + Package::new("x", "1.0.1").publish(); + Package::new("x", "1.0.0").publish(); + Package::new("x", "0.9.1").publish(); + Package::new("x", "0.9.0").publish(); + + let p = project("foo") + .file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b", "c"] + "#, + ) + .file("a/src/lib.rs", "") + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "0.1.0" + + [dependencies] + x="0.9" + "#, + ) + .file("b/src/lib.rs", "") + .file( + "b/Cargo.toml", + r#" + [package] + name = "b" + version = "0.1.0" + + [dependencies] + x=">= 0.9, < 2.0" + "#, + ) + .file("c/src/lib.rs", "") + .file( + "c/Cargo.toml", + r#" + [package] + name = "c" + version = "0.1.0" + + [dependencies] + x="1.0" + "#, + ) + .build(); + + // Generate a lock file + assert_that(p.cargo("generate-lockfile"), execs().with_status(0)); + + let good_lock = fs::read_to_string(&p.root().join("Cargo.lock")).unwrap(); + + assert_that(p.cargo("update -p c"), execs().with_status(0)); + + let new_lock = fs::read_to_string(&p.root().join("Cargo.lock")).unwrap(); + + assert_eq!(good_lock, new_lock); +} + #[test] fn invalid_path_dep_in_workspace_with_lockfile() { Package::new("bar", "1.0.0").publish(); From b236b42501f9e5157724f557ee2e0fc453ffd676 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 11 Jul 2018 23:17:20 -0400 Subject: [PATCH 2/3] remove code that is now redundunt --- src/cargo/core/registry.rs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 451efcb57e6..02b1f897b6a 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -556,13 +556,6 @@ fn lock(locked: &LockedMap, patches: &HashMap>, summary: Sum // requirement has changed. In this case we must discard the // locked version because the dependency needs to be // re-resolved. - // - // 3. We don't have a lock entry for this dependency, in which - // case it was likely an optional dependency which wasn't - // included previously so we just pass it through anyway. - // - // Cases 1/2 are handled by `matches_id` and case 3 is handled by - // falling through to the logic below. if let Some(&(_, ref locked_deps)) = pair { let locked = locked_deps.iter().find(|id| dep.matches_id(id)); if let Some(locked) = locked { @@ -573,20 +566,6 @@ fn lock(locked: &LockedMap, patches: &HashMap>, summary: Sum } } - // If this dependency did not have a locked version, then we query - // all known locked packages to see if they match this dependency. - // If anything does then we lock it to that and move on. - let v = locked - .get(dep.source_id()) - .and_then(|map| map.get(&*dep.name())) - .and_then(|vec| vec.iter().find(|&&(ref id, _)| dep.matches_id(id))); - if let Some(&(ref id, _)) = v { - trace!("\tsecond hit on {}", id); - let mut dep = dep.clone(); - dep.lock_to(id); - return dep; - } - // Finally we check to see if any registered patches correspond to // this dependency. let v = patches.get(dep.source_id().url()).map(|vec| { From 38c6023a4d93edb42ae61d72a96f0b318c4fa5e8 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Sat, 14 Jul 2018 18:21:04 -0400 Subject: [PATCH 3/3] limit the heuristic to git dependencies --- src/cargo/core/registry.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 02b1f897b6a..6747720a5d5 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -556,6 +556,13 @@ fn lock(locked: &LockedMap, patches: &HashMap>, summary: Sum // requirement has changed. In this case we must discard the // locked version because the dependency needs to be // re-resolved. + // + // 3. We don't have a lock entry for this dependency, in which + // case it was likely an optional dependency which wasn't + // included previously so we just pass it through anyway. + // + // Cases 1/2 are handled by `matches_id` and case 3 is handled by + // falling through to the logic below. if let Some(&(_, ref locked_deps)) = pair { let locked = locked_deps.iter().find(|id| dep.matches_id(id)); if let Some(locked) = locked { @@ -566,6 +573,24 @@ fn lock(locked: &LockedMap, patches: &HashMap>, summary: Sum } } + // Querying a git dependency has the side effect of pulling changes. + // So even the conservative re-resolution may be to aggressive. + // If this git dependency did not have a locked version, then we query + // all known locked packages to see if they match this dependency. + // If anything does then we lock it to that and move on. + if dep.source_id().git_reference().is_some() { + let v = locked + .get(dep.source_id()) + .and_then(|map| map.get(&*dep.name())) + .and_then(|vec| vec.iter().find(|&&(ref id, _)| dep.matches_id(id))); + if let Some(&(ref id, _)) = v { + trace!("\tsecond hit on {}", id); + let mut dep = dep.clone(); + dep.lock_to(id); + return dep; + } + } + // Finally we check to see if any registered patches correspond to // this dependency. let v = patches.get(dep.source_id().url()).map(|vec| {