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 #14414
  • Loading branch information
epage committed Sep 19, 2024
1 parent 64abeb2 commit 94db932
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 31 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
4 changes: 3 additions & 1 deletion src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,9 @@ This was stabilized in 1.79 in [#13608](https://github.com/rust-lang/cargo/pull/
- `package.edition = "2024"` (only in workspace root)

The resolver will prefer dependencies with a `package.rust-version` that is the same or older than your project's MSRV.
Your project's MSRV is determined by taking the lowest `package.rust-version` set among your workspace members.
As the resolver is unable to determine which workspace members will eventually
depend on a package when it is being selected, we prioritize versions based on
how many workspace member MSRVs they are compatible with.
If there is no MSRV set then your toolchain version will be used, allowing it to pick up the toolchain version from pinned in rustup (e.g. `rust-toolchain.toml`).

#### `resolver.incompatible-rust-versions`
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/rust_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ higher v0.0.1 ([ROOT]/foo)
.with_stderr_data(str![[r#"
[UPDATING] `dummy-registry` index
[LOCKING] 6 packages to latest Rust 1.50.0 compatible versions
[ADDING] higher-newer-and-older v1.65.0 (requires Rust 1.65.0)
[ADDING] higher-newer-and-older v1.55.0 (available: v1.65.0, requires Rust 1.65.0)
[ADDING] higher-only-newer v1.65.0 (requires Rust 1.65.0)
[ADDING] lower-newer-and-older v1.45.0 (available: v1.55.0, requires Rust 1.55.0)
[ADDING] lower-only-newer v1.65.0 (requires Rust 1.65.0)
Expand All @@ -522,7 +522,7 @@ higher v0.0.1 ([ROOT]/foo)
p.cargo("tree")
.with_stdout_data(str![[r#"
higher v0.0.1 ([ROOT]/foo)
├── higher-newer-and-older v1.65.0
├── higher-newer-and-older v1.55.0
├── higher-only-newer v1.65.0
├── shared-newer-and-older v1.45.0
└── shared-only-newer v1.65.0
Expand Down

0 comments on commit 94db932

Please sign in to comment.