Skip to content

Commit 5eae31f

Browse files
konstinkonstin
committed
Use Ranges::from_iter over Ranges::iter_mut
Use astral-sh/pubgrub#34 (pubgrub-rs/pubgrub#273) for safer ranges modification that can't break pubgrub invariants. In the process, I removed the `&mut` references in favor of a direct immutable-to-immutable mapping.
1 parent eec586a commit 5eae31f

File tree

3 files changed

+106
-87
lines changed

3 files changed

+106
-87
lines changed

‎Cargo.lock‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,8 @@ petgraph = { version = "0.6.5" }
128128
platform-info = { version = "2.0.3" }
129129
procfs = { version = "0.17.0" , default-features = false, features = ["flate2"] }
130130
proc-macro2 = { version = "1.0.86" }
131-
pubgrub = { git = "https://github.com/astral-sh/pubgrub", branch = "charlie/mut" }
132-
version-ranges = { git = "https://github.com/astral-sh/pubgrub", branch = "charlie/mut" }
131+
pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "30502ceb17c033be408d9766a32ebc211518033f" }
132+
version-ranges = { git = "https://github.com/astral-sh/pubgrub", rev = "30502ceb17c033be408d9766a32ebc211518033f" }
133133
quote = { version = "1.0.37" }
134134
rayon = { version = "1.10.0" }
135135
reflink-copy = { version = "0.1.19" }

‎crates/uv-resolver/src/error.rs‎

