From 903258942cac252c4f36fe8bea99b6de93865f2c Mon Sep 17 00:00:00 2001 From: Ana Hobden Date: Fri, 3 Mar 2023 14:20:17 -0800 Subject: [PATCH] Add more failure context / Improve error structure (#296) * wip: add more context to errors * Add a bunch fo context * Repair source handling * Add remaining contexts * Add some context, but some of it is not right... * Tidy up contexts properly * Get command errors working how I want * Remove some debug statements * Repair mac build * Move typetag to Action * newtypes! * Fix doctest --- Cargo.lock | 1 + Cargo.toml | 2 +- src/action/base/add_user_to_group.rs | 42 ++-- src/action/base/create_directory.rs | 3 + src/action/base/create_file.rs | 5 +- src/action/base/create_group.rs | 17 +- src/action/base/create_or_insert_into_file.rs | 5 +- src/action/base/create_user.rs | 47 ++--- src/action/base/fetch_and_unpack_nix.rs | 5 +- src/action/base/move_unpacked_nix.rs | 5 +- src/action/base/setup_default_profile.rs | 36 ++-- src/action/common/configure_init_service.rs | 69 +++---- src/action/common/configure_nix.rs | 80 ++++++-- src/action/common/configure_shell_profile.rs | 66 ++++--- src/action/common/create_nix_tree.rs | 29 ++- src/action/common/create_users_and_groups.rs | 39 +++- .../common/place_channel_configuration.rs | 28 +-- src/action/common/place_nix_configuration.rs | 44 +++-- src/action/common/provision_nix.rs | 79 +++++--- src/action/linux/start_systemd_unit.rs | 26 +-- src/action/macos/bootstrap_apfs_volume.rs | 18 +- src/action/macos/create_apfs_volume.rs | 14 +- src/action/macos/create_fstab_entry.rs | 8 +- src/action/macos/create_nix_volume.rs | 181 +++++++++++------- src/action/macos/create_synthetic_objects.rs | 5 +- src/action/macos/enable_ownership.rs | 11 +- src/action/macos/encrypt_apfs_volume.rs | 22 +-- src/action/macos/unmount_apfs_volume.rs | 11 +- src/action/mod.rs | 136 ++++++++++++- src/action/stateful.rs | 11 +- src/bin/nix-installer.rs | 3 +- src/cli/subcommand/install.rs | 10 +- src/cli/subcommand/plan.rs | 10 +- src/cli/subcommand/uninstall.rs | 6 +- src/diagnostics.rs | 62 +++++- src/error.rs | 36 +++- src/lib.rs | 25 +-- src/plan.rs | 18 +- src/planner/mod.rs | 8 + src/settings.rs | 10 +- tests/fixtures/linux/linux.json | 2 +- tests/fixtures/linux/steam-deck.json | 2 +- tests/fixtures/macos/macos.json | 2 +- 43 files changed, 786 insertions(+), 453 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5608e4a..c3a561e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -203,6 +203,7 @@ dependencies = [ "once_cell", "owo-colors", "tracing-error", + "url", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 15a50f1..b742b8c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ async-trait = { version = "0.1.57", default-features = false } atty = { version = "0.2.14", default-features = false, optional = true } bytes = { version = "1.2.1", default-features = false, features = ["std", "serde"] } clap = { version = "4", features = ["std", "color", "usage", "help", "error-context", "suggestions", "derive", "env"], optional = true } -color-eyre = { version = "0.6.2", default-features = false, features = [ "track-caller", "tracing-error", "capture-spantrace", "color-spantrace" ], optional = true } +color-eyre = { version = "0.6.2", default-features = false, features = [ "track-caller", "issue-url", "tracing-error", "capture-spantrace", "color-spantrace" ], optional = true } eyre = { version = "0.6.8", default-features = false, features = [ "track-caller" ], optional = true } glob = { version = "0.3.0", default-features = false } nix = { version = "0.26.0", default-features = false, features = ["user", "fs", "process", "term"] } diff --git a/src/action/base/add_user_to_group.rs b/src/action/base/add_user_to_group.rs index 6d33043..444be54 100644 --- a/src/action/base/add_user_to_group.rs +++ b/src/action/base/add_user_to_group.rs @@ -70,9 +70,11 @@ impl AddUserToGroup { command.arg(&this.groupname); command.stdout(Stdio::piped()); command.stderr(Stdio::piped()); - let command_str = format!("{:?}", command.as_std()); - tracing::trace!("Executing `{command_str}`"); - let output = command.output().await.map_err(ActionError::Command)?; + tracing::trace!("Executing `{:?}`", command.as_std()); + let output = command + .output() + .await + .map_err(|e| ActionError::command(&command, e))?; match output.status.code() { Some(0) => { // yes {user} is a member of {groupname} @@ -96,18 +98,7 @@ impl AddUserToGroup { }, _ => { // Some other issue - return Err(ActionError::Command(std::io::Error::new( - std::io::ErrorKind::Other, - format!( - "Command `{command_str}` failed{}, stderr:\n{}\n", - if let Some(code) = output.status.code() { - format!(" status {code}") - } else { - "".to_string() - }, - String::from_utf8_lossy(&output.stderr), - ), - ))); + return Err(ActionError::command_output(&command, output)); }, }; }, @@ -118,8 +109,7 @@ impl AddUserToGroup { .arg(&this.name) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; let output_str = String::from_utf8(output.stdout)?; let user_in_group = output_str.split(" ").any(|v| v == &this.groupname); @@ -138,6 +128,9 @@ impl AddUserToGroup { #[async_trait::async_trait] #[typetag::serde(name = "add_user_to_group")] impl Action for AddUserToGroup { + fn action_tag() -> crate::action::ActionTag { + crate::action::ActionTag("add_user_to_group") + } fn tracing_synopsis(&self) -> String { format!( "Add user `{}` (UID {}) to group `{}` (GID {})", @@ -194,8 +187,7 @@ impl Action for AddUserToGroup { .arg(&name) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; execute_command( Command::new("/usr/sbin/dseditgroup") .process_group(0) @@ -207,8 +199,7 @@ impl Action for AddUserToGroup { .arg(groupname) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; }, _ => { execute_command( @@ -218,8 +209,7 @@ impl Action for AddUserToGroup { .args([&name.to_string(), &groupname.to_string()]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; }, } @@ -262,8 +252,7 @@ impl Action for AddUserToGroup { .arg(&name) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; }, _ => { execute_command( @@ -273,8 +262,7 @@ impl Action for AddUserToGroup { .args([&name.to_string(), &groupname.to_string()]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; }, }; diff --git a/src/action/base/create_directory.rs b/src/action/base/create_directory.rs index 4c592a6..915e737 100644 --- a/src/action/base/create_directory.rs +++ b/src/action/base/create_directory.rs @@ -99,6 +99,9 @@ impl CreateDirectory { #[async_trait::async_trait] #[typetag::serde(name = "create_directory")] impl Action for CreateDirectory { + fn action_tag() -> crate::action::ActionTag { + crate::action::ActionTag("create_directory") + } fn tracing_synopsis(&self) -> String { format!("Create directory `{}`", self.path.display()) } diff --git a/src/action/base/create_file.rs b/src/action/base/create_file.rs index a9d5f9a..baffd1e 100644 --- a/src/action/base/create_file.rs +++ b/src/action/base/create_file.rs @@ -10,7 +10,7 @@ use tokio::{ io::{AsyncReadExt, AsyncWriteExt}, }; -use crate::action::{Action, ActionDescription, ActionError, StatefulAction}; +use crate::action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction}; /** Create a file at the given location with the provided `buf`, optionally with an owning user, group, and mode. @@ -134,6 +134,9 @@ impl CreateFile { #[async_trait::async_trait] #[typetag::serde(name = "create_file")] impl Action for CreateFile { + fn action_tag() -> ActionTag { + ActionTag("create_file") + } fn tracing_synopsis(&self) -> String { format!("Create or overwrite file `{}`", self.path.display()) } diff --git a/src/action/base/create_group.rs b/src/action/base/create_group.rs index 082d3dc..b16c83b 100644 --- a/src/action/base/create_group.rs +++ b/src/action/base/create_group.rs @@ -2,7 +2,7 @@ use nix::unistd::Group; use tokio::process::Command; use tracing::{span, Span}; -use crate::action::ActionError; +use crate::action::{ActionError, ActionTag}; use crate::execute_command; use crate::action::{Action, ActionDescription, StatefulAction}; @@ -45,6 +45,9 @@ impl CreateGroup { #[async_trait::async_trait] #[typetag::serde(name = "create_group")] impl Action for CreateGroup { + fn action_tag() -> ActionTag { + ActionTag("create_group") + } fn tracing_synopsis(&self) -> String { format!("Create group `{}` (GID {})", self.name, self.gid) } @@ -93,8 +96,7 @@ impl Action for CreateGroup { ]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; }, _ => { execute_command( @@ -103,8 +105,7 @@ impl Action for CreateGroup { .args(["-g", &gid.to_string(), "--system", &name]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; }, }; @@ -138,8 +139,7 @@ impl Action for CreateGroup { .args([".", "-delete", &format!("/Groups/{name}")]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; if !output.status.success() {} }, _ => { @@ -149,8 +149,7 @@ impl Action for CreateGroup { .arg(&name) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; }, }; diff --git a/src/action/base/create_or_insert_into_file.rs b/src/action/base/create_or_insert_into_file.rs index 372a472..9afe1f6 100644 --- a/src/action/base/create_or_insert_into_file.rs +++ b/src/action/base/create_or_insert_into_file.rs @@ -1,6 +1,6 @@ use nix::unistd::{chown, Group, User}; -use crate::action::{Action, ActionDescription, ActionError, StatefulAction}; +use crate::action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction}; use rand::Rng; use std::{ io::SeekFrom, @@ -140,6 +140,9 @@ impl CreateOrInsertIntoFile { #[async_trait::async_trait] #[typetag::serde(name = "create_or_insert_into_file")] impl Action for CreateOrInsertIntoFile { + fn action_tag() -> ActionTag { + ActionTag("create_or_insert_into_file") + } fn tracing_synopsis(&self) -> String { format!("Create or insert file `{}`", self.path.display()) } diff --git a/src/action/base/create_user.rs b/src/action/base/create_user.rs index c334d6c..3bac3a6 100644 --- a/src/action/base/create_user.rs +++ b/src/action/base/create_user.rs @@ -2,7 +2,7 @@ use nix::unistd::User; use tokio::process::Command; use tracing::{span, Span}; -use crate::action::ActionError; +use crate::action::{ActionError, ActionTag}; use crate::execute_command; use crate::action::{Action, ActionDescription, StatefulAction}; @@ -60,6 +60,9 @@ impl CreateUser { #[async_trait::async_trait] #[typetag::serde(name = "create_user")] impl Action for CreateUser { + fn action_tag() -> ActionTag { + ActionTag("create_user") + } fn tracing_synopsis(&self) -> String { format!( "Create user `{}` (UID {}) in group `{}` (GID {})", @@ -110,8 +113,7 @@ impl Action for CreateUser { .args([".", "-create", &format!("/Users/{name}")]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; execute_command( Command::new("/usr/bin/dscl") .process_group(0) @@ -124,8 +126,7 @@ impl Action for CreateUser { ]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; execute_command( Command::new("/usr/bin/dscl") .process_group(0) @@ -138,8 +139,7 @@ impl Action for CreateUser { ]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; execute_command( Command::new("/usr/bin/dscl") .process_group(0) @@ -152,8 +152,7 @@ impl Action for CreateUser { ]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; execute_command( Command::new("/usr/bin/dscl") .process_group(0) @@ -166,16 +165,14 @@ impl Action for CreateUser { ]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; execute_command( Command::new("/usr/bin/dscl") .process_group(0) .args([".", "-create", &format!("/Users/{name}"), "IsHidden", "1"]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; }, _ => { execute_command( @@ -202,8 +199,7 @@ impl Action for CreateUser { ]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; }, } @@ -251,7 +247,7 @@ impl Action for CreateUser { let output = command .output() .await - .map_err(|e| ActionError::Command(e))?; + .map_err(|e| ActionError::command(&command, e))?; let stderr = String::from_utf8_lossy(&output.stderr); match output.status.code() { Some(0) => (), @@ -260,21 +256,9 @@ impl Action for CreateUser { // These Macs cannot always delete users, as sometimes there is no graphical login tracing::warn!("Encountered an exit code 40 with -14120 error while removing user, this is likely because the initial executing user did not have a secure token, or that there was no graphical login session. To delete the user, log in graphically, then run `/usr/bin/dscl . -delete /Users/{name}"); }, - status => { - let command_str = format!("{:?}", command.as_std()); + _ => { // Something went wrong - return Err(ActionError::Command(std::io::Error::new( - std::io::ErrorKind::Other, - format!( - "Command `{command_str}` failed{}, stderr:\n{}\n", - if let Some(status) = status { - format!(" {status}") - } else { - "".to_string() - }, - stderr - ), - ))); + return Err(ActionError::command_output(&command, output)); }, } }, @@ -285,8 +269,7 @@ impl Action for CreateUser { .args([&name.to_string()]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; }, }; diff --git a/src/action/base/fetch_and_unpack_nix.rs b/src/action/base/fetch_and_unpack_nix.rs index 536d8e9..65635e1 100644 --- a/src/action/base/fetch_and_unpack_nix.rs +++ b/src/action/base/fetch_and_unpack_nix.rs @@ -4,7 +4,7 @@ use bytes::{Buf, Bytes}; use reqwest::Url; use tracing::{span, Span}; -use crate::action::{Action, ActionDescription, ActionError, StatefulAction}; +use crate::action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction}; /** Fetch a URL to the given path @@ -37,6 +37,9 @@ impl FetchAndUnpackNix { #[async_trait::async_trait] #[typetag::serde(name = "fetch_and_unpack_nix")] impl Action for FetchAndUnpackNix { + fn action_tag() -> ActionTag { + ActionTag("fetch_and_unpack_nix") + } fn tracing_synopsis(&self) -> String { format!("Fetch `{}` to `{}`", self.url, self.dest.display()) } diff --git a/src/action/base/move_unpacked_nix.rs b/src/action/base/move_unpacked_nix.rs index b8a21ab..e4c6d2b 100644 --- a/src/action/base/move_unpacked_nix.rs +++ b/src/action/base/move_unpacked_nix.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use tracing::{span, Span}; -use crate::action::{Action, ActionDescription, ActionError, StatefulAction}; +use crate::action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction}; pub(crate) const DEST: &str = "/nix/"; @@ -25,6 +25,9 @@ impl MoveUnpackedNix { #[async_trait::async_trait] #[typetag::serde(name = "mount_unpacked_nix")] impl Action for MoveUnpackedNix { + fn action_tag() -> ActionTag { + ActionTag("move_unpacked_nix") + } fn tracing_synopsis(&self) -> String { "Move the downloaded Nix into `/nix`".to_string() } diff --git a/src/action/base/setup_default_profile.rs b/src/action/base/setup_default_profile.rs index 39ac403..8ab21fb 100644 --- a/src/action/base/setup_default_profile.rs +++ b/src/action/base/setup_default_profile.rs @@ -1,7 +1,7 @@ use std::path::{Path, PathBuf}; use crate::{ - action::{ActionError, StatefulAction}, + action::{ActionError, ActionTag, StatefulAction}, execute_command, set_env, ChannelValue, }; @@ -30,6 +30,9 @@ impl SetupDefaultProfile { #[async_trait::async_trait] #[typetag::serde(name = "setup_default_profile")] impl Action for SetupDefaultProfile { + fn action_tag() -> ActionTag { + ActionTag("setup_default_profile") + } fn tracing_synopsis(&self) -> String { "Setup the default Nix profile".to_string() } @@ -119,14 +122,14 @@ impl Action for SetupDefaultProfile { ActionError::Custom(Box::new(SetupDefaultProfileError::NoRootHome)) })?, ); - let load_db_command_str = format!("{:?}", load_db_command.as_std()); tracing::trace!( - "Executing `{load_db_command_str}` with stdin from `{}`", + "Executing `{:?}` with stdin from `{}`", + load_db_command.as_std(), reginfo_path.display() ); let mut handle = load_db_command .spawn() - .map_err(|e| ActionError::Command(e))?; + .map_err(|e| ActionError::command(&load_db_command, e))?; let mut stdin = handle.stdin.take().unwrap(); stdin @@ -146,20 +149,9 @@ impl Action for SetupDefaultProfile { let output = handle .wait_with_output() .await - .map_err(ActionError::Command)?; + .map_err(|e| ActionError::command(&load_db_command, e))?; if !output.status.success() { - return Err(ActionError::Command(std::io::Error::new( - std::io::ErrorKind::Other, - format!( - "Command `{load_db_command_str}` failed{}, stderr:\n{}\n", - if let Some(code) = output.status.code() { - format!(" status {code}") - } else { - "".to_string() - }, - String::from_utf8_lossy(&output.stderr) - ), - ))); + return Err(ActionError::command_output(&load_db_command, output)); }; } @@ -181,8 +173,7 @@ impl Action for SetupDefaultProfile { nss_ca_cert_pkg.join("etc/ssl/certs/ca-bundle.crt"), ), /* This is apparently load bearing... */ ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; // Install `nix` itself into the store execute_command( @@ -202,8 +193,7 @@ impl Action for SetupDefaultProfile { nss_ca_cert_pkg.join("etc/ssl/certs/ca-bundle.crt"), ), /* This is apparently load bearing... */ ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; set_env( "NIX_SSL_CERT_FILE", @@ -223,9 +213,7 @@ impl Action for SetupDefaultProfile { ); command.stdin(std::process::Stdio::null()); - execute_command(&mut command) - .await - .map_err(|e| ActionError::Command(e))?; + execute_command(&mut command).await?; } Ok(()) diff --git a/src/action/common/configure_init_service.rs b/src/action/common/configure_init_service.rs index 82eea1a..8c4e470 100644 --- a/src/action/common/configure_init_service.rs +++ b/src/action/common/configure_init_service.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use tokio::process::Command; use tracing::{span, Span}; -use crate::action::{ActionError, StatefulAction}; +use crate::action::{ActionError, ActionTag, StatefulAction}; use crate::execute_command; use crate::action::{Action, ActionDescription}; @@ -42,8 +42,11 @@ impl ConfigureInitService { } #[async_trait::async_trait] -#[typetag::serde(name = "configure_nix_daemon")] +#[typetag::serde(name = "configure_init_service")] impl Action for ConfigureInitService { + fn action_tag() -> ActionTag { + ActionTag("configure_init_service") + } fn tracing_synopsis(&self) -> String { match self.init { #[cfg(target_os = "linux")] @@ -58,7 +61,7 @@ impl Action for ConfigureInitService { } fn tracing_span(&self) -> Span { - span!(tracing::Level::DEBUG, "configure_nix_daemon",) + span!(tracing::Level::DEBUG, "configure_init_service",) } fn execute_description(&self) -> Vec { @@ -118,8 +121,7 @@ impl Action for ConfigureInitService { .arg(DARWIN_NIX_DAEMON_DEST) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; if *start_daemon { execute_command( @@ -130,8 +132,7 @@ impl Action for ConfigureInitService { .arg("system/org.nixos.nix-daemon") .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; } }, #[cfg(target_os = "linux")] @@ -154,8 +155,7 @@ impl Action for ConfigureInitService { .arg("--prefix=/nix/var/nix") .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; execute_command( Command::new("systemctl") @@ -164,8 +164,7 @@ impl Action for ConfigureInitService { .arg(SERVICE_SRC) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; execute_command( Command::new("systemctl") @@ -174,8 +173,7 @@ impl Action for ConfigureInitService { .arg(SOCKET_SRC) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; if *start_daemon { execute_command( @@ -184,8 +182,7 @@ impl Action for ConfigureInitService { .arg("daemon-reload") .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; execute_command( Command::new("systemctl") @@ -194,8 +191,7 @@ impl Action for ConfigureInitService { .arg("--now") .arg(SOCKET_SRC), ) - .await - .map_err(ActionError::Command)?; + .await?; } }, #[cfg(not(target_os = "macos"))] @@ -244,8 +240,7 @@ impl Action for ConfigureInitService { .arg("unload") .arg(DARWIN_NIX_DAEMON_DEST), ) - .await - .map_err(ActionError::Command)?; + .await?; }, #[cfg(target_os = "linux")] InitSystem::Systemd => { @@ -263,8 +258,7 @@ impl Action for ConfigureInitService { .args(["stop", "nix-daemon.socket"]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; } if socket_is_enabled { @@ -274,8 +268,7 @@ impl Action for ConfigureInitService { .args(["disable", "nix-daemon.socket"]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; } if service_is_active { @@ -285,8 +278,7 @@ impl Action for ConfigureInitService { .args(["stop", "nix-daemon.service"]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; } if service_is_enabled { @@ -296,8 +288,7 @@ impl Action for ConfigureInitService { .args(["disable", "nix-daemon.service"]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; } execute_command( @@ -307,8 +298,7 @@ impl Action for ConfigureInitService { .arg("--prefix=/nix/var/nix") .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; tokio::fs::remove_file(TMPFILES_DEST) .await @@ -320,8 +310,7 @@ impl Action for ConfigureInitService { .arg("daemon-reload") .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; }, #[cfg(not(target_os = "macos"))] InitSystem::None => { @@ -342,12 +331,13 @@ pub enum ConfigureNixDaemonServiceError { #[cfg(target_os = "linux")] async fn is_active(unit: &str) -> Result { - let output = Command::new("systemctl") - .arg("is-active") - .arg(unit) + let mut command = Command::new("systemctl"); + command.arg("is-active"); + command.arg(unit); + let output = command .output() .await - .map_err(ActionError::Command)?; + .map_err(|e| ActionError::command(&command, e))?; if String::from_utf8(output.stdout)?.starts_with("active") { tracing::trace!(%unit, "Is active"); Ok(true) @@ -359,12 +349,13 @@ async fn is_active(unit: &str) -> Result { #[cfg(target_os = "linux")] async fn is_enabled(unit: &str) -> Result { - let output = Command::new("systemctl") - .arg("is-enabled") - .arg(unit) + let mut command = Command::new("systemctl"); + command.arg("is-enabled"); + command.arg(unit); + let output = command .output() .await - .map_err(ActionError::Command)?; + .map_err(|e| ActionError::command(&command, e))?; let stdout = String::from_utf8(output.stdout)?; if stdout.starts_with("enabled") || stdout.starts_with("linked") { tracing::trace!(%unit, "Is enabled"); diff --git a/src/action/common/configure_nix.rs b/src/action/common/configure_nix.rs index b7a6118..9099744 100644 --- a/src/action/common/configure_nix.rs +++ b/src/action/common/configure_nix.rs @@ -2,7 +2,7 @@ use crate::{ action::{ base::SetupDefaultProfile, common::{ConfigureShellProfile, PlaceChannelConfiguration, PlaceNixConfiguration}, - Action, ActionDescription, ActionError, StatefulAction, + Action, ActionDescription, ActionError, ActionTag, StatefulAction, }, settings::CommonSettings, }; @@ -23,21 +23,30 @@ pub struct ConfigureNix { impl ConfigureNix { #[tracing::instrument(level = "debug", skip_all)] pub async fn plan(settings: &CommonSettings) -> Result, ActionError> { - let setup_default_profile = SetupDefaultProfile::plan(settings.channels.clone()).await?; + let setup_default_profile = SetupDefaultProfile::plan(settings.channels.clone()) + .await + .map_err(|e| ActionError::Child(SetupDefaultProfile::action_tag(), Box::new(e)))?; let configure_shell_profile = if settings.modify_profile { - Some(ConfigureShellProfile::plan().await?) + Some(ConfigureShellProfile::plan().await.map_err(|e| { + ActionError::Child(ConfigureShellProfile::action_tag(), Box::new(e)) + })?) } else { None }; let place_channel_configuration = - PlaceChannelConfiguration::plan(settings.channels.clone(), settings.force).await?; + PlaceChannelConfiguration::plan(settings.channels.clone(), settings.force) + .await + .map_err(|e| { + ActionError::Child(PlaceChannelConfiguration::action_tag(), Box::new(e)) + })?; let place_nix_configuration = PlaceNixConfiguration::plan( settings.nix_build_group_name.clone(), settings.extra_conf.clone(), settings.force, ) - .await?; + .await + .map_err(|e| ActionError::Child(PlaceNixConfiguration::action_tag(), Box::new(e)))?; Ok(Self { place_channel_configuration, @@ -52,6 +61,9 @@ impl ConfigureNix { #[async_trait::async_trait] #[typetag::serde(name = "configure_nix")] impl Action for ConfigureNix { + fn action_tag() -> ActionTag { + ActionTag("configure_nix") + } fn tracing_synopsis(&self) -> String { "Configure Nix".to_string() } @@ -103,24 +115,39 @@ impl Action for ConfigureNix { .try_execute() .instrument(setup_default_profile_span) .await + .map_err(|e| { + ActionError::Child(setup_default_profile.action_tag(), Box::new(e)) + }) }, async move { place_nix_configuration .try_execute() .instrument(place_nix_configuration_span) .await + .map_err(|e| { + ActionError::Child(place_nix_configuration.action_tag(), Box::new(e)) + }) }, async move { place_channel_configuration .try_execute() .instrument(place_channel_configuration_span) .await + .map_err(|e| { + ActionError::Child( + place_channel_configuration.action_tag(), + Box::new(e), + ) + }) }, async move { configure_shell_profile .try_execute() .instrument(configure_shell_profile_span) .await + .map_err(|e| { + ActionError::Child(configure_shell_profile.action_tag(), Box::new(e)) + }) }, )?; } else { @@ -135,18 +162,30 @@ impl Action for ConfigureNix { .try_execute() .instrument(setup_default_profile_span) .await + .map_err(|e| { + ActionError::Child(setup_default_profile.action_tag(), Box::new(e)) + }) }, async move { place_nix_configuration .try_execute() .instrument(place_nix_configuration_span) .await + .map_err(|e| { + ActionError::Child(place_nix_configuration.action_tag(), Box::new(e)) + }) }, async move { place_channel_configuration .try_execute() .instrument(place_channel_configuration_span) .await + .map_err(|e| { + ActionError::Child( + place_channel_configuration.action_tag(), + Box::new(e), + ) + }) }, )?; }; @@ -175,19 +214,26 @@ impl Action for ConfigureNix { #[tracing::instrument(level = "debug", skip_all)] async fn revert(&mut self) -> Result<(), ActionError> { - let Self { - setup_default_profile, - place_nix_configuration, - place_channel_configuration, - configure_shell_profile, - } = self; - - if let Some(configure_shell_profile) = configure_shell_profile { - configure_shell_profile.try_revert().await?; + if let Some(configure_shell_profile) = &mut self.configure_shell_profile { + configure_shell_profile.try_revert().await.map_err(|e| { + ActionError::Child(configure_shell_profile.action_tag(), Box::new(e)) + })?; } - place_channel_configuration.try_revert().await?; - place_nix_configuration.try_revert().await?; - setup_default_profile.try_revert().await?; + self.place_channel_configuration + .try_revert() + .await + .map_err(|e| { + ActionError::Child(self.place_channel_configuration.action_tag(), Box::new(e)) + })?; + self.place_nix_configuration + .try_revert() + .await + .map_err(|e| { + ActionError::Child(self.place_nix_configuration.action_tag(), Box::new(e)) + })?; + self.setup_default_profile.try_revert().await.map_err(|e| { + ActionError::Child(self.setup_default_profile.action_tag(), Box::new(e)) + })?; Ok(()) } diff --git a/src/action/common/configure_shell_profile.rs b/src/action/common/configure_shell_profile.rs index 0f2361e..bfb8bf8 100644 --- a/src/action/common/configure_shell_profile.rs +++ b/src/action/common/configure_shell_profile.rs @@ -1,5 +1,5 @@ use crate::action::base::{create_or_insert_into_file, CreateDirectory, CreateOrInsertIntoFile}; -use crate::action::{Action, ActionDescription, ActionError, StatefulAction}; +use crate::action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction}; use nix::unistd::User; use std::path::{Path, PathBuf}; @@ -166,6 +166,9 @@ impl ConfigureShellProfile { #[async_trait::async_trait] #[typetag::serde(name = "configure_shell_profile")] impl Action for ConfigureShellProfile { + fn action_tag() -> ActionTag { + ActionTag("configure_shell_profile") + } fn tracing_synopsis(&self) -> String { "Configure the shell profiles".to_string() } @@ -183,26 +186,29 @@ impl Action for ConfigureShellProfile { #[tracing::instrument(level = "debug", skip_all)] async fn execute(&mut self) -> Result<(), ActionError> { - let Self { - create_or_insert_into_files, - create_directories, - } = self; - - for create_directory in create_directories { + for create_directory in &mut self.create_directories { create_directory.try_execute().await?; } let mut set = JoinSet::new(); let mut errors = Vec::default(); - for (idx, create_or_insert_into_file) in create_or_insert_into_files.iter().enumerate() { + for (idx, create_or_insert_into_file) in + self.create_or_insert_into_files.iter_mut().enumerate() + { let span = tracing::Span::current().clone(); let mut create_or_insert_into_file_clone = create_or_insert_into_file.clone(); let _abort_handle = set.spawn(async move { create_or_insert_into_file_clone .try_execute() .instrument(span) - .await?; + .await + .map_err(|e| { + ActionError::Child( + create_or_insert_into_file_clone.action_tag(), + Box::new(e), + ) + })?; Result::<_, ActionError>::Ok((idx, create_or_insert_into_file_clone)) }); } @@ -210,18 +216,20 @@ impl Action for ConfigureShellProfile { while let Some(result) = set.join_next().await { match result { Ok(Ok((idx, create_or_insert_into_file))) => { - create_or_insert_into_files[idx] = create_or_insert_into_file + self.create_or_insert_into_files[idx] = create_or_insert_into_file }, - Ok(Err(e)) => errors.push(Box::new(e)), - Err(e) => return Err(e.into()), + Ok(Err(e)) => errors.push(e), + Err(e) => return Err(e)?, }; } if !errors.is_empty() { if errors.len() == 1 { - return Err(errors.into_iter().next().unwrap().into()); + return Err(errors.into_iter().next().unwrap())?; } else { - return Err(ActionError::Children(errors)); + return Err(ActionError::Children( + errors.into_iter().map(|v| Box::new(v)).collect(), + )); } } @@ -237,15 +245,12 @@ impl Action for ConfigureShellProfile { #[tracing::instrument(level = "debug", skip_all)] async fn revert(&mut self) -> Result<(), ActionError> { - let Self { - create_directories, - create_or_insert_into_files, - } = self; - let mut set = JoinSet::new(); - let mut errors: Vec> = Vec::default(); + let mut errors: Vec = Vec::default(); - for (idx, create_or_insert_into_file) in create_or_insert_into_files.iter().enumerate() { + for (idx, create_or_insert_into_file) in + self.create_or_insert_into_files.iter_mut().enumerate() + { let mut create_or_insert_file_clone = create_or_insert_into_file.clone(); let _abort_handle = set.spawn(async move { create_or_insert_file_clone.try_revert().await?; @@ -256,22 +261,27 @@ impl Action for ConfigureShellProfile { while let Some(result) = set.join_next().await { match result { Ok(Ok((idx, create_or_insert_into_file))) => { - create_or_insert_into_files[idx] = create_or_insert_into_file + self.create_or_insert_into_files[idx] = create_or_insert_into_file }, - Ok(Err(e)) => errors.push(Box::new(e)), - Err(e) => return Err(e.into()), + Ok(Err(e)) => errors.push(e), + Err(e) => return Err(e)?, }; } - for create_directory in create_directories { - create_directory.try_revert().await?; + for create_directory in self.create_directories.iter_mut() { + create_directory + .try_revert() + .await + .map_err(|e| ActionError::Child(create_directory.action_tag(), Box::new(e)))?; } if !errors.is_empty() { if errors.len() == 1 { - return Err(errors.into_iter().next().unwrap().into()); + return Err(errors.into_iter().next().unwrap())?; } else { - return Err(ActionError::Children(errors)); + return Err(ActionError::Children( + errors.into_iter().map(|v| Box::new(v)).collect(), + )); } } diff --git a/src/action/common/create_nix_tree.rs b/src/action/common/create_nix_tree.rs index 3664a08..2148c3e 100644 --- a/src/action/common/create_nix_tree.rs +++ b/src/action/common/create_nix_tree.rs @@ -1,7 +1,7 @@ use tracing::{span, Span}; use crate::action::base::CreateDirectory; -use crate::action::{Action, ActionDescription, ActionError, StatefulAction}; +use crate::action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction}; const PATHS: &[&str] = &[ "/nix/var", @@ -33,7 +33,11 @@ impl CreateNixTree { let mut create_directories = Vec::default(); for path in PATHS { // We use `create_dir` over `create_dir_all` to ensure we always set permissions right - create_directories.push(CreateDirectory::plan(path, None, None, 0o0755, false).await?) + create_directories.push( + CreateDirectory::plan(path, None, None, 0o0755, false) + .await + .map_err(|e| ActionError::Child(CreateDirectory::action_tag(), Box::new(e)))?, + ) } Ok(Self { create_directories }.into()) @@ -43,6 +47,9 @@ impl CreateNixTree { #[async_trait::async_trait] #[typetag::serde(name = "create_nix_tree")] impl Action for CreateNixTree { + fn action_tag() -> ActionTag { + ActionTag("create_nix_tree") + } fn tracing_synopsis(&self) -> String { "Create a directory tree in `/nix`".to_string() } @@ -68,11 +75,12 @@ impl Action for CreateNixTree { #[tracing::instrument(level = "debug", skip_all)] async fn execute(&mut self) -> Result<(), ActionError> { - let Self { create_directories } = self; - // Just do sequential since parallelizing this will have little benefit - for create_directory in create_directories { - create_directory.try_execute().await? + for create_directory in self.create_directories.iter_mut() { + create_directory + .try_execute() + .await + .map_err(|e| ActionError::Child(create_directory.action_tag(), Box::new(e)))? } Ok(()) @@ -100,11 +108,12 @@ impl Action for CreateNixTree { #[tracing::instrument(level = "debug", skip_all)] async fn revert(&mut self) -> Result<(), ActionError> { - let Self { create_directories } = self; - // Just do sequential since parallelizing this will have little benefit - for create_directory in create_directories.iter_mut().rev() { - create_directory.try_revert().await? + for create_directory in self.create_directories.iter_mut().rev() { + create_directory + .try_revert() + .await + .map_err(|e| ActionError::Child(create_directory.action_tag(), Box::new(e)))? } Ok(()) diff --git a/src/action/common/create_users_and_groups.rs b/src/action/common/create_users_and_groups.rs index 3112f23..07fb74c 100644 --- a/src/action/common/create_users_and_groups.rs +++ b/src/action/common/create_users_and_groups.rs @@ -1,7 +1,7 @@ use crate::{ action::{ base::{AddUserToGroup, CreateGroup, CreateUser}, - Action, ActionDescription, ActionError, StatefulAction, + Action, ActionDescription, ActionError, ActionTag, StatefulAction, }, settings::CommonSettings, }; @@ -37,7 +37,8 @@ impl CreateUsersAndGroups { settings.nix_build_group_name.clone(), settings.nix_build_group_id, ) - .await?, + .await + .map_err(|e| ActionError::Child(CreateUser::action_tag(), Box::new(e)))?, ); add_users_to_groups.push( AddUserToGroup::plan( @@ -46,7 +47,8 @@ impl CreateUsersAndGroups { settings.nix_build_group_name.clone(), settings.nix_build_group_id, ) - .await?, + .await + .map_err(|e| ActionError::Child(AddUserToGroup::action_tag(), Box::new(e)))?, ); } Ok(Self { @@ -66,6 +68,9 @@ impl CreateUsersAndGroups { #[async_trait::async_trait] #[typetag::serde(name = "create_users_and_group")] impl Action for CreateUsersAndGroups { + fn action_tag() -> ActionTag { + ActionTag("create_users_and_group") + } fn tracing_synopsis(&self) -> String { format!( "Create build users (UID {}-{}) and group (GID {})", @@ -151,12 +156,18 @@ impl Action for CreateUsersAndGroups { } | OperatingSystem::Darwin => { for create_user in create_users.iter_mut() { - create_user.try_execute().await?; + create_user + .try_execute() + .await + .map_err(|e| ActionError::Child(create_user.action_tag(), Box::new(e)))?; } }, _ => { for create_user in create_users.iter_mut() { - create_user.try_execute().await?; + create_user + .try_execute() + .await + .map_err(|e| ActionError::Child(create_user.action_tag(), Box::new(e)))?; } // While we may be tempted to do something like this, it can break on many older OSes like Ubuntu 18.04: // ``` @@ -194,7 +205,10 @@ impl Action for CreateUsersAndGroups { }; for add_user_to_group in add_users_to_groups.iter_mut() { - add_user_to_group.try_execute().await?; + add_user_to_group + .try_execute() + .await + .map_err(|e| ActionError::Child(add_user_to_group.action_tag(), Box::new(e)))?; } Ok(()) @@ -260,7 +274,11 @@ impl Action for CreateUsersAndGroups { let span = tracing::Span::current().clone(); let mut create_user_clone = create_user.clone(); let _abort_handle = set.spawn(async move { - create_user_clone.try_revert().instrument(span).await?; + create_user_clone + .try_revert() + .instrument(span) + .await + .map_err(|e| ActionError::Child(create_user_clone.action_tag(), Box::new(e)))?; Result::<_, ActionError>::Ok((idx, create_user_clone)) }); } @@ -275,7 +293,7 @@ impl Action for CreateUsersAndGroups { if !errors.is_empty() { if errors.len() == 1 { - return Err(errors.into_iter().next().unwrap().into()); + return Err(*errors.into_iter().next().unwrap()); } else { return Err(ActionError::Children(errors)); } @@ -287,7 +305,10 @@ impl Action for CreateUsersAndGroups { // } // Create group - create_group.try_revert().await?; + create_group + .try_revert() + .await + .map_err(|e| ActionError::Child(create_group.action_tag(), Box::new(e)))?; Ok(()) } diff --git a/src/action/common/place_channel_configuration.rs b/src/action/common/place_channel_configuration.rs index c87b1d4..78c5501 100644 --- a/src/action/common/place_channel_configuration.rs +++ b/src/action/common/place_channel_configuration.rs @@ -1,6 +1,6 @@ use crate::action::base::CreateFile; -use crate::action::ActionError; use crate::action::{Action, ActionDescription, StatefulAction}; +use crate::action::{ActionError, ActionTag}; use crate::ChannelValue; use tracing::{span, Span}; @@ -36,7 +36,8 @@ impl PlaceChannelConfiguration { buf, force, ) - .await?; + .await + .map_err(|e| ActionError::Child(CreateFile::action_tag(), Box::new(e)))?; Ok(Self { create_file, channels, @@ -48,6 +49,9 @@ impl PlaceChannelConfiguration { #[async_trait::async_trait] #[typetag::serde(name = "place_channel_configuration")] impl Action for PlaceChannelConfiguration { + fn action_tag() -> ActionTag { + ActionTag("place_channel_configuration") + } fn tracing_synopsis(&self) -> String { format!( "Place channel configuration at `{}`", @@ -74,12 +78,10 @@ impl Action for PlaceChannelConfiguration { #[tracing::instrument(level = "debug", skip_all)] async fn execute(&mut self) -> Result<(), ActionError> { - let Self { - create_file, - channels: _, - } = self; - - create_file.try_execute().await?; + self.create_file + .try_execute() + .await + .map_err(|e| ActionError::Child(self.create_file.action_tag(), Box::new(e)))?; Ok(()) } @@ -96,12 +98,10 @@ impl Action for PlaceChannelConfiguration { #[tracing::instrument(level = "debug", skip_all)] async fn revert(&mut self) -> Result<(), ActionError> { - let Self { - create_file, - channels: _, - } = self; - - create_file.try_revert().await?; + self.create_file + .try_revert() + .await + .map_err(|e| ActionError::Child(self.create_file.action_tag(), Box::new(e)))?; Ok(()) } diff --git a/src/action/common/place_nix_configuration.rs b/src/action/common/place_nix_configuration.rs index 48ccf69..e7bc637 100644 --- a/src/action/common/place_nix_configuration.rs +++ b/src/action/common/place_nix_configuration.rs @@ -1,7 +1,7 @@ use tracing::{span, Span}; use crate::action::base::{CreateDirectory, CreateFile}; -use crate::action::{Action, ActionDescription, ActionError, StatefulAction}; +use crate::action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction}; const NIX_CONF_FOLDER: &str = "/etc/nix"; const NIX_CONF: &str = "/etc/nix/nix.conf"; @@ -41,9 +41,12 @@ impl PlaceNixConfiguration { extra_conf = extra_conf.join("\n"), version = env!("CARGO_PKG_VERSION"), ); - let create_directory = - CreateDirectory::plan(NIX_CONF_FOLDER, None, None, 0o0755, force).await?; - let create_file = CreateFile::plan(NIX_CONF, None, None, 0o0664, buf, force).await?; + let create_directory = CreateDirectory::plan(NIX_CONF_FOLDER, None, None, 0o0755, force) + .await + .map_err(|e| ActionError::Child(CreateDirectory::action_tag(), Box::new(e)))?; + let create_file = CreateFile::plan(NIX_CONF, None, None, 0o0664, buf, force) + .await + .map_err(|e| ActionError::Child(CreateFile::action_tag(), Box::new(e)))?; Ok(Self { create_directory, create_file, @@ -55,6 +58,9 @@ impl PlaceNixConfiguration { #[async_trait::async_trait] #[typetag::serde(name = "place_nix_configuration")] impl Action for PlaceNixConfiguration { + fn action_tag() -> ActionTag { + ActionTag("place_nix_configuration") + } fn tracing_synopsis(&self) -> String { format!("Place the Nix configuration in `{NIX_CONF}`") } @@ -75,13 +81,14 @@ impl Action for PlaceNixConfiguration { #[tracing::instrument(level = "debug", skip_all)] async fn execute(&mut self) -> Result<(), ActionError> { - let Self { - create_file, - create_directory, - } = self; - - create_directory.try_execute().await?; - create_file.try_execute().await?; + self.create_directory + .try_execute() + .await + .map_err(|e| ActionError::Child(self.create_directory.action_tag(), Box::new(e)))?; + self.create_file + .try_execute() + .await + .map_err(|e| ActionError::Child(self.create_file.action_tag(), Box::new(e)))?; Ok(()) } @@ -98,13 +105,14 @@ impl Action for PlaceNixConfiguration { #[tracing::instrument(level = "debug", skip_all)] async fn revert(&mut self) -> Result<(), ActionError> { - let Self { - create_file, - create_directory, - } = self; - - create_file.try_revert().await?; - create_directory.try_revert().await?; + self.create_file + .try_revert() + .await + .map_err(|e| ActionError::Child(self.create_file.action_tag(), Box::new(e)))?; + self.create_directory + .try_revert() + .await + .map_err(|e| ActionError::Child(self.create_directory.action_tag(), Box::new(e)))?; Ok(()) } diff --git a/src/action/common/provision_nix.rs b/src/action/common/provision_nix.rs index b337b0c..d28d9a9 100644 --- a/src/action/common/provision_nix.rs +++ b/src/action/common/provision_nix.rs @@ -4,7 +4,7 @@ use super::{CreateNixTree, CreateUsersAndGroups}; use crate::{ action::{ base::{FetchAndUnpackNix, MoveUnpackedNix}, - Action, ActionDescription, ActionError, StatefulAction, + Action, ActionDescription, ActionError, ActionTag, StatefulAction, }, settings::CommonSettings, }; @@ -29,10 +29,15 @@ impl ProvisionNix { PathBuf::from("/nix/temp-install-dir"), ) .await?; - let create_users_and_group = CreateUsersAndGroups::plan(settings.clone()).await?; - let create_nix_tree = CreateNixTree::plan().await?; - let move_unpacked_nix = - MoveUnpackedNix::plan(PathBuf::from("/nix/temp-install-dir")).await?; + let create_users_and_group = CreateUsersAndGroups::plan(settings.clone()) + .await + .map_err(|e| ActionError::Child(CreateUsersAndGroups::action_tag(), Box::new(e)))?; + let create_nix_tree = CreateNixTree::plan() + .await + .map_err(|e| ActionError::Child(CreateNixTree::action_tag(), Box::new(e)))?; + let move_unpacked_nix = MoveUnpackedNix::plan(PathBuf::from("/nix/temp-install-dir")) + .await + .map_err(|e| ActionError::Child(MoveUnpackedNix::action_tag(), Box::new(e)))?; Ok(Self { fetch_nix, create_users_and_group, @@ -46,6 +51,9 @@ impl ProvisionNix { #[async_trait::async_trait] #[typetag::serde(name = "provision_nix")] impl Action for ProvisionNix { + fn action_tag() -> ActionTag { + ActionTag("provision_nix") + } fn tracing_synopsis(&self) -> String { "Provision Nix".to_string() } @@ -73,25 +81,32 @@ impl Action for ProvisionNix { #[tracing::instrument(level = "debug", skip_all)] async fn execute(&mut self) -> Result<(), ActionError> { - let Self { - fetch_nix, - create_nix_tree, - create_users_and_group, - move_unpacked_nix, - } = self; - // We fetch nix while doing the rest, then move it over. - let mut fetch_nix_clone = fetch_nix.clone(); + let mut fetch_nix_clone = self.fetch_nix.clone(); let fetch_nix_handle = tokio::task::spawn(async { - fetch_nix_clone.try_execute().await?; + fetch_nix_clone + .try_execute() + .await + .map_err(|e| ActionError::Child(fetch_nix_clone.action_tag(), Box::new(e)))?; Result::<_, ActionError>::Ok(fetch_nix_clone) }); - create_users_and_group.try_execute().await?; - create_nix_tree.try_execute().await?; + self.create_users_and_group + .try_execute() + .await + .map_err(|e| { + ActionError::Child(self.create_users_and_group.action_tag(), Box::new(e)) + })?; + self.create_nix_tree + .try_execute() + .await + .map_err(|e| ActionError::Child(self.create_nix_tree.action_tag(), Box::new(e)))?; - *fetch_nix = fetch_nix_handle.await.map_err(ActionError::Join)??; - move_unpacked_nix.try_execute().await?; + self.fetch_nix = fetch_nix_handle.await.map_err(ActionError::Join)??; + self.move_unpacked_nix + .try_execute() + .await + .map_err(|e| ActionError::Child(self.move_unpacked_nix.action_tag(), Box::new(e)))?; Ok(()) } @@ -114,31 +129,33 @@ impl Action for ProvisionNix { #[tracing::instrument(level = "debug", skip_all)] async fn revert(&mut self) -> Result<(), ActionError> { - let Self { - fetch_nix, - create_nix_tree, - create_users_and_group, - move_unpacked_nix, - } = self; - // We fetch nix while doing the rest, then move it over. - let mut fetch_nix_clone = fetch_nix.clone(); + let mut fetch_nix_clone = self.fetch_nix.clone(); let fetch_nix_handle = tokio::task::spawn(async { - fetch_nix_clone.try_revert().await?; + fetch_nix_clone + .try_revert() + .await + .map_err(|e| ActionError::Child(fetch_nix_clone.action_tag(), Box::new(e)))?; Result::<_, ActionError>::Ok(fetch_nix_clone) }); - if let Err(err) = create_users_and_group.try_revert().await { + if let Err(err) = self.create_users_and_group.try_revert().await { fetch_nix_handle.abort(); return Err(err); } - if let Err(err) = create_nix_tree.try_revert().await { + if let Err(err) = self.create_nix_tree.try_revert().await { fetch_nix_handle.abort(); return Err(err); } - *fetch_nix = fetch_nix_handle.await.map_err(ActionError::Join)??; - move_unpacked_nix.try_revert().await?; + self.fetch_nix = fetch_nix_handle + .await + .map_err(ActionError::Join)? + .map_err(|e| ActionError::Child(self.fetch_nix.action_tag(), Box::new(e)))?; + self.move_unpacked_nix + .try_revert() + .await + .map_err(|e| ActionError::Child(self.move_unpacked_nix.action_tag(), Box::new(e)))?; Ok(()) } diff --git a/src/action/linux/start_systemd_unit.rs b/src/action/linux/start_systemd_unit.rs index 485d0b0..a2f059b 100644 --- a/src/action/linux/start_systemd_unit.rs +++ b/src/action/linux/start_systemd_unit.rs @@ -1,7 +1,7 @@ use tokio::process::Command; use tracing::{span, Span}; -use crate::action::{ActionError, ActionState, StatefulAction}; +use crate::action::{ActionError, ActionState, ActionTag, StatefulAction}; use crate::execute_command; use crate::action::{Action, ActionDescription}; @@ -22,12 +22,13 @@ impl StartSystemdUnit { enable: bool, ) -> Result, ActionError> { let unit = unit.as_ref(); - let output = Command::new("systemctl") - .arg("is-active") - .arg(unit) + let mut command = Command::new("systemctl"); + command.arg("is-active"); + command.arg(unit); + let output = command .output() .await - .map_err(ActionError::Command)?; + .map_err(|e| ActionError::command(&command, e))?; let state = if output.status.success() { tracing::debug!("Starting systemd unit `{}` already complete", unit); @@ -49,6 +50,9 @@ impl StartSystemdUnit { #[async_trait::async_trait] #[typetag::serde(name = "start_systemd_unit")] impl Action for StartSystemdUnit { + fn action_tag() -> ActionTag { + ActionTag("start_systemd_unit") + } fn tracing_synopsis(&self) -> String { format!("Enable (and start) the systemd unit {}", self.unit) } @@ -80,8 +84,7 @@ impl Action for StartSystemdUnit { .arg(format!("{unit}")) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Custom(Box::new(StartSystemdUnitError::Command(e))))?; + .await?; }, false => { // TODO(@Hoverbear): Handle proxy vars @@ -92,8 +95,7 @@ impl Action for StartSystemdUnit { .arg(format!("{unit}")) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Custom(Box::new(StartSystemdUnitError::Command(e))))?; + .await?; }, } @@ -119,8 +121,7 @@ impl Action for StartSystemdUnit { .arg(format!("{unit}")) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Custom(Box::new(StartSystemdUnitError::Command(e))))?; + .await?; }; // We do both to avoid an error doing `disable --now` if the user did stop it already somehow. @@ -131,8 +132,7 @@ impl Action for StartSystemdUnit { .arg(format!("{unit}")) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Custom(Box::new(StartSystemdUnitError::Command(e))))?; + .await?; Ok(()) } diff --git a/src/action/macos/bootstrap_apfs_volume.rs b/src/action/macos/bootstrap_apfs_volume.rs index c448a82..6026fd4 100644 --- a/src/action/macos/bootstrap_apfs_volume.rs +++ b/src/action/macos/bootstrap_apfs_volume.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use tokio::process::Command; use tracing::{span, Span}; -use crate::action::{ActionError, StatefulAction}; +use crate::action::{ActionError, ActionTag, StatefulAction}; use crate::execute_command; use crate::action::{Action, ActionDescription}; @@ -27,8 +27,11 @@ impl BootstrapApfsVolume { } #[async_trait::async_trait] -#[typetag::serde(name = "bootstrap_volume")] +#[typetag::serde(name = "bootstrap_apfs_volume")] impl Action for BootstrapApfsVolume { + fn action_tag() -> ActionTag { + ActionTag("bootstrap_apfs_volume") + } fn tracing_synopsis(&self) -> String { format!("Bootstrap and kickstart `{}`", self.path.display()) } @@ -36,7 +39,7 @@ impl Action for BootstrapApfsVolume { fn tracing_span(&self) -> Span { span!( tracing::Level::DEBUG, - "bootstrap_volume", + "bootstrap_apfs_volume", path = %self.path.display(), ) } @@ -56,16 +59,14 @@ impl Action for BootstrapApfsVolume { .arg(path) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; execute_command( Command::new("launchctl") .process_group(0) .args(["kickstart", "-k", "system/org.nixos.darwin-store"]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; Ok(()) } @@ -88,8 +89,7 @@ impl Action for BootstrapApfsVolume { .arg(path) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; Ok(()) } diff --git a/src/action/macos/create_apfs_volume.rs b/src/action/macos/create_apfs_volume.rs index 00e05a4..4a53dff 100644 --- a/src/action/macos/create_apfs_volume.rs +++ b/src/action/macos/create_apfs_volume.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use tokio::process::Command; use tracing::{span, Span}; -use crate::action::{ActionError, StatefulAction}; +use crate::action::{ActionError, ActionTag, StatefulAction}; use crate::execute_command; use serde::Deserialize; @@ -25,8 +25,7 @@ impl CreateApfsVolume { ) -> Result, ActionError> { let output = execute_command(Command::new("/usr/sbin/diskutil").args(["apfs", "list", "-plist"])) - .await - .map_err(ActionError::Command)?; + .await?; let parsed: DiskUtilApfsListOutput = plist::from_bytes(&output.stdout)?; for container in parsed.containers { @@ -51,6 +50,9 @@ impl CreateApfsVolume { #[async_trait::async_trait] #[typetag::serde(name = "create_volume")] impl Action for CreateApfsVolume { + fn action_tag() -> ActionTag { + ActionTag("create_apfs_volume") + } fn tracing_synopsis(&self) -> String { format!( "Create an APFS volume on `{}` named `{}`", @@ -98,8 +100,7 @@ impl Action for CreateApfsVolume { ]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; Ok(()) } @@ -129,8 +130,7 @@ impl Action for CreateApfsVolume { .args(["apfs", "deleteVolume", name]) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; Ok(()) } diff --git a/src/action/macos/create_fstab_entry.rs b/src/action/macos/create_fstab_entry.rs index cfac0cb..b7cbb83 100644 --- a/src/action/macos/create_fstab_entry.rs +++ b/src/action/macos/create_fstab_entry.rs @@ -1,7 +1,7 @@ use uuid::Uuid; use crate::{ - action::{Action, ActionDescription, ActionError, StatefulAction}, + action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction}, execute_command, }; use serde::Deserialize; @@ -60,6 +60,9 @@ impl CreateFstabEntry { #[async_trait::async_trait] #[typetag::serde(name = "create_fstab_entry")] impl Action for CreateFstabEntry { + fn action_tag() -> ActionTag { + ActionTag("create_fstab_entry") + } fn tracing_synopsis(&self) -> String { format!( "Add a UUID based entry for the APFS volume `{}` to `/etc/fstab`", @@ -178,8 +181,7 @@ async fn get_uuid_for_label(apfs_volume_label: &str) -> Result{v}\n")).collect::>().join("\n") ); let setup_volume_daemon = - CreateFile::plan(NIX_VOLUME_MOUNTD_DEST, None, None, None, mount_plist, false).await?; + CreateFile::plan(NIX_VOLUME_MOUNTD_DEST, None, None, None, mount_plist, false) + .await + .map_err(|e| ActionError::Child(CreateFile::action_tag(), Box::new(e)))?; - let bootstrap_volume = BootstrapApfsVolume::plan(NIX_VOLUME_MOUNTD_DEST).await?; - let enable_ownership = EnableOwnership::plan("/nix").await?; + let bootstrap_volume = BootstrapApfsVolume::plan(NIX_VOLUME_MOUNTD_DEST) + .await + .map_err(|e| ActionError::Child(BootstrapApfsVolume::action_tag(), Box::new(e)))?; + let enable_ownership = EnableOwnership::plan("/nix") + .await + .map_err(|e| ActionError::Child(EnableOwnership::action_tag(), Box::new(e)))?; Ok(Self { disk: disk.to_path_buf(), @@ -133,6 +145,9 @@ impl CreateNixVolume { #[async_trait::async_trait] #[typetag::serde(name = "create_apfs_volume")] impl Action for CreateNixVolume { + fn action_tag() -> ActionTag { + ActionTag("create_nix_volume") + } fn tracing_synopsis(&self) -> String { format!( "Create an APFS volume `{}` for Nix on `{}`", @@ -159,44 +174,57 @@ impl Action for CreateNixVolume { #[tracing::instrument(level = "debug", skip_all)] async fn execute(&mut self) -> Result<(), ActionError> { - let Self { - disk: _, - name: _, - case_sensitive: _, - encrypt: _, - create_or_append_synthetic_conf, - create_synthetic_objects, - unmount_volume, - create_volume, - create_fstab_entry, - encrypt_volume, - setup_volume_daemon, - bootstrap_volume, - enable_ownership, - } = self; - - create_or_append_synthetic_conf.try_execute().await?; - create_synthetic_objects.try_execute().await?; - unmount_volume.try_execute().await.ok(); // We actually expect this may fail. - create_volume.try_execute().await?; - create_fstab_entry.try_execute().await?; - if let Some(encrypt_volume) = encrypt_volume { - encrypt_volume.try_execute().await?; + self.create_or_append_synthetic_conf + .try_execute() + .await + .map_err(|e| { + ActionError::Child( + self.create_or_append_synthetic_conf.action_tag(), + Box::new(e), + ) + })?; + self.create_synthetic_objects + .try_execute() + .await + .map_err(|e| { + ActionError::Child(self.create_synthetic_objects.action_tag(), Box::new(e)) + })?; + self.unmount_volume.try_execute().await.ok(); // We actually expect this may fail. + self.create_volume + .try_execute() + .await + .map_err(|e| ActionError::Child(self.create_volume.action_tag(), Box::new(e)))?; + self.create_fstab_entry + .try_execute() + .await + .map_err(|e| ActionError::Child(self.create_fstab_entry.action_tag(), Box::new(e)))?; + if let Some(encrypt_volume) = &mut self.encrypt_volume { + encrypt_volume + .try_execute() + .await + .map_err(|e| ActionError::Child(encrypt_volume.action_tag(), Box::new(e)))?; } - setup_volume_daemon.try_execute().await?; + self.setup_volume_daemon + .try_execute() + .await + .map_err(|e| ActionError::Child(self.setup_volume_daemon.action_tag(), Box::new(e)))?; - bootstrap_volume.try_execute().await?; + self.bootstrap_volume + .try_execute() + .await + .map_err(|e| ActionError::Child(self.bootstrap_volume.action_tag(), Box::new(e)))?; let mut retry_tokens: usize = 50; loop { tracing::trace!(%retry_tokens, "Checking for Nix Store existence"); - let status = Command::new("/usr/sbin/diskutil") - .args(["info", "/nix"]) - .stderr(std::process::Stdio::null()) - .stdout(std::process::Stdio::null()) + let mut command = Command::new("/usr/sbin/diskutil"); + command.args(["info", "/nix"]); + command.stderr(std::process::Stdio::null()); + command.stdout(std::process::Stdio::null()); + let status = command .status() .await - .map_err(|e| ActionError::Command(e))?; + .map_err(|e| ActionError::command(&command, e))?; if status.success() || retry_tokens == 0 { break; } else { @@ -205,7 +233,10 @@ impl Action for CreateNixVolume { tokio::time::sleep(Duration::from_millis(100)).await; } - enable_ownership.try_execute().await?; + self.enable_ownership + .try_execute() + .await + .map_err(|e| ActionError::Child(self.enable_ownership.action_tag(), Box::new(e)))?; Ok(()) } @@ -222,36 +253,54 @@ impl Action for CreateNixVolume { #[tracing::instrument(level = "debug", skip_all)] async fn revert(&mut self) -> Result<(), ActionError> { - let Self { - disk: _, - name: _, - case_sensitive: _, - encrypt: _, - create_or_append_synthetic_conf, - create_synthetic_objects, - unmount_volume, - create_volume, - create_fstab_entry, - encrypt_volume, - setup_volume_daemon, - bootstrap_volume, - enable_ownership, - } = self; - - enable_ownership.try_revert().await?; - bootstrap_volume.try_revert().await?; - setup_volume_daemon.try_revert().await?; - if let Some(encrypt_volume) = encrypt_volume { - encrypt_volume.try_revert().await?; + self.enable_ownership + .try_revert() + .await + .map_err(|e| ActionError::Child(self.enable_ownership.action_tag(), Box::new(e)))?; + self.bootstrap_volume + .try_revert() + .await + .map_err(|e| ActionError::Child(self.bootstrap_volume.action_tag(), Box::new(e)))?; + self.setup_volume_daemon + .try_revert() + .await + .map_err(|e| ActionError::Child(self.setup_volume_daemon.action_tag(), Box::new(e)))?; + if let Some(encrypt_volume) = &mut self.encrypt_volume { + encrypt_volume + .try_revert() + .await + .map_err(|e| ActionError::Child(encrypt_volume.action_tag(), Box::new(e)))?; } - create_fstab_entry.try_revert().await?; + self.create_fstab_entry + .try_revert() + .await + .map_err(|e| ActionError::Child(self.create_fstab_entry.action_tag(), Box::new(e)))?; - unmount_volume.try_revert().await?; - create_volume.try_revert().await?; + self.unmount_volume + .try_revert() + .await + .map_err(|e| ActionError::Child(self.unmount_volume.action_tag(), Box::new(e)))?; + self.create_volume + .try_revert() + .await + .map_err(|e| ActionError::Child(self.create_volume.action_tag(), Box::new(e)))?; // Purposefully not reversed - create_or_append_synthetic_conf.try_revert().await?; - create_synthetic_objects.try_revert().await?; + self.create_or_append_synthetic_conf + .try_revert() + .await + .map_err(|e| { + ActionError::Child( + self.create_or_append_synthetic_conf.action_tag(), + Box::new(e), + ) + })?; + self.create_synthetic_objects + .try_revert() + .await + .map_err(|e| { + ActionError::Child(self.create_synthetic_objects.action_tag(), Box::new(e)) + })?; Ok(()) } diff --git a/src/action/macos/create_synthetic_objects.rs b/src/action/macos/create_synthetic_objects.rs index f671881..aa56a3a 100644 --- a/src/action/macos/create_synthetic_objects.rs +++ b/src/action/macos/create_synthetic_objects.rs @@ -3,7 +3,7 @@ use tracing::{span, Span}; use crate::execute_command; -use crate::action::{Action, ActionDescription, ActionError, StatefulAction}; +use crate::action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction}; /// Create the synthetic objects defined in `/etc/syntethic.conf` #[derive(Debug, serde::Deserialize, serde::Serialize, Clone)] @@ -19,6 +19,9 @@ impl CreateSyntheticObjects { #[async_trait::async_trait] #[typetag::serde(name = "create_synthetic_objects")] impl Action for CreateSyntheticObjects { + fn action_tag() -> ActionTag { + ActionTag("create_synthetic_objects") + } fn tracing_synopsis(&self) -> String { "Create objects defined in `/etc/synthetic.conf`".to_string() } diff --git a/src/action/macos/enable_ownership.rs b/src/action/macos/enable_ownership.rs index a59e86e..d62f882 100644 --- a/src/action/macos/enable_ownership.rs +++ b/src/action/macos/enable_ownership.rs @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf}; use tokio::process::Command; use tracing::{span, Span}; -use crate::action::{ActionError, StatefulAction}; +use crate::action::{ActionError, ActionTag, StatefulAction}; use crate::execute_command; use crate::action::{Action, ActionDescription}; @@ -31,6 +31,9 @@ impl EnableOwnership { #[async_trait::async_trait] #[typetag::serde(name = "enable_ownership")] impl Action for EnableOwnership { + fn action_tag() -> ActionTag { + ActionTag("enable_ownership") + } fn tracing_synopsis(&self) -> String { format!("Enable ownership on {}", self.path.display()) } @@ -59,8 +62,7 @@ impl Action for EnableOwnership { .arg(&path) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)? + .await? .stdout; let the_plist: DiskUtilOutput = plist::from_reader(Cursor::new(buf))?; @@ -75,8 +77,7 @@ impl Action for EnableOwnership { .arg(path) .stdin(std::process::Stdio::null()), ) - .await - .map_err(ActionError::Command)?; + .await?; } Ok(()) diff --git a/src/action/macos/encrypt_apfs_volume.rs b/src/action/macos/encrypt_apfs_volume.rs index 4b80992..e763d3f 100644 --- a/src/action/macos/encrypt_apfs_volume.rs +++ b/src/action/macos/encrypt_apfs_volume.rs @@ -1,6 +1,7 @@ use crate::{ action::{ - macos::NIX_VOLUME_MOUNTD_DEST, Action, ActionDescription, ActionError, StatefulAction, + macos::NIX_VOLUME_MOUNTD_DEST, Action, ActionDescription, ActionError, ActionTag, + StatefulAction, }, execute_command, }; @@ -36,6 +37,9 @@ impl EncryptApfsVolume { #[async_trait::async_trait] #[typetag::serde(name = "encrypt_volume")] impl Action for EncryptApfsVolume { + fn action_tag() -> ActionTag { + ActionTag("encrypt_apfs_volume") + } fn tracing_synopsis(&self) -> String { format!( "Encrypt volume `{}` on disk `{}`", @@ -80,9 +84,7 @@ impl Action for EncryptApfsVolume { let disk_str = disk.to_str().expect("Could not turn disk into string"); /* Should not reasonably ever fail */ - execute_command(Command::new("/usr/sbin/diskutil").arg("mount").arg(&name)) - .await - .map_err(ActionError::Command)?; + execute_command(Command::new("/usr/sbin/diskutil").arg("mount").arg(&name)).await?; // Add the password to the user keychain so they can unlock it later. execute_command( @@ -112,8 +114,7 @@ impl Action for EncryptApfsVolume { "/Library/Keychains/System.keychain", ]), ) - .await - .map_err(ActionError::Command)?; + .await?; // Encrypt the mounted volume execute_command(Command::new("/usr/sbin/diskutil").process_group(0).args([ @@ -125,8 +126,7 @@ impl Action for EncryptApfsVolume { "-passphrase", password.as_str(), ])) - .await - .map_err(ActionError::Command)?; + .await?; execute_command( Command::new("/usr/sbin/diskutil") @@ -135,8 +135,7 @@ impl Action for EncryptApfsVolume { .arg("force") .arg(&name), ) - .await - .map_err(ActionError::Command)?; + .await?; Ok(()) } @@ -178,8 +177,7 @@ impl Action for EncryptApfsVolume { .as_str(), ]), ) - .await - .map_err(ActionError::Command)?; + .await?; Ok(()) } diff --git a/src/action/macos/unmount_apfs_volume.rs b/src/action/macos/unmount_apfs_volume.rs index e8ba716..38b3fa8 100644 --- a/src/action/macos/unmount_apfs_volume.rs +++ b/src/action/macos/unmount_apfs_volume.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use tokio::process::Command; use tracing::{span, Span}; -use crate::action::{ActionError, StatefulAction}; +use crate::action::{ActionError, ActionTag, StatefulAction}; use crate::execute_command; use crate::action::{Action, ActionDescription}; @@ -31,6 +31,9 @@ impl UnmountApfsVolume { #[async_trait::async_trait] #[typetag::serde(name = "unmount_volume")] impl Action for UnmountApfsVolume { + fn action_tag() -> ActionTag { + ActionTag("unmount_apfs_volume") + } fn tracing_synopsis(&self) -> String { format!("Unmount the `{}` APFS volume", self.name) } @@ -59,8 +62,7 @@ impl Action for UnmountApfsVolume { .arg(name) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; Ok(()) } @@ -80,8 +82,7 @@ impl Action for UnmountApfsVolume { .arg(name) .stdin(std::process::Stdio::null()), ) - .await - .map_err(|e| ActionError::Command(e))?; + .await?; Ok(()) } diff --git a/src/action/mod.rs b/src/action/mod.rs index 0505ecf..f36a10e 100644 --- a/src/action/mod.rs +++ b/src/action/mod.rs @@ -68,6 +68,9 @@ impl MyAction { #[async_trait::async_trait] #[typetag::serde(name = "my_action")] impl Action for MyAction { + fn action_tag() -> nix_installer::action::ActionTag { + "my_action".into() + } fn tracing_synopsis(&self) -> String { "My action".to_string() } @@ -171,7 +174,7 @@ pub mod macos; mod stateful; pub use stateful::{ActionState, StatefulAction}; -use std::error::Error; +use std::{error::Error, process::Output}; use tokio::task::JoinError; use tracing::Span; @@ -185,6 +188,9 @@ use crate::error::HasExpectedErrors; #[async_trait::async_trait] #[typetag::serde(tag = "action")] pub trait Action: Send + Sync + std::fmt::Debug + dyn_clone::DynClone { + fn action_tag() -> ActionTag + where + Self: Sized; /// A synopsis of the action for tracing purposes fn tracing_synopsis(&self) -> String; /// A tracing span suitable for the action @@ -252,6 +258,27 @@ impl ActionDescription { } } +/// A 'tag' name an action has that corresponds to the one we serialize in [`typetag]` +pub struct ActionTag(&'static str); + +impl std::fmt::Display for ActionTag { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.0) + } +} + +impl std::fmt::Debug for ActionTag { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.0) + } +} + +impl From<&'static str> for ActionTag { + fn from(value: &'static str) -> Self { + Self(value) + } +} + /// An error occurring during an action #[non_exhaustive] #[derive(thiserror::Error, Debug, strum::IntoStaticStr)] @@ -260,10 +287,10 @@ pub enum ActionError { #[error(transparent)] Custom(Box), /// A child error - #[error(transparent)] - Child(#[from] Box), + #[error("Child action `{0}`")] + Child(ActionTag, #[source] Box), /// Several child errors - #[error("Multiple errors: {}", .0.iter().map(|v| { + #[error("Child action errors: {}", .0.iter().map(|v| { if let Some(source) = v.source() { format!("{v} ({source})") } else { @@ -341,8 +368,33 @@ pub enum ActionError { #[error("Chowning path `{0}`")] Chown(std::path::PathBuf, #[source] nix::errno::Errno), /// Failed to execute command - #[error("Failed to execute command")] - Command(#[source] std::io::Error), + #[error("Failed to execute command `{command}`", + command = .command, + )] + Command { + #[cfg(feature = "diagnostics")] + program: String, + command: String, + #[source] + error: std::io::Error, + }, + #[error( + "Failed to execute command{maybe_status} `{command}`, stdout: {stdout}\nstderr: {stderr}\n", + command = .command, + stdout = String::from_utf8_lossy(&.output.stdout), + stderr = String::from_utf8_lossy(&.output.stderr), + maybe_status = if let Some(status) = .output.status.code() { + format!(" with status {status}") + } else { + "".to_string() + } + )] + CommandOutput { + #[cfg(feature = "diagnostics")] + program: String, + command: String, + output: Output, + }, #[error("Joining spawned async task")] Join( #[source] @@ -360,6 +412,25 @@ pub enum ActionError { Plist(#[from] plist::Error), } +impl ActionError { + pub fn command(command: &tokio::process::Command, error: std::io::Error) -> Self { + Self::Command { + #[cfg(feature = "diagnostics")] + program: command.as_std().get_program().to_string_lossy().into(), + command: format!("{:?}", command.as_std()), + error, + } + } + pub fn command_output(command: &tokio::process::Command, output: std::process::Output) -> Self { + Self::CommandOutput { + #[cfg(feature = "diagnostics")] + program: command.as_std().get_program().to_string_lossy().into(), + command: format!("{:?}", command.as_std()), + output, + } + } +} + impl HasExpectedErrors for ActionError { fn expected<'a>(&'a self) -> Option> { match self { @@ -370,3 +441,56 @@ impl HasExpectedErrors for ActionError { } } } + +#[cfg(feature = "diagnostics")] +impl crate::diagnostics::ErrorDiagnostic for ActionError { + fn diagnostic(&self) -> String { + let static_str: &'static str = (self).into(); + let context = match self { + Self::Child(action, _) => vec![action.to_string()], + Self::Read(path, _) + | Self::Open(path, _) + | Self::Write(path, _) + | Self::Flush(path, _) + | Self::SetPermissions(_, path, _) + | Self::GettingMetadata(path, _) + | Self::CreateDirectory(path, _) + | Self::PathWasNotFile(path) => { + vec![path.to_string_lossy().to_string()] + }, + Self::Rename(first_path, second_path, _) + | Self::Copy(first_path, second_path, _) + | Self::Symlink(first_path, second_path, _) => { + vec![ + first_path.to_string_lossy().to_string(), + second_path.to_string_lossy().to_string(), + ] + }, + Self::NoGroup(name) | Self::NoUser(name) => { + vec![name.clone()] + }, + Self::Command { + program, + command: _, + error: _, + } + | Self::CommandOutput { + program, + command: _, + output: _, + } => { + vec![program.clone()] + }, + _ => vec![], + }; + return format!( + "{}({})", + static_str, + context + .iter() + .map(|v| format!("\"{v}\"")) + .collect::>() + .join(", ") + ); + } +} diff --git a/src/action/stateful.rs b/src/action/stateful.rs index e2929cc..3a997fa 100644 --- a/src/action/stateful.rs +++ b/src/action/stateful.rs @@ -1,7 +1,7 @@ use serde::{Deserialize, Serialize}; use tracing::{Instrument, Span}; -use super::{Action, ActionDescription, ActionError}; +use super::{Action, ActionDescription, ActionError, ActionTag}; /// A wrapper around an [`Action`](crate::action::Action) which tracks the [`ActionState`] and /// handles some tracing output @@ -24,6 +24,9 @@ where } impl StatefulAction> { + pub fn inner_typetag_name(&self) -> &'static str { + self.action.typetag_name() + } pub fn tracing_synopsis(&self) -> String { self.action.tracing_synopsis() } @@ -109,6 +112,12 @@ impl StatefulAction where A: Action, { + pub fn tag() -> ActionTag { + A::action_tag() + } + pub fn action_tag(&self) -> ActionTag { + A::action_tag() + } pub fn tracing_synopsis(&self) -> String { self.action.tracing_synopsis() } diff --git a/src/bin/nix-installer.rs b/src/bin/nix-installer.rs index 6bc1064..7f27cd3 100644 --- a/src/bin/nix-installer.rs +++ b/src/bin/nix-installer.rs @@ -4,8 +4,9 @@ use clap::Parser; use nix_installer::cli::CommandExecute; #[tokio::main] -async fn main() -> color_eyre::Result { +async fn main() -> eyre::Result { color_eyre::config::HookBuilder::default() + .issue_url(concat!(env!("CARGO_PKG_REPOSITORY"), "/issues/new")) .theme(if !atty::is(atty::Stream::Stderr) { color_eyre::config::Theme::new() } else { diff --git a/src/cli/subcommand/install.rs b/src/cli/subcommand/install.rs index eee10b1..3923276 100644 --- a/src/cli/subcommand/install.rs +++ b/src/cli/subcommand/install.rs @@ -139,7 +139,7 @@ impl CommandExecute for Install { eprintln!("{}", expected.red()); return Ok(ExitCode::FAILURE); } - return Err(err.into()) + return Err(err)?; } } }, @@ -212,12 +212,12 @@ impl CommandExecute for Install { let rx2 = tx.subscribe(); let res = install_plan.uninstall(rx2).await; - if let Err(e) = res { - if let Some(expected) = e.expected() { + if let Err(err) = res { + if let Some(expected) = err.expected() { eprintln!("{}", expected.red()); return Ok(ExitCode::FAILURE); } - return Err(e.into()); + return Err(err)?; } else { println!( "\ @@ -233,7 +233,7 @@ impl CommandExecute for Install { } let error = eyre!(err).wrap_err("Install failure"); - return Err(error); + return Err(error)?; } }, Ok(_) => { diff --git a/src/cli/subcommand/plan.rs b/src/cli/subcommand/plan.rs index a1d4877..9fb9356 100644 --- a/src/cli/subcommand/plan.rs +++ b/src/cli/subcommand/plan.rs @@ -25,21 +25,19 @@ impl CommandExecute for Plan { let planner = match planner { Some(planner) => planner, - None => BuiltinPlanner::default() - .await - .map_err(|e| eyre::eyre!(e))?, + None => BuiltinPlanner::default().await?, }; let res = planner.plan().await; let install_plan = match res { Ok(plan) => plan, - Err(e) => { - if let Some(expected) = e.expected() { + Err(err) => { + if let Some(expected) = err.expected() { eprintln!("{}", expected.red()); return Ok(ExitCode::FAILURE); } - return Err(e.into()); + return Err(err)?; }, }; diff --git a/src/cli/subcommand/uninstall.rs b/src/cli/subcommand/uninstall.rs index def2067..04de8c5 100644 --- a/src/cli/subcommand/uninstall.rs +++ b/src/cli/subcommand/uninstall.rs @@ -124,12 +124,12 @@ impl CommandExecute for Uninstall { let (_tx, rx) = signal_channel().await?; let res = plan.uninstall(rx).await; - if let Err(e) = res { - if let Some(expected) = e.expected() { + if let Err(err) = res { + if let Some(expected) = err.expected() { println!("{}", expected.red()); return Ok(ExitCode::FAILURE); } - return Err(e.into()); + return Err(err)?; } // TODO(@hoverbear): It would be so nice to catch errors and offer the user a way to keep going... diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 9b52e90..097d73f 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -10,6 +10,10 @@ use std::time::Duration; use os_release::OsRelease; use reqwest::Url; +use crate::{ + action::ActionError, planner::PlannerError, settings::InstallSettingsError, NixInstallerError, +}; + /// The static of an action attempt #[derive(Debug, serde::Deserialize, serde::Serialize, Clone)] pub enum DiagnosticStatus { @@ -39,7 +43,7 @@ pub struct DiagnosticReport { pub action: DiagnosticAction, pub status: DiagnosticStatus, /// Generally this includes the [`strum::IntoStaticStr`] representation of the error, we take special care not to include parameters of the error (which may include secrets) - pub failure_variant: Option, + pub failure_chain: Option>, } /// A preparation of data to be sent to the `endpoint`. @@ -53,7 +57,8 @@ pub struct DiagnosticData { triple: String, is_ci: bool, endpoint: Option, - failure_variant: Option, + /// Generally this includes the [`strum::IntoStaticStr`] representation of the error, we take special care not to include parameters of the error (which may include secrets) + failure_chain: Option>, } impl DiagnosticData { @@ -73,12 +78,42 @@ impl DiagnosticData { os_version, triple: target_lexicon::HOST.to_string(), is_ci, - failure_variant: None, + failure_chain: None, } } - pub fn variant(mut self, variant: String) -> Self { - self.failure_variant = Some(variant); + pub fn failure(mut self, err: &NixInstallerError) -> Self { + let mut failure_chain = vec![]; + let diagnostic = err.diagnostic(); + failure_chain.push(diagnostic); + + let mut walker: &dyn std::error::Error = &err; + while let Some(source) = walker.source() { + if let Some(downcasted) = source.downcast_ref::() { + let downcasted_diagnostic = downcasted.diagnostic(); + failure_chain.push(downcasted_diagnostic); + } + if let Some(downcasted) = source.downcast_ref::>() { + let downcasted_diagnostic = downcasted.diagnostic(); + failure_chain.push(downcasted_diagnostic); + } + if let Some(downcasted) = source.downcast_ref::() { + let downcasted_diagnostic = downcasted.diagnostic(); + failure_chain.push(downcasted_diagnostic); + } + if let Some(downcasted) = source.downcast_ref::() { + let downcasted_diagnostic = downcasted.diagnostic(); + failure_chain.push(downcasted_diagnostic); + } + if let Some(downcasted) = source.downcast_ref::() { + let downcasted_diagnostic = downcasted.diagnostic(); + failure_chain.push(downcasted_diagnostic); + } + + walker = source; + } + + self.failure_chain = Some(failure_chain); self } @@ -92,7 +127,7 @@ impl DiagnosticData { triple, is_ci, endpoint: _, - failure_variant: variant, + failure_chain, } = self; DiagnosticReport { version: version.clone(), @@ -104,7 +139,7 @@ impl DiagnosticData { is_ci: *is_ci, action, status, - failure_variant: variant.clone(), + failure_chain: failure_chain.clone(), } } @@ -153,7 +188,7 @@ impl DiagnosticData { } #[non_exhaustive] -#[derive(thiserror::Error, Debug)] +#[derive(thiserror::Error, Debug, strum::IntoStaticStr)] pub enum DiagnosticError { #[error("Unknown url scheme")] UnknownUrlScheme, @@ -172,3 +207,14 @@ pub enum DiagnosticError { serde_json::Error, ), } + +pub trait ErrorDiagnostic { + fn diagnostic(&self) -> String; +} + +impl ErrorDiagnostic for DiagnosticError { + fn diagnostic(&self) -> String { + let static_str: &'static str = (self).into(); + return static_str.to_string(); + } +} diff --git a/src/error.rs b/src/error.rs index aabda62..3c19ea0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,18 +1,18 @@ use std::path::PathBuf; -use crate::{action::ActionError, planner::PlannerError, settings::InstallSettingsError}; +use crate::{ + action::{ActionError, ActionTag}, + planner::PlannerError, + settings::InstallSettingsError, +}; /// An error occurring during a call defined in this crate #[non_exhaustive] #[derive(thiserror::Error, Debug, strum::IntoStaticStr)] pub enum NixInstallerError { /// An error originating from an [`Action`](crate::action::Action) - #[error("Error executing action")] - Action( - #[source] - #[from] - ActionError, - ), + #[error("Error executing action `{0}`")] + Action(ActionTag, #[source] ActionError), /// An error while writing the [`InstallPlan`](crate::InstallPlan) #[error("Recording install receipt")] RecordingReceipt(PathBuf, #[source] std::io::Error), @@ -72,7 +72,7 @@ pub(crate) trait HasExpectedErrors: std::error::Error + Sized + Send + Sync { impl HasExpectedErrors for NixInstallerError { fn expected<'a>(&'a self) -> Option> { match self { - NixInstallerError::Action(action_error) => action_error.expected(), + NixInstallerError::Action(_, action_error) => action_error.expected(), NixInstallerError::RecordingReceipt(_, _) => None, NixInstallerError::CopyingSelf(_) => None, NixInstallerError::SerializingReceipt(_) => None, @@ -85,3 +85,23 @@ impl HasExpectedErrors for NixInstallerError { } } } + +#[cfg(feature = "diagnostics")] +impl crate::diagnostics::ErrorDiagnostic for NixInstallerError { + fn diagnostic(&self) -> String { + let static_str: &'static str = (self).into(); + let context = match self { + Self::Action(action, _) => vec![action.to_string()], + _ => vec![], + }; + return format!( + "{}({})", + static_str, + context + .iter() + .map(|v| format!("\"{v}\"")) + .collect::>() + .join(", ") + ); + } +} diff --git a/src/lib.rs b/src/lib.rs index 65cd6f1..7ca6b97 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,7 +83,7 @@ pub mod settings; use std::{ffi::OsStr, process::Output}; -use action::Action; +use action::{Action, ActionError}; pub use channel_value::ChannelValue; pub use error::NixInstallerError; @@ -93,24 +93,15 @@ use planner::BuiltinPlanner; use tokio::process::Command; #[tracing::instrument(level = "debug", skip_all, fields(command = %format!("{:?}", command.as_std())))] -async fn execute_command(command: &mut Command) -> Result { - let command_str = format!("{:?}", command.as_std()); - tracing::trace!("Executing `{command_str}`"); - let output = command.output().await?; +async fn execute_command(command: &mut Command) -> Result { + tracing::trace!("Executing `{:?}`", command.as_std()); + let output = command + .output() + .await + .map_err(|e| ActionError::command(command, e))?; match output.status.success() { true => Ok(output), - false => Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!( - "Command `{command_str}` failed{}, stderr:\n{}\n", - if let Some(code) = output.status.code() { - format!(" status {code}") - } else { - "".to_string() - }, - String::from_utf8_lossy(&output.stderr) - ), - )), + false => Err(ActionError::command_output(command, output)), } } diff --git a/src/plan.rs b/src/plan.rs index d144835..d65e531 100644 --- a/src/plan.rs +++ b/src/plan.rs @@ -154,18 +154,17 @@ impl InstallPlan { } tracing::info!("Step: {}", action.tracing_synopsis()); + let typetag_name = action.inner_typetag_name(); if let Err(err) = action.try_execute().await { if let Err(err) = write_receipt(self.clone()).await { tracing::error!("Error saving receipt: {:?}", err); } + let err = NixInstallerError::Action(typetag_name.into(), err); #[cfg(feature = "diagnostics")] if let Some(diagnostic_data) = &self.diagnostic_data { diagnostic_data .clone() - .variant({ - let x: &'static str = (&err).into(); - x.to_string() - }) + .failure(&err) .send( crate::diagnostics::DiagnosticAction::Install, crate::diagnostics::DiagnosticStatus::Failure, @@ -173,7 +172,7 @@ impl InstallPlan { .await?; } - return Err(NixInstallerError::Action(err)); + return Err(err); } } @@ -287,25 +286,24 @@ impl InstallPlan { } tracing::info!("Revert: {}", action.tracing_synopsis()); + let typetag_name = action.inner_typetag_name(); if let Err(err) = action.try_revert().await { if let Err(err) = write_receipt(self.clone()).await { tracing::error!("Error saving receipt: {:?}", err); } + let err = NixInstallerError::Action(typetag_name.into(), err); #[cfg(feature = "diagnostics")] if let Some(diagnostic_data) = &self.diagnostic_data { diagnostic_data .clone() - .variant({ - let x: &'static str = (&err).into(); - x.to_string() - }) + .failure(&err) .send( crate::diagnostics::DiagnosticAction::Uninstall, crate::diagnostics::DiagnosticStatus::Failure, ) .await?; } - return Err(NixInstallerError::Action(err)); + return Err(err); } } diff --git a/src/planner/mod.rs b/src/planner/mod.rs index 75a05f8..4bc28be 100644 --- a/src/planner/mod.rs +++ b/src/planner/mod.rs @@ -322,3 +322,11 @@ impl HasExpectedErrors for PlannerError { } } } + +#[cfg(feature = "diagnostics")] +impl crate::diagnostics::ErrorDiagnostic for PlannerError { + fn diagnostic(&self) -> String { + let static_str: &'static str = (self).into(); + return static_str.to_string(); + } +} diff --git a/src/settings.rs b/src/settings.rs index c33f44e..1b05c77 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -562,7 +562,7 @@ impl InitSettings { /// An error originating from a [`Planner::settings`](crate::planner::Planner::settings) #[non_exhaustive] -#[derive(thiserror::Error, Debug)] +#[derive(thiserror::Error, Debug, strum::IntoStaticStr)] pub enum InstallSettingsError { /// `nix-installer` does not support the architecture right now #[error("`nix-installer` does not support the `{0}` architecture right now")] @@ -584,3 +584,11 @@ pub enum InstallSettingsError { #[error("No supported init system found")] InitNotSupported, } + +#[cfg(feature = "diagnostics")] +impl crate::diagnostics::ErrorDiagnostic for InstallSettingsError { + fn diagnostic(&self) -> String { + let static_str: &'static str = (self).into(); + return static_str.to_string(); + } +} diff --git a/tests/fixtures/linux/linux.json b/tests/fixtures/linux/linux.json index dcc5a7c..bd69213 100644 --- a/tests/fixtures/linux/linux.json +++ b/tests/fixtures/linux/linux.json @@ -884,7 +884,7 @@ }, { "action": { - "action": "configure_nix_daemon", + "action": "configure_init_service", "init": "Systemd", "start_daemon": true }, diff --git a/tests/fixtures/linux/steam-deck.json b/tests/fixtures/linux/steam-deck.json index 9805009..26cc16e 100644 --- a/tests/fixtures/linux/steam-deck.json +++ b/tests/fixtures/linux/steam-deck.json @@ -906,7 +906,7 @@ }, { "action": { - "action": "configure_nix_daemon", + "action": "configure_init_service", "init": "Systemd", "start_daemon": true }, diff --git a/tests/fixtures/macos/macos.json b/tests/fixtures/macos/macos.json index 5200f39..3b943b1 100644 --- a/tests/fixtures/macos/macos.json +++ b/tests/fixtures/macos/macos.json @@ -938,7 +938,7 @@ }, { "action": { - "action": "configure_nix_daemon", + "action": "configure_init_service", "init": "Launchd", "start_daemon": true },