Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sort dependencies in pyproject.toml #6388

Merged
merged 19 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/uv-workspace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ toml = { workspace = true }
toml_edit = { workspace = true }
tracing = { workspace = true }
url = { workspace = true }
itertools = { workspace = true }

[dev-dependencies]
insta = { version = "1.39.0", features = ["filters", "json", "redactions"] }
Expand Down
47 changes: 44 additions & 3 deletions crates/uv-workspace/src/pyproject_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::path::Path;
use std::str::FromStr;
use std::{fmt, mem};

use itertools::Itertools;
use pep440_rs::{Version, VersionSpecifier, VersionSpecifiers};
use pep508_rs::{ExtraName, MarkerTree, PackageName, Requirement, VersionOrUrl};
use thiserror::Error;
Expand Down Expand Up @@ -503,13 +504,53 @@ pub fn add_dependency(
deps: &mut Array,
has_source: bool,
) -> Result<ArrayEdit, Error> {
// Find matching dependencies.
let mut to_replace = find_dependencies(&req.name, Some(&req.marker), deps);

match to_replace.as_slice() {
[] => {
deps.push(req.to_string());
// 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.
let sorted = deps.iter().all(toml_edit::Value::is_str)
&& deps
.iter()
.tuple_windows()
.all(|(a, b)| a.as_str() <= b.as_str());

let req_string = req.to_string();
let index = if sorted {
deps.iter()
.position(|d: &Value| d.as_str() > Some(req_string.as_str()))
.unwrap_or(deps.len())
} else {
deps.len()
};

deps.insert(index, req_string);
// `reformat_array_multiline` uses the indentation of the first dependency entry.
// Therefore, we retrieve the indentation of the first dependency entry and apply it to
// the new entry. Note that it is only necessary if the newly added dependency is going
// to be the first in the list _and_ the dependency list was not empty prior to adding
// the new dependency.
if deps.len() > 1 && index == 0 {
let prefix = deps
.clone()
.get(index + 1)
.unwrap()
.decor()
.prefix()
.unwrap()
.clone();

deps.get_mut(index).unwrap().decor_mut().set_prefix(prefix);
}

reformat_array_multiline(deps);
Ok(ArrayEdit::Add(deps.len() - 1))

Ok(ArrayEdit::Add(index))
}
[_] => {
let (i, mut old_req) = to_replace.remove(0);
Expand Down
134 changes: 123 additions & 11 deletions crates/uv/tests/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2077,8 +2077,8 @@ fn add_update_marker() -> Result<()> {
version = "0.1.0"
requires-python = ">=3.8"
dependencies = [
"requests>=2.30; python_version >= '3.11'",
"requests>=2.0,<2.29 ; python_full_version < '3.11'",
"requests>=2.30; python_version >= '3.11'",
]
"###
);
Expand Down Expand Up @@ -2111,8 +2111,8 @@ fn add_update_marker() -> Result<()> {
version = "0.1.0"
requires-python = ">=3.8"
dependencies = [
"requests>=2.30; python_version >= '3.11'",
"requests>=2.0,<2.20 ; python_full_version < '3.11'",
"requests>=2.30; python_version >= '3.11'",
]
"###
);
Expand Down Expand Up @@ -2149,8 +2149,8 @@ fn add_update_marker() -> Result<()> {
version = "0.1.0"
requires-python = ">=3.8"
dependencies = [
"requests>=2.30; python_version >= '3.11'",
"requests>=2.0,<2.20 ; python_full_version < '3.11'",
"requests>=2.30; python_version >= '3.11'",
"requests>=2.31 ; python_full_version >= '3.12' and sys_platform == 'win32'",
]
"###
Expand Down Expand Up @@ -2185,10 +2185,10 @@ fn add_update_marker() -> Result<()> {
version = "0.1.0"
requires-python = ">=3.8"
dependencies = [
"requests>=2.30; python_version >= '3.11'",
"requests>=2.0,<2.20 ; python_full_version < '3.11'",
"requests>=2.31 ; python_full_version >= '3.12' and sys_platform == 'win32'",
"requests>=2.10 ; sys_platform == 'win32'",
"requests>=2.30; python_version >= '3.11'",
"requests>=2.31 ; python_full_version >= '3.12' and sys_platform == 'win32'",
]
"###
);
Expand Down Expand Up @@ -3484,8 +3484,8 @@ fn add_requirements_file() -> Result<()> {
# ...
requires-python = ">=3.12"
dependencies = [
"flask==2.3.2",
"anyio",
"flask==2.3.2",
]

[tool.uv.sources]
Expand Down Expand Up @@ -3564,9 +3564,9 @@ fn add_script() -> Result<()> {
# /// script
# requires-python = ">=3.11"
# dependencies = [
# "anyio",
# "requests<3",
# "rich",
# "anyio",
# ]
# ///

Expand Down Expand Up @@ -3616,8 +3616,8 @@ fn add_script_without_metadata_table() -> Result<()> {
# /// script
# requires-python = ">=3.12"
# dependencies = [
# "rich",
# "requests<3",
# "rich",
# ]
# ///
import requests
Expand Down Expand Up @@ -3668,8 +3668,8 @@ fn add_script_without_metadata_table_with_shebang() -> Result<()> {
# /// script
# requires-python = ">=3.12"
# dependencies = [
# "rich",
# "requests<3",
# "rich",
# ]
# ///
import requests
Expand Down Expand Up @@ -3724,8 +3724,8 @@ fn add_script_with_metadata_table_and_shebang() -> Result<()> {
# /// script
# requires-python = ">=3.12"
# dependencies = [
# "rich",
# "requests<3",
# "rich",
# ]
# ///
import requests
Expand Down Expand Up @@ -3775,8 +3775,8 @@ fn add_script_without_metadata_table_with_docstring() -> Result<()> {
# /// script
# requires-python = ">=3.12"
# dependencies = [
# "rich",
# "requests<3",
# "rich",
# ]
# ///
"""This is a script."""
Expand Down Expand Up @@ -4038,3 +4038,115 @@ fn fail_to_add_revert_project() -> Result<()> {

Ok(())
}

/// Ensure that the added dependencies are sorted
/// if the dependency list was already sorted prior to adding the new one.
#[test]
fn sorted_dependencies() -> Result<()> {
let context = TestContext::new("3.12");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(indoc! {r#"
[project]
name = "project"
version = "0.1.0"
description = "Add your description here"
requires-python = ">=3.12"
dependencies = [
"CacheControl[filecache]>=0.14,<0.15",
"mwparserfromhell",
"pywikibot",
"sentry-sdk",
"yarl",
]
"#})?;

uv_snapshot!(context.filters(), context.add(&["pydantic"]).arg("--frozen"), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
"###);

let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?;

insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
pyproject_toml, @r###"
[project]
name = "project"
version = "0.1.0"
description = "Add your description here"
requires-python = ">=3.12"
dependencies = [
"CacheControl[filecache]>=0.14,<0.15",
"mwparserfromhell",
"pydantic",
"pywikibot",
"sentry-sdk",
"yarl",
]
"###
);
});
Ok(())
}

/// Ensure that the custom ordering of the dependencies is preserved
/// after adding a package.
#[test]
fn custom_dependencies() -> Result<()> {
let context = TestContext::new("3.12");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(indoc! {r#"
[project]
name = "project"
version = "0.1.0"
description = "Add your description here"
requires-python = ">=3.12"
dependencies = [
"yarl",
"CacheControl[filecache]>=0.14,<0.15",
"mwparserfromhell",
"pywikibot",
"sentry-sdk",
]
"#})?;

uv_snapshot!(context.filters(), context.add(&["pydantic"]).arg("--frozen"), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
"###);

let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?;

insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
pyproject_toml, @r###"
[project]
name = "project"
version = "0.1.0"
description = "Add your description here"
requires-python = ">=3.12"
dependencies = [
"yarl",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wanted to test the case where there's an empty line between dependencies (and it being preserved after uv add) but uv add seems to remove it currently - should probably be done as a separate PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6727? not sure if it's worth it, though, since it's strictly just a formatting nit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah #6727 sounds like a good candidate for a separate PR

"CacheControl[filecache]>=0.14,<0.15",
"mwparserfromhell",
"pywikibot",
"sentry-sdk",
"pydantic",
]
"###
);
});
Ok(())
}