Skip to content

Commit

Permalink
Add local
Browse files Browse the repository at this point in the history
Reverts

Check-in updated tests
  • Loading branch information
charliermarsh committed Mar 14, 2024
1 parent 1773224 commit bf08dc5
Show file tree
Hide file tree
Showing 11 changed files with 323 additions and 105 deletions.
33 changes: 26 additions & 7 deletions PIP_COMPATIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,32 @@ broadly.

## Local version identifiers

uv does not implement spec-compliant handling of local version identifiers (e.g., `1.0.0+local`).
Though local version identifiers are rare in published packages (and, e.g., disallowed on PyPI),
they're common in the PyTorch ecosystem. uv's incorrect handling of local version identifiers
may lead to resolution failures in some cases.

In the future, uv intends to implement spec-compliant handling of local version identifiers.
For more, see [#1855](https://github.com/astral-sh/uv/issues/1855).
uv does not implement spec-compliant handling of local version identifiers (e.g., `1.2.3+local`).
This is considered a known limitation. Although local version identifiers are rare in published
packages (and, e.g., disallowed on PyPI), they're common in the PyTorch ecosystem, and uv's approach
to local versions _does_ support typical PyTorch workflows to succeed out-of-the-box.

[PEP 440](https://peps.python.org/pep-0440/#version-specifiers) specifies that the local version
segment should typically be ignored when evaluating version specifiers, with a few exceptions.
For example, `foo==1.2.3` should accept `1.2.3+local`, but `foo==1.2.3+local` should _not_ accept
`1.2.3`. These asymmetries are hard to model in a resolution algorithm. As such, uv treats `1.2.3`
and `1.2.3+local` as entirely separate versions, but respects local versions provided as direct
dependencies throughout the resolution, such that if you provide `foo==1.2.3+local` as a direct
dependency, `1.2.3+local` _will_ be accepted for any transitive dependencies that request
`foo==1.2.3`.

To take an example from the PyTorch ecosystem, it's common to specify `torch==2.0.0+cu118` and
`torchvision==0.15.1+cu118` as direct dependencies. `torchvision @ 0.15.1+cu118` declares a
dependency on `torch==2.0.0`. In this case, uv would recognize that `torch==2.0.0+cu118` satisfies
the specifier, since it was provided as a direct dependency.

As compared to pip, the main differences in observed behavior are as follows:

- In general, local versions must be provided as direct dependencies. Resolution may succeed for
transitive dependencies that request a non-local version, but this is not guaranteed.
- If _only_ local versions exist for a package `foo` at a given version (e.g., `1.2.3+local` exists,
but `1.2.3` does not), `uv pip install foo==1.2.3` will fail, while `pip install foo==1.2.3` will
resolve to an arbitrary local version.

## Packages that exist on multiple indexes

Expand Down
15 changes: 12 additions & 3 deletions crates/pep440-rs/src/version_specifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,18 @@ impl Serialize for VersionSpecifier {
}

impl VersionSpecifier {
/// Create a new version specifier from an operator and a version.
///
/// Warning: This function is not recommended for general use. It is intended for use in
/// situations where the operator and version are known to be valid. For general use, prefer
/// [`VersionSpecifier::from_pattern`].
pub fn new(operator: Operator, version: Version) -> Self {
Self { operator, version }
}

/// Build from parts, validating that the operator is allowed with that version. The last
/// parameter indicates a trailing `.*`, to differentiate between `1.1.*` and `1.1`
pub fn new(
pub fn from_pattern(
operator: Operator,
version_pattern: VersionPattern,
) -> Result<Self, VersionSpecifierBuildError> {
Expand Down Expand Up @@ -545,7 +554,7 @@ impl FromStr for VersionSpecifier {
}
let vpat = version.parse().map_err(ParseErrorKind::InvalidVersion)?;
let version_specifier =
Self::new(operator, vpat).map_err(ParseErrorKind::InvalidSpecifier)?;
Self::from_pattern(operator, vpat).map_err(ParseErrorKind::InvalidSpecifier)?;
s.eat_while(|c: char| c.is_whitespace());
if !s.done() {
return Err(ParseErrorKind::InvalidTrailing(s.after().to_string()).into());
Expand Down Expand Up @@ -1664,7 +1673,7 @@ Failed to parse version: Unexpected end of version specifier, expected operator:
let op = Operator::TildeEqual;
let v = Version::new([5]);
let vpat = VersionPattern::verbatim(v);
assert_eq!(err, VersionSpecifier::new(op, vpat).unwrap_err());
assert_eq!(err, VersionSpecifier::from_pattern(op, vpat).unwrap_err());
assert_eq!(
err.to_string(),
"The ~= operator requires at least two segments in the release version"
Expand Down
4 changes: 2 additions & 2 deletions crates/pep508-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1198,12 +1198,12 @@ mod tests {
],
version_or_url: Some(VersionOrUrl::VersionSpecifier(
[
VersionSpecifier::new(
VersionSpecifier::from_pattern(
Operator::GreaterThanEqual,
VersionPattern::verbatim(Version::new([2, 8, 1])),
)
.unwrap(),
VersionSpecifier::new(
VersionSpecifier::from_pattern(
Operator::Equal,
VersionPattern::wildcard(Version::new([2, 8])),
)
Expand Down
8 changes: 4 additions & 4 deletions crates/pep508-rs/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ impl MarkerExpression {
Some(operator) => operator,
};

let specifier = match VersionSpecifier::new(operator, r_vpat) {
let specifier = match VersionSpecifier::from_pattern(operator, r_vpat) {
Ok(specifier) => specifier,
Err(err) => {
reporter(
Expand Down Expand Up @@ -674,7 +674,7 @@ impl MarkerExpression {
Some(operator) => operator,
};

let specifier = match VersionSpecifier::new(
let specifier = match VersionSpecifier::from_pattern(
operator,
VersionPattern::verbatim(r_version.clone()),
) {
Expand Down Expand Up @@ -784,7 +784,7 @@ impl MarkerExpression {
let r_vpat = r_string.parse::<VersionPattern>().ok()?;
let operator = operator.to_pep440_operator()?;
// operator and right hand side make the specifier
let specifier = VersionSpecifier::new(operator, r_vpat).ok()?;
let specifier = VersionSpecifier::from_pattern(operator, r_vpat).ok()?;

let compatible = python_versions
.iter()
Expand All @@ -808,7 +808,7 @@ impl MarkerExpression {
let compatible = python_versions.iter().any(|r_version| {
// operator and right hand side make the specifier and in this case the
// right hand is `python_version` so changes every iteration
match VersionSpecifier::new(
match VersionSpecifier::from_pattern(
operator,
VersionPattern::verbatim(r_version.clone()),
) {
Expand Down
3 changes: 0 additions & 3 deletions crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ pub enum ResolveError {
#[error("There are conflicting URLs for package `{0}`:\n- {1}\n- {2}")]
ConflictingUrlsTransitive(PackageName, String, String),

#[error("There are conflicting versions for `{0}`: {1}")]
ConflictingVersions(String, String),

#[error("Package `{0}` attempted to resolve via URL: {1}. URL dependencies must be expressed as direct requirements or constraints. Consider adding `{0} @ {1}` to your dependencies or constraints file.")]
DisallowedUrl(PackageName, String),

Expand Down
38 changes: 27 additions & 11 deletions crates/uv-resolver/src/pubgrub/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,23 @@ use crate::constraints::Constraints;
use crate::overrides::Overrides;
use crate::pubgrub::specifier::PubGrubSpecifier;
use crate::pubgrub::PubGrubPackage;
use crate::resolver::Urls;
use crate::resolver::{Locals, Urls};
use crate::ResolveError;

#[derive(Debug)]
pub struct PubGrubDependencies(Vec<(PubGrubPackage, Range<Version>)>);

impl PubGrubDependencies {
/// Generate a set of `PubGrub` dependencies from a set of requirements.
#[allow(clippy::too_many_arguments)]
pub(crate) fn from_requirements(
requirements: &[Requirement],
constraints: &Constraints,
overrides: &Overrides,
source_name: Option<&PackageName>,
source_extra: Option<&ExtraName>,
urls: &Urls,
locals: &Locals,
env: &MarkerEnvironment,
) -> Result<Self, ResolveError> {
let mut dependencies = Vec::default();
Expand All @@ -42,12 +44,12 @@ impl PubGrubDependencies {
}

// Add the package, plus any extra variants.
for result in std::iter::once(to_pubgrub(requirement, None, urls)).chain(
for result in std::iter::once(to_pubgrub(requirement, None, urls, locals)).chain(
requirement
.extras
.clone()
.into_iter()
.map(|extra| to_pubgrub(requirement, Some(extra), urls)),
.map(|extra| to_pubgrub(requirement, Some(extra), urls, locals)),
) {
let (mut package, version) = result?;

Expand Down Expand Up @@ -76,12 +78,12 @@ impl PubGrubDependencies {
}

// Add the package, plus any extra variants.
for result in std::iter::once(to_pubgrub(constraint, None, urls)).chain(
for result in std::iter::once(to_pubgrub(constraint, None, urls, locals)).chain(
constraint
.extras
.clone()
.into_iter()
.map(|extra| to_pubgrub(constraint, Some(extra), urls)),
.map(|extra| to_pubgrub(constraint, Some(extra), urls, locals)),
) {
let (mut package, version) = result?;

Expand Down Expand Up @@ -128,6 +130,7 @@ fn to_pubgrub(
requirement: &Requirement,
extra: Option<ExtraName>,
urls: &Urls,
locals: &Locals,
) -> Result<(PubGrubPackage, Range<Version>), ResolveError> {
match requirement.version_or_url.as_ref() {
// The requirement has no specifier (e.g., `flask`).
Expand All @@ -138,12 +141,25 @@ fn to_pubgrub(

// The requirement has a specifier (e.g., `flask>=1.0`).
Some(VersionOrUrl::VersionSpecifier(specifiers)) => {
let version = specifiers
.iter()
.map(PubGrubSpecifier::try_from)
.fold_ok(Range::full(), |range, specifier| {
range.intersection(&specifier.into())
})?;
// If the specifier is an exact version, and the user requested a local version that's
// more precise than the specifier, use the local version instead.
let version = if let Some(expected) = locals.get(&requirement.name) {
specifiers
.iter()
.map(|specifier| Locals::map(expected, specifier))
.map(|specifier| PubGrubSpecifier::try_from(&specifier))
.fold_ok(Range::full(), |range, specifier| {
range.intersection(&specifier.into())
})?
} else {
specifiers
.iter()
.map(PubGrubSpecifier::try_from)
.fold_ok(Range::full(), |range, specifier| {
range.intersection(&specifier.into())
})?
};

Ok((
PubGrubPackage::from_package(requirement.name.clone(), extra, urls),
version,
Expand Down
165 changes: 165 additions & 0 deletions crates/uv-resolver/src/resolver/locals.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
use rustc_hash::FxHashMap;

use pep440_rs::{Operator, Version, VersionSpecifier};
use pep508_rs::{MarkerEnvironment, VersionOrUrl};
use uv_normalize::PackageName;

use crate::Manifest;

#[derive(Debug, Default)]
pub(crate) struct Locals {
/// A map of package names to their associated, required local versions.
required: FxHashMap<PackageName, Version>,
}

impl Locals {
/// Determine the set of permitted local versions in the [`Manifest`].
pub(crate) fn from_manifest(manifest: &Manifest, markers: &MarkerEnvironment) -> Self {
let mut required: FxHashMap<PackageName, Version> = FxHashMap::default();

// Add all direct requirements and constraints. There's no need to look for conflicts,
// since conflicting versions will be tracked upstream.
for requirement in manifest
.requirements
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(
manifest
.constraints
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata
.requires_dist
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &editable.extras))
}))
.chain(
manifest
.overrides
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
{
if let Some(VersionOrUrl::VersionSpecifier(specifiers)) =
requirement.version_or_url.as_ref()
{
for specifier in specifiers.iter() {
if let Some(version) = to_local(specifier) {
required.insert(requirement.name.clone(), version.clone());
}
}
}
}

Self { required }
}

/// Return the local [`Version`] to which a package is pinned, if any.
pub(crate) fn get(&self, package: &PackageName) -> Option<&Version> {
self.required.get(package)
}

/// 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) -> VersionSpecifier {
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::new(Operator::Equal, local.clone())
} else {
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 including `foo @ 1.0.0` in the resolution, since we _know_
// `foo @ 1.0.0+local` is required and would conflict.
if is_compatible(local, specifier.version()) {
VersionSpecifier::new(Operator::NotEqual, local.clone())
} else {
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`.
// There's no risk of including `foo @ 1.0.0.post1` in the resolution, since we
// _know_ `foo @ 1.0.0+local` is required and would conflict.
if is_compatible(local, specifier.version()) {
VersionSpecifier::new(Operator::LessThanEqual, local.clone())
} else {
specifier.clone()
}
}
Operator::GreaterThan => {
// 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`.
if is_compatible(local, specifier.version()) {
VersionSpecifier::new(Operator::GreaterThan, local.clone())
} else {
specifier.clone()
}
}
Operator::ExactEqual => {
// Given `foo===1.0.0`, `1.0.0+local` is already disallowed.
specifier.clone()
}
Operator::TildeEqual => {
// Given `foo~=1.0.0`, `foo~=1.0.0+local` is already allowed.
specifier.clone()
}
Operator::LessThan => {
// Given `foo<1.0.0`, `1.0.0+local` is already disallowed.
specifier.clone()
}
Operator::GreaterThanEqual => {
// Given `foo>=1.0.0`, `foo>1.0.0+local` is already allowed.
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`] represents exact equality against a local version, return the local
/// version.
fn to_local(specifier: &VersionSpecifier) -> Option<&Version> {
if !matches!(specifier.operator(), Operator::Equal | Operator::ExactEqual) {
return None;
};

if specifier.version().local().is_empty() {
return None;
}

Some(specifier.version())
}
Loading

0 comments on commit bf08dc5

Please sign in to comment.