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 14, 2024
1 parent 9629f13 commit b3bbb50
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 30 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
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() {
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) {
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
132 changes: 119 additions & 13 deletions crates/uv/src/commands/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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<dyn std::error::Error + Send + Sync>;

Expand Down Expand Up @@ -184,14 +185,24 @@ pub(crate) fn download_and_build(sdist: Box<SourceDist>, 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<Ranges<Version>> = 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)
}
}),
Expand Down Expand Up @@ -230,14 +241,24 @@ pub(crate) fn download(sdist: Box<BuiltDist>, 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<Ranges<Version>> = 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)
}
}),
Expand Down Expand Up @@ -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<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.
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(&Ranges::from_range_bounds((lower, upper)));
}
range
}
12 changes: 6 additions & 6 deletions crates/uv/tests/it/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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"]
"#,
)?;

Expand Down Expand Up @@ -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(())
Expand All @@ -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"] }
"#,
)?;

Expand Down Expand Up @@ -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(())
Expand Down Expand Up @@ -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(())
Expand Down

0 comments on commit b3bbb50

Please sign in to comment.