Merge pull request #852 from epage/dry-run

fix(upgrade): Include --locked failure in dry-run

src/bin/upgrade/upgrade.rs

@@ -437,134 +437,133 @@ fn exec(args: UpgradeArgs) -> CargoResult<()> {
         }
     }
 
-    if !modified_crates.is_empty() && !args.dry_run {
-        if args.locked {
-            anyhow::bail!("cannot upgrade due to `--locked`");
-        } else {
-            // Ensure lock file is updated and collect data for `recursive`
-            let metadata = resolve_ws(Some(&root_manifest_path), args.locked, args.offline)?;
-            let mut locked = metadata.packages;
-
-            let precise_deps = selected_dependencies
-                .iter()
-                .filter_map(|(name, req)| {
-                    req.as_ref()
-                        .and_then(|req| semver::VersionReq::parse(req).ok())
-                        .and_then(|req| {
-                            let precise = precise_version(&req)?;
-                            Some((name, (req, precise)))
-                        })
-                })
-                .collect::<BTreeMap<_, _>>();
-            if !precise_deps.is_empty() {
-                // Rollback the updates to the precise version
-                //
-                // Reusing updates (resolve_ws) so we know what lock_version to reference
-                for (name, (req, precise)) in &precise_deps {
-                    #[allow(clippy::unnecessary_lazy_evaluations)] // requires 1.62
-                    for lock_version in locked
-                        .iter()
-                        .filter(|p| p.name == **name)
-                        .map(|p| &p.version)
-                        .filter_map(|v| req.matches(v).then(|| v))
-                    {
-                        let mut cmd = std::process::Command::new("cargo");
-                        cmd.arg("update");
-                        cmd.arg("--manifest-path").arg(&root_manifest_path);
-                        if args.locked {
-                            cmd.arg("--locked");
-                        }
-                        // NOTE: This will skip the official recursive check and we don't
-                        // recursively update its dependencies
-                        let dep = format!("{name}@{lock_version}");
-                        cmd.arg("--precise").arg(precise);
-                        cmd.arg("--package").arg(dep);
-                        // If we're going to request an update, it would have already been done by now
-                        cmd.arg("--offline");
-                        let output = cmd.output().context("failed to lock to precise version")?;
-                        if !output.status.success() {
-                            return Err(anyhow::format_err!(
-                                "{}",
-                                String::from_utf8_lossy(&output.stderr)
-                            ))
-                            .context("failed to lock to precise version");
-                        }
+    if modified_crates.is_empty() {
+    } else if args.locked {
+        anyhow::bail!("cannot upgrade due to `--locked`");
+    } else if args.dry_run {
+    } else {
+        // Ensure lock file is updated and collect data for `recursive`
+        let metadata = resolve_ws(Some(&root_manifest_path), args.locked, args.offline)?;
+        let mut locked = metadata.packages;
+
+        let precise_deps = selected_dependencies
+            .iter()
+            .filter_map(|(name, req)| {
+                req.as_ref()
+                    .and_then(|req| semver::VersionReq::parse(req).ok())
+                    .and_then(|req| {
+                        let precise = precise_version(&req)?;
+                        Some((name, (req, precise)))
+                    })
+            })
+            .collect::<BTreeMap<_, _>>();
+        if !precise_deps.is_empty() {
+            // Rollback the updates to the precise version
+            //
+            // Reusing updates (resolve_ws) so we know what lock_version to reference
+            for (name, (req, precise)) in &precise_deps {
+                #[allow(clippy::unnecessary_lazy_evaluations)] // requires 1.62
+                for lock_version in locked
+                    .iter()
+                    .filter(|p| p.name == **name)
+                    .map(|p| &p.version)
+                    .filter_map(|v| req.matches(v).then(|| v))
+                {
+                    let mut cmd = std::process::Command::new("cargo");
+                    cmd.arg("update");
+                    cmd.arg("--manifest-path").arg(&root_manifest_path);
+                    if args.locked {
+                        cmd.arg("--locked");
+                    }
+                    // NOTE: This will skip the official recursive check and we don't
+                    // recursively update its dependencies
+                    let dep = format!("{name}@{lock_version}");
+                    cmd.arg("--precise").arg(precise);
+                    cmd.arg("--package").arg(dep);
+                    // If we're going to request an update, it would have already been done by now
+                    cmd.arg("--offline");
+                    let output = cmd.output().context("failed to lock to precise version")?;
+                    if !output.status.success() {
+                        return Err(anyhow::format_err!(
+                            "{}",
+                            String::from_utf8_lossy(&output.stderr)
+                        ))
+                        .context("failed to lock to precise version");
                     }
                 }
-
-                // Update data for `recursive` with precise_deps
-                let offline = true; // index should already be updated
-                let metadata = resolve_ws(Some(&root_manifest_path), args.locked, offline)?;
-                locked = metadata.packages;
             }
 
-            if !git_crates.is_empty() && args.compatible.as_bool() {
-                shell_status("Upgrading", "git dependencies")?;
-                let mut cmd = std::process::Command::new("cargo");
-                cmd.arg("update");
-                cmd.arg("--manifest-path").arg(&root_manifest_path);
-                if args.locked {
-                    cmd.arg("--locked");
-                }
-                for dep in git_crates.iter() {
-                    for lock_version in locked
-                        .iter()
-                        .filter(|p| {
-                            p.name == *dep
-                                && p.source
-                                    .as_ref()
-                                    .map(|s| s.repr.starts_with("git+"))
-                                    .unwrap_or(false)
-                        })
-                        .map(|p| &p.version)
-                    {
-                        let dep = format!("{dep}@{lock_version}");
-                        cmd.arg("--package").arg(dep);
-                    }
-                }
-                // If we're going to request an update, it would have already been done by now
-                cmd.arg("--offline");
-                let status = cmd.status().context("recursive dependency update failed")?;
-                if !status.success() {
-                    anyhow::bail!("recursive dependency update failed");
-                }
+            // Update data for `recursive` with precise_deps
+            let offline = true; // index should already be updated
+            let metadata = resolve_ws(Some(&root_manifest_path), args.locked, offline)?;
+            locked = metadata.packages;
+        }
 
-                // Update data for `recursive` with precise_deps
-                let offline = true; // index should already be updated
-                let metadata = resolve_ws(Some(&root_manifest_path), args.locked, offline)?;
-                locked = metadata.packages;
+        if !git_crates.is_empty() && args.compatible.as_bool() {
+            shell_status("Upgrading", "git dependencies")?;
+            let mut cmd = std::process::Command::new("cargo");
+            cmd.arg("update");
+            cmd.arg("--manifest-path").arg(&root_manifest_path);
+            if args.locked {
+                cmd.arg("--locked");
             }
-
-            if args.recursive {
-                shell_status("Upgrading", "recursive dependencies")?;
-                let mut cmd = std::process::Command::new("cargo");
-                cmd.arg("update");
-                cmd.arg("--manifest-path").arg(&root_manifest_path);
-                if args.locked {
-                    cmd.arg("--locked");
-                }
-                // Limit recursive update to what we touched
-                cmd.arg("--aggressive");
-                let mut still_run = false;
-                for dep in modified_crates
+            for dep in git_crates.iter() {
+                for lock_version in locked
                     .iter()
-                    // Already updated so avoid discarding the precise version selection
-                    .filter(|c| !precise_deps.contains_key(c))
+                    .filter(|p| {
+                        p.name == *dep
+                            && p.source
+                                .as_ref()
+                                .map(|s| s.repr.starts_with("git+"))
+                                .unwrap_or(false)
+                    })
+                    .map(|p| &p.version)
                 {
-                    for lock_version in locked.iter().filter(|p| p.name == *dep).map(|p| &p.version)
-                    {
-                        let dep = format!("{dep}@{lock_version}");
-                        cmd.arg("--package").arg(dep);
-                        still_run = true;
-                    }
+                    let dep = format!("{dep}@{lock_version}");
+                    cmd.arg("--package").arg(dep);
                 }
-                // If we're going to request an update, it would have already been done by now
-                cmd.arg("--offline");
-                if still_run {
-                    let status = cmd.status().context("recursive dependency update failed")?;
-                    if !status.success() {
-                        anyhow::bail!("recursive dependency update failed");
-                    }
+            }
+            // If we're going to request an update, it would have already been done by now
+            cmd.arg("--offline");
+            let status = cmd.status().context("recursive dependency update failed")?;
+            if !status.success() {
+                anyhow::bail!("recursive dependency update failed");
+            }
+
+            // Update data for `recursive` with precise_deps
+            let offline = true; // index should already be updated
+            let metadata = resolve_ws(Some(&root_manifest_path), args.locked, offline)?;
+            locked = metadata.packages;
+        }
+
+        if args.recursive {
+            shell_status("Upgrading", "recursive dependencies")?;
+            let mut cmd = std::process::Command::new("cargo");
+            cmd.arg("update");
+            cmd.arg("--manifest-path").arg(&root_manifest_path);
+            if args.locked {
+                cmd.arg("--locked");
+            }
+            // Limit recursive update to what we touched
+            cmd.arg("--aggressive");
+            let mut still_run = false;
+            for dep in modified_crates
+                .iter()
+                // Already updated so avoid discarding the precise version selection
+                .filter(|c| !precise_deps.contains_key(c))
+            {
+                for lock_version in locked.iter().filter(|p| p.name == *dep).map(|p| &p.version) {
+                    let dep = format!("{dep}@{lock_version}");
+                    cmd.arg("--package").arg(dep);
+                    still_run = true;
+                }
+            }
+            // If we're going to request an update, it would have already been done by now
+            cmd.arg("--offline");
+            if still_run {
+                let status = cmd.status().context("recursive dependency update failed")?;
+                if !status.success() {
+                    anyhow::bail!("recursive dependency update failed");
                 }
             }
         }

tests/cargo-upgrade/locked_dry_run/in/Cargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+
+[dependencies]
+my-package = "0.1.1"

tests/cargo-upgrade/locked_dry_run/mod.rs

@@ -0,0 +1,25 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::Project;
+
+use crate::init_registry;
+use crate::CargoCommand;
+use cargo_test_support::curr_dir;
+
+#[cargo_test]
+fn case() {
+    init_registry();
+    let project = Project::from_template(curr_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    snapbox::cmd::Command::cargo_ui()
+        .arg("upgrade")
+        .args(["--locked", "--incompatible", "--dry-run"])
+        .current_dir(cwd)
+        .assert()
+        .failure()
+        .stdout_matches_path(curr_dir!().join("stdout.log"))
+        .stderr_matches_path(curr_dir!().join("stderr.log"));
+
+    assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
+}

tests/cargo-upgrade/locked_dry_run/out/Cargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+
+[dependencies]
+my-package = "0.1.1"

tests/cargo-upgrade/locked_dry_run/stderr.log

@@ -0,0 +1,3 @@
+    Updating '[ROOTURL]/registry' index
+    Checking cargo-list-test-fixture's dependencies
+Error: cannot upgrade due to `--locked`

tests/cargo-upgrade/locked_dry_run/stdout.log

@@ -0,0 +1,3 @@
+name       old req compatible latest    new req  
+====       ======= ========== ======    =======  
+my-package 0.1.1   0.1.1      99999.0.0 99999.0.0

tests/cargo-upgrade/main.rs

@@ -15,6 +15,7 @@ mod invalid_flag;
 mod invalid_manifest;
 mod invalid_workspace_root_manifest;
 mod locked;
+mod locked_dry_run;
 mod lockfile;
 mod optional_dep;
 mod pinned;