From 705b3da9133e9ca36aba4eb41532f426d6b2988f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 21 Dec 2024 09:43:43 -0500 Subject: [PATCH] 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. --- crates/uv-workspace/src/pyproject_mut.rs | 45 +++++++++----- crates/uv/tests/it/edit.rs | 75 +++++++++++++++++------- 2 files changed, 85 insertions(+), 35 deletions(-) diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs index 35c66550f..25544cc05 100644 --- a/crates/uv-workspace/src/pyproject_mut.rs +++ b/crates/uv-workspace/src/pyproject_mut.rs @@ -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, }; diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index d0747e01c..5b7f90bb6 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -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(()) }