Preserve sort when deciding on requirement placement (#10078)

## Summary

We had the right logic for determining whether the list is already
sorted, but we forgot to apply the same logic when deciding where to
insert the requirement, which made the list _unsorted_ for future
operations.

Closes https://github.com/astral-sh/uv/issues/10076.
This commit is contained in:
Charlie Marsh 2024-12-21 09:43:43 -05:00 committed by GitHub
parent 19a6b5fe4b
commit 705b3da913
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 85 additions and 35 deletions

View file

@ -1,3 +1,4 @@
use std::cmp::Ordering;
use std::path::Path;
use std::str::FromStr;
use std::{fmt, mem};
@ -918,14 +919,35 @@ pub fn add_dependency(
[] => {
#[derive(Debug, Copy, Clone)]
enum Sort {
/// The list is sorted in a case-sensitive manner.
CaseSensitive,
/// The list is sorted in a case-insensitive manner.
CaseInsensitive,
/// The list is sorted in a case-sensitive manner.
CaseSensitive,
/// The list is unsorted.
Unsorted,
}
/// Compare two [`Value`] requirements case-insensitively.
fn case_insensitive(a: &Value, b: &Value) -> Ordering {
a.as_str()
.map(str::to_lowercase)
.as_deref()
.map(split_specifiers)
.cmp(
&b.as_str()
.map(str::to_lowercase)
.as_deref()
.map(split_specifiers),
)
}
/// Compare two [`Value`] requirements case-sensitively.
fn case_sensitive(a: &Value, b: &Value) -> Ordering {
a.as_str()
.map(split_specifiers)
.cmp(&b.as_str().map(split_specifiers))
}
// Determine if the dependency list is sorted prior to
// adding the new dependency; the new dependency list
// will be sorted only when the original list is sorted
@ -940,18 +962,11 @@ pub fn add_dependency(
.all(Value::is_str)
.then(|| {
if deps.iter().tuple_windows().all(|(a, b)| {
a.as_str()
.map(str::to_lowercase)
.as_deref()
.map(split_specifiers)
<= b.as_str()
.map(str::to_lowercase)
.as_deref()
.map(split_specifiers)
matches!(case_insensitive(a, b), Ordering::Less | Ordering::Equal)
}) {
Some(Sort::CaseInsensitive)
} else if deps.iter().tuple_windows().all(|(a, b)| {
a.as_str().map(split_specifiers) <= b.as_str().map(split_specifiers)
matches!(case_sensitive(a, b), Ordering::Less | Ordering::Equal)
}) {
Some(Sort::CaseSensitive)
} else {
@ -963,11 +978,11 @@ pub fn add_dependency(
let req_string = req.to_string();
let index = match sort {
Sort::CaseSensitive => deps
.iter()
.position(|d| d.as_str() > Some(req_string.as_str())),
Sort::CaseInsensitive => deps.iter().position(|d| {
d.as_str().map(str::to_lowercase) > Some(req_string.as_str().to_lowercase())
case_insensitive(d, &Value::from(req_string.as_str())) == Ordering::Greater
}),
Sort::CaseSensitive => deps.iter().position(|d| {
case_sensitive(d, &Value::from(req_string.as_str())) == Ordering::Greater
}),
Sort::Unsorted => None,
};

View file

@ -5853,29 +5853,30 @@ fn sorted_dependencies_name_specifiers() -> Result<()> {
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(indoc! {r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = [
"typing>=3",
"typing-extensions>=4",
]
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12.0"
dependencies = [
"pytest>=8",
"typing-extensions>=4.10.0",
]
"#})?;
uv_snapshot!(context.filters(), context.add().args(["anyio"]), @r###"
uv_snapshot!(context.filters(), universal_windows_filters=true, context.add().args(["pytest-mock"]), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 6 packages in [TIME]
Prepared 5 packages in [TIME]
Installed 5 packages in [TIME]
+ anyio==4.3.0
+ idna==3.6
+ sniffio==1.3.1
+ typing==3.10.0.0
Resolved 8 packages in [TIME]
Prepared 6 packages in [TIME]
Installed 6 packages in [TIME]
+ iniconfig==2.0.0
+ packaging==24.0
+ pluggy==1.4.0
+ pytest==8.1.1
+ pytest-mock==3.14.0
+ typing-extensions==4.10.0
"###);
@ -5889,15 +5890,49 @@ fn sorted_dependencies_name_specifiers() -> Result<()> {
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
requires-python = ">=3.12.[X]"
dependencies = [
"anyio>=4.3.0",
"typing>=3",
"typing-extensions>=4",
"pytest>=8",
"pytest-mock>=3.14.0",
"typing-extensions>=4.10.0",
]
"###
);
});
uv_snapshot!(context.filters(), universal_windows_filters=true, context.add().args(["pytest-randomly"]), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 9 packages in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ pytest-randomly==3.15.0
"###);
let pyproject_toml = context.read("pyproject.toml");
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
pyproject_toml, @r###"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12.[X]"
dependencies = [
"pytest>=8",
"pytest-mock>=3.14.0",
"pytest-randomly>=3.15.0",
"typing-extensions>=4.10.0",
]
"###
);
});
Ok(())
}