Skip to content

Commit 353cd87

Browse files
epageepage
committed
fix(resolve): With latest message, differentiate actionable updates
Instead of always listing the absolute latest version as a warning color, we now differentiate - compatible updates are always actionable - incompatible, direct deps are always actionable These get reported and made yellow while non-actionable messages are unstyled. This is not intended as *the* solution for #13908 though it makes improvements in that direction. This is prep work for improved MSRV reporting where we will differentiate this further by only considering MSRV-compatible updates as actionable (or rustc-compatible when not using MSRV-aware reslver). I just used a broad stroke to say "compatible" in the message means "semver compatible" and use `^` - We could focus on "compatible with dependent version reqs" which is what will be most actionable but seeing if we can get away without having to track all in-coming version reqs. - We could be more nuanced in language but the more verbose we are, the more we take away from higher priority messages
1 parent 545ca21 commit 353cd87

File tree

7 files changed

+69
-21
lines changed

7 files changed

+69
-21
lines changed

‎src/cargo/ops/cargo_update.rs

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -756,14 +756,31 @@ fn report_latest(possibilities: &[IndexSummary], change: &PackageChange) -> Opti
756756
return None;
757757
}
758758

759+
let version_req = package_id.version().to_caret_req();
759760
if let Some(version) = possibilities
760761
.iter()
761762
.map(|s| s.as_summary())
762-
.filter(|s| is_latest(s.version(), package_id.version()))
763+
.filter(|s| package_id.version() != s.version() && version_req.matches(s.version()))
763764
.map(|s| s.version().clone())
764765
.max()
765766
{
766767
let warn = style::WARN;
768+
let report = format!(" {warn}(latest compatible: v{version}){warn:#}");
769+
return Some(report);
770+
}
771+
772+
if let Some(version) = possibilities
773+
.iter()
774+
.map(|s| s.as_summary())
775+
.filter(|s| is_latest(s.version(), package_id.version()))
776+
.map(|s| s.version().clone())
777+
.max()
778+
{
779+
let warn = if change.is_transitive.unwrap_or(true) {
780+
Default::default()
781+
} else {
782+
style::WARN
783+
};
767784
let report = format!(" {warn}(latest: v{version}){warn:#}");
768785
return Some(report);
769786
}
@@ -801,13 +818,14 @@ struct PackageChange {
801818
previous_id: Option<PackageId>,
802819
kind: PackageChangeKind,
803820
is_member: Option<bool>,
821+
is_transitive: Option<bool>,
804822
required_rust_version: Option<PartialVersion>,
805823
}
806824

