From dad818098545ee536a45bd1d8c242dffdb22a824 Mon Sep 17 00:00:00 2001 From: Ana Hobden Date: Mon, 12 Dec 2022 12:20:50 -0800 Subject: [PATCH] Allow expected errors (#116) * Move binary out of /nix if it is there during uninstall * Add tracing * Sorta working... * Have expected() return an err * Better handle expected errors during install * Hello trailing whitespace --- src/action/mod.rs | 8 +++++++ src/cli/subcommand/install.rs | 41 +++++++++++++++++++++++++++++---- src/cli/subcommand/plan.rs | 16 +++++++++++-- src/cli/subcommand/uninstall.rs | 38 +++++++++++++++++++++++++++++- src/error.rs | 19 +++++++++++++++ src/planner/linux/multi.rs | 8 ++----- src/planner/mod.rs | 19 +++++++++++++++ 7 files changed, 136 insertions(+), 13 deletions(-) diff --git a/src/action/mod.rs b/src/action/mod.rs index 648a0c0..34fb41b 100644 --- a/src/action/mod.rs +++ b/src/action/mod.rs @@ -160,6 +160,8 @@ pub use stateful::{ActionState, StatefulAction}; use std::error::Error; use tokio::task::JoinError; +use crate::error::HasExpectedErrors; + /// An action which can be reverted or completed, with an action state /// /// This trait interacts with [`StatefulAction`] which does the [`ActionState`] manipulation and provides some tracing facilities. @@ -308,3 +310,9 @@ pub enum ActionError { std::string::FromUtf8Error, ), } + +impl HasExpectedErrors for ActionError { + fn expected(&self) -> Option> { + None + } +} diff --git a/src/cli/subcommand/install.rs b/src/cli/subcommand/install.rs index bbeaa3c..3761bf5 100644 --- a/src/cli/subcommand/install.rs +++ b/src/cli/subcommand/install.rs @@ -6,6 +6,7 @@ use std::{ use crate::{ action::ActionState, cli::{ensure_root, interaction, signal_channel, CommandExecute}, + error::HasExpectedErrors, plan::RECEIPT_LOCATION, planner::Planner, BuiltinPlanner, InstallPlan, @@ -99,7 +100,17 @@ impl CommandExecute for Install { let builtin_planner = BuiltinPlanner::default() .await .map_err(|e| eyre::eyre!(e))?; - builtin_planner.plan().await.map_err(|e| eyre!(e))? + let res = builtin_planner.plan().await; + match res { + Ok(plan) => plan, + Err(e) => { + if let Some(expected) = e.expected() { + eprintln!("{}", expected.red()); + return Ok(ExitCode::FAILURE); + } + return Err(e.into()) + } + } }, (Some(_), Some(_)) => return Err(eyre!("`--plan` conflicts with passing a planner, a planner creates plans, so passing an existing plan doesn't make sense")), }; @@ -120,9 +131,17 @@ impl CommandExecute for Install { let (tx, rx1) = signal_channel().await?; if let Err(err) = install_plan.install(rx1).await { - let error = eyre!(err).wrap_err("Install failure"); if !no_confirm { - tracing::error!("{:?}", error); + let mut was_expected = false; + if let Some(expected) = err.expected() { + was_expected = true; + eprintln!("{}", expected.red()) + } + if !was_expected { + let error = eyre!(err).wrap_err("Install failure"); + tracing::error!("{:?}", error); + }; + if !interaction::confirm( install_plan .describe_uninstall(explain) @@ -134,8 +153,22 @@ impl CommandExecute for Install { interaction::clean_exit_with_message("Okay, didn't do anything! Bye!").await; } let rx2 = tx.subscribe(); - install_plan.uninstall(rx2).await? + let res = install_plan.uninstall(rx2).await; + + if let Err(e) = res { + if let Some(expected) = e.expected() { + eprintln!("{}", expected.red()); + return Ok(ExitCode::FAILURE); + } + return Err(e.into()); + } } else { + if let Some(expected) = err.expected() { + eprintln!("{}", expected.red()); + return Ok(ExitCode::FAILURE); + } + + let error = eyre!(err).wrap_err("Install failure"); return Err(error); } } diff --git a/src/cli/subcommand/plan.rs b/src/cli/subcommand/plan.rs index 3a51547..318ff3b 100644 --- a/src/cli/subcommand/plan.rs +++ b/src/cli/subcommand/plan.rs @@ -1,9 +1,10 @@ use std::{path::PathBuf, process::ExitCode}; -use crate::BuiltinPlanner; +use crate::{error::HasExpectedErrors, BuiltinPlanner}; use clap::Parser; use eyre::WrapErr; +use owo_colors::OwoColorize; use crate::cli::CommandExecute; @@ -29,7 +30,18 @@ impl CommandExecute for Plan { .map_err(|e| eyre::eyre!(e))?, }; - let install_plan = planner.plan().await.map_err(|e| eyre::eyre!(e))?; + let res = planner.plan().await; + + let install_plan = match res { + Ok(plan) => plan, + Err(e) => { + if let Some(expected) = e.expected() { + eprintln!("{}", expected.red()); + return Ok(ExitCode::FAILURE); + } + return Err(e.into()); + }, + }; let json = serde_json::to_string_pretty(&install_plan)?; tokio::fs::write(output, json) diff --git a/src/cli/subcommand/uninstall.rs b/src/cli/subcommand/uninstall.rs index 0f41fcd..bc8781a 100644 --- a/src/cli/subcommand/uninstall.rs +++ b/src/cli/subcommand/uninstall.rs @@ -6,6 +6,7 @@ use std::{ use crate::{ cli::{ensure_root, signal_channel}, + error::HasExpectedErrors, plan::RECEIPT_LOCATION, InstallPlan, }; @@ -93,6 +94,33 @@ impl CommandExecute for Uninstall { } } + // During install, `harmonic` will store a copy of itself in `/nix/harmonic` + // If the user opted to run that particular copy of Harmonic to do this uninstall, + // well, we have a problem, since the binary would delete itself. + // Instead, detect if we're in that location, if so, move the binary and `execv` it. + if let Ok(current_exe) = std::env::current_exe() { + if current_exe.as_path() == Path::new("/nix/harmonic") { + tracing::debug!( + "Detected uninstall from `/nix/harmonic`, moving executable and re-executing" + ); + let temp = std::env::temp_dir(); + let temp_exe = temp.join("harmonic"); + tokio::fs::copy(¤t_exe, &temp_exe) + .await + .wrap_err("Copying harmonic to tempdir")?; + let args = std::env::args(); + let mut arg_vec_cstring = vec![]; + for arg in args { + arg_vec_cstring.push(CString::new(arg).wrap_err("Making arg into C string")?); + } + let temp_exe_cstring = CString::new(temp_exe.to_string_lossy().into_owned()) + .wrap_err("Making C string of executable path")?; + + nix::unistd::execv(&temp_exe_cstring, &arg_vec_cstring) + .wrap_err("Executing copied Harmonic")?; + } + } + let install_receipt_string = tokio::fs::read_to_string(receipt) .await .wrap_err("Reading receipt")?; @@ -111,7 +139,15 @@ impl CommandExecute for Uninstall { let (_tx, rx) = signal_channel().await?; - plan.uninstall(rx).await?; + let res = plan.uninstall(rx).await; + if let Err(e) = res { + if let Some(expected) = e.expected() { + println!("{}", expected.red()); + return Ok(ExitCode::FAILURE); + } + return Err(e.into()); + } + // TODO(@hoverbear): It would be so nice to catch errors and offer the user a way to keep going... // However that will require being able to link error -> step and manually setting that step as `Uncompleted`. diff --git a/src/error.rs b/src/error.rs index 658ec0f..544cd53 100644 --- a/src/error.rs +++ b/src/error.rs @@ -54,3 +54,22 @@ pub enum HarmonicError { InstallSettingsError, ), } + +pub(crate) trait HasExpectedErrors { + fn expected<'a>(&'a self) -> Option>; +} + +impl HasExpectedErrors for HarmonicError { + fn expected<'a>(&'a self) -> Option> { + match self { + HarmonicError::Action(action_error) => action_error.expected(), + HarmonicError::RecordingReceipt(_, _) => None, + HarmonicError::CopyingSelf(_) => None, + HarmonicError::SerializingReceipt(_) => None, + HarmonicError::Cancelled => None, + HarmonicError::SemVer(_) => None, + HarmonicError::Planner(planner_error) => planner_error.expected(), + HarmonicError::InstallSettings(_) => None, + } + } +} diff --git a/src/planner/linux/multi.rs b/src/planner/linux/multi.rs index 911b6fe..2975d41 100644 --- a/src/planner/linux/multi.rs +++ b/src/planner/linux/multi.rs @@ -33,7 +33,7 @@ impl Planner for LinuxMulti { // If on NixOS, running `harmonic` is pointless // NixOS always sets up this file as part of setting up /etc itself: https://github.com/NixOS/nixpkgs/blob/bdd39e5757d858bd6ea58ed65b4a2e52c8ed11ca/nixos/modules/system/etc/setup-etc.pl#L145 if Path::new("/etc/NIXOS").exists() { - return Err(PlannerError::Custom(Box::new(LinuxMultiError::NixOs))); + return Err(PlannerError::NixOs); } // For now, we don't try to repair the user's Nix install or anything special. @@ -43,7 +43,7 @@ impl Planner for LinuxMulti { .status() .await { - return Err(PlannerError::Custom(Box::new(LinuxMultiError::NixExists))); + return Err(PlannerError::NixExists); } Ok(vec![ @@ -84,10 +84,6 @@ impl Into for LinuxMulti { #[derive(thiserror::Error, Debug)] enum LinuxMultiError { - #[error("NixOS already has Nix installed")] - NixOs, - #[error("`nix` is already a valid command, so it is installed")] - NixExists, #[error("Error planning action")] Action( #[source] diff --git a/src/planner/mod.rs b/src/planner/mod.rs index 9c67cb6..90dd5fa 100644 --- a/src/planner/mod.rs +++ b/src/planner/mod.rs @@ -81,6 +81,7 @@ use std::collections::HashMap; use crate::{ action::{ActionError, StatefulAction}, + error::HasExpectedErrors, settings::InstallSettingsError, Action, HarmonicError, InstallPlan, }; @@ -181,4 +182,22 @@ pub enum PlannerError { /// Custom planner error #[error("Custom planner error")] Custom(#[source] Box), + #[error("NixOS already has Nix installed")] + NixOs, + #[error("`nix` is already a valid command, so it is installed")] + NixExists, +} + +impl HasExpectedErrors for PlannerError { + fn expected<'a>(&'a self) -> Option> { + match self { + this @ PlannerError::UnsupportedArchitecture(_) => Some(Box::new(this)), + PlannerError::Action(_) => None, + PlannerError::InstallSettings(_) => None, + PlannerError::Plist(_) => None, + PlannerError::Custom(_) => None, + this @ PlannerError::NixOs => Some(Box::new(this)), + this @ PlannerError::NixExists => Some(Box::new(this)), + } + } }