Skip to content

Commit

Permalink
Normalize specifiers by sorting
Browse files Browse the repository at this point in the history
We currently normalize package and extra names and drop the whitespace from version specifiers, but we were not normalizing the order of the specifiers. By sorting them we match the behavior of `packaging` and become independent of build backends reordering specifiers (#6332).

Surprisingly, the snapshot diff isn't large - most people were already writing sorted specifiers. Still, this will lead to observable differences in lockfiles between releases.
  • Loading branch information
konstin committed Aug 21, 2024
1 parent c45f658 commit 002ff34
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 24 deletions.
18 changes: 12 additions & 6 deletions crates/pep440-rs/src/version_specifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ use crate::{
version, Operator, OperatorParseError, Version, VersionPattern, VersionPatternParseError,
};

/// A thin wrapper around `Vec<VersionSpecifier>` with a serde implementation
/// Sorted version specifiers, such as `>=2.1,<3`.
///
/// Python requirements can contain multiple version specifier so we need to store them in a list,
/// such as `>1.2,<2.0` being `[">1.2", "<2.0"]`.
///
/// You can use the serde implementation to e.g. parse `requires-python` from pyproject.toml
///
/// ```rust
/// # use std::str::FromStr;
/// # use pep440_rs::{VersionSpecifiers, Version, Operator};
Expand Down Expand Up @@ -77,19 +75,27 @@ impl VersionSpecifiers {
pub fn is_empty(&self) -> bool {
self.0.is_empty()
}

/// Sort the specifiers.
fn from_unsorted(mut specifiers: Vec<VersionSpecifier>) -> Self {
// TODO(konsti): This seems better than sorting on insert and not getting the size hint,
// but i haven't measured it.
specifiers.sort_by(|a, b| a.version().cmp(b.version()));
Self(specifiers)
}
}

impl FromIterator<VersionSpecifier> for VersionSpecifiers {
fn from_iter<T: IntoIterator<Item = VersionSpecifier>>(iter: T) -> Self {
Self(iter.into_iter().collect())
Self::from_unsorted(iter.into_iter().collect())
}
}

impl FromStr for VersionSpecifiers {
type Err = VersionSpecifiersParseError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
parse_version_specifiers(s).map(Self)
parse_version_specifiers(s).map(Self::from_unsorted)
}
}

Expand Down Expand Up @@ -1742,7 +1748,7 @@ mod tests {
VersionSpecifiers::from_str(">=3.7, < 4.0, != 3.9.0")
.unwrap()
.to_string(),
">=3.7, <4.0, !=3.9.0"
">=3.7, !=3.9.0, <4.0"
);
}