807825
impl PackageChange {
808826
pub fn new(ws: &Workspace<'_>, resolve: &Resolve) -> IndexMap<PackageId, Self> {
809827
let diff = PackageDiff::new(resolve);
810-
Self::with_diff(diff, ws)
828+
Self::with_diff(diff, ws, resolve)
811829
}
812830

813831
pub fn diff(
@@ -816,12 +834,13 @@ impl PackageChange {
816834
resolve: &Resolve,
817835
) -> IndexMap<PackageId, Self> {
818836
let diff = PackageDiff::diff(previous_resolve, resolve);
819-
Self::with_diff(diff, ws)
837+
Self::with_diff(diff, ws, resolve)
820838
}
821839

822840
fn with_diff(
823841
diff: impl Iterator<Item = PackageDiff>,
824842
ws: &Workspace<'_>,
843+
resolve: &Resolve,
825844
) -> IndexMap<PackageId, Self> {
826845
let member_ids: HashSet<_> = ws.members().map(|p| p.package_id()).collect();
827846

@@ -840,35 +859,41 @@ impl PackageChange {
840859
PackageChangeKind::Upgraded
841860
};
842861
let is_member = Some(member_ids.contains(&package_id));
862+
let is_transitive = Some(true);
843863
let change = Self {
844864
package_id,
845865
previous_id: Some(previous_id),
846866
kind,
847867
is_member,
868+
is_transitive,
848869
required_rust_version: None,
849870
};
850871
changes.insert(change.package_id, change);
851872
} else {
852873
for package_id in diff.removed {
853874
let kind = PackageChangeKind::Removed;
854875
let is_member = None;
876+
let is_transitive = None;
855877
let change = Self {
856878
package_id,
857879
previous_id: None,
858880
kind,
859881
is_member,
882+
is_transitive,
860883
required_rust_version: None,
861884
};
862885
changes.insert(change.package_id, change);
863886
}
864887
for package_id in diff.added {
865888
let kind = PackageChangeKind::Added;
866889
let is_member = Some(member_ids.contains(&package_id));
890+
let is_transitive = Some(true);
867891
let change = Self {
868892
package_id,
869893
previous_id: None,
870894
kind,
871895
is_member,
896+
is_transitive,
872897
required_rust_version: None,
873898
};
874899
changes.insert(change.package_id, change);
@@ -877,17 +902,32 @@ impl PackageChange {
877902
for package_id in diff.unchanged {
878903
let kind = PackageChangeKind::Unchanged;
879904
let is_member = Some(member_ids.contains(&package_id));
905+
let is_transitive = Some(true);
880906
let change = Self {
881907
package_id,
882908
previous_id: None,
883909
kind,
884910
is_member,
911+
is_transitive,
885912
required_rust_version: None,
886913
};
887914
changes.insert(change.package_id, change);
888915
}
889916
}
890917

918+
for member_id in &member_ids {
919+
let Some(change) = changes.get_mut(member_id) else {
920+
continue;
921+
};
922+
change.is_transitive = Some(false);
923+
for (direct_dep_id, _) in resolve.deps(*member_id) {
924+
let Some(change) = changes.get_mut(&direct_dep_id) else {
925+
continue;
926+
};
927+
change.is_transitive = Some(false);
928+
}
929+
}
930+
891931
changes
892932
}
893933

‎src/cargo/util/semver_ext.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,26 @@ use std::fmt::{self, Display};
55
pub trait VersionExt {
66
fn is_prerelease(&self) -> bool;
77

8-
fn to_exact_req(&self) -> VersionReq;
8+
fn to_req(&self, op: Op) -> VersionReq;
9+
10+
fn to_exact_req(&self) -> VersionReq {
11+
self.to_req(Op::Exact)
12+
}
13+
14+
fn to_caret_req(&self) -> VersionReq {
15+
self.to_req(Op::Caret)
16+
}
917
}
1018

1119
impl VersionExt for Version {
1220
fn is_prerelease(&self) -> bool {
1321
!self.pre.is_empty()
1422
}
1523

16-
fn to_exact_req(&self) -> VersionReq {
24+
fn to_req(&self, op: Op) -> VersionReq {
1725
VersionReq {
1826
comparators: vec![Comparator {
19-
op: Op::Exact,
27+
op,
2028
major: self.major,
2129
minor: Some(self.minor),
2230
patch: Some(self.patch),

‎tests/testsuite/direct_minimal_versions.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ fn simple() {
3333
.with_stderr_data(str![[r#"
3434
[UPDATING] `dummy-registry` index
3535
[LOCKING] 1 package
36-
[ADDING] dep v1.0.0 (latest: v1.1.0)
36+
[ADDING] dep v1.0.0 (latest compatible: v1.1.0)
3737
3838
"#]])
3939
.run();
@@ -122,7 +122,7 @@ fn yanked() {
122122
.with_stderr_data(str![[r#"
123123
[UPDATING] `dummy-registry` index
124124
[LOCKING] 1 package
125-
[ADDING] dep v1.1.0 (latest: v1.2.0)
125+
[ADDING] dep v1.1.0 (latest compatible: v1.2.0)
126126
127127
"#]])
128128
.run();
@@ -176,7 +176,7 @@ fn indirect() {
176176
.with_stderr_data(str![[r#"
177177
[UPDATING] `dummy-registry` index
178178
[LOCKING] 2 packages
179-
[ADDING] direct v1.0.0 (latest: v1.1.0)
179+
[ADDING] direct v1.0.0 (latest compatible: v1.1.0)
180180
181181
"#]])
182182
.run();

‎tests/testsuite/minimal_versions.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ fn minimal_version_cli() {
3535
.with_stderr_data(str![[r#"
3636
[UPDATING] `dummy-registry` index
3737
[LOCKING] 1 package to earliest compatible version
38-
[ADDING] dep v1.0.0 (latest: v1.1.0)
38+
[ADDING] dep v1.0.0 (latest compatible: v1.1.0)
3939
4040
"#]])
4141
.run();

‎tests/testsuite/rust_version.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ foo v0.0.1 ([ROOT]/foo)
242242
.with_stderr_data(str![[r#"
243243
[UPDATING] `dummy-registry` index
244244
[LOCKING] 2 packages to latest Rust 1.60.0 compatible versions
245-
[ADDING] newer-and-older v1.5.0 (latest: v1.6.0)
245+
[ADDING] newer-and-older v1.5.0 (latest compatible: v1.6.0)
246246
[ADDING] only-newer v1.6.0 (requires Rust 1.65.0)
247247
248248
"#]])
@@ -319,7 +319,7 @@ foo v0.0.1 ([ROOT]/foo)
319319
.with_stderr_data(str![[r#"
320320
[UPDATING] `dummy-registry` index
321321
[LOCKING] 2 packages to latest Rust 1.60.0 compatible versions
322-
[ADDING] newer-and-older v1.5.0 (latest: v1.6.0)
322+
[ADDING] newer-and-older v1.5.0 (latest compatible: v1.6.0)
323323
[ADDING] only-newer v1.6.0 (requires Rust 1.2345)
324324
325325
"#]])
@@ -490,7 +490,7 @@ higher v0.0.1 ([ROOT]/foo)
490490
.with_stderr_data(str![[r#"
491491
[UPDATING] `dummy-registry` index
492492
[LOCKING] 2 packages to latest Rust 1.50.0 compatible versions
493-
[ADDING] newer-and-older v1.5.0 (latest: v1.6.0)
493+
[ADDING] newer-and-older v1.5.0 (latest compatible: v1.6.0)
494494
[ADDING] only-newer v1.6.0 (requires Rust 1.65.0)
495495
496496
"#]])
@@ -619,7 +619,7 @@ fn resolve_edition2024() {
619619
.with_stderr_data(str![[r#"
620620
[UPDATING] `dummy-registry` index
621621
[LOCKING] 2 packages to latest Rust 1.60.0 compatible versions
622-
[ADDING] newer-and-older v1.5.0 (latest: v1.6.0)
622+
[ADDING] newer-and-older v1.5.0 (latest compatible: v1.6.0)
623623
[ADDING] only-newer v1.6.0 (requires Rust 1.65.0)
624624
625625
"#]])
@@ -723,7 +723,7 @@ fn resolve_v3() {
723723
.with_stderr_data(str![[r#"
724724
[UPDATING] `dummy-registry` index
725725
[LOCKING] 2 packages to latest Rust 1.60.0 compatible versions
726-
[ADDING] newer-and-older v1.5.0 (latest: v1.6.0)
726+
[ADDING] newer-and-older v1.5.0 (latest compatible: v1.6.0)
727727
[ADDING] only-newer v1.6.0 (requires Rust 1.65.0)
728728
729729
"#]])
@@ -871,7 +871,7 @@ fn update_msrv_resolve() {
871871
.with_stderr_data(str![[r#"
872872
[UPDATING] `dummy-registry` index
873873
[LOCKING] 1 package to latest Rust 1.60.0 compatible version
874-
[ADDING] bar v1.5.0 (latest: v1.6.0)
874+
[ADDING] bar v1.5.0 (latest compatible: v1.6.0)
875875
876876
"#]])
877877
.run();
@@ -932,7 +932,7 @@ fn update_precise_overrides_msrv_resolver() {
932932
.with_stderr_data(str![[r#"
933933
[UPDATING] `dummy-registry` index
934934
[LOCKING] 1 package to latest Rust 1.60.0 compatible version
935-
[ADDING] bar v1.5.0 (latest: v1.6.0)
935+
[ADDING] bar v1.5.0 (latest compatible: v1.6.0)
936936
937937
"#]])
938938
.run();
@@ -1019,7 +1019,7 @@ foo v0.0.1 ([ROOT]/foo)
10191019
.with_stderr_data(str![[r#"
10201020
[UPDATING] `dummy-registry` index
10211021
[LOCKING] 2 packages to latest Rust 1.60.0 compatible versions
1022-
[ADDING] newer-and-older v1.5.0 (latest: v1.6.0)
1022+
[ADDING] newer-and-older v1.5.0 (latest compatible: v1.6.0)
10231023
[ADDING] only-newer v1.6.0 (requires Rust 1.65.0)
10241024
[DOWNLOADING] crates ...
10251025
[DOWNLOADED] newer-and-older v1.5.0 (registry `dummy-registry`)

‎tests/testsuite/update.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,7 +1534,7 @@ fn report_behind() {
15341534
[UPDATING] `dummy-registry` index
15351535
[LOCKING] 1 package to latest compatible version
15361536
[UPDATING] breaking v0.1.0 -> v0.1.1 (latest: v0.2.0)
1537-
[UNCHANGED] pre v1.0.0-alpha.0 (latest: v1.0.0-alpha.1)
1537+
[UNCHANGED] pre v1.0.0-alpha.0 (latest compatible: v1.0.0-alpha.1)
15381538
[UNCHANGED] two-ver v0.1.0 (latest: v0.2.0)
15391539
[NOTE] to see how you depend on a package, run `cargo tree --invert --package <dep>@<ver>`
15401540
[WARNING] not updating lockfile due to dry run
@@ -1559,7 +1559,7 @@ fn report_behind() {
15591559
[UPDATING] `dummy-registry` index
15601560
[LOCKING] 0 packages to latest compatible versions
15611561
[UNCHANGED] breaking v0.1.1 (latest: v0.2.0)
1562-
[UNCHANGED] pre v1.0.0-alpha.0 (latest: v1.0.0-alpha.1)
1562+
[UNCHANGED] pre v1.0.0-alpha.0 (latest compatible: v1.0.0-alpha.1)
15631563
[UNCHANGED] two-ver v0.1.0 (latest: v0.2.0)
15641564
[NOTE] to see how you depend on a package, run `cargo tree --invert --package <dep>@<ver>`
15651565
[WARNING] not updating lockfile due to dry run

‎tests/testsuite/workspaces.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,7 @@ fn share_dependencies() {
697697
.with_stderr_data(str![[r#"
698698
[UPDATING] `dummy-registry` index
699699
[LOCKING] 1 package to latest compatible version
700-
[ADDING] dep1 v0.1.3 (latest: v0.1.8)
700+
[ADDING] dep1 v0.1.3 (latest compatible: v0.1.8)
701701
[DOWNLOADING] crates ...
702702
[DOWNLOADED] dep1 v0.1.3 (registry `dummy-registry`)
703703
[CHECKING] dep1 v0.1.3

0 commit comments

Comments
 (0)