From b4e9a128b50390e3c2558d8d2e47bcee7d99bf4b Mon Sep 17 00:00:00 2001 From: konstin Date: Tue, 21 May 2024 11:52:04 +0200 Subject: [PATCH] Arc-wrap `PubGrubPackage` for cheap cloning in pubgrub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pubgrub stores incompatibilities as (package name, version range) tuples, meaning it needs to clone the package name for each incompatibility, and each non-borrowed operation on incompatibilities. https://github.com/astral-sh/uv/pull/3673 made me realize that `PubGrubPackage` has gotten large (expensive to copy), so like `Version` and other structs, i've added an `Arc` wrapper around it. It's a pity clippy forbids `.deref()`, it's less opaque than `&**` and has IDE support (clicking on `.deref()` jumps to the right impl). ## Benchmarks It looks like this matters most for complex resolutions which, i assume because they carry larger `PubGrubPackageInner::Package` and `PubGrubPackageInner::Extra` types. ```bash hyperfine --warmup 5 "./uv-main pip compile -q ./scripts/requirements/jupyter.in" "./uv-branch pip compile -q ./scripts/requirements/jupyter.in" hyperfine --warmup 5 "./uv-main pip compile -q ./scripts/requirements/airflow.in" "./uv-branch pip compile -q ./scripts/requirements/airflow.in" hyperfine --warmup 5 "./uv-main pip compile -q ./scripts/requirements/boto3.in" "./uv-branch pip compile -q ./scripts/requirements/boto3.in" ``` ``` Benchmark 1: ./uv-main pip compile -q ./scripts/requirements/jupyter.in Time (mean ± σ): 18.2 ms ± 1.6 ms [User: 14.4 ms, System: 26.0 ms] Range (min … max): 15.8 ms … 22.5 ms 181 runs Benchmark 2: ./uv-branch pip compile -q ./scripts/requirements/jupyter.in Time (mean ± σ): 17.8 ms ± 1.4 ms [User: 14.4 ms, System: 25.3 ms] Range (min … max): 15.4 ms … 23.1 ms 159 runs Summary ./uv-branch pip compile -q ./scripts/requirements/jupyter.in ran 1.02 ± 0.12 times faster than ./uv-main pip compile -q ./scripts/requirements/jupyter.in ``` ``` Benchmark 1: ./uv-main pip compile -q ./scripts/requirements/airflow.in Time (mean ± σ): 153.7 ms ± 3.5 ms [User: 165.2 ms, System: 157.6 ms] Range (min … max): 150.4 ms … 163.0 ms 19 runs Benchmark 2: ./uv-branch pip compile -q ./scripts/requirements/airflow.in Time (mean ± σ): 123.9 ms ± 4.6 ms [User: 152.4 ms, System: 133.8 ms] Range (min … max): 118.4 ms … 138.1 ms 24 runs Summary ./uv-branch pip compile -q ./scripts/requirements/airflow.in ran 1.24 ± 0.05 times faster than ./uv-main pip compile -q ./scripts/requirements/airflow.in ``` ``` Benchmark 1: ./uv-main pip compile -q ./scripts/requirements/boto3.in Time (mean ± σ): 327.0 ms ± 3.8 ms [User: 344.5 ms, System: 71.6 ms] Range (min … max): 322.7 ms … 334.6 ms 10 runs Benchmark 2: ./uv-branch pip compile -q ./scripts/requirements/boto3.in Time (mean ± σ): 311.2 ms ± 3.1 ms [User: 339.3 ms, System: 63.1 ms] Range (min … max): 307.8 ms … 317.0 ms 10 runs Summary ./uv-branch pip compile -q ./scripts/requirements/boto3.in ran 1.05 ± 0.02 times faster than ./uv-main pip compile -q ./scripts/requirements/boto3.in ``` --- crates/uv-resolver/Cargo.toml | 2 +- crates/uv-resolver/src/error.rs | 34 ++++---- .../uv-resolver/src/pubgrub/dependencies.rs | 22 +++--- crates/uv-resolver/src/pubgrub/mod.rs | 2 +- crates/uv-resolver/src/pubgrub/package.rs | 39 +++++++-- crates/uv-resolver/src/pubgrub/priority.rs | 25 +++--- crates/uv-resolver/src/pubgrub/report.rs | 45 ++++++----- crates/uv-resolver/src/resolution/graph.rs | 22 +++--- .../src/resolver/batch_prefetch.rs | 11 ++- crates/uv-resolver/src/resolver/mod.rs | 79 ++++++++++--------- 10 files changed, 159 insertions(+), 122 deletions(-) diff --git a/crates/uv-resolver/Cargo.toml b/crates/uv-resolver/Cargo.toml index 5dfbbcb2b7e8..97400263f40d 100644 --- a/crates/uv-resolver/Cargo.toml +++ b/crates/uv-resolver/Cargo.toml @@ -37,6 +37,7 @@ anstream = { workspace = true } anyhow = { workspace = true } chrono = { workspace = true } clap = { workspace = true, features = ["derive"], optional = true } +dashmap = { workspace = true } derivative = { workspace = true } either = { workspace = true } futures = { workspace = true } @@ -56,7 +57,6 @@ tokio = { workspace = true } tokio-stream = { workspace = true } tracing = { workspace = true } url = { workspace = true } -dashmap = { workspace = true } [dev-dependencies] uv-interpreter = { workspace = true } diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs index 7ca4536bbd33..ebaf41f904ed 100644 --- a/crates/uv-resolver/src/error.rs +++ b/crates/uv-resolver/src/error.rs @@ -16,7 +16,7 @@ use uv_normalize::PackageName; use crate::candidate_selector::CandidateSelector; use crate::dependency_provider::UvDependencyProvider; -use crate::pubgrub::{PubGrubPackage, PubGrubPython, PubGrubReportFormatter}; +use crate::pubgrub::{PubGrubPackage, PubGrubPackageInner, PubGrubPython, PubGrubReportFormatter}; use crate::python_requirement::PythonRequirement; use crate::resolver::{ FxOnceMap, IncompletePackage, UnavailablePackage, UnavailableReason, VersionsResponse, @@ -114,7 +114,7 @@ impl From> for ResolveError { } /// Given a [`DerivationTree`], collapse any [`External::FromDependencyOf`] incompatibilities -/// wrap an [`PubGrubPackage::Extra`] package. +/// wrap an [`PubGrubPackageInner::Extra`] package. fn collapse_extra_proxies( derivation_tree: &mut DerivationTree, UnavailableReason>, ) { @@ -126,22 +126,16 @@ fn collapse_extra_proxies( Arc::make_mut(&mut derived.cause2), ) { ( - DerivationTree::External(External::FromDependencyOf( - PubGrubPackage::Extra { .. }, - .., - )), + DerivationTree::External(External::FromDependencyOf(package, ..)), ref mut cause, - ) => { + ) if matches!(&**package, PubGrubPackageInner::Extra { .. }) => { collapse_extra_proxies(cause); *derivation_tree = cause.clone(); } ( ref mut cause, - DerivationTree::External(External::FromDependencyOf( - PubGrubPackage::Extra { .. }, - .., - )), - ) => { + DerivationTree::External(External::FromDependencyOf(package, ..)), + ) if matches!(&**package, PubGrubPackageInner::Extra { .. }) => { collapse_extra_proxies(cause); *derivation_tree = cause.clone(); } @@ -241,22 +235,22 @@ impl NoSolutionError { ) -> Self { let mut available_versions = IndexMap::default(); for package in self.derivation_tree.packages() { - match package { - PubGrubPackage::Root(_) => {} - PubGrubPackage::Python(PubGrubPython::Installed) => { + match &**package { + PubGrubPackageInner::Root(_) => {} + PubGrubPackageInner::Python(PubGrubPython::Installed) => { available_versions.insert( package.clone(), BTreeSet::from([python_requirement.installed().deref().clone()]), ); } - PubGrubPackage::Python(PubGrubPython::Target) => { + PubGrubPackageInner::Python(PubGrubPython::Target) => { available_versions.insert( package.clone(), BTreeSet::from([python_requirement.target().deref().clone()]), ); } - PubGrubPackage::Extra { .. } => {} - PubGrubPackage::Package { name, .. } => { + PubGrubPackageInner::Extra { .. } => {} + PubGrubPackageInner::Package { name, .. } => { // Avoid including available versions for packages that exist in the derivation // tree, but were never visited during resolution. We _may_ have metadata for // these packages, but it's non-deterministic, and omitting them ensures that @@ -304,7 +298,7 @@ impl NoSolutionError { ) -> Self { let mut new = FxHashMap::default(); for package in self.derivation_tree.packages() { - if let PubGrubPackage::Package { name, .. } = package { + if let PubGrubPackageInner::Package { name, .. } = &**package { if let Some(reason) = unavailable_packages.get(name) { new.insert(name.clone(), reason.clone()); } @@ -322,7 +316,7 @@ impl NoSolutionError { ) -> Self { let mut new = FxHashMap::default(); for package in self.derivation_tree.packages() { - if let PubGrubPackage::Package { name, .. } = package { + if let PubGrubPackageInner::Package { name, .. } = &**package { if let Some(versions) = incomplete_packages.get(name) { for entry in versions.iter() { let (version, reason) = entry.pair(); diff --git a/crates/uv-resolver/src/pubgrub/dependencies.rs b/crates/uv-resolver/src/pubgrub/dependencies.rs index 2cc1f8207cd7..eda3c99081c4 100644 --- a/crates/uv-resolver/src/pubgrub/dependencies.rs +++ b/crates/uv-resolver/src/pubgrub/dependencies.rs @@ -10,7 +10,7 @@ use uv_configuration::{Constraints, Overrides}; use uv_normalize::{ExtraName, PackageName}; use crate::pubgrub::specifier::PubGrubSpecifier; -use crate::pubgrub::PubGrubPackage; +use crate::pubgrub::{PubGrubPackage, PubGrubPackageInner}; use crate::resolver::{Locals, Urls}; use crate::ResolveError; @@ -102,8 +102,8 @@ fn add_requirements( })) { let PubGrubRequirement { package, version } = result?; - match &package { - PubGrubPackage::Package { name, .. } => { + match &*package { + PubGrubPackageInner::Package { name, .. } => { // Detect self-dependencies. if source_name.is_some_and(|source_name| source_name == name) { warn!("{name} has a dependency on itself"); @@ -112,7 +112,7 @@ fn add_requirements( dependencies.push((package.clone(), version.clone())); } - PubGrubPackage::Extra { name, extra, .. } => { + PubGrubPackageInner::Extra { name, extra, .. } => { // Recursively add the dependencies of the current package (e.g., `black` depending on // `black[colorama]`). if source_name.is_some_and(|source_name| source_name == name) { @@ -158,7 +158,7 @@ fn add_requirements( PubGrubRequirement::from_constraint(constraint, urls, locals)?; // Ignore self-dependencies. - if let PubGrubPackage::Package { name, .. } = &package { + if let PubGrubPackageInner::Package { name, .. } = &*package { // Detect self-dependencies. if source_name.is_some_and(|source_name| source_name == name) { warn!("{name} has a dependency on itself"); @@ -249,12 +249,12 @@ impl PubGrubRequirement { } Ok(Self { - package: PubGrubPackage::Package { + package: PubGrubPackage::from(PubGrubPackageInner::Package { name: requirement.name.clone(), extra, marker: None, url: Some(expected.clone()), - }, + }), version: Range::full(), }) } @@ -275,12 +275,12 @@ impl PubGrubRequirement { } Ok(Self { - package: PubGrubPackage::Package { + package: PubGrubPackage::from(PubGrubPackageInner::Package { name: requirement.name.clone(), extra, marker: None, url: Some(expected.clone()), - }, + }), version: Range::full(), }) } @@ -301,12 +301,12 @@ impl PubGrubRequirement { } Ok(Self { - package: PubGrubPackage::Package { + package: PubGrubPackage::from(PubGrubPackageInner::Package { name: requirement.name.clone(), extra, marker: None, url: Some(expected.clone()), - }, + }), version: Range::full(), }) } diff --git a/crates/uv-resolver/src/pubgrub/mod.rs b/crates/uv-resolver/src/pubgrub/mod.rs index 1176297cc6cf..ff6c9ae83d95 100644 --- a/crates/uv-resolver/src/pubgrub/mod.rs +++ b/crates/uv-resolver/src/pubgrub/mod.rs @@ -1,6 +1,6 @@ pub(crate) use crate::pubgrub::dependencies::{PubGrubDependencies, PubGrubRequirement}; pub(crate) use crate::pubgrub::distribution::PubGrubDistribution; -pub(crate) use crate::pubgrub::package::{PubGrubPackage, PubGrubPython}; +pub(crate) use crate::pubgrub::package::{PubGrubPackage, PubGrubPackageInner, PubGrubPython}; pub(crate) use crate::pubgrub::priority::{PubGrubPriorities, PubGrubPriority}; pub(crate) use crate::pubgrub::report::PubGrubReportFormatter; pub(crate) use crate::pubgrub::specifier::PubGrubSpecifier; diff --git a/crates/uv-resolver/src/pubgrub/package.rs b/crates/uv-resolver/src/pubgrub/package.rs index c7eaaa3a51fe..0867ac2397ce 100644 --- a/crates/uv-resolver/src/pubgrub/package.rs +++ b/crates/uv-resolver/src/pubgrub/package.rs @@ -1,9 +1,36 @@ use distribution_types::VerbatimParsedUrl; use pep508_rs::MarkerTree; +use std::fmt::{Display, Formatter}; +use std::ops::Deref; +use std::sync::Arc; use uv_normalize::{ExtraName, PackageName}; use crate::resolver::Urls; +/// [`Arc`] wrapper around [`PubGrubPackageInner`] to make cloning (inside PubGrub) cheap. +#[derive(Debug, Clone, Eq, Hash, PartialEq, PartialOrd, Ord)] +pub struct PubGrubPackage(Arc); + +impl Deref for PubGrubPackage { + type Target = PubGrubPackageInner; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl Display for PubGrubPackage { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + Display::fmt(&self.0, f) + } +} + +impl From for PubGrubPackage { + fn from(package: PubGrubPackageInner) -> Self { + Self(Arc::new(package)) + } +} + /// A PubGrub-compatible wrapper around a "Python package", with two notable characteristics: /// /// 1. Includes a [`PubGrubPackage::Root`] variant, to satisfy PubGrub's requirement that a @@ -12,7 +39,7 @@ use crate::resolver::Urls; /// package (e.g., `black[colorama]`), and mark it as a dependency of the real package (e.g., /// `black`). We then discard the virtual packages at the end of the resolution process. #[derive(Debug, Clone, Eq, Hash, PartialEq, PartialOrd, Ord)] -pub enum PubGrubPackage { +pub enum PubGrubPackageInner { /// The root package, which is used to start the resolution process. Root(Option), /// A Python version. @@ -91,19 +118,19 @@ impl PubGrubPackage { ) -> Self { let url = urls.get(&name).cloned(); if let Some(extra) = extra { - Self::Extra { + Self(Arc::new(PubGrubPackageInner::Extra { name, extra, marker, url, - } + })) } else { - Self::Package { + Self(Arc::new(PubGrubPackageInner::Package { name, extra, marker, url, - } + })) } } } @@ -116,7 +143,7 @@ pub enum PubGrubPython { Target, } -impl std::fmt::Display for PubGrubPackage { +impl std::fmt::Display for PubGrubPackageInner { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Root(name) => { diff --git a/crates/uv-resolver/src/pubgrub/priority.rs b/crates/uv-resolver/src/pubgrub/priority.rs index f203411a90c0..40bee0df13af 100644 --- a/crates/uv-resolver/src/pubgrub/priority.rs +++ b/crates/uv-resolver/src/pubgrub/priority.rs @@ -7,6 +7,7 @@ use pep440_rs::Version; use uv_normalize::PackageName; use crate::pubgrub::package::PubGrubPackage; +use crate::pubgrub::PubGrubPackageInner; /// A prioritization map to guide the PubGrub resolution process. /// @@ -24,14 +25,14 @@ impl PubGrubPriorities { /// Add a [`PubGrubPackage`] to the priority map. pub(crate) fn insert(&mut self, package: &PubGrubPackage, version: &Range) { let next = self.0.len(); - match package { - PubGrubPackage::Root(_) => {} - PubGrubPackage::Python(_) => {} + match &**package { + PubGrubPackageInner::Root(_) => {} + PubGrubPackageInner::Python(_) => {} - PubGrubPackage::Extra { + PubGrubPackageInner::Extra { name, url: None, .. } - | PubGrubPackage::Package { + | PubGrubPackageInner::Package { name, url: None, .. } => { match self.0.entry(name.clone()) { @@ -66,10 +67,10 @@ impl PubGrubPriorities { } } } - PubGrubPackage::Extra { + PubGrubPackageInner::Extra { name, url: Some(_), .. } - | PubGrubPackage::Package { + | PubGrubPackageInner::Package { name, url: Some(_), .. } => { match self.0.entry(name.clone()) { @@ -101,11 +102,11 @@ impl PubGrubPriorities { /// Return the [`PubGrubPriority`] of the given package, if it exists. pub(crate) fn get(&self, package: &PubGrubPackage) -> Option { - match package { - PubGrubPackage::Root(_) => Some(PubGrubPriority::Root), - PubGrubPackage::Python(_) => Some(PubGrubPriority::Root), - PubGrubPackage::Extra { name, .. } => self.0.get(name).copied(), - PubGrubPackage::Package { name, .. } => self.0.get(name).copied(), + match &**package { + PubGrubPackageInner::Root(_) => Some(PubGrubPriority::Root), + PubGrubPackageInner::Python(_) => Some(PubGrubPriority::Root), + PubGrubPackageInner::Extra { name, .. } => self.0.get(name).copied(), + PubGrubPackageInner::Package { name, .. } => self.0.get(name).copied(), } } } diff --git a/crates/uv-resolver/src/pubgrub/report.rs b/crates/uv-resolver/src/pubgrub/report.rs index dafd4d97c1f5..b69e96f961a0 100644 --- a/crates/uv-resolver/src/pubgrub/report.rs +++ b/crates/uv-resolver/src/pubgrub/report.rs @@ -19,7 +19,7 @@ use crate::candidate_selector::CandidateSelector; use crate::python_requirement::PythonRequirement; use crate::resolver::{IncompletePackage, UnavailablePackage, UnavailableReason}; -use super::PubGrubPackage; +use super::{PubGrubPackage, PubGrubPackageInner}; #[derive(Debug)] pub(crate) struct PubGrubReportFormatter<'a> { @@ -44,7 +44,7 @@ impl ReportFormatter, UnavailableReason> format!("we are solving dependencies of {package} {version}") } External::NoVersions(package, set) => { - if matches!(package, PubGrubPackage::Python(_)) { + if matches!(&**package, PubGrubPackageInner::Python(_)) { if let Some(python) = self.python_requirement { if python.target() == python.installed() { // Simple case, the installed version is the same as the target version @@ -107,11 +107,11 @@ impl ReportFormatter, UnavailableReason> } } } - External::Custom(package, set, reason) => match package { - PubGrubPackage::Root(Some(name)) => { + External::Custom(package, set, reason) => match &**package { + PubGrubPackageInner::Root(Some(name)) => { format!("{name} cannot be used because {reason}") } - PubGrubPackage::Root(None) => { + PubGrubPackageInner::Root(None) => { format!("your requirements cannot be used because {reason}") } _ => match reason { @@ -131,12 +131,12 @@ impl ReportFormatter, UnavailableReason> External::FromDependencyOf(package, package_set, dependency, dependency_set) => { let package_set = self.simplify_set(package_set, package); let dependency_set = self.simplify_set(dependency_set, dependency); - match package { - PubGrubPackage::Root(Some(name)) => format!( + match &**package { + PubGrubPackageInner::Root(Some(name)) => format!( "{name} depends on {}", PackageRange::dependency(dependency, &dependency_set) ), - PubGrubPackage::Root(None) => format!( + PubGrubPackageInner::Root(None) => format!( "you require {}", PackageRange::dependency(dependency, &dependency_set) ), @@ -157,15 +157,22 @@ impl ReportFormatter, UnavailableReason> // by package first. terms_vec.sort_by(|&(pkg1, _), &(pkg2, _)| pkg1.cmp(pkg2)); match terms_vec.as_slice() { - [] | [(PubGrubPackage::Root(_), _)] => "the requirements are unsatisfiable".into(), - [(package @ PubGrubPackage::Package { .. }, Term::Positive(range))] => { + [] => "the requirements are unsatisfiable".into(), + [(root, _)] if matches!(&**(*root), PubGrubPackageInner::Root(_)) => { + "the requirements are unsatisfiable".into() + } + [(package, Term::Positive(range))] + if matches!(&**(*package), PubGrubPackageInner::Package { .. }) => + { let range = self.simplify_set(range, package); format!( "{} cannot be used", PackageRange::compatibility(package, &range) ) } - [(package @ PubGrubPackage::Package { .. }, Term::Negative(range))] => { + [(package, Term::Negative(range))] + if matches!(&**(*package), PubGrubPackageInner::Package { .. }) => + { let range = self.simplify_set(range, package); format!( "{} must be used", @@ -339,13 +346,13 @@ impl PubGrubReportFormatter<'_> { let dependency_set2 = self.simplify_set(dependency_set2, dependency2); let dependency2 = PackageRange::dependency(dependency2, &dependency_set2); - match package1 { - PubGrubPackage::Root(Some(name)) => format!( + match &**package1 { + PubGrubPackageInner::Root(Some(name)) => format!( "{name} depends on {}and {}", Padded::new("", &dependency1, " "), dependency2, ), - PubGrubPackage::Root(None) => format!( + PubGrubPackageInner::Root(None) => format!( "you require {}and {}", Padded::new("", &dependency1, " "), dependency2, @@ -402,7 +409,7 @@ impl PubGrubReportFormatter<'_> { ) -> IndexSet { /// Returns `true` if pre-releases were allowed for a package. fn allowed_prerelease(package: &PubGrubPackage, selector: &CandidateSelector) -> bool { - let PubGrubPackage::Package { name, .. } = package else { + let PubGrubPackageInner::Package { name, .. } = &**package else { return false; }; selector.prerelease_strategy().allows(name) @@ -460,7 +467,7 @@ impl PubGrubReportFormatter<'_> { let no_find_links = index_locations.flat_index().peekable().peek().is_none(); - if let PubGrubPackage::Package { name, .. } = package { + if let PubGrubPackageInner::Package { name, .. } = &**package { // Add hints due to the package being entirely unavailable. match unavailable_packages.get(name) { Some(UnavailablePackage::NoIndex) => { @@ -970,9 +977,9 @@ impl std::fmt::Display for Padded<'_, T> { } fn fmt_package(package: &PubGrubPackage) -> String { - match package { - PubGrubPackage::Root(Some(name)) => name.to_string(), - PubGrubPackage::Root(None) => "you require".to_string(), + match &**package { + PubGrubPackageInner::Root(Some(name)) => name.to_string(), + PubGrubPackageInner::Root(None) => "you require".to_string(), _ => format!("{package}"), } } diff --git a/crates/uv-resolver/src/resolution/graph.rs b/crates/uv-resolver/src/resolution/graph.rs index 20aeeebe4e40..5e240c997157 100644 --- a/crates/uv-resolver/src/resolution/graph.rs +++ b/crates/uv-resolver/src/resolution/graph.rs @@ -18,7 +18,7 @@ use crate::dependency_provider::UvDependencyProvider; use crate::editables::Editables; use crate::pins::FilePins; use crate::preferences::Preferences; -use crate::pubgrub::{PubGrubDistribution, PubGrubPackage}; +use crate::pubgrub::{PubGrubDistribution, PubGrubPackageInner}; use crate::redirect::url_to_precise; use crate::resolution::AnnotatedDist; use crate::resolver::FxOnceMap; @@ -55,8 +55,8 @@ impl ResolutionGraph { let mut extras = FxHashMap::default(); let mut diagnostics = Vec::new(); for (package, version) in selection { - match package { - PubGrubPackage::Package { + match &**package { + PubGrubPackageInner::Package { name, extra: Some(extra), marker: None, @@ -95,7 +95,7 @@ impl ResolutionGraph { }); } } - PubGrubPackage::Package { + PubGrubPackageInner::Package { name, extra: Some(extra), marker: None, @@ -159,8 +159,8 @@ impl ResolutionGraph { FxHashMap::with_capacity_and_hasher(selection.len(), BuildHasherDefault::default()); for (package, version) in selection { - match package { - PubGrubPackage::Package { + match &**package { + PubGrubPackageInner::Package { name, extra: None, marker: None, @@ -229,7 +229,7 @@ impl ResolutionGraph { }); inverse.insert(name, index); } - PubGrubPackage::Package { + PubGrubPackageInner::Package { name, extra: None, marker: None, @@ -328,16 +328,16 @@ impl ResolutionGraph { continue; } - let PubGrubPackage::Package { + let PubGrubPackageInner::Package { name: self_name, .. - } = self_package + } = &**self_package else { continue; }; - let PubGrubPackage::Package { + let PubGrubPackageInner::Package { name: dependency_name, .. - } = dependency_package + } = &**dependency_package else { continue; }; diff --git a/crates/uv-resolver/src/resolver/batch_prefetch.rs b/crates/uv-resolver/src/resolver/batch_prefetch.rs index edfe18367898..05535d58eb23 100644 --- a/crates/uv-resolver/src/resolver/batch_prefetch.rs +++ b/crates/uv-resolver/src/resolver/batch_prefetch.rs @@ -10,7 +10,7 @@ use distribution_types::DistributionMetadata; use pep440_rs::Version; use crate::candidate_selector::{CandidateDist, CandidateSelector}; -use crate::pubgrub::PubGrubPackage; +use crate::pubgrub::{PubGrubPackage, PubGrubPackageInner}; use crate::resolver::Request; use crate::{InMemoryIndex, ResolveError, VersionsResponse}; @@ -53,12 +53,12 @@ impl BatchPrefetcher { index: &InMemoryIndex, selector: &CandidateSelector, ) -> anyhow::Result<(), ResolveError> { - let PubGrubPackage::Package { + let PubGrubPackageInner::Package { name, extra: None, marker: None, url: None, - } = &next + } = &**next else { return Ok(()); }; @@ -163,7 +163,10 @@ impl BatchPrefetcher { /// Each time we tried a version for a package, we register that here. pub(crate) fn version_tried(&mut self, package: PubGrubPackage) { // Only track base packages, no virtual packages from extras. - if matches!(package, PubGrubPackage::Package { extra: Some(_), .. }) { + if matches!( + &*package, + PubGrubPackageInner::Package { extra: Some(_), .. } + ) { return; } *self.tried_versions.entry(package).or_default() += 1; diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index fcd43ff2227c..2507841b4b4f 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -43,8 +43,8 @@ use crate::manifest::Manifest; use crate::pins::FilePins; use crate::preferences::Preferences; use crate::pubgrub::{ - PubGrubDependencies, PubGrubDistribution, PubGrubPackage, PubGrubPriorities, PubGrubPython, - PubGrubRequirement, PubGrubSpecifier, + PubGrubDependencies, PubGrubDistribution, PubGrubPackage, PubGrubPackageInner, + PubGrubPriorities, PubGrubPython, PubGrubRequirement, PubGrubSpecifier, }; use crate::python_requirement::PythonRequirement; use crate::resolution::ResolutionGraph; @@ -401,7 +401,7 @@ impl ResolverState, request_sink: Sender, ) -> Result { - let root = PubGrubPackage::Root(self.project.clone()); + let root = PubGrubPackage::from(PubGrubPackageInner::Root(self.project.clone())); let mut prefetcher = BatchPrefetcher::default(); let mut state = SolveState { pubgrub: State::init(root.clone(), MIN_VERSION.clone()), @@ -480,7 +480,7 @@ impl ResolverState ResolverState ResolverState, ) -> Result<(), ResolveError> { - match package { - PubGrubPackage::Root(_) => {} - PubGrubPackage::Python(_) => {} - PubGrubPackage::Extra { .. } => {} - PubGrubPackage::Package { + match &**package { + PubGrubPackageInner::Root(_) => {} + PubGrubPackageInner::Python(_) => {} + PubGrubPackageInner::Extra { .. } => {} + PubGrubPackageInner::Package { name, url: None, .. } => { // Verify that the package is allowed under the hash-checking policy. @@ -650,7 +653,7 @@ impl ResolverState ResolverState ResolverState, request_sink: &Sender, ) -> Result, ResolveError> { - match package { - PubGrubPackage::Root(_) => Ok(Some(ResolverVersion::Available(MIN_VERSION.clone()))), + match &**package { + PubGrubPackageInner::Root(_) => { + Ok(Some(ResolverVersion::Available(MIN_VERSION.clone()))) + } - PubGrubPackage::Python(PubGrubPython::Installed) => { + PubGrubPackageInner::Python(PubGrubPython::Installed) => { let version = self.python_requirement.installed(); if range.contains(version) { Ok(Some(ResolverVersion::Available(version.deref().clone()))) @@ -723,7 +728,7 @@ impl ResolverState { + PubGrubPackageInner::Python(PubGrubPython::Target) => { let version = self.python_requirement.target(); if range.contains(version) { Ok(Some(ResolverVersion::Available(version.deref().clone()))) @@ -732,12 +737,12 @@ impl ResolverState ResolverState { // Wait for the metadata to be available. @@ -916,7 +921,7 @@ impl ResolverState ResolverState, ) -> Result { - match package { - PubGrubPackage::Root(_) => { + match &**package { + PubGrubPackageInner::Root(_) => { // Add the root requirements. let dependencies = PubGrubDependencies::from_requirements( &self.requirements, @@ -1021,9 +1026,9 @@ impl ResolverState Ok(Dependencies::Available(Vec::default())), + PubGrubPackageInner::Python(_) => Ok(Dependencies::Available(Vec::default())), - PubGrubPackage::Package { + PubGrubPackageInner::Package { name, extra, marker: _marker, @@ -1192,28 +1197,28 @@ impl ResolverState Ok(Dependencies::Available(vec![ ( - PubGrubPackage::Package { + PubGrubPackage::from(PubGrubPackageInner::Package { name: name.clone(), extra: None, marker: None, url: url.clone(), - }, + }), Range::singleton(version.clone()), ), ( - PubGrubPackage::Package { + PubGrubPackage::from(PubGrubPackageInner::Package { name: name.clone(), extra: Some(extra.clone()), marker: None, url: url.clone(), - }, + }), Range::singleton(version.clone()), ), ])), @@ -1438,18 +1443,18 @@ impl ResolverState {} - PubGrubPackage::Python(_) => {} - PubGrubPackage::Extra { .. } => {} - PubGrubPackage::Package { + match &**package { + PubGrubPackageInner::Root(_) => {} + PubGrubPackageInner::Python(_) => {} + PubGrubPackageInner::Extra { .. } => {} + PubGrubPackageInner::Package { name, url: Some(url), .. } => { reporter.on_progress(name, &VersionOrUrlRef::Url(&url.verbatim)); } - PubGrubPackage::Package { + PubGrubPackageInner::Package { name, url: None, .. } => { reporter.on_progress(name, &VersionOrUrlRef::Version(version));