Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify version sets in error reports #8

Closed
wants to merge 10 commits into from
40 changes: 40 additions & 0 deletions .github/workflows/permaref.yaml
Original file line number Diff line number Diff line change
@@ -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:
create-permaref:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

- 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 "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-//' \
)" >> $GITHUB_OUTPUT

- name: Configure Git
run: |
git config user.name "$GITHUB_ACTOR"
git config user.email "[email protected]"

- 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 }}
44 changes: 44 additions & 0 deletions examples/holes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use pubgrub::error::PubGrubError;
use pubgrub::range::Range;
use pubgrub::report::{DefaultStringReporter, Reporter};
use pubgrub::solver::{resolve, OfflineDependencyProvider};
use pubgrub::version::SemanticVersion;
use rustc_hash::FxHashMap;

fn main() {
let mut dependency_provider = OfflineDependencyProvider::<&str, Range<SemanticVersion>>::new();

// root depends on foo...
dependency_provider.add_dependencies("root", (1, 0, 0), vec![("foo", Range::full())]);

for i in 1..5 {
// foo depends on bar...
dependency_provider.add_dependencies("foo", (i, 0, 0), vec![("bar", Range::full())]);
}

let mut versions = FxHashMap::default();
for package in dependency_provider.packages() {
versions.insert(
*package,
dependency_provider
.versions(package)
.expect("Package must have versions")
.cloned()
.collect::<Vec<_>>(),
);
}

match resolve(&dependency_provider, "root", (1, 0, 0)) {
Ok(sol) => println!("{:?}", sol),
Err(PubGrubError::NoSolution(derivation_tree)) => {
let simple_deriviation_tree = derivation_tree.simplify_versions(&versions);
eprintln!("{}", DefaultStringReporter::report(&derivation_tree));
eprintln!("\n----------\n");
eprintln!(
"{}",
DefaultStringReporter::report(&simple_deriviation_tree)
);
}
Err(err) => panic!("{:?}", err),
};
}
2 changes: 1 addition & 1 deletion src/internal/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub struct State<P: Package, VS: VersionSet, Priority: Ord + Clone> {
root_package: P,
root_version: VS::V,

incompatibilities: Map<P, Vec<IncompId<P, VS>>>,
pub incompatibilities: Map<P, Vec<IncompId<P, VS>>>,

/// Store the ids of incompatibilities that are already contradicted
/// and will stay that way until the next conflict and backtrack is operated.
Expand Down
20 changes: 18 additions & 2 deletions src/internal/incompatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,22 @@ use crate::version_set::VersionSet;
#[derive(Debug, Clone)]
pub struct Incompatibility<P: Package, VS: VersionSet> {
package_terms: SmallMap<P, Term<VS>>,
kind: Kind<P, VS>,
pub kind: Kind<P, VS>,
}

/// Type alias of unique identifiers for incompatibilities.
pub type IncompId<P, VS> = Id<Incompatibility<P, VS>>;

#[derive(Debug, Clone)]
enum Kind<P: Package, VS: VersionSet> {
pub enum Kind<P: Package, VS: VersionSet> {
/// Initial incompatibility aiming at picking the root package for the first decision.
NotRoot(P, VS::V),
/// There are no versions in the given range for this package.
NoVersions(P, VS),
/// Dependencies of the package are unavailable for versions in that range.
UnavailableDependencies(P, VS),
/// Dependencies of the package are unusable for versions in that range.
UnusableDependencies(P, VS, Option<String>),
/// Incompatibility coming from the dependencies of a given package.
FromDependencyOf(P, VS, P, VS),
/// Derived from two causes. Stores cause ids.
Expand Down Expand Up @@ -104,6 +106,17 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
}
}

/// Create an incompatibility to remember
/// that a package version is not selectable
/// because its dependencies are not usable.
pub fn unusable_dependencies(package: P, version: VS::V, reason: Option<String>) -> Self {
let set = VS::singleton(version);
Self {
package_terms: SmallMap::One([(package.clone(), Term::Positive(set.clone()))]),
kind: Kind::UnusableDependencies(package, set, reason),
}
}

/// Build an incompatibility from a given dependency.
pub fn from_dependency(package: P, version: VS::V, dep: (&P, &VS)) -> Self {
let set1 = VS::singleton(version);
Expand Down Expand Up @@ -206,6 +219,9 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
Kind::UnavailableDependencies(package, set) => DerivationTree::External(
External::UnavailableDependencies(package.clone(), set.clone()),
),
Kind::UnusableDependencies(package, set, reason) => DerivationTree::External(
External::UnusableDependencies(package.clone(), set.clone(), reason.clone()),
),
Kind::FromDependencyOf(package, set, dep_package, dep_set) => {
DerivationTree::External(External::FromDependencyOf(
package.clone(),
Expand Down
18 changes: 18 additions & 0 deletions src/internal/partial_solution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,24 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> PartialSolution<P, VS, P
}
}

pub fn prioritized_packages(&self) -> impl Iterator<Item = (&P, &VS)> {
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;
self.package_assignments
.get_range(self.changed_this_decision_level..)
.unwrap()
.iter()
.filter(move |(_, 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))
}

pub fn pick_highest_priority_pkg(
&mut self,
prioritizer: impl Fn(&P, &VS) -> Priority,
Expand Down
3 changes: 1 addition & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,7 @@
//! with a cache, you may want to know that some versions
//! do not exist in your cache.

#![allow(clippy::rc_buffer)]
#![warn(missing_docs)]
#![allow(clippy::all, unreachable_pub)]

pub mod error;
pub mod package;
Expand Down
66 changes: 35 additions & 31 deletions src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ impl<V> Range<V> {
segments: SmallVec::one((Included(v1.into()), Excluded(v2.into()))),
}
}

pub fn is_empty(&self) -> bool {
self.segments.is_empty()
}
}

impl<V: Clone> Range<V> {
Expand Down Expand Up @@ -387,36 +391,6 @@ impl<V: Ord + Clone> Range<V> {
Self { segments }.check_invariants()
}

/// Returns a simpler Range that contains the same versions
///
/// For every one of the Versions provided in versions the existing range and
/// the simplified range will agree on whether it is contained.
/// The simplified version may include or exclude versions that are not in versions as the implementation wishes.
/// For example:
/// - If all the versions are contained in the original than the range will be simplified to `full`.
/// - If none of the versions are contained in the original than the range will be simplified to `empty`.
///
/// If versions are not sorted the correctness of this function is not guaranteed.
pub fn simplify<'v, I>(&self, versions: I) -> Self
where
I: Iterator<Item = &'v V> + 'v,
V: 'v,
{
// Return the segment index in the range for each version in the range, None otherwise
let version_locations = versions.scan(0, move |i, v| {
while let Some(segment) = self.segments.get(*i) {
match within_bounds(v, segment) {
Ordering::Less => return Some(None),
Ordering::Equal => return Some(Some(*i)),
Ordering::Greater => *i += 1,
}
}
Some(None)
});
let kept_segments = group_adjacent_locations(version_locations);
self.keep_segments(kept_segments)
}

/// Create a new range with a subset of segments at given location bounds.
///
/// Each new segment is constructed from a pair of segments, taking the
Expand Down Expand Up @@ -466,6 +440,36 @@ impl<T: Debug + Display + Clone + Eq + Ord> VersionSet for Range<T> {
fn union(&self, other: &Self) -> Self {
Range::union(self, other)
}

/// Returns a simpler Range that contains the same versions
///
/// For every one of the Versions provided in versions the existing range and
/// the simplified range will agree on whether it is contained.
/// The simplified version may include or exclude versions that are not in versions as the implementation wishes.
/// For example:
/// - If all the versions are contained in the original than the range will be simplified to `full`.
/// - If none of the versions are contained in the original than the range will be simplified to `empty`.
///
/// If versions are not sorted the correctness of this function is not guaranteed.
fn simplify<'s, I>(&self, versions: I) -> Self
where
I: Iterator<Item = &'s Self::V> + 's,
Self::V: 's,
{
// Return the segment index in the range for each version in the range, None otherwise
let version_locations = versions.scan(0, move |i, v| {
while let Some(segment) = self.segments.get(*i) {
match within_bounds(v, segment) {
Ordering::Less => return Some(None),
Ordering::Equal => return Some(Some(*i)),
Ordering::Greater => *i += 1,
}
}
Some(None)
});
let kept_segments = group_adjacent_locations(version_locations);
self.keep_segments(kept_segments)
}
}

// REPORT ######################################################################
Expand All @@ -486,7 +490,7 @@ impl<V: Display + Eq> Display for Range<V> {
(Included(v), Unbounded) => write!(f, ">={v}")?,
(Included(v), Included(b)) => {
if v == b {
write!(f, "{v}")?
write!(f, "=={v}")?
} else {
write!(f, ">={v}, <={b}")?
}
Expand Down
Loading
Loading