Lines changed: 102 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -230,81 +230,94 @@ impl NoSolutionError {
230230
/// implement PEP 440 semantics for local version equality. For example, `1.0.0+foo` needs to
231231
/// satisfy `==1.0.0`.
232232
pub(crate) fn collapse_local_version_segments(derivation_tree: ErrorTree) -> ErrorTree {
233-
/// Remove local versions sentinels (`+[max]`) from the given version ranges.
234-
fn strip_sentinel(versions: &mut Ranges<Version>) {
235-
versions.iter_mut().for_each(|(lower, upper)| {
236-
match (&lower, &upper) {
237-
(Bound::Unbounded, Bound::Unbounded) => {}
238-
(Bound::Unbounded, Bound::Included(v)) => {
239-
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
240-
if v.local() == LocalVersionSlice::Sentinel {
241-
*upper = Bound::Included(v.clone().without_local());
242-
}
233+
/// Remove local versions sentinels (`+[max]`) from the interval.
234+
fn strip_sentinel(
235+
mut lower: Bound<Version>,
236+
mut upper: Bound<Version>,
237+
) -> (Bound<Version>, Bound<Version>) {
238+
match (&lower, &upper) {
239+
(Bound::Unbounded, Bound::Unbounded) => {}
240+
(Bound::Unbounded, Bound::Included(v)) => {
241+
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
242+
if v.local() == LocalVersionSlice::Sentinel {
243+
upper = Bound::Included(v.clone().without_local());
243244
}
244-
(Bound::Unbounded, Bound::Excluded(v)) => {
245-
// `<1.0.0+[max]` is equivalent to `<1.0.0`
246-
if v.local() == LocalVersionSlice::Sentinel {
247-
*upper = Bound::Excluded(v.clone().without_local());
248-
}
245+
}
246+
(Bound::Unbounded, Bound::Excluded(v)) => {
247+
// `<1.0.0+[max]` is equivalent to `<1.0.0`
248+
if v.local() == LocalVersionSlice::Sentinel {
249+
upper = Bound::Excluded(v.clone().without_local());
249250
}
250-
(Bound::Included(v), Bound::Unbounded) => {
251-
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
252-
if v.local() == LocalVersionSlice::Sentinel {
253-
*lower = Bound::Excluded(v.clone().without_local());
254-
}
251+
}
252+
(Bound::Included(v), Bound::Unbounded) => {
253+
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
254+
if v.local() == LocalVersionSlice::Sentinel {
255+
lower = Bound::Excluded(v.clone().without_local());
255256
}
256-
(Bound::Included(v), Bound::Included(b)) => {
257-
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
258-
if v.local() == LocalVersionSlice::Sentinel {
259-
*lower = Bound::Excluded(v.clone().without_local());
260-
}
261-
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
262-
if b.local() == LocalVersionSlice::Sentinel {
263-
*upper = Bound::Included(b.clone().without_local());
264-
}
257+
}
258+
(Bound::Included(v), Bound::Included(b)) => {
259+
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
260+
if v.local() == LocalVersionSlice::Sentinel {
261+
lower = Bound::Excluded(v.clone().without_local());
265262
}
266-
(Bound::Included(v), Bound::Excluded(b)) => {
267-
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
268-
if v.local() == LocalVersionSlice::Sentinel {
269-
*lower = Bound::Excluded(v.clone().without_local());
270-
}
271-
// `<1.0.0+[max]` is equivalent to `<1.0.0`
272-
if b.local() == LocalVersionSlice::Sentinel {
273-
*upper = Bound::Included(b.clone().without_local());
274-
}
263+
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
264+
if b.local() == LocalVersionSlice::Sentinel {
265+
upper = Bound::Included(b.clone().without_local());
275266
}
276-
(Bound::Excluded(v), Bound::Unbounded) => {
277-
// `>1.0.0+[max]` is equivalent to `>1.0.0`
278-
if v.local() == LocalVersionSlice::Sentinel {
279-
*lower = Bound::Excluded(v.clone().without_local());
280-
}
267+
}
268+
(Bound::Included(v), Bound::Excluded(b)) => {
269+
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
270+
if v.local() == LocalVersionSlice::Sentinel {
271+
lower = Bound::Excluded(v.clone().without_local());
281272
}
282-
(Bound::Excluded(v), Bound::Included(b)) => {
283-
// `>1.0.0+[max]` is equivalent to `>1.0.0`
284-
if v.local() == LocalVersionSlice::Sentinel {
285-
*lower = Bound::Excluded(v.clone().without_local());
286-
}
287-
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
288-
if b.local() == LocalVersionSlice::Sentinel {
289-
*upper = Bound::Included(b.clone().without_local());
290-
}
273+
// `<1.0.0+[max]` is equivalent to `<1.0.0`
274+
if b.local() == LocalVersionSlice::Sentinel {
275+
upper = Bound::Included(b.clone().without_local());
291276
}
292-
(Bound::Excluded(v), Bound::Excluded(b)) => {
293-
// `>1.0.0+[max]` is equivalent to `>1.0.0`
294-
if v.local() == LocalVersionSlice::Sentinel {
295-
*lower = Bound::Excluded(v.clone().without_local());
296-
}
297-
// `<1.0.0+[max]` is equivalent to `<1.0.0`
298-
if b.local() == LocalVersionSlice::Sentinel {
299-
*upper = Bound::Excluded(b.clone().without_local());
300-
}
277+
}
278+
(Bound::Excluded(v), Bound::Unbounded) => {
279+
// `>1.0.0+[max]` is equivalent to `>1.0.0`
280+
if v.local() == LocalVersionSlice::Sentinel {
281+
lower = Bound::Excluded(v.clone().without_local());
301282
}
302283
}
303-
});
284+
(Bound::Excluded(v), Bound::Included(b)) => {
285+
// `>1.0.0+[max]` is equivalent to `>1.0.0`
286+
if v.local() == LocalVersionSlice::Sentinel {
287+
lower = Bound::Excluded(v.clone().without_local());
288+
}
289+
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
290+
if b.local() == LocalVersionSlice::Sentinel {
291+
upper = Bound::Included(b.clone().without_local());
292+
}
293+
}
294+
(Bound::Excluded(v), Bound::Excluded(b)) => {
295+
// `>1.0.0+[max]` is equivalent to `>1.0.0`
296+
if v.local() == LocalVersionSlice::Sentinel {
297+
lower = Bound::Excluded(v.clone().without_local());
298+
}
299+
// `<1.0.0+[max]` is equivalent to `<1.0.0`
300+
if b.local() == LocalVersionSlice::Sentinel {
301+
upper = Bound::Excluded(b.clone().without_local());
302+
}
303+
}
304+
}
305+
(lower, upper)
306+
}
307+
308+
/// Remove local versions sentinels (`+[max]`) from the version ranges.
309+
// TODO(konsti): Add `impl IntoIterator for Ranges` without leaking the internal
310+
// smallvec to remove the cloning
311+
#[allow(clippy::needless_pass_by_value)]
312+
fn strip_sentinels(versions: Ranges<Version>) -> Ranges<Version> {
313+
versions
314+
.iter()
315+
.map(|(lower, upper)| strip_sentinel(lower.clone(), upper.clone()))
316+
.collect()
304317
}
305318

306319
/// Returns `true` if the range appears to be, e.g., `>1.0.0, <1.0.0+[max]`.
307-
fn is_sentinel(versions: &mut Ranges<Version>) -> bool {
320+
fn is_sentinel(versions: &Ranges<Version>) -> bool {
308321
versions.iter().all(|(lower, upper)| {
309322
let (Bound::Excluded(lower), Bound::Excluded(upper)) = (lower, upper) else {
310323
return false;
@@ -319,30 +332,36 @@ impl NoSolutionError {
319332
})
320333
}
321334

322-
fn collapse(mut derivation_tree: ErrorTree) -> Option<ErrorTree> {
335+
fn collapse(derivation_tree: ErrorTree) -> Option<ErrorTree> {
323336
match derivation_tree {
324337
DerivationTree::External(External::NotRoot(_, _)) => Some(derivation_tree),
325-
DerivationTree::External(External::NoVersions(_, ref mut versions)) => {
326-
if is_sentinel(versions) {
338+
DerivationTree::External(External::NoVersions(package, versions)) => {
339+
if is_sentinel(&versions) {
327340
return None;
328341
}
329342

330-
strip_sentinel(versions);
331-
Some(derivation_tree)
343+
let versions = strip_sentinels(versions);
344+
Some(DerivationTree::External(External::NoVersions(
345+
package, versions,
346+
)))
332347
}
333348
DerivationTree::External(External::FromDependencyOf(
334-
_,
335-
ref mut versions1,
336-
_,
337-
ref mut versions2,
349+
package1,
350+
versions1,
351+
package2,
352+
versions2,
338353
)) => {
339-
strip_sentinel(versions1);
340-
strip_sentinel(versions2);
341-
Some(derivation_tree)
354+
let versions1 = strip_sentinels(versions1);
355+
let versions2 = strip_sentinels(versions2);
356+
Some(DerivationTree::External(External::FromDependencyOf(
357+
package1, versions1, package2, versions2,
358+
)))
342359
}
343-
DerivationTree::External(External::Custom(_, ref mut versions, _)) => {
344-
strip_sentinel(versions);
345-
Some(derivation_tree)
360+
DerivationTree::External(External::Custom(package, versions, reason)) => {
361+
let versions = strip_sentinels(versions);
362+
Some(DerivationTree::External(External::Custom(
363+
package, versions, reason,
364+
)))
346365
}
347366
DerivationTree::Derived(mut derived) => {
348367
let cause1 = collapse((*derived.cause1).clone());
@@ -353,15 +372,15 @@ impl NoSolutionError {
353372
cause2: Arc::new(cause2),
354373
terms: std::mem::take(&mut derived.terms)
355374
.into_iter()
356-
.map(|(pkg, mut term)| {
357-
match &mut term {
375+
.map(|(pkg, term)| {
376+
let term = match term {
358377
Term::Positive(versions) => {
359-
strip_sentinel(versions);
378+
Term::Positive(strip_sentinels(versions))
360379
}
361380
Term::Negative(versions) => {
362-
strip_sentinel(versions);
381+
Term::Negative(strip_sentinels(versions))
363382
}
364-
}
383+
};
365384
(pkg, term)
366385
})
367386
.collect(),

0 commit comments

Comments
 (0)