diff --git a/crates/pixi_core/src/workspace/workspace_mut.rs b/crates/pixi_core/src/workspace/workspace_mut.rs index d9db4e28d6..8d0a7570ee 100644 --- a/crates/pixi_core/src/workspace/workspace_mut.rs +++ b/crates/pixi_core/src/workspace/workspace_mut.rs @@ -288,7 +288,7 @@ impl WorkspaceMut { feature_name, Some(editable), DependencyOverwriteBehavior::Overwrite, - location.as_ref(), + location, )?; if added { if spec.version_or_url.is_none() { @@ -623,7 +623,7 @@ impl WorkspaceMut { feature_name, Some(editable), DependencyOverwriteBehavior::Overwrite, - location.as_ref(), + location, )?; } } diff --git a/crates/pixi_manifest/src/lib.rs b/crates/pixi_manifest/src/lib.rs index 6521fd183a..20f405cff5 100644 --- a/crates/pixi_manifest/src/lib.rs +++ b/crates/pixi_manifest/src/lib.rs @@ -89,6 +89,7 @@ pub enum DependencyOverwriteBehavior { Error, } +#[derive(Copy, Clone)] pub enum PypiDependencyLocation { /// [pypi-dependencies] in pixi.toml or [tool.pixi.pypi-dependencies] in /// pyproject.toml diff --git a/crates/pixi_manifest/src/manifests/document.rs b/crates/pixi_manifest/src/manifests/document.rs index 0416a7f576..5b7738b848 100644 --- a/crates/pixi_manifest/src/manifests/document.rs +++ b/crates/pixi_manifest/src/manifests/document.rs @@ -1,10 +1,5 @@ use std::{fmt, str::FromStr, sync::Arc}; -use crate::{ - FeatureName, LibCSystemRequirement, ManifestKind, ManifestProvenance, PypiDependencyLocation, - SpecType, SystemRequirements, Task, TomlError, manifests::table_name::TableName, - toml::TomlDocument, utils::WithSourceCode, -}; use miette::{Diagnostic, NamedSource}; use pixi_consts::consts; use pixi_pypi_spec::{PixiPypiSpec, PypiPackageName}; @@ -13,6 +8,12 @@ use rattler_conda_types::{PackageName, Platform}; use thiserror::Error; use toml_edit::{Array, DocumentMut, Item, Table, Value, value}; +use crate::{ + FeatureName, LibCSystemRequirement, ManifestKind, ManifestProvenance, PypiDependencyLocation, + SpecType, SystemRequirements, Task, TomlError, manifests::table_name::TableName, + toml::TomlDocument, utils::WithSourceCode, +}; + /// Discriminates between a 'pixi.toml' and a 'pyproject.toml' manifest. #[derive(Debug, Clone)] pub enum ManifestDocument { @@ -245,39 +246,14 @@ impl ManifestDocument { ) -> Result<(), TomlError> { // For 'pyproject.toml' manifest, try and remove the dependency from native // arrays - let remove_requirement = - |source: &mut ManifestDocument, table, array_name| -> Result<(), TomlError> { - let array = source - .manifest_mut() - .get_mut_toml_array(table, array_name)?; - if let Some(array) = array { - array.retain(|x| { - let req: pep508_rs::Requirement = x - .as_str() - .unwrap_or("") - .parse() - .expect("should be a valid pep508 dependency"); - let name = PypiPackageName::from_normalized(req.name); - name != *dep - }); - if array.is_empty() { - source - .manifest_mut() - .get_or_insert_nested_table(table)? - .remove(array_name); - } - } - Ok(()) - }; - match self { ManifestDocument::PyProjectToml(_) if feature_name.is_default() => { - remove_requirement(self, "project", "dependencies")?; + self.remove_pypi_requirement("project", "dependencies", dep)?; } ManifestDocument::PyProjectToml(_) => { let name = feature_name.to_string(); - remove_requirement(self, "project.optional-dependencies", &name)?; - remove_requirement(self, "dependency-groups", &name)?; + self.remove_pypi_requirement("project.optional-dependencies", &name, dep)?; + self.remove_pypi_requirement("dependency-groups", &name, dep)?; } _ => (), }; @@ -296,6 +272,33 @@ impl ManifestDocument { Ok(()) } + /// Removes a pypi requirement from a particular array. + fn remove_pypi_requirement( + &mut self, + table: &str, + array_name: &str, + dependency_name: &PypiPackageName, + ) -> Result<(), TomlError> { + let array = self.manifest_mut().get_mut_toml_array(table, array_name)?; + if let Some(array) = array { + array.retain(|x| { + let req: pep508_rs::Requirement = x + .as_str() + .unwrap_or("") + .parse() + .expect("should be a valid pep508 dependency"); + let name = PypiPackageName::from_normalized(req.name); + name != *dependency_name + }); + if array.is_empty() { + self.manifest_mut() + .get_or_insert_nested_table(table)? + .remove(array_name); + } + } + Ok(()) + } + /// Removes a conda or pypi dependency from the TOML manifest's pixi table /// for either a 'pyproject.toml' and 'pixi.toml' /// @@ -338,7 +341,30 @@ impl ManifestDocument { self.manifest_mut() .get_or_insert_nested_table(dependency_table.to_string().as_str()) - .map(|t| t.insert(name.as_normalized(), Item::Value(spec.to_toml_value())))?; + .map(|t| { + let mut new_value = spec.to_toml_value(); + + // Check if there is an existing entry that is represented by an inline value. + let existing_value = t.iter_mut().find_map(|(key, value)| { + let package_key_name = PackageName::from_str(key.get()).ok()?; + if package_key_name == *name { + value.as_value_mut() + } else { + None + } + }); + + // If there exists an existing value, we update it with the new value, but we + // keep the decoration. + if let Some(existing_value) = existing_value { + *new_value.decor_mut() = existing_value.decor().clone(); + *existing_value = new_value; + } else { + // Otherwise, just reinsert the value. This might overwrite an existing + // decorations. + t.insert(name.as_normalized(), Item::Value(new_value)); + } + })?; Ok(()) } @@ -354,19 +380,8 @@ impl ManifestDocument { platform: Option, feature_name: &FeatureName, editable: Option, - location: Option<&PypiDependencyLocation>, + location: Option, ) -> Result<(), TomlError> { - // Pypi dependencies can be stored in different places in pyproject.toml - // manifests so we remove any potential dependency of the same name - // before adding it back - if matches!(self, ManifestDocument::PyProjectToml(_)) { - self.remove_pypi_dependency( - &PypiPackageName::from_normalized(requirement.name.clone()), - platform, - feature_name, - )?; - } - // The '[pypi-dependencies]' or '[tool.pixi.pypi-dependencies]' table is // selected // - For 'pixi.toml' manifests where it is the only choice @@ -385,18 +400,44 @@ impl ManifestDocument { pypi_requirement.set_editable(editable); } - let dependency_table = TableName::new() + let dependency_table_name = TableName::new() .with_prefix(self.table_prefix()) .with_platform(platform.as_ref()) .with_feature_name(Some(feature_name)) .with_table(Some(consts::PYPI_DEPENDENCIES)); - self.manifest_mut() - .get_or_insert_nested_table(dependency_table.to_string().as_str())? - .insert( - requirement.name.as_ref(), - Item::Value(pypi_requirement.into()), - ); + let table = self + .manifest_mut() + .get_or_insert_nested_table(dependency_table_name.to_string().as_str())?; + + let mut new_value = Value::from(pypi_requirement); + + // Check if there exists an existing entry in the table that we should overwrite + // instead. + let existing_value = table.iter_mut().find_map(|(key, value)| { + let existing_name = pep508_rs::PackageName::from_str(key.get()).ok()?; + if existing_name == requirement.name { + value.as_value_mut() + } else { + None + } + }); + + // If there exists an existing entry, we overwrite it but keep the decoration. + if let Some(existing_value) = existing_value { + *new_value.decor_mut() = existing_value.decor().clone(); + *existing_value = new_value; + } else { + table.insert(requirement.name.as_ref(), Item::Value(new_value)); + } + + // Remove the entry from the project native array. + self.remove_pypi_requirement( + "project", + "dependencies", + &PypiPackageName::from_normalized(requirement.name.clone()), + )?; + return Ok(()); } @@ -406,10 +447,26 @@ impl ManifestDocument { // - optional-dependencies is explicitly requested as location let add_requirement = |source: &mut ManifestDocument, table, array| -> Result<(), TomlError> { - source + let array = source .manifest_mut() - .get_or_insert_toml_array_mut(table, array)? - .push(requirement.to_string()); + .get_or_insert_toml_array_mut(table, array)?; + + // Check if there is an existing entry that we should replace. Replacing will + // preserve any existing formatting. + let existing_entry_idx = array.iter().position(|item| { + let Ok(req): Result = + item.as_str().unwrap_or_default().parse() + else { + return false; + }; + req.name == requirement.name + }); + + if let Some(idx) = existing_entry_idx { + array.replace(idx, requirement.to_string()); + } else { + array.push(requirement.to_string()); + } Ok(()) }; if feature_name.is_default() @@ -749,3 +806,125 @@ impl ManifestDocument { Ok(()) } } + +#[cfg(test)] +mod test { + use super::*; + + /// This test checks that when calling `add_dependency` with a dependency + /// that already exists in the document the formatting and comments are + /// preserved. + /// + /// This test also verifies that the name of the dependency is correctly + /// found and kept in the situation where the user has a non-normalized name + /// in the document (e.g. "hTTPx" vs 'HtTpX'). + #[test] + pub fn add_dependency_retains_decoration() { + let manifest_content = r#"[project] +name = "test" + +[tool.pixi.project] +channels = [] +platforms = [] + +[tool.pixi.dependencies] +# Hello world! +hTTPx = ">=0.28.1,<0.29" # Some comment. + +# newline +"#; + + let mut document = ManifestDocument::PyProjectToml(TomlDocument::new( + DocumentMut::from_str(manifest_content).unwrap(), + )); + + // Overwrite the existing dependency. + document + .add_dependency( + &PackageName::from_str("HtTpX").unwrap(), + &PixiSpec::Version("0.1.*".parse().unwrap()), + SpecType::Run, + None, + &FeatureName::default(), + ) + .unwrap(); + + insta::assert_snapshot!(document.to_string()); + } + + /// This test checks that when calling `add_pypi_dependency` with + /// dependencies that already exist in different locations the + /// formatting and comments are preserved across all PyPI dependency + /// storage formats. + /// + /// This test also verifies that the name of the dependency is correctly + /// found and kept in the situation where the user has a non-normalized name + /// in the document (e.g. "rEquEsTs" vs 'ReQuEsTs'). + #[test] + pub fn add_pypi_dependency_retains_decoration() { + let manifest_content = r#"[project] +name = "test" +dependencies = [ + # Main dependency comment + "rEquEsTs>=2.28.1,<3.0", # inline comment +] + +[project.optional-dependencies] +dev = [ + # Dev dependency comment + "PyYaML>=6.0", # dev inline comment +] + +[tool.pixi.project] +channels = [] +platforms = [] + +[tool.pixi.pypi-dependencies] +# Table dependency comment +NumPy = ">=1.20.0" # table inline comment +"#; + + let mut document = ManifestDocument::PyProjectToml(TomlDocument::new( + DocumentMut::from_str(manifest_content).unwrap(), + )); + + // Update dependencies in different locations + let requests_req = pep508_rs::Requirement::from_str("ReQuEsTs>=2.30.0").unwrap(); + document + .add_pypi_dependency( + &requests_req, + None, + None, + &FeatureName::default(), + None, + Some(PypiDependencyLocation::Dependencies), + ) + .unwrap(); + + let pyyaml_req = pep508_rs::Requirement::from_str("PyYAML>=6.1.0").unwrap(); + document + .add_pypi_dependency( + &pyyaml_req, + None, + None, + &FeatureName::from_str("dev").unwrap(), + None, + Some(PypiDependencyLocation::OptionalDependencies), + ) + .unwrap(); + + let numpy_req = pep508_rs::Requirement::from_str("numpy>=1.21.0").unwrap(); + document + .add_pypi_dependency( + &numpy_req, + None, + None, + &FeatureName::default(), + None, + Some(crate::PypiDependencyLocation::PixiPypiDependencies), + ) + .unwrap(); + + insta::assert_snapshot!(document.to_string()); + } +} diff --git a/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_retains_decoration.snap b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_retains_decoration.snap new file mode 100644 index 0000000000..1d12874085 --- /dev/null +++ b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_dependency_retains_decoration.snap @@ -0,0 +1,16 @@ +--- +source: crates/pixi_manifest/src/manifests/document.rs +expression: document.to_string() +--- +[project] +name = "test" + +[tool.pixi.project] +channels = [] +platforms = [] + +[tool.pixi.dependencies] +# Hello world! +hTTPx = "0.1.*" # Some comment. + +# newline diff --git a/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_retains_decoration.snap b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_retains_decoration.snap new file mode 100644 index 0000000000..412713ba01 --- /dev/null +++ b/crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__document__test__add_pypi_dependency_retains_decoration.snap @@ -0,0 +1,24 @@ +--- +source: crates/pixi_manifest/src/manifests/document.rs +expression: document.to_string() +--- +[project] +name = "test" +dependencies = [ + # Main dependency comment + "requests>=2.30.0", # inline comment +] + +[project.optional-dependencies] +dev = [ + # Dev dependency comment + "pyyaml>=6.1.0", # dev inline comment +] + +[tool.pixi.project] +channels = [] +platforms = [] + +[tool.pixi.pypi-dependencies] +# Table dependency comment +NumPy = ">=1.21.0" # table inline comment diff --git a/crates/pixi_manifest/src/manifests/workspace.rs b/crates/pixi_manifest/src/manifests/workspace.rs index d9de5005bd..ab2da2bf7d 100644 --- a/crates/pixi_manifest/src/manifests/workspace.rs +++ b/crates/pixi_manifest/src/manifests/workspace.rs @@ -507,7 +507,7 @@ impl WorkspaceManifestMut<'_> { feature_name: &FeatureName, editable: Option, overwrite_behavior: DependencyOverwriteBehavior, - location: Option<&PypiDependencyLocation>, + location: Option, ) -> miette::Result { let mut any_added = false; for platform in to_options(platforms) { diff --git a/tests/integration_rust/snapshots/integration_rust__upgrade_tests__pypi_dependency_index_preserved_on_upgrade.snap b/tests/integration_rust/snapshots/integration_rust__upgrade_tests__pypi_dependency_index_preserved_on_upgrade.snap index 3b29b31d41..86efdd01ff 100644 --- a/tests/integration_rust/snapshots/integration_rust__upgrade_tests__pypi_dependency_index_preserved_on_upgrade.snap +++ b/tests/integration_rust/snapshots/integration_rust__upgrade_tests__pypi_dependency_index_preserved_on_upgrade.snap @@ -8,7 +8,7 @@ expression: redacted_content exclude-newer = "2025-05-19" [pypi-dependencies] -click = { version = ">=8.2.0, <9", index = "https://pypi.tuna.tsinghua.edu.cn/simple" } + click = { version = ">=8.2.0, <9", index = "https://pypi.tuna.tsinghua.edu.cn/simple" } [dependencies] -python = ">=3.13.3,<3.14" + python = ">=3.13.3,<3.14"