From 0deaae9e52d63e3d70e9073962c6d0cccf9e77a3 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Tue, 20 Mar 2018 11:38:25 -0700
Subject: [PATCH] Don't require `cargo update` when bumping versions

One historical annoyance I've always had with Cargo that I've found surprising
is that in some situations when you bump version numbers you'll have to end up
running `cargo update` later on to get everything to build. You get pretty wonky
error messages in this case as well saying a package doesn't exist when it
clearly does at a particular location!

I've had difficulty historically nailing down a test case for this but it looks
like we ironically already had one in our test suite and I also jury-rigged up
one from a case I ran into in the wild today.
---
 src/cargo/ops/resolve.rs  | 34 +++++++++++++++++++++++-------
 tests/testsuite/build.rs  | 13 +-----------
 tests/testsuite/update.rs | 44 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 19 deletions(-)

diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
index e52e71de298..470e36f48ba 100644
--- a/src/cargo/ops/resolve.rs
+++ b/src/cargo/ops/resolve.rs
@@ -354,6 +354,18 @@ fn register_previous_locks<'a>(
     resolve: &'a Resolve,
     keep: &Fn(&&'a PackageId) -> bool,
 ) {
+    let path_pkg = |id: &SourceId| {
+        if !id.is_path() {
+            return None;
+        }
+        if let Ok(path) = id.url().to_file_path() {
+            if let Ok(pkg) = ws.load(&path.join("Cargo.toml")) {
+                return Some(pkg);
+            }
+        }
+        None
+    };
+
     // Ok so we've been passed in a `keep` function which basically says "if I
     // return true then this package wasn't listed for an update on the command
     // line". AKA if we run `cargo update -p foo` then `keep(bar)` will return
@@ -376,10 +388,22 @@ fn register_previous_locks<'a>(
     // however, nothing else in the dependency graph depends on `log` and the
     // newer version of `serde` requires a new version of `log` it'll get pulled
     // in (as we didn't accidentally lock it to an old version).
+    //
+    // Additionally here we process all path dependencies listed in the previous
+    // resolve. They can not only have their dependencies change but also
+    // the versions of the package change as well. If this ends up happening
+    // then we want to make sure we don't lock a package id node that doesn't
+    // actually exist. Note that we don't do transitive visits of all the
+    // package's dependencies here as that'll be covered below to poison those
+    // if they changed.
     let mut avoid_locking = HashSet::new();
     for node in resolve.iter() {
         if !keep(&node) {
             add_deps(resolve, node, &mut avoid_locking);
+        } else if let Some(pkg) = path_pkg(node.source_id()) {
+            if pkg.package_id() != node {
+                avoid_locking.insert(node);
+            }
         }
     }
 
@@ -425,13 +449,9 @@ fn register_previous_locks<'a>(
 
             // If this is a path dependency then try to push it onto our
             // worklist
-            if source.is_path() {
-                if let Ok(path) = source.url().to_file_path() {
-                    if let Ok(pkg) = ws.load(&path.join("Cargo.toml")) {
-                        path_deps.push(pkg);
-                    }
-                    continue;
-                }
+            if let Some(pkg) = path_pkg(source) {
+                path_deps.push(pkg);
+                continue;
             }
 
             // If we match *anything* in the dependency graph then we consider
diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs
index c87d588c1d3..c009098905b 100644
--- a/tests/testsuite/build.rs
+++ b/tests/testsuite/build.rs
@@ -1453,18 +1453,7 @@ fn compile_path_dep_then_change_version() {
         )
         .unwrap();
 
-    assert_that(
-        p.cargo("build"),
-        execs().with_status(101).with_stderr(
-            "\
-error: no matching version `= 0.0.1` found for package `bar`
-location searched: [..]
-versions found: 0.0.2
-required by package `foo v0.0.1 ([..]/foo)`
-consider running `cargo update` to update a path dependency's locked version
-",
-        ),
-    );
+    assert_that(p.cargo("build"), execs().with_status(0));
 }
 
 #[test]
diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs
index c75a4c5fdad..fd106ca0166 100644
--- a/tests/testsuite/update.rs
+++ b/tests/testsuite/update.rs
@@ -314,3 +314,47 @@ fn everything_real_deep() {
     p.uncomment_root_manifest();
     assert_that(p.cargo("build"), execs().with_status(0));
 }
+
+#[test]
+fn change_package_version() {
+    let p = project("foo")
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "a-foo"
+                version = "0.2.0-alpha"
+                authors = []
+
+                [dependencies]
+                bar = { path = "bar", version = "0.2.0-alpha" }
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .file(
+            "bar/Cargo.toml",
+            r#"
+                [package]
+                name = "bar"
+                version = "0.2.0-alpha"
+                authors = []
+            "#,
+        )
+        .file("bar/src/lib.rs", "")
+        .file(
+            "Cargo.lock",
+            r#"
+                [[package]]
+                name = "foo"
+                version = "0.2.0"
+                dependencies = ["bar 0.2.0"]
+
+                [[package]]
+                name = "bar"
+                version = "0.2.0"
+            "#,
+        )
+        .build();
+
+    assert_that(p.cargo("build"), execs().with_status(0));
+}