fix: project platform remove (#3017)

Author: Hofer-Julian

crates/pixi_manifest/src/manifests/manifest.rs

@@ -352,35 +352,41 @@ impl Manifest {
     /// Remove the platform(s) from the project
     pub fn remove_platforms(
         &mut self,
-        platforms: impl IntoIterator<Item = Platform>,
+        platforms: impl IntoIterator<Item = Platform> + Clone,
         feature_name: &FeatureName,
     ) -> miette::Result<()> {
-        // Get current platforms and platform to remove for the feature
+        // Get current platforms for the feature
         let current = match feature_name {
             FeatureName::Default => self.workspace.workspace.platforms.get_mut(),
             FeatureName::Named(_) => self.feature_mut(feature_name)?.platforms_mut(),
         };
-        // Get the platforms to remove, while checking if they exist
-        let to_remove: IndexSet<_> = platforms
-            .into_iter()
-            .map(|c| {
-                current
-                    .iter()
-                    .position(|x| *x == c)
-                    .ok_or_else(|| miette::miette!("platform {} does not exist", c))
-                    .map(|_| c)
-            })
-            .collect::<Result<_, _>>()?;
 
-        let retained: IndexSet<_> = current.difference(&to_remove).cloned().collect();
+        // Check if some platforms are not part of current
+        let missing = platforms
+            .clone()
+            .into_iter()
+            .filter(|p| !current.contains(p))
+            .collect_vec();
+        if !missing.is_empty() {
+            return Err(miette::miette!(
+                "The following platform{} are not part of {}: {}",
+                if missing.len() > 1 { "s are" } else { " is" },
+                feature_name,
+                missing.into_iter().join(", ")
+            ));
+        }
 
         // Remove platforms from the manifest
-        current.retain(|p| retained.contains(p));
+        current.retain(|p| !platforms.clone().into_iter().contains(p));
 
         // And from the TOML document
-        let retained = retained.iter().map(|p| p.to_string()).collect_vec();
+        let retained = current.iter().map(|p| p.to_string()).collect_vec();
         let platforms = self.source.get_array_mut("platforms", feature_name)?;
-        platforms.retain(|x| retained.contains(&x.to_string()));
+        platforms.retain(|p| {
+            p.as_str()
+                .map(|p| retained.contains(&p.to_string()))
+                .unwrap_or(false)
+        });
 
         Ok(())
     }

src/cli/project/platform/remove.rs

@@ -1,20 +1,17 @@
-use std::str::FromStr;
-
 use crate::lock_file::UpdateMode;
 use crate::{
     environment::{get_update_lock_file_and_prefix, LockFileUsage},
     Project, UpdateLockFileOptions,
 };
 use clap::Parser;
-use miette::IntoDiagnostic;
 use pixi_manifest::FeatureName;
 use rattler_conda_types::Platform;
 
 #[derive(Parser, Debug, Default)]
 pub struct Args {
     /// The platform name(s) to remove.
     #[clap(required = true, num_args=1..)]
-    pub platform: Vec<String>,
+    pub platforms: Vec<Platform>,
 
     /// Don't update the environment, only remove the platform(s) from the
     /// lock-file.
@@ -31,18 +28,10 @@ pub async fn execute(mut project: Project, args: Args) -> miette::Result<()> {
         .feature
         .map_or(FeatureName::Default, FeatureName::Named);
 
-    // Determine which platforms to remove
-    let platforms = args
-        .platform
-        .into_iter()
-        .map(|platform_str| Platform::from_str(&platform_str))
-        .collect::<Result<Vec<_>, _>>()
-        .into_diagnostic()?;
-
     // Remove the platform(s) from the manifest
     project
         .manifest
-        .remove_platforms(platforms.clone(), &feature_name)?;
+        .remove_platforms(args.platforms.clone(), &feature_name)?;
 
     get_update_lock_file_and_prefix(
         &project.default_environment(),
@@ -57,7 +46,7 @@ pub async fn execute(mut project: Project, args: Args) -> miette::Result<()> {
     project.save()?;
 
     // Report back to the user
-    for platform in platforms {
+    for platform in args.platforms {
         eprintln!(
             "{}Removed {}",
             console::style(console::Emoji("✔ ", "")).green(),

tests/integration_python/test_main_cli.py

@@ -116,13 +116,14 @@ def test_project_commands(pixi: Path, tmp_pixi_workspace: Path) -> None:
             "platform",
             "add",
             "linux-64",
+            "osx-arm64",
         ],
         ExitCode.SUCCESS,
     )
     verify_cli_command(
         [pixi, "project", "--manifest-path", manifest_path, "platform", "list"],
         ExitCode.SUCCESS,
-        stdout_contains="linux-64",
+        stdout_contains=["linux-64", "osx-arm64"],
     )
     verify_cli_command(
         [
@@ -136,9 +137,23 @@ def test_project_commands(pixi: Path, tmp_pixi_workspace: Path) -> None:
         ],
         ExitCode.SUCCESS,
     )
+    verify_cli_command(
+        [
+            pixi,
+            "project",
+            "--manifest-path",
+            manifest_path,
+            "platform",
+            "remove",
+            "linux-64",
+        ],
+        ExitCode.FAILURE,
+        stderr_contains="linux-64",
+    )
     verify_cli_command(
         [pixi, "project", "--manifest-path", manifest_path, "platform", "list"],
         ExitCode.SUCCESS,
+        stdout_contains="osx-arm64",
         stdout_excludes="linux-64",
     )