diff --git a/src/cli/run.rs b/src/cli/run.rs index f90224b12..e34beda2c 100644 --- a/src/cli/run.rs +++ b/src/cli/run.rs @@ -6,6 +6,7 @@ use miette::{miette, Context, Diagnostic, IntoDiagnostic}; use rattler_conda_types::Platform; use crate::environment::LockFileUsage; +use crate::project::errors::UnsupportedPlatformError; use crate::task::{ ExecutableTask, FailedToParseShellScript, InvalidWorkingDirectory, TraversalError, }; @@ -90,6 +91,9 @@ enum TaskExecutionError { #[error(transparent)] TraverseError(#[from] TraversalError), + + #[error(transparent)] + UnsupportedPlatformError(#[from] UnsupportedPlatformError), } /// Called to execute a single command. @@ -130,8 +134,8 @@ async fn execute_task<'p>( if status_code == 127 { let available_tasks = task .project() - .manifest - .tasks(Some(Platform::current())) + .default_environment() + .tasks(Some(Platform::current()))? .into_keys() .sorted() .collect_vec(); diff --git a/src/cli/task.rs b/src/cli/task.rs index 2f8200c79..01a02f6a4 100644 --- a/src/cli/task.rs +++ b/src/cli/task.rs @@ -1,10 +1,12 @@ -use crate::project::manifest::FeatureName; +use crate::project::manifest::{EnvironmentName, FeatureName}; use crate::task::{quote, Alias, CmdArgs, Execute, Task}; use crate::Project; use clap::Parser; use itertools::Itertools; +use miette::miette; use rattler_conda_types::Platform; use std::path::PathBuf; +use std::str::FromStr; use toml_edit::{Array, Item, Table, Value}; #[derive(Parser, Debug)] @@ -35,6 +37,10 @@ pub struct RemoveArgs { /// The platform for which the task should be removed #[arg(long, short)] pub platform: Option, + + /// The feature for which the task should be removed + #[arg(long, short)] + pub feature: Option, } #[derive(Parser, Debug, Clone)] @@ -84,6 +90,11 @@ pub struct AliasArgs { pub struct ListArgs { #[arg(long, short)] pub summary: bool, + + /// The environment the list should be generated for + /// If not specified, the default environment is used. + #[arg(long, short)] + pub environment: Option, } impl From for Task { @@ -153,7 +164,7 @@ pub fn execute(args: Args) -> miette::Result<()> { .add_task(name, task.clone(), args.platform, &feature)?; project.save()?; eprintln!( - "{}Added task {}: {}", + "{}Added task `{}`: {}", console::style(console::Emoji("✔ ", "+")).green(), console::style(&name).bold(), task, @@ -161,11 +172,14 @@ pub fn execute(args: Args) -> miette::Result<()> { } Operation::Remove(args) => { let mut to_remove = Vec::new(); + let feature = args + .feature + .map_or(FeatureName::Default, FeatureName::Named); for name in args.names.iter() { if let Some(platform) = args.platform { if !project .manifest - .tasks(Some(platform)) + .tasks(Some(platform), &feature)? .contains_key(name.as_str()) { eprintln!( @@ -176,11 +190,16 @@ pub fn execute(args: Args) -> miette::Result<()> { ); continue; } - } else if !project.manifest.tasks(None).contains_key(name.as_str()) { + } else if !project + .manifest + .tasks(None, &feature)? + .contains_key(name.as_str()) + { eprintln!( - "{}Task {} does not exist", + "{}Task `{}` does not exist for the `{}` feature", console::style(console::Emoji("❌ ", "X")).red(), console::style(&name).bold(), + console::style(&feature).bold(), ); continue; } @@ -207,10 +226,10 @@ pub fn execute(args: Args) -> miette::Result<()> { } for (name, platform) in to_remove { - project.manifest.remove_task(name, platform)?; + project.manifest.remove_task(name, platform, &feature)?; project.save()?; eprintln!( - "{}Removed task {} ", + "{}Removed task `{}` ", console::style(console::Emoji("✔ ", "+")).green(), console::style(&name).bold(), ); @@ -224,15 +243,18 @@ pub fn execute(args: Args) -> miette::Result<()> { .add_task(name, task.clone(), args.platform, &FeatureName::Default)?; project.save()?; eprintln!( - "{} Added alias {}: {}", + "{} Added alias `{}`: {}", console::style("@").blue(), console::style(&name).bold(), task, ); } Operation::List(args) => { + let env = EnvironmentName::from_str(args.environment.as_deref().unwrap_or("default"))?; let tasks = project - .tasks(Some(Platform::current())) + .environment(&env) + .ok_or(miette!("Environment `{}` not found in project", env))? + .tasks(Some(Platform::current()))? .into_keys() .collect_vec(); if tasks.is_empty() { diff --git a/src/consts.rs b/src/consts.rs index e384d079c..45f2fbe4f 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -6,3 +6,5 @@ pub const ENVIRONMENTS_DIR: &str = "envs"; pub const PYPI_DEPENDENCIES: &str = "pypi-dependencies"; pub const DEFAULT_ENVIRONMENT_NAME: &str = "default"; + +pub const DEFAULT_FEATURE_NAME: &str = DEFAULT_ENVIRONMENT_NAME; diff --git a/src/project/environment.rs b/src/project/environment.rs index cbe63e395..374dfbcc4 100644 --- a/src/project/environment.rs +++ b/src/project/environment.rs @@ -253,7 +253,7 @@ impl<'p> Environment<'p> { if let Some(platform) = platform { if !self.platforms().contains(&platform) { return Err(UnsupportedPlatformError { - project: self.project, + environments_platforms: self.platforms().into_iter().collect(), environment: self.name().clone(), platform, }); diff --git a/src/project/errors.rs b/src/project/errors.rs index 7fa663aa5..a7997e37e 100644 --- a/src/project/errors.rs +++ b/src/project/errors.rs @@ -11,9 +11,9 @@ use thiserror::Error; /// TODO: Make this error better by also explaining to the user why a certain platform was not /// supported and with suggestions as how to fix it. #[derive(Debug, Clone)] -pub struct UnsupportedPlatformError<'p> { - /// The project that the platform is not supported for. - pub project: &'p Project, +pub struct UnsupportedPlatformError { + /// Platforms supported by the environment + pub environments_platforms: Vec, /// The environment that the platform is not supported for. pub environment: EnvironmentName, @@ -22,9 +22,9 @@ pub struct UnsupportedPlatformError<'p> { pub platform: Platform, } -impl<'p> Error for UnsupportedPlatformError<'p> {} +impl Error for UnsupportedPlatformError {} -impl<'p> Display for UnsupportedPlatformError<'p> { +impl Display for UnsupportedPlatformError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match &self.environment { EnvironmentName::Default => { @@ -39,16 +39,15 @@ impl<'p> Display for UnsupportedPlatformError<'p> { } } -impl<'p> Diagnostic for UnsupportedPlatformError<'p> { +impl Diagnostic for UnsupportedPlatformError { fn code(&self) -> Option> { Some(Box::new("unsupported-platform".to_string())) } fn help(&self) -> Option> { - let env = self.project.environment(&self.environment)?; Some(Box::new(format!( "supported platforms are {}", - env.platforms().into_iter().format(", ") + self.environments_platforms.iter().format(", ") ))) } diff --git a/src/project/manifest/environment.rs b/src/project/manifest/environment.rs index 90d78a020..8f9877a58 100644 --- a/src/project/manifest/environment.rs +++ b/src/project/manifest/environment.rs @@ -5,6 +5,7 @@ use miette::Diagnostic; use regex::Regex; use serde::{self, Deserialize, Deserializer}; use std::borrow::Borrow; +use std::fmt; use std::hash::{Hash, Hasher}; use std::str::FromStr; use thiserror::Error; @@ -38,6 +39,16 @@ impl Borrow for EnvironmentName { self.as_str() } } + +impl fmt::Display for EnvironmentName { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + EnvironmentName::Default => write!(f, "{}", consts::DEFAULT_ENVIRONMENT_NAME), + EnvironmentName::Named(name) => write!(f, "{}", name), + } + } +} + #[derive(Debug, Clone, Error, Diagnostic, PartialEq)] #[error("Failed to parse environment name '{attempted_parse}', please use only lowercase letters, numbers and dashes")] pub struct ParseEnvironmentNameError { diff --git a/src/project/manifest/feature.rs b/src/project/manifest/feature.rs index 9a4d1be54..9e45cfa8c 100644 --- a/src/project/manifest/feature.rs +++ b/src/project/manifest/feature.rs @@ -28,7 +28,7 @@ impl<'de> Deserialize<'de> for FeatureName { D: serde::Deserializer<'de>, { match String::deserialize(deserializer)?.as_str() { - consts::DEFAULT_ENVIRONMENT_NAME => Err(D::Error::custom( + consts::DEFAULT_FEATURE_NAME => Err(D::Error::custom( "The name 'default' is reserved for the default feature", )), name => Ok(FeatureName::Named(name.to_string())), @@ -39,7 +39,7 @@ impl<'de> Deserialize<'de> for FeatureName { impl<'s> From<&'s str> for FeatureName { fn from(value: &'s str) -> Self { match value { - consts::DEFAULT_ENVIRONMENT_NAME => FeatureName::Default, + consts::DEFAULT_FEATURE_NAME => FeatureName::Default, name => FeatureName::Named(name.to_string()), } } @@ -54,7 +54,7 @@ impl FeatureName { } pub fn as_str(&self) -> &str { - self.name().unwrap_or(consts::DEFAULT_ENVIRONMENT_NAME) + self.name().unwrap_or(consts::DEFAULT_FEATURE_NAME) } } @@ -67,7 +67,7 @@ impl Borrow for FeatureName { impl From for String { fn from(name: FeatureName) -> Self { match name { - FeatureName::Default => consts::DEFAULT_ENVIRONMENT_NAME.to_string(), + FeatureName::Default => consts::DEFAULT_FEATURE_NAME.to_string(), FeatureName::Named(name) => name, } } @@ -75,7 +75,7 @@ impl From for String { impl<'a> From<&'a FeatureName> for String { fn from(name: &'a FeatureName) -> Self { match name { - FeatureName::Default => consts::DEFAULT_ENVIRONMENT_NAME.to_string(), + FeatureName::Default => consts::DEFAULT_FEATURE_NAME.to_string(), FeatureName::Named(name) => name.clone(), } } @@ -83,7 +83,7 @@ impl<'a> From<&'a FeatureName> for String { impl fmt::Display for FeatureName { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - FeatureName::Default => write!(f, "{}", consts::DEFAULT_ENVIRONMENT_NAME), + FeatureName::Default => write!(f, "{}", consts::DEFAULT_FEATURE_NAME), FeatureName::Named(name) => write!(f, "{}", name), } } diff --git a/src/project/manifest/mod.rs b/src/project/manifest/mod.rs index 172232580..7252d6027 100644 --- a/src/project/manifest/mod.rs +++ b/src/project/manifest/mod.rs @@ -19,7 +19,7 @@ pub use feature::{Feature, FeatureName}; use indexmap::{Equivalent, IndexMap}; use itertools::Itertools; pub use metadata::ProjectMetadata; -use miette::{miette, IntoDiagnostic, LabeledSpan, NamedSource}; +use miette::{miette, Diagnostic, IntoDiagnostic, LabeledSpan, NamedSource}; pub use python::PyPiRequirement; use rattler_conda_types::{ Channel, ChannelConfig, MatchSpec, NamelessMatchSpec, PackageName, Platform, Version, @@ -33,8 +33,16 @@ use std::{ }; pub use system_requirements::{LibCFamilyAndVersion, LibCSystemRequirement, SystemRequirements}; pub use target::{Target, TargetSelector, Targets}; +use thiserror::Error; use toml_edit::{value, Array, Document, Item, Table, TomlError, Value}; +/// Errors that can occur when getting a feature. +#[derive(Debug, Clone, Error, Diagnostic)] +pub enum GetFeatureError { + #[error("feature `{0}` does not exist")] + FeatureDoesNotExist(FeatureName), +} + /// Handles the project's manifest file. /// This struct is responsible for reading, parsing, editing, and saving the manifest. /// It encapsulates all logic related to the manifest's TOML format and structure. @@ -125,8 +133,15 @@ impl Manifest { /// Returns a hashmap of the tasks that should run only the given platform. If the platform is /// `None`, only the default targets tasks are returned. - pub fn tasks(&self, platform: Option) -> HashMap<&str, &Task> { - self.default_feature() + pub fn tasks( + &self, + platform: Option, + feature_name: &FeatureName, + ) -> Result, GetFeatureError> { + Ok(self + .feature(feature_name) + // Return error if feature does not exist + .ok_or(GetFeatureError::FeatureDoesNotExist(feature_name.clone()))? .targets .resolve(platform) .collect_vec() @@ -134,7 +149,7 @@ impl Manifest { .rev() .flat_map(|target| target.tasks.iter()) .map(|(name, task)| (name.as_str(), task)) - .collect() + .collect()) } /// Add a task to the project @@ -146,8 +161,10 @@ impl Manifest { feature_name: &FeatureName, ) -> miette::Result<()> { // Check if the task already exists - if self.tasks(platform).contains_key(name.as_ref()) { - miette::bail!("task {} already exists", name.as_ref()); + if let Ok(tasks) = self.tasks(platform, feature_name) { + if tasks.contains_key(name.as_ref()) { + miette::bail!("task {} already exists", name.as_ref()); + } } // Get the table that contains the tasks. @@ -171,20 +188,22 @@ impl Manifest { &mut self, name: impl AsRef, platform: Option, + feature_name: &FeatureName, ) -> miette::Result<()> { - self.tasks(platform) + self.tasks(platform, feature_name)? .get(name.as_ref()) .ok_or_else(|| miette::miette!("task {} does not exist", name.as_ref()))?; // Get the task table either from the target platform or the default tasks. let tasks_table = - get_or_insert_toml_table(&mut self.document, platform, &FeatureName::Default, "tasks")?; + get_or_insert_toml_table(&mut self.document, platform, feature_name, "tasks")?; // If it does not exist in toml, consider this ok as we want to remove it anyways tasks_table.remove(name.as_ref()); // Remove the task from the internal manifest - self.default_feature_mut() + self.feature_mut(feature_name) + .expect("feature should exist") .targets .for_opt_target_mut(platform.map(TargetSelector::from).as_ref()) .map(|target| target.tasks.remove(name.as_ref())); @@ -499,7 +518,7 @@ impl Manifest { self.parsed.default_feature_mut() } - /// Returns the feature with the given name or `None` if it does not exist. + /// Returns the mutable feature with the given name or `None` if it does not exist. pub fn feature_mut(&mut self, name: &Q) -> Option<&mut Feature> where Q: Hash + Equivalent, @@ -507,6 +526,14 @@ impl Manifest { self.parsed.features.get_mut(name) } + /// Returns the feature with the given name or `None` if it does not exist. + pub fn feature(&self, name: &Q) -> Option<&Feature> + where + Q: Hash + Equivalent, + { + self.parsed.features.get(name) + } + /// Returns the default environment /// /// This is the environment that is added implicitly as the environment with only the default diff --git a/src/project/mod.rs b/src/project/mod.rs index c09115860..5f9b3dbc5 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -404,6 +404,7 @@ impl Display for DependencyKind { #[cfg(test)] mod tests { use super::*; + use crate::project::manifest::FeatureName; use insta::{assert_debug_snapshot, assert_display_snapshot}; use itertools::Itertools; use rattler_virtual_packages::{LibC, VirtualPackage}; @@ -572,8 +573,17 @@ mod tests { let project = Project::from_manifest(manifest); - assert_debug_snapshot!(project.manifest.tasks(Some(Platform::Osx64))); - assert_debug_snapshot!(project.manifest.tasks(Some(Platform::Win64))); - assert_debug_snapshot!(project.manifest.tasks(Some(Platform::Linux64))); + assert_debug_snapshot!(project + .manifest + .tasks(Some(Platform::Osx64), &FeatureName::Default) + .unwrap()); + assert_debug_snapshot!(project + .manifest + .tasks(Some(Platform::Win64), &FeatureName::Default) + .unwrap()); + assert_debug_snapshot!(project + .manifest + .tasks(Some(Platform::Linux64), &FeatureName::Default) + .unwrap()); } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 320c019ee..7ccac8d65 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -332,12 +332,14 @@ impl TasksControl<'_> { &self, name: impl ToString, platform: Option, + feature_name: Option, ) -> miette::Result<()> { task::execute(task::Args { manifest_path: Some(self.pixi.manifest_path()), operation: task::Operation::Remove(task::RemoveArgs { names: vec![name.to_string()], platform, + feature: feature_name, }), }) } diff --git a/tests/task_tests.rs b/tests/task_tests.rs index b1d37d75c..6722bb7a8 100644 --- a/tests/task_tests.rs +++ b/tests/task_tests.rs @@ -21,13 +21,21 @@ pub async fn add_remove_task() { .unwrap(); let project = pixi.project().unwrap(); - let tasks = project.tasks(None); + let tasks = project.default_environment().tasks(None).unwrap(); let task = tasks.get("test").unwrap(); assert!(matches!(task, Task::Plain(s) if s == "echo hello")); // Remove the task - pixi.tasks().remove("test", None).await.unwrap(); - assert_eq!(pixi.project().unwrap().tasks(None).len(), 0); + pixi.tasks().remove("test", None, None).await.unwrap(); + assert_eq!( + pixi.project() + .unwrap() + .default_environment() + .tasks(None) + .unwrap() + .len(), + 0 + ); } #[tokio::test] @@ -49,7 +57,7 @@ pub async fn add_command_types() { .unwrap(); let project = pixi.project().unwrap(); - let tasks = project.tasks(None); + let tasks = project.default_environment().tasks(None).unwrap(); let task2 = tasks.get("test2").unwrap(); let task = tasks.get("test").unwrap(); assert!(matches!(task2, Task::Execute(cmd) if matches!(cmd.cmd, CmdArgs::Single(_)))); @@ -64,11 +72,11 @@ pub async fn add_command_types() { // Create an alias pixi.tasks() .alias("testing", None) - .with_depends_on(["test"]) + .with_depends_on(["test", "test3"]) .execute() .unwrap(); let project = pixi.project().unwrap(); - let tasks = project.tasks(None); + let tasks = project.default_environment().tasks(None).unwrap(); let task = tasks.get("testing").unwrap(); assert!(matches!(task, Task::Alias(a) if a.depends_on.get(0).unwrap() == "test")); } @@ -124,7 +132,12 @@ pub async fn add_remove_target_specific_task() { .unwrap(); let project = pixi.project().unwrap(); - let task = *project.tasks(Some(Platform::Win64)).get("test").unwrap(); + let task = *project + .default_environment() + .tasks(Some(Platform::Win64)) + .unwrap() + .get("test") + .unwrap(); assert!(matches!(task, Task::Plain(s) if s == "echo only_on_windows")); // Simple task @@ -136,11 +149,15 @@ pub async fn add_remove_target_specific_task() { // Remove the task pixi.tasks() - .remove("test", Some(Platform::Win64)) + .remove("test", Some(Platform::Win64), None) .await .unwrap(); assert_eq!( - project.tasks(Some(Platform::Win64)).len(), + project + .default_environment() + .tasks(Some(Platform::Win64)) + .unwrap() + .len(), // The default task is still there 1 );