From 01dbc0588d57bf5d9e087e177aeb3031d9f35077 Mon Sep 17 00:00:00 2001
From: Tor Hovland <55164+torhovland@users.noreply.github.com>
Date: Thu, 30 May 2024 14:35:33 +0200
Subject: [PATCH] In case of --breaking, don't touch the non-breaking deps.

---
 src/bin/cargo/commands/update.rs |  2 +-
 src/cargo/ops/cargo_update.rs    | 38 +++++++++++++++++++++++---------
 tests/testsuite/update.rs        | 13 ++++++-----
 3 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/src/bin/cargo/commands/update.rs b/src/bin/cargo/commands/update.rs
index 672dc1bb5d36..40e847766d71 100644
--- a/src/bin/cargo/commands/update.rs
+++ b/src/bin/cargo/commands/update.rs
@@ -100,7 +100,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
     };
 
     let upgrades = ops::update_manifests(&mut ws, &update_opts)?;
-    ops::update_lockfile(&ws, &update_opts)?;
+    ops::update_lockfile(&ws, &update_opts, &upgrades)?;
     ops::write_manifests(&ws, &update_opts, &upgrades)?;
 
     Ok(())
diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs
index 571bc63ee92c..3c86249fc4e0 100644
--- a/src/cargo/ops/cargo_update.rs
+++ b/src/cargo/ops/cargo_update.rs
@@ -45,7 +45,11 @@ pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> {
     Ok(())
 }
 
-pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoResult<()> {
+pub fn update_lockfile(
+    ws: &Workspace<'_>,
+    opts: &UpdateOptions<'_>,
+    upgrades: &HashMap<String, Version>,
+) -> CargoResult<()> {
     if opts.recursive && opts.precise.is_some() {
         anyhow::bail!("cannot specify both recursive and precise simultaneously")
     }
@@ -161,7 +165,12 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
         .filter(|s| !s.is_registry())
         .collect();
 
-    let keep = |p: &PackageId| !to_avoid_sources.contains(&p.source_id()) && !to_avoid.contains(p);
+    let keep = |p: &PackageId| {
+        (!to_avoid_sources.contains(&p.source_id()) && !to_avoid.contains(p))
+        // In case of `--breaking`, we want to keep all packages unchanged that
+        // didn't get upgraded.
+            || (opts.breaking && !upgrades.contains_key(&p.name().to_string()))
+    };
 
     let mut resolve = ops::resolve_with_previous(
         &mut registry,
@@ -238,7 +247,7 @@ pub fn update_manifests(
             .summary()
             .clone()
             .map_dependencies(|dependency| {
-                let req = if let OptVersionReq::Req(current) = dependency.version_req() {
+                if let OptVersionReq::Req(current) = dependency.version_req() {
                     let query = crate::core::dependency::Dependency::parse(
                         dependency.package_name(),
                         None,
@@ -269,6 +278,13 @@ pub fn update_manifests(
 
                     if let Some(latest) = latest.clone() {
                         if !current.matches(&latest) {
+                            debug!(
+                                "upgrading {} from {} to {}",
+                                dependency.package_name(),
+                                current,
+                                latest
+                            );
+
                             opts.gctx
                                 .shell()
                                 .status_with_color(
@@ -284,17 +300,17 @@ pub fn update_manifests(
                                 .unwrap();
 
                             upgrades.insert(dependency.package_name().to_string(), latest.clone());
+
+                            let req =
+                                OptVersionReq::Req(VersionReq::parse(&latest.to_string()).unwrap());
+                            let mut dep = dependency.clone();
+                            dep.set_version_req(req);
+                            return dep;
                         }
                     }
+                }
 
-                    OptVersionReq::Req(VersionReq::parse(&latest.unwrap().to_string()).unwrap())
-                } else {
-                    dependency.version_req().clone()
-                };
-
-                let mut dep = dependency.clone();
-                dep.set_version_req(req);
-                dep
+                dependency.clone()
             });
 
         let summary = member.manifest_mut().summary_mut();
diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs
index 0376d2ee1228..be2b2a091a1d 100644
--- a/tests/testsuite/update.rs
+++ b/tests/testsuite/update.rs
@@ -1661,7 +1661,7 @@ fn update_with_missing_feature() {
 }
 
 #[cargo_test]
-fn update_compatible() {
+fn update_nonbreaking() {
     Package::new("compatible", "1.0.0").publish();
     Package::new("incompatible", "1.0.0").publish();
 
@@ -1701,7 +1701,7 @@ fn update_compatible() {
 }
 
 #[cargo_test]
-fn update_incompatible() {
+fn update_breaking() {
     Package::new("compatible", "1.0.0").publish();
     Package::new("incompatible", "1.0.0").publish();
 
@@ -1729,14 +1729,17 @@ incompatible  =  "1.0"  # This line gets partially rewritten
     Package::new("incompatible", "2.0.0").publish();
 
     p.cargo("update --breaking")
-        // .env("CARGO_LOG", "cargo::ops=trace,cargo::util::toml=debug")
+        // .env(
+        //     "CARGO_LOG",
+        //     "cargo::ops=trace,cargo::util::toml=debug,cargo::core::resolver=debug",
+        // )
         .with_stderr(
             "\
 [UPDATING] `[..]` index
 [UPGRADING] incompatible ^1.0 -> v2.0.0
-[LOCKING] 2 packages to latest compatible versions
-[UPDATING] compatible v1.0.0 -> v1.0.1
+[LOCKING] 1 package to latest compatible version
 [UPDATING] incompatible v1.0.0 -> v2.0.0
+[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
 ",
         )
         .run();