From 7767ef2af546b577c096bff90351a5871a8f2f94 Mon Sep 17 00:00:00 2001 From: konsti Date: Fri, 20 Dec 2024 16:58:07 +0100 Subject: [PATCH 1/2] Allow `Ranges::contains` to accept (e.g.) `&str` for `Ranges` (#35) (#301) ## Summary This PR borrows a trick from [HashMap](https://doc.rust-lang.org/std/collections/struct.HashMap.html#method.contains_key) to enable users to pass (e.g.) `&str` to `Ranges::contains`, given `Ranges`. Co-authored-by: Charlie Marsh --- version-ranges/src/lib.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/version-ranges/src/lib.rs b/version-ranges/src/lib.rs index 8b19881b..e8eadb9f 100644 --- a/version-ranges/src/lib.rs +++ b/version-ranges/src/lib.rs @@ -229,7 +229,11 @@ impl Ranges { } /// Returns true if self contains the specified value. - pub fn contains(&self, version: &V) -> bool { + pub fn contains(&self, version: &Q) -> bool + where + V: Borrow, + Q: ?Sized + PartialOrd, + { self.segments .binary_search_by(|segment| { // We have to reverse because we need the segment wrt to the version, while @@ -470,10 +474,14 @@ impl Ord for Ranges { /// ^ ^ ^ /// less equal greater /// ``` -fn within_bounds(version: &V, segment: &Interval) -> Ordering { +fn within_bounds(version: &Q, segment: &Interval) -> Ordering +where + V: Borrow, + Q: ?Sized + PartialOrd, +{ let below_lower_bound = match segment { - (Excluded(start), _) => version <= start, - (Included(start), _) => version < start, + (Excluded(start), _) => version <= start.borrow(), + (Included(start), _) => version < start.borrow(), (Unbounded, _) => false, }; if below_lower_bound { @@ -481,8 +489,8 @@ fn within_bounds(version: &V, segment: &Interval) -> Ordering } let below_upper_bound = match segment { (_, Unbounded) => true, - (_, Included(end)) => version <= end, - (_, Excluded(end)) => version < end, + (_, Included(end)) => version <= end.borrow(), + (_, Excluded(end)) => version < end.borrow(), }; if below_upper_bound { return Ordering::Equal; @@ -1304,7 +1312,7 @@ pub mod tests { #[test] fn always_contains_exact(version in version_strat()) { - assert!(Ranges::singleton(version).contains(&version)); + assert!(Ranges::::singleton(version).contains(&version)); } #[test] @@ -1326,7 +1334,7 @@ pub mod tests { #[test] fn from_range_bounds(range in any::<(Bound, Bound)>(), version in version_strat()) { - let rv: Ranges<_> = Ranges::from_range_bounds(range); + let rv: Ranges<_> = Ranges::::from_range_bounds(range); assert_eq!(range.contains(&version), rv.contains(&version)); } From 10cb803f1e2028947b67983459ae364db7032fc7 Mon Sep 17 00:00:00 2001 From: konsti Date: Fri, 20 Dec 2024 17:01:28 +0100 Subject: [PATCH 2/2] Add `State::add_incompatibility_from_dependencies` (#27) (#300) This wrapper avoids accessing the `incompatibility_store` directly in uv code. Before: ```rust let dep_incompats = self.pubgrub.add_version( package.clone(), version.clone(), dependencies, ); self.pubgrub.partial_solution.add_version( package.clone(), version.clone(), dep_incompats, &self.pubgrub.incompatibility_store, ); ``` After: ```rust self.pubgrub.add_incompatibility_from_dependencies(package.clone(), version.clone(), dependencies); ``` `add_incompatibility_from_dependencies` is one of the main methods for the custom interface to pubgrub. --- src/internal/core.rs | 21 +++++++++++++++++++-- src/internal/partial_solution.rs | 15 +++++++++------ src/solver.rs | 12 +++--------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/internal/core.rs b/src/internal/core.rs index 4dc5d447..1b82c1af 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -7,8 +7,8 @@ use std::collections::HashSet as Set; use std::sync::Arc; use crate::internal::{ - Arena, DecisionLevel, HashArena, Id, IncompDpId, Incompatibility, PartialSolution, Relation, - SatisfierSearch, SmallVec, + Arena, DecisionLevel, HashArena, Id, IncompDpId, IncompId, Incompatibility, PartialSolution, + Relation, SatisfierSearch, SmallVec, }; use crate::{DependencyProvider, DerivationTree, Map, NoSolutionError, VersionSet}; @@ -73,6 +73,23 @@ impl State { } } + /// Add the dependencies for the current version of the current package as incompatibilities. + pub(crate) fn add_package_version_dependencies( + &mut self, + package: Id, + version: DP::V, + dependencies: impl IntoIterator, + ) -> Option> { + let dep_incompats = + self.add_incompatibility_from_dependencies(package, version.clone(), dependencies); + self.partial_solution.add_package_version_incompatibilities( + package, + version.clone(), + dep_incompats, + &self.incompatibility_store, + ) + } + /// Add an incompatibility to the state. pub(crate) fn add_incompatibility(&mut self, incompat: Incompatibility) { let id = self.incompatibility_store.alloc(incompat); diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index f1345634..2ae6cae8 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -417,12 +417,15 @@ impl PartialSolution { self.has_ever_backtracked = true; } - /// We can add the version to the partial solution as a decision - /// if it doesn't produce any conflict with the new incompatibilities. - /// In practice I think it can only produce a conflict if one of the dependencies - /// (which are used to make the new incompatibilities) - /// is already in the partial solution with an incompatible version. - pub(crate) fn add_version( + /// Add a package version as decision if none of its dependencies conflicts with the partial + /// solution. + /// + /// If the resolution never backtracked before, a fast path adds the package version directly + /// without checking dependencies. + /// + /// Returns the incompatibility that caused the current version to be rejected, if a conflict + /// in the dependencies was found. + pub(crate) fn add_package_version_incompatibilities( &mut self, package: Id, version: DP::V, diff --git a/src/solver.rs b/src/solver.rs index 7c4ed0f8..df667df0 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -228,15 +228,9 @@ pub fn resolve( }; // Add that package and version if the dependencies are not problematic. - let dep_incompats = - state.add_incompatibility_from_dependencies(p, v.clone(), dependencies); - - if let Some(conflict) = state.partial_solution.add_version( - p, - v, - dep_incompats, - &state.incompatibility_store, - ) { + if let Some(conflict) = + state.add_package_version_dependencies(p, v.clone(), dependencies) + { conflict_tracker.entry(p).or_default().dependencies_affected += 1; for (incompat_package, _) in state.incompatibility_store[conflict].iter() { if incompat_package == p {