From 7e7e9f8a0ccbed59262e7372815dd8e147e34532 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 29 Oct 2023 11:31:55 -0700 Subject: [PATCH] Add support for pre-release versions (#216) We now accept a pre-release if (1) all versions are pre-releases, or (2) there was a pre-release marker in the dependency specifiers for a direct dependency. The code is written such that we can support a variety of pre-release strategies. Closes https://github.com/astral-sh/puffin/issues/191. --- crates/pep440-rs/src/version_specifier.rs | 5 + crates/puffin-cli/src/commands/pip_compile.rs | 10 +- crates/puffin-dispatch/src/lib.rs | 5 +- .../puffin-resolver/src/candidate_selector.rs | 260 ++++++++++++++++++ crates/puffin-resolver/src/lib.rs | 11 +- crates/puffin-resolver/src/manifest.rs | 32 +++ crates/puffin-resolver/src/prerelease_mode.rs | 98 +++++++ crates/puffin-resolver/src/pubgrub/version.rs | 4 + crates/puffin-resolver/src/resolution_mode.rs | 45 +++ crates/puffin-resolver/src/resolver.rs | 36 +-- crates/puffin-resolver/src/selector.rs | 212 -------------- crates/puffin-resolver/tests/resolver.rs | 191 ++++++++++--- ...__black_allow_prerelease_if_necessary.snap | 20 ++ .../resolver__black_disallow_prerelease.snap | 5 + .../snapshots/resolver__black_lowest.snap | 12 +- .../resolver__black_lowest_direct.snap | 8 +- ...llow_explicit_prerelease_with_marker.snap} | 0 ...ow_explicit_prerelease_without_marker.snap | 12 + .../resolver__pylint_allow_prerelease.snap | 12 + .../resolver__pylint_disallow_prerelease.snap | 12 + 20 files changed, 695 insertions(+), 295 deletions(-) create mode 100644 crates/puffin-resolver/src/candidate_selector.rs create mode 100644 crates/puffin-resolver/src/manifest.rs create mode 100644 crates/puffin-resolver/src/prerelease_mode.rs create mode 100644 crates/puffin-resolver/src/resolution_mode.rs delete mode 100644 crates/puffin-resolver/src/selector.rs create mode 100644 crates/puffin-resolver/tests/snapshots/resolver__black_allow_prerelease_if_necessary.snap create mode 100644 crates/puffin-resolver/tests/snapshots/resolver__black_disallow_prerelease.snap rename crates/puffin-resolver/tests/snapshots/{resolver__pylint.snap => resolver__pylint_allow_explicit_prerelease_with_marker.snap} (100%) create mode 100644 crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_explicit_prerelease_without_marker.snap create mode 100644 crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_prerelease.snap create mode 100644 crates/puffin-resolver/tests/snapshots/resolver__pylint_disallow_prerelease.snap diff --git a/crates/pep440-rs/src/version_specifier.rs b/crates/pep440-rs/src/version_specifier.rs index 13fdf0395..e4f96b298 100644 --- a/crates/pep440-rs/src/version_specifier.rs +++ b/crates/pep440-rs/src/version_specifier.rs @@ -364,6 +364,11 @@ impl VersionSpecifier { pub fn version(&self) -> &Version { &self.version } + + /// Whether the version marker includes a prerelease. + pub fn any_prerelease(&self) -> bool { + self.version.any_prerelease() + } } impl VersionSpecifier { diff --git a/crates/puffin-cli/src/commands/pip_compile.rs b/crates/puffin-cli/src/commands/pip_compile.rs index ed4d3c96c..0ff00e4b0 100644 --- a/crates/puffin-cli/src/commands/pip_compile.rs +++ b/crates/puffin-cli/src/commands/pip_compile.rs @@ -16,7 +16,7 @@ use platform_tags::Tags; use puffin_client::RegistryClientBuilder; use puffin_dispatch::BuildDispatch; use puffin_interpreter::Virtualenv; -use puffin_resolver::{ResolutionManifest, ResolutionMode}; +use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode}; use crate::commands::{elapsed, ExitStatus}; use crate::index_urls::IndexUrls; @@ -56,7 +56,13 @@ pub(crate) async fn pip_compile( .unwrap_or_default(); // Create a manifest of the requirements. - let manifest = ResolutionManifest::new(requirements, constraints, preferences, resolution_mode); + let manifest = Manifest::new( + requirements, + constraints, + preferences, + resolution_mode, + PreReleaseMode::default(), + ); // Detect the current Python interpreter. let platform = Platform::current()?; diff --git a/crates/puffin-dispatch/src/lib.rs b/crates/puffin-dispatch/src/lib.rs index afcd7410b..5aa90af52 100644 --- a/crates/puffin-dispatch/src/lib.rs +++ b/crates/puffin-dispatch/src/lib.rs @@ -19,7 +19,7 @@ use puffin_installer::{ Downloader, Installer, PartitionedRequirements, RemoteDistribution, Unzipper, }; use puffin_interpreter::{InterpreterInfo, Virtualenv}; -use puffin_resolver::{ResolutionManifest, ResolutionMode, Resolver, WheelFinder}; +use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode, Resolver, WheelFinder}; use puffin_traits::BuildContext; /// The main implementation of [`BuildContext`], used by the CLI, see [`BuildContext`] @@ -71,11 +71,12 @@ impl BuildContext for BuildDispatch { self.interpreter_info.simple_version(), )?; let resolver = Resolver::new( - ResolutionManifest::new( + Manifest::new( requirements.to_vec(), Vec::default(), Vec::default(), ResolutionMode::default(), + PreReleaseMode::default(), ), self.interpreter_info.markers(), &tags, diff --git a/crates/puffin-resolver/src/candidate_selector.rs b/crates/puffin-resolver/src/candidate_selector.rs new file mode 100644 index 000000000..8f29f5731 --- /dev/null +++ b/crates/puffin-resolver/src/candidate_selector.rs @@ -0,0 +1,260 @@ +use fxhash::FxHashMap; +use pubgrub::range::Range; + +use pep508_rs::{Requirement, VersionOrUrl}; +use puffin_package::package_name::PackageName; + +use crate::distribution::DistributionFile; +use crate::prerelease_mode::PreReleaseStrategy; +use crate::pubgrub::version::PubGrubVersion; +use crate::resolution_mode::ResolutionStrategy; +use crate::resolver::VersionMap; +use crate::Manifest; + +#[derive(Debug)] +pub(crate) struct CandidateSelector { + resolution_strategy: ResolutionStrategy, + prerelease_strategy: PreReleaseStrategy, + preferences: Preferences, +} + +impl From<&Manifest> for CandidateSelector { + /// Return a [`CandidateSelector`] for the given [`Manifest`]. + fn from(manifest: &Manifest) -> Self { + Self { + resolution_strategy: ResolutionStrategy::from_mode( + manifest.resolution_mode, + manifest.requirements.as_slice(), + ), + prerelease_strategy: PreReleaseStrategy::from_mode( + manifest.prerelease_mode, + manifest.requirements.as_slice(), + ), + preferences: Preferences::from(manifest.preferences.as_slice()), + } + } +} + +/// A set of pinned packages that should be preserved during resolution, if possible. +#[derive(Debug)] +struct Preferences(FxHashMap); + +impl Preferences { + fn get(&self, package_name: &PackageName) -> Option<&PubGrubVersion> { + self.0.get(package_name) + } +} + +impl From<&[Requirement]> for Preferences { + fn from(requirements: &[Requirement]) -> Self { + Self( + requirements + .iter() + .filter_map(|requirement| { + let Some(VersionOrUrl::VersionSpecifier(version_specifiers)) = + requirement.version_or_url.as_ref() + else { + return None; + }; + let [version_specifier] = &**version_specifiers else { + return None; + }; + let package_name = PackageName::normalize(&requirement.name); + let version = PubGrubVersion::from(version_specifier.version().clone()); + Some((package_name, version)) + }) + .collect(), + ) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +enum AllowPreRelease { + Yes, + No, + IfNecessary, +} + +impl CandidateSelector { + /// Select a [`Candidate`] from a set of candidate versions and files. + pub(crate) fn select( + &self, + package_name: &PackageName, + range: &Range, + version_map: &VersionMap, + ) -> Option { + // If the package has a preference (e.g., an existing version from an existing lockfile), + // and the preference satisfies the current range, use that. + if let Some(version) = self.preferences.get(package_name) { + if range.contains(version) { + if let Some(file) = version_map.get(version) { + return Some(Candidate { + package_name: package_name.clone(), + version: version.clone(), + file: file.clone(), + }); + } + } + } + + // Determine the appropriate prerelease strategy for the current package. + let allow_prerelease = match &self.prerelease_strategy { + PreReleaseStrategy::DisallowAll => AllowPreRelease::No, + PreReleaseStrategy::AllowAll => AllowPreRelease::Yes, + PreReleaseStrategy::IfNecessary => AllowPreRelease::IfNecessary, + PreReleaseStrategy::Explicit(packages) => { + if packages.contains(package_name) { + AllowPreRelease::Yes + } else { + AllowPreRelease::No + } + } + PreReleaseStrategy::IfNecessaryOrExplicit(packages) => { + if packages.contains(package_name) { + AllowPreRelease::Yes + } else { + AllowPreRelease::IfNecessary + } + } + }; + + match &self.resolution_strategy { + ResolutionStrategy::Highest => Self::select_candidate( + version_map.iter().rev(), + package_name, + range, + allow_prerelease, + ), + ResolutionStrategy::Lowest => { + Self::select_candidate(version_map.iter(), package_name, range, allow_prerelease) + } + ResolutionStrategy::LowestDirect(direct_dependencies) => { + if direct_dependencies.contains(package_name) { + Self::select_candidate( + version_map.iter(), + package_name, + range, + allow_prerelease, + ) + } else { + Self::select_candidate( + version_map.iter().rev(), + package_name, + range, + allow_prerelease, + ) + } + } + } + } + + /// Select the first-matching [`Candidate`] from a set of candidate versions and files, + /// preferring wheels over sdists. + fn select_candidate<'a>( + versions: impl Iterator, + package_name: &PackageName, + range: &Range, + allow_prerelease: AllowPreRelease, + ) -> Option { + // We prefer a stable wheel, followed by a prerelease wheel, followed by a stable sdist, + // followed by a prerelease sdist. + let mut sdist = None; + let mut prerelease_sdist = None; + let mut prerelease_wheel = None; + + for (version, file) in versions { + if range.contains(version) { + match file { + DistributionFile::Wheel(_) => { + if version.any_prerelease() { + match allow_prerelease { + AllowPreRelease::Yes => { + // If prereleases are allowed, treat them equivalently + // to stable wheels. + return Some(Candidate { + package_name: package_name.clone(), + version: version.clone(), + file: file.clone(), + }); + } + AllowPreRelease::IfNecessary => { + // If prereleases are allowed as a fallback, store the + // first-matching prerelease wheel. + if prerelease_wheel.is_none() { + prerelease_wheel = Some(Candidate { + package_name: package_name.clone(), + version: version.clone(), + file: file.clone(), + }); + } + } + AllowPreRelease::No => { + continue; + } + } + } else { + // Always return the first-matching stable wheel. + return Some(Candidate { + package_name: package_name.clone(), + version: version.clone(), + file: file.clone(), + }); + } + } + DistributionFile::Sdist(_) => { + if version.any_prerelease() { + match allow_prerelease { + AllowPreRelease::Yes => { + // If prereleases are allowed, treat them equivalently to + // stable sdists. + if sdist.is_none() { + sdist = Some(Candidate { + package_name: package_name.clone(), + version: version.clone(), + file: file.clone(), + }); + } + } + AllowPreRelease::IfNecessary => { + // If prereleases are allowed as a fallback, store the + // first-matching prerelease sdist. + if prerelease_sdist.is_none() { + prerelease_sdist = Some(Candidate { + package_name: package_name.clone(), + version: version.clone(), + file: file.clone(), + }); + } + } + AllowPreRelease::No => { + continue; + } + } + } else { + // Store the first-matching stable sdist. + if sdist.is_none() { + sdist = Some(Candidate { + package_name: package_name.clone(), + version: version.clone(), + file: file.clone(), + }); + } + } + } + } + } + } + + sdist.or(prerelease_wheel).or(prerelease_sdist) + } +} + +#[derive(Debug, Clone)] +pub(crate) struct Candidate { + /// The name of the package. + pub(crate) package_name: PackageName, + /// The version of the package. + pub(crate) version: PubGrubVersion, + /// The file of the package. + pub(crate) file: DistributionFile, +} diff --git a/crates/puffin-resolver/src/lib.rs b/crates/puffin-resolver/src/lib.rs index b105d0236..5ec0e935d 100644 --- a/crates/puffin-resolver/src/lib.rs +++ b/crates/puffin-resolver/src/lib.rs @@ -1,15 +1,20 @@ pub use error::ResolveError; +pub use manifest::Manifest; +pub use prerelease_mode::PreReleaseMode; pub use resolution::{Graph, PinnedPackage}; -pub use resolver::{ResolutionManifest, Resolver}; -pub use selector::ResolutionMode; +pub use resolution_mode::ResolutionMode; +pub use resolver::Resolver; pub use source_distribution::BuiltSourceDistributionCache; pub use wheel_finder::{Reporter, WheelFinder}; +mod candidate_selector; mod distribution; mod error; +mod manifest; +mod prerelease_mode; mod pubgrub; mod resolution; +mod resolution_mode; mod resolver; -mod selector; mod source_distribution; mod wheel_finder; diff --git a/crates/puffin-resolver/src/manifest.rs b/crates/puffin-resolver/src/manifest.rs new file mode 100644 index 000000000..8b710d180 --- /dev/null +++ b/crates/puffin-resolver/src/manifest.rs @@ -0,0 +1,32 @@ +use pep508_rs::Requirement; + +use crate::prerelease_mode::PreReleaseMode; +use crate::resolution_mode::ResolutionMode; + +/// A manifest of requirements, constraints, and preferences. +#[derive(Debug)] +pub struct Manifest { + pub(crate) requirements: Vec, + pub(crate) constraints: Vec, + pub(crate) preferences: Vec, + pub(crate) resolution_mode: ResolutionMode, + pub(crate) prerelease_mode: PreReleaseMode, +} + +impl Manifest { + pub fn new( + requirements: Vec, + constraints: Vec, + preferences: Vec, + resolution_mode: ResolutionMode, + prerelease_mode: PreReleaseMode, + ) -> Self { + Self { + requirements, + constraints, + preferences, + resolution_mode, + prerelease_mode, + } + } +} diff --git a/crates/puffin-resolver/src/prerelease_mode.rs b/crates/puffin-resolver/src/prerelease_mode.rs new file mode 100644 index 000000000..9cb84ae02 --- /dev/null +++ b/crates/puffin-resolver/src/prerelease_mode.rs @@ -0,0 +1,98 @@ +use fxhash::FxHashSet; + +use pep508_rs::{Requirement, VersionOrUrl}; +use puffin_package::package_name::PackageName; + +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "clap", derive(clap::ValueEnum))] +pub enum PreReleaseMode { + /// Disallow all pre-release versions. + DisallowAll, + + /// Allow all pre-release versions. + AllowAll, + + /// Allow pre-release versions if all versions of a package are pre-release. + IfNecessary, + + /// Allow pre-release versions for first-party packages with explicit pre-release markers in + /// their version requirements. + Explicit, + + /// Allow pre-release versions if all versions of a package are pre-release, or if the package + /// has an explicit pre-release marker in its version requirements. + #[default] + IfNecessaryOrExplicit, +} + +/// Like [`PreReleaseMode`], but with any additional information required to select a candidate, +/// like the set of direct dependencies. +#[derive(Debug)] +pub(crate) enum PreReleaseStrategy { + /// Disallow all pre-release versions. + DisallowAll, + + /// Allow all pre-release versions. + AllowAll, + + /// Allow pre-release versions if all versions of a package are pre-release. + IfNecessary, + + /// Allow pre-release versions for first-party packages with explicit pre-release markers in + /// their version requirements. + Explicit(FxHashSet), + + /// Allow pre-release versions if all versions of a package are pre-release, or if the package + /// has an explicit pre-release marker in its version requirements. + IfNecessaryOrExplicit(FxHashSet), +} + +impl PreReleaseStrategy { + pub(crate) fn from_mode(mode: PreReleaseMode, direct_dependencies: &[Requirement]) -> Self { + match mode { + PreReleaseMode::DisallowAll => Self::DisallowAll, + PreReleaseMode::AllowAll => Self::AllowAll, + PreReleaseMode::IfNecessary => Self::IfNecessary, + PreReleaseMode::Explicit => Self::Explicit( + direct_dependencies + .iter() + .filter(|requirement| { + let Some(version_or_url) = &requirement.version_or_url else { + return false; + }; + let version_specifiers = match version_or_url { + VersionOrUrl::VersionSpecifier(version_specifiers) => { + version_specifiers + } + VersionOrUrl::Url(_) => return false, + }; + version_specifiers + .iter() + .any(pep440_rs::VersionSpecifier::any_prerelease) + }) + .map(|requirement| PackageName::normalize(&requirement.name)) + .collect(), + ), + PreReleaseMode::IfNecessaryOrExplicit => Self::IfNecessaryOrExplicit( + direct_dependencies + .iter() + .filter(|requirement| { + let Some(version_or_url) = &requirement.version_or_url else { + return false; + }; + let version_specifiers = match version_or_url { + VersionOrUrl::VersionSpecifier(version_specifiers) => { + version_specifiers + } + VersionOrUrl::Url(_) => return false, + }; + version_specifiers + .iter() + .any(pep440_rs::VersionSpecifier::any_prerelease) + }) + .map(|requirement| PackageName::normalize(&requirement.name)) + .collect(), + ), + } + } +} diff --git a/crates/puffin-resolver/src/pubgrub/version.rs b/crates/puffin-resolver/src/pubgrub/version.rs index 5f6ca2435..884d44d1b 100644 --- a/crates/puffin-resolver/src/pubgrub/version.rs +++ b/crates/puffin-resolver/src/pubgrub/version.rs @@ -20,6 +20,10 @@ impl PubGrubVersion { } next } + + pub fn any_prerelease(&self) -> bool { + self.0.any_prerelease() + } } impl std::fmt::Display for PubGrubVersion { diff --git a/crates/puffin-resolver/src/resolution_mode.rs b/crates/puffin-resolver/src/resolution_mode.rs new file mode 100644 index 000000000..973508875 --- /dev/null +++ b/crates/puffin-resolver/src/resolution_mode.rs @@ -0,0 +1,45 @@ +use fxhash::FxHashSet; + +use pep508_rs::Requirement; +use puffin_package::package_name::PackageName; + +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "clap", derive(clap::ValueEnum))] +pub enum ResolutionMode { + /// Resolve the highest compatible version of each package. + #[default] + Highest, + /// Resolve the lowest compatible version of each package. + Lowest, + /// Resolve the lowest compatible version of any direct dependencies, and the highest + /// compatible version of any transitive dependencies. + LowestDirect, +} + +/// Like [`ResolutionMode`], but with any additional information required to select a candidate, +/// like the set of direct dependencies. +#[derive(Debug)] +pub(crate) enum ResolutionStrategy { + /// Resolve the highest compatible version of each package. + Highest, + /// Resolve the lowest compatible version of each package. + Lowest, + /// Resolve the lowest compatible version of any direct dependencies, and the highest + /// compatible version of any transitive dependencies. + LowestDirect(FxHashSet), +} + +impl ResolutionStrategy { + pub(crate) fn from_mode(mode: ResolutionMode, direct_dependencies: &[Requirement]) -> Self { + match mode { + ResolutionMode::Highest => Self::Highest, + ResolutionMode::Lowest => Self::Lowest, + ResolutionMode::LowestDirect => Self::LowestDirect( + direct_dependencies + .iter() + .map(|requirement| PackageName::normalize(&requirement.name)) + .collect(), + ), + } + } +} diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index 94fba34d4..b30afe161 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -29,41 +29,17 @@ use puffin_package::metadata::Metadata21; use puffin_package::package_name::PackageName; use puffin_traits::BuildContext; +use crate::candidate_selector::CandidateSelector; use crate::distribution::{DistributionFile, SdistFile, WheelFile}; use crate::error::ResolveError; +use crate::manifest::Manifest; use crate::pubgrub::package::PubGrubPackage; use crate::pubgrub::version::{PubGrubVersion, MIN_VERSION}; use crate::pubgrub::{iter_requirements, version_range}; use crate::resolution::Graph; -use crate::selector::{CandidateSelector, ResolutionMode}; use crate::source_distribution::{download_and_build_sdist, read_dist_info}; use crate::BuiltSourceDistributionCache; -/// A manifest of requirements, constraints, and preferences. -#[derive(Debug)] -pub struct ResolutionManifest { - requirements: Vec, - constraints: Vec, - preferences: Vec, - mode: ResolutionMode, -} - -impl ResolutionManifest { - pub fn new( - requirements: Vec, - constraints: Vec, - preferences: Vec, - mode: ResolutionMode, - ) -> Self { - Self { - requirements, - constraints, - preferences, - mode, - } - } -} - pub struct Resolver<'a, Context: BuildContext + Sync> { requirements: Vec, constraints: Vec, @@ -78,18 +54,14 @@ pub struct Resolver<'a, Context: BuildContext + Sync> { impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { /// Initialize a new resolver. pub fn new( - manifest: ResolutionManifest, + manifest: Manifest, markers: &'a MarkerEnvironment, tags: &'a Tags, client: &'a RegistryClient, build_context: &'a Context, ) -> Self { Self { - selector: CandidateSelector::from_mode( - manifest.mode, - &manifest.requirements, - &manifest.preferences, - ), + selector: CandidateSelector::from(&manifest), index: Arc::new(Index::default()), requirements: manifest.requirements, constraints: manifest.constraints, diff --git a/crates/puffin-resolver/src/selector.rs b/crates/puffin-resolver/src/selector.rs deleted file mode 100644 index 30a3caff0..000000000 --- a/crates/puffin-resolver/src/selector.rs +++ /dev/null @@ -1,212 +0,0 @@ -use fxhash::{FxHashMap, FxHashSet}; -use pubgrub::range::Range; - -use pep508_rs::{Requirement, VersionOrUrl}; -use puffin_package::package_name::PackageName; - -use crate::distribution::DistributionFile; -use crate::pubgrub::version::PubGrubVersion; -use crate::resolver::VersionMap; - -#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] -#[cfg_attr(feature = "clap", derive(clap::ValueEnum))] -pub enum ResolutionMode { - /// Resolve the highest compatible version of each package. - #[default] - Highest, - /// Resolve the lowest compatible version of each package. - Lowest, - /// Resolve the lowest compatible version of any direct dependencies, and the highest - /// compatible version of any transitive dependencies. - LowestDirect, -} - -/// Like [`ResolutionMode`], but with any additional information required to select a candidate, -/// like the set of direct dependencies. -#[derive(Debug)] -enum ResolutionStrategy { - /// Resolve the highest compatible version of each package. - Highest, - /// Resolve the lowest compatible version of each package. - Lowest, - /// Resolve the lowest compatible version of any direct dependencies, and the highest - /// compatible version of any transitive dependencies. - LowestDirect(FxHashSet), -} - -impl ResolutionStrategy { - fn from_mode(mode: ResolutionMode, direct_dependencies: &[Requirement]) -> Self { - match mode { - ResolutionMode::Highest => Self::Highest, - ResolutionMode::Lowest => Self::Lowest, - ResolutionMode::LowestDirect => Self::LowestDirect( - direct_dependencies - .iter() - .map(|requirement| PackageName::normalize(&requirement.name)) - .collect(), - ), - } - } -} - -/// A set of pinned packages that should be preserved during resolution, if possible. -#[derive(Debug)] -struct Preferences(FxHashMap); - -impl Preferences { - fn get(&self, package_name: &PackageName) -> Option<&PubGrubVersion> { - self.0.get(package_name) - } -} - -impl From<&[Requirement]> for Preferences { - fn from(requirements: &[Requirement]) -> Self { - Self( - requirements - .iter() - .filter_map(|requirement| { - let Some(VersionOrUrl::VersionSpecifier(version_specifiers)) = - requirement.version_or_url.as_ref() - else { - return None; - }; - let [version_specifier] = &**version_specifiers else { - return None; - }; - let package_name = PackageName::normalize(&requirement.name); - let version = PubGrubVersion::from(version_specifier.version().clone()); - Some((package_name, version)) - }) - .collect(), - ) - } -} - -#[derive(Debug)] -pub(crate) struct CandidateSelector { - strategy: ResolutionStrategy, - preferences: Preferences, -} - -impl CandidateSelector { - /// Return a candidate selector for the given resolution mode. - pub(crate) fn from_mode( - mode: ResolutionMode, - direct_dependencies: &[Requirement], - resolution: &[Requirement], - ) -> Self { - Self { - strategy: ResolutionStrategy::from_mode(mode, direct_dependencies), - preferences: Preferences::from(resolution), - } - } -} - -impl CandidateSelector { - /// Select a [`Candidate`] from a set of candidate versions and files. - pub(crate) fn select( - &self, - package_name: &PackageName, - range: &Range, - version_map: &VersionMap, - ) -> Option { - if let Some(version) = self.preferences.get(package_name) { - if range.contains(version) { - if let Some(file) = version_map.get(version) { - return Some(Candidate { - package_name: package_name.clone(), - version: version.clone(), - file: file.clone(), - }); - } - } - } - - match &self.strategy { - ResolutionStrategy::Highest => Self::select_highest(package_name, range, version_map), - ResolutionStrategy::Lowest => Self::select_lowest(package_name, range, version_map), - ResolutionStrategy::LowestDirect(direct_dependencies) => { - if direct_dependencies.contains(package_name) { - Self::select_lowest(package_name, range, version_map) - } else { - Self::select_highest(package_name, range, version_map) - } - } - } - } - - /// Select the highest-compatible [`Candidate`] from a set of candidate versions and files, - /// preferring wheels over sdists. - fn select_highest( - package_name: &PackageName, - range: &Range, - version_map: &VersionMap, - ) -> Option { - let mut sdist = None; - for (version, file) in version_map.iter().rev() { - if range.contains(version) { - match file { - DistributionFile::Wheel(_) => { - return Some(Candidate { - package_name: package_name.clone(), - version: version.clone(), - file: file.clone(), - }); - } - DistributionFile::Sdist(_) if sdist.is_none() => { - sdist = Some(Candidate { - package_name: package_name.clone(), - version: version.clone(), - file: file.clone(), - }); - } - DistributionFile::Sdist(_) => { - // We already selected a more recent source distribution - } - } - } - } - sdist - } - - /// Select the highest-compatible [`Candidate`] from a set of candidate versions and files, - /// preferring wheels over sdists. - fn select_lowest( - package_name: &PackageName, - range: &Range, - version_map: &VersionMap, - ) -> Option { - let mut sdist = None; - for (version, file) in version_map { - if range.contains(version) { - match file { - DistributionFile::Wheel(_) => { - return Some(Candidate { - package_name: package_name.clone(), - version: version.clone(), - file: file.clone(), - }); - } - DistributionFile::Sdist(_) => { - sdist = Some(Candidate { - package_name: package_name.clone(), - version: version.clone(), - file: file.clone(), - }); - } - } - } - } - sdist - } -} - -#[derive(Debug, Clone)] -pub(crate) struct Candidate { - /// The name of the package. - pub(crate) package_name: PackageName, - /// The version of the package. - pub(crate) version: PubGrubVersion, - /// The file of the package. - pub(crate) file: DistributionFile, -} diff --git a/crates/puffin-resolver/tests/resolver.rs b/crates/puffin-resolver/tests/resolver.rs index d7738edc5..9e202ca83 100644 --- a/crates/puffin-resolver/tests/resolver.rs +++ b/crates/puffin-resolver/tests/resolver.rs @@ -16,7 +16,7 @@ use platform_host::{Arch, Os, Platform}; use platform_tags::Tags; use puffin_client::RegistryClientBuilder; use puffin_interpreter::{InterpreterInfo, Virtualenv}; -use puffin_resolver::{ResolutionManifest, ResolutionMode, Resolver}; +use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode, Resolver}; use puffin_traits::BuildContext; struct DummyContext; @@ -58,38 +58,18 @@ impl BuildContext for DummyContext { } } -#[tokio::test] -async fn pylint() -> Result<()> { - colored::control::set_override(false); - - let client = RegistryClientBuilder::default().build(); - - let manifest = ResolutionManifest::new( - vec![Requirement::from_str("pylint==2.3.0").unwrap()], - vec![], - vec![], - ResolutionMode::default(), - ); - - let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); - let resolution = resolver.resolve().await?; - - insta::assert_display_snapshot!(resolution); - - Ok(()) -} - #[tokio::test] async fn black() -> Result<()> { colored::control::set_override(false); let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![], vec![], ResolutionMode::default(), + PreReleaseMode::default(), ); let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); @@ -106,11 +86,12 @@ async fn black_colorama() -> Result<()> { let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black[colorama]<=23.9.1").unwrap()], vec![], vec![], ResolutionMode::default(), + PreReleaseMode::default(), ); let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); @@ -127,11 +108,12 @@ async fn black_python_310() -> Result<()> { let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![], vec![], ResolutionMode::default(), + PreReleaseMode::default(), ); let resolver = Resolver::new(manifest, &MARKERS_310, &TAGS_310, &client, &DummyContext); @@ -150,11 +132,12 @@ async fn black_mypy_extensions() -> Result<()> { let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("mypy-extensions<1").unwrap()], vec![], ResolutionMode::default(), + PreReleaseMode::default(), ); let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); @@ -173,11 +156,12 @@ async fn black_mypy_extensions_extra() -> Result<()> { let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("mypy-extensions[extra]<1").unwrap()], vec![], ResolutionMode::default(), + PreReleaseMode::default(), ); let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); @@ -196,11 +180,12 @@ async fn black_flake8() -> Result<()> { let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![Requirement::from_str("flake8<1").unwrap()], vec![], ResolutionMode::default(), + PreReleaseMode::default(), ); let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); @@ -217,11 +202,12 @@ async fn black_lowest() -> Result<()> { let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black>21").unwrap()], vec![], vec![], ResolutionMode::Lowest, + PreReleaseMode::default(), ); let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); @@ -238,11 +224,12 @@ async fn black_lowest_direct() -> Result<()> { let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black>21").unwrap()], vec![], vec![], ResolutionMode::LowestDirect, + PreReleaseMode::default(), ); let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); @@ -259,11 +246,12 @@ async fn black_respect_preference() -> Result<()> { let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![], vec![Requirement::from_str("black==23.9.0").unwrap()], ResolutionMode::default(), + PreReleaseMode::default(), ); let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); @@ -280,11 +268,150 @@ async fn black_ignore_preference() -> Result<()> { let client = RegistryClientBuilder::default().build(); - let manifest = ResolutionManifest::new( + let manifest = Manifest::new( vec![Requirement::from_str("black<=23.9.1").unwrap()], vec![], vec![Requirement::from_str("black==23.9.2").unwrap()], ResolutionMode::default(), + PreReleaseMode::default(), + ); + + let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); + let resolution = resolver.resolve().await?; + + insta::assert_display_snapshot!(resolution); + + Ok(()) +} + +#[tokio::test] +async fn black_disallow_prerelease() -> Result<()> { + colored::control::set_override(false); + + let client = RegistryClientBuilder::default().build(); + + let manifest = Manifest::new( + vec![Requirement::from_str("black<=20.0").unwrap()], + vec![], + vec![], + ResolutionMode::default(), + PreReleaseMode::DisallowAll, + ); + + let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); + let err = resolver.resolve().await.unwrap_err(); + + insta::assert_display_snapshot!(err); + + Ok(()) +} + +#[tokio::test] +async fn black_allow_prerelease_if_necessary() -> Result<()> { + colored::control::set_override(false); + + let client = RegistryClientBuilder::default().build(); + + let manifest = Manifest::new( + vec![Requirement::from_str("black<=20.0").unwrap()], + vec![], + vec![], + ResolutionMode::default(), + PreReleaseMode::IfNecessary, + ); + + let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); + let resolution = resolver.resolve().await?; + + insta::assert_display_snapshot!(resolution); + + Ok(()) +} + +#[tokio::test] +async fn pylint_disallow_prerelease() -> Result<()> { + colored::control::set_override(false); + + let client = RegistryClientBuilder::default().build(); + + let manifest = Manifest::new( + vec![Requirement::from_str("pylint==2.3.0").unwrap()], + vec![], + vec![], + ResolutionMode::default(), + PreReleaseMode::DisallowAll, + ); + + let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); + let resolution = resolver.resolve().await?; + + insta::assert_display_snapshot!(resolution); + + Ok(()) +} + +#[tokio::test] +async fn pylint_allow_prerelease() -> Result<()> { + colored::control::set_override(false); + + let client = RegistryClientBuilder::default().build(); + + let manifest = Manifest::new( + vec![Requirement::from_str("pylint==2.3.0").unwrap()], + vec![], + vec![], + ResolutionMode::default(), + PreReleaseMode::AllowAll, + ); + + let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); + let resolution = resolver.resolve().await?; + + insta::assert_display_snapshot!(resolution); + + Ok(()) +} + +#[tokio::test] +async fn pylint_allow_explicit_prerelease_without_marker() -> Result<()> { + colored::control::set_override(false); + + let client = RegistryClientBuilder::default().build(); + + let manifest = Manifest::new( + vec![ + Requirement::from_str("pylint==2.3.0").unwrap(), + Requirement::from_str("isort>=5.0.0").unwrap(), + ], + vec![], + vec![], + ResolutionMode::default(), + PreReleaseMode::Explicit, + ); + + let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); + let resolution = resolver.resolve().await?; + + insta::assert_display_snapshot!(resolution); + + Ok(()) +} + +#[tokio::test] +async fn pylint_allow_explicit_prerelease_with_marker() -> Result<()> { + colored::control::set_override(false); + + let client = RegistryClientBuilder::default().build(); + + let manifest = Manifest::new( + vec![ + Requirement::from_str("pylint==2.3.0").unwrap(), + Requirement::from_str("isort>=5.0.0b").unwrap(), + ], + vec![], + vec![], + ResolutionMode::default(), + PreReleaseMode::Explicit, ); let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext); diff --git a/crates/puffin-resolver/tests/snapshots/resolver__black_allow_prerelease_if_necessary.snap b/crates/puffin-resolver/tests/snapshots/resolver__black_allow_prerelease_if_necessary.snap new file mode 100644 index 000000000..84472682f --- /dev/null +++ b/crates/puffin-resolver/tests/snapshots/resolver__black_allow_prerelease_if_necessary.snap @@ -0,0 +1,20 @@ +--- +source: crates/puffin-resolver/tests/resolver.rs +expression: resolution +--- +appdirs==1.4.4 + # via black +attrs==23.1.0 + # via black +black==19.10b0 +click==8.1.7 + # via black +pathspec==0.11.2 + # via black +regex==2023.10.3 + # via black +toml==0.10.2 + # via black +typed-ast==1.5.5 + # via black + diff --git a/crates/puffin-resolver/tests/snapshots/resolver__black_disallow_prerelease.snap b/crates/puffin-resolver/tests/snapshots/resolver__black_disallow_prerelease.snap new file mode 100644 index 000000000..211085917 --- /dev/null +++ b/crates/puffin-resolver/tests/snapshots/resolver__black_disallow_prerelease.snap @@ -0,0 +1,5 @@ +--- +source: crates/puffin-resolver/tests/resolver.rs +expression: err +--- +No solution diff --git a/crates/puffin-resolver/tests/snapshots/resolver__black_lowest.snap b/crates/puffin-resolver/tests/snapshots/resolver__black_lowest.snap index a8485dd3b..26a0043e9 100644 --- a/crates/puffin-resolver/tests/snapshots/resolver__black_lowest.snap +++ b/crates/puffin-resolver/tests/snapshots/resolver__black_lowest.snap @@ -2,17 +2,15 @@ source: crates/puffin-resolver/tests/resolver.rs expression: resolution --- -appdirs==1.4.0 - # via black -black==21.4b0 -click==7.1.2 +black==22.1.0 +click==8.0.0 # via black mypy-extensions==0.4.3 # via black -pathspec==0.7.0 +pathspec==0.9.0 # via black -regex==2022.9.11 +platformdirs==2.0.0 # via black -toml==0.10.1 +tomli==1.1.0 # via black diff --git a/crates/puffin-resolver/tests/snapshots/resolver__black_lowest_direct.snap b/crates/puffin-resolver/tests/snapshots/resolver__black_lowest_direct.snap index b9c96047d..d452da36d 100644 --- a/crates/puffin-resolver/tests/snapshots/resolver__black_lowest_direct.snap +++ b/crates/puffin-resolver/tests/snapshots/resolver__black_lowest_direct.snap @@ -2,17 +2,15 @@ source: crates/puffin-resolver/tests/resolver.rs expression: resolution --- -appdirs==1.4.4 - # via black -black==21.4b0 +black==22.1.0 click==8.1.7 # via black mypy-extensions==1.0.0 # via black pathspec==0.11.2 # via black -regex==2023.10.3 +platformdirs==3.11.0 # via black -toml==0.10.2 +tomli==2.0.1 # via black diff --git a/crates/puffin-resolver/tests/snapshots/resolver__pylint.snap b/crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_explicit_prerelease_with_marker.snap similarity index 100% rename from crates/puffin-resolver/tests/snapshots/resolver__pylint.snap rename to crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_explicit_prerelease_with_marker.snap diff --git a/crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_explicit_prerelease_without_marker.snap b/crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_explicit_prerelease_without_marker.snap new file mode 100644 index 000000000..f527152f8 --- /dev/null +++ b/crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_explicit_prerelease_without_marker.snap @@ -0,0 +1,12 @@ +--- +source: crates/puffin-resolver/tests/resolver.rs +expression: resolution +--- +astroid==3.0.1 + # via pylint +isort==5.12.0 + # via pylint +mccabe==0.7.0 + # via pylint +pylint==2.3.0 + diff --git a/crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_prerelease.snap b/crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_prerelease.snap new file mode 100644 index 000000000..0fb2b52cd --- /dev/null +++ b/crates/puffin-resolver/tests/snapshots/resolver__pylint_allow_prerelease.snap @@ -0,0 +1,12 @@ +--- +source: crates/puffin-resolver/tests/resolver.rs +expression: resolution +--- +astroid==3.0.1 + # via pylint +isort==6.0.0b2 + # via pylint +mccabe==0.7.0 + # via pylint +pylint==2.3.0 + diff --git a/crates/puffin-resolver/tests/snapshots/resolver__pylint_disallow_prerelease.snap b/crates/puffin-resolver/tests/snapshots/resolver__pylint_disallow_prerelease.snap new file mode 100644 index 000000000..f527152f8 --- /dev/null +++ b/crates/puffin-resolver/tests/snapshots/resolver__pylint_disallow_prerelease.snap @@ -0,0 +1,12 @@ +--- +source: crates/puffin-resolver/tests/resolver.rs +expression: resolution +--- +astroid==3.0.1 + # via pylint +isort==5.12.0 + # via pylint +mccabe==0.7.0 + # via pylint +pylint==2.3.0 +