From 0924f839f37b2241be4234ed5d1ec91330fd0b49 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 14 Aug 2024 09:49:56 -0500 Subject: [PATCH 1/5] refactor(update): Consolidate latest message rendering --- src/cargo/ops/cargo_update.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs index ab78988e53d..f0f02b6a8a9 100644 --- a/src/cargo/ops/cargo_update.rs +++ b/src/cargo/ops/cargo_update.rs @@ -498,10 +498,6 @@ fn print_lockfile_generation( status_locking(ws, num_pkgs)?; for diff in diff { - fn format_latest(version: semver::Version) -> String { - let warn = style::WARN; - format!(" {warn}(latest: v{version}){warn:#}") - } let possibilities = if let Some(query) = diff.alternatives_query() { loop { match registry.query_vec(&query, QueryKind::Exact) { @@ -555,10 +551,6 @@ fn print_lockfile_sync( status_locking(ws, num_pkgs)?; for diff in diff { - fn format_latest(version: semver::Version) -> String { - let warn = style::WARN; - format!(" {warn}(latest: v{version}){warn:#}") - } let possibilities = if let Some(query) = diff.alternatives_query() { loop { match registry.query_vec(&query, QueryKind::Exact) { @@ -650,10 +642,6 @@ fn print_lockfile_updates( let mut unchanged_behind = 0; for diff in diff { - fn format_latest(version: semver::Version) -> String { - let warn = style::WARN; - format!(" {warn}(latest: v{version}){warn:#}") - } let possibilities = if let Some(query) = diff.alternatives_query() { loop { match registry.query_vec(&query, QueryKind::Exact) { @@ -807,6 +795,11 @@ fn status_locking(ws: &Workspace<'_>, num_pkgs: usize) -> CargoResult<()> { Ok(()) } +fn format_latest(version: semver::Version) -> String { + let warn = style::WARN; + format!(" {warn}(latest: v{version}){warn:#}") +} + fn is_latest(candidate: &semver::Version, current: &semver::Version) -> bool { current < candidate // Only match pre-release if major.minor.patch are the same From ad4f6471e796b143eab084b3d81b22fe9c2529c0 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 14 Aug 2024 10:00:56 -0500 Subject: [PATCH 2/5] refactor(update): Consolidate latest calculation --- src/cargo/ops/cargo_update.rs | 74 ++++++++--------------------------- 1 file changed, 16 insertions(+), 58 deletions(-) diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs index f0f02b6a8a9..f0a94b754e1 100644 --- a/src/cargo/ops/cargo_update.rs +++ b/src/cargo/ops/cargo_update.rs @@ -7,6 +7,7 @@ use crate::core::{PackageId, PackageIdSpec, PackageIdSpecQuery}; use crate::core::{Resolve, SourceId, Workspace}; use crate::ops; use crate::sources::source::QueryKind; +use crate::sources::IndexSummary; use crate::util::cache_lock::CacheLockMode; use crate::util::context::GlobalContext; use crate::util::toml_mut::dependency::{MaybeWorkspace, Source}; @@ -512,17 +513,7 @@ fn print_lockfile_generation( }; for package in diff.added.iter() { - let latest = if !possibilities.is_empty() { - possibilities - .iter() - .map(|s| s.as_summary()) - .filter(|s| is_latest(s.version(), package.version())) - .map(|s| s.version().clone()) - .max() - .map(format_latest) - } else { - None - }; + let latest = report_latest(&possibilities, *package); if let Some(latest) = latest { ws.gctx().shell().status_with_color( @@ -602,18 +593,7 @@ fn print_lockfile_sync( } } else { for package in diff.added.iter() { - let latest = if !possibilities.is_empty() { - possibilities - .iter() - .map(|s| s.as_summary()) - .filter(|s| is_latest(s.version(), package.version())) - .map(|s| s.version().clone()) - .max() - .map(format_latest) - } else { - None - } - .unwrap_or_default(); + let latest = report_latest(&possibilities, *package).unwrap_or_default(); ws.gctx().shell().status_with_color( "Adding", @@ -656,18 +636,7 @@ fn print_lockfile_updates( }; if let Some((removed, added)) = diff.change() { - let latest = if !possibilities.is_empty() { - possibilities - .iter() - .map(|s| s.as_summary()) - .filter(|s| is_latest(s.version(), added.version())) - .map(|s| s.version().clone()) - .max() - .map(format_latest) - } else { - None - } - .unwrap_or_default(); + let latest = report_latest(&possibilities, *added).unwrap_or_default(); let msg = if removed.source_id().is_git() { format!( @@ -700,18 +669,7 @@ fn print_lockfile_updates( )?; } for package in diff.added.iter() { - let latest = if !possibilities.is_empty() { - possibilities - .iter() - .map(|s| s.as_summary()) - .filter(|s| is_latest(s.version(), package.version())) - .map(|s| s.version().clone()) - .max() - .map(format_latest) - } else { - None - } - .unwrap_or_default(); + let latest = report_latest(&possibilities, *package).unwrap_or_default(); ws.gctx().shell().status_with_color( "Adding", @@ -721,17 +679,7 @@ fn print_lockfile_updates( } } for package in &diff.unchanged { - let latest = if !possibilities.is_empty() { - possibilities - .iter() - .map(|s| s.as_summary()) - .filter(|s| is_latest(s.version(), package.version())) - .map(|s| s.version().clone()) - .max() - .map(format_latest) - } else { - None - }; + let latest = report_latest(&possibilities, *package); if let Some(latest) = latest { unchanged_behind += 1; @@ -795,6 +743,16 @@ fn status_locking(ws: &Workspace<'_>, num_pkgs: usize) -> CargoResult<()> { Ok(()) } +fn report_latest(possibilities: &[IndexSummary], package: PackageId) -> Option { + possibilities + .iter() + .map(|s| s.as_summary()) + .filter(|s| is_latest(s.version(), package.version())) + .map(|s| s.version().clone()) + .max() + .map(format_latest) +} + fn format_latest(version: semver::Version) -> String { let warn = style::WARN; format!(" {warn}(latest: v{version}){warn:#}") From 497c8b68e262908413f64a32fa260e2ae32f0d76 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 14 Aug 2024 10:07:51 -0500 Subject: [PATCH 3/5] refactor(update): Extract rust-version lookup --- src/cargo/ops/cargo_update.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs index f0a94b754e1..45465693994 100644 --- a/src/cargo/ops/cargo_update.rs +++ b/src/cargo/ops/cargo_update.rs @@ -16,6 +16,7 @@ use crate::util::toml_mut::upgrade::upgrade_requirement; use crate::util::{style, OptVersionReq}; use crate::util::{CargoResult, VersionExt}; use anyhow::Context as _; +use cargo_util_schemas::core::PartialVersion; use itertools::Itertools; use semver::{Op, Version, VersionReq}; use std::cmp::Ordering; @@ -724,14 +725,7 @@ fn status_locking(ws: &Workspace<'_>, num_pkgs: usize) -> CargoResult<()> { write!(&mut cfg, " latest")?; } - if ws.resolve_honors_rust_version() { - let rust_version = if let Some(ver) = ws.rust_version() { - ver.clone().into_partial() - } else { - let rustc = ws.gctx().load_global_rustc(Some(ws))?; - let rustc_version = rustc.version.clone().into(); - rustc_version - }; + if let Some(rust_version) = required_rust_version(ws) { write!(&mut cfg, " Rust {rust_version}")?; } write!(&mut cfg, " compatible version{plural}")?; @@ -743,6 +737,20 @@ fn status_locking(ws: &Workspace<'_>, num_pkgs: usize) -> CargoResult<()> { Ok(()) } +fn required_rust_version(ws: &Workspace<'_>) -> Option { + if !ws.resolve_honors_rust_version() { + return None; + } + + if let Some(ver) = ws.rust_version() { + Some(ver.clone().into_partial()) + } else { + let rustc = ws.gctx().load_global_rustc(Some(ws)).ok()?; + let rustc_version = rustc.version.clone().into(); + Some(rustc_version) + } +} + fn report_latest(possibilities: &[IndexSummary], package: PackageId) -> Option { possibilities .iter() From a4c4c2d42aace8fd7b08b2cb87f072ddefb5c8c5 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 14 Aug 2024 11:40:11 -0500 Subject: [PATCH 4/5] fix(update): Only show 'latest' messages for registry sources --- src/cargo/ops/cargo_update.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs index 45465693994..a1d69bf7412 100644 --- a/src/cargo/ops/cargo_update.rs +++ b/src/cargo/ops/cargo_update.rs @@ -752,6 +752,10 @@ fn required_rust_version(ws: &Workspace<'_>) -> Option { } fn report_latest(possibilities: &[IndexSummary], package: PackageId) -> Option { + if !package.source_id().is_registry() { + return None; + } + possibilities .iter() .map(|s| s.as_summary()) From 428e1732f1802ce759d733dda524ea118fd57cbd Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 14 Aug 2024 11:49:41 -0500 Subject: [PATCH 5/5] feat(update): Report when incompatible-rust-version packages are selected In discussin this in #13873, it highlighted that we need to make sure we tell people when we get in this state. I decided to keep "latest" and "required rust version" messages mutually exclusive to avoid too much noise. I gave "required rust version" higher precedence as its the more critical to operation and, if you are using an MSRV-incompatible package, it likely is "latest" already. I was tempted to change colors to make "required rust version" stand out from "latest" but was unsure what direction to go, so I held off. Options included - red for "required rust version", yellow for "latest" - yellow for "required rust version", nothing for "latest" There is also more discussion on how to format "latest" at #13908. --- src/cargo/ops/cargo_update.rs | 57 ++++++++++++++++++++++++++------- tests/testsuite/rust_version.rs | 9 +++++- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/cargo/ops/cargo_update.rs b/src/cargo/ops/cargo_update.rs index a1d69bf7412..64990c2591f 100644 --- a/src/cargo/ops/cargo_update.rs +++ b/src/cargo/ops/cargo_update.rs @@ -514,12 +514,14 @@ fn print_lockfile_generation( }; for package in diff.added.iter() { + let required_rust_version = report_required_rust_version(ws, resolve, *package); let latest = report_latest(&possibilities, *package); + let note = required_rust_version.or(latest); - if let Some(latest) = latest { + if let Some(note) = note { ws.gctx().shell().status_with_color( "Adding", - format!("{package}{latest}"), + format!("{package}{note}"), &style::NOTE, )?; } @@ -594,11 +596,13 @@ fn print_lockfile_sync( } } else { for package in diff.added.iter() { - let latest = report_latest(&possibilities, *package).unwrap_or_default(); + let required_rust_version = report_required_rust_version(ws, resolve, *package); + let latest = report_latest(&possibilities, *package); + let note = required_rust_version.or(latest).unwrap_or_default(); ws.gctx().shell().status_with_color( "Adding", - format!("{package}{latest}"), + format!("{package}{note}"), &style::NOTE, )?; } @@ -637,15 +641,17 @@ fn print_lockfile_updates( }; if let Some((removed, added)) = diff.change() { - let latest = report_latest(&possibilities, *added).unwrap_or_default(); + let required_rust_version = report_required_rust_version(ws, resolve, *added); + let latest = report_latest(&possibilities, *added); + let note = required_rust_version.or(latest).unwrap_or_default(); let msg = if removed.source_id().is_git() { format!( - "{removed} -> #{}", + "{removed} -> #{}{note}", &added.source_id().precise_git_fragment().unwrap()[..8], ) } else { - format!("{removed} -> v{}{latest}", added.version()) + format!("{removed} -> v{}{note}", added.version()) }; // If versions differ only in build metadata, we call it an "update" @@ -670,24 +676,30 @@ fn print_lockfile_updates( )?; } for package in diff.added.iter() { - let latest = report_latest(&possibilities, *package).unwrap_or_default(); + let required_rust_version = report_required_rust_version(ws, resolve, *package); + let latest = report_latest(&possibilities, *package); + let note = required_rust_version.or(latest).unwrap_or_default(); ws.gctx().shell().status_with_color( "Adding", - format!("{package}{latest}"), + format!("{package}{note}"), &style::NOTE, )?; } } for package in &diff.unchanged { + let required_rust_version = report_required_rust_version(ws, resolve, *package); let latest = report_latest(&possibilities, *package); + let note = required_rust_version.as_deref().or(latest.as_deref()); - if let Some(latest) = latest { - unchanged_behind += 1; + if let Some(note) = note { + if latest.is_some() { + unchanged_behind += 1; + } if ws.gctx().shell().verbosity() == Verbosity::Verbose { ws.gctx().shell().status_with_color( "Unchanged", - format!("{package}{latest}"), + format!("{package}{note}"), &anstyle::Style::new().bold(), )?; } @@ -751,6 +763,27 @@ 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() { + return None; + } + let summary = resolve.summary(package); + 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) { + return None; + } + + let warn = style::WARN; + Some(format!( + " {warn}(requires Rust {package_rust_version}){warn:#}" + )) +} + fn report_latest(possibilities: &[IndexSummary], package: PackageId) -> Option { if !package.source_id().is_registry() { return None; diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index 3d696fc3345..2eeb22064e7 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -241,6 +241,7 @@ foo v0.0.1 ([ROOT]/foo) [UPDATING] `dummy-registry` index [LOCKING] 3 packages to latest Rust 1.60.0 compatible versions [ADDING] newer-and-older v1.5.0 (latest: v1.6.0) +[ADDING] only-newer v1.6.0 (requires Rust 1.65.0) "#]]) .run(); @@ -315,6 +316,7 @@ foo v0.0.1 ([ROOT]/foo) [UPDATING] `dummy-registry` index [LOCKING] 3 packages to latest Rust 1.60.0 compatible versions [ADDING] newer-and-older v1.5.0 (latest: v1.6.0) +[ADDING] only-newer v1.6.0 (requires Rust 1.2345) "#]]) .run(); @@ -387,6 +389,7 @@ foo v0.0.1 ([ROOT]/foo) .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index [LOCKING] 3 packages to latest Rust 1.60.0 compatible versions +[ADDING] has-rust-version v1.6.0 (requires Rust 1.65.0) "#]]) .run(); @@ -484,6 +487,7 @@ higher v0.0.1 ([ROOT]/foo) [UPDATING] `dummy-registry` index [LOCKING] 4 packages to latest Rust 1.50.0 compatible versions [ADDING] newer-and-older v1.5.0 (latest: v1.6.0) +[ADDING] only-newer v1.6.0 (requires Rust 1.65.0) "#]]) .run(); @@ -612,6 +616,7 @@ fn resolve_edition2024() { [UPDATING] `dummy-registry` index [LOCKING] 3 packages to latest Rust 1.60.0 compatible versions [ADDING] newer-and-older v1.5.0 (latest: v1.6.0) +[ADDING] only-newer v1.6.0 (requires Rust 1.65.0) "#]]) .run(); @@ -715,6 +720,7 @@ fn resolve_v3() { [UPDATING] `dummy-registry` index [LOCKING] 3 packages to latest Rust 1.60.0 compatible versions [ADDING] newer-and-older v1.5.0 (latest: v1.6.0) +[ADDING] only-newer v1.6.0 (requires Rust 1.65.0) "#]]) .run(); @@ -932,7 +938,7 @@ fn update_precise_overrides_msrv_resolver() { .masquerade_as_nightly_cargo(&["msrv-policy"]) .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index -[UPDATING] bar v1.5.0 -> v1.6.0 +[UPDATING] bar v1.5.0 -> v1.6.0 (requires Rust 1.65.0) "#]]) .run(); @@ -1010,6 +1016,7 @@ foo v0.0.1 ([ROOT]/foo) [UPDATING] `dummy-registry` index [LOCKING] 3 packages to latest Rust 1.60.0 compatible versions [ADDING] newer-and-older v1.5.0 (latest: v1.6.0) +[ADDING] only-newer v1.6.0 (requires Rust 1.65.0) [DOWNLOADING] crates ... [DOWNLOADED] newer-and-older v1.5.0 (registry `dummy-registry`) [CHECKING] newer-and-older v1.5.0