diff --git a/crates/pixi_cli/src/global/install.rs b/crates/pixi_cli/src/global/install.rs index 8a42701745..d8cdb880cf 100644 --- a/crates/pixi_cli/src/global/install.rs +++ b/crates/pixi_cli/src/global/install.rs @@ -5,7 +5,7 @@ use indexmap::IndexMap; use clap::Parser; use fancy_display::FancyDisplay; use itertools::Itertools; -use miette::{Context, IntoDiagnostic}; +use miette::{Context, IntoDiagnostic, Report}; use rattler_conda_types::{MatchSpec, NamedChannelOrUrl, Platform}; use crate::global::{global_specs::GlobalSpecs, revert_environment_after_error}; @@ -119,14 +119,12 @@ pub async fn execute(args: Args) -> miette::Result<()> { let mut env_changes = EnvChanges::default(); let mut last_updated_project = project_original; + let mut errors: Vec<(EnvironmentName, Report)> = Vec::new(); // Convert the packages into named global specs for (env_name, specs) in &env_to_specs { let mut project = last_updated_project.clone(); - match setup_environment(env_name, &args, specs, &mut project) - .await - .wrap_err_with(|| format!("Couldn't install {}", env_name.fancy_display())) - { + match setup_environment(env_name, &args, specs, &mut project).await { Ok(state_changes) => { if state_changes.has_changed() { env_changes @@ -138,6 +136,8 @@ pub async fn execute(args: Args) -> miette::Result<()> { EnvState::NotChanged(NotChangedReason::AlreadyInstalled), ) }; + // Only advance project on success + last_updated_project = project; } Err(err) => { if let Err(revert_err) = @@ -146,10 +146,9 @@ pub async fn execute(args: Args) -> miette::Result<()> { tracing::warn!("Reverting of the operation failed"); tracing::info!("Reversion error: {:?}", revert_err); } - return Err(err); + errors.push((env_name.clone(), err)); } } - last_updated_project = project; } // After installing, we always want to list the changed environments @@ -162,7 +161,17 @@ pub async fn execute(args: Args) -> miette::Result<()> { ) .await?; - Ok(()) + if errors.is_empty() { + Ok(()) + } else { + for (env_name, err) in errors { + tracing::warn!( + "Couldn't install {}\n{err:?}", + env_name.fancy_display() + ); + } + Err(miette::miette!("Some environments couldn't be installed.")) + } } async fn setup_environment( diff --git a/crates/pixi_cli/src/global/uninstall.rs b/crates/pixi_cli/src/global/uninstall.rs index 79c07b1973..da7b151eb8 100644 --- a/crates/pixi_cli/src/global/uninstall.rs +++ b/crates/pixi_cli/src/global/uninstall.rs @@ -1,7 +1,7 @@ use crate::global::revert_environment_after_error; use clap::Parser; use fancy_display::FancyDisplay; -use miette::Context; +use miette::Report; use pixi_config::{Config, ConfigCli}; use pixi_global::StateChanges; use pixi_global::{EnvironmentName, Project}; @@ -37,27 +37,35 @@ pub async fn execute(args: Args) -> miette::Result<()> { Ok(state_changes) } + let mut errors: Vec<(EnvironmentName, Report)> = Vec::new(); let mut last_updated_project = project_original; for env_name in &args.environment { let mut project = last_updated_project.clone(); - match apply_changes(env_name, &mut project) - .await - .wrap_err_with(|| format!("Couldn't remove {}", env_name.fancy_display())) - { + match apply_changes(env_name, &mut project).await { Ok(state_changes) => { state_changes.report(); + // Only advance the project when successful + last_updated_project = project; } Err(err) => { + // Revert any partial change for this environment, then continue if let Err(revert_err) = revert_environment_after_error(env_name, &last_updated_project).await { tracing::warn!("Reverting of the operation failed"); tracing::info!("Reversion error: {:?}", revert_err); } - return Err(err); + errors.push((env_name.clone(), err)); } } - last_updated_project = project; } - Ok(()) + + if errors.is_empty() { + Ok(()) + } else { + for (env_name, err) in errors { + tracing::warn!("Couldn't remove {}\n{err:?}", env_name.fancy_display()); + } + Err(miette::miette!("Some environments couldn't be removed.")) + } } diff --git a/tests/integration_python/pixi_global/test_global.py b/tests/integration_python/pixi_global/test_global.py index 5a74983c9d..81fb02936d 100644 --- a/tests/integration_python/pixi_global/test_global.py +++ b/tests/integration_python/pixi_global/test_global.py @@ -1122,6 +1122,36 @@ def test_install_only_reverts_failing(pixi: Path, tmp_path: Path, dummy_channel_ assert not dummy_x.is_file() +def test_install_continues_past_failing_env( + pixi: Path, tmp_path: Path, dummy_channel_1: str +) -> None: + env = {"PIXI_HOME": str(tmp_path)} + + dummy_a = tmp_path / "bin" / exec_extension("dummy-a") + dummy_b = tmp_path / "bin" / exec_extension("dummy-b") + + # Order matters: fail in the middle (dummy-x), ensure it continues to install later packages + verify_cli_command( + [ + pixi, + "global", + "install", + "--channel", + dummy_channel_1, + "dummy-a", + "dummy-x", # does not exist in dummy_channel_1 + "dummy-b", + ], + ExitCode.FAILURE, + env=env, + stderr_contains="No candidates were found for dummy-x", + ) + + # Both valid packages are installed despite the error in between + assert dummy_a.is_file() + assert dummy_b.is_file() + + @pytest.mark.slow def test_install_platform(pixi: Path, tmp_path: Path) -> None: env = {"PIXI_HOME": str(tmp_path)} @@ -1495,6 +1525,38 @@ def test_uninstall_only_reverts_failing(pixi: Path, tmp_path: Path, dummy_channe assert tmp_path.joinpath("envs", "dummy-b").is_dir() +def test_uninstall_continues_past_missing_env( + pixi: Path, tmp_path: Path, dummy_channel_1: str +) -> None: + env = {"PIXI_HOME": str(tmp_path)} + + dummy_a = tmp_path / "bin" / exec_extension("dummy-a") + dummy_b = tmp_path / "bin" / exec_extension("dummy-b") + + # Install two environments + verify_cli_command( + [pixi, "global", "install", "--channel", dummy_channel_1, "dummy-a", "dummy-b"], + env=env, + ) + assert dummy_a.is_file() + assert dummy_b.is_file() + + # Uninstall with a missing environment in between; should continue and remove both existing ones + missing_env = "does-not-exist" + verify_cli_command( + [pixi, "global", "uninstall", "dummy-a", missing_env, "dummy-b"], + ExitCode.FAILURE, + env=env, + stderr_contains=f"Environment {missing_env} doesn't exist", + ) + + # Both existing environments are removed despite the error in between + assert not dummy_a.is_file() + assert not tmp_path.joinpath("envs", "dummy-a").is_dir() + assert not dummy_b.is_file() + assert not tmp_path.joinpath("envs", "dummy-b").is_dir() + + def test_global_update_single_package( pixi: Path, tmp_path: Path, multiple_versions_channel_1: str ) -> None: