Skip to content

Commit

Permalink
Reverts
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Mar 14, 2024
1 parent 356fa5f commit e404dc9
Show file tree
Hide file tree
Showing 15 changed files with 901 additions and 1,015 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
11 changes: 8 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,14 @@ impl Serialize for VersionSpecifier {
}

impl VersionSpecifier {
/// Create a new version specifier from an operator and a version.
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 +550,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 +1669,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
55 changes: 27 additions & 28 deletions crates/platform-tags/src/tags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,34 +203,33 @@ impl Tags {
wheel_abi_tags: &[String],
wheel_platform_tags: &[String],
) -> TagCompatibility {
return TagCompatibility::Compatible(TagPriority(NonZeroU32::new(1).unwrap()));
// let mut max_compatibility = TagCompatibility::Incompatible(IncompatibleTag::Invalid);
//
// for wheel_py in wheel_python_tags {
// let Some(abis) = self.map.get(wheel_py) else {
// max_compatibility =
// max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Python));
// continue;
// };
// for wheel_abi in wheel_abi_tags {
// let Some(platforms) = abis.get(wheel_abi) else {
// max_compatibility =
// max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Abi));
// continue;
// };
// for wheel_platform in wheel_platform_tags {
// let priority = platforms.get(wheel_platform).copied();
// if let Some(priority) = priority {
// max_compatibility =
// max_compatibility.max(TagCompatibility::Compatible(priority));
// } else {
// max_compatibility = max_compatibility
// .max(TagCompatibility::Incompatible(IncompatibleTag::Platform));
// }
// }
// }
// }
// max_compatibility
let mut max_compatibility = TagCompatibility::Incompatible(IncompatibleTag::Invalid);

for wheel_py in wheel_python_tags {
let Some(abis) = self.map.get(wheel_py) else {
max_compatibility =
max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Python));
continue;
};
for wheel_abi in wheel_abi_tags {
let Some(platforms) = abis.get(wheel_abi) else {
max_compatibility =
max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Abi));
continue;
};
for wheel_platform in wheel_platform_tags {
let priority = platforms.get(wheel_platform).copied();
if let Some(priority) = priority {
max_compatibility =
max_compatibility.max(TagCompatibility::Compatible(priority));
} else {
max_compatibility = max_compatibility
.max(TagCompatibility::Incompatible(IncompatibleTag::Platform));
}
}
}
}
max_compatibility
}
}

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 @@ -55,9 +55,6 @@ pub enum ResolveError {
#[error("There are conflicting local versions requested for package `{0}`: {1} vs. {2}")]
ConflictingLocal(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
51 changes: 19 additions & 32 deletions crates/uv-resolver/src/pubgrub/dependencies.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use itertools::Itertools;
use pubgrub::range::Range;
use tracing::{debug, warn};
use tracing::warn;

use distribution_types::Verbatim;
use pep440_rs::{Operator, Version};
use pep440_rs::Version;
use pep508_rs::{MarkerEnvironment, Requirement, VersionOrUrl};
use uv_normalize::{ExtraName, PackageName};

Expand All @@ -19,6 +19,7 @@ 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,
Expand Down Expand Up @@ -142,37 +143,23 @@ fn to_pubgrub(
Some(VersionOrUrl::VersionSpecifier(specifiers)) => {
// 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.
if let [specifier] = specifiers.as_ref() {
if *specifier.operator() == Operator::Equal {
if let Some(expected) = locals.get(&requirement.name) {
return if locals.is_allowed( expected, specifier.version()) {
debug!(
"using local version {expected} for {name} instead of {specifier}",
expected = expected,
name = requirement.name,
specifier = specifier.version(),
);
Ok((
PubGrubPackage::from_package(requirement.name.clone(), extra, urls),
Range::singleton(expected.clone()),
))
} else {
Err(ResolveError::ConflictingLocal(
requirement.name.clone(),
expected.to_string(),
specifier.version().to_string(),
))
}
}
}
}
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())
})?
};

let version = 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
Loading

0 comments on commit e404dc9

Please sign in to comment.