Skip to content

Commit 6790bbe

Browse files
committed
Add local
Reverts Check-in updated tests
1 parent 1773224 commit 6790bbe

File tree

10 files changed

+310
-103
lines changed

10 files changed

+310
-103
lines changed

PIP_COMPATIBILITY.md

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,32 @@ broadly.
132132

133133
## Local version identifiers
134134

135-
uv does not implement spec-compliant handling of local version identifiers (e.g., `1.0.0+local`).
136-
Though local version identifiers are rare in published packages (and, e.g., disallowed on PyPI),
137-
they're common in the PyTorch ecosystem. uv's incorrect handling of local version identifiers
138-
may lead to resolution failures in some cases.
139-
140-
In the future, uv intends to implement spec-compliant handling of local version identifiers.
141-
For more, see [#1855](https://github.com/astral-sh/uv/issues/1855).
135+
uv does not implement spec-compliant handling of local version identifiers (e.g., `1.2.3+local`).
136+
This is considered a known limitation. Although local version identifiers are rare in published
137+
packages (and, e.g., disallowed on PyPI), they're common in the PyTorch ecosystem, and uv's approach
138+
to local versions _does_ support typical PyTorch workflows to succeed out-of-the-box.
139+
140+
[PEP 440](https://peps.python.org/pep-0440/#version-specifiers) specifies that the local version
141+
segment should typically be ignored when evaluating version specifiers, with a few exceptions.
142+
For example, `foo==1.2.3` should accept `1.2.3+local`, but `foo==1.2.3+local` should _not_ accept
143+
`1.2.3`. These asymmetries are hard to model in a resolution algorithm. As such, uv treats `1.2.3`
144+
and `1.2.3+local` as entirely separate versions, but respects local versions provided as direct
145+
dependencies throughout the resolution, such that if you provide `foo==1.2.3+local` as a direct
146+
dependency, `1.2.3+local` _will_ be accepted for any transitive dependencies that request
147+
`foo==1.2.3`.
148+
149+
To take an example from the PyTorch ecosystem, it's common to specify `torch==2.0.0+cu118` and
150+
`torchvision==0.15.1+cu118` as direct dependencies. `torchvision @ 0.15.1+cu118` declares a
151+
dependency on `torch==2.0.0`. In this case, uv would recognize that `torch==2.0.0+cu118` satisfies
152+
the specifier, since it was provided as a direct dependency.
153+
154+
As compared to pip, the main differences in observed behavior are as follows:
155+
156+
- In general, local versions must be provided as direct dependencies. Resolution may succeed for
157+
transitive dependencies that request a non-local version, but this is not guaranteed.
158+
- If _only_ local versions exist for a package `foo` at a given version (e.g., `1.2.3+local` exists,
159+
but `1.2.3` does not), `uv pip install foo==1.2.3` will fail, while `pip install foo==1.2.3` will
160+
resolve to an arbitrary local version.
142161

143162
## Packages that exist on multiple indexes
144163

crates/pep440-rs/src/version_specifier.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -340,9 +340,18 @@ impl Serialize for VersionSpecifier {
340340
}
341341

342342
impl VersionSpecifier {
343+
/// Create a new version specifier from an operator and a version.
344+
///
345+
/// Warning: This function is not recommended for general use. It is intended for use in
346+
/// situations where the operator and version are known to be valid. For general use, prefer
347+
/// [`VersionSpecifier::from_pattern`].
348+
pub fn new(operator: Operator, version: Version) -> Self {
349+
Self { operator, version }
350+
}
351+
343352
/// Build from parts, validating that the operator is allowed with that version. The last
344353
/// parameter indicates a trailing `.*`, to differentiate between `1.1.*` and `1.1`
345-
pub fn new(
354+
pub fn from_pattern(
346355
operator: Operator,
347356
version_pattern: VersionPattern,
348357
) -> Result<Self, VersionSpecifierBuildError> {
@@ -545,7 +554,7 @@ impl FromStr for VersionSpecifier {
545554
}
546555
let vpat = version.parse().map_err(ParseErrorKind::InvalidVersion)?;
547556
let version_specifier =
548-
Self::new(operator, vpat).map_err(ParseErrorKind::InvalidSpecifier)?;
557+
Self::from_pattern(operator, vpat).map_err(ParseErrorKind::InvalidSpecifier)?;
549558
s.eat_while(|c: char| c.is_whitespace());
550559
if !s.done() {
551560
return Err(ParseErrorKind::InvalidTrailing(s.after().to_string()).into());
@@ -1664,7 +1673,7 @@ Failed to parse version: Unexpected end of version specifier, expected operator:
16641673
let op = Operator::TildeEqual;
16651674
let v = Version::new([5]);
16661675
let vpat = VersionPattern::verbatim(v);
1667-
assert_eq!(err, VersionSpecifier::new(op, vpat).unwrap_err());
1676+
assert_eq!(err, VersionSpecifier::from_pattern(op, vpat).unwrap_err());
16681677
assert_eq!(
16691678
err.to_string(),
16701679
"The ~= operator requires at least two segments in the release version"

crates/pep508-rs/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,12 +1198,12 @@ mod tests {
11981198
],
11991199
version_or_url: Some(VersionOrUrl::VersionSpecifier(
12001200
[
1201-
VersionSpecifier::new(
1201+
VersionSpecifier::from_pattern(
12021202
Operator::GreaterThanEqual,
12031203
VersionPattern::verbatim(Version::new([2, 8, 1])),
12041204
)
12051205
.unwrap(),
1206-
VersionSpecifier::new(
1206+
VersionSpecifier::from_pattern(
12071207
Operator::Equal,
12081208
VersionPattern::wildcard(Version::new([2, 8])),
12091209
)

crates/pep508-rs/src/marker.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,7 @@ impl MarkerExpression {
595595
Some(operator) => operator,
596596
};
597597

598-
let specifier = match VersionSpecifier::new(operator, r_vpat) {
598+
let specifier = match VersionSpecifier::from_pattern(operator, r_vpat) {
599599
Ok(specifier) => specifier,
600600
Err(err) => {
601601
reporter(
@@ -674,7 +674,7 @@ impl MarkerExpression {
674674
Some(operator) => operator,
675675
};
676676

677-
let specifier = match VersionSpecifier::new(
677+
let specifier = match VersionSpecifier::from_pattern(
678678
operator,
679679
VersionPattern::verbatim(r_version.clone()),
680680
) {
@@ -784,7 +784,7 @@ impl MarkerExpression {
784784
let r_vpat = r_string.parse::<VersionPattern>().ok()?;
785785
let operator = operator.to_pep440_operator()?;
786786
// operator and right hand side make the specifier
787-
let specifier = VersionSpecifier::new(operator, r_vpat).ok()?;
787+
let specifier = VersionSpecifier::from_pattern(operator, r_vpat).ok()?;
788788

789789
let compatible = python_versions
790790
.iter()
@@ -808,7 +808,7 @@ impl MarkerExpression {
808808
let compatible = python_versions.iter().any(|r_version| {
809809
// operator and right hand side make the specifier and in this case the
810810
// right hand is `python_version` so changes every iteration
811-
match VersionSpecifier::new(
811+
match VersionSpecifier::from_pattern(
812812
operator,
813813
VersionPattern::verbatim(r_version.clone()),
814814
) {

crates/uv-resolver/src/error.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,6 @@ pub enum ResolveError {
5252
#[error("There are conflicting URLs for package `{0}`:\n- {1}\n- {2}")]
5353
ConflictingUrlsTransitive(PackageName, String, String),
5454

55-
#[error("There are conflicting versions for `{0}`: {1}")]
56-
ConflictingVersions(String, String),
57-
5855
#[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.")]
5956
DisallowedUrl(PackageName, String),
6057

crates/uv-resolver/src/pubgrub/dependencies.rs

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,23 @@ use crate::constraints::Constraints;
1111
use crate::overrides::Overrides;
1212
use crate::pubgrub::specifier::PubGrubSpecifier;
1313
use crate::pubgrub::PubGrubPackage;
14-
use crate::resolver::Urls;
14+
use crate::resolver::{Locals, Urls};
1515
use crate::ResolveError;
1616

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

2020
impl PubGrubDependencies {
2121
/// Generate a set of `PubGrub` dependencies from a set of requirements.
22+
#[allow(clippy::too_many_arguments)]
2223
pub(crate) fn from_requirements(
2324
requirements: &[Requirement],
2425
constraints: &Constraints,
2526
overrides: &Overrides,
2627
source_name: Option<&PackageName>,
2728
source_extra: Option<&ExtraName>,
2829
urls: &Urls,
30+
locals: &Locals,
2931
env: &MarkerEnvironment,
3032
) -> Result<Self, ResolveError> {
3133
let mut dependencies = Vec::default();
@@ -42,12 +44,12 @@ impl PubGrubDependencies {
4244
}
4345

4446
// Add the package, plus any extra variants.
45-
for result in std::iter::once(to_pubgrub(requirement, None, urls)).chain(
47+
for result in std::iter::once(to_pubgrub(requirement, None, urls, locals)).chain(
4648
requirement
4749
.extras
4850
.clone()
4951
.into_iter()
50-
.map(|extra| to_pubgrub(requirement, Some(extra), urls)),
52+
.map(|extra| to_pubgrub(requirement, Some(extra), urls, locals)),
5153
) {
5254
let (mut package, version) = result?;
5355

@@ -76,12 +78,12 @@ impl PubGrubDependencies {
7678
}
7779

7880
// Add the package, plus any extra variants.
79-
for result in std::iter::once(to_pubgrub(constraint, None, urls)).chain(
81+
for result in std::iter::once(to_pubgrub(constraint, None, urls, locals)).chain(
8082
constraint
8183
.extras
8284
.clone()
8385
.into_iter()
84-
.map(|extra| to_pubgrub(constraint, Some(extra), urls)),
86+
.map(|extra| to_pubgrub(constraint, Some(extra), urls, locals)),
8587
) {
8688
let (mut package, version) = result?;
8789

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

139142
// The requirement has a specifier (e.g., `flask>=1.0`).
140143
Some(VersionOrUrl::VersionSpecifier(specifiers)) => {
141-
let version = specifiers
142-
.iter()
143-
.map(PubGrubSpecifier::try_from)
144-
.fold_ok(Range::full(), |range, specifier| {
145-
range.intersection(&specifier.into())
146-
})?;
144+
// If the specifier is an exact version, and the user requested a local version that's
145+
// more precise than the specifier, use the local version instead.
146+
let version = if let Some(expected) = locals.get(&requirement.name) {
147+
specifiers
148+
.iter()
149+
.map(|specifier| Locals::map(expected, specifier))
150+
.map(|specifier| PubGrubSpecifier::try_from(&specifier))
151+
.fold_ok(Range::full(), |range, specifier| {
152+
range.intersection(&specifier.into())
153+
})?
154+
} else {
155+
specifiers
156+
.iter()
157+
.map(PubGrubSpecifier::try_from)
158+
.fold_ok(Range::full(), |range, specifier| {
159+
range.intersection(&specifier.into())
160+
})?
161+
};
162+
147163
Ok((
148164
PubGrubPackage::from_package(requirement.name.clone(), extra, urls),
149165
version,
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
use rustc_hash::FxHashMap;
2+
3+
use pep440_rs::{Operator, Version, VersionSpecifier};
4+
use pep508_rs::{MarkerEnvironment, VersionOrUrl};
5+
use uv_normalize::PackageName;
6+
7+
use crate::Manifest;
8+
9+
#[derive(Debug, Default)]
10+
pub(crate) struct Locals {
11+
/// A map of package names to their associated, required local versions.
12+
required: FxHashMap<PackageName, Version>,
13+
}
14+
15+
impl Locals {
16+
/// Determine the set of permitted local versions in the [`Manifest`].
17+
pub(crate) fn from_manifest(manifest: &Manifest, markers: &MarkerEnvironment) -> Self {
18+
let mut required: FxHashMap<PackageName, Version> = FxHashMap::default();
19+
20+
// Add all direct requirements and constraints. There's no need to look for conflicts,
21+
// since conflicting versions will be tracked upstream.
22+
for requirement in manifest
23+
.requirements
24+
.iter()
25+
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
26+
.chain(
27+
manifest
28+
.constraints
29+
.iter()
30+
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
31+
)
32+
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
33+
metadata
34+
.requires_dist
35+
.iter()
36+
.filter(|requirement| requirement.evaluate_markers(markers, &editable.extras))
37+
}))
38+
.chain(
39+
manifest
40+
.overrides
41+
.iter()
42+
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
43+
)
44+
{
45+
if let Some(VersionOrUrl::VersionSpecifier(specifiers)) =
46+
requirement.version_or_url.as_ref()
47+
{
48+
for specifier in specifiers.iter() {
49+
if let Some(version) = to_local(specifier) {
50+
required.insert(requirement.name.clone(), version.clone());
51+
}
52+
}
53+
}
54+
}
55+
56+
Self { required }
57+
}
58+
59+
/// Return the local [`Version`] to which a package is pinned, if any.
60+
pub(crate) fn get(&self, package: &PackageName) -> Option<&Version> {
61+
self.required.get(package)
62+
}
63+
64+
/// Given a specifier that may include the version _without_ a local segment, return a specifier
65+
/// that includes the local segment from the expected version.
66+
pub(crate) fn map(local: &Version, specifier: &VersionSpecifier) -> VersionSpecifier {
67+
match specifier.operator() {
68+
Operator::Equal | Operator::EqualStar => {
69+
// Given `foo==1.0.0`, if the local version is `1.0.0+local`, map to
70+
// `foo==1.0.0+local`. This has the intended effect of allowing `1.0.0+local`.
71+
if is_compatible(local, specifier.version()) {
72+
VersionSpecifier::new(Operator::Equal, local.clone())
73+
} else {
74+
specifier.clone()
75+
}
76+
}
77+
Operator::NotEqual | Operator::NotEqualStar => {
78+
// Given `foo!=1.0.0`, if the local version is `1.0.0+local`, map to
79+
// `foo!=1.0.0+local`. This has the intended effect of disallowing `1.0.0+local`.
80+
// There's no risk of including `foo @ 1.0.0` in the resolution, since we _know_
81+
// `foo @ 1.0.0+local` is required and would conflict.
82+
if is_compatible(local, specifier.version()) {
83+
VersionSpecifier::new(Operator::NotEqual, local.clone())
84+
} else {
85+
specifier.clone()
86+
}
87+
}
88+
Operator::LessThanEqual => {
89+
// Given `foo<=1.0.0`, if the local version is `1.0.0+local`, map to
90+
// `foo<=1.0.0+local`. This has the intended effect of allowing `1.0.0+local`.
91+
// There's no risk of including `foo @ 1.0.0.post1` in the resolution, since we
92+
// _know_ `foo @ 1.0.0+local` is required and would conflict.
93+
if is_compatible(local, specifier.version()) {
94+
VersionSpecifier::new(Operator::LessThanEqual, local.clone())
95+
} else {
96+
specifier.clone()
97+
}
98+
}
99+
Operator::GreaterThan => {
100+
// Given `foo>1.0.0`, if the local version is `1.0.0+local`, map to
101+
// `foo>1.0.0+local`. This has the intended effect of disallowing `1.0.0+local`.
102+
if is_compatible(local, specifier.version()) {
103+
VersionSpecifier::new(Operator::GreaterThan, local.clone())
104+
} else {
105+
specifier.clone()
106+
}
107+
}
108+
Operator::ExactEqual => {
109+
// Given `foo===1.0.0`, `1.0.0+local` is already disallowed.
110+
specifier.clone()
111+
}
112+
Operator::TildeEqual => {
113+
// Given `foo~=1.0.0`, `foo~=1.0.0+local` is already allowed.
114+
specifier.clone()
115+
}
116+
Operator::LessThan => {
117+
// Given `foo<1.0.0`, `1.0.0+local` is already disallowed.
118+
specifier.clone()
119+
}
120+
Operator::GreaterThanEqual => {
121+
// Given `foo>=1.0.0`, `foo>1.0.0+local` is already allowed.
122+
specifier.clone()
123+
}
124+
}
125+
}
126+
}
127+
128+
/// Returns `true` if a provided version is compatible with the expected local version.
129+
///
130+
/// The versions are compatible if they are the same including their local segment, or the
131+
/// same except for the local segment, which is empty in the provided version.
132+
///
133+
/// For example, if the expected version is `1.0.0+local` and the provided version is `1.0.0+other`,
134+
/// this function will return `false`.
135+
///
136+
/// If the expected version is `1.0.0+local` and the provided version is `1.0.0`, the function will
137+
/// return `true`.
138+
fn is_compatible(expected: &Version, provided: &Version) -> bool {
139+
// The requirements should be the same, ignoring local segments.
140+
if expected.clone().without_local() != provided.clone().without_local() {
141+
return false;
142+
}
143+
144+
// If the provided version has a local segment, it should be the same as the expected
145+
// version.
146+
if provided.local().is_empty() {
147+
true
148+
} else {
149+
expected.local() == provided.local()
150+
}
151+
}
152+
153+
/// If a [`VersionSpecifier`] represents exact equality against a local version, return the local
154+
/// version.
155+
fn to_local(specifier: &VersionSpecifier) -> Option<&Version> {
156+
if !matches!(specifier.operator(), Operator::Equal | Operator::ExactEqual) {
157+
return None;
158+
};
159+
160+
if specifier.version().local().is_empty() {
161+
return None;
162+
}
163+
164+
Some(specifier.version())
165+
}

0 commit comments

Comments
 (0)