fix: update --breaking now handles mixed renaming and pinning.

rust-lang · Jun 20, 2024 · 28f57be · 28f57be
1 parent 34c728a
commit 28f57be
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 19 deletions.
diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs
@@ -363,7 363,7 @@ fn upgrade_dependency(
  return Ok(dependency);
  }
 
- let Some(new_req_string) = upgrade_requirement(&current.to_string(), latest)? else {
  let Some((new_req_string, _)) = upgrade_requirement(&current.to_string(), latest)? else {
  trace!(
  "skipping dependency `{}` because the version requirement didn't change",
  name
@@ -426,41 426,62 @@ pub fn write_manifest_upgrades(
  dep_key_str,
  dep_item,
  )?;
  let name = &dependency.name;
 
  if let Some(renamed_to) = dependency.rename {
  trace!(
  "skipping dependency renamed from `{}` to `{}`",
  name,
  renamed_to
  );
  continue;
  }
 
  let Some(current) = dependency.version() else {
- trace!("skipping dependency without a version: {}", dependency.name);
  trace!("skipping dependency without a version: {}", name);
  continue;
  };
 
  let (MaybeWorkspace::Other(source_id), Some(Source::Registry(source))) =
  (dependency.source_id(ws.gctx())?, dependency.source())
  else {
- trace!("skipping non-registry dependency: {}", dependency.name);
  trace!("skipping non-registry dependency: {}", name);
  continue;
  };
 
- let Some(latest) = upgrades.get(&(dependency.name.to_owned(), source_id)) else {
  let Some(latest) = upgrades.get(&(name.to_owned(), source_id)) else {
  trace!("skipping dependency without an upgrade: {}", name);
  continue;
  };
 
  let Some((new_req_string, new_req)) = upgrade_requirement(current, latest)? else {
  trace!(
- "skipping dependency without an upgrade: {}",
- dependency.name
  "skipping dependency `{}` because the version requirement didn't change",
  name
  );
  continue;
  };
 
- let Some(new_req_string) = upgrade_requirement(current, latest)? else {
  let [comparator] = &new_req.comparators[..] else {
  trace!(
- "skipping dependency `{}` because the version requirement didn't change",
- dependency.name
  "skipping dependency `{}` with multiple version comparators: {:?}",
  name,
  new_req.comparators
  );
  continue;
  };
 
  if comparator.op != Op::Caret {
  trace!("skipping non-caret dependency `{}`: {}", name, comparator);
  continue;
  }
 
  let mut dep = dependency.clone();
  let mut source = source.clone();
  source.version = new_req_string;
  dep.source = Some(Source::Registry(source));
 
- trace!("upgrading dependency {}", dependency.name);
  trace!("upgrading dependency {}", name);
  dep.update_toml(&crate_root, &mut dep_key, dep_item);
  manifest_has_changed = true;
  any_file_has_changed = true;

diff --git a/src/cargo/util/toml_mut/upgrade.rs b/src/cargo/util/toml_mut/upgrade.rs
@@ -7,7 7,7 @@ use crate::CargoResult;
 pub(crate) fn upgrade_requirement(
  req: &str,
  version: &semver::Version,
-) -> CargoResult<Option<String>> {
 ) -> CargoResult<Option<(String, semver::VersionReq)>> {
  let req_text = req.to_string();
  let raw_req = semver::VersionReq::parse(&req_text)
  .expect("semver to generate valid version requirements");
@@ -40,7 40,7 @@ pub(crate) fn upgrade_requirement(
  if new_req_text == req_text {
  Ok(None)
  } else {
- Ok(Some(new_req_text))
  Ok(Some((new_req_text, new_req)))
  }
  }
 }
@@ -103,9 103,10 @@ mod test {
  #[track_caller]
  fn assert_req_bump<'a, O: Into<Option<&'a str>>>(version: &str, req: &str, expected: O) {
  let version = semver::Version::parse(version).unwrap();
- let actual = upgrade_requirement(req, &version).unwrap();
  let (actual, _) = upgrade_requirement(req, &version).unwrap().unwrap();
  let actual = actual.as_str();
  let expected = expected.into();
- assert_eq!(actual.as_deref(), expected);
  assert_eq!(Some(actual), expected);
  }
 
  #[test]

diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs
@@ -2545,20 2545,18 @@ fn update_breaking_mixed_pinning_renaming() {
 
  assert_e2e().eq(
  &root_manifest,
- // FIXME: The pinned dependency should not be upgraded.
  str![[r#"
 
  [workspace]
  members = ["foo", "bar"]
 
  [workspace.dependencies]
- mixed-ws-pinned = "=2.0"
  mixed-ws-pinned = "=1.0"
  "#]],
  );
 
  assert_e2e().eq(
  &foo_manifest,
- // FIXME: The pinned and renamed dependencies should not be upgraded.
  str![[r#"
 
  [package]
@@ -2568,9 2566,9 @@ fn update_breaking_mixed_pinning_renaming() {
  authors = []
 
  [dependencies]
- mixed-pinned = "=2.0"
  mixed-pinned = "=1.0"
  mixed-ws-pinned.workspace = true
- renamed-to = { package = "renamed-from", version = "2.0" }
  renamed-to = { package = "renamed-from", version = "1.0" }
  "#]],
  );