Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(resolve): Report incompatible packages with precise Rust version #14457

Merged
merged 4 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}