Remove all special-casing for local version identifiers (#8818)

After https://github.com/astral-sh/uv/pull/8797, we have spec-compliant
handling for local version identifiers and can completely remove all the
special-casing around it.
This commit is contained in:
Charlie Marsh 2024-11-05 22:26:31 -05:00 committed by Zanie Blue
parent c49c7bdf97
commit 90653e1f5e
6 changed files with 3 additions and 396 deletions

View file

@ -106,7 +106,6 @@ impl Manifest {
/// - Determining which requirements should allow yanked versions.
/// - Determining which requirements should allow pre-release versions (e.g., `torch>=2.2.0a1`).
/// - Determining which requirements should allow direct URLs (e.g., `torch @ https://...`).
/// - Determining which requirements should allow local version specifiers (e.g., `torch==2.2.0+cpu`).
pub fn requirements<'a>(
&'a self,
env: &'a ResolverEnvironment,

View file

@ -1,201 +0,0 @@
use std::str::FromStr;
use uv_distribution_filename::{SourceDistFilename, WheelFilename};
use uv_distribution_types::RemoteSource;
use uv_pep440::{Operator, Version, VersionSpecifier, VersionSpecifierBuildError};
use uv_pep508::PackageName;
use uv_pypi_types::RequirementSource;
use crate::resolver::ForkMap;
use crate::{DependencyMode, Manifest, ResolverEnvironment};
/// A map of package names to their associated, required local versions across all forks.
#[derive(Debug, Default, Clone)]
pub(crate) struct Locals(ForkMap<Version>);
impl Locals {
/// Determine the set of permitted local versions in the [`Manifest`].
pub(crate) fn from_manifest(
manifest: &Manifest,
env: &ResolverEnvironment,
dependencies: DependencyMode,
) -> Self {
let mut locals = ForkMap::default();
// Add all direct requirements and constraints. There's no need to look for conflicts,
// since conflicts will be enforced by the solver.
for requirement in manifest.requirements(env, dependencies) {
if let Some(local) = from_source(&requirement.source) {
locals.add(&requirement, local);
}
}
Self(locals)
}
/// Return a list of local versions that are compatible with a package in the given fork.
pub(crate) fn get(
&self,
package_name: &PackageName,
env: &ResolverEnvironment,
) -> Vec<&Version> {
self.0.get(package_name, env)
}
/// Given a specifier that may include the version _without_ a local segment, return a specifier
/// that includes the local segment from the expected version.
pub(crate) fn map(
local: &Version,
specifier: &VersionSpecifier,
) -> Result<VersionSpecifier, VersionSpecifierBuildError> {
match specifier.operator() {
Operator::Equal | Operator::EqualStar => {
// Given `foo==1.0.0`, if the local version is `1.0.0+local`, map to
// `foo==1.0.0+local`.
//
// This has the intended effect of allowing `1.0.0+local`.
if is_compatible(local, specifier.version()) {
VersionSpecifier::from_version(Operator::Equal, local.clone())
} else {
Ok(specifier.clone())
}
}
Operator::NotEqual | Operator::NotEqualStar => {
// Given `foo!=1.0.0`, if the local version is `1.0.0+local`, map to
// `foo!=1.0.0+local`.
//
// This has the intended effect of disallowing `1.0.0+local`.
//
// There's no risk of accidentally including `foo @ 1.0.0` in the resolution, since
// we _know_ `foo @ 1.0.0+local` is required and would therefore conflict.
if is_compatible(local, specifier.version()) {
VersionSpecifier::from_version(Operator::NotEqual, local.clone())
} else {
Ok(specifier.clone())
}
}
Operator::LessThanEqual => {
// Given `foo<=1.0.0`, if the local version is `1.0.0+local`, map to
// `foo==1.0.0+local`.
//
// This has the intended effect of allowing `1.0.0+local`.
//
// Since `foo==1.0.0+local` is already required, we know that to satisfy
// `foo<=1.0.0`, we _must_ satisfy `foo==1.0.0+local`. We _could_ map to
// `foo<=1.0.0+local`, but local versions are _not_ allowed in exclusive ordered
// specifiers, so introducing `foo<=1.0.0+local` would risk breaking invariants.
if is_compatible(local, specifier.version()) {
VersionSpecifier::from_version(Operator::Equal, local.clone())
} else {
Ok(specifier.clone())
}
}
Operator::GreaterThan => {
// Given `foo>1.0.0`, `foo @ 1.0.0+local` is already (correctly) disallowed.
Ok(specifier.clone())
}
Operator::ExactEqual => {
// Given `foo===1.0.0`, `1.0.0+local` is already (correctly) disallowed.
Ok(specifier.clone())
}
Operator::TildeEqual => {
// Given `foo~=1.0.0`, `foo~=1.0.0+local` is already (correctly) allowed.
Ok(specifier.clone())
}
Operator::LessThan => {
// Given `foo<1.0.0`, `1.0.0+local` is already (correctly) disallowed.
Ok(specifier.clone())
}
Operator::GreaterThanEqual => {
// Given `foo>=1.0.0`, `foo @ 1.0.0+local` is already (correctly) allowed.
Ok(specifier.clone())
}
}
}
}
/// Returns `true` if a provided version is compatible with the expected local version.
///
/// The versions are compatible if they are the same including their local segment, or the
/// same except for the local segment, which is empty in the provided version.
///
/// For example, if the expected version is `1.0.0+local` and the provided version is `1.0.0+other`,
/// this function will return `false`.
///
/// If the expected version is `1.0.0+local` and the provided version is `1.0.0`, the function will
/// return `true`.
fn is_compatible(expected: &Version, provided: &Version) -> bool {
// The requirements should be the same, ignoring local segments.
if expected.clone().without_local() != provided.clone().without_local() {
return false;
}
// If the provided version has a local segment, it should be the same as the expected
// version.
if provided.local().is_empty() {
true
} else {
expected.local() == provided.local()
}
}
/// If a [`VersionSpecifier`] contains an exact equality specifier for a local version,
/// returns the local version.
pub(crate) fn from_source(source: &RequirementSource) -> Option<Version> {
match source {
// Extract all local versions from specifiers that require an exact version (e.g.,
// `==1.0.0+local`).
RequirementSource::Registry {
specifier: version, ..
} => version
.iter()
.filter(|specifier| {
matches!(specifier.operator(), Operator::Equal | Operator::ExactEqual)
})
.filter(|specifier| !specifier.version().local().is_empty())
.map(|specifier| specifier.version().clone())
// It's technically possible for there to be multiple local segments here.
// For example, `a==1.0+foo,==1.0+bar`. However, in that case resolution
// will fail later.
.next(),
// Exact a local version from a URL, if it includes a fully-qualified filename (e.g.,
// `torch-2.2.1%2Bcu118-cp311-cp311-linux_x86_64.whl`).
RequirementSource::Url { url, .. } => url
.filename()
.ok()
.and_then(|filename| {
if let Ok(filename) = WheelFilename::from_str(&filename) {
Some(filename.version)
} else if let Ok(filename) =
SourceDistFilename::parsed_normalized_filename(&filename)
{
Some(filename.version)
} else {
None
}
})
.filter(uv_pep440::Version::is_local),
RequirementSource::Git { .. } => None,
RequirementSource::Path {
install_path: path, ..
} => path
.file_name()
.and_then(|filename| {
let filename = filename.to_string_lossy();
if let Ok(filename) = WheelFilename::from_str(&filename) {
Some(filename.version)
} else if let Ok(filename) =
SourceDistFilename::parsed_normalized_filename(&filename)
{
Some(filename.version)
} else {
None
}
})
.filter(uv_pep440::Version::is_local),
RequirementSource::Directory { .. } => None,
}
}
#[cfg(test)]
mod tests;

View file

@ -1,130 +0,0 @@
use std::str::FromStr;
use anyhow::Result;
use url::Url;
use uv_pep440::{Operator, Version, VersionSpecifier, VersionSpecifiers};
use uv_pep508::VerbatimUrl;
use uv_pypi_types::ParsedUrl;
use uv_pypi_types::RequirementSource;
use super::{from_source, Locals};
#[test]
fn extract_locals() -> Result<()> {
// Extract from a source distribution in a URL.
let url = VerbatimUrl::from_url(Url::parse("https://example.com/foo-1.0.0+local.tar.gz")?);
let source =
RequirementSource::from_parsed_url(ParsedUrl::try_from(url.to_url()).unwrap(), url);
let locals: Vec<_> = from_source(&source).into_iter().collect();
assert_eq!(locals, vec![Version::from_str("1.0.0+local")?]);
// Extract from a wheel in a URL.
let url = VerbatimUrl::from_url(Url::parse(
"https://example.com/foo-1.0.0+local-cp39-cp39-linux_x86_64.whl",
)?);
let source =
RequirementSource::from_parsed_url(ParsedUrl::try_from(url.to_url()).unwrap(), url);
let locals: Vec<_> = from_source(&source).into_iter().collect();
assert_eq!(locals, vec![Version::from_str("1.0.0+local")?]);
// Don't extract anything if the URL is opaque.
let url = VerbatimUrl::from_url(Url::parse("git+https://example.com/foo/bar")?);
let source =
RequirementSource::from_parsed_url(ParsedUrl::try_from(url.to_url()).unwrap(), url);
let locals: Vec<_> = from_source(&source).into_iter().collect();
assert!(locals.is_empty());
// Extract from `==` specifiers.
let version = VersionSpecifiers::from_iter([
VersionSpecifier::from_version(Operator::GreaterThan, Version::from_str("1.0.0")?)?,
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+local")?)?,
]);
let source = RequirementSource::Registry {
specifier: version,
index: None,
};
let locals: Vec<_> = from_source(&source).into_iter().collect();
assert_eq!(locals, vec![Version::from_str("1.0.0+local")?]);
// Ignore other specifiers.
let version = VersionSpecifiers::from_iter([VersionSpecifier::from_version(
Operator::NotEqual,
Version::from_str("1.0.0+local")?,
)?]);
let source = RequirementSource::Registry {
specifier: version,
index: None,
};
let locals: Vec<_> = from_source(&source).into_iter().collect();
assert!(locals.is_empty());
Ok(())
}
#[test]
fn map_version() -> Result<()> {
// Given `==1.0.0`, if the local version is `1.0.0+local`, map to `==1.0.0+local`.
let local = Version::from_str("1.0.0+local")?;
let specifier = VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0")?)?;
assert_eq!(
Locals::map(&local, &specifier)?,
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+local")?)?
);
// Given `!=1.0.0`, if the local version is `1.0.0+local`, map to `!=1.0.0+local`.
let local = Version::from_str("1.0.0+local")?;
let specifier =
VersionSpecifier::from_version(Operator::NotEqual, Version::from_str("1.0.0")?)?;
assert_eq!(
Locals::map(&local, &specifier)?,
VersionSpecifier::from_version(Operator::NotEqual, Version::from_str("1.0.0+local")?)?
);
// Given `<=1.0.0`, if the local version is `1.0.0+local`, map to `==1.0.0+local`.
let local = Version::from_str("1.0.0+local")?;
let specifier =
VersionSpecifier::from_version(Operator::LessThanEqual, Version::from_str("1.0.0")?)?;
assert_eq!(
Locals::map(&local, &specifier)?,
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+local")?)?
);
// Given `>1.0.0`, `1.0.0+local` is already (correctly) disallowed.
let local = Version::from_str("1.0.0+local")?;
let specifier =
VersionSpecifier::from_version(Operator::GreaterThan, Version::from_str("1.0.0")?)?;
assert_eq!(
Locals::map(&local, &specifier)?,
VersionSpecifier::from_version(Operator::GreaterThan, Version::from_str("1.0.0")?)?
);
// Given `===1.0.0`, `1.0.0+local` is already (correctly) disallowed.
let local = Version::from_str("1.0.0+local")?;
let specifier =
VersionSpecifier::from_version(Operator::ExactEqual, Version::from_str("1.0.0")?)?;
assert_eq!(
Locals::map(&local, &specifier)?,
VersionSpecifier::from_version(Operator::ExactEqual, Version::from_str("1.0.0")?)?
);
// Given `==1.0.0+local`, `1.0.0+local` is already (correctly) allowed.
let local = Version::from_str("1.0.0+local")?;
let specifier =
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+local")?)?;
assert_eq!(
Locals::map(&local, &specifier)?,
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+local")?)?
);
// Given `==1.0.0+other`, `1.0.0+local` is already (correctly) disallowed.
let local = Version::from_str("1.0.0+local")?;
let specifier =
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+other")?)?;
assert_eq!(
Locals::map(&local, &specifier)?,
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+other")?)?
);
Ok(())
}

View file

@ -24,7 +24,6 @@ use tracing::{debug, info, instrument, trace, warn, Level};
pub use environment::ResolverEnvironment;
pub(crate) use fork_map::{ForkMap, ForkSet};
use locals::Locals;
pub(crate) use urls::Urls;
use uv_configuration::{Constraints, Overrides};
use uv_distribution::{ArchiveMetadata, DistributionDatabase};
@ -81,7 +80,6 @@ mod fork_map;
mod groups;
mod index;
mod indexes;
mod locals;
mod provider;
mod reporter;
mod urls;
@ -105,7 +103,6 @@ struct ResolverState<InstalledPackages: InstalledPackagesProvider> {
locations: IndexLocations,
exclusions: Exclusions,
urls: Urls,
locals: Locals,
indexes: Indexes,
dependency_mode: DependencyMode,
hasher: HashStrategy,
@ -211,7 +208,6 @@ impl<Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvider>
selector: CandidateSelector::for_resolution(options, &manifest, &env),
dependency_mode: options.dependency_mode,
urls: Urls::from_manifest(&manifest, &env, git, options.dependency_mode)?,
locals: Locals::from_manifest(&manifest, &env, options.dependency_mode),
indexes: Indexes::from_manifest(&manifest, &env, options.dependency_mode),
groups: Groups::from_manifest(&manifest, &env),
project: manifest.project,
@ -535,7 +531,6 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
&version,
&self.urls,
&self.indexes,
&self.locals,
dependencies.clone(),
&self.git,
self.selector.resolution_strategy(),
@ -704,7 +699,6 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
version,
&self.urls,
&self.indexes,
&self.locals,
fork.dependencies.clone(),
&self.git,
self.selector.resolution_strategy(),
@ -2164,14 +2158,13 @@ impl ForkState {
}
/// Add the dependencies for the selected version of the current package, checking for
/// self-dependencies, and handling URLs and locals.
/// self-dependencies and handling URLs.
fn add_package_version_dependencies(
&mut self,
for_package: Option<&str>,
version: &Version,
urls: &Urls,
indexes: &Indexes,
locals: &Locals,
mut dependencies: Vec<PubGrubDependency>,
git: &GitResolver,
resolution_strategy: &ResolutionStrategy,
@ -2180,8 +2173,8 @@ impl ForkState {
let PubGrubDependency {
package,
version,
specifier,
url,
..
} = dependency;
let mut has_url = false;
@ -2195,31 +2188,6 @@ impl ForkState {
has_url = true;
};
// If the specifier is an exact version and the user requested a local version for this
// fork that's more precise than the specifier, use the local version instead.
if let Some(specifier) = specifier {
let locals = locals.get(name, &self.env);
// It's possible that there are multiple matching local versions requested with
// different marker expressions. All of these are potentially compatible until we
// narrow to a specific fork.
for local in locals {
let local = specifier
.iter()
.map(|specifier| {
Locals::map(local, specifier)
.map_err(ResolveError::InvalidVersion)
.map(Ranges::from)
})
.fold_ok(Range::full(), |range, specifier| {
range.intersection(&specifier)
})?;
// Add the local version.
*version = version.union(&local);
}
}
// If the package is pinned to an exact index, add it to the fork.
for index in indexes.get(name, &self.env) {
self.fork_indexes.insert(name, index, &self.env)?;