From b6d079f7a25430482a288f81dafbf46aa04a406b Mon Sep 17 00:00:00 2001 From: Ahmed Ilyas Date: Wed, 2 Oct 2024 11:48:37 +0200 Subject: [PATCH 1/3] Preserve case-insensitive sorts in `uv add` --- crates/uv-workspace/src/pyproject_mut.rs | 11 ++++++----- crates/uv/tests/edit.rs | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs index bc8a7cd6b79f7..d5f0a7b94e019 100644 --- a/crates/uv-workspace/src/pyproject_mut.rs +++ b/crates/uv-workspace/src/pyproject_mut.rs @@ -540,15 +540,16 @@ pub fn add_dependency( // 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()); + && deps.iter().tuple_windows().all(|(a, b)| { + a.as_str().map(str::to_lowercase) <= b.as_str().map(str::to_lowercase) + }); let req_string = req.to_string(); let index = if sorted { deps.iter() - .position(|d: &Value| d.as_str() > Some(req_string.as_str())) + .position(|d: &Value| { + d.as_str().map(str::to_lowercase) > Some(req_string.as_str().to_lowercase()) + }) .unwrap_or(deps.len()) } else { deps.len() diff --git a/crates/uv/tests/edit.rs b/crates/uv/tests/edit.rs index f8132ca4d3846..44bdc6a16623c 100644 --- a/crates/uv/tests/edit.rs +++ b/crates/uv/tests/edit.rs @@ -4692,8 +4692,8 @@ fn sorted_dependencies() -> Result<()> { description = "Add your description here" requires-python = ">=3.12" dependencies = [ - "CacheControl[filecache]>=0.14,<0.15", "anyio>=4.3.0", + "CacheControl[filecache]>=0.14,<0.15", "iniconfig", "typing-extensions>=4.10.0", ] From 7b3de53c9c87d406a571b72a1402d6e7dc943319 Mon Sep 17 00:00:00 2001 From: Ahmed Ilyas Date: Thu, 3 Oct 2024 18:56:49 +0200 Subject: [PATCH 2/3] Respect case sensitive sorting in `uv add` --- crates/uv-workspace/src/pyproject_mut.rs | 20 +++++-- crates/uv/tests/edit.rs | 69 ++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs index d5f0a7b94e019..c19b34efb29fa 100644 --- a/crates/uv-workspace/src/pyproject_mut.rs +++ b/crates/uv-workspace/src/pyproject_mut.rs @@ -539,16 +539,30 @@ pub fn add_dependency( // 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) + // We account for both case-sensitive and case-insensitive sorting. + let all_strings = deps.iter().all(toml_edit::Value::is_str); + + let case_insensitive_sorted = all_strings && deps.iter().tuple_windows().all(|(a, b)| { a.as_str().map(str::to_lowercase) <= b.as_str().map(str::to_lowercase) }); + let case_sensitive_sorted = all_strings + && deps + .iter() + .tuple_windows() + .all(|(a, b)| a.as_str() <= b.as_str()); + let req_string = req.to_string(); - let index = if sorted { + let index = if case_sensitive_sorted || case_insensitive_sorted { deps.iter() .position(|d: &Value| { - d.as_str().map(str::to_lowercase) > Some(req_string.as_str().to_lowercase()) + if case_insensitive_sorted { + d.as_str().map(str::to_lowercase) + > Some(req_string.as_str().to_lowercase()) + } else { + d.as_str() > Some(req_string.as_str()) + } }) .unwrap_or(deps.len()) } else { diff --git a/crates/uv/tests/edit.rs b/crates/uv/tests/edit.rs index 44bdc6a16623c..519b06c0ba7ca 100644 --- a/crates/uv/tests/edit.rs +++ b/crates/uv/tests/edit.rs @@ -4703,6 +4703,75 @@ fn sorted_dependencies() -> Result<()> { Ok(()) } +/// Ensure that the added dependencies are case sensitive sorted if the dependency list was already +/// case sensitive sorted prior to the operation. +#[test] +fn case_sensitive_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", + "PyYAML", + "iniconfig", + ] + "#})?; + + uv_snapshot!(context.filters(), context.add().args(["typing-extensions", "anyio"]), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 14 packages in [TIME] + Prepared 13 packages in [TIME] + Installed 13 packages in [TIME] + + anyio==4.3.0 + + cachecontrol==0.14.0 + + certifi==2024.2.2 + + charset-normalizer==3.3.2 + + filelock==3.13.1 + + idna==3.6 + + iniconfig==2.0.0 + + msgpack==1.0.8 + + pyyaml==6.0.1 + + requests==2.31.0 + + sniffio==1.3.1 + + typing-extensions==4.10.0 + + urllib3==2.2.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" + description = "Add your description here" + requires-python = ">=3.12" + dependencies = [ + "CacheControl[filecache]>=0.14,<0.15", + "PyYAML", + "anyio>=4.3.0", + "iniconfig", + "typing-extensions>=4.10.0", + ] + "### + ); + }); + Ok(()) +} + /// Ensure that the custom ordering of the dependencies is preserved /// after adding a package. #[test] From 3ed9652a5dc10ffb0ea74fc5e8cfc27b2442505c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 4 Oct 2024 11:51:16 +0100 Subject: [PATCH 3/3] Use enum --- crates/uv-workspace/src/pyproject_mut.rs | 66 +++++++++++++++--------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs index c19b34efb29fa..75de5e001a4a3 100644 --- a/crates/uv-workspace/src/pyproject_mut.rs +++ b/crates/uv-workspace/src/pyproject_mut.rs @@ -533,41 +533,57 @@ pub fn add_dependency( match to_replace.as_slice() { [] => { + #[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 unsorted. + Unsorted, + } + // 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. + // // We account for both case-sensitive and case-insensitive sorting. - let all_strings = deps.iter().all(toml_edit::Value::is_str); - - let case_insensitive_sorted = all_strings - && deps.iter().tuple_windows().all(|(a, b)| { - a.as_str().map(str::to_lowercase) <= b.as_str().map(str::to_lowercase) - }); - - let case_sensitive_sorted = all_strings - && deps - .iter() - .tuple_windows() - .all(|(a, b)| a.as_str() <= b.as_str()); + let sort = deps + .iter() + .all(Value::is_str) + .then(|| { + if deps.iter().tuple_windows().all(|(a, b)| { + a.as_str().map(str::to_lowercase) <= b.as_str().map(str::to_lowercase) + }) { + Some(Sort::CaseInsensitive) + } else if deps + .iter() + .tuple_windows() + .all(|(a, b)| a.as_str() <= b.as_str()) + { + Some(Sort::CaseSensitive) + } else { + None + } + }) + .flatten() + .unwrap_or(Sort::Unsorted); let req_string = req.to_string(); - let index = if case_sensitive_sorted || case_insensitive_sorted { - deps.iter() - .position(|d: &Value| { - if case_insensitive_sorted { - d.as_str().map(str::to_lowercase) - > Some(req_string.as_str().to_lowercase()) - } else { - d.as_str() > Some(req_string.as_str()) - } - }) - .unwrap_or(deps.len()) - } else { - deps.len() + 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()) + }), + Sort::Unsorted => None, }; + let index = index.unwrap_or(deps.len()); deps.insert(index, req_string); // `reformat_array_multiline` uses the indentation of the first dependency entry.