From a16fb1f7762e23f1ef5fc7b58eedbb601370375b Mon Sep 17 00:00:00 2001 From: Tor Hovland <55164+torhovland@users.noreply.github.com> Date: Mon, 27 May 2024 12:08:32 +0200 Subject: [PATCH] Implement `cargo update --breaking`. Work in progress. --- src/bin/cargo/commands/update.rs | 17 +- src/cargo/core/summary.rs | 16 +- src/cargo/ops/cargo_update.rs | 177 +++++++++++++++++- src/cargo/ops/mod.rs | 2 + src/cargo/util/toml/mod.rs | 7 + src/cargo/util/toml_mut/manifest.rs | 56 ++++++ .../cargo_update/help/stdout.term.svg | 44 ++--- tests/testsuite/update.rs | 160 +++++++++++++--- 8 files changed, 421 insertions(+), 58 deletions(-) diff --git a/src/bin/cargo/commands/update.rs b/src/bin/cargo/commands/update.rs index fb394e4aa336..40e847766d71 100644 --- a/src/bin/cargo/commands/update.rs +++ b/src/bin/cargo/commands/update.rs @@ -35,6 +35,13 @@ pub fn cli() -> Command { .value_name("PRECISE") .requires("package-group"), ) + .arg( + flag( + "breaking", + "Update [SPEC] to latest incompatible versions (unless pinned)", + ) + .short('b'), + ) .arg_silent_suggestion() .arg( flag("workspace", "Only update the workspace packages") @@ -59,7 +66,8 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { gctx.cli_unstable().msrv_policy, )?; } - let ws = args.workspace(gctx)?; + + let mut ws = args.workspace(gctx)?; if args.is_present_with_zero_values("package") { print_available_packages(&ws)?; @@ -84,11 +92,16 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { let update_opts = UpdateOptions { recursive: args.flag("recursive"), precise: args.get_one::("precise").map(String::as_str), + breaking: args.flag("breaking"), to_update, dry_run: args.dry_run(), workspace: args.flag("workspace"), gctx, }; - ops::update_lockfile(&ws, &update_opts)?; + + let upgrades = ops::update_manifests(&mut ws, &update_opts)?; + ops::update_lockfile(&ws, &update_opts, &upgrades)?; + ops::write_manifests(&ws, &update_opts, &upgrades)?; + Ok(()) } diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index ec0197cf40d0..d7744e24ef73 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -103,15 +103,25 @@ impl Summary { Rc::make_mut(&mut self.inner).checksum = Some(cksum); } - pub fn map_dependencies(mut self, f: F) -> Summary + pub fn map_dependencies(self, mut f: F) -> Summary where F: FnMut(Dependency) -> Dependency, + { + self.try_map_dependencies(|dep| Ok(f(dep))).unwrap() + } + + pub fn try_map_dependencies(mut self, f: F) -> CargoResult + where + F: FnMut(Dependency) -> CargoResult, { { let slot = &mut Rc::make_mut(&mut self.inner).dependencies; - *slot = mem::take(slot).into_iter().map(f).collect(); + *slot = mem::take(slot) + .into_iter() + .map(f) + .collect::>()?; } - self + Ok(self) } pub fn map_source(self, to_replace: SourceId, replace_with: SourceId) -> Summary { diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs index fc22608e8f23..8907a31c4c90 100644 --- a/src/cargo/ops/cargo_update.rs +++ b/src/cargo/ops/cargo_update.rs @@ -8,10 +8,13 @@ use crate::ops; use crate::sources::source::QueryKind; use crate::util::cache_lock::CacheLockMode; use crate::util::context::GlobalContext; -use crate::util::style; +use crate::util::toml_mut::dependency::{RegistrySource, Source}; +use crate::util::toml_mut::manifest::LocalManifest; use crate::util::CargoResult; +use crate::util::{style, OptVersionReq}; +use semver::{Version, VersionReq}; use std::cmp::Ordering; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use tracing::debug; pub struct UpdateOptions<'a> { @@ -19,6 +22,7 @@ pub struct UpdateOptions<'a> { pub to_update: Vec, pub precise: Option<&'a str>, pub recursive: bool, + pub breaking: bool, pub dry_run: bool, pub workspace: bool, } @@ -41,7 +45,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: &HashMap, +) -> CargoResult<()> { if opts.recursive && opts.precise.is_some() { anyhow::bail!("cannot specify both recursive and precise simultaneously") } @@ -157,7 +165,12 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes .filter(|s| !s.is_registry()) .collect(); - let keep = |p: &PackageId| !to_avoid_sources.contains(&p.source_id()) && !to_avoid.contains(p); + let keep = |p: &PackageId| { + (!to_avoid_sources.contains(&p.source_id()) && !to_avoid.contains(p)) + // In case of `--breaking`, we want to keep all packages unchanged that + // didn't get upgraded. + || (opts.breaking && !upgrades.contains_key(&p.name().to_string())) + }; let mut resolve = ops::resolve_with_previous( &mut registry, @@ -207,6 +220,162 @@ pub fn print_lockfile_changes( } } +pub fn update_manifests( + ws: &mut Workspace<'_>, + opts: &UpdateOptions<'_>, +) -> CargoResult> { + let mut upgrades = HashMap::new(); + + if !opts.breaking { + return Ok(upgrades); + } + + // Updates often require a lot of modifications to the registry, so ensure + // that we're synchronized against other Cargos. + let _lock = ws + .gctx() + .acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?; + + let mut registry = PackageRegistry::new(opts.gctx)?; + registry.lock_patches(); + + for member in ws.members_mut() { + debug!("updating manifest for {}", member.name()); + + let new_summary = member.manifest().summary().clone().try_map_dependencies( + |dependency| -> CargoResult<_> { + if let OptVersionReq::Req(current) = dependency.version_req() { + let query = crate::core::dependency::Dependency::parse( + dependency.package_name(), + None, + dependency.source_id().clone(), + )?; + + let possibilities = { + loop { + match registry.query_vec(&query, QueryKind::Exact) { + std::task::Poll::Ready(res) => { + break res?; + } + std::task::Poll::Pending => registry.block_until_ready()?, + } + } + }; + + let latest = if !possibilities.is_empty() { + possibilities + .iter() + .map(|s| s.as_summary()) + .map(|s| s.version().clone()) + .max() + } else { + None + }; + + if let Some(latest) = latest.clone() { + if !current.matches(&latest) { + debug!( + "upgrading {} from {} to {}", + dependency.package_name(), + current, + latest + ); + + opts.gctx.shell().status_with_color( + "Upgrading", + format!( + "{} {} -> v{}", + dependency.package_name(), + current, + latest.to_string() + ), + &style::GOOD, + )?; + + upgrades.insert(dependency.package_name().to_string(), latest.clone()); + + let req = OptVersionReq::Req(VersionReq::parse(&latest.to_string())?); + let mut dep = dependency.clone(); + dep.set_version_req(req); + return Ok(dep); + } + } + } + + Ok(dependency) + }, + )?; + + let summary = member.manifest_mut().summary_mut(); + *summary = new_summary; + } + + Ok(upgrades) +} + +pub fn write_manifests( + ws: &Workspace<'_>, + opts: &UpdateOptions<'_>, + upgrades: &HashMap, +) -> CargoResult<()> { + if !opts.breaking { + return Ok(()); + } + + for member in ws.members() { + debug!("writing manifest for {}", member.name()); + + let manifest_path = member.manifest_path(); + + let mut local_manifest = LocalManifest::try_new(&manifest_path)?; + for dep_table in local_manifest.get_dependency_tables_mut() { + for (dep_key, dep_item) in dep_table.iter_mut() { + debug!("updating dependency {}", dep_key); + + let dep_key = dep_key.get(); + let dependency = match crate::util::toml_mut::dependency::Dependency::from_toml( + &manifest_path, + dep_key, + dep_item, + ) { + Ok(dependency) => dependency, + Err(err) => { + opts.gctx + .shell() + .warn(&format!("ignoring {dep_key}, unsupported entry: {err}"))?; + continue; + } + }; + + if let crate::util::toml_mut::dependency::MaybeWorkspace::Other(_) = + dependency.source_id(opts.gctx)? + { + if let Some(latest) = upgrades.get(&dependency.name) { + let mut dep = + crate::util::toml_mut::dependency::Dependency::new(&dependency.name); + dep.source = Some(Source::Registry(RegistrySource { + version: latest.to_string(), + })); + + *dep_item = dep.to_toml(manifest_path); + } + } + } + } + + if opts.dry_run { + opts.gctx + .shell() + .warn("not updating manifest due to dry run")?; + } else { + debug!("writing updated manifest to {}", manifest_path.display()); + local_manifest.write()?; + } + } + + Ok(()) +} + fn print_lockfile_generation( ws: &Workspace<'_>, resolve: &Resolve, diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index 88b422e5b301..45bfc693d6df 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -19,6 +19,8 @@ pub use self::cargo_uninstall::uninstall; pub use self::cargo_update::generate_lockfile; pub use self::cargo_update::print_lockfile_changes; pub use self::cargo_update::update_lockfile; +pub use self::cargo_update::update_manifests; +pub use self::cargo_update::write_manifests; pub use self::cargo_update::UpdateOptions; pub use self::fix::{fix, fix_exec_rustc, fix_get_proxy_lock_addr, FixOptions}; pub use self::lockfile::{load_pkg_lockfile, resolve_to_string, write_pkg_lockfile}; diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 9313a0ddd2c0..0c57d8af0543 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -4,6 +4,7 @@ use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::rc::Rc; use std::str::{self, FromStr}; +use tracing::debug; use crate::AlreadyPrintedError; use anyhow::{anyhow, bail, Context as _}; @@ -60,6 +61,12 @@ pub fn read_manifest( let mut warnings = Default::default(); let mut errors = Default::default(); + debug!( + "read_manifest; path={}; source-id={}", + path.display(), + source_id + ); + let contents = read_toml_string(path, gctx).map_err(|err| ManifestError::new(err, path.into()))?; let document = diff --git a/src/cargo/util/toml_mut/manifest.rs b/src/cargo/util/toml_mut/manifest.rs index 1fbfb9deccab..30f554bc4b45 100644 --- a/src/cargo/util/toml_mut/manifest.rs +++ b/src/cargo/util/toml_mut/manifest.rs @@ -64,6 +64,14 @@ impl DepTable { vec![self.kind.kind_table()] } } + + fn kind_table(&self) -> &str { + match self.kind { + DepKind::Normal => "dependencies", + DepKind::Development => "dev-dependencies", + DepKind::Build => "build-dependencies", + } + } } impl Default for DepTable { @@ -397,6 +405,54 @@ impl LocalManifest { Ok(()) } + /// Allow mutating depedencies, wherever they live + pub fn get_dependency_tables_mut( + &mut self, + ) -> impl Iterator + '_ { + let root = self.data.as_table_mut(); + root.iter_mut().flat_map(|(k, v)| { + if DepTable::KINDS + .iter() + .any(|kind| kind.kind_table() == k.get()) + { + v.as_table_like_mut().into_iter().collect::>() + } else if k == "workspace" { + v.as_table_like_mut() + .unwrap() + .iter_mut() + .filter_map(|(k, v)| { + if k.get() == "dependencies" { + v.as_table_like_mut() + } else { + None + } + }) + .collect::>() + } else if k == "target" { + v.as_table_like_mut() + .unwrap() + .iter_mut() + .flat_map(|(_, v)| { + v.as_table_like_mut().into_iter().flat_map(|v| { + v.iter_mut().filter_map(|(k, v)| { + if DepTable::KINDS + .iter() + .any(|kind| kind.kind_table() == k.get()) + { + v.as_table_like_mut() + } else { + None + } + }) + }) + }) + .collect::>() + } else { + Vec::new() + } + }) + } + /// Remove references to `dep_key` if its no longer present. pub fn gc_dep(&mut self, dep_key: &str) { let explicit_dep_activation = self.is_explicit_dep_activation(dep_key); diff --git a/tests/testsuite/cargo_update/help/stdout.term.svg b/tests/testsuite/cargo_update/help/stdout.term.svg index 9642c49d2a2d..9e597e0e9ecd 100644 --- a/tests/testsuite/cargo_update/help/stdout.term.svg +++ b/tests/testsuite/cargo_update/help/stdout.term.svg @@ -1,4 +1,4 @@ - +