Skip to content

Commit f57599c

Browse files
committed
feat: Make cargo update --precise capable of doing breaking upgrades.
1 parent 42bdec1 commit f57599c

File tree

5 files changed

+231
-163
lines changed

5 files changed

+231
-163
lines changed

src/bin/cargo/commands/update.rs

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
use std::collections::HashMap;
2+
13
use crate::command_prelude::*;
24

35
use anyhow::anyhow;
46
use cargo::ops::{self, UpdateOptions};
57
use cargo::util::print_available_packages;
8+
use tracing::trace;
69

710
pub fn cli() -> Command {
811
subcommand("update")
@@ -92,28 +95,39 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
9295
let update_opts = UpdateOptions {
9396
recursive: args.flag("recursive"),
9497
precise: args.get_one::<String>("precise").map(String::as_str),
98+
breaking: args.flag("breaking"),
9599
to_update,
96100
dry_run: args.dry_run(),
97101
workspace: args.flag("workspace"),
98102
gctx,
99103
};
100104

101-
if args.flag("breaking") {
102-
gctx.cli_unstable()
103-
.fail_if_stable_opt("--breaking", 12425)?;
104-
105-
let upgrades = ops::upgrade_manifests(&mut ws, &update_opts.to_update)?;
106-
ops::resolve_ws(&ws, update_opts.dry_run)?;
107-
ops::write_manifest_upgrades(&ws, &upgrades, update_opts.dry_run)?;
105+
let breaking_update = update_opts.breaking
106+
|| (update_opts.precise.is_some() && gctx.cli_unstable().unstable_options);
108107

109-
if update_opts.dry_run {
110-
update_opts
111-
.gctx
112-
.shell()
113-
.warn("aborting update due to dry run")?;
108+
// We are using the term "upgrade" here, which is the typical case, but
109+
// it can also be a downgrade. In general, it is a change to a version
110+
// req matching a precise version.
111+
let upgrades = if breaking_update {
112+
if update_opts.breaking {
113+
gctx.cli_unstable()
114+
.fail_if_stable_opt("--breaking", 12425)?;
114115
}
116+
117+
trace!("allowing breaking updates");
118+
ops::upgrade_manifests(&mut ws, &update_opts.to_update, &update_opts.precise)?
115119
} else {
116-
ops::update_lockfile(&ws, &update_opts)?;
120+
HashMap::new()
121+
};
122+
123+
ops::update_lockfile(&ws, &update_opts, &upgrades)?;
124+
ops::write_manifest_upgrades(&ws, &upgrades, update_opts.dry_run)?;
125+
126+
if update_opts.dry_run {
127+
update_opts
128+
.gctx
129+
.shell()
130+
.warn("aborting update due to dry run")?;
117131
}
118132

119133
Ok(())

src/cargo/ops/cargo_update.rs

Lines changed: 69 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,31 @@ use crate::util::toml_mut::manifest::LocalManifest;
1414
use crate::util::toml_mut::upgrade::upgrade_requirement;
1515
use crate::util::{style, OptVersionReq};
1616
use crate::util::{CargoResult, VersionExt};
17+
use anyhow::Context as _;
1718
use itertools::Itertools;
1819
use semver::{Op, Version, VersionReq};
1920
use std::cmp::Ordering;
2021
use std::collections::{BTreeMap, HashMap, HashSet};
2122
use tracing::{debug, trace};
2223

23-
pub type UpgradeMap = HashMap<(String, SourceId), Version>;
24+
/// A map of all breaking upgrades which is filled in by
25+
/// upgrade_manifests/upgrade_dependency when going through workspace member
26+
/// manifests, and later used by write_manifest_upgrades in order to know which
27+
/// upgrades to write to manifest files on disk. Also used by update_lockfile to
28+
/// know which dependencies to keep unchanged if any have been upgraded (we will
29+
/// do either breaking or non-breaking updates, but not both).
30+
pub type UpgradeMap = HashMap<
31+
// The key is a tuple of the package name and the source id of the package.
32+
(String, SourceId),
33+
// The value is the upgraded version of the package.
34+
Version,
35+
>;
2436

2537
pub struct UpdateOptions<'a> {
2638
pub gctx: &'a GlobalContext,
2739
pub to_update: Vec<String>,
2840
pub precise: Option<&'a str>,
41+
pub breaking: bool,
2942
pub recursive: bool,
3043
pub dry_run: bool,
3144
pub workspace: bool,
@@ -49,7 +62,11 @@ pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> {
4962
Ok(())
5063
}
5164

52-
pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoResult<()> {
65+
pub fn update_lockfile(
66+
ws: &Workspace<'_>,
67+
opts: &UpdateOptions<'_>,
68+
upgrades: &UpgradeMap,
69+
) -> CargoResult<()> {
5370
if opts.recursive && opts.precise.is_some() {
5471
anyhow::bail!("cannot specify both recursive and precise simultaneously")
5572
}
@@ -165,7 +182,15 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
165182
.filter(|s| !s.is_registry())
166183
.collect();
167184

168-
let keep = |p: &PackageId| !to_avoid_sources.contains(&p.source_id()) && !to_avoid.contains(p);
185+
let breaking_precise_upgrades = opts.precise.is_some() && !upgrades.is_empty();
186+
let breaking_mode = opts.breaking || breaking_precise_upgrades;
187+
188+
let keep = |p: &PackageId| {
189+
(!to_avoid_sources.contains(&p.source_id()) && !to_avoid.contains(p))
190+
// In case of `--breaking` or precise upgrades, we want to keep all
191+
// packages unchanged that didn't get upgraded.
192+
|| (breaking_mode && !upgrades.contains_key(&(p.name().to_string(), p.source_id())))
193+
};
169194

170195
let mut resolve = ops::resolve_with_previous(
171196
&mut registry,
@@ -185,11 +210,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
185210
opts.precise.is_some(),
186211
&mut registry,
187212
)?;
188-
if opts.dry_run {
189-
opts.gctx
190-
.shell()
191-
.warn("not updating lockfile due to dry run")?;
192-
} else {
213+
if !opts.dry_run {
193214
ops::write_pkg_lockfile(ws, &mut resolve)?;
194215
}
195216
Ok(())
@@ -217,6 +238,7 @@ pub fn print_lockfile_changes(
217238
pub fn upgrade_manifests(
218239
ws: &mut Workspace<'_>,
219240
to_update: &Vec<String>,
241+
precise: &Option<&str>,
220242
) -> CargoResult<UpgradeMap> {
221243
let gctx = ws.gctx();
222244
let mut upgrades = HashMap::new();
@@ -245,6 +267,7 @@ pub fn upgrade_manifests(
245267
upgrade_dependency(
246268
&gctx,
247269
&to_update,
270+
precise,
248271
&mut registry,
249272
&mut upgrades,
250273
&mut upgrade_messages,
@@ -259,6 +282,7 @@ pub fn upgrade_manifests(
259282
fn upgrade_dependency(
260283
gctx: &GlobalContext,
261284
to_update: &Vec<PackageIdSpec>,
285+
precise: &Option<&str>,
262286
registry: &mut PackageRegistry<'_>,
263287
upgrades: &mut UpgradeMap,
264288
upgrade_messages: &mut HashSet<String>,
@@ -316,7 +340,7 @@ fn upgrade_dependency(
316340
let query =
317341
crate::core::dependency::Dependency::parse(name, None, dependency.source_id().clone())?;
318342

319-
let possibilities = {
343+
let possibilities = if precise.is_none() {
320344
loop {
321345
match registry.query_vec(&query, QueryKind::Exact) {
322346
std::task::Poll::Ready(res) => {
@@ -325,6 +349,8 @@ fn upgrade_dependency(
325349
std::task::Poll::Pending => registry.block_until_ready()?,
326350
}
327351
}
352+
} else {
353+
Vec::new()
328354
};
329355

330356
let latest = if !possibilities.is_empty() {
@@ -338,32 +364,60 @@ fn upgrade_dependency(
338364
None
339365
};
340366

341-
let Some(latest) = latest else {
367+
let new_version = if let Some(precise) = precise {
368+
Version::parse(precise)
369+
.with_context(|| format!("invalid version format for precise version `{precise}`"))?
370+
} else if let Some(latest) = latest {
371+
latest.clone()
372+
} else {
373+
// Breaking (not precise) upgrade did not find a latest version
342374
trace!("skipping dependency `{name}` without any published versions");
343375
return Ok(dependency);
344376
};
345377

346-
if current.matches(&latest) {
378+
if current.matches(&new_version) {
347379
trace!("skipping dependency `{name}` without a breaking update available");
348380
return Ok(dependency);
349381
}
350382

351-
let Some((new_req_string, _)) = upgrade_requirement(&current.to_string(), latest)? else {
383+
let Some((new_req_string, _)) = upgrade_requirement(&current.to_string(), &new_version)? else {
352384
trace!("skipping dependency `{name}` because the version requirement didn't change");
353385
return Ok(dependency);
354386
};
355387

356388
let upgrade_message = format!("{name} {current} -> {new_req_string}");
357389
trace!(upgrade_message);
358390

391+
let old_version = semver::Version::new(
392+
comparator.major,
393+
comparator.minor.unwrap_or_default(),
394+
comparator.patch.unwrap_or_default(),
395+
);
396+
let is_downgrade = new_version < old_version;
397+
let status = if is_downgrade {
398+
"Downgrading"
399+
} else {
400+
"Upgrading"
401+
};
402+
359403
if upgrade_messages.insert(upgrade_message.clone()) {
360404
gctx.shell()
361-
.status_with_color("Upgrading", &upgrade_message, &style::GOOD)?;
405+
.status_with_color(status, &upgrade_message, &style::WARN)?;
362406
}
363407

364-
upgrades.insert((name.to_string(), dependency.source_id()), latest.clone());
408+
upgrades.insert(
409+
(name.to_string(), dependency.source_id()),
410+
new_version.clone(),
411+
);
412+
413+
let new_version_req = VersionReq::parse(&new_version.to_string())?;
414+
415+
let req = if precise.is_some() {
416+
OptVersionReq::Precise(new_version, new_version_req)
417+
} else {
418+
OptVersionReq::Req(new_version_req)
419+
};
365420

366-
let req = OptVersionReq::Req(VersionReq::parse(&latest.to_string())?);
367421
let mut dep = dependency.clone();
368422
dep.set_version_req(req);
369423
Ok(dep)

src/cargo/util/toml_mut/upgrade.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use std::fmt::Display;
22

3+
use anyhow::bail;
4+
35
use crate::CargoResult;
46

57
/// Upgrade an existing requirement to a new version.
@@ -27,15 +29,8 @@ pub(crate) fn upgrade_requirement(
2729
new_req_text.remove(0);
2830
}
2931
// Validate contract
30-
#[cfg(debug_assertions)]
31-
{
32-
assert!(
33-
new_req.matches(version),
34-
"New req {} is invalid, because {} does not match {}",
35-
new_req_text,
36-
new_req,
37-
version
38-
)
32+
if !new_req.matches(version) {
33+
bail!("new requirement {new_req_text} is invalid, because it doesn't match {version}")
3934
}
4035
if new_req_text == req_text {
4136
Ok(None)

tests/testsuite/precise_pre_release.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,12 @@ fn update_pre_release() {
6565
p.cargo("update my-dependency --precise 0.1.2-pre.0 -Zunstable-options")
6666
.masquerade_as_nightly_cargo(&["precise-pre-release"])
6767
.with_stderr_data(str![[r#"
68+
[UPGRADING] my-dependency ^0.1.1 -> ^0.1.2-pre.0
6869
[UPDATING] `dummy-registry` index
69-
[UPDATING] my-dependency v0.1.1 -> v0.1.2-pre.0
7070
7171
"#]])
7272
.run();
73+
7374
let lockfile = p.read_lockfile();
7475
assert!(lockfile.contains("\nname = \"my-dependency\"\nversion = \"0.1.2-pre.0\""));
7576
}
@@ -98,8 +99,8 @@ fn update_pre_release_differ() {
9899
p.cargo("update -p my-dependency --precise 0.1.2-pre.0 -Zunstable-options")
99100
.masquerade_as_nightly_cargo(&["precise-pre-release"])
100101
.with_stderr_data(str![[r#"
102+
[DOWNGRADING] my-dependency ^0.1.2 -> ^0.1.2-pre.0
101103
[UPDATING] `dummy-registry` index
102-
[DOWNGRADING] my-dependency v0.1.2 -> v0.1.2-pre.0
103104
104105
"#]])
105106
.run();

0 commit comments

Comments
 (0)