From 3d37271fdb05ee997e37d292f32ec213337e0c08 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 13 Nov 2024 23:11:15 -0500 Subject: [PATCH] Include version constraints in derivation chains --- Cargo.lock | 2 + crates/uv-distribution-types/Cargo.toml | 1 + .../uv-distribution-types/src/derivation.rs | 15 +- crates/uv-resolver/src/error.rs | 229 +++++++++--------- crates/uv-resolver/src/lib.rs | 2 +- crates/uv-resolver/src/resolver/derivation.rs | 24 +- crates/uv/Cargo.toml | 1 + crates/uv/src/commands/diagnostics.rs | 47 +++- crates/uv/tests/it/lock.rs | 12 +- crates/uv/tests/it/pip_compile.rs | 2 +- crates/uv/tests/it/pip_install.rs | 2 +- 11 files changed, 200 insertions(+), 137 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 417c40c83610..b9d8f4225cc5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4459,6 +4459,7 @@ dependencies = [ "uv-virtualenv", "uv-warnings", "uv-workspace", + "version-ranges", "walkdir", "which", "zip", @@ -4905,6 +4906,7 @@ dependencies = [ "uv-pep508", "uv-platform-tags", "uv-pypi-types", + "version-ranges", ] [[package]] diff --git a/crates/uv-distribution-types/Cargo.toml b/crates/uv-distribution-types/Cargo.toml index 123a55ad41f2..c3995f15f59c 100644 --- a/crates/uv-distribution-types/Cargo.toml +++ b/crates/uv-distribution-types/Cargo.toml @@ -43,3 +43,4 @@ thiserror = { workspace = true } tracing = { workspace = true } url = { workspace = true } urlencoding = { workspace = true } +version-ranges = { workspace = true } diff --git a/crates/uv-distribution-types/src/derivation.rs b/crates/uv-distribution-types/src/derivation.rs index e114945fb0da..6084a9f0b056 100644 --- a/crates/uv-distribution-types/src/derivation.rs +++ b/crates/uv-distribution-types/src/derivation.rs @@ -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. @@ -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, } 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) -> Self { + Self { + name, + version, + range, + } } } diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs index cf668c08946e..b783719dc6c3 100644 --- a/crates/uv-resolver/src/error.rs +++ b/crates/uv-resolver/src/error.rs @@ -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, - mut upper: Bound, - ) -> (Bound, Bound) { - 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) -> Ranges { - 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) -> 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 { 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, ))) @@ -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, ))) @@ -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) @@ -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); + +impl<'range> From<&'range Range> for SentinelRange<'range> { + fn from(range: &'range Range) -> 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 { + 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, + mut upper: Bound, + ) -> (Bound, Bound) { + 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. diff --git a/crates/uv-resolver/src/lib.rs b/crates/uv-resolver/src/lib.rs index 919dea1e7b82..af379a95fa44 100644 --- a/crates/uv-resolver/src/lib.rs +++ b/crates/uv-resolver/src/lib.rs @@ -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}; diff --git a/crates/uv-resolver/src/resolver/derivation.rs b/crates/uv-resolver/src/resolver/derivation.rs index fc643a35e029..bbf7856981be 100644 --- a/crates/uv-resolver/src/resolver/derivation.rs +++ b/crates/uv-resolver/src/resolver/derivation.rs @@ -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::{ @@ -58,6 +58,7 @@ impl DerivationChainBuilder { path.push(DerivationStep::new( dist.name().clone(), dist.version().clone(), + Range::empty(), )); for neighbor in resolution .graph() @@ -88,20 +89,29 @@ impl DerivationChainBuilder { solution: &SelectedDependencies, path: &mut Vec, ) -> 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) { diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index 1c0fc5159fba..0aeca208399c 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -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 } diff --git a/crates/uv/src/commands/diagnostics.rs b/crates/uv/src/commands/diagnostics.rs index 8d0a3a2a456e..edabbf4d7ca2 100644 --- a/crates/uv/src/commands/diagnostics.rs +++ b/crates/uv/src/commands/diagnostics.rs @@ -3,9 +3,12 @@ use std::sync::LazyLock; use owo_colors::OwoColorize; use rustc_hash::FxHashMap; +use version_ranges::Ranges; use uv_distribution_types::{BuiltDist, DerivationChain, Name, SourceDist}; use uv_normalize::PackageName; +use uv_pep440::Version; +use uv_resolver::SentinelRange; use crate::commands::pip; @@ -184,14 +187,28 @@ pub(crate) fn download_and_build(sdist: Box, chain: &DerivationChain None } else { let mut message = format!("`{}` was included because", sdist.name().cyan()); + let mut range: Option> = None; for (i, step) in chain.iter().enumerate() { - if i == 0 { - message = format!("{message} `{}` depends on", step.cyan()); + if i > 0 { + if let Some(range) = range.filter(|range| !range.is_empty()) { + message = format!( + "{message} `{}` which depends on", + format!("{step} ({range})").cyan() + ); + } else { + message = format!("{message} `{}` which depends on", step.cyan()); + } } else { - message = format!("{message} `{}` which depends on", step.cyan()); + message = format!("{message} `{}` depends on", step.cyan()); } + range = Some(SentinelRange::from(&step.range).strip()); + } + let step = sdist.name(); + if let Some(range) = range.filter(|range| !range.is_empty()) { + message = format!("{message} `{}`", format!("{step} ({range})").cyan()); + } else { + message = format!("{message} `{}`", step.cyan()); } - message = format!("{message} `{}`", sdist.name().cyan()); Some(message) } }), @@ -230,14 +247,28 @@ pub(crate) fn download(sdist: Box, chain: &DerivationChain, cause: Er None } else { let mut message = format!("`{}` was included because", sdist.name().cyan()); + let mut range: Option> = None; for (i, step) in chain.iter().enumerate() { - if i == 0 { - message = format!("{message} `{}` depends on", step.cyan()); + if i > 0 { + if let Some(range) = range.filter(|range| !range.is_empty()) { + message = format!( + "{message} `{}` which depends on", + format!("{step} ({range})").cyan() + ); + } else { + message = format!("{message} `{}` which depends on", step.cyan()); + } } else { - message = format!("{message} `{}` which depends on", step.cyan()); + message = format!("{message} `{}` depends on", step.cyan()); } + range = Some(SentinelRange::from(&step.range).strip()); + } + let step = sdist.name(); + if let Some(range) = range.filter(|range| !range.is_empty()) { + message = format!("{message} `{}`", format!("{step} ({range})").cyan()); + } else { + message = format!("{message} `{}`", step.cyan()); } - message = format!("{message} `{}`", sdist.name().cyan()); Some(message) } }), diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index c6e058677c11..940ba11780f3 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -19832,7 +19832,7 @@ fn lock_dynamic_version() -> Result<()> { } #[test] -fn lock_derivation_chain() -> Result<()> { +fn lock_derivation_chain_prod() -> Result<()> { let context = TestContext::new("3.12"); let pyproject_toml = context.temp_dir.child("pyproject.toml"); @@ -19842,7 +19842,7 @@ fn lock_derivation_chain() -> Result<()> { name = "project" version = "0.1.0" requires-python = ">=3.12" - dependencies = ["wsgiref"] + dependencies = ["wsgiref==0.1.2"] "#, )?; @@ -19882,7 +19882,7 @@ fn lock_derivation_chain() -> Result<()> { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)? - help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref` + help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref (==0.1.2)` "###); Ok(()) @@ -19900,7 +19900,7 @@ fn lock_derivation_chain_extra() -> Result<()> { version = "0.1.0" requires-python = ">=3.12" dependencies = [] - optional-dependencies = { wsgi = ["wsgiref"] } + optional-dependencies = { wsgi = ["wsgiref>=0.1"] } "#, )?; @@ -19940,7 +19940,7 @@ fn lock_derivation_chain_extra() -> Result<()> { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)? - help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref` + help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref (>=0.1)` "###); Ok(()) @@ -20000,7 +20000,7 @@ fn lock_derivation_chain_group() -> Result<()> { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)? - help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref` + help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref (*)` "###); Ok(()) diff --git a/crates/uv/tests/it/pip_compile.rs b/crates/uv/tests/it/pip_compile.rs index e67b0980d631..c6cfd19c7214 100644 --- a/crates/uv/tests/it/pip_compile.rs +++ b/crates/uv/tests/it/pip_compile.rs @@ -13292,7 +13292,7 @@ fn compile_derivation_chain() -> Result<()> { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)? - help: `wsgiref` was included because `child==0.1.0` depends on `wsgiref` + help: `wsgiref` was included because `child==0.1.0` depends on `wsgiref (*)` "### ); diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 2c1e2442553e..1cdd0b6879f5 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -7372,7 +7372,7 @@ fn resolve_derivation_chain() -> Result<()> { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)? - help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref` + help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref (*)` "### );