Skip to content

Commit 70c01f9

Browse files
committed
fix: update --breaking now handles mixed renaming and pinning.
1 parent 34c728a commit 70c01f9

File tree

3 files changed

+39
-18
lines changed

3 files changed

+39
-18
lines changed

src/cargo/ops/cargo_update.rs

+31-10
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ fn upgrade_dependency(
363363
return Ok(dependency);
364364
}
365365

366-
let Some(new_req_string) = upgrade_requirement(&current.to_string(), latest)? else {
366+
let Some((new_req_string, _)) = upgrade_requirement(&current.to_string(), latest)? else {
367367
trace!(
368368
"skipping dependency `{}` because the version requirement didn't change",
369369
name
@@ -426,41 +426,62 @@ pub fn write_manifest_upgrades(
426426
dep_key_str,
427427
dep_item,
428428
)?;
429+
let name = &dependency.name;
430+
431+
if let Some(renamed_to) = dependency.rename {
432+
trace!(
433+
"skipping dependency renamed from `{}` to `{}`",
434+
name,
435+
renamed_to
436+
);
437+
continue;
438+
}
429439

430440
let Some(current) = dependency.version() else {
431-
trace!("skipping dependency without a version: {}", dependency.name);
441+
trace!("skipping dependency without a version: {}", name);
432442
continue;
433443
};
434444

435445
let (MaybeWorkspace::Other(source_id), Some(Source::Registry(source))) =
436446
(dependency.source_id(ws.gctx())?, dependency.source())
437447
else {
438-
trace!("skipping non-registry dependency: {}", dependency.name);
448+
trace!("skipping non-registry dependency: {}", name);
439449
continue;
440450
};
441451

442-
let Some(latest) = upgrades.get(&(dependency.name.to_owned(), source_id)) else {
452+
let Some(latest) = upgrades.get(&(name.to_owned(), source_id)) else {
453+
trace!("skipping dependency without an upgrade: {}", name);
454+
continue;
455+
};
456+
457+
let Some((new_req_string, new_req)) = upgrade_requirement(current, latest)? else {
443458
trace!(
444-
"skipping dependency without an upgrade: {}",
445-
dependency.name
459+
"skipping dependency `{}` because the version requirement didn't change",
460+
name
446461
);
447462
continue;
448463
};
449464

450-
let Some(new_req_string) = upgrade_requirement(current, latest)? else {
465+
let [comparator] = &new_req.comparators[..] else {
451466
trace!(
452-
"skipping dependency `{}` because the version requirement didn't change",
453-
dependency.name
467+
"skipping dependency `{}` with multiple version comparators: {:?}",
468+
name,
469+
new_req.comparators
454470
);
455471
continue;
456472
};
457473

474+
if comparator.op != Op::Caret {
475+
trace!("skipping non-caret dependency `{}`: {}", name, comparator);
476+
continue;
477+
}
478+
458479
let mut dep = dependency.clone();
459480
let mut source = source.clone();
460481
source.version = new_req_string;
461482
dep.source = Some(Source::Registry(source));
462483

463-
trace!("upgrading dependency {}", dependency.name);
484+
trace!("upgrading dependency {}", name);
464485
dep.update_toml(&crate_root, &mut dep_key, dep_item);
465486
manifest_has_changed = true;
466487
any_file_has_changed = true;

src/cargo/util/toml_mut/upgrade.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::CargoResult;
77
pub(crate) fn upgrade_requirement(
88
req: &str,
99
version: &semver::Version,
10-
) -> CargoResult<Option<String>> {
10+
) -> CargoResult<Option<(String, semver::VersionReq)>> {
1111
let req_text = req.to_string();
1212
let raw_req = semver::VersionReq::parse(&req_text)
1313
.expect("semver to generate valid version requirements");
@@ -40,7 +40,7 @@ pub(crate) fn upgrade_requirement(
4040
if new_req_text == req_text {
4141
Ok(None)
4242
} else {
43-
Ok(Some(new_req_text))
43+
Ok(Some((new_req_text, new_req)))
4444
}
4545
}
4646
}
@@ -103,7 +103,9 @@ mod test {
103103
#[track_caller]
104104
fn assert_req_bump<'a, O: Into<Option<&'a str>>>(version: &str, req: &str, expected: O) {
105105
let version = semver::Version::parse(version).unwrap();
106-
let actual = upgrade_requirement(req, &version).unwrap();
106+
let actual = upgrade_requirement(req, &version)
107+
.unwrap()
108+
.map(|(actual, _req)| actual);
107109
let expected = expected.into();
108110
assert_eq!(actual.as_deref(), expected);
109111
}

tests/testsuite/update.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -2545,20 +2545,18 @@ fn update_breaking_mixed_pinning_renaming() {
25452545

25462546
assert_e2e().eq(
25472547
&root_manifest,
2548-
// FIXME: The pinned dependency should not be upgraded.
25492548
str![[r#"
25502549
25512550
[workspace]
25522551
members = ["foo", "bar"]
25532552
25542553
[workspace.dependencies]
2555-
mixed-ws-pinned = "=2.0"
2554+
mixed-ws-pinned = "=1.0"
25562555
"#]],
25572556
);
25582557

25592558
assert_e2e().eq(
25602559
&foo_manifest,
2561-
// FIXME: The pinned and renamed dependencies should not be upgraded.
25622560
str![[r#"
25632561
25642562
[package]
@@ -2568,9 +2566,9 @@ fn update_breaking_mixed_pinning_renaming() {
25682566
authors = []
25692567
25702568
[dependencies]
2571-
mixed-pinned = "=2.0"
2569+
mixed-pinned = "=1.0"
25722570
mixed-ws-pinned.workspace = true
2573-
renamed-to = { package = "renamed-from", version = "2.0" }
2571+
renamed-to = { package = "renamed-from", version = "1.0" }
25742572
"#]],
25752573
);
25762574

0 commit comments

Comments
 (0)