From 4fff2554a4f0db1d45f0d09f23c61bcb110f2214 Mon Sep 17 00:00:00 2001 From: Marcelo Trevisani Date: Tue, 2 Jan 2024 09:15:35 +0000 Subject: [PATCH 1/9] Add option remove --pypi to remove dependencies on pypi-dependencies section in pixi.toml --- src/cli/remove.rs | 55 ++++++++++++++++++++++++++++++------- src/consts.rs | 1 + src/project/manifest.rs | 61 +++++++++++++++++++++++++++++++++++++++++ src/project/python.rs | 26 ++++++++++++++++++ 4 files changed, 133 insertions(+), 10 deletions(-) diff --git a/src/cli/remove.rs b/src/cli/remove.rs index 6da6eadc9..8993e286c 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -1,9 +1,10 @@ use std::path::PathBuf; use clap::Parser; -use rattler_conda_types::{PackageName, Platform}; +use rattler_conda_types::{NamelessMatchSpec, PackageName, Platform}; use crate::environment::LockFileUsage; +use crate::project::python::PyPiRequirement; use crate::{environment::get_up_to_date_prefix, project::SpecType, Project}; /// Remove the dependency from the project @@ -25,11 +26,20 @@ pub struct Args { #[arg(long, conflicts_with = "host")] pub build: bool, + /// Whether the dependency is a pypi package + #[arg(long)] + pub pypi: bool, + /// The platform for which the dependency should be removed #[arg(long, short)] pub platform: Option, } +enum DependencyRemovalResult { + PixiDeps(miette::Result<(String, NamelessMatchSpec)>), + PyPiDeps(miette::Result<(rip::types::PackageName, PyPiRequirement)>), +} + pub async fn execute(args: Args) -> miette::Result<()> { let mut project = Project::load_or_else_discover(args.manifest_path.as_deref())?; let deps = args.deps; @@ -44,22 +54,43 @@ pub async fn execute(args: Args) -> miette::Result<()> { let results = deps .iter() .map(|dep| { - if let Some(p) = &args.platform { - project - .manifest - .remove_target_dependency(dep, &spec_type, p) + if args.pypi { + DependencyRemovalResult::PyPiDeps(project.manifest.remove_pypi_dependency(dep)) } else { - project.manifest.remove_dependency(dep, &spec_type) + DependencyRemovalResult::PixiDeps(if let Some(p) = &args.platform { + project + .manifest + .remove_target_dependency(dep, &spec_type, p) + } else { + project.manifest.remove_dependency(dep, &spec_type) + }) } }) - .collect::>(); + .collect::>(); project.save()?; // updating prefix after removing from toml let _ = get_up_to_date_prefix(&project, LockFileUsage::Update, false, None).await?; - for (removed, spec) in results.iter().flatten() { + for result in results.iter() { + let removed = match result { + DependencyRemovalResult::PixiDeps(pixi_result) => { + pixi_result.as_ref().unwrap().0.to_string() + } + DependencyRemovalResult::PyPiDeps(pypi_result) => { + pypi_result.as_ref().unwrap().0.as_str().to_string() + } + }; + let spec = match result { + DependencyRemovalResult::PixiDeps(pixi_result) => { + pixi_result.as_ref().unwrap().1.to_string() + } + DependencyRemovalResult::PyPiDeps(pypi_result) => { + pypi_result.as_ref().unwrap().1.to_string() + } + }; + let table_name = if let Some(p) = &args.platform { format!("target.{}.{}", p.as_str(), spec_type.name()) } else { @@ -74,8 +105,12 @@ pub async fn execute(args: Args) -> miette::Result<()> { } for result in &results { - if let Err(e) = result { - eprintln!("{e}"); + match result { + DependencyRemovalResult::PixiDeps(Err(e)) + | DependencyRemovalResult::PyPiDeps(Err(e)) => { + eprintln!("{e}"); + } + _ => {} } } diff --git a/src/consts.rs b/src/consts.rs index 7696a5532..263e1ce5e 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -3,3 +3,4 @@ pub const PROJECT_LOCK_FILE: &str = "pixi.lock"; pub const PIXI_DIR: &str = ".pixi"; pub const PREFIX_FILE_NAME: &str = "prefix"; pub const ENVIRONMENT_DIR: &str = "env"; +pub const PYPI_DEPENDENCIES: &str = "pypi-dependencies"; diff --git a/src/project/manifest.rs b/src/project/manifest.rs index 8aa992d25..1269990c6 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -468,6 +468,22 @@ impl Manifest { Ok(()) } + pub fn remove_pypi_dependency( + &mut self, + dep: &rattler_conda_types::PackageName, + ) -> miette::Result<(PackageName, PyPiRequirement)> { + if let Item::Table(ref mut t) = self.document[consts::PYPI_DEPENDENCIES] { + if t.contains_key(dep.as_normalized()) && t.remove(dep.as_normalized()).is_some() { + return self.parsed.remove_pypi_dependencies(dep.as_normalized()); + } + } + + Err(miette::miette!( + "Couldn't find {} in PyPi", + console::style(dep.as_normalized()).bold(), + )) + } + /// Removes a dependency from `pixi.toml` based on `SpecType`. pub fn remove_dependency( &mut self, @@ -802,6 +818,30 @@ impl ProjectManifest { } } + /// Remove PyPi dependencies + pub fn remove_pypi_dependencies( + &mut self, + dep: &str, + ) -> miette::Result<(PackageName, PyPiRequirement)> { + self.pypi_dependencies + .as_mut() + .and_then(|pypi_deps| { + let key_to_remove = pypi_deps + .keys() + .find(|&pkg_name| pkg_name.as_str() == dep) + .cloned(); + + // Step 2: Use the key to remove the entry + key_to_remove.and_then(|pkg_key| pypi_deps.shift_remove_entry(&pkg_key)) + }) + .ok_or_else(|| { + miette::miette!( + "Couldn't remove PyPi dependency: {}", + console::style(dep).bold(), + ) + }) + } + /// Remove dependency given a `SpecType`. pub fn remove_dependency( &mut self, @@ -1444,6 +1484,27 @@ mod test { assert_debug_snapshot!(manifest.parsed.target); } + #[test] + fn test_remove_pypi_dependencies() { + let pixi_cfg = r#"[project] +name = "pixi_fun" +version = "0.1.0" +channels = [] +platforms = ["linux-64", "win-64"] + +[dependencies] +python = ">=3.12.1,<3.13" + +[pypi-dependencies] +jax = { version = "*", extras = ["cpu"] } +"#; + let mut manifest = Manifest::from_str(Path::new(""), pixi_cfg).unwrap(); + manifest + .remove_pypi_dependency(&rattler_conda_types::PackageName::try_from("jax").unwrap()) + .unwrap(); + assert!(manifest.parsed.pypi_dependencies.unwrap().is_empty()) + } + #[test] fn test_set_version() { // Using known files in the project so the test succeed including the file check. diff --git a/src/project/python.rs b/src/project/python.rs index 80ce30b15..6d5676214 100644 --- a/src/project/python.rs +++ b/src/project/python.rs @@ -1,6 +1,7 @@ use pep440_rs::VersionSpecifiers; use serde::de::{Error, MapAccess, Visitor}; use serde::{de, Deserialize, Deserializer}; +use std::fmt; use std::fmt::Formatter; use std::str::FromStr; use thiserror::Error; @@ -25,6 +26,22 @@ pub enum ParsePyPiRequirementError { MissingOperator(String), } +impl fmt::Display for PyPiRequirement { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let all_extras = self + .extras + .clone() + .map(|each_str| each_str.join(", ")) + .unwrap_or_default(); + write!( + f, + "version=\"{}\", extras=\"{}\"", + self.version.clone().unwrap(), + all_extras + ) + } +} + impl From for Item { /// PyPiRequirement to a toml_edit item, to put in the manifest file. fn from(val: PyPiRequirement) -> Item { @@ -179,6 +196,15 @@ mod test { use super::*; use indexmap::IndexMap; + #[test] + fn test_pypi_to_string() { + let pypi = PyPiRequirement::from_str(">=1.0.0,<2.0.0"); + assert_eq!( + pypi.unwrap().to_string(), + "version=\">=1.0.0, <2.0.0\", extras=\"\"" + ); + } + #[test] fn test_only_version() { let requirement: IndexMap = From d0d9aef99b4766185674baa3dbe838c66e07b457 Mon Sep 17 00:00:00 2001 From: Marcelo Trevisani Date: Tue, 2 Jan 2024 23:56:43 +0000 Subject: [PATCH 2/9] Improvements from review --- src/cli/remove.rs | 46 ++++++++++++++++------------------------- src/project/manifest.rs | 33 +++++++++++++++++++++-------- src/project/mod.rs | 2 +- src/project/python.rs | 21 +++++++------------ 4 files changed, 50 insertions(+), 52 deletions(-) diff --git a/src/cli/remove.rs b/src/cli/remove.rs index 8993e286c..1afa7d6c1 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -1,4 +1,5 @@ use std::path::PathBuf; +use std::str::FromStr; use clap::Parser; use rattler_conda_types::{NamelessMatchSpec, PackageName, Platform}; @@ -36,8 +37,8 @@ pub struct Args { } enum DependencyRemovalResult { - PixiDeps(miette::Result<(String, NamelessMatchSpec)>), - PyPiDeps(miette::Result<(rip::types::PackageName, PyPiRequirement)>), + Conda(miette::Result<(String, NamelessMatchSpec)>), + PyPi(miette::Result<(rip::types::PackageName, PyPiRequirement)>), } pub async fn execute(args: Args) -> miette::Result<()> { @@ -55,9 +56,11 @@ pub async fn execute(args: Args) -> miette::Result<()> { .iter() .map(|dep| { if args.pypi { - DependencyRemovalResult::PyPiDeps(project.manifest.remove_pypi_dependency(dep)) + let pkg_name = rip::types::PackageName::from_str(dep.as_source()) + .expect("Expected dependency name."); + DependencyRemovalResult::PyPi(project.manifest.remove_pypi_dependency(&pkg_name)) } else { - DependencyRemovalResult::PixiDeps(if let Some(p) = &args.platform { + DependencyRemovalResult::Conda(if let Some(p) = &args.platform { project .manifest .remove_target_dependency(dep, &spec_type, p) @@ -74,20 +77,17 @@ pub async fn execute(args: Args) -> miette::Result<()> { let _ = get_up_to_date_prefix(&project, LockFileUsage::Update, false, None).await?; for result in results.iter() { - let removed = match result { - DependencyRemovalResult::PixiDeps(pixi_result) => { - pixi_result.as_ref().unwrap().0.to_string() + let removed_and_spec = match result { + DependencyRemovalResult::Conda(Ok(pixi_result)) => { + (pixi_result.0.to_string(), pixi_result.1.to_string()) } - DependencyRemovalResult::PyPiDeps(pypi_result) => { - pypi_result.as_ref().unwrap().0.as_str().to_string() - } - }; - let spec = match result { - DependencyRemovalResult::PixiDeps(pixi_result) => { - pixi_result.as_ref().unwrap().1.to_string() - } - DependencyRemovalResult::PyPiDeps(pypi_result) => { - pypi_result.as_ref().unwrap().1.to_string() + DependencyRemovalResult::PyPi(Ok(pypi_result)) => ( + pypi_result.0.as_str().to_string(), + pypi_result.1.to_string(), + ), + DependencyRemovalResult::Conda(Err(e)) | DependencyRemovalResult::PyPi(Err(e)) => { + eprintln!("{e}"); + return Ok(()); } }; @@ -99,20 +99,10 @@ pub async fn execute(args: Args) -> miette::Result<()> { eprintln!( "Removed {} from [{}]", - console::style(format!("{removed} {spec}")).bold(), + console::style(format!("{} {}", removed_and_spec.0, removed_and_spec.1)).bold(), console::style(table_name).bold(), ); } - for result in &results { - match result { - DependencyRemovalResult::PixiDeps(Err(e)) - | DependencyRemovalResult::PyPiDeps(Err(e)) => { - eprintln!("{e}"); - } - _ => {} - } - } - Ok(()) } diff --git a/src/project/manifest.rs b/src/project/manifest.rs index 1269990c6..c1513e47b 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -470,17 +470,18 @@ impl Manifest { pub fn remove_pypi_dependency( &mut self, - dep: &rattler_conda_types::PackageName, - ) -> miette::Result<(PackageName, PyPiRequirement)> { + dep: &rip::types::PackageName, + ) -> miette::Result<(rip::types::PackageName, PyPiRequirement)> { if let Item::Table(ref mut t) = self.document[consts::PYPI_DEPENDENCIES] { - if t.contains_key(dep.as_normalized()) && t.remove(dep.as_normalized()).is_some() { - return self.parsed.remove_pypi_dependencies(dep.as_normalized()); + if t.contains_key(dep.as_str()) && t.remove(dep.as_str()).is_some() { + return self.parsed.remove_pypi_dependencies(dep.as_str()); } } Err(miette::miette!( - "Couldn't find {} in PyPi", - console::style(dep.as_normalized()).bold(), + "Couldn't find {} in {}", + console::style(dep.as_str()).bold(), + consts::PYPI_DEPENDENCIES, )) } @@ -1497,12 +1498,26 @@ python = ">=3.12.1,<3.13" [pypi-dependencies] jax = { version = "*", extras = ["cpu"] } +requests = "*" +xpackage = "==1.2.3" +ypackage = {version = ">=1.2.3"} "#; - let mut manifest = Manifest::from_str(Path::new(""), pixi_cfg).unwrap(); + let tmpdir = tempdir().unwrap(); + let mut manifest = Manifest::from_str(tmpdir.path(), pixi_cfg).unwrap(); manifest - .remove_pypi_dependency(&rattler_conda_types::PackageName::try_from("jax").unwrap()) + .remove_pypi_dependency(&rip::types::PackageName::from_str("jax").unwrap()) .unwrap(); - assert!(manifest.parsed.pypi_dependencies.unwrap().is_empty()) + let unwrapped_pypi = manifest.parsed.pypi_dependencies.as_ref().unwrap(); + assert!(!unwrapped_pypi.contains_key(&rip::types::PackageName::from_str("jax").unwrap())); + assert!( + unwrapped_pypi.contains_key(&rip::types::PackageName::from_str("requests").unwrap()) + ); + assert!( + unwrapped_pypi.contains_key(&rip::types::PackageName::from_str("xpackage").unwrap()) + ); + assert!( + unwrapped_pypi.contains_key(&rip::types::PackageName::from_str("ypackage").unwrap()) + ); } #[test] diff --git a/src/project/mod.rs b/src/project/mod.rs index bf0ac8672..97a30b3a4 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -41,7 +41,7 @@ impl DependencyType { pub fn name(&self) -> &'static str { match self { DependencyType::CondaDependency(dep) => dep.name(), - DependencyType::PypiDependency => "pypi-dependencies", + DependencyType::PypiDependency => consts::PYPI_DEPENDENCIES, } } } diff --git a/src/project/python.rs b/src/project/python.rs index 6d5676214..ff7433711 100644 --- a/src/project/python.rs +++ b/src/project/python.rs @@ -28,17 +28,8 @@ pub enum ParsePyPiRequirementError { impl fmt::Display for PyPiRequirement { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let all_extras = self - .extras - .clone() - .map(|each_str| each_str.join(", ")) - .unwrap_or_default(); - write!( - f, - "version=\"{}\", extras=\"{}\"", - self.version.clone().unwrap(), - all_extras - ) + let item: Item = self.clone().into(); + write!(f, "{item}") } } @@ -198,10 +189,12 @@ mod test { #[test] fn test_pypi_to_string() { - let pypi = PyPiRequirement::from_str(">=1.0.0,<2.0.0"); + let req = pep508_rs::Requirement::from_str("numpy[testing]==1.0.0; os_name == \"posix\"") + .unwrap(); + let pypi = PyPiRequirement::from(req); assert_eq!( - pypi.unwrap().to_string(), - "version=\">=1.0.0, <2.0.0\", extras=\"\"" + pypi.to_string(), + "{ version = \"==1.0.0\", extras = [\"testing\"] }" ); } From 364a22cb6612e87130c39818a1976ce830d883ef Mon Sep 17 00:00:00 2001 From: Marcelo Trevisani Date: Wed, 3 Jan 2024 20:24:17 +0000 Subject: [PATCH 3/9] Refactored execute when removing dependency --- src/cli/remove.rs | 48 +++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/cli/remove.rs b/src/cli/remove.rs index 1afa7d6c1..aa4e53d76 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -75,34 +75,34 @@ pub async fn execute(args: Args) -> miette::Result<()> { // updating prefix after removing from toml let _ = get_up_to_date_prefix(&project, LockFileUsage::Update, false, None).await?; - + let table_name = if let Some(p) = &args.platform { + format!("target.{}.{}", p.as_str(), spec_type.name()) + } else { + spec_type.name().to_string() + }; + fn print_ok_dep_removal(pkg_name: &String, pkg_extras: &String, table_name: &String) { + eprintln!( + "Removed {} from [{}]", + console::style(format!("{pkg_name} {pkg_extras}")).bold(), + console::style(table_name).bold() + ) + } for result in results.iter() { - let removed_and_spec = match result { - DependencyRemovalResult::Conda(Ok(pixi_result)) => { - (pixi_result.0.to_string(), pixi_result.1.to_string()) - } - DependencyRemovalResult::PyPi(Ok(pypi_result)) => ( - pypi_result.0.as_str().to_string(), - pypi_result.1.to_string(), + match result { + DependencyRemovalResult::Conda(Ok(pixi_result)) => print_ok_dep_removal( + &pixi_result.0.to_string(), + &pixi_result.1.to_string(), + &table_name, + ), + DependencyRemovalResult::PyPi(Ok(pypi_result)) => print_ok_dep_removal( + &pypi_result.0.as_str().to_string(), + &pypi_result.1.to_string(), + &table_name, ), DependencyRemovalResult::Conda(Err(e)) | DependencyRemovalResult::PyPi(Err(e)) => { - eprintln!("{e}"); - return Ok(()); + eprintln!("{e}") } - }; - - let table_name = if let Some(p) = &args.platform { - format!("target.{}.{}", p.as_str(), spec_type.name()) - } else { - spec_type.name().to_string() - }; - - eprintln!( - "Removed {} from [{}]", - console::style(format!("{} {}", removed_and_spec.0, removed_and_spec.1)).bold(), - console::style(table_name).bold(), - ); + } } - Ok(()) } From 0802bfb4b61697d57380f52ce4154d257589038f Mon Sep 17 00:00:00 2001 From: Marcelo Trevisani Date: Wed, 3 Jan 2024 22:13:39 +0000 Subject: [PATCH 4/9] Refactoring to be able to remove pypi-dependencies based on target platform as well --- src/cli/remove.rs | 6 +- src/project/manifest.rs | 96 ++++++++++- ...__test__remove_target_pypi_dependency.snap | 153 ++++++++++++++++++ 3 files changed, 253 insertions(+), 2 deletions(-) create mode 100644 src/project/snapshots/pixi__project__manifest__test__remove_target_pypi_dependency.snap diff --git a/src/cli/remove.rs b/src/cli/remove.rs index aa4e53d76..f962d3ef3 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -58,7 +58,11 @@ pub async fn execute(args: Args) -> miette::Result<()> { if args.pypi { let pkg_name = rip::types::PackageName::from_str(dep.as_source()) .expect("Expected dependency name."); - DependencyRemovalResult::PyPi(project.manifest.remove_pypi_dependency(&pkg_name)) + DependencyRemovalResult::PyPi(if let Some(p) = &args.platform { + project.manifest.remove_target_pypi_dependency(&pkg_name, p) + } else { + project.manifest.remove_pypi_dependency(&pkg_name) + }) } else { DependencyRemovalResult::Conda(if let Some(p) = &args.platform { project diff --git a/src/project/manifest.rs b/src/project/manifest.rs index c1513e47b..25815e00f 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -519,6 +519,18 @@ impl Manifest { .remove_target_dependency(dep.as_normalized(), spec_type, platform) } + /// Removes a target specific dependency from `pixi.toml` based on pypi-dependencies + pub fn remove_target_pypi_dependency( + &mut self, + dep: &rip::types::PackageName, + platform: &Platform, + ) -> miette::Result<(rip::types::PackageName, PyPiRequirement)> { + let table = get_toml_target_table(&mut self.document, platform, consts::PYPI_DEPENDENCIES)?; + table.remove(dep.as_str()); + self.parsed + .remove_target_pypi_dependency(dep.as_str(), platform) + } + /// Returns a mutable reference to the channels array. fn channels_array_mut(&mut self) -> miette::Result<&mut Array> { let project = &mut self.document["project"]; @@ -832,7 +844,6 @@ impl ProjectManifest { .find(|&pkg_name| pkg_name.as_str() == dep) .cloned(); - // Step 2: Use the key to remove the entry key_to_remove.and_then(|pkg_key| pypi_deps.shift_remove_entry(&pkg_key)) }) .ok_or_else(|| { @@ -869,6 +880,57 @@ impl ProjectManifest { } } + pub fn remove_target_pypi_dependency( + &mut self, + dep: &str, + platform: &Platform, + ) -> miette::Result<(PackageName, PyPiRequirement)> { + let target = PixiSpanned::from(TargetSelector::Platform(*platform)); + let target_metadata = self.target.get_mut(&target).ok_or(miette::miette!( + "Platform: {} is not configured for this project", + console::style(platform.as_str()).bold(), + ))?; + let dependencies = target_metadata.pypi_dependencies.as_mut(); + + if let Some(deps) = dependencies { + let key_to_remove = deps.keys().find(|k| k.as_str() == dep); + if let Some(key_dep) = key_to_remove { + deps.shift_remove_entry(&key_dep.clone()) + .ok_or(miette::miette!( + "Couldn't find {} in [{}]", + console::style(dep).bold(), + console::style(format!( + "target.{}.{}", + platform.as_str(), + consts::PYPI_DEPENDENCIES + )) + .bold(), + )) + } else { + Err(miette::miette!( + "Couldn't find {} in [{}]", + console::style(dep).bold(), + console::style(format!( + "target.{}.{}", + platform.as_str(), + consts::PYPI_DEPENDENCIES + )) + .bold(), + )) + } + } else { + Err(miette::miette!( + "[{}] doesn't exist", + console::style(format!( + "target.{}.{}", + platform.as_str(), + consts::PYPI_DEPENDENCIES + )) + .bold(), + )) + } + } + /// Remove a dependency for a `Platform`. pub fn remove_target_dependency( &mut self, @@ -1485,6 +1547,38 @@ mod test { assert_debug_snapshot!(manifest.parsed.target); } + #[test] + fn test_remove_target_pypi_dependency() { + let pixi_cfg = r#"[project] +name = "pixi_fun" +version = "0.1.0" +channels = [] +platforms = ["linux-64", "win-64"] + +[dependencies] +python = ">=3.12.1,<3.13" + +[target.win-64.pypi-dependencies] +jax = { version = "*", extras = ["cpu"] } +requests = "*" + +[target.linux-64.pypi-dependencies] +xpackage = "==1.2.3" +ypackage = {version = ">=1.2.3"} +"#; + let tmpdir = tempdir().unwrap(); + let mut manifest = Manifest::from_str(tmpdir.path(), pixi_cfg).unwrap(); + manifest + .parsed + .remove_target_pypi_dependency("xpackage", &Platform::Linux64) + .unwrap(); + manifest + .parsed + .remove_target_pypi_dependency("jax", &Platform::Win64) + .unwrap(); + assert_debug_snapshot!(manifest.parsed); + } + #[test] fn test_remove_pypi_dependencies() { let pixi_cfg = r#"[project] diff --git a/src/project/snapshots/pixi__project__manifest__test__remove_target_pypi_dependency.snap b/src/project/snapshots/pixi__project__manifest__test__remove_target_pypi_dependency.snap new file mode 100644 index 000000000..8da2a8c0c --- /dev/null +++ b/src/project/snapshots/pixi__project__manifest__test__remove_target_pypi_dependency.snap @@ -0,0 +1,153 @@ +--- +source: src/project/manifest.rs +assertion_line: 1548 +expression: manifest.parsed +--- +ProjectManifest { + project: ProjectMetadata { + name: "pixi_fun", + version: Some( + Version { + version: [[0], [0], [1], [0]], + local: [], + }, + ), + description: None, + authors: [], + channels: [], + platforms: PixiSpanned { + span: Some( + 72..94, + ), + value: [ + Linux64, + Win64, + ], + }, + license: None, + license_file: None, + readme: None, + homepage: None, + repository: None, + documentation: None, + }, + tasks: {}, + system_requirements: SystemRequirements { + windows: None, + unix: None, + macos: None, + linux: None, + cuda: None, + libc: None, + archspec: None, + }, + dependencies: { + "python": NamelessMatchSpec { + version: Some( + Group( + And, + [ + Range( + GreaterEquals, + Version { + version: [[0], [3], [12], [1]], + local: [], + }, + ), + Range( + Less, + Version { + version: [[0], [3], [13]], + local: [], + }, + ), + ], + ), + ), + build: None, + build_number: None, + file_name: None, + channel: None, + subdir: None, + namespace: None, + md5: None, + sha256: None, + }, + }, + host_dependencies: None, + build_dependencies: None, + target: { + PixiSpanned { + span: Some( + 146..152, + ), + value: Platform( + Win64, + ), + }: TargetMetadata { + dependencies: {}, + host_dependencies: None, + build_dependencies: None, + pypi_dependencies: Some( + { + PackageName { + source: "requests", + normalized: "requests", + }: PyPiRequirement { + version: None, + extras: None, + }, + }, + ), + activation: None, + tasks: {}, + }, + PixiSpanned { + span: Some( + 238..246, + ), + value: Platform( + Linux64, + ), + }: TargetMetadata { + dependencies: {}, + host_dependencies: None, + build_dependencies: None, + pypi_dependencies: Some( + { + PackageName { + source: "ypackage", + normalized: "ypackage", + }: PyPiRequirement { + version: Some( + VersionSpecifiers( + [ + VersionSpecifier { + operator: GreaterThanEqual, + version: Version { + epoch: 0, + release: [ + 1, + 2, + 3, + ], + pre: None, + post: None, + dev: None, + local: None, + }, + }, + ], + ), + ), + extras: None, + }, + }, + ), + activation: None, + tasks: {}, + }, + }, + activation: None, + pypi_dependencies: None, +} From 2127e1e221059fe88f6d24209ab1d878ebdb3fbe Mon Sep 17 00:00:00 2001 From: Marcelo Trevisani Date: Thu, 4 Jan 2024 09:17:57 +0000 Subject: [PATCH 5/9] Add error handling for remove --pypi cli --- src/cli/remove.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/cli/remove.rs b/src/cli/remove.rs index f962d3ef3..730ba9055 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -39,6 +39,7 @@ pub struct Args { enum DependencyRemovalResult { Conda(miette::Result<(String, NamelessMatchSpec)>), PyPi(miette::Result<(rip::types::PackageName, PyPiRequirement)>), + Error(miette::Report), } pub async fn execute(args: Args) -> miette::Result<()> { @@ -56,13 +57,20 @@ pub async fn execute(args: Args) -> miette::Result<()> { .iter() .map(|dep| { if args.pypi { - let pkg_name = rip::types::PackageName::from_str(dep.as_source()) - .expect("Expected dependency name."); - DependencyRemovalResult::PyPi(if let Some(p) = &args.platform { - project.manifest.remove_target_pypi_dependency(&pkg_name, p) - } else { - project.manifest.remove_pypi_dependency(&pkg_name) - }) + match rip::types::PackageName::from_str(dep.as_source()) { + Ok(pkg_name) => { + if let Some(p) = &args.platform { + DependencyRemovalResult::PyPi( + project.manifest.remove_target_pypi_dependency(&pkg_name, p), + ) + } else { + DependencyRemovalResult::PyPi( + project.manifest.remove_pypi_dependency(&pkg_name), + ) + } + } + Err(e) => DependencyRemovalResult::Error(e.into()), + } } else { DependencyRemovalResult::Conda(if let Some(p) = &args.platform { project @@ -103,7 +111,9 @@ pub async fn execute(args: Args) -> miette::Result<()> { &pypi_result.1.to_string(), &table_name, ), - DependencyRemovalResult::Conda(Err(e)) | DependencyRemovalResult::PyPi(Err(e)) => { + DependencyRemovalResult::Conda(Err(e)) + | DependencyRemovalResult::PyPi(Err(e)) + | DependencyRemovalResult::Error(e) => { eprintln!("{e}") } } From 87125e4328a6f7db9ccd54250abedb64e19062b8 Mon Sep 17 00:00:00 2001 From: Marcelo Trevisani Date: Thu, 4 Jan 2024 09:24:31 +0000 Subject: [PATCH 6/9] Properly return Err when an error occur when removing a dependency --- src/cli/remove.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cli/remove.rs b/src/cli/remove.rs index 730ba9055..08d61a2e4 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -2,6 +2,7 @@ use std::path::PathBuf; use std::str::FromStr; use clap::Parser; +use miette::miette; use rattler_conda_types::{NamelessMatchSpec, PackageName, Platform}; use crate::environment::LockFileUsage; @@ -114,7 +115,7 @@ pub async fn execute(args: Args) -> miette::Result<()> { DependencyRemovalResult::Conda(Err(e)) | DependencyRemovalResult::PyPi(Err(e)) | DependencyRemovalResult::Error(e) => { - eprintln!("{e}") + return Err(miette!("{e}")); } } } From 498815ca7f15b345e5f7e7faea36b7b597a9bb90 Mon Sep 17 00:00:00 2001 From: Marcelo Trevisani Date: Thu, 4 Jan 2024 10:46:10 +0000 Subject: [PATCH 7/9] Show all errors instead of the first one found --- src/cli/remove.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/cli/remove.rs b/src/cli/remove.rs index 08d61a2e4..9614cf327 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -2,7 +2,6 @@ use std::path::PathBuf; use std::str::FromStr; use clap::Parser; -use miette::miette; use rattler_conda_types::{NamelessMatchSpec, PackageName, Platform}; use crate::environment::LockFileUsage; @@ -100,6 +99,7 @@ pub async fn execute(args: Args) -> miette::Result<()> { console::style(table_name).bold() ) } + let mut all_errors: Vec = Vec::new(); for result in results.iter() { match result { DependencyRemovalResult::Conda(Ok(pixi_result)) => print_ok_dep_removal( @@ -115,9 +115,14 @@ pub async fn execute(args: Args) -> miette::Result<()> { DependencyRemovalResult::Conda(Err(e)) | DependencyRemovalResult::PyPi(Err(e)) | DependencyRemovalResult::Error(e) => { - return Err(miette!("{e}")); + all_errors.push(format!("{e}")); } } } - Ok(()) + + if all_errors.is_empty() { + Ok(()) + } else { + Err(miette::miette!(all_errors.join("\n"))) + } } From 19fd1384f69549f351fab107c18110eb95d63572 Mon Sep 17 00:00:00 2001 From: Marcelo Trevisani Date: Fri, 5 Jan 2024 01:08:27 +0000 Subject: [PATCH 8/9] Parse deps name to package name first and after that remove dependency --- src/cli/remove.rs | 119 ++++++++++++++++++++-------------------------- 1 file changed, 52 insertions(+), 67 deletions(-) diff --git a/src/cli/remove.rs b/src/cli/remove.rs index 9614cf327..b4bd4ab24 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -2,18 +2,18 @@ use std::path::PathBuf; use std::str::FromStr; use clap::Parser; -use rattler_conda_types::{NamelessMatchSpec, PackageName, Platform}; +use miette::miette; +use rattler_conda_types::Platform; use crate::environment::LockFileUsage; -use crate::project::python::PyPiRequirement; -use crate::{environment::get_up_to_date_prefix, project::SpecType, Project}; +use crate::{consts, environment::get_up_to_date_prefix, project::SpecType, Project}; /// Remove the dependency from the project #[derive(Debug, Default, Parser)] pub struct Args { /// List of dependencies you wish to remove from the project #[arg(required = true)] - pub deps: Vec, + pub deps: Vec, /// The path to 'pixi.toml' #[arg(long)] @@ -36,10 +36,16 @@ pub struct Args { pub platform: Option, } -enum DependencyRemovalResult { - Conda(miette::Result<(String, NamelessMatchSpec)>), - PyPi(miette::Result<(rip::types::PackageName, PyPiRequirement)>), - Error(miette::Report), +fn convert_pkg_name(deps: &[String]) -> miette::Result> +where + T: FromStr, +{ + deps.iter() + .map(|dep| { + T::from_str(dep) + .map_err(|_| miette!("Can't convert dependency name `{dep}` to package name")) + }) + .collect() } pub async fn execute(args: Args) -> miette::Result<()> { @@ -53,45 +59,17 @@ pub async fn execute(args: Args) -> miette::Result<()> { SpecType::Run }; - let results = deps - .iter() - .map(|dep| { - if args.pypi { - match rip::types::PackageName::from_str(dep.as_source()) { - Ok(pkg_name) => { - if let Some(p) = &args.platform { - DependencyRemovalResult::PyPi( - project.manifest.remove_target_pypi_dependency(&pkg_name, p), - ) - } else { - DependencyRemovalResult::PyPi( - project.manifest.remove_pypi_dependency(&pkg_name), - ) - } - } - Err(e) => DependencyRemovalResult::Error(e.into()), - } - } else { - DependencyRemovalResult::Conda(if let Some(p) = &args.platform { - project - .manifest - .remove_target_dependency(dep, &spec_type, p) - } else { - project.manifest.remove_dependency(dep, &spec_type) - }) - } - }) - .collect::>(); - - project.save()?; - - // updating prefix after removing from toml - let _ = get_up_to_date_prefix(&project, LockFileUsage::Update, false, None).await?; - let table_name = if let Some(p) = &args.platform { - format!("target.{}.{}", p.as_str(), spec_type.name()) + let section_name: String = if args.pypi { + consts::PYPI_DEPENDENCIES.to_string() } else { spec_type.name().to_string() }; + let table_name = if let Some(p) = &args.platform { + format!("target.{}.{}", p.as_str(), section_name) + } else { + section_name + }; + fn print_ok_dep_removal(pkg_name: &String, pkg_extras: &String, table_name: &String) { eprintln!( "Removed {} from [{}]", @@ -99,30 +77,37 @@ pub async fn execute(args: Args) -> miette::Result<()> { console::style(table_name).bold() ) } - let mut all_errors: Vec = Vec::new(); - for result in results.iter() { - match result { - DependencyRemovalResult::Conda(Ok(pixi_result)) => print_ok_dep_removal( - &pixi_result.0.to_string(), - &pixi_result.1.to_string(), - &table_name, - ), - DependencyRemovalResult::PyPi(Ok(pypi_result)) => print_ok_dep_removal( - &pypi_result.0.as_str().to_string(), - &pypi_result.1.to_string(), + + if args.pypi { + let all_pkg_name = convert_pkg_name::(&deps)?; + for dep in all_pkg_name.iter() { + let result = if let Some(p) = &args.platform { + project.manifest.remove_target_pypi_dependency(dep, p)? + } else { + project.manifest.remove_pypi_dependency(dep)? + }; + print_ok_dep_removal( + &result.0.as_str().to_string(), + &result.1.to_string(), &table_name, - ), - DependencyRemovalResult::Conda(Err(e)) - | DependencyRemovalResult::PyPi(Err(e)) - | DependencyRemovalResult::Error(e) => { - all_errors.push(format!("{e}")); - } + ); } - } - - if all_errors.is_empty() { - Ok(()) } else { - Err(miette::miette!(all_errors.join("\n"))) - } + let all_pkg_name = convert_pkg_name::(&deps)?; + for dep in all_pkg_name.iter() { + let result = if let Some(p) = &args.platform { + project + .manifest + .remove_target_dependency(dep, &spec_type, p)? + } else { + project.manifest.remove_dependency(dep, &spec_type)? + }; + print_ok_dep_removal(&result.0, &result.1.to_string(), &table_name); + } + }; + project.save()?; + + // updating prefix after removing from toml + let _ = get_up_to_date_prefix(&project, LockFileUsage::Update, false, None).await?; + Ok(()) } From 3a791252ac76e08be9497a465986ff166127deb0 Mon Sep 17 00:00:00 2001 From: Marcelo Trevisani Date: Fri, 5 Jan 2024 19:01:50 +0000 Subject: [PATCH 9/9] Print the sucessful removals only after the project is saved. --- src/cli/remove.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/cli/remove.rs b/src/cli/remove.rs index b4bd4ab24..dd2386408 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -70,14 +70,14 @@ pub async fn execute(args: Args) -> miette::Result<()> { section_name }; - fn print_ok_dep_removal(pkg_name: &String, pkg_extras: &String, table_name: &String) { - eprintln!( + fn format_ok_message(pkg_name: &String, pkg_extras: &String, table_name: &String) -> String { + format!( "Removed {} from [{}]", console::style(format!("{pkg_name} {pkg_extras}")).bold(), console::style(table_name).bold() ) } - + let mut sucessful_output: Vec = Vec::with_capacity(deps.len()); if args.pypi { let all_pkg_name = convert_pkg_name::(&deps)?; for dep in all_pkg_name.iter() { @@ -86,11 +86,11 @@ pub async fn execute(args: Args) -> miette::Result<()> { } else { project.manifest.remove_pypi_dependency(dep)? }; - print_ok_dep_removal( + sucessful_output.push(format_ok_message( &result.0.as_str().to_string(), &result.1.to_string(), &table_name, - ); + )); } } else { let all_pkg_name = convert_pkg_name::(&deps)?; @@ -102,10 +102,15 @@ pub async fn execute(args: Args) -> miette::Result<()> { } else { project.manifest.remove_dependency(dep, &spec_type)? }; - print_ok_dep_removal(&result.0, &result.1.to_string(), &table_name); + sucessful_output.push(format_ok_message( + &result.0, + &result.1.to_string(), + &table_name, + )); } }; project.save()?; + eprintln!("{}", sucessful_output.join("\n")); // updating prefix after removing from toml let _ = get_up_to_date_prefix(&project, LockFileUsage::Update, false, None).await?;