From 47277fd252586c43ad444f7710862fc5bd4eee0e Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Mon, 2 Sep 2024 11:01:19 +0200 Subject: [PATCH 1/3] feat: improve error and feedback when target does not exist --- crates/pixi_manifest/src/manifest.rs | 49 ++++++++++++++++++++-------- src/cli/remove.rs | 29 +++++++++------- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/crates/pixi_manifest/src/manifest.rs b/crates/pixi_manifest/src/manifest.rs index f81396237..561c57b4b 100644 --- a/crates/pixi_manifest/src/manifest.rs +++ b/crates/pixi_manifest/src/manifest.rs @@ -420,7 +420,9 @@ impl Manifest { // Remove the dependency from the manifest match self .target_mut(platform, feature_name) - .ok_or_else(|| Self::handle_target_missing(platform.as_ref(), feature_name))? + .ok_or_else(|| { + Self::handle_target_missing(platform.as_ref(), feature_name, "dependencies") + })? .remove_dependency(dep, spec_type) { Ok(_) => (), @@ -447,7 +449,13 @@ impl Manifest { // Remove the dependency from the manifest match self .target_mut(platform, feature_name) - .ok_or_else(|| Self::handle_target_missing(platform.as_ref(), feature_name))? + .ok_or_else(|| { + Self::handle_target_missing( + platform.as_ref(), + feature_name, + "pypi-dependencies", + ) + })? .remove_pypi_dependency(dep) { Ok(_) => (), @@ -463,22 +471,35 @@ impl Manifest { Ok(()) } + /// Handles the target missing error cases fn handle_target_missing( platform: Option<&Platform>, feature_name: &FeatureName, + section: &str, ) -> miette::Report { - match platform { - None => { - miette!("No target for feature `{}`", feature_name) - } - Some(platform) => { - miette!( - "No target for feature `{}` on platform `{}`", - feature_name, - platform - ) - } - } + let platform = platform.copied().unwrap_or_else(Platform::current); + + let help = if feature_name.is_default() { + format!( + r#"Expected target for `{name}`, e.g.: `[target.{platform}.{section}]`"#, + name = feature_name, + platform = platform, + section = section + ) + } else { + format!( + r#"Expected target for `{name}`, e.g.: `[feature.{name}.target.{platform}.{section}]`"#, + name = feature_name, + platform = platform, + section = section + ) + }; + miette!( + help = &help, + "No target for feature `{name}` found on platform `{platform}`", + name = feature_name, + platform = platform + ) } /// Returns true if any of the features has pypi dependencies defined. diff --git a/src/cli/remove.rs b/src/cli/remove.rs index e8c60d87a..3e386d970 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -1,4 +1,5 @@ use clap::Parser; +use miette::Context; use crate::environment::update_prefix; use crate::DependencyType; @@ -41,21 +42,27 @@ pub async fn execute(args: Args) -> miette::Result<()> { match dependency_type { DependencyType::PypiDependency => { for name in dependency_config.pypi_deps(&project)?.keys() { - project.manifest.remove_pypi_dependency( - name, - &dependency_config.platform, - &dependency_config.feature_name(), - )?; + project + .manifest + .remove_pypi_dependency( + name, + &dependency_config.platform, + &dependency_config.feature_name(), + ) + .context("failed to remove pypi dependency")?; } } DependencyType::CondaDependency(spec_type) => { for name in dependency_config.specs()?.keys() { - project.manifest.remove_dependency( - name, - spec_type, - &dependency_config.platform, - &dependency_config.feature_name(), - )?; + project + .manifest + .remove_dependency( + name, + spec_type, + &dependency_config.platform, + &dependency_config.feature_name(), + ) + .context("failed to remove dependency")?; } } }; From caafcd7482df7155cc86c29666f65aff0914be19 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Mon, 2 Sep 2024 13:24:49 +0200 Subject: [PATCH 2/3] feat: updated error message --- src/cli/remove.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/cli/remove.rs b/src/cli/remove.rs index 3e386d970..d9c1a0cff 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -49,7 +49,10 @@ pub async fn execute(args: Args) -> miette::Result<()> { &dependency_config.platform, &dependency_config.feature_name(), ) - .context("failed to remove pypi dependency")?; + .wrap_err(format!( + "failed to remove PyPI dependency: '{}'", + name.as_source() + ))?; } } DependencyType::CondaDependency(spec_type) => { @@ -62,7 +65,10 @@ pub async fn execute(args: Args) -> miette::Result<()> { &dependency_config.platform, &dependency_config.feature_name(), ) - .context("failed to remove dependency")?; + .wrap_err(format!( + "failed to remove dependency: '{}'", + name.as_source() + ))?; } } }; From e90d7d9c8541ec1cf011e7d03fef79d64f0be5b6 Mon Sep 17 00:00:00 2001 From: Tim de Jager Date: Mon, 2 Sep 2024 13:30:58 +0200 Subject: [PATCH 3/3] feat: change to use constants --- crates/pixi_consts/src/consts.rs | 1 + crates/pixi_manifest/src/manifest.rs | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/pixi_consts/src/consts.rs b/crates/pixi_consts/src/consts.rs index 81b8720fb..46f7f8301 100644 --- a/crates/pixi_consts/src/consts.rs +++ b/crates/pixi_consts/src/consts.rs @@ -17,6 +17,7 @@ pub const PREFIX_FILE_NAME: &str = "pixi_env_prefix"; pub const ENVIRONMENTS_DIR: &str = "envs"; pub const SOLVE_GROUP_ENVIRONMENTS_DIR: &str = "solve-group-envs"; pub const PYPI_DEPENDENCIES: &str = "pypi-dependencies"; +pub const DEPENDENCIES: &str = "dependencies"; pub const TASK_CACHE_DIR: &str = "task-cache-v0"; pub const PIXI_UV_INSTALLER: &str = "uv-pixi"; pub const CONDA_PACKAGE_CACHE_DIR: &str = rattler_cache::PACKAGE_CACHE_DIR; diff --git a/crates/pixi_manifest/src/manifest.rs b/crates/pixi_manifest/src/manifest.rs index 561c57b4b..59dbe3d68 100644 --- a/crates/pixi_manifest/src/manifest.rs +++ b/crates/pixi_manifest/src/manifest.rs @@ -421,7 +421,11 @@ impl Manifest { match self .target_mut(platform, feature_name) .ok_or_else(|| { - Self::handle_target_missing(platform.as_ref(), feature_name, "dependencies") + Self::handle_target_missing( + platform.as_ref(), + feature_name, + consts::DEPENDENCIES, + ) })? .remove_dependency(dep, spec_type) { @@ -453,7 +457,7 @@ impl Manifest { Self::handle_target_missing( platform.as_ref(), feature_name, - "pypi-dependencies", + consts::PYPI_DEPENDENCIES, ) })? .remove_pypi_dependency(dep)