Skip to content

Commit 5a319e0

Browse files
charliermarshcharliermarsh
committed
Update tests
1 parent ab69fe2 commit 5a319e0

File tree

9 files changed

+1240
-179
lines changed

9 files changed

+1240
-179
lines changed

‎crates/pep440-rs/src/lib.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,10 @@ pub use {
4141
LocalSegment, Operator, OperatorParseError, PreRelease, PreReleaseKind, Version,
4242
VersionParseError, VersionPattern, VersionPatternParseError, MIN_VERSION,
4343
},
44-
version_specifier::{VersionSpecifier, VersionSpecifiers, VersionSpecifiersParseError},
44+
version_specifier::{
45+
VersionSpecifier, VersionSpecifierBuildError, VersionSpecifiers,
46+
VersionSpecifiersParseError,
47+
},
4548
};
4649

4750
mod version;

‎crates/pep440-rs/src/version_specifier.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -340,15 +340,6 @@ 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-
352343
/// Build from parts, validating that the operator is allowed with that version. The last
353344
/// parameter indicates a trailing `.*`, to differentiate between `1.1.*` and `1.1`
354345
pub fn from_pattern(
@@ -357,10 +348,6 @@ impl VersionSpecifier {
357348
) -> Result<Self, VersionSpecifierBuildError> {
358349
let star = version_pattern.is_wildcard();
359350
let version = version_pattern.into_version();
360-
// "Local version identifiers are NOT permitted in this version specifier."
361-
if version.is_local() && !operator.is_local_compatible() {
362-
return Err(BuildErrorKind::OperatorLocalCombo { operator, version }.into());
363-
}
364351

365352
// Check if there are star versions and if so, switch operator to star version
366353
let operator = if star {
@@ -374,6 +361,19 @@ impl VersionSpecifier {
374361
operator
375362
};
376363

364+
Self::from_version(operator, version)
365+
}
366+
367+
/// Create a new version specifier from an operator and a version.
368+
pub fn from_version(
369+
operator: Operator,
370+
version: Version,
371+
) -> Result<Self, VersionSpecifierBuildError> {
372+
// "Local version identifiers are NOT permitted in this version specifier."
373+
if version.is_local() && !operator.is_local_compatible() {
374+
return Err(BuildErrorKind::OperatorLocalCombo { operator, version }.into());
375+
}
376+
377377
if operator == Operator::TildeEqual && version.release().len() < 2 {
378378
return Err(BuildErrorKind::CompatibleRelease.into());
379379
}

‎crates/uv-resolver/src/error.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ pub enum ResolveError {
8484
version: Box<Version>,
8585
},
8686

87+
#[error("Attempted to construct an invalid version specifier")]
88+
InvalidVersion(#[from] pep440_rs::VersionSpecifierBuildError),
89+
8790
/// Something unexpected happened.
8891
#[error("{0}")]
8992
Failure(String),

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,11 @@ fn to_pubgrub(
146146
let version = if let Some(expected) = locals.get(&requirement.name) {
147147
specifiers
148148
.iter()
149-
.map(|specifier| Locals::map(expected, specifier))
150-
.map(|specifier| PubGrubSpecifier::try_from(&specifier))
149+
.map(|specifier| {
150+
Locals::map(expected, specifier)
151+
.map_err(ResolveError::InvalidVersion)
152+
.and_then(|specifier| PubGrubSpecifier::try_from(&specifier))
153+
})
151154
.fold_ok(Range::full(), |range, specifier| {
152155
range.intersection(&specifier.into())
153156
})?

‎crates/uv-resolver/src/resolver/locals.rs‎

Lines changed: 117 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use rustc_hash::FxHashMap;
22

3-
use pep440_rs::{Operator, Version, VersionSpecifier};
3+
use pep440_rs::{Operator, Version, VersionSpecifier, VersionSpecifierBuildError};
44
use pep508_rs::{MarkerEnvironment, VersionOrUrl};
55
use uv_normalize::PackageName;
66

@@ -63,63 +63,71 @@ impl Locals {
6363

6464
/// Given a specifier that may include the version _without_ a local segment, return a specifier
6565
/// that includes the local segment from the expected version.
66-
pub(crate) fn map(local: &Version, specifier: &VersionSpecifier) -> VersionSpecifier {
66+
pub(crate) fn map(
67+
local: &Version,
68+
specifier: &VersionSpecifier,
69+
) -> Result<VersionSpecifier, VersionSpecifierBuildError> {
6770
match specifier.operator() {
6871
Operator::Equal | Operator::EqualStar => {
6972
// 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`.
73+
// `foo==1.0.0+local`.
74+
//
75+
// This has the intended effect of allowing `1.0.0+local`.
7176
if is_compatible(local, specifier.version()) {
72-
VersionSpecifier::new(Operator::Equal, local.clone())
77+
VersionSpecifier::from_version(Operator::Equal, local.clone())
7378
} else {
74-
specifier.clone()
79+
Ok(specifier.clone())
7580
}
7681
}
7782
Operator::NotEqual | Operator::NotEqualStar => {
7883
// 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.
84+
// `foo!=1.0.0+local`.
85+
//
86+
// This has the intended effect of disallowing `1.0.0+local`.
87+
//
88+
// There's no risk of accidentally including `foo @ 1.0.0` in the resolution, since
89+
// we _know_ `foo @ 1.0.0+local` is required and would therefore conflict.
8290
if is_compatible(local, specifier.version()) {
83-
VersionSpecifier::new(Operator::NotEqual, local.clone())
91+
VersionSpecifier::from_version(Operator::NotEqual, local.clone())
8492
} else {
85-
specifier.clone()
93+
Ok(specifier.clone())
8694
}
8795
}
8896
Operator::LessThanEqual => {
8997
// 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.
98+
// `foo==1.0.0+local`.
99+
//
100+
// This has the intended effect of allowing `1.0.0+local`.
101+
//
102+
// Since `foo==1.0.0+local` is already required, we know that to satisfy
103+
// `foo<=1.0.0`, we _must_ satisfy `foo==1.0.0+local`. We _could_ map to
104+
// `foo<=1.0.0+local`, but local versions are _not_ allowed in exclusive ordered
105+
// specifiers, so introducing `foo<=1.0.0+local` would risk breaking invariants.
93106
if is_compatible(local, specifier.version()) {
94-
VersionSpecifier::new(Operator::LessThanEqual, local.clone())
107+
VersionSpecifier::from_version(Operator::Equal, local.clone())
95108
} else {
96-
specifier.clone()
109+
Ok(specifier.clone())
97110
}
98111
}
99112
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-
}
113+
// Given `foo>1.0.0`, `foo @ 1.0.0+local` is already (correctly) disallowed.
114+
Ok(specifier.clone())
107115
}
108116
Operator::ExactEqual => {
109-
// Given `foo===1.0.0`, `1.0.0+local` is already disallowed.
110-
specifier.clone()
117+
// Given `foo===1.0.0`, `1.0.0+local` is already (correctly) disallowed.
118+
Ok(specifier.clone())
111119
}
112120
Operator::TildeEqual => {
113-
// Given `foo~=1.0.0`, `foo~=1.0.0+local` is already allowed.
114-
specifier.clone()
121+
// Given `foo~=1.0.0`, `foo~=1.0.0+local` is already (correctly) allowed.
122+
Ok(specifier.clone())
115123
}
116124
Operator::LessThan => {
117-
// Given `foo<1.0.0`, `1.0.0+local` is already disallowed.
118-
specifier.clone()
125+
// Given `foo<1.0.0`, `1.0.0+local` is already (correctly) disallowed.
126+
Ok(specifier.clone())
119127
}
120128
Operator::GreaterThanEqual => {
121-
// Given `foo>=1.0.0`, `foo>1.0.0+local` is already allowed.
122-
specifier.clone()
129+
// Given `foo>=1.0.0`, `foo @ 1.0.0+local` is already (correctly) allowed.
130+
Ok(specifier.clone())
123131
}
124132
}
125133
}
@@ -163,3 +171,82 @@ fn to_local(specifier: &VersionSpecifier) -> Option<&Version> {
163171

164172
Some(specifier.version())
165173
}
174+
175+
#[cfg(test)]
176+
mod tests {
177+
use std::str::FromStr;
178+
179+
use anyhow::Result;
180+
181+
use pep440_rs::{Operator, Version, VersionSpecifier};
182+
183+
use super::Locals;
184+
185+
#[test]
186+
fn map_version() -> Result<()> {
187+
// Given `==1.0.0`, if the local version is `1.0.0+local`, map to `==1.0.0+local`.
188+
let local = Version::from_str("1.0.0+local")?;
189+
let specifier =
190+
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0")?)?;
191+
assert_eq!(
192+
Locals::map(&local, &specifier)?,
193+
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+local")?)?
194+
);
195+
196+
// Given `!=1.0.0`, if the local version is `1.0.0+local`, map to `!=1.0.0+local`.
197+
let local = Version::from_str("1.0.0+local")?;
198+
let specifier =
199+
VersionSpecifier::from_version(Operator::NotEqual, Version::from_str("1.0.0")?)?;
200+
assert_eq!(
201+
Locals::map(&local, &specifier)?,
202+
VersionSpecifier::from_version(Operator::NotEqual, Version::from_str("1.0.0+local")?)?
203+
);
204+
205+
// Given `<=1.0.0`, if the local version is `1.0.0+local`, map to `==1.0.0+local`.
206+
let local = Version::from_str("1.0.0+local")?;
207+
let specifier =
208+
VersionSpecifier::from_version(Operator::LessThanEqual, Version::from_str("1.0.0")?)?;
209+
assert_eq!(
210+
Locals::map(&local, &specifier)?,
211+
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+local")?)?
212+
);
213+
214+
// Given `>1.0.0`, `1.0.0+local` is already (correctly) disallowed.
215+
let local = Version::from_str("1.0.0+local")?;
216+
let specifier =
217+
VersionSpecifier::from_version(Operator::GreaterThan, Version::from_str("1.0.0")?)?;
218+
assert_eq!(
219+
Locals::map(&local, &specifier)?,
220+
VersionSpecifier::from_version(Operator::GreaterThan, Version::from_str("1.0.0")?)?
221+
);
222+
223+
// Given `===1.0.0`, `1.0.0+local` is already (correctly) disallowed.
224+
let local = Version::from_str("1.0.0+local")?;
225+
let specifier =
226+
VersionSpecifier::from_version(Operator::ExactEqual, Version::from_str("1.0.0")?)?;
227+
assert_eq!(
228+
Locals::map(&local, &specifier)?,
229+
VersionSpecifier::from_version(Operator::ExactEqual, Version::from_str("1.0.0")?)?
230+
);
231+
232+
// Given `==1.0.0+local`, `1.0.0+local` is already (correctly) allowed.
233+
let local = Version::from_str("1.0.0+local")?;
234+
let specifier =
235+
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+local")?)?;
236+
assert_eq!(
237+
Locals::map(&local, &specifier)?,
238+
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+local")?)?
239+
);
240+
241+
// Given `==1.0.0+other`, `1.0.0+local` is already (correctly) disallowed.
242+
let local = Version::from_str("1.0.0+local")?;
243+
let specifier =
244+
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+other")?)?;
245+
assert_eq!(
246+
Locals::map(&local, &specifier)?,
247+
VersionSpecifier::from_version(Operator::Equal, Version::from_str("1.0.0+other")?)?
248+
);
249+
250+
Ok(())
251+
}
252+
}

‎crates/uv-resolver/src/resolver/mod.rs‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -881,7 +881,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
881881
.await
882882
.ok_or(ResolveError::Unregistered)?;
883883

884-
let constraints = PubGrubDependencies::from_requirements(
884+
let mut constraints = PubGrubDependencies::from_requirements(
885885
&metadata.requires_dist,
886886
&self.constraints,
887887
&self.overrides,
@@ -890,14 +890,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
890890
&self.urls,
891891
&self.locals,
892892
self.markers,
893-
);
894-
895-
let mut constraints = match constraints {
896-
Ok(constraints) => constraints,
897-
Err(err) => {
898-
return Ok(Dependencies::Unavailable(uncapitalize(err.to_string())));
899-
}
900-
};
893+
)?;
901894

902895
for (package, version) in constraints.iter() {
903896
debug!("Adding transitive dependency: {package}{version}");

‎crates/uv/tests/pip_compile.rs‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,22 +1501,11 @@ fn disallowed_transitive_url_dependency() -> Result<()> {
15011501
.arg("requirements.in")
15021502
.env("HATCHLING", hatchling_path.as_os_str()), @r###"
15031503
success: false
1504-
exit_code: 1
1504+
exit_code: 2
15051505
----- stdout -----
15061506
15071507
----- stderr -----
1508-
× No solution found when resolving dependencies:
1509-
╰─▶ Because only hatchling-editable==0.1.0 is available and
1510-
hatchling-editable==0.1.0 is unusable because its dependencies are
1511-
unusable because package `iniconfig` attempted to resolve via URL:
1512-
git+https://github.com/pytest-dev/iniconfig@9cae43103df70bac6fde7b9f35ad11a9f1be0cb4.
1513-
URL dependencies must be expressed as direct
1514-
requirements or constraints. Consider adding `iniconfig @
1515-
git+https://github.com/pytest-dev/iniconfig@9cae43103df70bac6fde7b9f35ad11a9f1be0cb4`
1516-
to your dependencies or constraints file., we can conclude that all
1517-
versions of hatchling-editable cannot be used.
1518-
And because you require hatchling-editable, we can conclude that the
1519-
requirements are unsatisfiable.
1508+
error: Package `iniconfig` attempted to resolve via URL: git+https://github.com/pytest-dev/iniconfig@9cae43103df70bac6fde7b9f35ad11a9f1be0cb4. URL dependencies must be expressed as direct requirements or constraints. Consider adding `iniconfig @ git+https://github.com/pytest-dev/iniconfig@9cae43103df70bac6fde7b9f35ad11a9f1be0cb4` to your dependencies or constraints file.
15201509
"###
15211510
);
15221511

0 commit comments

Comments
 (0)