From 0cad13a6776ee6ca79347aaed9055869d238b004 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 2 Jan 2024 20:04:57 -0500 Subject: [PATCH] Use a dedicated struct --- crates/puffin-resolver/src/lib.rs | 1 + .../puffin-resolver/src/python_requirement.rs | 40 ++++++++++++ crates/puffin-resolver/src/resolver.rs | 65 +++++++++---------- crates/puffin-resolver/src/version_map.rs | 17 ++--- 4 files changed, 77 insertions(+), 46 deletions(-) create mode 100644 crates/puffin-resolver/src/python_requirement.rs diff --git a/crates/puffin-resolver/src/lib.rs b/crates/puffin-resolver/src/lib.rs index f2de17c7330cb..9e31b91e7c426 100644 --- a/crates/puffin-resolver/src/lib.rs +++ b/crates/puffin-resolver/src/lib.rs @@ -18,6 +18,7 @@ mod overrides; mod pins; mod prerelease_mode; mod pubgrub; +mod python_requirement; mod resolution; mod resolution_mode; mod resolution_options; diff --git a/crates/puffin-resolver/src/python_requirement.rs b/crates/puffin-resolver/src/python_requirement.rs new file mode 100644 index 0000000000000..6487df018abe6 --- /dev/null +++ b/crates/puffin-resolver/src/python_requirement.rs @@ -0,0 +1,40 @@ +use pep440_rs::Version; +use pep508_rs::MarkerEnvironment; +use puffin_interpreter::Interpreter; + +#[derive(Debug, Clone)] +pub struct PythonRequirement<'a> { + /// The installed version of Python. + installed: &'a Version, + /// The target version of Python; that is, the version of Python for which we are resolving + /// dependencies. This is typically the same as the installed version, but may be different + /// when specifying an alternate Python version for the resolution. + target: Option<&'a Version>, +} + +impl<'a> PythonRequirement<'a> { + pub fn new(interpreter: &'a Interpreter, markers: &'a MarkerEnvironment) -> Self { + let installed = interpreter.version(); + let target = &markers.python_version.version; + Self { + installed, + target: if installed == target { + None + } else { + Some(target) + }, + } + } + + /// Return a version in the given range. + pub(crate) fn version(&self) -> &'a Version { + self.installed + } + + /// Returns an iterator over the versions of Python to consider when resolving dependencies. + pub(crate) fn versions(&self) -> impl Iterator { + self.target + .into_iter() + .chain(std::iter::once(self.installed)) + } +} diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index af82c09778410..220ac3affed0c 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -41,6 +41,7 @@ use crate::pubgrub::{ PubGrubDependencies, PubGrubDistribution, PubGrubPackage, PubGrubPriorities, PubGrubSpecifier, PubGrubVersion, MIN_VERSION, }; +use crate::python_requirement::PythonRequirement; use crate::resolution::ResolutionGraph; use crate::version_map::VersionMap; use crate::yanks::AllowedYanks; @@ -76,9 +77,8 @@ pub trait ResolverProvider: Send + Sync { pub struct DefaultResolverProvider<'a, Context: BuildContext + Send + Sync> { client: &'a RegistryClient, fetcher: DistributionDatabase<'a, Context>, - build_context: &'a Context, tags: &'a Tags, - markers: &'a MarkerEnvironment, + python_requirement: PythonRequirement<'a>, exclude_newer: Option>, allowed_yanks: AllowedYanks, } @@ -87,18 +87,16 @@ impl<'a, Context: BuildContext + Send + Sync> DefaultResolverProvider<'a, Contex pub fn new( client: &'a RegistryClient, fetcher: DistributionDatabase<'a, Context>, - build_context: &'a Context, tags: &'a Tags, - markers: &'a MarkerEnvironment, + python_requirement: PythonRequirement<'a>, exclude_newer: Option>, allowed_yanks: AllowedYanks, ) -> Self { Self { client, fetcher, - build_context, tags, - markers, + python_requirement, exclude_newer, allowed_yanks, } @@ -122,8 +120,7 @@ impl<'a, Context: BuildContext + Send + Sync> ResolverProvider metadata, package_name, self.tags, - self.markers, - self.build_context.interpreter(), + &self.python_requirement, &self.allowed_yanks, self.exclude_newer.as_ref(), ), @@ -155,7 +152,7 @@ pub struct Resolver<'a, Provider: ResolverProvider> { overrides: Overrides, allowed_urls: AllowedUrls, markers: &'a MarkerEnvironment, - interpreter: &'a Interpreter, + python_requirement: PythonRequirement<'a>, selector: CandidateSelector, index: Arc, editables: FxHashMap, @@ -177,9 +174,8 @@ impl<'a, Context: BuildContext + Send + Sync> Resolver<'a, DefaultResolverProvid let provider = DefaultResolverProvider::new( client, DistributionDatabase::new(build_context.cache(), tags, client, build_context), - build_context, tags, - markers, + PythonRequirement::new(interpreter, markers), options.exclude_newer, manifest .requirements @@ -187,7 +183,13 @@ impl<'a, Context: BuildContext + Send + Sync> Resolver<'a, DefaultResolverProvid .chain(manifest.constraints.iter()) .collect(), ); - Self::new_custom_io(manifest, options, markers, interpreter, provider) + Self::new_custom_io( + manifest, + options, + markers, + PythonRequirement::new(interpreter, markers), + provider, + ) } } @@ -197,7 +199,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { manifest: Manifest, options: ResolutionOptions, markers: &'a MarkerEnvironment, - interpreter: &'a Interpreter, + python_requirement: PythonRequirement<'a>, provider: Provider, ) -> Self { let selector = CandidateSelector::for_resolution(&manifest, options); @@ -252,7 +254,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { constraints: manifest.constraints, overrides: Overrides::from_requirements(manifest.overrides), markers, - interpreter, + python_requirement, editables, reporter: None, provider, @@ -497,10 +499,10 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { // If the version is incompatible, short-circuit. if let Some(requires_python) = candidate.resolve().requires_python.as_ref() { - let interpreter_version = self.interpreter.version(); - let marker_version = &self.markers.python_version.version; - if !requires_python.contains(interpreter_version) - || !requires_python.contains(marker_version) + if self + .python_requirement + .versions() + .any(|version| !requires_python.contains(version)) { self.index .incompatibilities @@ -531,20 +533,15 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { PubGrubPackage::Root(_) => Ok(Some(MIN_VERSION.clone())), PubGrubPackage::Python => { - // The version used in the current Python interpreter. - let interpreter_version = PubGrubVersion::from(self.interpreter.version().clone()); - if !range.contains(&interpreter_version) { - return Ok(None); - } - - // The version against which we're resolving. - let marker_version = - PubGrubVersion::from(self.markers.python_version.version.clone()); - if !range.contains(&marker_version) { + if self + .python_requirement + .versions() + .any(|version| !range.contains(&PubGrubVersion::from(version.clone()))) + { return Ok(None); } - - Ok(Some(marker_version)) + let version = PubGrubVersion::from(self.python_requirement.version().clone()); + Ok(Some(version)) } PubGrubPackage::Package(package_name, extra, Some(url)) => { @@ -609,10 +606,10 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> { // If the version is incompatible, short-circuit. if let Some(requires_python) = candidate.resolve().requires_python.as_ref() { - let interpreter_version = self.interpreter.version(); - let marker_version = &self.markers.python_version.version; - if !requires_python.contains(interpreter_version) - || !requires_python.contains(marker_version) + if self + .python_requirement + .versions() + .any(|version| !requires_python.contains(version)) { self.index .incompatibilities diff --git a/crates/puffin-resolver/src/version_map.rs b/crates/puffin-resolver/src/version_map.rs index 28cac8547c572..321885774ecb7 100644 --- a/crates/puffin-resolver/src/version_map.rs +++ b/crates/puffin-resolver/src/version_map.rs @@ -5,16 +5,15 @@ use chrono::{DateTime, Utc}; use tracing::{instrument, warn}; use distribution_filename::DistFilename; -use pep508_rs::MarkerEnvironment; use platform_tags::{TagPriority, Tags}; use puffin_client::SimpleMetadata; -use puffin_interpreter::Interpreter; use puffin_normalize::PackageName; use puffin_warnings::warn_user_once; use pypi_types::Yanked; use crate::file::{DistFile, SdistFile, WheelFile}; use crate::pubgrub::PubGrubVersion; +use crate::python_requirement::PythonRequirement; use crate::yanks::AllowedYanks; /// A map from versions to distributions. @@ -28,8 +27,7 @@ impl VersionMap { metadata: SimpleMetadata, package_name: &PackageName, tags: &Tags, - markers: &MarkerEnvironment, - interpreter: &Interpreter, + python_requirement: &PythonRequirement, allowed_yanks: &AllowedYanks, exclude_newer: Option<&DateTime>, ) -> Self { @@ -75,14 +73,9 @@ impl VersionMap { file.requires_python .as_ref() .map_or(true, |requires_python| { - // The interpreter and marker version are often the same, but can differ. - // For example, if the user is resolving against a target Python version - // passed in via the command line, that version will differ from the - // interpreter version. - let interpreter_version = interpreter.version(); - let marker_version = &markers.python_version.version; - requires_python.contains(interpreter_version) - && requires_python.contains(marker_version) + python_requirement + .versions() + .all(|version| requires_python.contains(version)) }) }); match version_map.entry(version.clone().into()) {