Skip to content

Commit

Permalink
Include version constraints in derivation chains
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Nov 15, 2024
1 parent 2b08d76 commit 3d37271
Show file tree
Hide file tree
Showing 11 changed files with 200 additions and 137 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/uv-distribution-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,4 @@ thiserror = { workspace = true }
tracing = { workspace = true }
url = { workspace = true }
urlencoding = { workspace = true }
version-ranges = { workspace = true }
15 changes: 11 additions & 4 deletions crates/uv-distribution-types/src/derivation.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use uv_normalize::PackageName;
use uv_pep440::Version;
use version_ranges::Ranges;

/// A chain of derivation steps from the root package to the current package, to explain why a
/// package is included in the resolution.
Expand Down Expand Up @@ -63,15 +64,21 @@ impl IntoIterator for DerivationChain {
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct DerivationStep {
/// The name of the package.
name: PackageName,
pub name: PackageName,
/// The version of the package.
version: Version,
pub version: Version,
/// The constraints applied to the subsequent package in the chain.
pub range: Ranges<Version>,
}

impl DerivationStep {
/// Create a [`DerivationStep`] from a package name and version.
pub fn new(name: PackageName, version: Version) -> Self {
Self { name, version }
pub fn new(name: PackageName, version: Version, range: Ranges<Version>) -> Self {
Self {
name,
version,
range,
}
}
}

Expand Down
229 changes: 120 additions & 109 deletions crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,117 +257,15 @@ impl NoSolutionError {
/// implement PEP 440 semantics for local version equality. For example, `1.0.0+foo` needs to
/// satisfy `==1.0.0`.
pub(crate) fn collapse_local_version_segments(derivation_tree: ErrorTree) -> ErrorTree {
/// Remove local versions sentinels (`+[max]`) from the interval.
fn strip_sentinel(
mut lower: Bound<Version>,
mut upper: Bound<Version>,
) -> (Bound<Version>, Bound<Version>) {
match (&lower, &upper) {
(Bound::Unbounded, Bound::Unbounded) => {}
(Bound::Unbounded, Bound::Included(v)) => {
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
if v.local() == LocalVersionSlice::Max {
upper = Bound::Included(v.clone().without_local());
}
}
(Bound::Unbounded, Bound::Excluded(v)) => {
// `<1.0.0+[max]` is equivalent to `<1.0.0`
if v.local() == LocalVersionSlice::Max {
upper = Bound::Excluded(v.clone().without_local());
}
}
(Bound::Included(v), Bound::Unbounded) => {
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
}
(Bound::Included(v), Bound::Included(b)) => {
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
if b.local() == LocalVersionSlice::Max {
upper = Bound::Included(b.clone().without_local());
}
}
(Bound::Included(v), Bound::Excluded(b)) => {
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
// `<1.0.0+[max]` is equivalent to `<1.0.0`
if b.local() == LocalVersionSlice::Max {
upper = Bound::Included(b.clone().without_local());
}
}
(Bound::Excluded(v), Bound::Unbounded) => {
// `>1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
}
(Bound::Excluded(v), Bound::Included(b)) => {
// `>1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
if b.local() == LocalVersionSlice::Max {
upper = Bound::Included(b.clone().without_local());
}
}
(Bound::Excluded(v), Bound::Excluded(b)) => {
// `>1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
// `<1.0.0+[max]` is equivalent to `<1.0.0`
if b.local() == LocalVersionSlice::Max {
upper = Bound::Excluded(b.clone().without_local());
}
}
}
(lower, upper)
}

/// Remove local versions sentinels (`+[max]`) from the version ranges.
#[allow(clippy::needless_pass_by_value)]
fn strip_sentinels(versions: Ranges<Version>) -> Ranges<Version> {
let mut range = Ranges::empty();
for (lower, upper) in versions.iter() {
let (lower, upper) = strip_sentinel(lower.clone(), upper.clone());
range = range.union(&Range::from_range_bounds((lower, upper)));
}
range
}

/// Returns `true` if the range appears to be, e.g., `>1.0.0, <1.0.0+[max]`.
fn is_sentinel(versions: &Ranges<Version>) -> bool {
versions.iter().all(|(lower, upper)| {
let (Bound::Excluded(lower), Bound::Excluded(upper)) = (lower, upper) else {
return false;
};
if lower.local() == LocalVersionSlice::Max {
return false;
}
if upper.local() != LocalVersionSlice::Max {
return false;
}
*lower == upper.clone().without_local()
})
}

fn strip(derivation_tree: ErrorTree) -> Option<ErrorTree> {
match derivation_tree {
DerivationTree::External(External::NotRoot(_, _)) => Some(derivation_tree),
DerivationTree::External(External::NoVersions(package, versions)) => {
if is_sentinel(&versions) {
if SentinelRange::from(&versions).is_sentinel() {
return None;
}

let versions = strip_sentinels(versions);
let versions = SentinelRange::from(&versions).strip();
Some(DerivationTree::External(External::NoVersions(
package, versions,
)))
Expand All @@ -378,14 +276,14 @@ impl NoSolutionError {
package2,
versions2,
)) => {
let versions1 = strip_sentinels(versions1);
let versions2 = strip_sentinels(versions2);
let versions1 = SentinelRange::from(&versions1).strip();
let versions2 = SentinelRange::from(&versions2).strip();
Some(DerivationTree::External(External::FromDependencyOf(
package1, versions1, package2, versions2,
)))
}
DerivationTree::External(External::Custom(package, versions, reason)) => {
let versions = strip_sentinels(versions);
let versions = SentinelRange::from(&versions).strip();
Some(DerivationTree::External(External::Custom(
package, versions, reason,
)))
Expand All @@ -402,10 +300,10 @@ impl NoSolutionError {
.map(|(pkg, term)| {
let term = match term {
Term::Positive(versions) => {
Term::Positive(strip_sentinels(versions))
Term::Positive(SentinelRange::from(&versions).strip())
}
Term::Negative(versions) => {
Term::Negative(strip_sentinels(versions))
Term::Negative(SentinelRange::from(&versions).strip())
}
};
(pkg, term)
Expand Down Expand Up @@ -900,6 +798,119 @@ fn drop_root_dependency_on_project(
}
}

/// A version range that may include local version sentinels (`+[max]`).
#[derive(Debug)]
pub struct SentinelRange<'range>(&'range Range<Version>);

impl<'range> From<&'range Range<Version>> for SentinelRange<'range> {
fn from(range: &'range Range<Version>) -> Self {
Self(range)
}
}

impl<'range> SentinelRange<'range> {
/// Returns `true` if the range appears to be, e.g., `>1.0.0, <1.0.0+[max]`.
pub fn is_sentinel(&self) -> bool {
self.0.iter().all(|(lower, upper)| {
let (Bound::Excluded(lower), Bound::Excluded(upper)) = (lower, upper) else {
return false;
};
if lower.local() == LocalVersionSlice::Max {
return false;
}
if upper.local() != LocalVersionSlice::Max {
return false;
}
*lower == upper.clone().without_local()
})
}

/// Remove local versions sentinels (`+[max]`) from the version ranges.
pub fn strip(&self) -> Ranges<Version> {
let mut range = Ranges::empty();
for (lower, upper) in self.0.iter() {
let (lower, upper) = Self::strip_sentinel(lower.clone(), upper.clone());
range = range.union(&Range::from_range_bounds((lower, upper)));
}
range
}

/// Remove local versions sentinels (`+[max]`) from the interval.
fn strip_sentinel(
mut lower: Bound<Version>,
mut upper: Bound<Version>,
) -> (Bound<Version>, Bound<Version>) {
match (&lower, &upper) {
(Bound::Unbounded, Bound::Unbounded) => {}
(Bound::Unbounded, Bound::Included(v)) => {
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
if v.local() == LocalVersionSlice::Max {
upper = Bound::Included(v.clone().without_local());
}
}
(Bound::Unbounded, Bound::Excluded(v)) => {
// `<1.0.0+[max]` is equivalent to `<1.0.0`
if v.local() == LocalVersionSlice::Max {
upper = Bound::Excluded(v.clone().without_local());
}
}
(Bound::Included(v), Bound::Unbounded) => {
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
}
(Bound::Included(v), Bound::Included(b)) => {
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
if b.local() == LocalVersionSlice::Max {
upper = Bound::Included(b.clone().without_local());
}
}
(Bound::Included(v), Bound::Excluded(b)) => {
// `>=1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
// `<1.0.0+[max]` is equivalent to `<1.0.0`
if b.local() == LocalVersionSlice::Max {
upper = Bound::Included(b.clone().without_local());
}
}
(Bound::Excluded(v), Bound::Unbounded) => {
// `>1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
}
(Bound::Excluded(v), Bound::Included(b)) => {
// `>1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
// `<=1.0.0+[max]` is equivalent to `<=1.0.0`
if b.local() == LocalVersionSlice::Max {
upper = Bound::Included(b.clone().without_local());
}
}
(Bound::Excluded(v), Bound::Excluded(b)) => {
// `>1.0.0+[max]` is equivalent to `>1.0.0`
if v.local() == LocalVersionSlice::Max {
lower = Bound::Excluded(v.clone().without_local());
}
// `<1.0.0+[max]` is equivalent to `<1.0.0`
if b.local() == LocalVersionSlice::Max {
upper = Bound::Excluded(b.clone().without_local());
}
}
}
(lower, upper)
}
}

#[derive(Debug)]
pub struct NoSolutionHeader {
/// The [`ResolverEnvironment`] that caused the failure.
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
pub use dependency_mode::DependencyMode;
pub use error::{NoSolutionError, NoSolutionHeader, ResolveError};
pub use error::{NoSolutionError, NoSolutionHeader, ResolveError, SentinelRange};
pub use exclude_newer::ExcludeNewer;
pub use exclusions::Exclusions;
pub use flat_index::{FlatDistributions, FlatIndex};
Expand Down
24 changes: 17 additions & 7 deletions crates/uv-resolver/src/resolver/derivation.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::collections::VecDeque;

use petgraph::Direction;
use pubgrub::{Kind, SelectedDependencies, State};
use pubgrub::{Kind, Range, SelectedDependencies, State};
use rustc_hash::FxHashSet;

use uv_distribution_types::{
Expand Down Expand Up @@ -58,6 +58,7 @@ impl DerivationChainBuilder {
path.push(DerivationStep::new(
dist.name().clone(),
dist.version().clone(),
Range::empty(),
));
for neighbor in resolution
.graph()
Expand Down Expand Up @@ -88,20 +89,29 @@ impl DerivationChainBuilder {
solution: &SelectedDependencies<UvDependencyProvider>,
path: &mut Vec<DerivationStep>,
) -> bool {
// Retrieve the incompatiblies for the current package.
let Some(incompats) = state.incompatibilities.get(package) else {
// Retrieve the incompatibilities for the current package.
let Some(incompatibilities) = state.incompatibilities.get(package) else {
return false;
};
for index in incompats {
for index in incompatibilities {
let incompat = &state.incompatibility_store[*index];

// Find a dependency from a package to the current package.
if let Kind::FromDependencyOf(p1, _v1, p2, v2) = &incompat.kind {
if let Kind::FromDependencyOf(p1, _, p2, v2) = &incompat.kind {
if p2 == package && v2.contains(version) {
if let Some(version) = solution.get(p1) {
if let Some(name) = p1.name_no_root() {
if p1.name_no_root() == p2.name_no_root() {
// Skip proxied dependencies.
if find_path(p1, version, state, solution, path) {
return true;
}
} else if let Some(name) = p1.name_no_root() {
// Add to the current path.
path.push(DerivationStep::new(name.clone(), version.clone()));
path.push(DerivationStep::new(
name.clone(),
version.clone(),
v2.clone(),
));

// Recursively search the next package.
if find_path(p1, version, state, solution, path) {
Expand Down
1 change: 1 addition & 0 deletions crates/uv/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ tracing-subscriber = { workspace = true, features = ["json"] }
tracing-tree = { workspace = true }
unicode-width = { workspace = true }
url = { workspace = true }
version-ranges = { workspace = true }
walkdir = { workspace = true }
which = { workspace = true }
zip = { workspace = true }
Expand Down
Loading

0 comments on commit 3d37271

Please sign in to comment.