Expand Down
10 changes: 5 additions & 5 deletions crates/pep508-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,7 @@ mod tests {

#[test]
fn basic_examples() {
let input = r"requests[security,tests]>=2.8.1,==2.8.* ; python_full_version < '2.7'";
let input = r"requests[security,tests]==2.8.*,>=2.8.1 ; python_full_version < '2.7'";
let requests = Requirement::<Url>::from_str(input).unwrap();
assert_eq!(input, requests.to_string());
let expected = Requirement {
Expand All @@ -1184,13 +1184,13 @@ mod tests {
version_or_url: Some(VersionOrUrl::VersionSpecifier(
[
VersionSpecifier::from_pattern(
Operator::GreaterThanEqual,
VersionPattern::verbatim(Version::new([2, 8, 1])),
Operator::Equal,
VersionPattern::wildcard(Version::new([2, 8])),
)
.unwrap(),
VersionSpecifier::from_pattern(
Operator::Equal,
VersionPattern::wildcard(Version::new([2, 8])),
Operator::GreaterThanEqual,
VersionPattern::verbatim(Version::new([2, 8, 1])),
)
.unwrap(),
]
Expand Down
12 changes: 6 additions & 6 deletions crates/uv/tests/pip_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ fn check_incompatible_packages() -> Result<()> {
Installed 1 package in [TIME]
- idna==3.6
+ idna==2.4
warning: The package `requests` requires `idna<4,>=2.5`, but `2.4` is installed
warning: The package `requests` requires `idna>=2.5,<4`, but `2.4` is installed
"###
);

Expand All @@ -121,7 +121,7 @@ fn check_incompatible_packages() -> Result<()> {
----- stderr -----
Checked 5 packages in [TIME]
Found 1 incompatibility
The package `requests` requires `idna<4,>=2.5`, but `2.4` is installed
The package `requests` requires `idna>=2.5,<4`, but `2.4` is installed
"###
);

Expand Down Expand Up @@ -180,8 +180,8 @@ fn check_multiple_incompatible_packages() -> Result<()> {
+ idna==2.4
- urllib3==2.2.1
+ urllib3==1.20
warning: The package `requests` requires `idna<4,>=2.5`, but `2.4` is installed
warning: The package `requests` requires `urllib3<3,>=1.21.1`, but `1.20` is installed
warning: The package `requests` requires `idna>=2.5,<4`, but `2.4` is installed
warning: The package `requests` requires `urllib3>=1.21.1,<3`, but `1.20` is installed
"###
);

Expand All @@ -193,8 +193,8 @@ fn check_multiple_incompatible_packages() -> Result<()> {
----- stderr -----
Checked 5 packages in [TIME]
Found 2 incompatibilities
The package `requests` requires `idna<4,>=2.5`, but `2.4` is installed
The package `requests` requires `urllib3<3,>=1.21.1`, but `1.20` is installed
The package `requests` requires `idna>=2.5,<4`, but `2.4` is installed
The package `requests` requires `urllib3>=1.21.1,<3`, but `1.20` is installed
"###
);

Expand Down
12 changes: 6 additions & 6 deletions crates/uv/tests/pip_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1534,9 +1534,9 @@ fn show_version_specifiers_simple() {
exit_code: 0
----- stdout -----
requests v2.31.0
├── charset-normalizer v3.3.2 [required: <4, >=2]
├── idna v3.6 [required: <4, >=2.5]
├── urllib3 v2.2.1 [required: <3, >=1.21.1]
├── charset-normalizer v3.3.2 [required: >=2, <4]
├── idna v3.6 [required: >=2.5, <4]
├── urllib3 v2.2.1 [required: >=1.21.1, <3]
└── certifi v2024.2.2 [required: >=2017.4.17]
----- stderr -----
Expand Down Expand Up @@ -1688,8 +1688,8 @@ fn show_version_specifiers_with_invert() {
joblib v1.3.2
└── scikit-learn v1.4.1.post1 [requires: joblib >=1.2.0]
numpy v1.26.4
├── scikit-learn v1.4.1.post1 [requires: numpy <2.0, >=1.19.5]
└── scipy v1.12.0 [requires: numpy <1.29.0, >=1.22.4]
├── scikit-learn v1.4.1.post1 [requires: numpy >=1.19.5, <2.0]
└── scipy v1.12.0 [requires: numpy >=1.22.4, <1.29.0]
└── scikit-learn v1.4.1.post1 [requires: scipy >=1.6.0]
threadpoolctl v3.4.0
└── scikit-learn v1.4.1.post1 [requires: threadpoolctl >=2.0.0]
Expand Down Expand Up @@ -1739,7 +1739,7 @@ fn show_version_specifiers_with_package() {
exit_code: 0
----- stdout -----
scipy v1.12.0
└── numpy v1.26.4 [required: <1.29.0, >=1.22.4]
└── numpy v1.26.4 [required: >=1.22.4, <1.29.0]
----- stderr -----
"###
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/snapshots/ecosystem__black-lock-file.snap
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ uvloop = [

[package.metadata]
requires-dist = [
{ name = "aiohttp", marker = "implementation_name == 'pypy' and sys_platform == 'win32' and extra == 'd'", specifier = "!=3.9.0,>=3.7.4" },
{ name = "aiohttp", marker = "implementation_name == 'pypy' and sys_platform == 'win32' and extra == 'd'", specifier = ">=3.7.4,!=3.9.0" },
{ name = "aiohttp", marker = "(implementation_name != 'pypy' and extra == 'd') or (sys_platform != 'win32' and extra == 'd')", specifier = ">=3.7.4" },
{ name = "click", specifier = ">=8.0.0" },
{ name = "colorama", marker = "extra == 'colorama'", specifier = ">=0.4.3" },
Expand Down

0 comments on commit 002ff34

Please sign in to comment.