mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-08-04 19:08:04 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Fix detection of sorted dependencies when include-group is used (#13354)

Browse source 
This follows on from #13334 to fix another case.

<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

If a dependency group contained any `{ include-group = "..." }` entries,
the sort detection would bail out. The root cause of the problem was
gating the sort detection behind `deps.iter().all(Value::is_str)`.

A public code search reveals that keeping include-groups at the top is
by far the most common, but keeping them at the bottom isn't uncommon.
In both of these cases, uv will now preserve the convention that is in
use.

Unless I've missed it, I don't think uv supports `uv add`ing an
include-group, and so that wasn't tested here.

## Test Plan

cargo test

---------

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
Frazer McLean 2025-05-10 20:00:27 +02:00 • committed by GitHub
parent 8d6d616791
commit 62692b4e1b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 247 additions and 73 deletions
Download patch file Download diff file Expand all files Collapse all files

125
crates/uv-workspace/src/pyproject_mut.rs
View file

@ -1,4 +1,3 @@
use std::cmp::Ordering;
use std::path::Path;
use std::str::FromStr;
use std::{fmt, mem};
@ -1019,96 +1018,76 @@ pub fn add_dependency(
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),
)
fn is_sorted<T, I>(items: I) -> bool
where
I: IntoIterator<Item = T>,
T: PartialOrd + Copy,
{
items.into_iter().tuple_windows().all(|(a, b)| a <= b)
}
/// Naively compare two [`Value`] requirements case-insensitively.
fn case_insensitive_naive(a: &Value, b: &Value) -> Ordering {
a.as_str()
.map(str::to_lowercase)
.cmp(&b.as_str().map(str::to_lowercase))
}
/// 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))
}
/// Naively compare two [`Value`] requirements case-sensitively.
fn case_sensitive_naive(a: &Value, b: &Value) -> Ordering {
a.as_str().cmp(&b.as_str())
}
// `deps` are either requirements (strings) or include groups (inline tables).
// Here we pull out just the requirements for determining the sort.
let reqs: Vec<_> = deps.iter().filter_map(Value::as_str).collect();
let reqs_lowercase: Vec<_> = reqs.iter().copied().map(str::to_lowercase).collect();
// 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
// so that user's custom dependency ordering is preserved.
//
// Additionally, if the table is invalid (i.e. contains non-string values)
// we still treat it as unsorted for the sake of simplicity.
// Any items which aren't strings are ignored, e.g.
// `{ include-group = "..." }` in dependency-groups.
//
// We account for both case-sensitive and case-insensitive sorting.
let sort = deps
.iter()
.all(Value::is_str)
.then(|| {
if deps.iter().tuple_windows().all(|(a, b)| {
matches!(case_insensitive(a, b), Ordering::Less | Ordering::Equal)
}) {
Some(Sort::CaseInsensitive)
} else if deps.iter().tuple_windows().all(|(a, b)| {
matches!(case_sensitive(a, b), Ordering::Less | Ordering::Equal)
}) {
Some(Sort::CaseSensitive)
} else if deps.iter().tuple_windows().all(|(a, b)| {
matches!(
case_insensitive_naive(a, b),
Ordering::Less | Ordering::Equal
)
}) {
Some(Sort::CaseInsensitiveNaive)
} else if deps.iter().tuple_windows().all(|(a, b)| {
matches!(case_sensitive_naive(a, b), Ordering::Less | Ordering::Equal)
}) {
Some(Sort::CaseSensitiveNaive)
} else {
None
}
})
.flatten()
.unwrap_or(Sort::Unsorted);
let sort = if is_sorted(
reqs_lowercase
.iter()
.map(String::as_str)
.map(split_specifiers),
) {
Sort::CaseInsensitive
} else if is_sorted(reqs.iter().copied().map(split_specifiers)) {
Sort::CaseSensitive
} else if is_sorted(reqs_lowercase.iter().map(String::as_str)) {
Sort::CaseInsensitiveNaive
} else if is_sorted(reqs) {
Sort::CaseSensitiveNaive
} else {
Sort::Unsorted
};
let req_string = req.to_string();
let index = match sort {
Sort::CaseInsensitive => deps.iter().position(|d| {
case_insensitive(d, &Value::from(req_string.as_str())) == Ordering::Greater
Sort::CaseInsensitive => deps.iter().position(|dep| {
dep.as_str().is_some_and(|dep| {
split_specifiers(&dep.to_lowercase())
> split_specifiers(&req_string.to_lowercase())
})
}),
Sort::CaseInsensitiveNaive => deps.iter().position(|d| {
case_insensitive_naive(d, &Value::from(req_string.as_str()))
== Ordering::Greater
Sort::CaseInsensitiveNaive => deps.iter().position(|dep| {
dep.as_str()
.is_some_and(|dep| dep.to_lowercase() > req_string.to_lowercase())
}),
Sort::CaseSensitive => deps.iter().position(|d| {
case_sensitive(d, &Value::from(req_string.as_str())) == Ordering::Greater
}),
Sort::CaseSensitiveNaive => deps.iter().position(|d| {
case_sensitive_naive(d, &Value::from(req_string.as_str())) == Ordering::Greater
Sort::CaseSensitive => deps.iter().position(|dep| {
dep.as_str()
.is_some_and(|dep| split_specifiers(dep) > split_specifiers(&req_string))
}),
Sort::CaseSensitiveNaive => deps
.iter()
.position(|dep| dep.as_str().is_some_and(|dep| *dep > *req_string)),
Sort::Unsorted => None,
};
let index = index.unwrap_or(deps.len());
let index = index.unwrap_or_else(|| {
// The dependency should be added to the end, ignoring any
// `include-group` items. This preserves the order for users who
// keep their `include-groups` at the bottom.
deps.iter()
.enumerate()
.filter_map(|(i, dep)| if dep.is_str() { Some(i + 1) } else { None })
.last()
.unwrap_or(deps.len())
});
let mut value = Value::from(req_string.as_str());

195
crates/uv/tests/it/edit.rs
View file

@ -7514,6 +7514,201 @@ fn sorted_dependencies_name_specifiers() -> Result<()> {
Ok(())
}
#[test]
fn sorted_dependencies_with_include_group() -> Result<()> {
let context = TestContext::new("3.12").with_filtered_counts();
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"
[dependency-groups]
dev = [
{ include-group = "coverage" },
"pytest-mock>=3.14",
"pytest>=8.1.1",
]
coverage = [
"coverage>=7.4.4",
]
"#})?;
uv_snapshot!(context.filters(), context.add().args(["--dev", "pytest-randomly"]), @r"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved [N] packages in [TIME]
Prepared [N] packages in [TIME]
Installed [N] packages in [TIME]
+ coverage==7.4.4
+ iniconfig==2.0.0
+ packaging==24.0
+ pluggy==1.4.0
+ pytest==8.1.1
+ pytest-mock==3.14.0
+ 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"
[dependency-groups]
dev = [
{ include-group = "coverage" },
"pytest-mock>=3.14",
"pytest-randomly>=3.15.0",
"pytest>=8.1.1",
]
coverage = [
"coverage>=7.4.4",
]
"#
);
});
Ok(())
}
#[test]
fn sorted_dependencies_new_dependency_after_include_group() -> Result<()> {
let context = TestContext::new("3.12").with_filtered_counts();
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"
[dependency-groups]
dev = [
{ include-group = "coverage" },
]
coverage = [
"coverage>=7.4.4",
]
"#})?;
uv_snapshot!(context.filters(), context.add().args(["--dev", "pytest"]), @r"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved [N] packages in [TIME]
Prepared [N] packages in [TIME]
Installed [N] packages in [TIME]
+ coverage==7.4.4
+ iniconfig==2.0.0
+ packaging==24.0
+ pluggy==1.4.0
+ pytest==8.1.1
");
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"
[dependency-groups]
dev = [
{ include-group = "coverage" },
"pytest>=8.1.1",
]
coverage = [
"coverage>=7.4.4",
]
"#
);
});
Ok(())
}
#[test]
fn sorted_dependencies_include_group_kept_at_bottom() -> Result<()> {
let context = TestContext::new("3.12").with_filtered_counts();
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"
[dependency-groups]
dev = [
"pytest>=8.1.1",
{ include-group = "coverage" },
]
coverage = [
"coverage>=7.4.4",
]
"#})?;
uv_snapshot!(context.filters(), context.add().args(["--dev", "pytest-randomly"]), @r"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved [N] packages in [TIME]
Prepared [N] packages in [TIME]
Installed [N] packages in [TIME]
+ coverage==7.4.4
+ iniconfig==2.0.0
+ packaging==24.0
+ pluggy==1.4.0
+ pytest==8.1.1
+ 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"
[dependency-groups]
dev = [
"pytest>=8.1.1",
"pytest-randomly>=3.15.0",
{ include-group = "coverage" },
]
coverage = [
"coverage>=7.4.4",
]
"#
);
});
Ok(())
}
/// Ensure that the custom ordering of the dependencies is preserved
/// after adding a package.
#[test]