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.

diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
index e52e71de298bcf58d63da5a7cd0dbdb3e5b31217..470e36f48ba1d0ca957b575c0fc91c94fda49ac0 100644 (file)
--- 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 c87d588c1d38320d54056f0e29d6cfb45b078760..c009098905be037873abf68d9c1d73e7098e4ba1 100644 (file)
--- 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 c75a4c5fdadd9ec22e9b6cd1d85226d13e0efba6..fd106ca01663927b996dbc47cfea1ddcbe39d383 100644 (file)
--- 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));
+}