Skip to content

Commit

Permalink
fix(resolve): Improve multi-MSRV workspaces
Browse files Browse the repository at this point in the history
We do this by resolving for a package version that is compatible
with the most number of MSRVs within a workspace.

If a version requirement is just right, every package will get the
highest MSRV-compatible dependency.

If its too high, packages will get MSRV-incompatible dependency
versions.
This will happen regardless of what we do due to the nature of
`"fallback"`.

If its too low, packages with higher MSRVs will get older-than-necessary
dependency versions.
This is similar to the "some with and without MSRV" workspaces.
When locking dependencies, we do report to users when newer MSRV/SemVer
compatible dependencies are available to help guide them to upgrading if
desired.

Fixes rust-lang#14414
  • Loading branch information
epage committed Sep 19, 2024
1 parent 5ff04c3 commit 7d8007c
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 30 deletions.
44 changes: 23 additions & 21 deletions src/cargo/core/resolver/version_prefs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub struct VersionPreferences {
try_to_use: HashSet<PackageId>,
prefer_patch_deps: HashMap<InternedString, HashSet<Dependency>>,
version_ordering: VersionOrdering,
max_rust_version: Option<PartialVersion>,
rust_versions: Vec<PartialVersion>,
}

#[derive(Copy, Clone, Default, PartialEq, Eq, Hash, Debug)]
Expand Down Expand Up @@ -49,8 +49,8 @@ impl VersionPreferences {
self.version_ordering = ordering;
}

pub fn max_rust_version(&mut self, ver: Option<PartialVersion>) {
self.max_rust_version = ver;
pub fn rust_versions(&mut self, vers: Vec<PartialVersion>) {
self.rust_versions = vers;
}

/// Sort (and filter) the given vector of summaries in-place
Expand All @@ -59,7 +59,7 @@ impl VersionPreferences {
///
/// Sort order:
/// 1. Preferred packages
/// 2. [`VersionPreferences::max_rust_version`]
/// 2. Most compatible [`VersionPreferences::rust_versions`]
/// 3. `first_version`, falling back to [`VersionPreferences::version_ordering`] when `None`
///
/// Filtering:
Expand All @@ -85,20 +85,11 @@ impl VersionPreferences {
return previous_cmp;
}

if let Some(max_rust_version) = &self.max_rust_version {
let a_is_compat = a
.rust_version()
.map(|a| a.is_compatible_with(max_rust_version))
.unwrap_or(true);
let b_is_compat = b
.rust_version()
.map(|b| b.is_compatible_with(max_rust_version))
.unwrap_or(true);
match (a_is_compat, b_is_compat) {
(true, true) => {} // fallback
(false, false) => {} // fallback
(true, false) => return Ordering::Less,
(false, true) => return Ordering::Greater,
if !self.rust_versions.is_empty() {
let a_compat_count = self.msrv_compat_count(a);
let b_compat_count = self.msrv_compat_count(b);
if b_compat_count != a_compat_count {
return b_compat_count.cmp(&a_compat_count);
}
}

Expand All @@ -112,6 +103,17 @@ impl VersionPreferences {
let _ = summaries.split_off(1);
}
}

fn msrv_compat_count(&self, summary: &Summary) -> usize {
let Some(rust_version) = summary.rust_version() else {
return self.rust_versions.len();
};

self.rust_versions
.iter()
.filter(|max| rust_version.is_compatible_with(max))
.count()
}
}

#[cfg(test)]
Expand Down Expand Up @@ -238,7 +240,7 @@ mod test {
#[test]
fn test_single_rust_version() {
let mut vp = VersionPreferences::default();
vp.max_rust_version(Some("1.50".parse().unwrap()));
vp.rust_versions(vec!["1.50".parse().unwrap()]);

let mut summaries = vec![
summ("foo", "1.2.4", None),
Expand Down Expand Up @@ -270,7 +272,7 @@ mod test {
#[test]
fn test_multiple_rust_versions() {
let mut vp = VersionPreferences::default();
vp.max_rust_version(Some("1.45".parse().unwrap()));
vp.rust_versions(vec!["1.45".parse().unwrap(), "1.55".parse().unwrap()]);

let mut summaries = vec![
summ("foo", "1.2.4", None),
Expand All @@ -286,7 +288,7 @@ mod test {
vp.sort_summaries(&mut summaries, None);
assert_eq!(
describe(&summaries),
"foo/1.2.4, foo/1.2.2, foo/1.2.0, foo/1.1.0, foo/1.0.9, foo/1.2.3, foo/1.2.1"
"foo/1.2.4, foo/1.2.2, foo/1.2.0, foo/1.1.0, foo/1.0.9, foo/1.2.1, foo/1.2.3"
.to_string()
);

Expand Down
17 changes: 10 additions & 7 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ use crate::util::errors::CargoResult;
use crate::util::CanonicalUrl;
use anyhow::Context as _;
use cargo_util::paths;
use cargo_util_schemas::core::PartialVersion;
use std::collections::{HashMap, HashSet};
use tracing::{debug, trace};

Expand Down Expand Up @@ -357,14 +358,16 @@ pub fn resolve_with_previous<'gctx>(
version_prefs.version_ordering(VersionOrdering::MinimumVersionsFirst)
}
if ws.resolve_honors_rust_version() {
let rust_version = if let Some(ver) = ws.lowest_rust_version() {
ver.clone().into_partial()
} else {
let mut rust_versions: Vec<_> = ws
.members()
.filter_map(|p| p.rust_version().map(|rv| rv.as_partial().clone()))
.collect();
if rust_versions.is_empty() {
let rustc = ws.gctx().load_global_rustc(Some(ws))?;
let rustc_version = rustc.version.clone().into();
rustc_version
};
version_prefs.max_rust_version(Some(rust_version));
let rust_version: PartialVersion = rustc.version.clone().into();
rust_versions.push(rust_version);
}
version_prefs.rust_versions(rust_versions);
}

let avoid_patch_ids = if register_patches {
Expand Down