diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf7df723..aba6531b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,7 @@ name: CI on: pull_request: + merge_group: push: branches: [ release, dev ] schedule: [ cron: "0 6 * * 4" ] @@ -103,6 +104,7 @@ jobs: check_commit_conventions: name: Commit messages follow project guidelines runs-on: ubuntu-latest + if: github.event_name != 'pull_request' || github.event.action == 'enqueued' steps: - uses: actions/checkout@v2 with: diff --git a/.github/workflows/permaref.yaml b/.github/workflows/permaref.yaml new file mode 100644 index 00000000..21e05507 --- /dev/null +++ b/.github/workflows/permaref.yaml @@ -0,0 +1,40 @@ +# Automatically creates a tag for each commit to `main` so when we rebase +# changes on top of the upstream, we retain permanent references to each +# previous commit so they are not orphaned and eventually deleted. +name: Create permanent reference + +on: + push: + branches: + - "main" + +jobs: + release: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Get the permanent ref number + id: get_version + run: | + # Enable pipefail so git command failures do not result in null versions downstream + set -x + + echo ::set-output name=LAST_PERMA_NUMBER::$(\ + git ls-remote --tags --refs --sort="v:refname" \ + https://github.com/zanieb/pubgrub.git | grep "tags/perma-" | tail -n1 | sed 's/.*\/perma-//' \ + ) + + - name: Configure Git + run: | + git config user.name "$GITHUB_ACTOR" + git config user.email "$GITHUB_ACTOR@users.noreply.github.com" + + - name: Create and push the new tag + run: | + TAG="perma-$((LAST_PERMA_NUMBER + 1))" + git tag -a "$TAG" -m "Automatically created on push to `main`" + git push origin "$TAG" + env: + LAST_PERMA_NUMBER: ${{ steps.get_version.outputs.LAST_PERMA_NUMBER }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 51f58e14..abe44da9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,7 +48,7 @@ The gist of it is: #### Added -- Links to code items in the code documenation. +- Links to code items in the code documentation. - New `"serde"` feature that allows serializing some library types, useful for making simple reproducible bug reports. - New variants for `error::PubGrubError` which are `DependencyOnTheEmptySet`, `SelfDependency`, `ErrorChoosingPackageVersion` and `ErrorInShouldCancel`. diff --git a/Cargo.toml b/Cargo.toml index 1f6b32fb..a4c94e69 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ authors = [ "Alex Tokarev ", "Jacob Finkelman ", ] -edition = "2018" +edition = "2021" description = "PubGrub version solving algorithm" readme = "README.md" repository = "https://github.com/pubgrub-rs/pubgrub" @@ -20,15 +20,19 @@ include = ["Cargo.toml", "LICENSE", "README.md", "src/**", "tests/**", "examples # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +indexmap = "2.0.2" +priority-queue = "1.1.1" thiserror = "1.0" rustc-hash = "1.1.0" serde = { version = "1.0", features = ["derive"], optional = true } +log = "0.4.14" # for debug logs in tests [dev-dependencies] proptest = "0.10.1" ron = "0.6" varisat = "0.2.2" criterion = "0.3" +env_logger = "0.9.0" [[bench]] name = "large_case" diff --git a/benches/large_case.rs b/benches/large_case.rs index 476228c8..8efdaa9e 100644 --- a/benches/large_case.rs +++ b/benches/large_case.rs @@ -5,16 +5,19 @@ extern crate criterion; use self::criterion::*; use pubgrub::package::Package; +use pubgrub::range::Range; use pubgrub::solver::{resolve, OfflineDependencyProvider}; -use pubgrub::version::{NumberVersion, SemanticVersion, Version}; +use pubgrub::version::{NumberVersion, SemanticVersion}; +use pubgrub::version_set::VersionSet; use serde::de::Deserialize; -use std::hash::Hash; -fn bench<'a, P: Package + Deserialize<'a>, V: Version + Hash + Deserialize<'a>>( +fn bench<'a, P: Package + Deserialize<'a>, VS: VersionSet + Deserialize<'a>>( b: &mut Bencher, case: &'a str, -) { - let dependency_provider: OfflineDependencyProvider = ron::de::from_str(&case).unwrap(); +) where + ::V: Deserialize<'a>, +{ + let dependency_provider: OfflineDependencyProvider = ron::de::from_str(&case).unwrap(); b.iter(|| { for p in dependency_provider.packages() { @@ -35,11 +38,11 @@ fn bench_nested(c: &mut Criterion) { let data = std::fs::read_to_string(&case).unwrap(); if name.ends_with("u16_NumberVersion.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| { - bench::<&str, SemanticVersion>(b, &data); + bench::<&str, Range>(b, &data); }); } } diff --git a/examples/branching_error_reporting.rs b/examples/branching_error_reporting.rs index c1daa1bf..d4dfb719 100644 --- a/examples/branching_error_reporting.rs +++ b/examples/branching_error_reporting.rs @@ -6,51 +6,53 @@ use pubgrub::report::{DefaultStringReporter, Reporter}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::version::SemanticVersion; +type SemVS = Range; + // https://github.com/dart-lang/pub/blob/master/doc/solver.md#branching-error-reporting fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new(); #[rustfmt::skip] // root 1.0.0 depends on foo ^1.0.0 dependency_provider.add_dependencies( "root", (1, 0, 0), - vec![("foo", Range::between((1, 0, 0), (2, 0, 0)))], + [("foo", Range::from_range_bounds((1, 0, 0)..(2, 0, 0)))], ); #[rustfmt::skip] // foo 1.0.0 depends on a ^1.0.0 and b ^1.0.0 dependency_provider.add_dependencies( "foo", (1, 0, 0), - vec![ - ("a", Range::between((1, 0, 0), (2, 0, 0))), - ("b", Range::between((1, 0, 0), (2, 0, 0))), + [ + ("a", Range::from_range_bounds((1, 0, 0)..(2, 0, 0))), + ("b", Range::from_range_bounds((1, 0, 0)..(2, 0, 0))), ], ); #[rustfmt::skip] // foo 1.1.0 depends on x ^1.0.0 and y ^1.0.0 dependency_provider.add_dependencies( "foo", (1, 1, 0), - vec![ - ("x", Range::between((1, 0, 0), (2, 0, 0))), - ("y", Range::between((1, 0, 0), (2, 0, 0))), + [ + ("x", Range::from_range_bounds((1, 0, 0)..(2, 0, 0))), + ("y", Range::from_range_bounds((1, 0, 0)..(2, 0, 0))), ], ); #[rustfmt::skip] // a 1.0.0 depends on b ^2.0.0 dependency_provider.add_dependencies( "a", (1, 0, 0), - vec![("b", Range::between((2, 0, 0), (3, 0, 0)))], + [("b", Range::from_range_bounds((2, 0, 0)..(3, 0, 0)))], ); // b 1.0.0 and 2.0.0 have no dependencies. - dependency_provider.add_dependencies("b", (1, 0, 0), vec![]); - dependency_provider.add_dependencies("b", (2, 0, 0), vec![]); + dependency_provider.add_dependencies("b", (1, 0, 0), []); + dependency_provider.add_dependencies("b", (2, 0, 0), []); #[rustfmt::skip] // x 1.0.0 depends on y ^2.0.0. dependency_provider.add_dependencies( "x", (1, 0, 0), - vec![("y", Range::between((2, 0, 0), (3, 0, 0)))], + [("y", Range::from_range_bounds((2, 0, 0)..(3, 0, 0)))], ); // y 1.0.0 and 2.0.0 have no dependencies. - dependency_provider.add_dependencies("y", (1, 0, 0), vec![]); - dependency_provider.add_dependencies("y", (2, 0, 0), vec![]); + dependency_provider.add_dependencies("y", (1, 0, 0), []); + dependency_provider.add_dependencies("y", (2, 0, 0), []); // Run the algorithm. match resolve(&dependency_provider, "root", (1, 0, 0)) { diff --git a/examples/caching_dependency_provider.rs b/examples/caching_dependency_provider.rs index bac730ea..6ef8e893 100644 --- a/examples/caching_dependency_provider.rs +++ b/examples/caching_dependency_provider.rs @@ -6,16 +6,21 @@ use std::error::Error; use pubgrub::package::Package; use pubgrub::range::Range; use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider}; -use pubgrub::version::{NumberVersion, Version}; +use pubgrub::version::NumberVersion; +use pubgrub::version_set::VersionSet; + +type NumVS = Range; // An example implementing caching dependency provider that will // store queried dependencies in memory and check them before querying more from remote. -struct CachingDependencyProvider> { +struct CachingDependencyProvider> { remote_dependencies: DP, - cached_dependencies: RefCell>, + cached_dependencies: RefCell>, } -impl> CachingDependencyProvider { +impl> + CachingDependencyProvider +{ pub fn new(remote_dependencies_provider: DP) -> Self { CachingDependencyProvider { remote_dependencies: remote_dependencies_provider, @@ -24,22 +29,15 @@ impl> CachingDependencyProv } } -impl> DependencyProvider - for CachingDependencyProvider +impl> DependencyProvider + for CachingDependencyProvider { - fn choose_package_version, U: std::borrow::Borrow>>( - &self, - packages: impl Iterator, - ) -> Result<(T, Option), Box> { - self.remote_dependencies.choose_package_version(packages) - } - // Caches dependencies if they were already queried fn get_dependencies( &self, package: &P, - version: &V, - ) -> Result, Box> { + version: &VS::V, + ) -> Result, Box> { let mut cache = self.cached_dependencies.borrow_mut(); match cache.get_dependencies(package, version) { Ok(Dependencies::Unknown) => { @@ -49,7 +47,7 @@ impl> DependencyProvider> DependencyProvider error, } } + + fn choose_version( + &self, + package: &P, + range: &VS, + ) -> Result, Box> { + self.remote_dependencies.choose_version(package, range) + } + + type Priority = DP::Priority; + + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + self.remote_dependencies.prioritize(package, range) + } } fn main() { // Simulating remote provider locally. - let mut remote_dependencies_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); + 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()); diff --git a/examples/doc_interface.rs b/examples/doc_interface.rs index b3e37838..ca6bcb93 100644 --- a/examples/doc_interface.rs +++ b/examples/doc_interface.rs @@ -4,19 +4,21 @@ use pubgrub::range::Range; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::version::NumberVersion; +type NumVS = Range; + // `root` depends on `menu` and `icons` // `menu` depends on `dropdown` // `dropdown` depends on `icons` // `icons` has no dependency #[rustfmt::skip] fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); + let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); dependency_provider.add_dependencies( - "root", 1, vec![("menu", Range::any()), ("icons", Range::any())], + "root", 1, [("menu", Range::full()), ("icons", Range::full())], ); - dependency_provider.add_dependencies("menu", 1, vec![("dropdown", Range::any())]); - dependency_provider.add_dependencies("dropdown", 1, vec![("icons", Range::any())]); - dependency_provider.add_dependencies("icons", 1, vec![]); + dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]); + dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]); + dependency_provider.add_dependencies("icons", 1, []); // Run the algorithm. let solution = resolve(&dependency_provider, "root", 1); diff --git a/examples/doc_interface_error.rs b/examples/doc_interface_error.rs index 0ef0f1ec..a78a3eb3 100644 --- a/examples/doc_interface_error.rs +++ b/examples/doc_interface_error.rs @@ -6,6 +6,8 @@ use pubgrub::report::{DefaultStringReporter, Reporter}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::version::SemanticVersion; +type SemVS = Range; + // `root` depends on `menu`, `icons 1.0.0` and `intl 5.0.0` // `menu 1.0.0` depends on `dropdown < 2.0.0` // `menu >= 1.1.0` depends on `dropdown >= 2.0.0` @@ -15,59 +17,59 @@ use pubgrub::version::SemanticVersion; // `intl` has no dependency #[rustfmt::skip] fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new(); // Direct dependencies: menu and icons. - dependency_provider.add_dependencies("root", (1, 0, 0), vec![ - ("menu", Range::any()), - ("icons", Range::exact((1, 0, 0))), - ("intl", Range::exact((5, 0, 0))), + dependency_provider.add_dependencies("root", (1, 0, 0), [ + ("menu", Range::full()), + ("icons", Range::singleton((1, 0, 0))), + ("intl", Range::singleton((5, 0, 0))), ]); // Dependencies of the menu lib. - dependency_provider.add_dependencies("menu", (1, 0, 0), vec![ - ("dropdown", Range::strictly_lower_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 0, 0), [ + ("dropdown", Range::from_range_bounds(..(2, 0, 0))), ]); - dependency_provider.add_dependencies("menu", (1, 1, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 1, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); - dependency_provider.add_dependencies("menu", (1, 2, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 2, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); - dependency_provider.add_dependencies("menu", (1, 3, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 3, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); - dependency_provider.add_dependencies("menu", (1, 4, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 4, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); - dependency_provider.add_dependencies("menu", (1, 5, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 5, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); // Dependencies of the dropdown lib. - dependency_provider.add_dependencies("dropdown", (1, 8, 0), vec![ - ("intl", Range::exact((3, 0, 0))), + dependency_provider.add_dependencies("dropdown", (1, 8, 0), [ + ("intl", Range::singleton((3, 0, 0))), ]); - dependency_provider.add_dependencies("dropdown", (2, 0, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + dependency_provider.add_dependencies("dropdown", (2, 0, 0), [ + ("icons", Range::singleton((2, 0, 0))), ]); - dependency_provider.add_dependencies("dropdown", (2, 1, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + dependency_provider.add_dependencies("dropdown", (2, 1, 0), [ + ("icons", Range::singleton((2, 0, 0))), ]); - dependency_provider.add_dependencies("dropdown", (2, 2, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + dependency_provider.add_dependencies("dropdown", (2, 2, 0), [ + ("icons", Range::singleton((2, 0, 0))), ]); - dependency_provider.add_dependencies("dropdown", (2, 3, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + dependency_provider.add_dependencies("dropdown", (2, 3, 0), [ + ("icons", Range::singleton((2, 0, 0))), ]); // Icons have no dependencies. - dependency_provider.add_dependencies("icons", (1, 0, 0), vec![]); - dependency_provider.add_dependencies("icons", (2, 0, 0), vec![]); + dependency_provider.add_dependencies("icons", (1, 0, 0), []); + dependency_provider.add_dependencies("icons", (2, 0, 0), []); // Intl have no dependencies. - dependency_provider.add_dependencies("intl", (3, 0, 0), vec![]); - dependency_provider.add_dependencies("intl", (4, 0, 0), vec![]); - dependency_provider.add_dependencies("intl", (5, 0, 0), vec![]); + dependency_provider.add_dependencies("intl", (3, 0, 0), []); + dependency_provider.add_dependencies("intl", (4, 0, 0), []); + dependency_provider.add_dependencies("intl", (5, 0, 0), []); // Run the algorithm. match resolve(&dependency_provider, "root", (1, 0, 0)) { diff --git a/examples/doc_interface_semantic.rs b/examples/doc_interface_semantic.rs index b4c352e1..17ff3c09 100644 --- a/examples/doc_interface_semantic.rs +++ b/examples/doc_interface_semantic.rs @@ -6,6 +6,8 @@ use pubgrub::report::{DefaultStringReporter, Reporter}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::version::SemanticVersion; +type SemVS = Range; + // `root` depends on `menu` and `icons 1.0.0` // `menu 1.0.0` depends on `dropdown < 2.0.0` // `menu >= 1.1.0` depends on `dropdown >= 2.0.0` @@ -14,51 +16,51 @@ use pubgrub::version::SemanticVersion; // `icons` has no dependency #[rustfmt::skip] fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new(); // Direct dependencies: menu and icons. - dependency_provider.add_dependencies("root", (1, 0, 0), vec![ - ("menu", Range::any()), - ("icons", Range::exact((1, 0, 0))), + dependency_provider.add_dependencies("root", (1, 0, 0), [ + ("menu", Range::full()), + ("icons", Range::singleton((1, 0, 0))), ]); // Dependencies of the menu lib. - dependency_provider.add_dependencies("menu", (1, 0, 0), vec![ - ("dropdown", Range::strictly_lower_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 0, 0), [ + ("dropdown", Range::from_range_bounds(..(2, 0, 0))), ]); - dependency_provider.add_dependencies("menu", (1, 1, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 1, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); - dependency_provider.add_dependencies("menu", (1, 2, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 2, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); - dependency_provider.add_dependencies("menu", (1, 3, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 3, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); - dependency_provider.add_dependencies("menu", (1, 4, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 4, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); - dependency_provider.add_dependencies("menu", (1, 5, 0), vec![ - ("dropdown", Range::higher_than((2, 0, 0))), + dependency_provider.add_dependencies("menu", (1, 5, 0), [ + ("dropdown", Range::from_range_bounds((2, 0, 0)..)), ]); // Dependencies of the dropdown lib. - dependency_provider.add_dependencies("dropdown", (1, 8, 0), vec![]); - dependency_provider.add_dependencies("dropdown", (2, 0, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + dependency_provider.add_dependencies("dropdown", (1, 8, 0), []); + dependency_provider.add_dependencies("dropdown", (2, 0, 0), [ + ("icons", Range::singleton((2, 0, 0))), ]); - dependency_provider.add_dependencies("dropdown", (2, 1, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + dependency_provider.add_dependencies("dropdown", (2, 1, 0), [ + ("icons", Range::singleton((2, 0, 0))), ]); - dependency_provider.add_dependencies("dropdown", (2, 2, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + dependency_provider.add_dependencies("dropdown", (2, 2, 0), [ + ("icons", Range::singleton((2, 0, 0))), ]); - dependency_provider.add_dependencies("dropdown", (2, 3, 0), vec![ - ("icons", Range::exact((2, 0, 0))), + dependency_provider.add_dependencies("dropdown", (2, 3, 0), [ + ("icons", Range::singleton((2, 0, 0))), ]); // Icons has no dependency. - dependency_provider.add_dependencies("icons", (1, 0, 0), vec![]); - dependency_provider.add_dependencies("icons", (2, 0, 0), vec![]); + dependency_provider.add_dependencies("icons", (1, 0, 0), []); + dependency_provider.add_dependencies("icons", (2, 0, 0), []); // Run the algorithm. match resolve(&dependency_provider, "root", (1, 0, 0)) { diff --git a/examples/linear_error_reporting.rs b/examples/linear_error_reporting.rs index 0673fe36..8624fe2a 100644 --- a/examples/linear_error_reporting.rs +++ b/examples/linear_error_reporting.rs @@ -6,33 +6,35 @@ use pubgrub::report::{DefaultStringReporter, Reporter}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::version::SemanticVersion; +type SemVS = Range; + // https://github.com/dart-lang/pub/blob/master/doc/solver.md#linear-error-reporting fn main() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new(); #[rustfmt::skip] // root 1.0.0 depends on foo ^1.0.0 and baz ^1.0.0 dependency_provider.add_dependencies( "root", (1, 0, 0), - vec![ - ("foo", Range::between((1, 0, 0), (2, 0, 0))), - ("baz", Range::between((1, 0, 0), (2, 0, 0))), + [ + ("foo", Range::from_range_bounds((1, 0, 0)..(2, 0, 0))), + ("baz", Range::from_range_bounds((1, 0, 0)..(2, 0, 0))), ], ); #[rustfmt::skip] // foo 1.0.0 depends on bar ^2.0.0 dependency_provider.add_dependencies( "foo", (1, 0, 0), - vec![("bar", Range::between((2, 0, 0), (3, 0, 0)))], + [("bar", Range::from_range_bounds((2, 0, 0)..(3, 0, 0)))], ); #[rustfmt::skip] // bar 2.0.0 depends on baz ^3.0.0 dependency_provider.add_dependencies( "bar", (2, 0, 0), - vec![("baz", Range::between((3, 0, 0), (4, 0, 0)))], + [("baz", Range::from_range_bounds((3, 0, 0)..(4, 0, 0)))], ); // baz 1.0.0 and 3.0.0 have no dependencies - dependency_provider.add_dependencies("baz", (1, 0, 0), vec![]); - dependency_provider.add_dependencies("baz", (3, 0, 0), vec![]); + dependency_provider.add_dependencies("baz", (1, 0, 0), []); + dependency_provider.add_dependencies("baz", (3, 0, 0), []); // Run the algorithm. match resolve(&dependency_provider, "root", (1, 0, 0)) { diff --git a/release.md b/release.md new file mode 100644 index 00000000..fdbd1a1f --- /dev/null +++ b/release.md @@ -0,0 +1,28 @@ +# Creation of a new release + +This is taking the 0.2.1 release as an example. + +## GitHub stuff + +- Checkout the prep-v0.2.1 branch +- Update the release date in the changelog and push to the PR. +- Squash merge the PR to the dev branch +- Check that the merged PR is passing the tests on the dev branch +- Pull the updated dev locally +- Switch to the release branch +- Merge locally dev into release in fast-forward mode, we want to keep the history of commits and the merge point. +- `git tag -a v0.2.1 -m "v0.2.1: mostly perf improvements"` +- (Optional) cryptographically sign the tag +- On GitHub, edit the branch protection setting for release: uncheck include admin, and save +- Push release to github: git push --follow-tags +- Reset the release branch protection to include admins +- On GitHub, create a release from that tag. + +## Crates.io stuff + +- `cargo publish --dry-run` +- `cargo publish` + +## Community stuff + +Talk about the awesome new features of the new release online. diff --git a/src/error.rs b/src/error.rs index 0553d8de..15a410e3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,14 +6,14 @@ use thiserror::Error; use crate::package::Package; use crate::report::DerivationTree; -use crate::version::Version; +use crate::version_set::VersionSet; /// Errors that may occur while solving dependencies. #[derive(Error, Debug)] -pub enum PubGrubError { +pub enum PubGrubError { /// There is no solution for this set of dependencies. #[error("No solution")] - NoSolution(DerivationTree), + NoSolution(DerivationTree), /// Error arising when the implementer of /// [DependencyProvider](crate::solver::DependencyProvider) @@ -24,25 +24,10 @@ pub enum PubGrubError { /// Package whose dependencies we want. package: P, /// Version of the package for which we want the dependencies. - version: V, + version: VS::V, /// Error raised by the implementer of /// [DependencyProvider](crate::solver::DependencyProvider). - source: Box, - }, - - /// Error arising when the implementer of - /// [DependencyProvider](crate::solver::DependencyProvider) - /// returned a dependency on an empty range. - /// This technically means that the package can not be selected, - /// but is clearly some kind of mistake. - #[error("Package {dependent} required by {package} {version} depends on the empty set")] - DependencyOnTheEmptySet { - /// Package whose dependencies we want. - package: P, - /// Version of the package for which we want the dependencies. - version: V, - /// The dependent package that requires us to pick from the empty set. - dependent: P, + source: Box, }, /// Error arising when the implementer of @@ -55,20 +40,20 @@ pub enum PubGrubError { /// Package whose dependencies we want. package: P, /// Version of the package for which we want the dependencies. - version: V, + version: VS::V, }, /// Error arising when the implementer of /// [DependencyProvider](crate::solver::DependencyProvider) /// returned an error in the method - /// [choose_package_version](crate::solver::DependencyProvider::choose_package_version). + /// [choose_version](crate::solver::DependencyProvider::choose_version). #[error("Decision making failed")] - ErrorChoosingPackageVersion(Box), + ErrorChoosingPackageVersion(Box), /// Error arising when the implementer of [DependencyProvider](crate::solver::DependencyProvider) /// returned an error in the method [should_cancel](crate::solver::DependencyProvider::should_cancel). #[error("We should cancel")] - ErrorInShouldCancel(Box), + ErrorInShouldCancel(Box), /// Something unexpected happened. #[error("{0}")] diff --git a/src/internal/arena.rs b/src/internal/arena.rs index 75d04c82..7b4fc160 100644 --- a/src/internal/arena.rs +++ b/src/internal/arena.rs @@ -55,7 +55,7 @@ impl Id { } fn from(n: u32) -> Self { Self { - raw: n as u32, + raw: n, _ty: PhantomData, } } diff --git a/src/internal/core.rs b/src/internal/core.rs index f923850a..06e3ae21 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -15,28 +15,27 @@ use crate::internal::partial_solution::{DecisionLevel, PartialSolution}; use crate::internal::small_vec::SmallVec; use crate::package::Package; use crate::report::DerivationTree; -use crate::solver::DependencyConstraints; -use crate::type_aliases::Map; -use crate::version::Version; +use crate::type_aliases::{DependencyConstraints, Map}; +use crate::version_set::VersionSet; /// Current state of the PubGrub algorithm. #[derive(Clone)] -pub struct State { +pub struct State { root_package: P, - root_version: V, + root_version: VS::V, - incompatibilities: Map>>, + incompatibilities: Map>>, /// Store the ids of incompatibilities that are already contradicted /// and will stay that way until the next conflict and backtrack is operated. - contradicted_incompatibilities: rustc_hash::FxHashSet>, + contradicted_incompatibilities: rustc_hash::FxHashSet>, /// Partial solution. /// TODO: remove pub. - pub partial_solution: PartialSolution, + pub partial_solution: PartialSolution, /// The store is the reference storage for all incompatibilities. - pub incompatibility_store: Arena>, + pub incompatibility_store: Arena>, /// This is a stack of work to be done in `unit_propagation`. /// It can definitely be a local variable to that method, but @@ -44,9 +43,9 @@ pub struct State { unit_propagation_buffer: SmallVec

