From 5fe23780e221808a88c46446d0b23668df3fa616 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Fri, 12 Jan 2024 22:25:50 +0000 Subject: [PATCH 1/2] feat: ues u32 insted of NumberVersion --- benches/large_case.rs | 6 +-- examples/caching_dependency_provider.rs | 7 ++- examples/doc_interface.rs | 13 +++-- src/lib.rs | 22 ++++---- src/solver.rs | 5 +- src/version.rs | 70 +------------------------ tests/examples.rs | 30 +++++------ tests/proptest.rs | 31 +++++------ tests/tests.rs | 27 +++++----- 9 files changed, 66 insertions(+), 145 deletions(-) diff --git a/benches/large_case.rs b/benches/large_case.rs index 8efdaa9e..368bc93d 100644 --- a/benches/large_case.rs +++ b/benches/large_case.rs @@ -7,7 +7,7 @@ use self::criterion::*; use pubgrub::package::Package; use pubgrub::range::Range; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::{NumberVersion, SemanticVersion}; +use pubgrub::version::SemanticVersion; use pubgrub::version_set::VersionSet; use serde::de::Deserialize; @@ -36,9 +36,9 @@ fn bench_nested(c: &mut Criterion) { let case = case.unwrap().path(); let name = case.file_name().unwrap().to_string_lossy(); let data = std::fs::read_to_string(&case).unwrap(); - if name.ends_with("u16_NumberVersion.ron") { + if name.ends_with("u16_NumberVersion.ron") || name.ends_with("u16_u32.ron") { group.bench_function(name, |b| { - bench::>(b, &data); + bench::>(b, &data); }); } else if name.ends_with("str_SemanticVersion.ron") { group.bench_function(name, |b| { diff --git a/examples/caching_dependency_provider.rs b/examples/caching_dependency_provider.rs index c4d82e20..6a1960f4 100644 --- a/examples/caching_dependency_provider.rs +++ b/examples/caching_dependency_provider.rs @@ -5,10 +5,9 @@ use std::cell::RefCell; use pubgrub::package::Package; use pubgrub::range::Range; use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider}; -use pubgrub::version::NumberVersion; use pubgrub::version_set::VersionSet; -type NumVS = Range; +type NumVS = Range; // An example implementing caching dependency provider that will // store queried dependencies in memory and check them before querying more from remote. @@ -77,11 +76,11 @@ fn main() { let mut remote_dependencies_provider = OfflineDependencyProvider::<&str, NumVS>::new(); // Add dependencies as needed. Here only root package is added. - remote_dependencies_provider.add_dependencies("root", 1, Vec::new()); + remote_dependencies_provider.add_dependencies("root", 1u32, Vec::new()); let caching_dependencies_provider = CachingDependencyProvider::new(remote_dependencies_provider); - let solution = resolve(&caching_dependencies_provider, "root", 1); + let solution = resolve(&caching_dependencies_provider, "root", 1u32); println!("Solution: {:?}", solution); } diff --git a/examples/doc_interface.rs b/examples/doc_interface.rs index ca6bcb93..d270106c 100644 --- a/examples/doc_interface.rs +++ b/examples/doc_interface.rs @@ -2,9 +2,8 @@ use pubgrub::range::Range; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::NumberVersion; -type NumVS = Range; +type NumVS = Range; // `root` depends on `menu` and `icons` // `menu` depends on `dropdown` @@ -14,13 +13,13 @@ type NumVS = Range; fn main() { let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); dependency_provider.add_dependencies( - "root", 1, [("menu", Range::full()), ("icons", Range::full())], + "root", 1u32, [("menu", Range::full()), ("icons", Range::full())], ); - dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]); - dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]); - dependency_provider.add_dependencies("icons", 1, []); + dependency_provider.add_dependencies("menu", 1u32, [("dropdown", Range::full())]); + dependency_provider.add_dependencies("dropdown", 1u32, [("icons", Range::full())]); + dependency_provider.add_dependencies("icons", 1u32, []); // Run the algorithm. - let solution = resolve(&dependency_provider, "root", 1); + let solution = resolve(&dependency_provider, "root", 1u32); println!("Solution: {:?}", solution); } diff --git a/src/lib.rs b/src/lib.rs index 11c20e80..4b3d499d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,6 +19,7 @@ //! [Clone] + [Eq] + [Hash] + [Debug] + [Display](std::fmt::Display). //! So things like [String] will work out of the box. //! +//! TODO! This is all wrong. Need to talk about VS, not Version. //! Our [Version](version::Version) trait requires //! [Clone] + [Ord] + [Debug] + [Display](std::fmt::Display) //! and also the definition of two methods, @@ -27,8 +28,7 @@ //! strictly higher than the current one. //! For convenience, this library already provides //! two implementations of [Version](version::Version). -//! The first one is [NumberVersion](version::NumberVersion), basically a newtype for [u32]. -//! The second one is [SemanticVersion](version::NumberVersion) +//! The second one is [SemanticVersion](version::SemanticVersion) //! that implements semantic versioning rules. //! //! # Basic example @@ -47,22 +47,21 @@ //! We can model that scenario with this library as follows //! ``` //! # use pubgrub::solver::{OfflineDependencyProvider, resolve}; -//! # use pubgrub::version::NumberVersion; //! # use pubgrub::range::Range; //! -//! type NumVS = Range; +//! type NumVS = Range; //! //! let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); //! //! dependency_provider.add_dependencies( -//! "root", 1, [("menu", Range::full()), ("icons", Range::full())], +//! "root", 1u32, [("menu", Range::full()), ("icons", Range::full())], //! ); -//! dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]); -//! dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]); -//! dependency_provider.add_dependencies("icons", 1, []); +//! dependency_provider.add_dependencies("menu", 1u32, [("dropdown", Range::full())]); +//! dependency_provider.add_dependencies("dropdown", 1u32, [("icons", Range::full())]); +//! dependency_provider.add_dependencies("icons", 1u32, []); //! //! // Run the algorithm. -//! let solution = resolve(&dependency_provider, "root", 1).unwrap(); +//! let solution = resolve(&dependency_provider, "root", 1u32).unwrap(); //! ``` //! //! # DependencyProvider trait @@ -183,14 +182,13 @@ //! # use pubgrub::solver::{resolve, OfflineDependencyProvider}; //! # use pubgrub::report::{DefaultStringReporter, Reporter}; //! # use pubgrub::error::PubGrubError; -//! # use pubgrub::version::NumberVersion; //! # use pubgrub::range::Range; //! # -//! # type NumVS = Range; +//! # type NumVS = Range; //! # //! # let dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); //! # let root_package = "root"; -//! # let root_version = 1; +//! # let root_version = 1u32; //! # //! match resolve(&dependency_provider, root_package, root_version) { //! Ok(solution) => println!("{:?}", solution), diff --git a/src/solver.rs b/src/solver.rs index 9ac2edfe..cbc3f074 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -43,16 +43,15 @@ //! ``` //! # use std::convert::Infallible; //! # use pubgrub::solver::{resolve, OfflineDependencyProvider}; -//! # use pubgrub::version::NumberVersion; //! # use pubgrub::error::PubGrubError; //! # use pubgrub::range::Range; //! # -//! # type NumVS = Range; +//! # type NumVS = Range; //! # //! # fn try_main() -> Result<(), PubGrubError<&'static str, NumVS, Infallible>> { //! # let dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); //! # let package = "root"; -//! # let version = 1; +//! # let version = 1u32; //! let solution = resolve(&dependency_provider, package, version)?; //! # Ok(()) //! # } diff --git a/src/version.rs b/src/version.rs index 6f67c7de..5c8a4145 100644 --- a/src/version.rs +++ b/src/version.rs @@ -6,15 +6,6 @@ use std::fmt::{self, Debug, Display}; use std::str::FromStr; use thiserror::Error; -/// Versions have a minimal version (a "0" version) -/// and are ordered such that every version has a next one. -pub trait Version: Clone + Ord + Debug + Display { - /// Returns the lowest version. - fn lowest() -> Self; - /// Returns the next version, the smallest strictly higher version. - fn bump(&self) -> Self; -} - /// Type for semantic versions: major.minor.patch. #[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] pub struct SemanticVersion { @@ -227,63 +218,4 @@ impl Display for SemanticVersion { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}.{}.{}", self.major, self.minor, self.patch) } -} - -// Implement Version for SemanticVersion. -impl Version for SemanticVersion { - fn lowest() -> Self { - Self::zero() - } - fn bump(&self) -> Self { - self.bump_patch() - } -} - -/// Simplest versions possible, just a positive number. -#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize,))] -#[cfg_attr(feature = "serde", serde(transparent))] -pub struct NumberVersion(pub u32); - -// Convert an usize into a version. -impl From for NumberVersion { - fn from(v: u32) -> Self { - Self(v) - } -} - -// Convert an &usize into a version. -impl From<&u32> for NumberVersion { - fn from(v: &u32) -> Self { - Self(*v) - } -} - -// Convert an &version into a version. -impl From<&NumberVersion> for NumberVersion { - fn from(v: &NumberVersion) -> Self { - *v - } -} - -// Convert a version into an usize. -impl From for u32 { - fn from(version: NumberVersion) -> Self { - version.0 - } -} - -impl Display for NumberVersion { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } -} - -impl Version for NumberVersion { - fn lowest() -> Self { - Self(0) - } - fn bump(&self) -> Self { - Self(self.0 + 1) - } -} +} \ No newline at end of file diff --git a/tests/examples.rs b/tests/examples.rs index f968bba1..1a5e8136 100644 --- a/tests/examples.rs +++ b/tests/examples.rs @@ -5,9 +5,9 @@ use pubgrub::range::Range; use pubgrub::report::{DefaultStringReporter, Reporter as _}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::type_aliases::Map; -use pubgrub::version::{NumberVersion, SemanticVersion}; +use pubgrub::version::SemanticVersion; -type NumVS = Range; +type NumVS = Range; type SemVS = Range; use log::LevelFilter; @@ -193,22 +193,22 @@ fn conflict_with_partial_satisfier() { fn double_choices() { init_log(); let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); - dependency_provider.add_dependencies("a", 0, [("b", Range::full()), ("c", Range::full())]); - dependency_provider.add_dependencies("b", 0, [("d", Range::singleton(0))]); - dependency_provider.add_dependencies("b", 1, [("d", Range::singleton(1))]); - dependency_provider.add_dependencies("c", 0, []); - dependency_provider.add_dependencies("c", 1, [("d", Range::singleton(2))]); - dependency_provider.add_dependencies("d", 0, []); + dependency_provider.add_dependencies("a", 0u32, [("b", Range::full()), ("c", Range::full())]); + dependency_provider.add_dependencies("b", 0u32, [("d", Range::singleton(0u32))]); + dependency_provider.add_dependencies("b", 1u32, [("d", Range::singleton(1u32))]); + dependency_provider.add_dependencies("c", 0u32, []); + dependency_provider.add_dependencies("c", 1u32, [("d", Range::singleton(2u32))]); + dependency_provider.add_dependencies("d", 0u32, []); // Solution. let mut expected_solution = Map::default(); - expected_solution.insert("a", 0.into()); - expected_solution.insert("b", 0.into()); - expected_solution.insert("c", 0.into()); - expected_solution.insert("d", 0.into()); + expected_solution.insert("a", 0u32); + expected_solution.insert("b", 0u32); + expected_solution.insert("c", 0u32); + expected_solution.insert("d", 0u32); // Run the algorithm. - let computed_solution = resolve(&dependency_provider, "a", 0).unwrap(); + let computed_solution = resolve(&dependency_provider, "a", 0u32).unwrap(); assert_eq!(expected_solution, computed_solution); } @@ -217,7 +217,7 @@ fn confusing_with_lots_of_holes() { let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); // root depends on foo... - dependency_provider.add_dependencies("root", 1, vec![("foo", Range::full())]); + dependency_provider.add_dependencies("root", 1u32, vec![("foo", Range::full())]); for i in 1..6 { // foo depends on bar... @@ -225,7 +225,7 @@ fn confusing_with_lots_of_holes() { } let Err(PubGrubError::NoSolution(mut derivation_tree)) = - resolve(&dependency_provider, "root", 1) + resolve(&dependency_provider, "root", 1u32) else { unreachable!() }; diff --git a/tests/proptest.rs b/tests/proptest.rs index 24dd64a4..338aba82 100644 --- a/tests/proptest.rs +++ b/tests/proptest.rs @@ -11,7 +11,6 @@ use pubgrub::range::Range; use pubgrub::report::{DefaultStringReporter, DerivationTree, External, Reporter}; use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider}; use pubgrub::type_aliases::SelectedDependencies; -use pubgrub::version::NumberVersion; #[cfg(feature = "serde")] use pubgrub::version::SemanticVersion; use pubgrub::version_set::VersionSet; @@ -118,7 +117,7 @@ fn timeout_resolve>( ) } -type NumVS = Range; +type NumVS = Range; #[cfg(feature = "serde")] type SemVS = Range; @@ -126,13 +125,13 @@ type SemVS = Range; #[should_panic] fn should_cancel_can_panic() { let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); - dependency_provider.add_dependencies(0, 0, [(666, Range::full())]); + dependency_provider.add_dependencies(0, 0u32, [(666, Range::full())]); // Run the algorithm. let _ = resolve( &TimeoutDependencyProvider::new(dependency_provider, 1), 0, - 0, + 0u32, ); } @@ -152,7 +151,7 @@ fn string_names() -> impl Strategy { /// This strategy has a high probability of having valid dependencies pub fn registry_strategy( name: impl Strategy, -) -> impl Strategy, Vec<(N, NumberVersion)>)> { +) -> impl Strategy, Vec<(N, u32)>)> { let max_crates = 40; let max_versions = 15; let shrinkage = 40; @@ -193,14 +192,10 @@ pub fn registry_strategy( ) .prop_map( move |(crate_vers_by_name, raw_dependencies, reverse_alphabetical, complicated_len)| { - let mut list_of_pkgid: Vec<((N, NumberVersion), Vec<(N, NumVS)>)> = - crate_vers_by_name - .iter() - .flat_map(|(name, vers)| { - vers.iter() - .map(move |x| ((name.clone(), NumberVersion::from(x)), vec![])) - }) - .collect(); + let mut list_of_pkgid: Vec<((N, u32), Vec<(N, NumVS)>)> = crate_vers_by_name + .iter() + .flat_map(|(name, vers)| vers.iter().map(move |&x| ((name.clone(), x), vec![]))) + .collect(); let len_all_pkgid = list_of_pkgid.len(); for (a, b, (c, d)) in raw_dependencies { let (a, b) = order_index(a, b, len_all_pkgid); @@ -355,7 +350,7 @@ fn retain_dependencies( fn errors_the_same_with_only_report_dependencies( dependency_provider: OfflineDependencyProvider, name: N, - ver: NumberVersion, + ver: u32, ) { let Err(PubGrubError::NoSolution(tree)) = timeout_resolve(dependency_provider.clone(), name.clone(), ver) @@ -532,7 +527,7 @@ proptest! { (dependency_provider, cases) in registry_strategy(0u16..665), indexes_to_remove in prop::collection::vec(any::(), 1..10) ) { - let all_versions: Vec<(u16, NumberVersion)> = dependency_provider + let all_versions: Vec<(u16, u32)> = dependency_provider .packages() .flat_map(|&p| { dependency_provider @@ -587,14 +582,14 @@ fn large_case() { eprint!("{} ", name); let data = std::fs::read_to_string(&case).unwrap(); let start_time = std::time::Instant::now(); - if name.ends_with("u16_NumberVersion.ron") { + if name.ends_with("u16_NumberVersion.ron") || name.ends_with("u16_u32.ron") { let dependency_provider: OfflineDependencyProvider = ron::de::from_str(&data).unwrap(); let mut sat = SatResolve::new(&dependency_provider); for p in dependency_provider.packages() { - for v in dependency_provider.versions(p).unwrap() { + for &v in dependency_provider.versions(p).unwrap() { let res = resolve(&dependency_provider, *p, v); - sat.check_resolve(&res, p, v); + sat.check_resolve(&res, p, &v); } } } else if name.ends_with("str_SemanticVersion.ron") { diff --git a/tests/tests.rs b/tests/tests.rs index 81fd5324..79b8cec8 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -3,23 +3,22 @@ use pubgrub::error::PubGrubError; use pubgrub::range::Range; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::NumberVersion; -type NumVS = Range; +type NumVS = Range; #[test] fn same_result_on_repeated_runs() { let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); - dependency_provider.add_dependencies("c", 0, []); - dependency_provider.add_dependencies("c", 2, []); - dependency_provider.add_dependencies("b", 0, []); - dependency_provider.add_dependencies("b", 1, [("c", Range::between(0, 1))]); + dependency_provider.add_dependencies("c", 0u32, []); + dependency_provider.add_dependencies("c", 2u32, []); + dependency_provider.add_dependencies("b", 0u32, []); + dependency_provider.add_dependencies("b", 1u32, [("c", Range::between(0u32, 1u32))]); - dependency_provider.add_dependencies("a", 0, [("b", Range::full()), ("c", Range::full())]); + dependency_provider.add_dependencies("a", 0u32, [("b", Range::full()), ("c", Range::full())]); let name = "a"; - let ver = NumberVersion(0); + let ver: u32 = 0; let one = resolve(&dependency_provider, name, ver); for _ in 0..10 { match (&one, &resolve(&dependency_provider, name, ver)) { @@ -32,15 +31,15 @@ fn same_result_on_repeated_runs() { #[test] fn should_always_find_a_satisfier() { let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); - dependency_provider.add_dependencies("a", 0, [("b", Range::empty())]); + dependency_provider.add_dependencies("a", 0u32, [("b", Range::empty())]); assert!(matches!( - resolve(&dependency_provider, "a", 0), + resolve(&dependency_provider, "a", 0u32), Err(PubGrubError::NoSolution { .. }) )); - dependency_provider.add_dependencies("c", 0, [("a", Range::full())]); + dependency_provider.add_dependencies("c", 0u32, [("a", Range::full())]); assert!(matches!( - resolve(&dependency_provider, "c", 0), + resolve(&dependency_provider, "c", 0u32), Err(PubGrubError::NoSolution { .. }) )); } @@ -48,9 +47,9 @@ fn should_always_find_a_satisfier() { #[test] fn cannot_depend_on_self() { let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); - dependency_provider.add_dependencies("a", 0, [("a", Range::full())]); + dependency_provider.add_dependencies("a", 0u32, [("a", Range::full())]); assert!(matches!( - resolve(&dependency_provider, "a", 0), + resolve(&dependency_provider, "a", 0u32), Err(PubGrubError::SelfDependency { .. }) )); } From d1699fa09128c9e502c4e4caf1de0385873f45e1 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Wed, 20 Mar 2024 21:02:49 +0000 Subject: [PATCH 2/2] clean up --- src/lib.rs | 16 +++++----------- src/version.rs | 2 +- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4b3d499d..6649f1e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,8 +11,8 @@ //! # Package and Version traits //! //! All the code in this crate is manipulating packages and versions, and for this to work -//! we defined a [Package](package::Package) and [Version](version::Version) traits -//! that are used as bounds on most of the exposed types and functions. +//! we defined a [Package](package::Package) trait +//! that is used as bounds on most of the exposed types and functions. //! //! Package identifiers needs to implement our [Package](package::Package) trait, //! which is automatic if the type already implements @@ -20,15 +20,9 @@ //! So things like [String] will work out of the box. //! //! TODO! This is all wrong. Need to talk about VS, not Version. -//! Our [Version](version::Version) trait requires -//! [Clone] + [Ord] + [Debug] + [Display](std::fmt::Display) -//! and also the definition of two methods, -//! [lowest() -> Self](version::Version::lowest) which returns the lowest version existing, -//! and [bump(&self) -> Self](version::Version::bump) which returns the next smallest version -//! strictly higher than the current one. -//! For convenience, this library already provides -//! two implementations of [Version](version::Version). -//! The second one is [SemanticVersion](version::SemanticVersion) +//! Our Version trait requires +//! [Clone] + [Ord] + [Debug] + [Display](std::fmt::Display). +//! For convenience, this library provides [SemanticVersion](version::SemanticVersion) //! that implements semantic versioning rules. //! //! # Basic example diff --git a/src/version.rs b/src/version.rs index 5c8a4145..e77a5683 100644 --- a/src/version.rs +++ b/src/version.rs @@ -218,4 +218,4 @@ impl Display for SemanticVersion { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}.{}.{}", self.major, self.minor, self.patch) } -} \ No newline at end of file +}