Skip to content

Commit

Permalink
Auto merge of #14457 - epage:msrv, r=ehuss
Browse files Browse the repository at this point in the history
fix(resolve): Report incompatible packages with precise Rust version

### What does this PR try to resolve?

In #14401, we reported about MSRV issues as if we were the resolver.  We can be smarter than that though and report precise MSRV information.  This allows us to elevate the message from color from yellow to red.

This is also prep work for telling users when a newer, MSRV-compatible version of a package is available.

### How should we test and review this PR?

The report function I added is a little odd because a follow up commit will update it to report when a package is incompatible with rustc when the MSRV resolver is disabled and do this on stable.

### Additional information

This builds on #14445 to improve #14401
  • Loading branch information
bors committed Aug 27, 2024
2 parents ef854d2 + 3e31551 commit b8a4f1b
Show file tree
Hide file tree
Showing 2 changed files with 200 additions and 31 deletions.
119 changes: 88 additions & 31 deletions src/cargo/ops/cargo_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,18 +492,19 @@ 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
.iter()
.values()
.filter(|change| change.kind.is_new() && !change.is_member.unwrap_or(false))
.count();
if num_pkgs == 0 {
// nothing worth reporting
return Ok(());
}
status_locking(ws, num_pkgs)?;
annotate_required_rust_version(ws, resolve, &mut changes);

for change in changes {
status_locking(ws, num_pkgs)?;
for change in changes.values() {
if change.is_member.unwrap_or(false) {
continue;
};
Expand All @@ -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);

Expand Down Expand Up @@ -553,18 +554,19 @@ 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
.iter()
.values()
.filter(|change| change.kind.is_new() && !change.is_member.unwrap_or(false))
.count();
if num_pkgs == 0 {
// nothing worth reporting
return Ok(());
}
status_locking(ws, num_pkgs)?;
annotate_required_rust_version(ws, resolve, &mut changes);

for change in changes {
status_locking(ws, num_pkgs)?;
for change in changes.values() {
if change.is_member.unwrap_or(false) {
continue;
};
Expand All @@ -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();

Expand All @@ -610,14 +612,18 @@ fn print_lockfile_updates(
precise: bool,
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 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 {
for change in changes.values() {
let possibilities = if let Some(query) = change.alternatives_query() {
loop {
match registry.query_vec(&query, QueryKind::Exact) {
Expand All @@ -636,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();

Expand All @@ -655,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());

Expand Down Expand Up @@ -731,24 +737,20 @@ fn required_rust_version(ws: &Workspace<'_>) -> Option<PartialVersion> {
}
}

fn report_required_rust_version(
ws: &Workspace<'_>,
resolve: &Resolve,
package: PackageId,
) -> Option<String> {
if package.source_id().is_path() {
fn report_required_rust_version(resolve: &Resolve, change: &PackageChange) -> Option<String> {
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;
}

let warn = style::WARN;
let error = style::ERROR;
Some(format!(
" {warn}(requires Rust {package_rust_version}){warn:#}"
" {error}(requires Rust {package_rust_version}){error:#}"
))
}

Expand Down Expand Up @@ -801,20 +803,28 @@ struct PackageChange {
previous_id: Option<PackageId>,
kind: PackageChangeKind,
is_member: Option<bool>,
required_rust_version: Option<PartialVersion>,
}

impl PackageChange {
pub fn new(ws: &Workspace<'_>, resolve: &Resolve) -> Vec<Self> {
pub fn new(ws: &Workspace<'_>, resolve: &Resolve) -> IndexMap<PackageId, Self> {
let diff = PackageDiff::new(resolve);
Self::with_diff(diff, ws)
}

pub fn diff(ws: &Workspace<'_>, previous_resolve: &Resolve, resolve: &Resolve) -> Vec<Self> {
pub fn diff(
ws: &Workspace<'_>,
previous_resolve: &Resolve,
resolve: &Resolve,
) -> IndexMap<PackageId, Self> {
let diff = PackageDiff::diff(previous_resolve, resolve);
Self::with_diff(diff, ws)
}

fn with_diff(diff: impl Iterator<Item = PackageDiff>, ws: &Workspace<'_>) -> Vec<Self> {
fn with_diff(
diff: impl Iterator<Item = PackageDiff>,
ws: &Workspace<'_>,
) -> IndexMap<PackageId, Self> {
let member_ids: HashSet<_> = ws.members().map(|p| p.package_id()).collect();

let mut changes = IndexMap::new();
Expand All @@ -837,6 +847,7 @@ impl PackageChange {
previous_id: Some(previous_id),
kind,
is_member,
required_rust_version: None,
};
changes.insert(change.package_id, change);
} else {
Expand All @@ -848,6 +859,7 @@ impl PackageChange {
previous_id: None,
kind,
is_member,
required_rust_version: None,
};
changes.insert(change.package_id, change);
}
Expand All @@ -859,6 +871,7 @@ impl PackageChange {
previous_id: None,
kind,
is_member,
required_rust_version: None,
};
changes.insert(change.package_id, change);
}
Expand All @@ -871,12 +884,13 @@ impl PackageChange {
previous_id: None,
kind,
is_member,
required_rust_version: None,
};
changes.insert(change.package_id, change);
}
}

changes.into_values().collect()
changes
}

/// For querying [`PackageRegistry`] for alternative versions to report to the user
Expand Down Expand Up @@ -1066,3 +1080,46 @@ impl PackageDiff {
}
}
}

fn annotate_required_rust_version(
ws: &Workspace<'_>,
resolve: &Resolve,
changes: &mut IndexMap<PackageId, PackageChange>,
) {
let rustc = ws.gctx().load_global_rustc(Some(ws)).ok();
let rustc_version: Option<PartialVersion> =
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(&current_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)),
);
}
}
}
112 changes: 112 additions & 0 deletions tests/testsuite/rust_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1076,3 +1076,115 @@ 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-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-incompatible v1.2345.0 (requires Rust 1.2345.0)
[ADDING] dep-shared-incompatible v1.75.0 (requires Rust 1.75.0)
"#]])
.run();
}

0 comments on commit b8a4f1b

Please sign in to comment.