From d57af514d94d8ea0b67b0410e4d4c02db4d42d59 Mon Sep 17 00:00:00 2001 From: konsti Date: Tue, 30 Apr 2024 18:45:05 +0200 Subject: [PATCH] Centralize installed dist satisfies requirement check (#3324) Another split out from https://github.com/astral-sh/uv/pull/3263. This abstracts the copy&pasted check whether an installed distribution satisfies a requirement used by both plan.rs and site_packages.rs into a shared module. It's less useful here than with the new requirement but helps with reducing https://github.com/astral-sh/uv/pull/3263 diff size. --- crates/pep508-rs/src/lib.rs | 13 ++- crates/uv-installer/src/lib.rs | 1 + crates/uv-installer/src/plan.rs | 76 +++++----------- crates/uv-installer/src/satisfies.rs | 70 +++++++++++++++ crates/uv-installer/src/site_packages.rs | 107 +++++------------------ 5 files changed, 125 insertions(+), 142 deletions(-) create mode 100644 crates/uv-installer/src/satisfies.rs diff --git a/crates/pep508-rs/src/lib.rs b/crates/pep508-rs/src/lib.rs index c6bdd26f4..5603ebd2a 100644 --- a/crates/pep508-rs/src/lib.rs +++ b/crates/pep508-rs/src/lib.rs @@ -463,7 +463,7 @@ pub enum VersionOrUrl { } /// Unowned version specifier or URL to install. -#[derive(Debug, Clone, Eq, Hash, PartialEq)] +#[derive(Debug, Clone, Copy, Eq, Hash, PartialEq)] pub enum VersionOrUrlRef<'a> { /// A PEP 440 version specifier set VersionSpecifier(&'a VersionSpecifiers), @@ -471,6 +471,17 @@ pub enum VersionOrUrlRef<'a> { Url(&'a VerbatimUrl), } +impl<'a> From<&'a VersionOrUrl> for VersionOrUrlRef<'a> { + fn from(value: &'a VersionOrUrl) -> Self { + match value { + VersionOrUrl::VersionSpecifier(version_specifier) => { + VersionOrUrlRef::VersionSpecifier(version_specifier) + } + VersionOrUrl::Url(url) => VersionOrUrlRef::Url(url), + } + } +} + /// A [`Cursor`] over a string. #[derive(Debug, Clone)] pub struct Cursor<'a> { diff --git a/crates/uv-installer/src/lib.rs b/crates/uv-installer/src/lib.rs index 159861706..4bf8dec5e 100644 --- a/crates/uv-installer/src/lib.rs +++ b/crates/uv-installer/src/lib.rs @@ -11,5 +11,6 @@ mod downloader; mod editable; mod installer; mod plan; +mod satisfies; mod site_packages; mod uninstall; diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs index f7155146c..89941626c 100644 --- a/crates/uv-installer/src/plan.rs +++ b/crates/uv-installer/src/plan.rs @@ -10,9 +10,9 @@ use distribution_types::{ BuiltDist, CachedDirectUrlDist, CachedDist, Dist, IndexLocations, InstalledDist, InstalledMetadata, InstalledVersion, Name, SourceDist, }; -use pep508_rs::{Requirement, VersionOrUrl}; +use pep508_rs::{Requirement, VersionOrUrl, VersionOrUrlRef}; use platform_tags::Tags; -use uv_cache::{ArchiveTarget, ArchiveTimestamp, Cache, CacheBucket, WheelCache}; +use uv_cache::{ArchiveTimestamp, Cache, CacheBucket, WheelCache}; use uv_configuration::{NoBinary, Reinstall}; use uv_distribution::{ BuiltWheelIndex, HttpArchivePointer, LocalArchivePointer, RegistryWheelIndex, @@ -21,6 +21,7 @@ use uv_fs::Simplified; use uv_interpreter::PythonEnvironment; use uv_types::HashStrategy; +use crate::satisfies::RequirementSatisfaction; use crate::{ResolvedEditable, SitePackages}; /// A planner to generate an [`Plan`] based on a set of requirements. @@ -182,10 +183,23 @@ impl<'a> Planner<'a> { match installed_dists.as_slice() { [] => {} [distribution] => { - if installed_satisfies_requirement(distribution, requirement)? { - debug!("Requirement already installed: {distribution}"); - installed.push(distribution.clone()); - continue; + match RequirementSatisfaction::check( + distribution, + requirement + .version_or_url + .as_ref() + .map(VersionOrUrlRef::from), + requirement, + )? { + RequirementSatisfaction::Mismatch => {} + RequirementSatisfaction::Satisfied => { + debug!("Requirement already installed: {distribution}"); + installed.push(distribution.clone()); + continue; + } + RequirementSatisfaction::OutOfDate => { + debug!("Requirement installed, but not fresh: {distribution}"); + } } reinstalls.push(distribution.clone()); } @@ -416,53 +430,3 @@ pub struct Plan { /// _not_ necessary to satisfy the requirements. pub extraneous: Vec, } - -/// Returns true if a requirement is satisfied by an installed distribution. -/// -/// Returns an error if IO fails during a freshness check for a local path. -fn installed_satisfies_requirement( - distribution: &InstalledDist, - requirement: &Requirement, -) -> Result { - // Filter out already-installed packages. - match requirement.version_or_url.as_ref() { - // Accept any version of the package. - None => return Ok(true), - - // If the requirement comes from a registry, check by name. - Some(VersionOrUrl::VersionSpecifier(version_specifier)) => { - if version_specifier.contains(distribution.version()) { - debug!("Requirement already satisfied: {distribution}"); - return Ok(true); - } - } - - // If the requirement comes from a direct URL, check by URL. - Some(VersionOrUrl::Url(url)) => { - if let InstalledDist::Url(installed) = &distribution { - if !installed.editable && &installed.url == url.raw() { - // If the requirement came from a local path, check freshness. - if let Some(archive) = (url.scheme() == "file") - .then(|| url.to_file_path().ok()) - .flatten() - { - if ArchiveTimestamp::up_to_date_with( - &archive, - ArchiveTarget::Install(distribution), - )? { - debug!("Requirement already satisfied (and up-to-date): {installed}"); - return Ok(true); - } - debug!("Requirement already satisfied (but not up-to-date): {installed}"); - } else { - // Otherwise, assume the requirement is up-to-date. - debug!("Requirement already satisfied (assumed up-to-date): {installed}"); - return Ok(true); - } - } - } - } - } - - Ok(false) -} diff --git a/crates/uv-installer/src/satisfies.rs b/crates/uv-installer/src/satisfies.rs new file mode 100644 index 000000000..6c4b6dc0b --- /dev/null +++ b/crates/uv-installer/src/satisfies.rs @@ -0,0 +1,70 @@ +use anyhow::Result; +use std::fmt::Debug; +use tracing::trace; + +use distribution_types::InstalledDist; +use pep508_rs::VersionOrUrlRef; + +use uv_cache::{ArchiveTarget, ArchiveTimestamp}; + +#[derive(Debug, Copy, Clone)] +pub(crate) enum RequirementSatisfaction { + Mismatch, + Satisfied, + OutOfDate, +} + +impl RequirementSatisfaction { + /// Returns true if a requirement is satisfied by an installed distribution. + /// + /// Returns an error if IO fails during a freshness check for a local path. + pub(crate) fn check( + distribution: &InstalledDist, + version_or_url: Option, + requirement: impl Debug, + ) -> Result { + trace!( + "Comparing installed with requirement: {:?} {:?}", + distribution, + requirement + ); + // Filter out already-installed packages. + match version_or_url { + // Accept any version of the package. + None => return Ok(Self::Satisfied), + + // If the requirement comes from a registry, check by name. + Some(VersionOrUrlRef::VersionSpecifier(version_specifier)) => { + if version_specifier.contains(distribution.version()) { + return Ok(Self::Satisfied); + } + } + + // If the requirement comes from a direct URL, check by URL. + Some(VersionOrUrlRef::Url(url)) => { + if let InstalledDist::Url(installed) = &distribution { + if !installed.editable && &installed.url == url.raw() { + // If the requirement came from a local path, check freshness. + return if let Some(archive) = (url.scheme() == "file") + .then(|| url.to_file_path().ok()) + .flatten() + { + if ArchiveTimestamp::up_to_date_with( + &archive, + ArchiveTarget::Install(distribution), + )? { + return Ok(Self::Satisfied); + } + Ok(Self::OutOfDate) + } else { + // Otherwise, assume the requirement is up-to-date. + Ok(Self::Satisfied) + }; + } + } + } + } + + Ok(Self::Mismatch) + } +} diff --git a/crates/uv-installer/src/site_packages.rs b/crates/uv-installer/src/site_packages.rs index 8c7a190fb..78047881b 100644 --- a/crates/uv-installer/src/site_packages.rs +++ b/crates/uv-installer/src/site_packages.rs @@ -9,7 +9,7 @@ use url::Url; use distribution_types::{InstalledDist, InstalledMetadata, InstalledVersion, Name}; use pep440_rs::{Version, VersionSpecifiers}; -use pep508_rs::{Requirement, VerbatimUrl}; +use pep508_rs::{Requirement, VerbatimUrl, VersionOrUrlRef}; use requirements_txt::{EditableRequirement, RequirementEntry, RequirementsTxtRequirement}; use uv_cache::{ArchiveTarget, ArchiveTimestamp}; use uv_interpreter::PythonEnvironment; @@ -17,6 +17,7 @@ use uv_normalize::PackageName; use uv_types::InstalledPackagesProvider; use crate::is_dynamic; +use crate::satisfies::RequirementSatisfaction; /// An index over the packages installed in an environment. /// @@ -382,95 +383,31 @@ impl<'a> SitePackages<'a> { return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); } [distribution] => { - // Validate that the installed version matches the requirement. - match entry.requirement.version_or_url() { - // Accept any installed version. - None => {} - - // If the requirement comes from a URL, verify by URL. - Some(pep508_rs::VersionOrUrlRef::Url(url)) => { - let InstalledDist::Url(installed) = &distribution else { - return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); - }; - - if installed.editable { - return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); - } - - if &installed.url != url.raw() { - return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); - } - - // If the requirement came from a local path, check freshness. - if url.scheme() == "file" { - if let Ok(archive) = url.to_file_path() { - if !ArchiveTimestamp::up_to_date_with( - &archive, - ArchiveTarget::Install(distribution), - )? { - return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); - } - } - } - } - - Some(pep508_rs::VersionOrUrlRef::VersionSpecifier(version_specifier)) => { - // The installed version doesn't satisfy the requirement. - if !version_specifier.contains(distribution.version()) { - return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); - } + match RequirementSatisfaction::check( + distribution, + entry.requirement.version_or_url(), + &entry.requirement, + )? { + RequirementSatisfaction::Mismatch | RequirementSatisfaction::OutOfDate => { + return Ok(SatisfiesResult::Unsatisfied(entry.to_string())) } + RequirementSatisfaction::Satisfied => {} } - // Validate that the installed version satisfies the constraints. for constraint in constraints { - if constraint.name != *distribution.name() { - continue; - } - - if !constraint.evaluate_markers(self.venv.interpreter().markers(), &[]) { - continue; - } - - match &constraint.version_or_url { - // Accept any installed version. - None => {} - - // If the requirement comes from a URL, verify by URL. - Some(pep508_rs::VersionOrUrl::Url(url)) => { - let InstalledDist::Url(installed) = &distribution else { - return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); - }; - - if installed.editable { - return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); - } - - if &installed.url != url.raw() { - return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); - } - - // If the requirement came from a local path, check freshness. - if url.scheme() == "file" { - if let Ok(archive) = url.to_file_path() { - if !ArchiveTimestamp::up_to_date_with( - &archive, - ArchiveTarget::Install(distribution), - )? { - return Ok(SatisfiesResult::Unsatisfied( - entry.to_string(), - )); - } - } - } - } - - Some(pep508_rs::VersionOrUrl::VersionSpecifier(version_specifier)) => { - // The installed version doesn't satisfy the requirement. - if !version_specifier.contains(distribution.version()) { - return Ok(SatisfiesResult::Unsatisfied(entry.to_string())); - } + match RequirementSatisfaction::check( + distribution, + constraint + .version_or_url + .as_ref() + .map(VersionOrUrlRef::from), + constraint, + )? { + RequirementSatisfaction::Mismatch + | RequirementSatisfaction::OutOfDate => { + return Ok(SatisfiesResult::Unsatisfied(constraint.to_string())) } + RequirementSatisfaction::Satisfied => {} } }