From 8bdc2a0db692fea8de3adc593809d13a61fb9094 Mon Sep 17 00:00:00 2001 From: Jack Marsh Date: Mon, 24 Aug 2026 11:11:29 +0100 Subject: [PATCH] Relax MSRV per requirement, not just per bucket Resolving rust-corpus for rustc 1.70 failed with Error: no release of thiserror satisfies ^2.0.20 which is not true. 2.0.20 exists. MSRV filtering removed it because it needs rustc 1.71, and the error said nothing about rustc, so a reader goes looking for a yanked release when the answer is that their compiler is one version too old. The existing relaxation is per bucket: it fires only when filtering empties a bucket. thiserror 2.x keeps eighteen releases on 1.70 and loses every one that satisfies ^2.0.20, so nothing relaxed. A requirement that no supported release satisfies now takes the newest release that does, and says which crate, which requirement, which version and which rustc it needs. That is the same choice the whole-bucket case already makes, applied where the requirement rather than the bucket is what ran out. The range is built from filtered candidates, so both halves are needed: bucket_range has to admit the relaxed version, and choose_version has to be able to return it. At rustc 1.70 the corpus now relaxes 140 crates rather than stopping at the first, and gets far enough to hit a real conflict: actix-http 3.13.3 accepts tokio-util up to 0.7.16 while the declarations require 0.7.19. That one is a genuine incompatibility and is reported as one. Nothing changes at a current toolchain. Both repos resolve with no warnings and no movement. --- README.md | 4 + tools/please_rust/src/pubgrub_solver.rs | 133 ++++++++++++++++++++++-- 2 files changed, 126 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index f4e2ebf..5aac7fb 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,10 @@ different one. Declaring the toolchain in a different package from the crates is the ordinary case, and on rustc 1.74 `clap` reaches 4.5.61 rather than 4.6.6, which needs 1.85. +Filtering never turns a solvable graph into an error. When nothing that +builds on your toolchain satisfies a requirement, the newest release that +does satisfy it is taken and named, along with the rustc it needs. + Declarations are shared by everyone working in the repo, so `lock` solves for every platform in `--targets` (linux x86_64 and both darwin arches by default) and declares the union. A linux developer adding `chrono` declares diff --git a/tools/please_rust/src/pubgrub_solver.rs b/tools/please_rust/src/pubgrub_solver.rs index d3fc328..87bd91a 100644 --- a/tools/please_rust/src/pubgrub_solver.rs +++ b/tools/please_rust/src/pubgrub_solver.rs @@ -206,6 +206,18 @@ impl<'a, S: ReleaseSource> Solver<'a, S> { /// skip yanked (unless pinned), then apply MSRV, relaxing it only if that /// would leave nothing to choose. fn candidates(&self, name: &str, bucket: Bucket) -> Result> { + Ok(self.candidates_split(name, bucket)?.0) + } + + /// The same, but keeping what MSRV filtering removed. + /// + /// Whether a requirement is satisfiable depends on the requirement, and + /// this only knows the bucket. A bucket can keep plenty of releases and + /// still lose every one that a particular dependent asked for: thiserror + /// 2.x has eighteen releases that build on rustc 1.70 and none of them + /// satisfies ^2.0.20, because 2.0.20 needs 1.71. The caller has the + /// requirement, so it gets both halves and decides. + fn candidates_split(&self, name: &str, bucket: Bucket) -> Result<(Vec, Vec)> { let pinned = self.pinned.get(name); let mut in_bucket: Vec = self .releases(name)? @@ -236,11 +248,16 @@ impl<'a, S: ReleaseSource> Solver<'a, S> { name, bucket, toolchain ); } - return Ok(in_bucket); + return Ok((in_bucket, Vec::new())); } - return Ok(ok); + let excluded: Vec = in_bucket + .iter() + .filter(|r| !ok.iter().any(|k| k.version == r.version)) + .cloned() + .collect(); + return Ok((ok, excluded)); } - Ok(in_bucket) + Ok((in_bucket, Vec::new())) } /// Buckets a requirement can be satisfied by, newest bucket first. @@ -305,12 +322,42 @@ impl<'a, S: ReleaseSource> Solver<'a, S> { bucket: Bucket, req: &VersionReq, ) -> Result> { + let (ok, excluded) = self.candidates_split(package, bucket)?; let mut range = Ranges::empty(); - for r in self.candidates(package, bucket)? { + for r in &ok { if req.matches(&r.version) { - range = range.union(&Ranges::singleton(r.version)); + range = range.union(&Ranges::singleton(r.version.clone())); } } + if !range.is_empty() { + return Ok(range); + } + // A bucket can keep most of its releases and still lose every one + // this requirement asked for. Refusing to resolve is worse than + // declaring a crate that needs a newer rustc, which is what the + // whole-bucket case already chooses; the difference is that leaving + // the range empty produces an error saying no release satisfies a + // requirement that several releases do satisfy. + if let Some(r) = excluded.iter().find(|r| req.matches(&r.version)) { + if self.relaxed.borrow_mut().insert(package.to_string()) { + eprintln!( + "warning: nothing that satisfies {} {} builds on rustc {}; taking {}, \ + which needs rustc {}", + package, + req, + self.msrv + .as_ref() + .map(|v| v.to_string()) + .unwrap_or_default(), + r.version, + r.rust_version + .as_ref() + .map(|v| v.to_string()) + .unwrap_or_else(|| "an unstated version".to_string()), + ); + } + return Ok(Ranges::singleton(r.version.clone())); + } Ok(range) } @@ -436,8 +483,8 @@ impl<'a, S: ReleaseSource> DependencyProvider for Solver<'a, S> { .find(|v| range.contains(v))) } Pkg::Crate { name, bucket } => { - let candidates = self - .candidates(name, *bucket) + let (candidates, excluded) = self + .candidates_split(name, *bucket) .map_err(|e| SolverError(e.to_string()))?; // An already-declared version wins when it is still in range, // so `lock --add` does not churn unrelated crates. @@ -450,10 +497,40 @@ impl<'a, S: ReleaseSource> DependencyProvider for Solver<'a, S> { return Ok(Some(p.clone())); } } - Ok(candidates - .into_iter() - .map(|r| r.version) - .find(|v| range.contains(v))) + if let Some(v) = candidates + .iter() + .map(|r| r.version.clone()) + .find(|v| range.contains(v)) + { + return Ok(Some(v)); + } + // Nothing this toolchain can build satisfies the requirement, + // but something does. Refusing to resolve is worse than + // declaring a crate that may need a newer rustc, which is the + // same choice the whole-bucket case above already makes. Said + // out loud, because the alternative is an error claiming no + // release satisfies a requirement that several do. + if let Some(r) = excluded.iter().find(|r| range.contains(&r.version)) { + if self.relaxed.borrow_mut().insert(name.to_string()) { + eprintln!( + "warning: no release of {} that supports rustc {} satisfies what \ + depends on it; taking {} {}, which needs rustc {}", + name, + self.msrv + .as_ref() + .map(|v| v.to_string()) + .unwrap_or_default(), + name, + r.version, + r.rust_version + .as_ref() + .map(|v| v.to_string()) + .unwrap_or_else(|| "an unstated version".to_string()), + ); + } + return Ok(Some(r.version.clone())); + } + Ok(None) } } } @@ -918,6 +995,40 @@ mod tests { assert_eq!(got["hard"], Version::parse("1.0.0").unwrap()); } + /// A bucket can keep most of its releases and still lose every one a + /// particular requirement asked for. Found in rust-corpus at rustc 1.70: + /// thiserror 2.x has eighteen releases that build on 1.70 and none of + /// them satisfies ^2.0.20, because 2.0.20 needs 1.71. Resolution failed + /// with "no release of thiserror satisfies ^2.0.20", which is not true. + #[test] + fn msrv_relaxes_for_a_requirement_no_supported_release_satisfies() { + let mut b = idx(); + // Plenty of the bucket builds on the toolchain, so the whole-bucket + // relaxation never fires. + b.add("t", "2.0.0", &[]); + b.add("t", "2.0.5", &[]); + b.add("t", "2.0.17", &[]); + // Only this one satisfies ^2.0.20, and it needs a newer rustc. + b.msrv_release("t", "2.0.20", "1.71.0"); + let f = b.build(); + + let got = solve_with(&f, &[("t", "^2.0.20")], &[], Some("1.70.0")).unwrap(); + assert_eq!( + got["t"], + Version::parse("2.0.20").unwrap(), + "the only release satisfying the requirement should be taken, with a warning" + ); + + // A requirement the toolchain can satisfy is unaffected: still the + // newest release that builds, not the newest release. + let got = solve_with(&f, &[("t", "^2")], &[], Some("1.70.0")).unwrap(); + assert_eq!(got["t"], Version::parse("2.0.17").unwrap()); + + // And with no MSRV at all, the newest wins as before. + let got = solve_with(&f, &[("t", "^2")], &[], None).unwrap(); + assert_eq!(got["t"], Version::parse("2.0.20").unwrap()); + } + #[test] fn msrv_off_by_default_takes_the_newest() { let mut b = idx();