Skip to content

Commit

Permalink
Normalize specifiers by sorting (#6333)
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 in cases where there are
entries in `requires-dist` that were not previously sorted (while the
total number of `requires-dist` is already small compared to the overall
lockfile).
  • Loading branch information
konstin authored Aug 29, 2024
1 parent 9814852 commit 2215448
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 29 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 @@ -1173,7 +1173,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 @@ -1185,13 +1185,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
22 changes: 11 additions & 11 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 @@ -1620,12 +1620,12 @@ fn show_version_specifiers_complex() {
│ ├── docutils v0.20.1 [required: >=0.13.1]
│ └── pygments v2.17.2 [required: >=2.5.1]
├── requests v2.31.0 [required: >=2.20]
│ ├── 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]
├── requests-toolbelt v1.0.0 [required: !=0.9.0, >=0.8.0]
│ └── requests v2.31.0 [required: <3.0.0, >=2.0.1] (*)
├── requests-toolbelt v1.0.0 [required: >=0.8.0, !=0.9.0]
│ └── requests v2.31.0 [required: >=2.0.1, <3.0.0] (*)
├── urllib3 v2.2.1 [required: >=1.26.0]
├── importlib-metadata v7.1.0 [required: >=3.6]
│ └── zipp v3.18.1 [required: >=0.5]
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 2215448

Please sign in to comment.