From b3bbb503e3f09ee5573077c5d85027aa8673289b 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/resolver/derivation.rs | 24 +++- crates/uv/Cargo.toml | 1 + crates/uv/src/commands/diagnostics.rs | 132 ++++++++++++++++-- crates/uv/tests/it/lock.rs | 12 +- 7 files changed, 157 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a06f6632f4cd..f785ff80fc15 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4441,6 +4441,7 @@ dependencies = [ "uv-virtualenv", "uv-warnings", "uv-workspace", + "version-ranges", "walkdir", "which", "zip", @@ -4887,6 +4888,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/resolver/derivation.rs b/crates/uv-resolver/src/resolver/derivation.rs index 0db7f24c6f59..bd1f4c684563 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() { + if p1.name() == p2.name() { + // Skip proxied dependencies. + if find_path(p1, version, state, solution, path) { + return true; + } + } else if let Some(name) = p1.name() { // 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 1d0af552e2d7..2e9e0206e50e 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..361c4a3d6a4d 100644 --- a/crates/uv/src/commands/diagnostics.rs +++ b/crates/uv/src/commands/diagnostics.rs @@ -1,13 +1,14 @@ +use std::collections::Bound; use std::str::FromStr; use std::sync::LazyLock; +use crate::commands::pip; use owo_colors::OwoColorize; use rustc_hash::FxHashMap; - use uv_distribution_types::{BuiltDist, DerivationChain, Name, SourceDist}; use uv_normalize::PackageName; - -use crate::commands::pip; +use uv_pep440::{LocalVersionSlice, Version}; +use version_ranges::Ranges; type Error = Box; @@ -184,14 +185,24 @@ pub(crate) fn download_and_build(sdist: Box, chain: &DerivationChain None } else { let mut message = format!("`{}` was included because", sdist.name().cyan()); - for (i, step) in chain.iter().enumerate() { - if i == 0 { - message = format!("{message} `{}` depends on", step.cyan()); + let mut range: Option> = None; + for step in chain { + if let Some(range) = range { + message = format!( + "{message} `{}` which depends on", + format!("{step} ({range})").cyan() + ); } else { - message = format!("{message} `{}` which depends on", step.cyan()); + message = format!("{message} `{}` depends on", step.cyan()); } + range = Some(strip_sentinels(&step.range)); + } + 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 +241,24 @@ pub(crate) fn download(sdist: Box, chain: &DerivationChain, cause: Er None } else { let mut message = format!("`{}` was included because", sdist.name().cyan()); - for (i, step) in chain.iter().enumerate() { - if i == 0 { - message = format!("{message} `{}` depends on", step.cyan()); + let mut range: Option> = None; + for step in chain { + if let Some(range) = range { + message = format!( + "{message} `{}` which depends on", + format!("{step} ({range})").cyan() + ); } else { - message = format!("{message} `{}` which depends on", step.cyan()); + message = format!("{message} `{}` depends on", step.cyan()); } + range = Some(strip_sentinels(&step.range)); + } + 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) } }), @@ -327,3 +348,88 @@ pub(crate) fn no_solution_hint(err: uv_resolver::NoSolutionError, help: String) let report = miette::Report::new(Error { header, err, help }); anstream::eprint!("{report:?}"); } + +/// 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. +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(&Ranges::from_range_bounds((lower, upper))); + } + range +} diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index 674ba17e5843..4c5b2828c4f3 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -19229,7 +19229,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"); @@ -19239,7 +19239,7 @@ fn lock_derivation_chain() -> Result<()> { name = "project" version = "0.1.0" requires-python = ">=3.12" - dependencies = ["wsgiref"] + dependencies = ["wsgiref==0.1.2"] "#, )?; @@ -19279,7 +19279,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(()) @@ -19297,7 +19297,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"] } "#, )?; @@ -19337,7 +19337,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(()) @@ -19397,7 +19397,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(())