fix: Use ops::update_lockfile for consistency with non-breaking update.

src/bin/cargo/commands/update.rs

@@ -1,8 +1,11 @@
+use std::collections::HashMap;
+
 use crate::command_prelude::*;
 
 use anyhow::anyhow;
 use cargo::ops::{self, UpdateOptions};
 use cargo::util::print_available_packages;
+use tracing::trace;
 
 pub fn cli() -> Command {
  subcommand("update")
@@ -92,28 +95,38 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
  let update_opts = UpdateOptions {
  recursive: args.flag("recursive"),
  precise: args.get_one::<String>("precise").map(String::as_str),
+ breaking: args.flag("breaking"),
  to_update,
  dry_run: args.dry_run(),
  workspace: args.flag("workspace"),
  gctx,
  };
 
- if args.flag("breaking") {
- gctx.cli_unstable()
- .fail_if_stable_opt("--breaking", 12425)?;
-
- let upgrades = ops::upgrade_manifests(&mut ws, &update_opts.to_update)?;
- ops::resolve_ws(&ws, update_opts.dry_run)?;
- ops::write_manifest_upgrades(&ws, &upgrades, update_opts.dry_run)?;
+ let breaking_update = update_opts.breaking; // or if doing a breaking precise update, coming in #14140.
 
- if update_opts.dry_run {
- update_opts
- .gctx
- .shell()
- .warn("aborting update due to dry run")?;
+ // We are using the term "upgrade" here, which is the typical case, but it
+ // can also be a downgrade (in the case of a precise update). In general, it
+ // is a change to a version req matching a precise version.
+ let upgrades = if breaking_update {
+ if update_opts.breaking {
+ gctx.cli_unstable()
+ .fail_if_stable_opt("--breaking", 12425)?;
  }
+
+ trace!("allowing breaking updates");
+ ops::upgrade_manifests(&mut ws, &update_opts.to_update)?
  } else {
- ops::update_lockfile(&ws, &update_opts)?;
+ HashMap::new()
+ };
+
+ ops::update_lockfile(&ws, &update_opts, &upgrades)?;
+ ops::write_manifest_upgrades(&ws, &upgrades, update_opts.dry_run)?;
+
+ if update_opts.dry_run {
+ update_opts
+ .gctx
+ .shell()
+ .warn("aborting update due to dry run")?;
  }
 
  Ok(())

src/cargo/ops/cargo_update.rs

@@ -9,6 +9,7 @@ use crate::ops;
 use crate::sources::source::QueryKind;
 use crate::util::cache_lock::CacheLockMode;
 use crate::util::context::GlobalContext;
+use crate::util::interning::InternedString;
 use crate::util::toml_mut::dependency::{MaybeWorkspace, Source};
 use crate::util::toml_mut::manifest::LocalManifest;
 use crate::util::toml_mut::upgrade::upgrade_requirement;
@@ -20,12 +21,25 @@ use std::cmp::Ordering;
 use std::collections::{BTreeMap, HashMap, HashSet};
 use tracing::{debug, trace};
 
-pub type UpgradeMap = HashMap<(String, SourceId), Version>;
+/// A map of all breaking upgrades which is filled in by
+/// upgrade_manifests/upgrade_dependency when going through workspace member
+/// manifests, and later used by write_manifest_upgrades in order to know which
+/// upgrades to write to manifest files on disk. Also used by update_lockfile to
+/// know which dependencies to keep unchanged if any have been upgraded (we will
+/// do either breaking or non-breaking updates, but not both).
+pub type UpgradeMap = HashMap<
+ // The key is a package identifier consisting of the name and the source id.
+ (InternedString, SourceId),
+ // The value is the original version requirement before upgrade, and the
+ // upgraded version.
+ (VersionReq, Version),
+>;
 
 pub struct UpdateOptions<'a> {
  pub gctx: &'a GlobalContext,
  pub to_update: Vec<String>,
  pub precise: Option<&'a str>,
+ pub breaking: bool,
  pub recursive: bool,
  pub dry_run: bool,
  pub workspace: bool,
@@ -49,7 +63,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: &UpgradeMap,
+) -> CargoResult<()> {
  if opts.recursive && opts.precise.is_some() {
  anyhow::bail!("cannot specify both recursive and precise simultaneously")
  }
@@ -91,7 +109,38 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
  let mut registry = ws.package_registry()?;
  let mut to_avoid = HashSet::new();
 
- if opts.to_update.is_empty() {
+ if opts.breaking {
+ // We don't necessarily want to update all specified packages. If we are
+ // doing a breaking update (or precise upgrades, coming in #14140), we
+ // don't want to touch any packages that have no breaking updates. So we
+ // want to only avoid all packages that got upgraded.
+
+ for name in opts.to_update.iter() {
+ // We still want to query any specified package, for the sake of
+ // outputting errors if they don't exist.
+ previous_resolve.query(name)?;
+ }
+
+ for ((name, source_id), (version_req, _)) in upgrades.iter() {
+ if let Some(matching_dep) = previous_resolve.iter().find(|dep| {
+ dep.name() == *name
+ && dep.source_id() == *source_id
+ && version_req.matches(dep.version())
+ }) {
+ let spec = PackageIdSpec::new(name.to_string())
+ .with_url(source_id.url().clone())
+ .with_version(matching_dep.version().clone().into());
+ let spec = format!("{spec}");
+ let pid = previous_resolve.query(&spec)?;
+ to_avoid.insert(pid);
+ } else {
+ // Should never happen
+ anyhow::bail!(
+ "no package named `{name}` with source `{source_id}` and version matching `{version_req}` in the previous lockfile",
+ )
+ }
+ }
+ } else if opts.to_update.is_empty() {
  if !opts.workspace {
  to_avoid.extend(previous_resolve.iter());
  to_avoid.extend(previous_resolve.unused_patches());
@@ -185,11 +234,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
  opts.precise.is_some(),
  &mut registry,
  )?;
- if opts.dry_run {
- opts.gctx
- .shell()
- .warn("not updating lockfile due to dry run")?;
- } else {
+ if !opts.dry_run {
  ops::write_pkg_lockfile(ws, &mut resolve)?;
  }
  Ok(())
@@ -361,7 +406,10 @@ fn upgrade_dependency(
  .status_with_color("Upgrading", &upgrade_message, &style::GOOD)?;
  }
 
- upgrades.insert((name.to_string(), dependency.source_id()), latest.clone());
+ upgrades.insert(
+ (name, dependency.source_id()),
+ (current.clone(), latest.clone()),
+ );
 
  let req = OptVersionReq::Req(VersionReq::parse(&latest.to_string())?);
  let mut dep = dependency.clone();
@@ -433,7 +481,7 @@ pub fn write_manifest_upgrades(
  continue;
  };
 
- let Some(latest) = upgrades.get(&(name.to_owned(), source_id)) else {
+ let Some((_, latest)) = upgrades.get(&(name.into(), source_id)) else {
  trace!("skipping dependency without an upgrade: {name}");
  continue;
  };

tests/testsuite/update.rs

@@ -925,7 +925,7 @@ fn dry_run_update() {
 [LOCKING] 1 package to latest compatible version
 [UPDATING] serde v0.1.0 -> v0.1.1
 [NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
-[WARNING] not updating lockfile due to dry run
+[WARNING] aborting update due to dry run
 
 "#]])
  .run();
@@ -1524,7 +1524,7 @@ fn report_behind() {
 [LOCKING] 1 package to latest compatible version
 [UPDATING] breaking v0.1.0 -> v0.1.1 (latest: v0.2.0)
 [NOTE] pass `--verbose` to see 2 unchanged dependencies behind latest
-[WARNING] not updating lockfile due to dry run
+[WARNING] aborting update due to dry run
 
 "#]])
  .run();
@@ -1537,7 +1537,7 @@ fn report_behind() {
 [UNCHANGED] pre v1.0.0-alpha.0 (latest: v1.0.0-alpha.1)
 [UNCHANGED] two-ver v0.1.0 (latest: v0.2.0)
 [NOTE] to see how you depend on a package, run `cargo tree --invert --package <dep>@<ver>`
-[WARNING] not updating lockfile due to dry run
+[WARNING] aborting update due to dry run
 
 "#]])
  .run();
@@ -1549,7 +1549,7 @@ fn report_behind() {
 [UPDATING] `dummy-registry` index
 [LOCKING] 0 packages to latest compatible versions
 [NOTE] pass `--verbose` to see 3 unchanged dependencies behind latest
-[WARNING] not updating lockfile due to dry run
+[WARNING] aborting update due to dry run
 
 "#]])
  .run();
@@ -1562,7 +1562,7 @@ fn report_behind() {
 [UNCHANGED] pre v1.0.0-alpha.0 (latest: v1.0.0-alpha.1)
 [UNCHANGED] two-ver v0.1.0 (latest: v0.2.0)
 [NOTE] to see how you depend on a package, run `cargo tree --invert --package <dep>@<ver>`
-[WARNING] not updating lockfile due to dry run
+[WARNING] aborting update due to dry run
 
 "#]])
  .run();
@@ -1912,10 +1912,13 @@ fn update_breaking() {
 [UPDATING] multiple-registries v2.0.0 (registry `alternative`) -> v3.0.0
 [UPDATING] multiple-registries v1.0.0 -> v2.0.0
 [UPDATING] multiple-source-types v1.0.0 -> v2.0.0
+[REMOVING] multiple-versions v1.0.0
+[REMOVING] multiple-versions v2.0.0
 [ADDING] multiple-versions v3.0.0
 [UPDATING] platform-specific v1.0.0 -> v2.0.0
 [UPDATING] shared v1.0.0 -> v2.0.0
 [UPDATING] ws v1.0.0 -> v2.0.0
+[NOTE] pass `--verbose` to see 4 unchanged dependencies behind latest
 
 "#]])
  .run();
@@ -2108,6 +2111,7 @@ fn update_breaking_specific_packages() {
 [UPDATING] transitive-compatible v1.0.0 -> v1.0.1
 [UPDATING] transitive-incompatible v1.0.0 -> v2.0.0
 [UPDATING] ws v1.0.0 -> v2.0.0
+[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
 
 "#]])
  .run();
@@ -2163,6 +2167,8 @@ fn update_breaking_specific_packages_that_wont_update() {
  .masquerade_as_nightly_cargo(&["update-breaking"])
  .with_stderr_data(str![[r#"
 [UPDATING] `dummy-registry` index
+[LOCKING] 0 packages to latest compatible versions
+[NOTE] pass `--verbose` to see 5 unchanged dependencies behind latest
 
 "#]])
  .run();
@@ -2271,13 +2277,27 @@ fn update_breaking_spec_version() {
  // Spec version not matching our current dependencies
  p.cargo("update -Zunstable-options --breaking [email protected]")
  .masquerade_as_nightly_cargo(&["update-breaking"])
- .with_stderr_data(str![[r#""#]])
+ .with_status(101)
+ .with_stderr_data(str![[r#"
+[ERROR] package ID specification `[email protected]` did not match any packages
+Did you mean one of these?
+
+ [email protected]
+
+"#]])
  .run();
 
  // Spec source not matching our current dependencies
  p.cargo("update -Zunstable-options --breaking https://alternative.com#[email protected]")
  .masquerade_as_nightly_cargo(&["update-breaking"])
- .with_stderr_data(str![[r#""#]])
+ .with_status(101)
+ .with_stderr_data(str![[r#"
+[ERROR] package ID specification `https://alternative.com/#[email protected]` did not match any packages
+Did you mean one of these?
+
+ [email protected]
+
+"#]])
  .run();
 
  // Accepted spec
@@ -2288,6 +2308,7 @@ fn update_breaking_spec_version() {
 [UPGRADING] incompatible ^1.0 -> ^2.0
 [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();
@@ -2301,6 +2322,7 @@ fn update_breaking_spec_version() {
 [UPGRADING] incompatible ^2.0 -> ^3.0
 [LOCKING] 1 package to latest compatible version
 [UPDATING] incompatible v2.0.0 -> v3.0.0
+[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
 
 "#]])
  .run();
@@ -2310,19 +2332,35 @@ fn update_breaking_spec_version() {
  .masquerade_as_nightly_cargo(&["update-breaking"])
  .with_stderr_data(str![[r#"
 [UPDATING] `dummy-registry` index
+[LOCKING] 0 packages to latest compatible versions
+[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
 
 "#]])
  .run();
 
  // Non-existing versions
  p.cargo("update -Zunstable-options --breaking [email protected]")
  .masquerade_as_nightly_cargo(&["update-breaking"])
- .with_stderr_data(str![[r#""#]])
+ .with_status(101)
+ .with_stderr_data(str![[r#"
+[ERROR] package ID specification `[email protected]` did not match any packages
+Did you mean one of these?
+
+ [email protected]
+
+"#]])
  .run();
 
  p.cargo("update -Zunstable-options --breaking [email protected]")
  .masquerade_as_nightly_cargo(&["update-breaking"])
- .with_stderr_data(str![[r#""#]])
+ .with_status(101)
+ .with_stderr_data(str![[r#"
+[ERROR] package ID specification `[email protected]` did not match any packages
+Did you mean one of these?
+
+ [email protected]
+
+"#]])
  .run();
 }
 
@@ -2376,6 +2414,7 @@ fn update_breaking_spec_version_transitive() {
 [UPGRADING] dep ^1.0 -> ^3.0
 [LOCKING] 1 package to latest compatible version
 [UPDATING] dep v1.0.0 -> v3.0.0
+[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
 
 "#]])
  .run();
@@ -2385,6 +2424,8 @@ fn update_breaking_spec_version_transitive() {
  .masquerade_as_nightly_cargo(&["update-breaking"])
  .with_stderr_data(str![[r#"
 [UPDATING] `dummy-registry` index
+[LOCKING] 0 packages to latest compatible versions
+[NOTE] pass `--verbose` to see 1 unchanged dependencies behind latest
 
 "#]])
  .run();
@@ -2453,6 +2494,8 @@ fn update_breaking_mixed_compatibility() {
 [UPDATING] `dummy-registry` index
 [UPGRADING] mixed-compatibility ^1.0 -> ^2.0
 [LOCKING] 1 package to latest compatible version
+[REMOVING] mixed-compatibility v1.0.0
+[REMOVING] mixed-compatibility v2.0.0
 [ADDING] mixed-compatibility v2.0.1
 
 "#]])
@@ -2544,6 +2587,7 @@ fn update_breaking_mixed_pinning_renaming() {
 [ADDING] mixed-pinned v2.0.0
 [ADDING] mixed-ws-pinned v2.0.0
 [ADDING] renamed-from v2.0.0
+[NOTE] pass `--verbose` to see 3 unchanged dependencies behind latest
 
 "#]])
  .run();