From 388bb07bff6d6feaa9d8bea1cdca603507071aea Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 23 Aug 2024 14:35:33 -0500 Subject: [PATCH 1/4] refactor(resolve): Allow lookups on changes by id --- src/cargo/ops/cargo_update.rs | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs index 42d0bfc8713..fe65fe6b726 100644 --- a/src/cargo/ops/cargo_update.rs +++ b/src/cargo/ops/cargo_update.rs @@ -494,7 +494,7 @@ fn print_lockfile_generation( ) -> CargoResult<()> { let changes = PackageChange::new(ws, resolve); let num_pkgs: usize = changes - .iter() + .values() .filter(|change| change.kind.is_new() && !change.is_member.unwrap_or(false)) .count(); if num_pkgs == 0 { @@ -503,7 +503,7 @@ fn print_lockfile_generation( } status_locking(ws, num_pkgs)?; - for change in changes { + for change in changes.values() { if change.is_member.unwrap_or(false) { continue; }; @@ -555,7 +555,7 @@ fn print_lockfile_sync( ) -> CargoResult<()> { let changes = PackageChange::diff(ws, previous_resolve, resolve); let num_pkgs: usize = changes - .iter() + .values() .filter(|change| change.kind.is_new() && !change.is_member.unwrap_or(false)) .count(); if num_pkgs == 0 { @@ -564,7 +564,7 @@ fn print_lockfile_sync( } status_locking(ws, num_pkgs)?; - for change in changes { + for change in changes.values() { if change.is_member.unwrap_or(false) { continue; }; @@ -611,13 +611,16 @@ fn print_lockfile_updates( registry: &mut PackageRegistry<'_>, ) -> CargoResult<()> { let changes = PackageChange::diff(ws, previous_resolve, resolve); - let num_pkgs: usize = changes.iter().filter(|change| change.kind.is_new()).count(); + let num_pkgs: usize = changes + .values() + .filter(|change| change.kind.is_new()) + .count(); if !precise { status_locking(ws, num_pkgs)?; } let mut unchanged_behind = 0; - for change in changes { + for change in changes.values() { let possibilities = if let Some(query) = change.alternatives_query() { loop { match registry.query_vec(&query, QueryKind::Exact) { @@ -804,17 +807,24 @@ struct PackageChange { } impl PackageChange { - pub fn new(ws: &Workspace<'_>, resolve: &Resolve) -> Vec { + pub fn new(ws: &Workspace<'_>, resolve: &Resolve) -> IndexMap { let diff = PackageDiff::new(resolve); Self::with_diff(diff, ws) } - pub fn diff(ws: &Workspace<'_>, previous_resolve: &Resolve, resolve: &Resolve) -> Vec { + pub fn diff( + ws: &Workspace<'_>, + previous_resolve: &Resolve, + resolve: &Resolve, + ) -> IndexMap { let diff = PackageDiff::diff(previous_resolve, resolve); Self::with_diff(diff, ws) } - fn with_diff(diff: impl Iterator, ws: &Workspace<'_>) -> Vec { + fn with_diff( + diff: impl Iterator, + ws: &Workspace<'_>, + ) -> IndexMap { let member_ids: HashSet<_> = ws.members().map(|p| p.package_id()).collect(); let mut changes = IndexMap::new(); @@ -876,7 +886,7 @@ impl PackageChange { } } - changes.into_values().collect() + changes } /// For querying [`PackageRegistry`] for alternative versions to report to the user From 89cf552c0653258aaaae245f745999a085c690a7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 23 Aug 2024 16:05:15 -0500 Subject: [PATCH 2/4] test(resolve): Explicitly verify msrv report --- tests/testsuite/rust_version.rs | 114 ++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index ee664e2f814..a33054bf1aa 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -1076,3 +1076,117 @@ fn cargo_install_ignores_msrv_config() { "#]]) .run(); } + +#[cargo_test] +fn report_rust_versions() { + Package::new("dep-only-low-compatible", "1.55.0") + .rust_version("1.55.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + Package::new("dep-only-low-incompatible", "1.75.0") + .rust_version("1.75.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + Package::new("dep-only-high-compatible", "1.65.0") + .rust_version("1.65.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + Package::new("dep-only-high-incompatible", "1.75.0") + .rust_version("1.75.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + Package::new("dep-only-unset-unset", "1.0.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + Package::new("dep-only-unset-compatible", "1.75.0") + .rust_version("1.75.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + Package::new("dep-only-unset-incompatible", "1.2345.0") + .rust_version("1.2345.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + Package::new("dep-shared-compatible", "1.55.0") + .rust_version("1.55.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + Package::new("dep-shared-incompatible", "1.75.0") + .rust_version("1.75.0") + .file("src/lib.rs", "fn other_stuff() {}") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["high", "low", "unset"] + "#, + ) + .file( + "high/Cargo.toml", + r#" + [package] + name = "high" + edition = "2015" + rust-version = "1.70.0" + + [dependencies] + dep-only-high-compatible = "1" + dep-only-high-incompatible = "1" + dep-shared-compatible = "1" + dep-shared-incompatible = "1" + "#, + ) + .file("high/src/main.rs", "fn main(){}") + .file( + "low/Cargo.toml", + r#" + [package] + name = "low" + edition = "2015" + rust-version = "1.60.0" + + [dependencies] + dep-only-low-compatible = "1" + dep-only-low-incompatible = "1" + dep-shared-compatible = "1" + dep-shared-incompatible = "1" + "#, + ) + .file("low/src/main.rs", "fn main(){}") + .file( + "unset/Cargo.toml", + r#" + [package] + name = "unset" + edition = "2015" + + [dependencies] + dep-only-unset-unset = "1" + dep-only-unset-compatible = "1" + dep-only-unset-incompatible = "1" + dep-shared-compatible = "1" + dep-shared-incompatible = "1" + "#, + ) + .file("unset/src/main.rs", "fn main(){}") + .build(); + + p.cargo("update") + .env("CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS", "fallback") + .arg("-Zmsrv-policy") + .masquerade_as_nightly_cargo(&["msrv-policy"]) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[LOCKING] 9 packages to latest Rust 1.60.0 compatible versions +[ADDING] dep-only-high-compatible v1.65.0 (requires Rust 1.65.0) +[ADDING] dep-only-high-incompatible v1.75.0 (requires Rust 1.75.0) +[ADDING] dep-only-low-incompatible v1.75.0 (requires Rust 1.75.0) +[ADDING] dep-only-unset-compatible v1.75.0 (requires Rust 1.75.0) +[ADDING] dep-only-unset-incompatible v1.2345.0 (requires Rust 1.2345.0) +[ADDING] dep-shared-incompatible v1.75.0 (requires Rust 1.75.0) + +"#]]) + .run(); +} From 437b73ceb36ff1afcc47ffa44cf56546a79dbb37 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 23 Aug 2024 15:06:38 -0500 Subject: [PATCH 3/4] fix(resolve): Report incompatible packages with precise Rust version The nesting in `annotate_required_rust_version` might seem weird but its for a follow up commit. --- src/cargo/ops/cargo_update.rs | 85 +++++++++++++++++++++++++-------- tests/testsuite/rust_version.rs | 2 - 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs index fe65fe6b726..fd2a88ca7dc 100644 --- a/src/cargo/ops/cargo_update.rs +++ b/src/cargo/ops/cargo_update.rs @@ -492,7 +492,7 @@ fn print_lockfile_generation( resolve: &Resolve, registry: &mut PackageRegistry<'_>, ) -> CargoResult<()> { - let changes = PackageChange::new(ws, resolve); + let mut changes = PackageChange::new(ws, resolve); let num_pkgs: usize = changes .values() .filter(|change| change.kind.is_new() && !change.is_member.unwrap_or(false)) @@ -501,8 +501,9 @@ fn print_lockfile_generation( // nothing worth reporting return Ok(()); } - status_locking(ws, num_pkgs)?; + annotate_required_rust_version(ws, resolve, &mut changes); + status_locking(ws, num_pkgs)?; for change in changes.values() { if change.is_member.unwrap_or(false) { continue; @@ -523,7 +524,7 @@ fn print_lockfile_generation( }; let package_id = change.package_id; - let required_rust_version = report_required_rust_version(ws, resolve, package_id); + let required_rust_version = report_required_rust_version(resolve, change); let latest = report_latest(&possibilities, package_id); let note = required_rust_version.or(latest); @@ -553,7 +554,7 @@ fn print_lockfile_sync( resolve: &Resolve, registry: &mut PackageRegistry<'_>, ) -> CargoResult<()> { - let changes = PackageChange::diff(ws, previous_resolve, resolve); + let mut changes = PackageChange::diff(ws, previous_resolve, resolve); let num_pkgs: usize = changes .values() .filter(|change| change.kind.is_new() && !change.is_member.unwrap_or(false)) @@ -562,8 +563,9 @@ fn print_lockfile_sync( // nothing worth reporting return Ok(()); } - status_locking(ws, num_pkgs)?; + annotate_required_rust_version(ws, resolve, &mut changes); + status_locking(ws, num_pkgs)?; for change in changes.values() { if change.is_member.unwrap_or(false) { continue; @@ -586,7 +588,7 @@ fn print_lockfile_sync( }; let package_id = change.package_id; - let required_rust_version = report_required_rust_version(ws, resolve, package_id); + let required_rust_version = report_required_rust_version(resolve, change); let latest = report_latest(&possibilities, package_id); let note = required_rust_version.or(latest).unwrap_or_default(); @@ -610,15 +612,16 @@ fn print_lockfile_updates( precise: bool, registry: &mut PackageRegistry<'_>, ) -> CargoResult<()> { - let changes = PackageChange::diff(ws, previous_resolve, resolve); + let mut changes = PackageChange::diff(ws, previous_resolve, resolve); let num_pkgs: usize = changes .values() .filter(|change| change.kind.is_new()) .count(); + annotate_required_rust_version(ws, resolve, &mut changes); + if !precise { status_locking(ws, num_pkgs)?; } - let mut unchanged_behind = 0; for change in changes.values() { let possibilities = if let Some(query) = change.alternatives_query() { @@ -639,7 +642,7 @@ fn print_lockfile_updates( | PackageChangeKind::Upgraded | PackageChangeKind::Downgraded => { let package_id = change.package_id; - let required_rust_version = report_required_rust_version(ws, resolve, package_id); + let required_rust_version = report_required_rust_version(resolve, change); let latest = report_latest(&possibilities, package_id); let note = required_rust_version.or(latest).unwrap_or_default(); @@ -658,7 +661,7 @@ fn print_lockfile_updates( } PackageChangeKind::Unchanged => { let package_id = change.package_id; - let required_rust_version = report_required_rust_version(ws, resolve, package_id); + let required_rust_version = report_required_rust_version(resolve, change); let latest = report_latest(&possibilities, package_id); let note = required_rust_version.as_deref().or(latest.as_deref()); @@ -734,18 +737,14 @@ fn required_rust_version(ws: &Workspace<'_>) -> Option { } } -fn report_required_rust_version( - ws: &Workspace<'_>, - resolve: &Resolve, - package: PackageId, -) -> Option { - if package.source_id().is_path() { +fn report_required_rust_version(resolve: &Resolve, change: &PackageChange) -> Option { + if change.package_id.source_id().is_path() { return None; } - let summary = resolve.summary(package); + let summary = resolve.summary(change.package_id); let package_rust_version = summary.rust_version()?; - let workspace_rust_version = required_rust_version(ws)?; - if package_rust_version.is_compatible_with(&workspace_rust_version) { + let required_rust_version = change.required_rust_version.as_ref()?; + if package_rust_version.is_compatible_with(required_rust_version) { return None; } @@ -804,6 +803,7 @@ struct PackageChange { previous_id: Option, kind: PackageChangeKind, is_member: Option, + required_rust_version: Option, } impl PackageChange { @@ -847,6 +847,7 @@ impl PackageChange { previous_id: Some(previous_id), kind, is_member, + required_rust_version: None, }; changes.insert(change.package_id, change); } else { @@ -858,6 +859,7 @@ impl PackageChange { previous_id: None, kind, is_member, + required_rust_version: None, }; changes.insert(change.package_id, change); } @@ -869,6 +871,7 @@ impl PackageChange { previous_id: None, kind, is_member, + required_rust_version: None, }; changes.insert(change.package_id, change); } @@ -881,6 +884,7 @@ impl PackageChange { previous_id: None, kind, is_member, + required_rust_version: None, }; changes.insert(change.package_id, change); } @@ -1076,3 +1080,46 @@ impl PackageDiff { } } } + +fn annotate_required_rust_version( + ws: &Workspace<'_>, + resolve: &Resolve, + changes: &mut IndexMap, +) { + let rustc = ws.gctx().load_global_rustc(Some(ws)).ok(); + let rustc_version: Option = + rustc.as_ref().map(|rustc| rustc.version.clone().into()); + + if ws.resolve_honors_rust_version() { + let mut queue: std::collections::VecDeque<_> = ws + .members() + .map(|p| { + ( + p.rust_version() + .map(|r| r.clone().into_partial()) + .or_else(|| rustc_version.clone()), + p.package_id(), + ) + }) + .collect(); + while let Some((required_rust_version, current_id)) = queue.pop_front() { + let Some(required_rust_version) = required_rust_version else { + continue; + }; + if let Some(change) = changes.get_mut(¤t_id) { + if let Some(existing) = change.required_rust_version.as_ref() { + if *existing <= required_rust_version { + // Stop early; we already walked down this path with a better match + continue; + } + } + change.required_rust_version = Some(required_rust_version.clone()); + } + queue.extend( + resolve + .deps(current_id) + .map(|(dep, _)| (Some(required_rust_version.clone()), dep)), + ); + } + } +} diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index a33054bf1aa..2de7e72dc2d 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -1180,10 +1180,8 @@ fn report_rust_versions() { .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index [LOCKING] 9 packages to latest Rust 1.60.0 compatible versions -[ADDING] dep-only-high-compatible v1.65.0 (requires Rust 1.65.0) [ADDING] dep-only-high-incompatible v1.75.0 (requires Rust 1.75.0) [ADDING] dep-only-low-incompatible v1.75.0 (requires Rust 1.75.0) -[ADDING] dep-only-unset-compatible v1.75.0 (requires Rust 1.75.0) [ADDING] dep-only-unset-incompatible v1.2345.0 (requires Rust 1.2345.0) [ADDING] dep-shared-incompatible v1.75.0 (requires Rust 1.75.0) From 3e315518c623b6debfb7aee5ffb3bd7478e6b338 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 26 Aug 2024 14:12:45 -0500 Subject: [PATCH 4/4] fix(resolve): Elevate the MSRV severity style now that its precise --- src/cargo/ops/cargo_update.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs index fd2a88ca7dc..4bedcfb15aa 100644 --- a/src/cargo/ops/cargo_update.rs +++ b/src/cargo/ops/cargo_update.rs @@ -748,9 +748,9 @@ fn report_required_rust_version(resolve: &Resolve, change: &PackageChange) -> Op return None; } - let warn = style::WARN; + let error = style::ERROR; Some(format!( - " {warn}(requires Rust {package_rust_version}){warn:#}" + " {error}(requires Rust {package_rust_version}){error:#}" )) }