Skip to content

Commit

Permalink
fix: update --breaking now handles mixed renaming and pinning.
Browse files Browse the repository at this point in the history
  • Loading branch information
torhovland committed Jun 20, 2024
1 parent 34c728a commit 28f57be
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 19 deletions.
41 changes: 31 additions & 10 deletions src/cargo/ops/cargo_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 5 additions & 4 deletions src/cargo/util/toml_mut/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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)))
}
}
}
Expand Down Expand Up @@ -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]
Expand Down
8 changes: 3 additions & 5 deletions tests/testsuite/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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" }
"#]],
);

Expand Down

0 comments on commit 28f57be

Please sign in to comment.