Skip to content

Commit

Permalink
Feature/add feature env to task cli (#694)
Browse files Browse the repository at this point in the history
Adds:
```
pixi task remove -f/--feature test 
pixi task list -e/--environment test
```
  • Loading branch information
ruben-arts authored Jan 23, 2024
1 parent 5d50682 commit 3f447f4
Show file tree
Hide file tree
Showing 11 changed files with 142 additions and 48 deletions.
8 changes: 6 additions & 2 deletions src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -90,6 +91,9 @@ enum TaskExecutionError {

#[error(transparent)]
TraverseError(#[from] TraversalError),

#[error(transparent)]
UnsupportedPlatformError(#[from] UnsupportedPlatformError),
}

/// Called to execute a single command.
Expand Down Expand Up @@ -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();
Expand Down
40 changes: 31 additions & 9 deletions src/cli/task.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -35,6 +37,10 @@ pub struct RemoveArgs {
/// The platform for which the task should be removed
#[arg(long, short)]
pub platform: Option<Platform>,

/// The feature for which the task should be removed
#[arg(long, short)]
pub feature: Option<String>,
}

#[derive(Parser, Debug, Clone)]
Expand Down Expand Up @@ -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<String>,
}

impl From<AddArgs> for Task {
Expand Down Expand Up @@ -153,19 +164,22 @@ 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,
);
}
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!(
Expand All @@ -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;
}
Expand All @@ -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(),
);
Expand All @@ -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() {
Expand Down
2 changes: 2 additions & 0 deletions src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
2 changes: 1 addition & 1 deletion src/project/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down
15 changes: 7 additions & 8 deletions src/project/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Platform>,

/// The environment that the platform is not supported for.
pub environment: EnvironmentName,
Expand All @@ -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 => {
Expand All @@ -39,16 +39,15 @@ impl<'p> Display for UnsupportedPlatformError<'p> {
}
}

impl<'p> Diagnostic for UnsupportedPlatformError<'p> {
impl Diagnostic for UnsupportedPlatformError {
fn code(&self) -> Option<Box<dyn Display + '_>> {
Some(Box::new("unsupported-platform".to_string()))
}

fn help(&self) -> Option<Box<dyn Display + '_>> {
let env = self.project.environment(&self.environment)?;
Some(Box::new(format!(
"supported platforms are {}",
env.platforms().into_iter().format(", ")
self.environments_platforms.iter().format(", ")
)))
}

Expand Down
11 changes: 11 additions & 0 deletions src/project/manifest/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -38,6 +39,16 @@ impl Borrow<str> 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 {
Expand Down
12 changes: 6 additions & 6 deletions src/project/manifest/feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())),
Expand All @@ -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()),
}
}
Expand All @@ -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)
}
}

Expand All @@ -67,23 +67,23 @@ impl Borrow<str> for FeatureName {
impl From<FeatureName> 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,
}
}
}
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(),
}
}
}
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),
}
}
Expand Down
47 changes: 37 additions & 10 deletions src/project/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -125,16 +133,23 @@ 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<Platform>) -> HashMap<&str, &Task> {
self.default_feature()
pub fn tasks(
&self,
platform: Option<Platform>,
feature_name: &FeatureName,
) -> Result<HashMap<&str, &Task>, 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()
.into_iter()
.rev()
.flat_map(|target| target.tasks.iter())
.map(|(name, task)| (name.as_str(), task))
.collect()
.collect())
}

/// Add a task to the project
Expand All @@ -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.
Expand All @@ -171,20 +188,22 @@ impl Manifest {
&mut self,
name: impl AsRef<str>,
platform: Option<Platform>,
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()));
Expand Down Expand Up @@ -499,14 +518,22 @@ 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<Q: ?Sized>(&mut self, name: &Q) -> Option<&mut Feature>
where
Q: Hash + Equivalent<FeatureName>,
{
self.parsed.features.get_mut(name)
}

/// Returns the feature with the given name or `None` if it does not exist.
pub fn feature<Q: ?Sized>(&self, name: &Q) -> Option<&Feature>
where
Q: Hash + Equivalent<FeatureName>,
{
self.parsed.features.get(name)
}

/// Returns the default environment
///
/// This is the environment that is added implicitly as the environment with only the default
Expand Down
Loading

0 comments on commit 3f447f4

Please sign in to comment.