, } -impl State { +impl State { /// Initialization of PubGrub state. - pub fn init(root_package: P, root_version: V) -> Self { + pub fn init(root_package: P, root_version: VS::V) -> Self { let mut incompatibility_store = Arena::new(); let not_root_id = incompatibility_store.alloc(Incompatibility::not_root( root_package.clone(), @@ -66,7 +65,7 @@ impl State { } /// Add an incompatibility to the state. - pub fn add_incompatibility(&mut self, incompat: Incompatibility) { + pub fn add_incompatibility(&mut self, incompat: Incompatibility) { let id = self.incompatibility_store.alloc(incompat); self.merge_incompatibility(id); } @@ -75,9 +74,9 @@ impl State { pub fn add_incompatibility_from_dependencies( &mut self, package: P, - version: V, - deps: &DependencyConstraints, - ) -> std::ops::Range> { + version: VS::V, + deps: &DependencyConstraints, + ) -> std::ops::Range> { // Create incompatibilities and allocate them in the store. let new_incompats_id_range = self .incompatibility_store @@ -91,14 +90,9 @@ impl State { new_incompats_id_range } - /// Check if an incompatibility is terminal. - pub fn is_terminal(&self, incompatibility: &Incompatibility) -> bool { - incompatibility.is_terminal(&self.root_package, &self.root_version) - } - /// Unit propagation is the core mechanism of the solving algorithm. /// CF - pub fn unit_propagation(&mut self, package: P) -> Result<(), PubGrubError> { + pub fn unit_propagation(&mut self, package: P) -> Result<(), PubGrubError> { self.unit_propagation_buffer.clear(); self.unit_propagation_buffer.push(package); while let Some(current_package) = self.unit_propagation_buffer.pop() { @@ -115,6 +109,10 @@ impl State { // If the partial solution satisfies the incompatibility // we must perform conflict resolution. Relation::Satisfied => { + log::info!( + "Start conflict resolution because incompat satisfied:\n {}", + current_incompat + ); conflict_id = Some(incompat_id); break; } @@ -158,8 +156,8 @@ impl State { /// CF fn conflict_resolution( &mut self, - incompatibility: IncompId, - ) -> Result<(P, IncompId), PubGrubError> { + incompatibility: IncompId, + ) -> Result<(P, IncompId), PubGrubError> { let mut current_incompat_id = incompatibility; let mut current_incompat_changed = false; loop { @@ -183,6 +181,7 @@ impl State { current_incompat_changed, previous_satisfier_level, ); + log::info!("backtrack to {:?}", previous_satisfier_level); return Ok((package, current_incompat_id)); } SameDecisionLevels { satisfier_cause } => { @@ -192,6 +191,7 @@ impl State { &package, &self.incompatibility_store, ); + log::info!("prior cause: {}", prior_cause); current_incompat_id = self.incompatibility_store.alloc(prior_cause); current_incompat_changed = true; } @@ -203,7 +203,7 @@ impl State { /// Backtracking. fn backtrack( &mut self, - incompat: IncompId, + incompat: IncompId, incompat_changed: bool, decision_level: DecisionLevel, ) { @@ -234,8 +234,11 @@ impl State { /// Here we do the simple stupid thing of just growing the Vec. /// It may not be trivial since those incompatibilities /// may already have derived others. - fn merge_incompatibility(&mut self, id: IncompId) { - for (pkg, _term) in self.incompatibility_store[id].iter() { + fn merge_incompatibility(&mut self, id: IncompId) { + for (pkg, term) in self.incompatibility_store[id].iter() { + if cfg!(debug_assertions) { + assert_ne!(term, &crate::term::Term::any()); + } self.incompatibilities .entry(pkg.clone()) .or_default() @@ -245,12 +248,12 @@ impl State { // Error reporting ######################################################### - fn build_derivation_tree(&self, incompat: IncompId) -> DerivationTree { + fn build_derivation_tree(&self, incompat: IncompId) -> DerivationTree { let shared_ids = self.find_shared_ids(incompat); Incompatibility::build_derivation_tree(incompat, &shared_ids, &self.incompatibility_store) } - fn find_shared_ids(&self, incompat: IncompId) -> Set> { + fn find_shared_ids(&self, incompat: IncompId) -> Set> { let mut all_ids = Set::new(); let mut shared_ids = Set::new(); let mut stack = vec![incompat]; diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index acf900b8..b56a3c44 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -9,10 +9,9 @@ use std::fmt; use crate::internal::arena::{Arena, Id}; use crate::internal::small_map::SmallMap; use crate::package::Package; -use crate::range::Range; use crate::report::{DefaultStringReporter, DerivationTree, Derived, External}; use crate::term::{self, Term}; -use crate::version::Version; +use crate::version_set::VersionSet; /// An incompatibility is a set of terms for different packages /// that should never be satisfied all together. @@ -30,26 +29,26 @@ use crate::version::Version; /// during conflict resolution. More about all this in /// [PubGrub documentation](https://github.com/dart-lang/pub/blob/master/doc/solver.md#incompatibility). #[derive(Debug, Clone)] -pub struct Incompatibility { - package_terms: SmallMap>, - kind: Kind, +pub struct Incompatibility { + package_terms: SmallMap>, + kind: Kind, } /// Type alias of unique identifiers for incompatibilities. -pub type IncompId = Id>; +pub type IncompId = Id>; #[derive(Debug, Clone)] -enum Kind { +enum Kind { /// Initial incompatibility aiming at picking the root package for the first decision. - NotRoot(P, V), + NotRoot(P, VS::V), /// There are no versions in the given range for this package. - NoVersions(P, Range), + NoVersions(P, VS), /// Dependencies of the package are unavailable for versions in that range. - UnavailableDependencies(P, Range), + UnavailableDependencies(P, VS), /// Incompatibility coming from the dependencies of a given package. - FromDependencyOf(P, Range, P, Range), + FromDependencyOf(P, VS, P, VS), /// Derived from two causes. Stores cause ids. - DerivedFrom(IncompId, IncompId), + DerivedFrom(IncompId, IncompId), } /// A Relation describes how a set of terms can be compared to an incompatibility. @@ -69,52 +68,56 @@ pub enum Relation { Inconclusive, } -impl Incompatibility { +impl Incompatibility { /// Create the initial "not Root" incompatibility. - pub fn not_root(package: P, version: V) -> Self { + pub fn not_root(package: P, version: VS::V) -> Self { Self { package_terms: SmallMap::One([( package.clone(), - Term::Negative(Range::exact(version.clone())), + Term::Negative(VS::singleton(version.clone())), )]), kind: Kind::NotRoot(package, version), } } /// Create an incompatibility to remember - /// that a given range does not contain any version. - pub fn no_versions(package: P, term: Term) -> Self { - let range = match &term { + /// that a given set does not contain any version. + pub fn no_versions(package: P, term: Term) -> Self { + let set = match &term { Term::Positive(r) => r.clone(), Term::Negative(_) => panic!("No version should have a positive term"), }; Self { package_terms: SmallMap::One([(package.clone(), term)]), - kind: Kind::NoVersions(package, range), + kind: Kind::NoVersions(package, set), } } /// Create an incompatibility to remember /// that a package version is not selectable /// because its list of dependencies is unavailable. - pub fn unavailable_dependencies(package: P, version: V) -> Self { - let range = Range::exact(version); + pub fn unavailable_dependencies(package: P, version: VS::V) -> Self { + let set = VS::singleton(version); Self { - package_terms: SmallMap::One([(package.clone(), Term::Positive(range.clone()))]), - kind: Kind::UnavailableDependencies(package, range), + package_terms: SmallMap::One([(package.clone(), Term::Positive(set.clone()))]), + kind: Kind::UnavailableDependencies(package, set), } } /// Build an incompatibility from a given dependency. - pub fn from_dependency(package: P, version: V, dep: (&P, &Range)) -> Self { - let range1 = Range::exact(version); - let (p2, range2) = dep; + pub fn from_dependency(package: P, version: VS::V, dep: (&P, &VS)) -> Self { + let set1 = VS::singleton(version); + let (p2, set2) = dep; Self { - package_terms: SmallMap::Two([ - (package.clone(), Term::Positive(range1.clone())), - (p2.clone(), Term::Negative(range2.clone())), - ]), - kind: Kind::FromDependencyOf(package, range1, p2.clone(), range2.clone()), + package_terms: if set2 == &VS::empty() { + SmallMap::One([(package.clone(), Term::Positive(set1.clone()))]) + } else { + SmallMap::Two([ + (package.clone(), Term::Positive(set1.clone())), + (p2.clone(), Term::Negative(set2.clone())), + ]) + }, + kind: Kind::FromDependencyOf(package, set1, p2.clone(), set2.clone()), } } @@ -145,24 +148,24 @@ impl Incompatibility { /// Check if an incompatibility should mark the end of the algorithm /// because it satisfies the root package. - pub fn is_terminal(&self, root_package: &P, root_version: &V) -> bool { + pub fn is_terminal(&self, root_package: &P, root_version: &VS::V) -> bool { if self.package_terms.len() == 0 { true } else if self.package_terms.len() > 1 { false } else { let (package, term) = self.package_terms.iter().next().unwrap(); - (package == root_package) && term.contains(&root_version) + (package == root_package) && term.contains(root_version) } } /// Get the term related to a given package (if it exists). - pub fn get(&self, package: &P) -> Option<&Term> { + pub fn get(&self, package: &P) -> Option<&Term> { self.package_terms.get(package) } /// Iterate over packages. - pub fn iter(&self) -> impl Iterator)> { + pub fn iter(&self) -> impl Iterator)> { self.package_terms.iter() } @@ -181,7 +184,7 @@ impl Incompatibility { self_id: Id, shared_ids: &Set>, store: &Arena, - ) -> DerivationTree { + ) -> DerivationTree { match &store[self_id].kind { Kind::DerivedFrom(id1, id2) => { let cause1 = Self::build_derivation_tree(*id1, shared_ids, store); @@ -197,30 +200,30 @@ impl Incompatibility { Kind::NotRoot(package, version) => { DerivationTree::External(External::NotRoot(package.clone(), version.clone())) } - Kind::NoVersions(package, range) => { - DerivationTree::External(External::NoVersions(package.clone(), range.clone())) + Kind::NoVersions(package, set) => { + DerivationTree::External(External::NoVersions(package.clone(), set.clone())) } - Kind::UnavailableDependencies(package, range) => DerivationTree::External( - External::UnavailableDependencies(package.clone(), range.clone()), + Kind::UnavailableDependencies(package, set) => DerivationTree::External( + External::UnavailableDependencies(package.clone(), set.clone()), ), - Kind::FromDependencyOf(package, range, dep_package, dep_range) => { + Kind::FromDependencyOf(package, set, dep_package, dep_set) => { DerivationTree::External(External::FromDependencyOf( package.clone(), - range.clone(), + set.clone(), dep_package.clone(), - dep_range.clone(), + dep_set.clone(), )) } } } } -impl<'a, P: Package, V: Version + 'a> Incompatibility { +impl<'a, P: Package, VS: VersionSet + 'a> Incompatibility { /// CF definition of Relation enum. - pub fn relation(&self, terms: impl Fn(&P) -> Option<&'a Term>) -> Relation

{ + pub fn relation(&self, terms: impl Fn(&P) -> Option<&'a Term>) -> Relation

{ let mut relation = Relation::Satisfied; for (package, incompat_term) in self.package_terms.iter() { - match terms(package).map(|term| incompat_term.relation_with(&term)) { + match terms(package).map(|term| incompat_term.relation_with(term)) { Some(term::Relation::Satisfied) => {} Some(term::Relation::Contradicted) => { return Relation::Contradicted(package.clone()); @@ -243,7 +246,7 @@ impl<'a, P: Package, V: Version + 'a> Incompatibility { } } -impl fmt::Display for Incompatibility { +impl fmt::Display for Incompatibility { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, @@ -258,6 +261,7 @@ impl fmt::Display for Incompatibility { #[cfg(test)] pub mod tests { use super::*; + use crate::range::Range; use crate::term::tests::strategy as term_strat; use crate::type_aliases::Map; use proptest::prelude::*; @@ -276,12 +280,12 @@ pub mod tests { let mut store = Arena::new(); let i1 = store.alloc(Incompatibility { package_terms: SmallMap::Two([("p1", t1.clone()), ("p2", t2.negate())]), - kind: Kind::UnavailableDependencies("0", Range::any()) + kind: Kind::UnavailableDependencies("0", Range::full()) }); let i2 = store.alloc(Incompatibility { package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]), - kind: Kind::UnavailableDependencies("0", Range::any()) + kind: Kind::UnavailableDependencies("0", Range::full()) }); let mut i3 = Map::default(); diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index ad454244..057dea13 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -3,17 +3,24 @@ //! A Memory acts like a structured partial solution //! where terms are regrouped by package in a [Map](crate::type_aliases::Map). +use std::fmt::Display; +use std::hash::BuildHasherDefault; + +use priority_queue::PriorityQueue; +use rustc_hash::FxHasher; + use crate::internal::arena::Arena; use crate::internal::incompatibility::{IncompId, Incompatibility, Relation}; use crate::internal::small_map::SmallMap; use crate::package::Package; -use crate::range::Range; use crate::term::Term; -use crate::type_aliases::{Map, SelectedDependencies}; -use crate::version::Version; +use crate::type_aliases::SelectedDependencies; +use crate::version_set::VersionSet; use super::small_vec::SmallVec; +type FnvIndexMap = indexmap::IndexMap>; + #[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] pub struct DecisionLevel(pub u32); @@ -26,58 +33,129 @@ impl DecisionLevel { /// The partial solution contains all package assignments, /// organized by package and historically ordered. #[derive(Clone, Debug)] -pub struct PartialSolution { +pub struct PartialSolution { next_global_index: u32, current_decision_level: DecisionLevel, - package_assignments: Map>, + /// `package_assignments` is primarily a HashMap from a package to its + /// `PackageAssignments`. But it can also keep the items in an order. + /// We maintain three sections in this order: + /// 1. `[..current_decision_level]` Are packages that have had a decision made sorted by the `decision_level`. + /// This makes it very efficient to extract the solution, And to backtrack to a particular decision level. + /// 2. `[current_decision_level..changed_this_decision_level]` Are packages that have **not** had there assignments + /// changed since the last time `prioritize` has bean called. Within this range there is no sorting. + /// 3. `[changed_this_decision_level..]` Containes all packages that **have** had there assignments changed since + /// the last time `prioritize` has bean called. The inverse is not necessarily true, some packages in the range + /// did not have a change. Within this range there is no sorting. + package_assignments: FnvIndexMap>, + /// `prioritized_potential_packages` is primarily a HashMap from a package with no desition and a positive assignment + /// to its `Priority`. But, it also maintains a max heap of packages by `Priority` order. + prioritized_potential_packages: PriorityQueue>, + changed_this_decision_level: usize, +} + +impl Display + for PartialSolution +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut assignments: Vec<_> = self + .package_assignments + .iter() + .map(|(p, pa)| format!("{}: {}", p, pa)) + .collect(); + assignments.sort(); + write!( + f, + "next_global_index: {}\ncurrent_decision_level: {:?}\npackage_assignements:\n{}", + self.next_global_index, + self.current_decision_level, + assignments.join("\t\n") + ) + } } /// Package assignments contain the potential decision and derivations /// that have already been made for a given package, /// as well as the intersection of terms by all of these. #[derive(Clone, Debug)] -struct PackageAssignments { +struct PackageAssignments { smallest_decision_level: DecisionLevel, highest_decision_level: DecisionLevel, - dated_derivations: SmallVec>, - assignments_intersection: AssignmentsIntersection, + dated_derivations: SmallVec>, + assignments_intersection: AssignmentsIntersection, +} + +impl Display for PackageAssignments { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let derivations: Vec<_> = self + .dated_derivations + .iter() + .map(|dd| dd.to_string()) + .collect(); + write!( + f, + "decision range: {:?}..{:?}\nderivations:\n {}\n,assignments_intersection: {}", + self.smallest_decision_level, + self.highest_decision_level, + derivations.join("\n "), + self.assignments_intersection + ) + } } #[derive(Clone, Debug)] -pub struct DatedDerivation { +pub struct DatedDerivation { global_index: u32, decision_level: DecisionLevel, - cause: IncompId, + cause: IncompId, +} + +impl Display for DatedDerivation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:?}, cause: {:?}", self.decision_level, self.cause) + } } #[derive(Clone, Debug)] -enum AssignmentsIntersection { - Decision((u32, V, Term)), - Derivations(Term), +enum AssignmentsIntersection { + Decision((u32, VS::V, Term)), + Derivations(Term), +} + +impl Display for AssignmentsIntersection { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Decision((lvl, version, _)) => { + write!(f, "Decision: level {}, v = {}", lvl, version) + } + Self::Derivations(term) => write!(f, "Derivations term: {}", term), + } + } } #[derive(Clone, Debug)] -pub enum SatisfierSearch { +pub enum SatisfierSearch { DifferentDecisionLevels { previous_satisfier_level: DecisionLevel, }, SameDecisionLevels { - satisfier_cause: IncompId, + satisfier_cause: IncompId, }, } -impl PartialSolution { +impl PartialSolution { /// Initialize an empty PartialSolution. pub fn empty() -> Self { Self { next_global_index: 0, current_decision_level: DecisionLevel(0), - package_assignments: Map::default(), + package_assignments: FnvIndexMap::default(), + prioritized_potential_packages: PriorityQueue::default(), + changed_this_decision_level: 0, } } /// Add a decision. - pub fn add_decision(&mut self, package: P, version: V) { + pub fn add_decision(&mut self, package: P, version: VS::V) { // Check that add_decision is never used in the wrong context. if cfg!(debug_assertions) { match self.package_assignments.get_mut(&package) { @@ -87,15 +165,26 @@ impl PartialSolution { AssignmentsIntersection::Decision(_) => panic!("Already existing decision"), // Cannot be called if the versions is not contained in the terms intersection. AssignmentsIntersection::Derivations(term) => { - debug_assert!(term.contains(&version)) + debug_assert!( + term.contains(&version), + "{}: {} was expected to be contained in {}", + package, + version, + term, + ) } }, } + assert_eq!( + self.changed_this_decision_level, + self.package_assignments.len() + ); } + let new_idx = self.current_decision_level.0 as usize; self.current_decision_level = self.current_decision_level.increment(); - let mut pa = self + let (old_idx, _, pa) = self .package_assignments - .get_mut(&package) + .get_full_mut(&package) .expect("Derivations must already exist"); pa.highest_decision_level = self.current_decision_level; pa.assignments_intersection = AssignmentsIntersection::Decision(( @@ -103,6 +192,10 @@ impl PartialSolution { version.clone(), Term::exact(version), )); + // Maintain that the beginning of the `package_assignments` Have all decisions in sorted order. + if new_idx != old_idx { + self.package_assignments.swap_indices(new_idx, old_idx); + } self.next_global_index += 1; } @@ -110,10 +203,10 @@ impl PartialSolution { pub fn add_derivation( &mut self, package: P, - cause: IncompId, - store: &Arena>, + cause: IncompId, + store: &Arena>, ) { - use std::collections::hash_map::Entry; + use indexmap::map::Entry; let term = store[cause].get(&package).unwrap().negate(); let dated_derivation = DatedDerivation { global_index: self.next_global_index, @@ -121,9 +214,11 @@ impl PartialSolution { cause, }; self.next_global_index += 1; + let pa_last_index = self.package_assignments.len().saturating_sub(1); match self.package_assignments.entry(package) { Entry::Occupied(mut occupied) => { - let mut pa = occupied.get_mut(); + let idx = occupied.index(); + let pa = occupied.get_mut(); pa.highest_decision_level = self.current_decision_level; match &mut pa.assignments_intersection { // Check that add_derivation is never called in the wrong context. @@ -132,11 +227,21 @@ impl PartialSolution { } AssignmentsIntersection::Derivations(t) => { *t = t.intersection(&term); + if t.is_positive() { + // we can use `swap_indices` to make `changed_this_decision_level` only go down by 1 + // but the copying is slower then the larger search + self.changed_this_decision_level = + std::cmp::min(self.changed_this_decision_level, idx); + } } } pa.dated_derivations.push(dated_derivation); } Entry::Vacant(v) => { + if term.is_positive() { + self.changed_this_decision_level = + std::cmp::min(self.changed_this_decision_level, pa_last_index); + } v.insert(PackageAssignments { smallest_decision_level: self.current_decision_level, highest_decision_level: self.current_decision_level, @@ -147,50 +252,55 @@ impl PartialSolution { } } - /// Extract potential packages for the next iteration of unit propagation. - /// Return `None` if there is no suitable package anymore, which stops the algorithm. - /// A package is a potential pick if there isn't an already - /// selected version (no "decision") - /// and if it contains at least one positive derivation term - /// in the partial solution. - pub fn potential_packages(&self) -> Option)>> { - let mut iter = self - .package_assignments + pub fn pick_highest_priority_pkg( + &mut self, + prioritizer: impl Fn(&P, &VS) -> Priority, + ) -> Option

{ + let check_all = self.changed_this_decision_level + == self.current_decision_level.0.saturating_sub(1) as usize; + let current_decision_level = self.current_decision_level; + let prioritized_potential_packages = &mut self.prioritized_potential_packages; + self.package_assignments + .get_range(self.changed_this_decision_level..) + .unwrap() .iter() + .filter(|(_, pa)| { + // We only actually need to update the package if its Been changed + // since the last time we called prioritize. + // Which means it's highest decision level is the current decision level, + // or if we backtracked in the mean time. + check_all || pa.highest_decision_level == current_decision_level + }) .filter_map(|(p, pa)| pa.assignments_intersection.potential_package_filter(p)) - .peekable(); - if iter.peek().is_some() { - Some(iter) - } else { - None - } + .for_each(|(p, r)| { + let priority = prioritizer(p, r); + prioritized_potential_packages.push(p.clone(), priority); + }); + self.changed_this_decision_level = self.package_assignments.len(); + prioritized_potential_packages.pop().map(|(p, _)| p) } /// If a partial solution has, for every positive derivation, /// a corresponding decision that satisfies that assignment, /// it's a total solution and version solving has succeeded. - pub fn extract_solution(&self) -> Option> { - let mut solution = Map::default(); - for (p, pa) in &self.package_assignments { - match &pa.assignments_intersection { - AssignmentsIntersection::Decision((_, v, _)) => { - solution.insert(p.clone(), v.clone()); - } - AssignmentsIntersection::Derivations(term) => { - if term.is_positive() { - return None; - } + pub fn extract_solution(&self) -> SelectedDependencies { + self.package_assignments + .iter() + .take(self.current_decision_level.0 as usize) + .map(|(p, pa)| match &pa.assignments_intersection { + AssignmentsIntersection::Decision((_, v, _)) => (p.clone(), v.clone()), + AssignmentsIntersection::Derivations(_) => { + panic!("Derivations in the Decision part") } - } - } - Some(solution) + }) + .collect() } /// Backtrack the partial solution to a given decision level. pub fn backtrack( &mut self, decision_level: DecisionLevel, - store: &Arena>, + store: &Arena>, ) { self.current_decision_level = decision_level; self.package_assignments.retain(|p, pa| { @@ -223,13 +333,16 @@ impl PartialSolution { pa.dated_derivations .iter() .fold(Term::any(), |acc, dated_derivation| { - let term = store[dated_derivation.cause].get(&p).unwrap().negate(); + let term = store[dated_derivation.cause].get(p).unwrap().negate(); acc.intersection(&term) }), ); true } }); + // Throw away all stored priority levels, And mark that they all need to be recomputed. + self.prioritized_potential_packages.clear(); + self.changed_this_decision_level = self.current_decision_level.0.saturating_sub(1) as usize; } /// We can add the version to the partial solution as a decision @@ -240,12 +353,12 @@ impl PartialSolution { pub fn add_version( &mut self, package: P, - version: V, - new_incompatibilities: std::ops::Range>, - store: &Arena>, + version: VS::V, + new_incompatibilities: std::ops::Range>, + store: &Arena>, ) { let exact = Term::exact(version.clone()); - let not_satisfied = |incompat: &Incompatibility| { + let not_satisfied = |incompat: &Incompatibility| { incompat.relation(|p| { if p == &package { Some(&exact) @@ -258,17 +371,24 @@ impl PartialSolution { // Check none of the dependencies (new_incompatibilities) // would create a conflict (be satisfied). if store[new_incompatibilities].iter().all(not_satisfied) { + log::info!("add_decision: {} @ {}", package, version); self.add_decision(package, version); + } else { + log::info!( + "not adding {} @ {} because of its dependencies", + package, + version + ); } } /// Check if the terms in the partial solution satisfy the incompatibility. - pub fn relation(&self, incompat: &Incompatibility) -> Relation

{ + pub fn relation(&self, incompat: &Incompatibility) -> Relation

{ incompat.relation(|package| self.term_intersection_for_package(package)) } /// Retrieve intersection of terms related to package. - pub fn term_intersection_for_package(&self, package: &P) -> Option<&Term> { + pub fn term_intersection_for_package(&self, package: &P) -> Option<&Term> { self.package_assignments .get(package) .map(|pa| pa.assignments_intersection.term()) @@ -277,9 +397,9 @@ impl PartialSolution { /// Figure out if the satisfier and previous satisfier are of different decision levels. pub fn satisfier_search( &self, - incompat: &Incompatibility, - store: &Arena>, - ) -> (P, SatisfierSearch) { + incompat: &Incompatibility, + store: &Arena>, + ) -> (P, SatisfierSearch) { let satisfied_map = Self::find_satisfier(incompat, &self.package_assignments, store); let (satisfier_package, &(satisfier_index, _, satisfier_decision_level)) = satisfied_map .iter() @@ -318,9 +438,9 @@ impl PartialSolution { /// It would be nice if we could get rid of it, but I don't know if then it will be possible /// to return a coherent previous_satisfier_level. fn find_satisfier( - incompat: &Incompatibility, - package_assignments: &Map>, - store: &Arena>, + incompat: &Incompatibility, + package_assignments: &FnvIndexMap>, + store: &Arena>, ) -> SmallMap { let mut satisfied = SmallMap::Empty; for (package, incompat_term) in incompat.iter() { @@ -337,11 +457,11 @@ impl PartialSolution { /// such that incompatibility is satisfied by the partial solution up to /// and including that assignment plus satisfier. fn find_previous_satisfier( - incompat: &Incompatibility, + incompat: &Incompatibility, satisfier_package: &P, mut satisfied_map: SmallMap, - package_assignments: &Map>, - store: &Arena>, + package_assignments: &FnvIndexMap>, + store: &Arena>, ) -> DecisionLevel { // First, let's retrieve the previous derivations and the initial accum_term. let satisfier_pa = package_assignments.get(satisfier_package).unwrap(); @@ -375,13 +495,13 @@ impl PartialSolution { } } -impl PackageAssignments { +impl PackageAssignments { fn satisfier( &self, package: &P, - incompat_term: &Term, - start_term: Term, - store: &Arena>, + incompat_term: &Term, + start_term: Term, + store: &Arena>, ) -> (usize, u32, DecisionLevel) { // Term where we accumulate intersections until incompat_term is satisfied. let mut accum_term = start_term; @@ -407,15 +527,24 @@ impl PackageAssignments { self.highest_decision_level, ), AssignmentsIntersection::Derivations(_) => { - panic!("This must be a decision") + unreachable!( + concat!( + "while processing package {}: ", + "accum_term = {} isn't a subset of incompat_term = {}, ", + "which means the last assignment should have been a decision, ", + "but instead it was a derivation. This shouldn't be possible! ", + "(Maybe your Version ordering is broken?)" + ), + package, accum_term, incompat_term + ) } } } } -impl AssignmentsIntersection { +impl AssignmentsIntersection { /// Returns the term intersection of all assignments (decision included). - fn term(&self) -> &Term { + fn term(&self) -> &Term { match self { Self::Decision((_, _, term)) => term, Self::Derivations(term) => term, @@ -429,7 +558,7 @@ impl AssignmentsIntersection { fn potential_package_filter<'a, P: Package>( &'a self, package: &'a P, - ) -> Option<(&'a P, &'a Range)> { + ) -> Option<(&'a P, &'a VS)> { match self { Self::Decision(_) => None, Self::Derivations(term_intersection) => { diff --git a/src/internal/small_vec.rs b/src/internal/small_vec.rs index 2c3fe4f4..fa720435 100644 --- a/src/internal/small_vec.rs +++ b/src/internal/small_vec.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::hash::{Hash, Hasher}; use std::ops::Deref; #[derive(Clone)] @@ -108,6 +109,13 @@ impl fmt::Debug for SmallVec { } } +impl Hash for SmallVec { + fn hash(&self, state: &mut H) { + self.len().hash(state); + Hash::hash_slice(self.as_slice(), state); + } +} + #[cfg(feature = "serde")] impl serde::Serialize for SmallVec { fn serialize(&self, s: S) -> Result { @@ -118,13 +126,70 @@ impl serde::Serialize for SmallVec { #[cfg(feature = "serde")] impl<'de, T: serde::Deserialize<'de>> serde::Deserialize<'de> for SmallVec { fn deserialize>(d: D) -> Result { - let items: Vec = serde::Deserialize::deserialize(d)?; + struct SmallVecVisitor { + marker: std::marker::PhantomData, + } + + impl<'de, T> serde::de::Visitor<'de> for SmallVecVisitor + where + T: serde::Deserialize<'de>, + { + type Value = SmallVec; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a sequence") + } + + fn visit_seq(self, mut seq: A) -> Result + where + A: serde::de::SeqAccess<'de>, + { + let mut values = SmallVec::empty(); + while let Some(value) = seq.next_element()? { + values.push(value); + } + Ok(values) + } + } + + let visitor = SmallVecVisitor { + marker: Default::default(), + }; + d.deserialize_seq(visitor) + } +} + +impl IntoIterator for SmallVec { + type Item = T; + type IntoIter = SmallVecIntoIter; - let mut v = Self::empty(); - for item in items { - v.push(item); + fn into_iter(self) -> Self::IntoIter { + match self { + SmallVec::Empty => SmallVecIntoIter::Empty, + SmallVec::One(a) => SmallVecIntoIter::One(a.into_iter()), + SmallVec::Two(a) => SmallVecIntoIter::Two(a.into_iter()), + SmallVec::Flexible(v) => SmallVecIntoIter::Flexible(v.into_iter()), + } + } +} + +pub enum SmallVecIntoIter { + Empty, + One(<[T; 1] as IntoIterator>::IntoIter), + Two(<[T; 2] as IntoIterator>::IntoIter), + Flexible( as IntoIterator>::IntoIter), +} + +impl Iterator for SmallVecIntoIter { + type Item = T; + + fn next(&mut self) -> Option { + match self { + SmallVecIntoIter::Empty => None, + SmallVecIntoIter::One(it) => it.next(), + SmallVecIntoIter::Two(it) => it.next(), + SmallVecIntoIter::Flexible(it) => it.next(), } - Ok(v) } } @@ -137,11 +202,11 @@ pub mod tests { proptest! { #[test] - fn push_and_pop(comands: Vec>) { + fn push_and_pop(commands: Vec>) { let mut v = vec![]; let mut sv = SmallVec::Empty; - for comand in comands { - match comand { + for command in commands { + match command { Some(i) => { v.push(i); sv.push(i); diff --git a/src/lib.rs b/src/lib.rs index 7a6e1737..5f61fb51 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -49,15 +49,17 @@ //! # use pubgrub::solver::{OfflineDependencyProvider, resolve}; //! # use pubgrub::version::NumberVersion; //! # use pubgrub::range::Range; -//! # -//! let mut dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); +//! +//! type NumVS = Range; +//! +//! let mut dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); //! //! dependency_provider.add_dependencies( -//! "root", 1, vec![("menu", Range::any()), ("icons", Range::any())], +//! "root", 1, [("menu", Range::full()), ("icons", Range::full())], //! ); -//! dependency_provider.add_dependencies("menu", 1, vec![("dropdown", Range::any())]); -//! dependency_provider.add_dependencies("dropdown", 1, vec![("icons", Range::any())]); -//! dependency_provider.add_dependencies("icons", 1, vec![]); +//! dependency_provider.add_dependencies("menu", 1, [("dropdown", Range::full())]); +//! dependency_provider.add_dependencies("dropdown", 1, [("icons", Range::full())]); +//! dependency_provider.add_dependencies("icons", 1, []); //! //! // Run the algorithm. //! let solution = resolve(&dependency_provider, "root", 1).unwrap(); @@ -73,7 +75,7 @@ //! trait for our own type. //! Let's say that we will use [String] for packages, //! and [SemanticVersion](version::SemanticVersion) for versions. -//! This may be done quite easily by implementing the two following functions. +//! This may be done quite easily by implementing the three following functions. //! ``` //! # use pubgrub::solver::{DependencyProvider, Dependencies}; //! # use pubgrub::version::SemanticVersion; @@ -84,8 +86,15 @@ //! # //! # struct MyDependencyProvider; //! # -//! impl DependencyProvider for MyDependencyProvider { -//! fn choose_package_version, U: Borrow>>(&self,packages: impl Iterator) -> Result<(T, Option), Box> { +//! type SemVS = Range; +//! +//! impl DependencyProvider for MyDependencyProvider { +//! fn choose_version(&self, package: &String, range: &SemVS) -> Result, Box> { +//! unimplemented!() +//! } +//! +//! type Priority = usize; +//! fn prioritize(&self, package: &String, range: &SemVS) -> Self::Priority { //! unimplemented!() //! } //! @@ -93,25 +102,24 @@ //! &self, //! package: &String, //! version: &SemanticVersion, -//! ) -> Result, Box> { +//! ) -> Result, Box> { //! unimplemented!() //! } //! } //! ``` //! //! The first method -//! [choose_package_version](crate::solver::DependencyProvider::choose_package_version) -//! chooses a package and available version compatible with the provided options. -//! A helper function -//! [choose_package_with_fewest_versions](crate::solver::choose_package_with_fewest_versions) -//! is provided for convenience -//! in cases when lists of available versions for packages are easily obtained. -//! The strategy of that helper function consists in choosing the package -//! with the fewest number of compatible versions to speed up resolution. +//! [choose_version](crate::solver::DependencyProvider::choose_version) +//! chooses a version compatible with the provided range for a package. +//! The second method +//! [prioritize](crate::solver::DependencyProvider::prioritize) +//! in which order different packages should be chosen. +//! Usually prioritizing packages +//! with the fewest number of compatible versions speeds up resolution. //! But in general you are free to employ whatever strategy suits you best //! to pick a package and a version. //! -//! The second method [get_dependencies](crate::solver::DependencyProvider::get_dependencies) +//! The third method [get_dependencies](crate::solver::DependencyProvider::get_dependencies) //! aims at retrieving the dependencies of a given package at a given version. //! Returns [None] if dependencies are unknown. //! @@ -153,13 +161,13 @@ //! [Output](crate::report::Reporter::Output) type and a single method. //! ``` //! # use pubgrub::package::Package; -//! # use pubgrub::version::Version; +//! # use pubgrub::version_set::VersionSet; //! # use pubgrub::report::DerivationTree; //! # -//! pub trait Reporter { +//! pub trait Reporter { //! type Output; //! -//! fn report(derivation_tree: &DerivationTree) -> Self::Output; +//! fn report(derivation_tree: &DerivationTree) -> Self::Output; //! } //! ``` //! Implementing a [Reporter](crate::report::Reporter) may involve a lot of heuristics @@ -173,8 +181,11 @@ //! # use pubgrub::report::{DefaultStringReporter, Reporter}; //! # use pubgrub::error::PubGrubError; //! # use pubgrub::version::NumberVersion; +//! # use pubgrub::range::Range; +//! # +//! # type NumVS = Range; //! # -//! # let dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); +//! # let dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); //! # let root_package = "root"; //! # let root_version = 1; //! # @@ -217,5 +228,6 @@ pub mod solver; pub mod term; pub mod type_aliases; pub mod version; +pub mod version_set; mod internal; diff --git a/src/package.rs b/src/package.rs index e36b91ed..36c6069e 100644 --- a/src/package.rs +++ b/src/package.rs @@ -2,16 +2,16 @@ //! Trait for identifying packages. //! Automatically implemented for traits implementing -//! [Clone] + [Eq] + [Hash] + [Debug] + [Display](std::fmt::Display). +//! [Clone] + [Eq] + [Hash] + [Debug] + [Display]. use std::fmt::{Debug, Display}; use std::hash::Hash; /// Trait for identifying packages. /// Automatically implemented for types already implementing -/// [Clone] + [Eq] + [Hash] + [Debug] + [Display](std::fmt::Display). +/// [Clone] + [Eq] + [Hash] + [Debug] + [Display]. pub trait Package: Clone + Eq + Hash + Debug + Display {} /// Automatically implement the Package trait for any type -/// that already implement [Clone] + [Eq] + [Hash] + [Debug] + [Display](std::fmt::Display). +/// that already implement [Clone] + [Eq] + [Hash] + [Debug] + [Display]. impl Package for T {} diff --git a/src/range.rs b/src/range.rs index 8de8b3ff..be7ff250 100644 --- a/src/range.rs +++ b/src/range.rs @@ -7,287 +7,416 @@ //! of the ranges building blocks. //! //! Those building blocks are: -//! - [none()](Range::none): the empty set -//! - [any()](Range::any): the set of all possible versions -//! - [exact(v)](Range::exact): the set containing only the version v +//! - [empty()](Range::empty): the empty set +//! - [full()](Range::full): the set of all possible versions +//! - [singleton(v)](Range::singleton): the set containing only the version v //! - [higher_than(v)](Range::higher_than): the set defined by `v <= versions` +//! - [strictly_higher_than(v)](Range::strictly_higher_than): the set defined by `v < versions` +//! - [lower_than(v)](Range::lower_than): the set defined by `versions <= v` //! - [strictly_lower_than(v)](Range::strictly_lower_than): the set defined by `versions < v` //! - [between(v1, v2)](Range::between): the set defined by `v1 <= versions < v2` - -use std::cmp::Ordering; -use std::fmt; - -use crate::internal::small_vec::SmallVec; -use crate::version::Version; - -/// A Range is a set of versions. -#[derive(Debug, Clone, Eq, PartialEq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +//! +//! Ranges can be created from any type that implements [`Ord`] + [`Clone`]. +//! +//! In order to advance the solver front, comparisons of versions sets are necessary in the algorithm. +//! To do those comparisons between two sets S1 and S2 we use the mathematical property that S1 ⊂ S2 if and only if S1 ∩ S2 == S1. +//! We can thus compute an intersection and evaluate an equality to answer if S1 is a subset of S2. +//! But this means that the implementation of equality must be correct semantically. +//! In practice, if equality is derived automatically, this means sets must have unique representations. +//! +//! By migrating from a custom representation for discrete sets in v0.2 +//! to a generic bounded representation for continuous sets in v0.3 +//! we are potentially breaking that assumption in two ways: +//! +//! 1. Minimal and maximal `Unbounded` values can be replaced by their equivalent if it exists. +//! 2. Simplifying adjacent bounds of discrete sets cannot be detected and automated in the generic intersection code. +//! +//! An example for each can be given when `T` is `u32`. +//! First, we can have both segments `S1 = (Unbounded, Included(42u32))` and `S2 = (Included(0), Included(42u32))` +//! that represent the same segment but are structurally different. +//! Thus, a derived equality check would answer `false` to `S1 == S2` while it's true. +//! +//! Second both segments `S1 = (Included(1), Included(5))` and `S2 = (Included(1), Included(3)) + (Included(4), Included(5))` are equal. +//! But without asking the user to provide a `bump` function for discrete sets, +//! the algorithm is not able tell that the space between the right `Included(3)` bound and the left `Included(4)` bound is empty. +//! Thus the algorithm is not able to reduce S2 to its canonical S1 form while computing sets operations like intersections in the generic code. +//! +//! This is likely to lead to user facing theoretically correct but practically nonsensical ranges, +//! like (Unbounded, Excluded(0)) or (Excluded(6), Excluded(7)). +//! In general nonsensical inputs often lead to hard to track bugs. +//! But as far as we can tell this should work in practice. +//! So for now this crate only provides an implementation for continuous ranges. +//! With the v0.3 api the user could choose to bring back the discrete implementation from v0.2, as documented in the guide. +//! If doing so regularly fixes bugs seen by users, we will bring it back into the core library. +//! If we do not see practical bugs, or we get a formal proof that the code cannot lead to error states, then we may remove this warning. + +use crate::{internal::small_vec::SmallVec, version_set::VersionSet}; +use std::ops::RangeBounds; +use std::{ + fmt::{Debug, Display, Formatter}, + ops::Bound::{self, Excluded, Included, Unbounded}, +}; + +/// A Range represents multiple intervals of a continuous range of monotone increasing +/// values. +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(feature = "serde", serde(transparent))] -pub struct Range { +pub struct Range { segments: SmallVec>, } -type Interval = (V, Option); +type Interval = (Bound, Bound); -// Range building blocks. -impl Range { +impl Range { /// Empty set of versions. - pub fn none() -> Self { + pub fn empty() -> Self { Self { segments: SmallVec::empty(), } } - /// Set of all possible versions. - pub fn any() -> Self { - Self::higher_than(V::lowest()) + /// Set of all possible versions + pub fn full() -> Self { + Self { + segments: SmallVec::one((Unbounded, Unbounded)), + } } - /// Set containing exactly one version. - pub fn exact(v: impl Into) -> Self { - let v = v.into(); + /// Set of all versions higher or equal to some version + pub fn higher_than(v: impl Into) -> Self { Self { - segments: SmallVec::one((v.clone(), Some(v.bump()))), + segments: SmallVec::one((Included(v.into()), Unbounded)), } } - /// Set of all versions higher or equal to some version. - pub fn higher_than(v: impl Into) -> Self { + /// Set of all versions higher to some version + pub fn strictly_higher_than(v: impl Into) -> Self { Self { - segments: SmallVec::one((v.into(), None)), + segments: SmallVec::one((Excluded(v.into()), Unbounded)), } } - /// Set of all versions strictly lower than some version. + /// Set of all versions lower to some version pub fn strictly_lower_than(v: impl Into) -> Self { - let v = v.into(); - if v == V::lowest() { - Self::none() - } else { - Self { - segments: SmallVec::one((V::lowest(), Some(v))), - } + Self { + segments: SmallVec::one((Unbounded, Excluded(v.into()))), } } - /// Set of all versions comprised between two given versions. - /// The lower bound is included and the higher bound excluded. - /// `v1 <= v < v2`. + /// Set of all versions lower or equal to some version + pub fn lower_than(v: impl Into) -> Self { + Self { + segments: SmallVec::one((Unbounded, Included(v.into()))), + } + } + + /// Set of versions greater or equal to `v1` but less than `v2`. pub fn between(v1: impl Into, v2: impl Into) -> Self { - let v1 = v1.into(); - let v2 = v2.into(); - if v1 < v2 { - Self { - segments: SmallVec::one((v1, Some(v2))), - } - } else { - Self::none() + Self { + segments: SmallVec::one((Included(v1.into()), Excluded(v2.into()))), } } } -// Set operations. -impl Range { - // Negate ################################################################## +impl Range { + /// Set containing exactly one version + pub fn singleton(v: impl Into) -> Self { + let v = v.into(); + Self { + segments: SmallVec::one((Included(v.clone()), Included(v))), + } + } - /// Compute the complement set of versions. - pub fn negate(&self) -> Self { + /// Returns the complement of this Range. + pub fn complement(&self) -> Self { match self.segments.first() { - None => Self::any(), // Complement of ∅ is * + // Complement of ∅ is ∞ + None => Self::full(), + + // Complement of ∞ is ∅ + Some((Unbounded, Unbounded)) => Self::empty(), // First high bound is +∞ - Some((v, None)) => { - // Complement of * is ∅ - if v == &V::lowest() { - Self::none() - // Complement of "v <= _" is "_ < v" - } else { - Self::strictly_lower_than(v.clone()) - } - } + Some((Included(v), Unbounded)) => Self::strictly_lower_than(v.clone()), + Some((Excluded(v), Unbounded)) => Self::lower_than(v.clone()), - // First high bound is not +∞ - Some((v1, Some(v2))) => { - if v1 == &V::lowest() { - Self::negate_segments(v2.clone(), &self.segments[1..]) - } else { - Self::negate_segments(V::lowest(), &self.segments) - } + Some((Unbounded, Included(v))) => { + Self::negate_segments(Excluded(v.clone()), &self.segments[1..]) } + Some((Unbounded, Excluded(v))) => { + Self::negate_segments(Included(v.clone()), &self.segments[1..]) + } + Some((Included(_), Included(_))) + | Some((Included(_), Excluded(_))) + | Some((Excluded(_), Included(_))) + | Some((Excluded(_), Excluded(_))) => Self::negate_segments(Unbounded, &self.segments), } } /// Helper function performing the negation of intervals in segments. - /// For example: - /// [ (v1, None) ] => [ (start, Some(v1)) ] - /// [ (v1, Some(v2)) ] => [ (start, Some(v1)), (v2, None) ] - fn negate_segments(start: V, segments: &[Interval]) -> Range { - let mut complement_segments = SmallVec::empty(); - let mut start = Some(start); - for (v1, maybe_v2) in segments { - // start.unwrap() is fine because `segments` is not exposed, - // and our usage guaranties that only the last segment may contain a None. - complement_segments.push((start.unwrap(), Some(v1.to_owned()))); - start = maybe_v2.to_owned(); - } - if let Some(last) = start { - complement_segments.push((last, None)); + fn negate_segments(start: Bound, segments: &[Interval]) -> Self { + let mut complement_segments: SmallVec> = SmallVec::empty(); + let mut start = start; + for (v1, v2) in segments { + complement_segments.push(( + start, + match v1 { + Included(v) => Excluded(v.clone()), + Excluded(v) => Included(v.clone()), + Unbounded => unreachable!(), + }, + )); + start = match v2 { + Included(v) => Excluded(v.clone()), + Excluded(v) => Included(v.clone()), + Unbounded => Unbounded, + } + } + if !matches!(start, Unbounded) { + complement_segments.push((start, Unbounded)); } Self { segments: complement_segments, } } +} - // Union and intersection ################################################## - - /// Compute the union of two sets of versions. - pub fn union(&self, other: &Self) -> Self { - self.negate().intersection(&other.negate()).negate() +impl Range { + /// Convert to something that can be used with + /// [BTreeMap::range](std::collections::BTreeMap::range). + /// All versions contained in self, will be in the output, + /// but there may be versions in the output that are not contained in self. + /// Returns None if the range is empty. + pub fn bounding_range(&self) -> Option<(Bound<&V>, Bound<&V>)> { + self.segments.first().map(|(start, _)| { + let end = self + .segments + .last() + .expect("if there is a first element, there must be a last element"); + (start.as_ref(), end.1.as_ref()) + }) } - /// Compute the intersection of two sets of versions. - pub fn intersection(&self, other: &Self) -> Self { - let mut segments = SmallVec::empty(); - let mut left_iter = self.segments.iter(); - let mut right_iter = other.segments.iter(); - let mut left = left_iter.next(); - let mut right = right_iter.next(); - loop { - match (left, right) { - // Both left and right still contain a finite interval: - (Some((l1, Some(l2))), Some((r1, Some(r2)))) => { - if l2 <= r1 { - // Intervals are disjoint, progress on the left. - left = left_iter.next(); - } else if r2 <= l1 { - // Intervals are disjoint, progress on the right. - right = right_iter.next(); - } else { - // Intervals are not disjoint. - let start = l1.max(r1).to_owned(); - if l2 < r2 { - segments.push((start, Some(l2.to_owned()))); - left = left_iter.next(); - } else { - segments.push((start, Some(r2.to_owned()))); - right = right_iter.next(); - } - } - } - - // Right contains an infinite interval: - (Some((l1, Some(l2))), Some((r1, None))) => match l2.cmp(r1) { - Ordering::Less => { - left = left_iter.next(); - } - Ordering::Equal => { - for l in left_iter.cloned() { - segments.push(l) - } - break; - } - Ordering::Greater => { - let start = l1.max(r1).to_owned(); - segments.push((start, Some(l2.to_owned()))); - for l in left_iter.cloned() { - segments.push(l) - } - break; - } - }, - - // Left contains an infinite interval: - (Some((l1, None)), Some((r1, Some(r2)))) => match r2.cmp(l1) { - Ordering::Less => { - right = right_iter.next(); - } - Ordering::Equal => { - for r in right_iter.cloned() { - segments.push(r) - } - break; - } - Ordering::Greater => { - let start = l1.max(r1).to_owned(); - segments.push((start, Some(r2.to_owned()))); - for r in right_iter.cloned() { - segments.push(r) - } - break; - } - }, + /// Returns true if the this Range contains the specified value. + pub fn contains(&self, v: &V) -> bool { + for segment in self.segments.iter() { + if match segment { + (Unbounded, Unbounded) => true, + (Unbounded, Included(end)) => v <= end, + (Unbounded, Excluded(end)) => v < end, + (Included(start), Unbounded) => v >= start, + (Included(start), Included(end)) => v >= start && v <= end, + (Included(start), Excluded(end)) => v >= start && v < end, + (Excluded(start), Unbounded) => v > start, + (Excluded(start), Included(end)) => v > start && v <= end, + (Excluded(start), Excluded(end)) => v > start && v < end, + } { + return true; + } + } + false + } - // Both sides contain an infinite interval: - (Some((l1, None)), Some((r1, None))) => { - let start = l1.max(r1).to_owned(); - segments.push((start, None)); - break; - } + /// Construct a simple range from anything that impls [RangeBounds] like `v1..v2`. + pub fn from_range_bounds(bounds: R) -> Self + where + R: RangeBounds, + IV: Clone + Into, + { + let start = match bounds.start_bound() { + Included(v) => Included(v.clone().into()), + Excluded(v) => Excluded(v.clone().into()), + Unbounded => Unbounded, + }; + let end = match bounds.end_bound() { + Included(v) => Included(v.clone().into()), + Excluded(v) => Excluded(v.clone().into()), + Unbounded => Unbounded, + }; + if valid_segment(&start, &end) { + Self { + segments: SmallVec::one((start, end)), + } + } else { + Self::empty() + } + } - // Left or right has ended. - _ => { - break; + fn check_invariants(self) -> Self { + if cfg!(debug_assertions) { + for p in self.segments.as_slice().windows(2) { + match (&p[0].1, &p[1].0) { + (Included(l_end), Included(r_start)) => assert!(l_end < r_start), + (Included(l_end), Excluded(r_start)) => assert!(l_end < r_start), + (Excluded(l_end), Included(r_start)) => assert!(l_end < r_start), + (Excluded(l_end), Excluded(r_start)) => assert!(l_end <= r_start), + (_, Unbounded) => panic!(), + (Unbounded, _) => panic!(), } } + for (s, e) in self.segments.iter() { + assert!(valid_segment(s, e)); + } } + self + } +} - Self { segments } +fn valid_segment(start: &Bound, end: &Bound) -> bool { + match (start, end) { + (Included(s), Included(e)) => s <= e, + (Included(s), Excluded(e)) => s < e, + (Excluded(s), Included(e)) => s < e, + (Excluded(s), Excluded(e)) => s < e, + (Unbounded, _) | (_, Unbounded) => true, } } -// Other useful functions. -impl Range { - /// Check if a range contains a given version. - pub fn contains(&self, version: &V) -> bool { - for (v1, maybe_v2) in &self.segments { - match maybe_v2 { - None => return v1 <= version, - Some(v2) => { - if version < v1 { - return false; - } else if version < v2 { - return true; - } - } +impl Range { + /// Computes the union of this `Range` and another. + pub fn union(&self, other: &Self) -> Self { + self.complement() + .intersection(&other.complement()) + .complement() + .check_invariants() + } + + /// Computes the intersection of two sets of versions. + pub fn intersection(&self, other: &Self) -> Self { + let mut segments: SmallVec> = SmallVec::empty(); + let mut left_iter = self.segments.iter().peekable(); + let mut right_iter = other.segments.iter().peekable(); + + while let (Some((left_start, left_end)), Some((right_start, right_end))) = + (left_iter.peek(), right_iter.peek()) + { + let start = match (left_start, right_start) { + (Included(l), Included(r)) => Included(std::cmp::max(l, r)), + (Excluded(l), Excluded(r)) => Excluded(std::cmp::max(l, r)), + + (Included(i), Excluded(e)) | (Excluded(e), Included(i)) if i <= e => Excluded(e), + (Included(i), Excluded(e)) | (Excluded(e), Included(i)) if e < i => Included(i), + (s, Unbounded) | (Unbounded, s) => s.as_ref(), + _ => unreachable!(), + } + .cloned(); + let end = match (left_end, right_end) { + (Included(l), Included(r)) => Included(std::cmp::min(l, r)), + (Excluded(l), Excluded(r)) => Excluded(std::cmp::min(l, r)), + + (Included(i), Excluded(e)) | (Excluded(e), Included(i)) if i >= e => Excluded(e), + (Included(i), Excluded(e)) | (Excluded(e), Included(i)) if e > i => Included(i), + (s, Unbounded) | (Unbounded, s) => s.as_ref(), + _ => unreachable!(), + } + .cloned(); + left_iter.next_if(|(_, e)| e == &end); + right_iter.next_if(|(_, e)| e == &end); + if valid_segment(&start, &end) { + segments.push((start, end)) } } - false + + Self { segments }.check_invariants() + } +} + +impl VersionSet for Range { + type V = T; + + fn empty() -> Self { + Range::empty() + } + + fn singleton(v: Self::V) -> Self { + Range::singleton(v) + } + + fn complement(&self) -> Self { + Range::complement(self) } - /// Return the lowest version in the range (if there is one). - pub fn lowest_version(&self) -> Option { - self.segments.first().map(|(start, _)| start).cloned() + fn intersection(&self, other: &Self) -> Self { + Range::intersection(self, other) + } + + fn contains(&self, v: &Self::V) -> bool { + Range::contains(self, v) + } + + fn full() -> Self { + Range::full() + } + + fn union(&self, other: &Self) -> Self { + Range::union(self, other) } } // REPORT ###################################################################### -impl fmt::Display for Range { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self.segments.as_slice() { - [] => write!(f, "∅"), - [(start, None)] if start == &V::lowest() => write!(f, "∗"), - [(start, None)] => write!(f, "{} <= v", start), - [(start, Some(end))] if end == &start.bump() => write!(f, "{}", start), - [(start, Some(end))] if start == &V::lowest() => write!(f, "v < {}", end), - [(start, Some(end))] => write!(f, "{} <= v < {}", start, end), - more_than_one_interval => { - let string_intervals: Vec<_> = more_than_one_interval - .iter() - .map(interval_to_string) - .collect(); - write!(f, "{}", string_intervals.join(" ")) +impl Display for Range { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + if self.segments.is_empty() { + write!(f, "∅")?; + } else { + for (idx, segment) in self.segments.iter().enumerate() { + if idx > 0 { + write!(f, " | ")?; + } + match segment { + (Unbounded, Unbounded) => write!(f, "*")?, + (Unbounded, Included(v)) => write!(f, "<={v}")?, + (Unbounded, Excluded(v)) => write!(f, "<{v}")?, + (Included(v), Unbounded) => write!(f, ">={v}")?, + (Included(v), Included(b)) => { + if v == b { + write!(f, "{v}")? + } else { + write!(f, ">={v}, <={b}")? + } + } + (Included(v), Excluded(b)) => write!(f, ">={v}, <{b}")?, + (Excluded(v), Unbounded) => write!(f, ">{v}")?, + (Excluded(v), Included(b)) => write!(f, ">{v}, <={b}")?, + (Excluded(v), Excluded(b)) => write!(f, ">{v}, <{b}")?, + }; } } + Ok(()) } } -fn interval_to_string((start, maybe_end): &Interval) -> String { - match maybe_end { - Some(end) => format!("[ {}, {} [", start, end), - None => format!("[ {}, ∞ [", start), +// SERIALIZATION ############################################################### + +#[cfg(feature = "serde")] +impl<'de, V: serde::Deserialize<'de>> serde::Deserialize<'de> for Range { + fn deserialize>(deserializer: D) -> Result { + // This enables conversion from the "old" discrete implementation of `Range` to the new + // bounded one. + // + // Serialization is always performed in the new format. + #[derive(serde::Deserialize)] + #[serde(untagged)] + enum EitherInterval { + B(Bound, Bound), + D(V, Option), + } + + let bounds: SmallVec> = serde::Deserialize::deserialize(deserializer)?; + + let mut segments = SmallVec::Empty; + for i in bounds { + match i { + EitherInterval::B(l, r) => segments.push((l, r)), + EitherInterval::D(l, Some(r)) => segments.push((Included(l), Excluded(r))), + EitherInterval::D(l, None) => segments.push((Included(l), Unbounded)), + } + } + + Ok(Range { segments }) } } @@ -297,28 +426,74 @@ fn interval_to_string((start, maybe_end): &Interval) -> String { pub mod tests { use proptest::prelude::*; - use crate::version::NumberVersion; - use super::*; - pub fn strategy() -> impl Strategy> { - prop::collection::vec(any::(), 0..10).prop_map(|mut vec| { - vec.sort_unstable(); - vec.dedup(); - let mut pair_iter = vec.chunks_exact(2); - let mut segments = SmallVec::empty(); - while let Some([v1, v2]) = pair_iter.next() { - segments.push((NumberVersion(*v1), Some(NumberVersion(*v2)))); - } - if let [v] = pair_iter.remainder() { - segments.push((NumberVersion(*v), None)); - } - Range { segments } - }) + /// Generate version sets from a random vector of deltas between bounds. + /// Each bound is randomly inclusive or exclusive. + pub fn strategy() -> impl Strategy> { + ( + any::(), + prop::collection::vec(any::<(u32, bool)>(), 1..10), + ) + .prop_map(|(start_unbounded, deltas)| { + let mut start = if start_unbounded { + Some(Unbounded) + } else { + None + }; + let mut largest: u32 = 0; + let mut last_bound_was_inclusive = false; + let mut segments = SmallVec::Empty; + for (delta, inclusive) in deltas { + // Add the offset to the current bound + largest = match largest.checked_add(delta) { + Some(s) => s, + None => { + // Skip this offset, if it would result in a too large bound. + continue; + } + }; + + let current_bound = if inclusive { + Included(largest) + } else { + Excluded(largest) + }; + + // If we already have a start bound, the next offset defines the complete range. + // If we don't have a start bound, we have to generate one. + if let Some(start_bound) = start.take() { + // If the delta from the start bound is 0, the only authorized configuration is + // Included(x), Included(x) + if delta == 0 && !(matches!(start_bound, Included(_)) && inclusive) { + start = Some(start_bound); + continue; + } + last_bound_was_inclusive = inclusive; + segments.push((start_bound, current_bound)); + } else { + // If the delta from the end bound of the last range is 0 and + // any of the last ending or current starting bound is inclusive, + // we skip the delta because they basically overlap. + if delta == 0 && (last_bound_was_inclusive || inclusive) { + continue; + } + start = Some(current_bound); + } + } + + // If we still have a start bound, but didn't have enough deltas to complete another + // segment, we add an unbounded upperbound. + if let Some(start_bound) = start { + segments.push((start_bound, Unbounded)); + } + + return Range { segments }.check_invariants(); + }) } - fn version_strat() -> impl Strategy { - any::().prop_map(NumberVersion) + fn version_strat() -> impl Strategy { + any::() } proptest! { @@ -327,17 +502,17 @@ pub mod tests { #[test] fn negate_is_different(range in strategy()) { - assert_ne!(range.negate(), range); + assert_ne!(range.complement(), range); } #[test] fn double_negate_is_identity(range in strategy()) { - assert_eq!(range.negate().negate(), range); + assert_eq!(range.complement().complement(), range); } #[test] fn negate_contains_opposite(range in strategy(), version in version_strat()) { - assert_ne!(range.contains(&version), range.negate().contains(&version)); + assert_ne!(range.contains(&version), range.complement().contains(&version)); } // Testing intersection ---------------------------- @@ -349,12 +524,12 @@ pub mod tests { #[test] fn intersection_with_any_is_identity(range in strategy()) { - assert_eq!(Range::any().intersection(&range), range); + assert_eq!(Range::full().intersection(&range), range); } #[test] fn intersection_with_none_is_none(range in strategy()) { - assert_eq!(Range::none().intersection(&range), Range::none()); + assert_eq!(Range::empty().intersection(&range), Range::empty()); } #[test] @@ -369,7 +544,7 @@ pub mod tests { #[test] fn intesection_of_complements_is_none(range in strategy()) { - assert_eq!(range.negate().intersection(&range), Range::none()); + assert_eq!(range.complement().intersection(&range), Range::empty()); } #[test] @@ -381,7 +556,7 @@ pub mod tests { #[test] fn union_of_complements_is_any(range in strategy()) { - assert_eq!(range.negate().union(&range), Range::any()); + assert_eq!(range.complement().union(&range), Range::full()); } #[test] @@ -393,17 +568,37 @@ pub mod tests { #[test] fn always_contains_exact(version in version_strat()) { - assert!(Range::exact(version).contains(&version)); + assert!(Range::singleton(version).contains(&version)); } #[test] fn contains_negation(range in strategy(), version in version_strat()) { - assert_ne!(range.contains(&version), range.negate().contains(&version)); + assert_ne!(range.contains(&version), range.complement().contains(&version)); } #[test] fn contains_intersection(range in strategy(), version in version_strat()) { - assert_eq!(range.contains(&version), range.intersection(&Range::exact(version)) != Range::none()); + assert_eq!(range.contains(&version), range.intersection(&Range::singleton(version)) != Range::empty()); + } + + #[test] + fn contains_bounding_range(range in strategy(), version in version_strat()) { + if range.contains(&version) { + assert!(range.bounding_range().map(|b| b.contains(&version)).unwrap_or(false)); + } + } + + #[test] + fn from_range_bounds(range in any::<(Bound, Bound)>(), version in version_strat()) { + let rv: Range<_> = Range::from_range_bounds(range); + assert_eq!(range.contains(&version), rv.contains(&version)); + } + + #[test] + fn from_range_bounds_round_trip(range in any::<(Bound, Bound)>()) { + let rv: Range = Range::from_range_bounds(range); + let rv2: Range = rv.bounding_range().map(Range::from_range_bounds::<_, u32>).unwrap_or_else(Range::empty); + assert_eq!(rv, rv2); } } } diff --git a/src/report.rs b/src/report.rs index 07dec364..ff0b2d3f 100644 --- a/src/report.rs +++ b/src/report.rs @@ -7,50 +7,49 @@ use std::fmt; use std::ops::{Deref, DerefMut}; use crate::package::Package; -use crate::range::Range; use crate::term::Term; use crate::type_aliases::Map; -use crate::version::Version; +use crate::version_set::VersionSet; /// Reporter trait. -pub trait Reporter { +pub trait Reporter { /// Output type of the report. type Output; /// Generate a report from the derivation tree /// describing the resolution failure. - fn report(derivation_tree: &DerivationTree) -> Self::Output; + fn report(derivation_tree: &DerivationTree) -> Self::Output; } /// Derivation tree resulting in the impossibility /// to solve the dependencies of our root package. #[derive(Debug, Clone)] -pub enum DerivationTree { +pub enum DerivationTree { /// External incompatibility. - External(External), + External(External), /// Incompatibility derived from two others. - Derived(Derived), + Derived(Derived), } /// Incompatibilities that are not derived from others, /// they have their own reason. #[derive(Debug, Clone)] -pub enum External { +pub enum External { /// Initial incompatibility aiming at picking the root package for the first decision. - NotRoot(P, V), - /// There are no versions in the given range for this package. - NoVersions(P, Range), - /// Dependencies of the package are unavailable for versions in that range. - UnavailableDependencies(P, Range), + NotRoot(P, VS::V), + /// There are no versions in the given set for this package. + NoVersions(P, VS), + /// Dependencies of the package are unavailable for versions in that set. + UnavailableDependencies(P, VS), /// Incompatibility coming from the dependencies of a given package. - FromDependencyOf(P, Range, P, Range), + FromDependencyOf(P, VS, P, VS), } /// Incompatibility derived from two others. #[derive(Debug, Clone)] -pub struct Derived { +pub struct Derived { /// Terms of the incompatibility. - pub terms: Map>, + pub terms: Map>, /// Indicate if that incompatibility is present multiple times /// in the derivation tree. /// If that is the case, it has a unique id, provided in that option. @@ -58,12 +57,12 @@ pub struct Derived { /// and refer to the explanation for the other times. pub shared_id: Option, /// First cause. - pub cause1: Box>, + pub cause1: Box>, /// Second cause. - pub cause2: Box>, + pub cause2: Box>, } -impl DerivationTree { +impl DerivationTree { /// Merge the [NoVersions](External::NoVersions) external incompatibilities /// with the other one they are matched with /// in a derived incompatibility. @@ -100,7 +99,7 @@ impl DerivationTree { } } - fn merge_no_versions(self, package: P, range: Range) -> Option { + fn merge_no_versions(self, package: P, set: VS) -> Option { match self { // TODO: take care of the Derived case. // Once done, we can remove the Option. @@ -109,19 +108,16 @@ impl DerivationTree { panic!("How did we end up with a NoVersions merged with a NotRoot?") } DerivationTree::External(External::NoVersions(_, r)) => Some(DerivationTree::External( - External::NoVersions(package, range.union(&r)), + External::NoVersions(package, set.union(&r)), )), - DerivationTree::External(External::UnavailableDependencies(_, r)) => { - Some(DerivationTree::External(External::UnavailableDependencies( - package, - range.union(&r), - ))) - } + DerivationTree::External(External::UnavailableDependencies(_, r)) => Some( + DerivationTree::External(External::UnavailableDependencies(package, set.union(&r))), + ), DerivationTree::External(External::FromDependencyOf(p1, r1, p2, r2)) => { if p1 == package { Some(DerivationTree::External(External::FromDependencyOf( p1, - r1.union(&range), + r1.union(&set), p2, r2, ))) @@ -130,7 +126,7 @@ impl DerivationTree { p1, r1, p2, - r2.union(&range), + r2.union(&set), ))) } } @@ -138,39 +134,39 @@ impl DerivationTree { } } -impl fmt::Display for External { +impl fmt::Display for External { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::NotRoot(package, version) => { write!(f, "we are solving dependencies of {} {}", package, version) } - Self::NoVersions(package, range) => { - if range == &Range::any() { + Self::NoVersions(package, set) => { + if set == &VS::full() { write!(f, "there is no available version for {}", package) } else { - write!(f, "there is no version of {} in {}", package, range) + write!(f, "there is no version of {} in {}", package, set) } } - Self::UnavailableDependencies(package, range) => { - if range == &Range::any() { + Self::UnavailableDependencies(package, set) => { + if set == &VS::full() { write!(f, "dependencies of {} are unavailable", package) } else { write!( f, "dependencies of {} at version {} are unavailable", - package, range + package, set ) } } - Self::FromDependencyOf(p, range_p, dep, range_dep) => { - if range_p == &Range::any() && range_dep == &Range::any() { + Self::FromDependencyOf(p, set_p, dep, set_dep) => { + if set_p == &VS::full() && set_dep == &VS::full() { write!(f, "{} depends on {}", p, dep) - } else if range_p == &Range::any() { - write!(f, "{} depends on {} {}", p, dep, range_dep) - } else if range_dep == &Range::any() { - write!(f, "{} {} depends on {}", p, range_p, dep) + } else if set_p == &VS::full() { + write!(f, "{} depends on {} {}", p, dep, set_dep) + } else if set_dep == &VS::full() { + write!(f, "{} {} depends on {}", p, set_p, dep) } else { - write!(f, "{} {} depends on {} {}", p, range_p, dep, range_dep) + write!(f, "{} {} depends on {} {}", p, set_p, dep, set_dep) } } } @@ -198,17 +194,17 @@ impl DefaultStringReporter { } } - fn build_recursive(&mut self, derived: &Derived) { + fn build_recursive(&mut self, derived: &Derived) { self.build_recursive_helper(derived); if let Some(id) = derived.shared_id { - if self.shared_with_ref.get(&id) == None { + if self.shared_with_ref.get(&id).is_none() { self.add_line_ref(); self.shared_with_ref.insert(id, self.ref_count); } }; } - fn build_recursive_helper(&mut self, current: &Derived) { + fn build_recursive_helper(&mut self, current: &Derived) { match (current.cause1.deref(), current.cause2.deref()) { (DerivationTree::External(external1), DerivationTree::External(external2)) => { // Simplest case, we just combine two external incompatibilities. @@ -264,7 +260,7 @@ impl DefaultStringReporter { // and finally conclude. (None, None) => { self.build_recursive(derived1); - if derived1.shared_id != None { + if derived1.shared_id.is_some() { self.lines.push("".into()); self.build_recursive(current); } else { @@ -285,11 +281,11 @@ impl DefaultStringReporter { /// /// The result will depend on the fact that the derived incompatibility /// has already been explained or not. - fn report_one_each( + fn report_one_each( &mut self, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map>, ) { match self.line_ref_of(derived.shared_id) { Some(ref_id) => self.lines.push(Self::explain_ref_and_external( @@ -303,11 +299,11 @@ impl DefaultStringReporter { } /// Report one derived (without a line ref yet) and one external. - fn report_recurse_one_each( + fn report_recurse_one_each( &mut self, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map>, ) { match (derived.cause1.deref(), derived.cause2.deref()) { // If the derived cause has itself one external prior cause, @@ -341,10 +337,10 @@ impl DefaultStringReporter { // String explanations ##################################################### /// Simplest case, we just combine two external incompatibilities. - fn explain_both_external( - external1: &External, - external2: &External, - current_terms: &Map>, + fn explain_both_external( + external1: &External, + external2: &External, + current_terms: &Map>, ) -> String { // TODO: order should be chosen to make it more logical. format!( @@ -356,12 +352,12 @@ impl DefaultStringReporter { } /// Both causes have already been explained so we use their refs. - fn explain_both_ref( + fn explain_both_ref( ref_id1: usize, - derived1: &Derived, + derived1: &Derived, ref_id2: usize, - derived2: &Derived, - current_terms: &Map>, + derived2: &Derived, + current_terms: &Map>, ) -> String { // TODO: order should be chosen to make it more logical. format!( @@ -377,11 +373,11 @@ impl DefaultStringReporter { /// One cause is derived (already explained so one-line), /// the other is a one-line external cause, /// and finally we conclude with the current incompatibility. - fn explain_ref_and_external( + fn explain_ref_and_external( ref_id: usize, - derived: &Derived, - external: &External, - current_terms: &Map>, + derived: &Derived, + external: &External, + current_terms: &Map>, ) -> String { // TODO: order should be chosen to make it more logical. format!( @@ -394,9 +390,9 @@ impl DefaultStringReporter { } /// Add an external cause to the chain of explanations. - fn and_explain_external( - external: &External, - current_terms: &Map>, + fn and_explain_external( + external: &External, + current_terms: &Map>, ) -> String { format!( "And because {}, {}.", @@ -406,10 +402,10 @@ impl DefaultStringReporter { } /// Add an already explained incompat to the chain of explanations. - fn and_explain_ref( + fn and_explain_ref( ref_id: usize, - derived: &Derived, - current_terms: &Map>, + derived: &Derived, + current_terms: &Map>, ) -> String { format!( "And because {} ({}), {}.", @@ -420,10 +416,10 @@ impl DefaultStringReporter { } /// Add an already explained incompat to the chain of explanations. - fn and_explain_prior_and_external( - prior_external: &External, - external: &External, - current_terms: &Map>, + fn and_explain_prior_and_external( + prior_external: &External, + external: &External, + current_terms: &Map>, ) -> String { format!( "And because {} and {}, {}.", @@ -434,7 +430,7 @@ impl DefaultStringReporter { } /// Try to print terms of an incompatibility in a human-readable way. - pub fn string_terms(terms: &Map>) -> String { + pub fn string_terms(terms: &Map>) -> String { let terms_vec: Vec<_> = terms.iter().collect(); match terms_vec.as_slice() { [] => "version solving failed".into(), @@ -469,10 +465,10 @@ impl DefaultStringReporter { } } -impl Reporter for DefaultStringReporter { +impl Reporter for DefaultStringReporter { type Output = String; - fn report(derivation_tree: &DerivationTree) -> Self::Output { + fn report(derivation_tree: &DerivationTree) -> Self::Output { match derivation_tree { DerivationTree::External(external) => external.to_string(), DerivationTree::Derived(derived) => { diff --git a/src/solver.rs b/src/solver.rs index 62015911..a1e5e06c 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -27,8 +27,8 @@ //! //! The algorithm is generic and works for any type of dependency system //! as long as packages (P) and versions (V) implement -//! the [Package](crate::package::Package) and [Version](crate::version::Version) traits. -//! [Package](crate::package::Package) is strictly equivalent and automatically generated +//! the [Package] and [Version](crate::version::Version) traits. +//! [Package] is strictly equivalent and automatically generated //! for any type that implement [Clone] + [Eq] + [Hash] + [Debug] + [Display](std::fmt::Display). //! [Version](crate::version::Version) simply states that versions are ordered, //! that there should be @@ -44,9 +44,12 @@ //! # use pubgrub::solver::{resolve, OfflineDependencyProvider}; //! # use pubgrub::version::NumberVersion; //! # use pubgrub::error::PubGrubError; +//! # use pubgrub::range::Range; //! # -//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumberVersion>> { -//! # let dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); +//! # type NumVS = Range; +//! # +//! # fn try_main() -> Result<(), PubGrubError<&'static str, NumVS>> { +//! # let dependency_provider = OfflineDependencyProvider::<&str, NumVS>::new(); //! # let package = "root"; //! # let version = 1; //! let solution = resolve(&dependency_provider, package, version)?; @@ -65,7 +68,7 @@ //! to satisfy the dependencies of that package and version pair. //! If there is no solution, the reason will be provided as clear as possible. -use std::borrow::Borrow; +use std::cmp::Reverse; use std::collections::{BTreeMap, BTreeSet as Set}; use std::error::Error; @@ -73,50 +76,54 @@ use crate::error::PubGrubError; use crate::internal::core::State; use crate::internal::incompatibility::Incompatibility; use crate::package::Package; -use crate::range::Range; -use crate::type_aliases::{Map, SelectedDependencies}; -use crate::version::Version; +use crate::type_aliases::{DependencyConstraints, Map, SelectedDependencies}; +use crate::version_set::VersionSet; +use log::{debug, info}; /// Main function of the library. /// Finds a set of packages satisfying dependency bounds for a given package + version pair. -pub fn resolve( - dependency_provider: &impl DependencyProvider, +pub fn resolve( + dependency_provider: &impl DependencyProvider, package: P, - version: impl Into, -) -> Result, PubGrubError> { + version: impl Into, +) -> Result, PubGrubError> { let mut state = State::init(package.clone(), version.into()); - let mut added_dependencies: Map> = Map::default(); + let mut added_dependencies: Map> = Map::default(); let mut next = package; loop { dependency_provider .should_cancel() .map_err(|err| PubGrubError::ErrorInShouldCancel(err))?; + info!("unit_propagation: {}", &next); state.unit_propagation(next)?; - let potential_packages = state.partial_solution.potential_packages(); - if potential_packages.is_none() { - drop(potential_packages); - // The borrow checker did not like using a match on potential_packages. - // This `if ... is_none ... drop` is a workaround. - // I believe this is a case where Polonius could help, when and if it lands in rustc. - return state.partial_solution.extract_solution().ok_or_else(|| { - PubGrubError::Failure( - "How did we end up with no package to choose but no solution?".into(), - ) - }); - } - let decision = dependency_provider - .choose_package_version(potential_packages.unwrap()) - .map_err(PubGrubError::ErrorChoosingPackageVersion)?; - next = decision.0.clone(); + debug!( + "Partial solution after unit propagation: {}", + state.partial_solution + ); + + let Some(highest_priority_pkg) = state + .partial_solution + .pick_highest_priority_pkg(|p, r| dependency_provider.prioritize(p, r)) + else { + return Ok(state.partial_solution.extract_solution()); + }; + next = highest_priority_pkg; - // Pick the next compatible version. let term_intersection = state .partial_solution .term_intersection_for_package(&next) - .expect("a package was chosen but we don't have a term."); - let v = match decision.1 { + .ok_or_else(|| { + PubGrubError::Failure("a package was chosen but we don't have a term.".into()) + })?; + let decision = dependency_provider + .choose_version(&next, term_intersection.unwrap_positive()) + .map_err(PubGrubError::ErrorChoosingPackageVersion)?; + info!("DP chose: {} @ {:?}", next, decision); + + // Pick the next compatible version. + let v = match decision { None => { let inc = Incompatibility::no_versions(next.clone(), term_intersection.clone()); state.add_incompatibility(inc); @@ -124,68 +131,53 @@ pub fn resolve( } Some(x) => x, }; + if !term_intersection.contains(&v) { return Err(PubGrubError::ErrorChoosingPackageVersion( "choose_package_version picked an incompatible version".into(), )); } - if added_dependencies + let is_new_dependency = added_dependencies .entry(next.clone()) .or_default() - .insert(v.clone()) - { + .insert(v.clone()); + + if is_new_dependency { // Retrieve that package dependencies. let p = &next; - let dependencies = - match dependency_provider - .get_dependencies(&p, &v) - .map_err(|err| PubGrubError::ErrorRetrievingDependencies { + let dependencies = dependency_provider.get_dependencies(p, &v).map_err(|err| { + PubGrubError::ErrorRetrievingDependencies { + package: p.clone(), + version: v.clone(), + source: err, + } + })?; + + let known_dependencies = match dependencies { + Dependencies::Unknown => { + state.add_incompatibility(Incompatibility::unavailable_dependencies( + p.clone(), + v.clone(), + )); + continue; + } + Dependencies::Known(x) if x.contains_key(p) => { + return Err(PubGrubError::SelfDependency { package: p.clone(), - version: v.clone(), - source: err, - })? { - Dependencies::Unknown => { - state.add_incompatibility(Incompatibility::unavailable_dependencies( - p.clone(), - v.clone(), - )); - continue; - } - Dependencies::Known(x) => { - if x.contains_key(&p) { - return Err(PubGrubError::SelfDependency { - package: p.clone(), - version: v.clone(), - }); - } - if let Some((dependent, _)) = x.iter().find(|(_, r)| r == &&Range::none()) { - return Err(PubGrubError::DependencyOnTheEmptySet { - package: p.clone(), - version: v.clone(), - dependent: dependent.clone(), - }); - } - x - } - }; + version: v, + }); + } + Dependencies::Known(x) => x, + }; // Add that package and version if the dependencies are not problematic. - let dep_incompats = - state.add_incompatibility_from_dependencies(p.clone(), v.clone(), &dependencies); + let dep_incompats = state.add_incompatibility_from_dependencies( + p.clone(), + v.clone(), + &known_dependencies, + ); - // TODO: I don't think this check can actually happen. - // We might want to put it under #[cfg(debug_assertions)]. - if state.incompatibility_store[dep_incompats.clone()] - .iter() - .any(|incompat| state.is_terminal(incompat)) - { - // For a dependency incompatibility to be terminal, - // it can only mean that root depend on not root? - return Err(PubGrubError::Failure( - "Root package depends on itself at a different version?".into(), - )); - } state.partial_solution.add_version( p.clone(), v, @@ -195,40 +187,37 @@ pub fn resolve( } else { // `dep_incompats` are already in `incompatibilities` so we know there are not satisfied // terms and can add the decision directly. + info!("add_decision (not first time): {} @ {}", &next, v); state.partial_solution.add_decision(next.clone(), v); } } } /// An enum used by [DependencyProvider] that holds information about package dependencies. -/// For each [Package] there is a [Range] of concrete versions it allows as a dependency. +/// For each [Package] there is a set of versions allowed as a dependency. #[derive(Clone)] -pub enum Dependencies { +pub enum Dependencies { /// Package dependencies are unavailable. Unknown, /// Container for all available package versions. - Known(DependencyConstraints), + Known(DependencyConstraints), } -/// Subtype of [Dependencies] which holds information about -/// all possible versions a given package can accept. -/// There is a difference in semantics between an empty [Map>](crate::type_aliases::Map) -/// inside [DependencyConstraints] and [Dependencies::Unknown]: -/// the former means the package has no dependencies and it is a known fact, -/// while the latter means they could not be fetched by [DependencyProvider]. -pub type DependencyConstraints = Map>; - /// Trait that allows the algorithm to retrieve available packages and their dependencies. /// An implementor needs to be supplied to the [resolve] function. -pub trait DependencyProvider { +pub trait DependencyProvider { /// [Decision making](https://github.com/dart-lang/pub/blob/master/doc/solver.md#decision-making) /// is the process of choosing the next package /// and version that will be appended to the partial solution. - /// Every time such a decision must be made, - /// potential valid packages and version ranges are preselected by the resolver, - /// and the dependency provider must choose. /// - /// The strategy employed to choose such package and version + /// Every time such a decision must be made, the resolver looks at all the potential valid + /// packages that have changed, and a asks the dependency provider how important each one is. + /// For each one it calls `prioritize` with the name of the package and the current set of + /// acceptable versions. + /// The resolver will then pick the package with the highes priority from all the potential valid + /// packages. + /// + /// The strategy employed to prioritize packages /// cannot change the existence of a solution or not, /// but can drastically change the performances of the solver, /// or the properties of the solution. @@ -241,73 +230,59 @@ pub trait DependencyProvider { /// > since these packages will run out of versions to try more quickly. /// > But there's likely room for improvement in these heuristics. /// - /// A helper function [choose_package_with_fewest_versions] is provided to ease - /// implementations of this method if you can produce an iterator - /// of the available versions in preference order for any package. + /// Note: the resolver may call this even when the range has not change, + /// if it is more efficient for the resolveres internal data structures. + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority; + /// The type returned from `prioritize`. The resolver does not care what type this is + /// as long as it can pick a largest one and clone it. /// - /// Note: the type `T` ensures that this returns an item from the `packages` argument. - fn choose_package_version, U: Borrow>>( + /// [std::cmp::Reverse] can be useful if you want to pick the package with + /// the fewest versions that match the outstanding constraint. + type Priority: Ord + Clone; + + /// Once the resolver has found the highest `Priority` package from all potential valid + /// packages, it needs to know what vertion of that package to use. The most common pattern + /// is to select the largest vertion that the range contains. + fn choose_version( &self, - potential_packages: impl Iterator, - ) -> Result<(T, Option), Box>; + package: &P, + range: &VS, + ) -> Result, Box>; /// Retrieves the package dependencies. /// Return [Dependencies::Unknown] if its dependencies are unknown. fn get_dependencies( &self, package: &P, - version: &V, - ) -> Result, Box>; + version: &VS::V, + ) -> Result, Box>; /// This is called fairly regularly during the resolution, /// if it returns an Err then resolution will be terminated. /// This is helpful if you want to add some form of early termination like a timeout, /// or you want to add some form of user feedback if things are taking a while. /// If not provided the resolver will run as long as needed. - fn should_cancel(&self) -> Result<(), Box> { + fn should_cancel(&self) -> Result<(), Box> { Ok(()) } } -/// This is a helper function to make it easy to implement -/// [DependencyProvider::choose_package_version]. -/// It takes a function `list_available_versions` that takes a package and returns an iterator -/// of the available versions in preference order. -/// The helper finds the package from the `packages` argument with the fewest versions from -/// `list_available_versions` contained in the constraints. Then takes that package and finds the -/// first version contained in the constraints. -pub fn choose_package_with_fewest_versions( - list_available_versions: F, - potential_packages: impl Iterator, -) -> (T, Option) -where - T: Borrow

, - U: Borrow>, - I: Iterator, - F: Fn(&P) -> I, -{ - let count_valid = |(p, range): &(T, U)| { - list_available_versions(p.borrow()) - .filter(|v| range.borrow().contains(v.borrow())) - .count() - }; - let (pkg, range) = potential_packages - .min_by_key(count_valid) - .expect("potential_packages gave us an empty iterator"); - let version = - list_available_versions(pkg.borrow()).find(|v| range.borrow().contains(v.borrow())); - (pkg, version) -} - /// A basic implementation of [DependencyProvider]. #[derive(Debug, Clone, Default)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr( + feature = "serde", + serde(bound( + serialize = "VS::V: serde::Serialize, VS: serde::Serialize, P: serde::Serialize", + deserialize = "VS::V: serde::Deserialize<'de>, VS: serde::Deserialize<'de>, P: serde::Deserialize<'de>" + )) +)] #[cfg_attr(feature = "serde", serde(transparent))] -pub struct OfflineDependencyProvider { - dependencies: Map>>, +pub struct OfflineDependencyProvider { + dependencies: Map>>, } -impl OfflineDependencyProvider { +impl OfflineDependencyProvider { /// Creates an empty OfflineDependencyProvider with no dependencies. pub fn new() -> Self { Self { @@ -325,10 +300,10 @@ impl OfflineDependencyProvider { /// The API does not allow to add dependencies one at a time to uphold an assumption that /// [OfflineDependencyProvider.get_dependencies(p, v)](OfflineDependencyProvider::get_dependencies) /// provides all dependencies of a given package (p) and version (v) pair. - pub fn add_dependencies)>>( + pub fn add_dependencies>( &mut self, package: P, - version: impl Into, + version: impl Into, dependencies: I, ) { let package_deps = dependencies.into_iter().collect(); @@ -348,44 +323,49 @@ impl OfflineDependencyProvider { /// Lists versions of saved packages in sorted order. /// Returns [None] if no information is available regarding that package. - pub fn versions(&self, package: &P) -> Option> { + pub fn versions(&self, package: &P) -> Option> { self.dependencies.get(package).map(|k| k.keys()) } /// Lists dependencies of a given package and version. /// Returns [None] if no information is available regarding that package and version pair. - fn dependencies(&self, package: &P, version: &V) -> Option> { + fn dependencies(&self, package: &P, version: &VS::V) -> Option> { self.dependencies.get(package)?.get(version).cloned() } } /// An implementation of [DependencyProvider] that /// contains all dependency information available in memory. -/// Packages are picked with the fewest versions contained in the constraints first. +/// Currently packages are picked with the fewest versions contained in the constraints first. +/// But, that may change in new versions if better heuristics are found. /// Versions are picked with the newest versions first. -impl DependencyProvider for OfflineDependencyProvider { - fn choose_package_version, U: Borrow>>( +impl DependencyProvider for OfflineDependencyProvider { + fn choose_version( &self, - potential_packages: impl Iterator, - ) -> Result<(T, Option), Box> { - Ok(choose_package_with_fewest_versions( - |p| { - self.dependencies - .get(p) - .into_iter() - .flat_map(|k| k.keys()) - .rev() - .cloned() - }, - potential_packages, - )) + package: &P, + range: &VS, + ) -> Result, Box> { + Ok(self + .dependencies + .get(package) + .and_then(|versions| versions.keys().rev().find(|v| range.contains(v)).cloned())) + } + + type Priority = Reverse; + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + Reverse( + self.dependencies + .get(package) + .map(|versions| versions.keys().filter(|v| range.contains(v)).count()) + .unwrap_or(0), + ) } fn get_dependencies( &self, package: &P, - version: &V, - ) -> Result, Box> { + version: &VS::V, + ) -> Result, Box> { Ok(match self.dependencies(package, version) { None => Dependencies::Unknown, Some(dependencies) => Dependencies::Known(dependencies), diff --git a/src/term.rs b/src/term.rs index bc038acf..cf7aa6f7 100644 --- a/src/term.rs +++ b/src/term.rs @@ -3,38 +3,37 @@ //! A term is the fundamental unit of operation of the PubGrub algorithm. //! It is a positive or negative expression regarding a set of versions. -use crate::range::Range; -use crate::version::Version; -use std::fmt; +use crate::version_set::VersionSet; +use std::fmt::{self, Display}; /// A positive or negative expression regarding a set of versions. #[derive(Debug, Clone, Eq, PartialEq)] -pub enum Term { +pub enum Term { /// For example, "1.0.0 <= v < 2.0.0" is a positive expression /// that is evaluated true if a version is selected /// and comprised between version 1.0.0 and version 2.0.0. - Positive(Range), + Positive(VS), /// The term "not v < 3.0.0" is a negative expression /// that is evaluated true if a version is selected >= 3.0.0 /// or if no version is selected at all. - Negative(Range), + Negative(VS), } /// Base methods. -impl Term { +impl Term { /// A term that is always true. pub(crate) fn any() -> Self { - Self::Negative(Range::none()) + Self::Negative(VS::empty()) } /// A term that is never true. pub(crate) fn empty() -> Self { - Self::Positive(Range::none()) + Self::Positive(VS::empty()) } /// A positive term containing exactly that version. - pub(crate) fn exact(version: V) -> Self { - Self::Positive(Range::exact(version)) + pub(crate) fn exact(version: VS::V) -> Self { + Self::Positive(VS::singleton(version)) } /// Simply check if a term is positive. @@ -50,41 +49,41 @@ impl Term { /// the opposite of the evaluation of the original one. pub(crate) fn negate(&self) -> Self { match self { - Self::Positive(range) => Self::Negative(range.clone()), - Self::Negative(range) => Self::Positive(range.clone()), + Self::Positive(set) => Self::Negative(set.clone()), + Self::Negative(set) => Self::Positive(set.clone()), } } /// Evaluate a term regarding a given choice of version. - pub(crate) fn contains(&self, v: &V) -> bool { + pub(crate) fn contains(&self, v: &VS::V) -> bool { match self { - Self::Positive(range) => range.contains(v), - Self::Negative(range) => !(range.contains(v)), + Self::Positive(set) => set.contains(v), + Self::Negative(set) => !(set.contains(v)), } } - /// Unwrap the range contains in a positive term. - /// Will panic if used on a negative range. - pub(crate) fn unwrap_positive(&self) -> &Range { + /// Unwrap the set contained in a positive term. + /// Will panic if used on a negative set. + pub(crate) fn unwrap_positive(&self) -> &VS { match self { - Self::Positive(range) => range, - _ => panic!("Negative term cannot unwrap positive range"), + Self::Positive(set) => set, + _ => panic!("Negative term cannot unwrap positive set"), } } } /// Set operations with terms. -impl Term { +impl Term { /// Compute the intersection of two terms. /// If at least one term is positive, the intersection is also positive. - pub(crate) fn intersection(&self, other: &Term) -> Term { + pub(crate) fn intersection(&self, other: &Self) -> Self { match (self, other) { (Self::Positive(r1), Self::Positive(r2)) => Self::Positive(r1.intersection(r2)), (Self::Positive(r1), Self::Negative(r2)) => { - Self::Positive(r1.intersection(&r2.negate())) + Self::Positive(r1.intersection(&r2.complement())) } (Self::Negative(r1), Self::Positive(r2)) => { - Self::Positive(r1.negate().intersection(r2)) + Self::Positive(r1.complement().intersection(r2)) } (Self::Negative(r1), Self::Negative(r2)) => Self::Negative(r1.union(r2)), } @@ -92,14 +91,14 @@ impl Term { /// Compute the union of two terms. /// If at least one term is negative, the union is also negative. - pub(crate) fn union(&self, other: &Term) -> Term { + pub(crate) fn union(&self, other: &Self) -> Self { (self.negate().intersection(&other.negate())).negate() } /// Indicate if this term is a subset of another term. /// Just like for sets, we say that t1 is a subset of t2 /// if and only if t1 ∩ t2 = t1. - pub(crate) fn subset_of(&self, other: &Term) -> bool { + pub(crate) fn subset_of(&self, other: &Self) -> bool { self == &self.intersection(other) } } @@ -120,7 +119,7 @@ pub(crate) enum Relation { } /// Relation between terms. -impl<'a, V: 'a + Version> Term { +impl Term { /// Check if a set of terms satisfies this term. /// /// We say that a set of terms S "satisfies" a term t @@ -129,7 +128,7 @@ impl<'a, V: 'a + Version> Term { /// It turns out that this can also be expressed with set operations: /// S satisfies t if and only if ⋂ S ⊆ t #[cfg(test)] - fn satisfied_by(&self, terms_intersection: &Term) -> bool { + fn satisfied_by(&self, terms_intersection: &Self) -> bool { terms_intersection.subset_of(self) } @@ -142,13 +141,13 @@ impl<'a, V: 'a + Version> Term { /// S contradicts t if and only if ⋂ S is disjoint with t /// S contradicts t if and only if (⋂ S) ⋂ t = ∅ #[cfg(test)] - fn contradicted_by(&self, terms_intersection: &Term) -> bool { + fn contradicted_by(&self, terms_intersection: &Self) -> bool { terms_intersection.intersection(self) == Self::empty() } /// Check if a set of terms satisfies or contradicts a given term. /// Otherwise the relation is inconclusive. - pub(crate) fn relation_with(&self, other_terms_intersection: &Term) -> Relation { + pub(crate) fn relation_with(&self, other_terms_intersection: &Self) -> Relation { let full_intersection = self.intersection(other_terms_intersection); if &full_intersection == other_terms_intersection { Relation::Satisfied @@ -160,19 +159,19 @@ impl<'a, V: 'a + Version> Term { } } -impl AsRef> for Term { - fn as_ref(&self) -> &Term { - &self +impl AsRef for Term { + fn as_ref(&self) -> &Self { + self } } // REPORT ###################################################################### -impl fmt::Display for Term { +impl Display for Term { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Positive(range) => write!(f, "{}", range), - Self::Negative(range) => write!(f, "Not ( {} )", range), + Self::Positive(set) => write!(f, "{}", set), + Self::Negative(set) => write!(f, "Not ( {} )", set), } } } @@ -182,10 +181,10 @@ impl fmt::Display for Term { #[cfg(test)] pub mod tests { use super::*; - use crate::version::NumberVersion; + use crate::range::Range; use proptest::prelude::*; - pub fn strategy() -> impl Strategy> { + pub fn strategy() -> impl Strategy>> { prop_oneof![ crate::range::tests::strategy().prop_map(Term::Positive), crate::range::tests::strategy().prop_map(Term::Negative), diff --git a/src/type_aliases.rs b/src/type_aliases.rs index d1cc378a..11cc37c7 100644 --- a/src/type_aliases.rs +++ b/src/type_aliases.rs @@ -6,5 +6,12 @@ pub type Map = rustc_hash::FxHashMap; /// Concrete dependencies picked by the library during [resolve](crate::solver::resolve) -/// from [DependencyConstraints](crate::solver::DependencyConstraints) +/// from [DependencyConstraints]. pub type SelectedDependencies = Map; + +/// Holds information about all possible versions a given package can accept. +/// There is a difference in semantics between an empty map +/// inside [DependencyConstraints] and [Dependencies::Unknown](crate::solver::Dependencies::Unknown): +/// the former means the package has no dependency and it is a known fact, +/// while the latter means they could not be fetched by the [DependencyProvider](crate::solver::DependencyProvider). +pub type DependencyConstraints = Map; diff --git a/src/version.rs b/src/version.rs index c7d749ee..6f67c7de 100644 --- a/src/version.rs +++ b/src/version.rs @@ -80,6 +80,21 @@ impl From<(u32, u32, u32)> for SemanticVersion { } } +// Convert a &(major, minor, patch) into a version. +impl From<&(u32, u32, u32)> for SemanticVersion { + fn from(tuple: &(u32, u32, u32)) -> Self { + let (major, minor, patch) = *tuple; + Self::new(major, minor, patch) + } +} + +// Convert an &version into a version. +impl From<&SemanticVersion> for SemanticVersion { + fn from(v: &SemanticVersion) -> Self { + *v + } +} + // Convert a version into a tuple (major, minor, patch). impl From for (u32, u32, u32) { fn from(v: SemanticVersion) -> Self { @@ -106,7 +121,7 @@ impl SemanticVersion { } /// Error creating [SemanticVersion] from [String]. -#[derive(Error, Debug, PartialEq)] +#[derive(Error, Debug, PartialEq, Eq)] pub enum VersionParseError { /// [SemanticVersion] must contain major, minor, patch versions. #[error("version {full_version} must contain 3 numbers separated by dot")] @@ -237,6 +252,20 @@ impl From for NumberVersion { } } +// 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 { diff --git a/src/version_set.rs b/src/version_set.rs new file mode 100644 index 00000000..501ec700 --- /dev/null +++ b/src/version_set.rs @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! As its name suggests, the [VersionSet] trait describes sets of versions. +//! +//! One needs to define +//! - the associate type for versions, +//! - two constructors for the empty set and a singleton set, +//! - the complement and intersection set operations, +//! - and a function to evaluate membership of versions. +//! +//! Two functions are automatically derived, thanks to the mathematical properties of sets. +//! You can overwrite those implementations, but we highly recommend that you don't, +//! except if you are confident in a correct implementation that brings much performance gains. +//! +//! It is also extremely important that the `Eq` trait is correctly implemented. +//! In particular, you can only use `#[derive(Eq, PartialEq)]` if `Eq` is strictly equivalent to the +//! structural equality, i.e. if version sets have canonical representations. +//! Such problems may arise if your implementations of `complement()` and `intersection()` do not +//! return canonical representations so be careful there. + +use std::fmt::{Debug, Display}; + +/// Trait describing sets of versions. +pub trait VersionSet: Debug + Display + Clone + Eq { + /// Version type associated with the sets manipulated. + type V: Debug + Display + Clone + Ord; + + // Constructors + /// Constructor for an empty set containing no version. + fn empty() -> Self; + /// Constructor for a set containing exactly one version. + fn singleton(v: Self::V) -> Self; + + // Operations + /// Compute the complement of this set. + fn complement(&self) -> Self; + /// Compute the intersection with another set. + fn intersection(&self, other: &Self) -> Self; + + // Membership + /// Evaluate membership of a version in this set. + fn contains(&self, v: &Self::V) -> bool; + + // Automatically implemented functions ########################### + + /// Constructor for the set containing all versions. + /// Automatically implemented as `Self::empty().complement()`. + fn full() -> Self { + Self::empty().complement() + } + + /// Compute the union with another set. + /// Thanks to set properties, this is automatically implemented as: + /// `self.complement().intersection(&other.complement()).complement()` + fn union(&self, other: &Self) -> Self { + self.complement() + .intersection(&other.complement()) + .complement() + } +} diff --git a/tests/examples.rs b/tests/examples.rs index 29901caf..9c11d4fe 100644 --- a/tests/examples.rs +++ b/tests/examples.rs @@ -5,22 +5,37 @@ use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::type_aliases::Map; use pubgrub::version::{NumberVersion, SemanticVersion}; +type NumVS = Range; +type SemVS = Range; + +use log::LevelFilter; +use std::io::Write; + +fn init_log() { + let _ = env_logger::builder() + .filter_level(LevelFilter::Trace) + .format(|buf, record| writeln!(buf, "{}", record.args())) + .is_test(true) + .try_init(); +} + #[test] /// https://github.com/dart-lang/pub/blob/master/doc/solver.md#no-conflicts fn no_conflict() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + init_log(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new(); #[rustfmt::skip] dependency_provider.add_dependencies( "root", (1, 0, 0), - vec![("foo", Range::between((1, 0, 0), (2, 0, 0)))], + [("foo", Range::between((1, 0, 0), (2, 0, 0)))], ); #[rustfmt::skip] dependency_provider.add_dependencies( "foo", (1, 0, 0), - vec![("bar", Range::between((1, 0, 0), (2, 0, 0)))], + [("bar", Range::between((1, 0, 0), (2, 0, 0)))], ); - dependency_provider.add_dependencies("bar", (1, 0, 0), vec![]); - dependency_provider.add_dependencies("bar", (2, 0, 0), vec![]); + dependency_provider.add_dependencies("bar", (1, 0, 0), []); + dependency_provider.add_dependencies("bar", (2, 0, 0), []); // Run the algorithm. let computed_solution = resolve(&dependency_provider, "root", (1, 0, 0)).unwrap(); @@ -38,11 +53,12 @@ fn no_conflict() { #[test] /// https://github.com/dart-lang/pub/blob/master/doc/solver.md#avoiding-conflict-during-decision-making fn avoiding_conflict_during_decision_making() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + init_log(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new(); #[rustfmt::skip] dependency_provider.add_dependencies( "root", (1, 0, 0), - vec![ + [ ("foo", Range::between((1, 0, 0), (2, 0, 0))), ("bar", Range::between((1, 0, 0), (2, 0, 0))), ], @@ -50,12 +66,12 @@ fn avoiding_conflict_during_decision_making() { #[rustfmt::skip] dependency_provider.add_dependencies( "foo", (1, 1, 0), - vec![("bar", Range::between((2, 0, 0), (3, 0, 0)))], + [("bar", Range::between((2, 0, 0), (3, 0, 0)))], ); - dependency_provider.add_dependencies("foo", (1, 0, 0), vec![]); - dependency_provider.add_dependencies("bar", (1, 0, 0), vec![]); - dependency_provider.add_dependencies("bar", (1, 1, 0), vec![]); - dependency_provider.add_dependencies("bar", (2, 0, 0), vec![]); + dependency_provider.add_dependencies("foo", (1, 0, 0), []); + dependency_provider.add_dependencies("bar", (1, 0, 0), []); + dependency_provider.add_dependencies("bar", (1, 1, 0), []); + dependency_provider.add_dependencies("bar", (2, 0, 0), []); // Run the algorithm. let computed_solution = resolve(&dependency_provider, "root", (1, 0, 0)).unwrap(); @@ -73,22 +89,23 @@ fn avoiding_conflict_during_decision_making() { #[test] /// https://github.com/dart-lang/pub/blob/master/doc/solver.md#performing-conflict-resolution fn conflict_resolution() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + init_log(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new(); #[rustfmt::skip] dependency_provider.add_dependencies( "root", (1, 0, 0), - vec![("foo", Range::higher_than((1, 0, 0)))], + [("foo", Range::higher_than((1, 0, 0)))], ); #[rustfmt::skip] dependency_provider.add_dependencies( "foo", (2, 0, 0), - vec![("bar", Range::between((1, 0, 0), (2, 0, 0)))], + [("bar", Range::between((1, 0, 0), (2, 0, 0)))], ); - dependency_provider.add_dependencies("foo", (1, 0, 0), vec![]); + dependency_provider.add_dependencies("foo", (1, 0, 0), []); #[rustfmt::skip] dependency_provider.add_dependencies( "bar", (1, 0, 0), - vec![("foo", Range::between((1, 0, 0), (2, 0, 0)))], + [("foo", Range::between((1, 0, 0), (2, 0, 0)))], ); // Run the algorithm. @@ -106,12 +123,13 @@ fn conflict_resolution() { #[test] /// https://github.com/dart-lang/pub/blob/master/doc/solver.md#conflict-resolution-with-a-partial-satisfier fn conflict_with_partial_satisfier() { - let mut dependency_provider = OfflineDependencyProvider::<&str, SemanticVersion>::new(); + init_log(); + let mut dependency_provider = OfflineDependencyProvider::<&str, SemVS>::new(); #[rustfmt::skip] // root 1.0.0 depends on foo ^1.0.0 and target ^2.0.0 dependency_provider.add_dependencies( "root", (1, 0, 0), - vec![ + [ ("foo", Range::between((1, 0, 0), (2, 0, 0))), ("target", Range::between((2, 0, 0), (3, 0, 0))), ], @@ -120,33 +138,33 @@ fn conflict_with_partial_satisfier() { // foo 1.1.0 depends on left ^1.0.0 and right ^1.0.0 dependency_provider.add_dependencies( "foo", (1, 1, 0), - vec![ + [ ("left", Range::between((1, 0, 0), (2, 0, 0))), ("right", Range::between((1, 0, 0), (2, 0, 0))), ], ); - dependency_provider.add_dependencies("foo", (1, 0, 0), vec![]); + dependency_provider.add_dependencies("foo", (1, 0, 0), []); #[rustfmt::skip] // left 1.0.0 depends on shared >=1.0.0 dependency_provider.add_dependencies( "left", (1, 0, 0), - vec![("shared", Range::higher_than((1, 0, 0)))], + [("shared", Range::higher_than((1, 0, 0)))], ); #[rustfmt::skip] // right 1.0.0 depends on shared <2.0.0 dependency_provider.add_dependencies( "right", (1, 0, 0), - vec![("shared", Range::strictly_lower_than((2, 0, 0)))], + [("shared", Range::strictly_lower_than((2, 0, 0)))], ); - dependency_provider.add_dependencies("shared", (2, 0, 0), vec![]); + dependency_provider.add_dependencies("shared", (2, 0, 0), []); #[rustfmt::skip] // shared 1.0.0 depends on target ^1.0.0 dependency_provider.add_dependencies( "shared", (1, 0, 0), - vec![("target", Range::between((1, 0, 0), (2, 0, 0)))], + [("target", Range::between((1, 0, 0), (2, 0, 0)))], ); - dependency_provider.add_dependencies("target", (2, 0, 0), vec![]); - dependency_provider.add_dependencies("target", (1, 0, 0), vec![]); + dependency_provider.add_dependencies("target", (2, 0, 0), []); + dependency_provider.add_dependencies("target", (1, 0, 0), []); // Run the algorithm. let computed_solution = resolve(&dependency_provider, "root", (1, 0, 0)).unwrap(); @@ -171,13 +189,14 @@ fn conflict_with_partial_satisfier() { /// /// Solution: a0, b0, c0, d0 fn double_choices() { - let mut dependency_provider = OfflineDependencyProvider::<&str, NumberVersion>::new(); - dependency_provider.add_dependencies("a", 0, vec![("b", Range::any()), ("c", Range::any())]); - dependency_provider.add_dependencies("b", 0, vec![("d", Range::exact(0))]); - dependency_provider.add_dependencies("b", 1, vec![("d", Range::exact(1))]); - dependency_provider.add_dependencies("c", 0, vec![]); - dependency_provider.add_dependencies("c", 1, vec![("d", Range::exact(2))]); - dependency_provider.add_dependencies("d", 0, vec![]); + 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, []); // Solution. let mut expected_solution = Map::default(); diff --git a/tests/proptest.rs b/tests/proptest.rs index d2da3730..017efed0 100644 --- a/tests/proptest.rs +++ b/tests/proptest.rs @@ -5,14 +5,13 @@ use std::{collections::BTreeSet as Set, error::Error}; use pubgrub::error::PubGrubError; use pubgrub::package::Package; use pubgrub::range::Range; -use pubgrub::report::{DefaultStringReporter, Reporter}; -use pubgrub::solver::{ - choose_package_with_fewest_versions, resolve, Dependencies, DependencyProvider, - OfflineDependencyProvider, -}; -use pubgrub::version::{NumberVersion, Version}; - -use proptest::collection::{btree_map, vec}; +use pubgrub::report::{DefaultStringReporter, DerivationTree, External, Reporter}; +use pubgrub::solver::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider}; +use pubgrub::type_aliases::SelectedDependencies; +use pubgrub::version::{NumberVersion, SemanticVersion}; +use pubgrub::version_set::VersionSet; + +use proptest::collection::{btree_map, btree_set, vec}; use proptest::prelude::*; use proptest::sample::Index; use proptest::string::string_regex; @@ -24,21 +23,39 @@ mod sat_dependency_provider; /// The same as [OfflineDependencyProvider] but takes versions from the opposite end: /// if [OfflineDependencyProvider] returns versions from newest to oldest, this returns them from oldest to newest. #[derive(Clone)] -struct OldestVersionsDependencyProvider(OfflineDependencyProvider); +struct OldestVersionsDependencyProvider( + OfflineDependencyProvider, +); -impl DependencyProvider for OldestVersionsDependencyProvider { - fn choose_package_version, U: std::borrow::Borrow>>( +impl DependencyProvider + for OldestVersionsDependencyProvider +{ + fn get_dependencies( &self, - potential_packages: impl Iterator, - ) -> Result<(T, Option), Box> { - Ok(choose_package_with_fewest_versions( - |p| self.0.versions(p).into_iter().flatten().cloned(), - potential_packages, - )) + p: &P, + v: &VS::V, + ) -> Result, Box> { + self.0.get_dependencies(p, v) } - fn get_dependencies(&self, p: &P, v: &V) -> Result, Box> { - self.0.get_dependencies(p, v) + fn choose_version( + &self, + package: &P, + range: &VS, + ) -> Result, Box> { + Ok(self + .0 + .versions(package) + .into_iter() + .flatten() + .find(|&v| range.contains(v)) + .cloned()) + } + + type Priority = as DependencyProvider>::Priority; + + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + self.0.prioritize(package, range) } } @@ -62,34 +79,60 @@ impl TimeoutDependencyProvider { } } -impl> DependencyProvider +impl> DependencyProvider for TimeoutDependencyProvider { - fn choose_package_version, U: std::borrow::Borrow>>( + fn get_dependencies( &self, - potential_packages: impl Iterator, - ) -> Result<(T, Option), Box> { - self.dp.choose_package_version(potential_packages) - } - - fn get_dependencies(&self, p: &P, v: &V) -> Result, Box> { + p: &P, + v: &VS::V, + ) -> Result, Box> { self.dp.get_dependencies(p, v) } - fn should_cancel(&self) -> Result<(), Box> { + fn should_cancel(&self) -> Result<(), Box> { assert!(self.start_time.elapsed().as_secs() < 60); let calls = self.call_count.get(); assert!(calls < self.max_calls); self.call_count.set(calls + 1); Ok(()) } + + fn choose_version( + &self, + package: &P, + range: &VS, + ) -> Result, Box> { + self.dp.choose_version(package, range) + } + + type Priority = DP::Priority; + + fn prioritize(&self, package: &P, range: &VS) -> Self::Priority { + self.dp.prioritize(package, range) + } } +fn timeout_resolve>( + dependency_provider: DP, + name: P, + version: impl Into, +) -> Result, PubGrubError> { + resolve( + &TimeoutDependencyProvider::new(dependency_provider, 50_000), + name, + version, + ) +} + +type NumVS = Range; +type SemVS = Range; + #[test] #[should_panic] fn should_cancel_can_panic() { - let mut dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); - dependency_provider.add_dependencies(0, 0, vec![(666, Range::any())]); + let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); + dependency_provider.add_dependencies(0, 0, [(666, Range::full())]); // Run the algorithm. let _ = resolve( @@ -115,25 +158,15 @@ fn string_names() -> impl Strategy { /// This strategy has a high probability of having valid dependencies pub fn registry_strategy( name: impl Strategy, - bad_name: N, -) -> impl Strategy< - Value = ( - OfflineDependencyProvider, - Vec<(N, NumberVersion)>, - ), -> { +) -> impl Strategy, Vec<(N, NumberVersion)>)> { let max_crates = 40; let max_versions = 15; let shrinkage = 40; let complicated_len = 10usize; - // If this is false than the crate will depend on the nonexistent "bad" - // instead of the complex set we generated for it. - let allow_deps = prop::bool::weighted(0.99); - let a_version = ..(max_versions as u32); - let list_of_versions = btree_map(a_version, allow_deps, 1..=max_versions) + let list_of_versions = btree_set(a_version, 1..=max_versions) .prop_map(move |ver| ver.into_iter().collect::>()); let list_of_crates_with_versions = btree_map(name, list_of_versions, 1..=max_crates); @@ -166,20 +199,14 @@ 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), - Option)>>, - )> = crate_vers_by_name - .iter() - .flat_map(|(name, vers)| { - vers.iter().map(move |x| { - ( - (name.clone(), NumberVersion::from(x.0)), - if x.1 { Some(vec![]) } else { None }, - ) + 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(); + .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); @@ -190,27 +217,27 @@ pub fn registry_strategy( } let s = &crate_vers_by_name[&dep_name]; let s_last_index = s.len() - 1; - let (c, d) = order_index(c, d, s.len()); - - if let (_, Some(deps)) = &mut list_of_pkgid[b] { - deps.push(( - dep_name, - if c == 0 && d == s_last_index { - Range::any() - } else if c == 0 { - Range::strictly_lower_than(s[d].0 + 1) - } else if d == s_last_index { - Range::higher_than(s[c].0) - } else if c == d { - Range::exact(s[c].0) - } else { - Range::between(s[c].0, s[d].0 + 1) - }, - )) - } + let (c, d) = order_index(c, d, s.len() + 1); + + list_of_pkgid[b].1.push(( + dep_name, + if c > s_last_index { + Range::empty() + } else if c == 0 && d >= s_last_index { + Range::full() + } else if c == 0 { + Range::strictly_lower_than(s[d] + 1) + } else if d >= s_last_index { + Range::higher_than(s[c]) + } else if c == d { + Range::singleton(s[c]) + } else { + Range::between(s[c], s[d] + 1) + }, + )); } - let mut dependency_provider = OfflineDependencyProvider::::new(); + let mut dependency_provider = OfflineDependencyProvider::::new(); let complicated_len = std::cmp::min(complicated_len, list_of_pkgid.len()); let complicated: Vec<_> = if reverse_alphabetical { @@ -223,11 +250,7 @@ pub fn registry_strategy( .collect(); for ((name, ver), deps) in list_of_pkgid { - dependency_provider.add_dependencies( - name, - ver, - deps.unwrap_or_else(|| vec![(bad_name.clone(), Range::any())]), - ); + dependency_provider.add_dependencies(name, ver, deps); } (dependency_provider, complicated) @@ -243,7 +266,7 @@ fn meta_test_deep_trees_from_strategy() { let mut dis = [0; 21]; - let strategy = registry_strategy(0u16..665, 666); + let strategy = registry_strategy(0u16..665); let mut test_runner = TestRunner::deterministic(); for _ in 0..128 { let (dependency_provider, cases) = strategy @@ -273,6 +296,110 @@ fn meta_test_deep_trees_from_strategy() { ); } +/// Removes versions from the dependency provider where the retain function returns false. +/// Solutions are constructed as a set of versions. +/// If there are fewer versions available, there are fewer valid solutions available. +/// If there was no solution to a resolution in the original dependency provider, +/// then there must still be no solution with some options removed. +/// If there was a solution to a resolution in the original dependency provider, +/// there may not be a solution after versions are removes iif removed versions were critical for all valid solutions. +fn retain_versions( + dependency_provider: &OfflineDependencyProvider, + mut retain: impl FnMut(&N, &VS::V) -> bool, +) -> OfflineDependencyProvider { + let mut smaller_dependency_provider = OfflineDependencyProvider::new(); + + for n in dependency_provider.packages() { + for v in dependency_provider.versions(n).unwrap() { + if !retain(n, v) { + continue; + } + let deps = match dependency_provider.get_dependencies(&n, &v).unwrap() { + Dependencies::Unknown => panic!(), + Dependencies::Known(deps) => deps, + }; + smaller_dependency_provider.add_dependencies(n.clone(), v.clone(), deps) + } + } + smaller_dependency_provider +} + +/// Removes dependencies from the dependency provider where the retain function returns false. +/// Solutions are constraned by having to fulfill all the dependencies. +/// If there are fewer dependencies required, there are more valid solutions. +/// If there was a solution to a resolution in the original dependency provider, +/// then there must still be a solution after dependencies are removed. +/// If there was no solution to a resolution in the original dependency provider, +/// there may now be a solution after dependencies are removed. +fn retain_dependencies( + dependency_provider: &OfflineDependencyProvider, + mut retain: impl FnMut(&N, &VS::V, &N) -> bool, +) -> OfflineDependencyProvider { + let mut smaller_dependency_provider = OfflineDependencyProvider::new(); + for n in dependency_provider.packages() { + for v in dependency_provider.versions(n).unwrap() { + let deps = match dependency_provider.get_dependencies(&n, &v).unwrap() { + Dependencies::Unknown => panic!(), + Dependencies::Known(deps) => deps, + }; + smaller_dependency_provider.add_dependencies( + n.clone(), + v.clone(), + deps.iter().filter_map(|(dep, range)| { + if !retain(n, v, dep) { + None + } else { + Some((dep.clone(), range.clone())) + } + }), + ); + } + } + smaller_dependency_provider +} + +fn errors_the_same_with_only_report_dependencies( + dependency_provider: OfflineDependencyProvider, + name: N, + ver: NumberVersion, +) { + let Err(PubGrubError::NoSolution(tree)) = + timeout_resolve(dependency_provider.clone(), name.clone(), ver) + else { + return; + }; + + fn recursive( + to_retain: &mut Vec<(N, VS, N)>, + tree: &DerivationTree, + ) { + match tree { + DerivationTree::External(External::FromDependencyOf(n1, vs1, n2, _)) => { + to_retain.push((n1.clone(), vs1.clone(), n2.clone())); + } + DerivationTree::Derived(d) => { + recursive(to_retain, &*d.cause1); + recursive(to_retain, &*d.cause2); + } + _ => {} + } + } + + let mut to_retain = Vec::new(); + recursive(&mut to_retain, &tree); + + let removed_provider = retain_dependencies(&dependency_provider, |p, v, d| { + to_retain + .iter() + .any(|(n1, vs1, n2)| n1 == p && vs1.contains(v) && n2 == d) + }); + + assert!( + timeout_resolve(removed_provider.clone(), name, ver).is_err(), + "The full index errored filtering to only dependencies in the derivation tree succeeded" + ); +} + proptest! { #![proptest_config(ProptestConfig { max_shrink_iters: @@ -290,51 +417,57 @@ proptest! { #[test] /// This test is mostly for profiling. fn prop_passes_string( - (dependency_provider, cases) in registry_strategy(string_names(), "bad".to_owned()) + (dependency_provider, cases) in registry_strategy(string_names()) ) { for (name, ver) in cases { - let _ = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); + _ = timeout_resolve(dependency_provider.clone(), name, ver); } } #[test] /// This test is mostly for profiling. fn prop_passes_int( - (dependency_provider, cases) in registry_strategy(0u16..665, 666) + (dependency_provider, cases) in registry_strategy(0u16..665) ) { for (name, ver) in cases { - let _ = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); + _ = timeout_resolve(dependency_provider.clone(), name, ver); } } #[test] fn prop_sat_errors_the_same( - (dependency_provider, cases) in registry_strategy(0u16..665, 666) + (dependency_provider, cases) in registry_strategy(0u16..665) ) { let mut sat = SatResolve::new(&dependency_provider); for (name, ver) in cases { - if let Ok(s) = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver) { - prop_assert!(sat.sat_is_valid_solution(&s)); - } else { - prop_assert!(!sat.sat_resolve(&name, &ver)); - } + let res = timeout_resolve(dependency_provider.clone(), name, ver); + sat.check_resolve(&res, &name, &ver); + } + } + + #[test] + fn prop_errors_the_same_with_only_report_dependencies( + (dependency_provider, cases) in registry_strategy(0u16..665) + ) { + for (name, ver) in cases { + errors_the_same_with_only_report_dependencies(dependency_provider.clone(), name, ver); } } #[test] /// This tests whether the algorithm is still deterministic. fn prop_same_on_repeated_runs( - (dependency_provider, cases) in registry_strategy(0u16..665, 666) + (dependency_provider, cases) in registry_strategy(0u16..665) ) { for (name, ver) in cases { - let one = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); + let one = timeout_resolve(dependency_provider.clone(), name, ver); for _ in 0..3 { - match (&one, &resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver)) { + match (&one, &timeout_resolve(dependency_provider.clone(), name, ver)) { (Ok(l), Ok(r)) => assert_eq!(l, r), (Err(PubGrubError::NoSolution(derivation_l)), Err(PubGrubError::NoSolution(derivation_r))) => { prop_assert_eq!( - DefaultStringReporter::report(&derivation_l), - DefaultStringReporter::report(&derivation_r) + DefaultStringReporter::report(derivation_l), + DefaultStringReporter::report(derivation_r) )}, _ => panic!("not the same result") } @@ -346,12 +479,12 @@ proptest! { /// [ReverseDependencyProvider] changes what order the candidates /// are tried but not the existence of a solution. fn prop_reversed_version_errors_the_same( - (dependency_provider, cases) in registry_strategy(0u16..665, 666) + (dependency_provider, cases) in registry_strategy(0u16..665) ) { let reverse_provider = OldestVersionsDependencyProvider(dependency_provider.clone()); for (name, ver) in cases { - let l = resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver); - let r = resolve(&TimeoutDependencyProvider::new(reverse_provider.clone(), 50_000), name, ver); + let l = timeout_resolve(dependency_provider.clone(), name, ver); + let r = timeout_resolve(reverse_provider.clone(), name, ver); match (&l, &r) { (Ok(_), Ok(_)) => (), (Err(_), Err(_)) => (), @@ -362,18 +495,18 @@ proptest! { #[test] fn prop_removing_a_dep_cant_break( - (dependency_provider, cases) in registry_strategy(0u16..665, 666), + (dependency_provider, cases) in registry_strategy(0u16..665), indexes_to_remove in prop::collection::vec((any::(), any::(), any::()), 1..10) ) { let packages: Vec<_> = dependency_provider.packages().collect(); - let mut removed_provider = dependency_provider.clone(); + let mut to_remove = Set::new(); for (package_idx, version_idx, dep_idx) in indexes_to_remove { let package = package_idx.get(&packages); let versions: Vec<_> = dependency_provider .versions(package) .unwrap().collect(); let version = version_idx.get(&versions); - let dependencies: Vec<(u16, Range)> = match dependency_provider + let dependencies: Vec<(u16, NumVS)> = match dependency_provider .get_dependencies(package, version) .unwrap() { @@ -381,29 +514,17 @@ proptest! { Dependencies::Known(d) => d.into_iter().collect(), }; if !dependencies.is_empty() { - let dependency = dep_idx.get(&dependencies).0; - removed_provider.add_dependencies( - **package, - **version, - dependencies.into_iter().filter(|x| x.0 != dependency), - ) + to_remove.insert((package, **version, dep_idx.get(&dependencies).0)); } } + let removed_provider = retain_dependencies( + &dependency_provider, + |p, v, d| {!to_remove.contains(&(&p, *v, *d))} + ); for (name, ver) in cases { - if resolve( - &TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), - name, - ver, - ) - .is_ok() - { + if timeout_resolve(dependency_provider.clone(), name, ver).is_ok() { prop_assert!( - resolve( - &TimeoutDependencyProvider::new(removed_provider.clone(), 50_000), - name, - ver - ) - .is_ok(), + timeout_resolve(removed_provider.clone(), name, ver).is_ok(), "full index worked for `{} = \"={}\"` but removing some deps broke it!", name, ver, @@ -414,7 +535,7 @@ proptest! { #[test] fn prop_limited_independence_of_irrelevant_alternatives( - (dependency_provider, cases) in registry_strategy(0u16..665, 666), + (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 @@ -423,29 +544,21 @@ proptest! { dependency_provider .versions(&p) .unwrap() - .map(move |v| (p, v.clone())) + .map(move |&v| (p, v)) }) .collect(); let to_remove: Set<(_, _)> = indexes_to_remove.iter().map(|x| x.get(&all_versions)).cloned().collect(); for (name, ver) in cases { - match resolve(&TimeoutDependencyProvider::new(dependency_provider.clone(), 50_000), name, ver) { + match timeout_resolve(dependency_provider.clone(), name, ver) { Ok(used) => { // If resolution was successful, then unpublishing a version of a crate // that was not selected should not change that. - let mut smaller_dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); - for &(n, v) in &all_versions { - if used.get(&n) == Some(&v) // it was used - || to_remove.get(&(n, v)).is_none() // or it is not one to be removed - { - let deps = match dependency_provider.get_dependencies(&n, &v).unwrap() { - Dependencies::Unknown => panic!(), - Dependencies::Known(deps) => deps, - }; - smaller_dependency_provider.add_dependencies(n, v, deps) - } - } + let smaller_dependency_provider = retain_versions(&dependency_provider, |n, v| { + used.get(&n) == Some(&v) // it was used + || to_remove.get(&(*n, *v)).is_none() // or it is not one to be removed + }); prop_assert!( - resolve(&TimeoutDependencyProvider::new(smaller_dependency_provider.clone(), 50_000), name, ver).is_ok(), + timeout_resolve(smaller_dependency_provider.clone(), name, ver).is_ok(), "unpublishing {:?} stopped `{} = \"={}\"` from working", to_remove, name, @@ -455,19 +568,11 @@ proptest! { Err(_) => { // If resolution was unsuccessful, then it should stay unsuccessful // even if any version of a crate is unpublished. - let mut smaller_dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); - for &(n, v) in &all_versions { - if to_remove.get(&(n, v)).is_none() // it is not one to be removed - { - let deps = match dependency_provider.get_dependencies(&n, &v).unwrap() { - Dependencies::Unknown => panic!(), - Dependencies::Known(deps) => deps, - }; - smaller_dependency_provider.add_dependencies(n, v, deps) - } - } + let smaller_dependency_provider = retain_versions(&dependency_provider, |n, v| { + to_remove.get(&(*n, *v)).is_some() // it is one to be removed + }); prop_assert!( - resolve(&TimeoutDependencyProvider::new(smaller_dependency_provider.clone(), 50_000), name, ver).is_err(), + timeout_resolve(smaller_dependency_provider.clone(), name, ver).is_err(), "full index did not work for `{} = \"={}\"` but unpublishing {:?} fixed it!", name, ver, @@ -485,36 +590,30 @@ fn large_case() { for case in std::fs::read_dir("test-examples").unwrap() { let case = case.unwrap().path(); let name = case.file_name().unwrap().to_string_lossy(); - eprintln!("{}", name); + 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") { - let dependency_provider: OfflineDependencyProvider = + let dependency_provider: OfflineDependencyProvider = ron::de::from_str(&data).unwrap(); let mut sat = SatResolve::new(&dependency_provider); for p in dependency_provider.packages() { - for n in dependency_provider.versions(p).unwrap() { - if let Ok(s) = resolve(&dependency_provider, p.clone(), n.clone()) { - assert!(sat.sat_is_valid_solution(&s)); - } else { - assert!(!sat.sat_resolve(p, &n)); - } + for v in dependency_provider.versions(p).unwrap() { + let res = resolve(&dependency_provider, p.clone(), v); + sat.check_resolve(&res, p, v); } } } else if name.ends_with("str_SemanticVersion.ron") { - let dependency_provider: OfflineDependencyProvider< - &str, - pubgrub::version::SemanticVersion, - > = ron::de::from_str(&data).unwrap(); + let dependency_provider: OfflineDependencyProvider<&str, SemVS> = + ron::de::from_str(&data).unwrap(); let mut sat = SatResolve::new(&dependency_provider); for p in dependency_provider.packages() { - for n in dependency_provider.versions(p).unwrap() { - if let Ok(s) = resolve(&dependency_provider, p.clone(), n.clone()) { - assert!(sat.sat_is_valid_solution(&s)); - } else { - assert!(!sat.sat_resolve(p, &n)); - } + for v in dependency_provider.versions(p).unwrap() { + let res = resolve(&dependency_provider, p.clone(), v); + sat.check_resolve(&res, p, v); } } } + eprintln!(" in {}s", start_time.elapsed().as_secs()) } } diff --git a/tests/sat_dependency_provider.rs b/tests/sat_dependency_provider.rs index fa964ca8..2bfb21e9 100644 --- a/tests/sat_dependency_provider.rs +++ b/tests/sat_dependency_provider.rs @@ -1,23 +1,12 @@ // SPDX-License-Identifier: MPL-2.0 +use pubgrub::error::PubGrubError; use pubgrub::package::Package; use pubgrub::solver::{Dependencies, DependencyProvider, OfflineDependencyProvider}; use pubgrub::type_aliases::{Map, SelectedDependencies}; -use pubgrub::version::Version; +use pubgrub::version_set::VersionSet; use varisat::ExtendFormula; -const fn num_bits() -> usize { - std::mem::size_of::() * 8 -} - -fn log_bits(x: usize) -> usize { - if x == 0 { - return 0; - } - assert!(x > 0); - (num_bits::() as u32 - x.leading_zeros()) as usize -} - fn sat_at_most_one(solver: &mut impl varisat::ExtendFormula, vars: &[varisat::Var]) { if vars.len() <= 1 { return; @@ -32,7 +21,8 @@ fn sat_at_most_one(solver: &mut impl varisat::ExtendFormula, vars: &[varisat::Va } // use the "Binary Encoding" from // https://www.it.uu.se/research/group/astra/ModRef10/papers/Alan%20M.%20Frisch%20and%20Paul%20A.%20Giannoros.%20SAT%20Encodings%20of%20the%20At-Most-k%20Constraint%20-%20ModRef%202010.pdf - let bits: Vec = solver.new_var_iter(log_bits(vars.len())).collect(); + let len_bits = vars.len().ilog2() as usize + 1; + let bits: Vec = solver.new_var_iter(len_bits).collect(); for (i, p) in vars.iter().enumerate() { for (j, &bit) in bits.iter().enumerate() { solver.add_clause(&[p.negative(), bit.lit(((1 << j) & i) > 0)]); @@ -46,17 +36,17 @@ fn sat_at_most_one(solver: &mut impl varisat::ExtendFormula, vars: &[varisat::Va /// /// The SAT library does not optimize for the newer version, /// so the selected packages may not match the real resolver. -pub struct SatResolve { +pub struct SatResolve { solver: varisat::Solver<'static>, - all_versions_by_p: Map>, + all_versions_by_p: Map>, } -impl SatResolve { - pub fn new(dp: &OfflineDependencyProvider) -> Self { +impl SatResolve { + pub fn new(dp: &OfflineDependencyProvider) -> Self { let mut cnf = varisat::CnfFormula::new(); let mut all_versions = vec![]; - let mut all_versions_by_p: Map> = Map::default(); + let mut all_versions_by_p: Map> = Map::default(); for p in dp.packages() { let mut versions_for_p = vec![]; @@ -82,7 +72,7 @@ impl SatResolve { for (p1, range) in &deps { let empty_vec = vec![]; let mut matches: Vec = all_versions_by_p - .get(&p1) + .get(p1) .unwrap_or(&empty_vec) .iter() .filter(|(v1, _)| range.contains(v1)) @@ -110,7 +100,7 @@ impl SatResolve { } } - pub fn sat_resolve(&mut self, name: &P, ver: &V) -> bool { + pub fn resolve(&mut self, name: &P, ver: &VS::V) -> bool { if let Some(vers) = self.all_versions_by_p.get(name) { if let Some((_, var)) = vers.iter().find(|(v, _)| v == ver) { self.solver.assume(&[var.positive()]); @@ -126,16 +116,13 @@ impl SatResolve { } } - pub fn sat_is_valid_solution(&mut self, pids: &SelectedDependencies) -> bool { + pub fn is_valid_solution(&mut self, pids: &SelectedDependencies) -> bool { let mut assumption = vec![]; for (p, vs) in &self.all_versions_by_p { + let pid_for_p = pids.get(p); for (v, var) in vs { - assumption.push(if pids.get(p) == Some(v) { - var.positive() - } else { - var.negative() - }) + assumption.push(var.lit(pid_for_p == Some(v))) } } @@ -145,4 +132,20 @@ impl SatResolve { .solve() .expect("docs say it can't error in default config") } + + pub fn check_resolve( + &mut self, + res: &Result, PubGrubError>, + p: &P, + v: &VS::V, + ) { + match res { + Ok(s) => { + assert!(self.is_valid_solution(s)); + } + Err(_) => { + assert!(!self.resolve(p, v)); + } + } + } } diff --git a/tests/tests.rs b/tests/tests.rs index 7aeed03b..81fd5324 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -5,16 +5,18 @@ use pubgrub::range::Range; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::version::NumberVersion; +type NumVS = Range; + #[test] fn same_result_on_repeated_runs() { - let mut dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); + let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); - dependency_provider.add_dependencies("c", 0, vec![]); - dependency_provider.add_dependencies("c", 2, vec![]); - dependency_provider.add_dependencies("b", 0, vec![]); - dependency_provider.add_dependencies("b", 1, vec![("c", Range::between(0, 1))]); + 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("a", 0, vec![("b", Range::any()), ("c", Range::any())]); + dependency_provider.add_dependencies("a", 0, [("b", Range::full()), ("c", Range::full())]); let name = "a"; let ver = NumberVersion(0); @@ -29,24 +31,24 @@ fn same_result_on_repeated_runs() { #[test] fn should_always_find_a_satisfier() { - let mut dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); - dependency_provider.add_dependencies("a", 0, vec![("b", Range::none())]); + let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); + dependency_provider.add_dependencies("a", 0, [("b", Range::empty())]); assert!(matches!( resolve(&dependency_provider, "a", 0), - Err(PubGrubError::DependencyOnTheEmptySet { .. }) + Err(PubGrubError::NoSolution { .. }) )); - dependency_provider.add_dependencies("c", 0, vec![("a", Range::any())]); + dependency_provider.add_dependencies("c", 0, [("a", Range::full())]); assert!(matches!( resolve(&dependency_provider, "c", 0), - Err(PubGrubError::DependencyOnTheEmptySet { .. }) + Err(PubGrubError::NoSolution { .. }) )); } #[test] fn cannot_depend_on_self() { - let mut dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new(); - dependency_provider.add_dependencies("a", 0, vec![("a", Range::any())]); + let mut dependency_provider = OfflineDependencyProvider::<_, NumVS>::new(); + dependency_provider.add_dependencies("a", 0, [("a", Range::full())]); assert!(matches!( resolve(&dependency_provider, "a", 0), Err(PubGrubError::SelfDependency { .. })