From 38ac18005269538e873f773400a999fa6bbaba0a Mon Sep 17 00:00:00 2001 From: Ana Hobden Date: Wed, 23 Nov 2022 09:18:38 -0800 Subject: [PATCH] Tidy tracing (#57) * Tidy tracing * Forgot a few changes * Remove more boilerplate * Repair Plan::describe_revert * More valid default settings * fmt * Use correct execute/revert calls * Split up Linux Daemon disable and stop * Detect state and act on it * Fixup pathes * Add a missing step to the mac bits * Unload instead of disable * Prune out again * Squelch some stdout * Clean lint * Better log for no-delete-directory case * Even more verbose messages on CreateDirectory * Less broken code * Use try_execute where it should be used * Final tweaks * Add some docs --- .../base/configure_nix_daemon_service.rs | 64 +++++----- src/action/base/create_directory.rs | 85 +++++--------- src/action/base/create_file.rs | 60 +++------- src/action/base/create_group.rs | 57 ++++----- src/action/base/create_or_append_file.rs | 69 ++++------- src/action/base/create_user.rs | 87 ++++++-------- src/action/base/fetch_nix.rs | 55 ++++----- src/action/base/move_unpacked_nix.rs | 49 ++++---- src/action/base/setup_default_profile.rs | 54 +++------ src/action/common/configure_nix.rs | 100 +++++++--------- src/action/common/configure_shell_profile.rs | 60 ++++------ src/action/common/create_nix_tree.rs | 110 ++++++++--------- src/action/common/create_users_and_group.rs | 64 ++++------ .../common/place_channel_configuration.rs | 79 ++++--------- src/action/common/place_nix_configuration.rs | 74 +++++------- src/action/common/provision_nix.rs | 82 ++++++------- src/action/darwin/bootstrap_volume.rs | 57 ++++----- src/action/darwin/create_apfs_volume.rs | 111 +++++++----------- src/action/darwin/create_synthetic_objects.rs | 54 +++------ src/action/darwin/create_volume.rs | 67 ++++------- src/action/darwin/enable_ownership.rs | 52 +++----- src/action/darwin/encrypt_volume.rs | 55 ++++----- .../darwin/kickstart_launchctl_service.rs | 61 +++------- src/action/darwin/unmount_volume.rs | 54 +++------ src/action/linux/create_systemd_sysext.rs | 92 ++++++--------- src/action/linux/mod.rs | 2 - src/action/linux/start_systemd_unit.rs | 65 +++++----- src/action/linux/systemd_sysext_merge.rs | 41 ++----- src/action/mod.rs | 66 ++++++++++- src/cli/subcommand/install.rs | 8 +- src/cli/subcommand/uninstall.rs | 2 +- src/lib.rs | 2 +- src/plan.rs | 52 +++++--- 33 files changed, 796 insertions(+), 1194 deletions(-) diff --git a/src/action/base/configure_nix_daemon_service.rs b/src/action/base/configure_nix_daemon_service.rs index 03dffb5..8acca11 100644 --- a/src/action/base/configure_nix_daemon_service.rs +++ b/src/action/base/configure_nix_daemon_service.rs @@ -48,20 +48,19 @@ impl ConfigureNixDaemonService { #[async_trait::async_trait] #[typetag::serde(name = "configure_nix_daemon")] impl Action for ConfigureNixDaemonService { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - "Configure Nix daemon related settings with systemd".to_string(), - vec![ - "Run `systemd-tempfiles --create --prefix=/nix/var/nix`".to_string(), - "Run `systemctl link {SERVICE_SRC}`".to_string(), - "Run `systemctl link {SOCKET_SRC}`".to_string(), - "Run `systemctl daemon-reload`".to_string(), - ], - )] - } + fn tracing_synopsis(&self) -> String { + "Configure Nix daemon related settings with systemd".to_string() + } + fn execute_description(&self) -> Vec { + vec![ActionDescription::new( + self.tracing_synopsis(), + vec![ + "Run `systemd-tempfiles --create --prefix=/nix/var/nix`".to_string(), + "Run `systemctl link {SERVICE_SRC}`".to_string(), + "Run `systemctl link {SOCKET_SRC}`".to_string(), + "Run `systemctl daemon-reload`".to_string(), + ], + )] } #[tracing::instrument(skip_all)] @@ -166,31 +165,20 @@ impl Action for ConfigureNixDaemonService { Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - "Unconfigure Nix daemon related settings with systemd".to_string(), - vec![ - "Run `systemctl disable {SOCKET_SRC}`".to_string(), - "Run `systemctl disable {SERVICE_SRC}`".to_string(), - "Run `systemd-tempfiles --remove --prefix=/nix/var/nix`".to_string(), - "Run `systemctl daemon-reload`".to_string(), - ], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + "Unconfigure Nix daemon related settings with systemd".to_string(), + vec![ + "Run `systemctl disable {SOCKET_SRC}`".to_string(), + "Run `systemctl disable {SERVICE_SRC}`".to_string(), + "Run `systemd-tempfiles --remove --prefix=/nix/var/nix`".to_string(), + "Run `systemctl daemon-reload`".to_string(), + ], + )] } #[tracing::instrument(skip_all)] async fn revert(&mut self) -> Result<(), Box> { - let Self { action_state } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unconfiguring nix daemon service"); - return Ok(()); - } - tracing::debug!("Unconfiguring nix daemon service"); - match OperatingSystem::host() { OperatingSystem::MacOSX { major: _, @@ -278,14 +266,16 @@ impl Action for ConfigureNixDaemonService { }, }; - tracing::trace!("Unconfigured nix daemon service"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/base/create_directory.rs b/src/action/base/create_directory.rs index e40edda..dd92dd8 100644 --- a/src/action/base/create_directory.rs +++ b/src/action/base/create_directory.rs @@ -39,6 +39,10 @@ impl CreateDirectory { CreateDirectoryError::GettingMetadata(path.to_path_buf(), e).boxed() })?; if metadata.is_dir() { + tracing::debug!( + "Creating directory `{}` already complete, skipping", + path.display(), + ); // TODO: Validate owner/group... ActionState::Completed } else { @@ -69,23 +73,12 @@ impl CreateDirectory { #[async_trait::async_trait] #[typetag::serde(name = "create_directory")] impl Action for CreateDirectory { - fn describe_execute(&self) -> Vec { - let Self { - path, - user: _, - group: _, - mode: _, - force_prune_on_revert: _, - action_state, - } = &self; - if *action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Create the directory `{}`", path.display()), - vec![], - )] - } + fn tracing_synopsis(&self) -> String { + format!("Create directory `{}`", self.path.display()) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -101,13 +94,8 @@ impl Action for CreateDirectory { group, mode, force_prune_on_revert: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating directory"); - return Ok(()); - } - tracing::debug!("Creating directory"); let gid = if let Some(group) = group { Some( @@ -143,12 +131,10 @@ impl Action for CreateDirectory { })?; } - tracing::trace!("Created directory"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { + fn revert_description(&self) -> Vec { let Self { path, user: _, @@ -157,22 +143,18 @@ impl Action for CreateDirectory { force_prune_on_revert, action_state: _, } = &self; - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!( - "Remove the directory `{}`{}", - path.display(), - if *force_prune_on_revert { - "" - } else { - " if no other contents exists" - } - ), - vec![], - )] - } + vec![ActionDescription::new( + format!( + "Remove the directory `{}`{}", + path.display(), + if *force_prune_on_revert { + "" + } else { + " if no other contents exists" + } + ), + vec![], + )] } #[tracing::instrument(skip_all, fields( @@ -188,15 +170,8 @@ impl Action for CreateDirectory { group: _, mode: _, force_prune_on_revert, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Removing directory"); - return Ok(()); - } - tracing::debug!("Removing directory"); - - tracing::trace!(path = %path.display(), "Removing directory"); let is_empty = path .read_dir() @@ -207,17 +182,21 @@ impl Action for CreateDirectory { (true, _) | (false, true) => remove_dir_all(path.clone()) .await .map_err(|e| CreateDirectoryError::Removing(path.clone(), e).boxed())?, - (false, false) => {}, + (false, false) => { + tracing::debug!("Not removing `{}`, the folder is not empty", path.display()); + }, }; - tracing::trace!("Removed directory"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/base/create_file.rs b/src/action/base/create_file.rs index 9b462f2..573d678 100644 --- a/src/action/base/create_file.rs +++ b/src/action/base/create_file.rs @@ -53,24 +53,11 @@ impl CreateFile { #[async_trait::async_trait] #[typetag::serde(name = "create_file")] impl Action for CreateFile { - fn describe_execute(&self) -> Vec { - let Self { - path, - user: _, - group: _, - mode: _, - buf: _, - force: _, - action_state: _, - } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Create or overwrite file `{}`", path.display()), - vec![], - )] - } + fn tracing_synopsis(&self) -> String { + format!("Create or overwrite file `{}`", self.path.display()) + } + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -87,13 +74,8 @@ impl Action for CreateFile { mode, buf, force: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating file"); - return Ok(()); - } - tracing::debug!("Creating file"); let mut options = OpenOptions::new(); options.create_new(true).write(true).read(true); @@ -133,12 +115,10 @@ impl Action for CreateFile { }; chown(path, uid, gid).map_err(|e| CreateFileError::Chown(path.clone(), e).boxed())?; - tracing::trace!("Created file"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { + fn revert_description(&self) -> Vec { let Self { path, user: _, @@ -148,14 +128,11 @@ impl Action for CreateFile { force: _, action_state: _, } = &self; - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!("Delete file `{}`", path.display()), - vec![format!("Delete file `{}`", path.display())], - )] - } + + vec![ActionDescription::new( + format!("Delete file `{}`", path.display()), + vec![format!("Delete file `{}`", path.display())], + )] } #[tracing::instrument(skip_all, fields( @@ -172,26 +149,23 @@ impl Action for CreateFile { mode: _, buf: _, force: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Deleting file"); - return Ok(()); - } - tracing::debug!("Deleting file"); remove_file(&path) .await .map_err(|e| CreateFileError::RemoveFile(path.to_owned(), e).boxed())?; - tracing::trace!("Deleted file"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/base/create_group.rs b/src/action/base/create_group.rs index 635251b..41dee0f 100644 --- a/src/action/base/create_group.rs +++ b/src/action/base/create_group.rs @@ -28,22 +28,21 @@ impl CreateGroup { #[async_trait::async_trait] #[typetag::serde(name = "create_group")] impl Action for CreateGroup { - fn describe_execute(&self) -> Vec { + fn tracing_synopsis(&self) -> String { + format!("Create group `{}` with GID `{}`", self.name, self.gid) + } + fn execute_description(&self) -> Vec { let Self { - name, - gid, + name: _, + gid: _, action_state: _, } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Create group {name} with GID {gid}"), - vec![format!( - "The nix daemon requires a system user group its system users can be part of" - )], - )] - } + vec![ActionDescription::new( + self.tracing_synopsis(), + vec![format!( + "The nix daemon requires a system user group its system users can be part of" + )], + )] } #[tracing::instrument(skip_all, fields( @@ -73,6 +72,7 @@ impl Action for CreateGroup { if Command::new("/usr/bin/dscl") .args([".", "-read", &format!("/Groups/{name}")]) .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) .status() .await? .success() @@ -112,22 +112,18 @@ impl Action for CreateGroup { Ok(()) } - fn describe_revert(&self) -> Vec { + fn revert_description(&self) -> Vec { let Self { name, gid: _, action_state: _, } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Delete group {name}"), - vec![format!( - "The nix daemon requires a system user group its system users can be part of" - )], - )] - } + vec![ActionDescription::new( + format!("Delete group {name}"), + vec![format!( + "The nix daemon requires a system user group its system users can be part of" + )], + )] } #[tracing::instrument(skip_all, fields( @@ -138,13 +134,8 @@ impl Action for CreateGroup { let Self { name, gid: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Deleting group"); - return Ok(()); - } - tracing::debug!("Deleting group"); use target_lexicon::OperatingSystem; match target_lexicon::OperatingSystem::host() { @@ -176,14 +167,16 @@ impl Action for CreateGroup { }, }; - tracing::trace!("Deleted group"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/base/create_or_append_file.rs b/src/action/base/create_or_append_file.rs index d4c6e52..1df96fe 100644 --- a/src/action/base/create_or_append_file.rs +++ b/src/action/base/create_or_append_file.rs @@ -50,23 +50,12 @@ impl CreateOrAppendFile { #[async_trait::async_trait] #[typetag::serde(name = "create_or_append_file")] impl Action for CreateOrAppendFile { - fn describe_execute(&self) -> Vec { - let Self { - path, - user: _, - group: _, - mode: _, - buf: _, - action_state: _, - } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Create or append file `{}`", path.display()), - vec![], - )] - } + fn tracing_synopsis(&self) -> String { + format!("Create or append file `{}`", self.path.display()) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -82,13 +71,8 @@ impl Action for CreateOrAppendFile { group, mode, buf, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating or appending fragment to file"); - return Ok(()); - } - tracing::debug!("Creating or appending fragment to file"); let mut file = OpenOptions::new() .create(true) @@ -138,12 +122,10 @@ impl Action for CreateOrAppendFile { chown(path, uid, gid) .map_err(|e| CreateOrAppendFileError::Chown(path.clone(), e).boxed())?; - tracing::trace!("Created or appended fragment to file"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { + fn revert_description(&self) -> Vec { let Self { path, user: _, @@ -152,17 +134,13 @@ impl Action for CreateOrAppendFile { buf, action_state: _, } = &self; - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!("Delete Nix related fragment from file `{}`", path.display()), - vec![format!( - "Delete Nix related fragment from file `{}`. Fragment: `{buf}`", - path.display() - )], - )] - } + vec![ActionDescription::new( + format!("Delete Nix related fragment from file `{}`", path.display()), + vec![format!( + "Delete Nix related fragment from file `{}`. Fragment: `{buf}`", + path.display() + )], + )] } #[tracing::instrument(skip_all, fields( @@ -178,14 +156,8 @@ impl Action for CreateOrAppendFile { group: _, mode: _, buf, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already completed: Removing fragment from file (and deleting it if it becomes empty)"); - return Ok(()); - } - tracing::debug!("Removing fragment from file (and deleting it if it becomes empty)"); - let mut file = OpenOptions::new() .create(false) .write(true) @@ -208,8 +180,6 @@ impl Action for CreateOrAppendFile { remove_file(&path) .await .map_err(|e| CreateOrAppendFileError::RemoveFile(path.to_owned(), e).boxed())?; - - tracing::trace!("Removed file (since all content was removed)"); } else { file.seek(SeekFrom::Start(0)) .await @@ -217,16 +187,17 @@ impl Action for CreateOrAppendFile { file.write_all(file_contents.as_bytes()) .await .map_err(|e| CreateOrAppendFileError::WriteFile(path.to_owned(), e).boxed())?; - - tracing::trace!("Removed fragment from from file"); } - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/base/create_user.rs b/src/action/base/create_user.rs index 24447da..f945e6c 100644 --- a/src/action/base/create_user.rs +++ b/src/action/base/create_user.rs @@ -32,25 +32,19 @@ impl CreateUser { #[async_trait::async_trait] #[typetag::serde(name = "create_user")] impl Action for CreateUser { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - let Self { - name, - uid, - groupname: _, - gid, - action_state: _, - } = self; - - vec![ActionDescription::new( - format!("Create user {name} with UID {uid} with group {gid}"), - vec![format!( - "The nix daemon requires system users it can act as in order to build" - )], - )] - } + fn tracing_synopsis(&self) -> String { + format!( + "Create user `{}` with UID `{}` with group `{}` (GID {})", + self.name, self.uid, self.groupname, self.gid + ) + } + fn execute_description(&self) -> Vec { + vec![ActionDescription::new( + self.tracing_synopsis(), + vec![format!( + "The nix daemon requires system users it can act as in order to build" + )], + )] } #[tracing::instrument(skip_all, fields( @@ -65,13 +59,8 @@ impl Action for CreateUser { uid, groupname, gid, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating user"); - return Ok(()); - } - tracing::debug!("Creating user"); use target_lexicon::OperatingSystem; match OperatingSystem::host() { @@ -87,6 +76,7 @@ impl Action for CreateUser { if Command::new("/usr/bin/dscl") .args([".", "-read", &format!("/Users/{name}")]) .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) .status() .await? .success() @@ -215,30 +205,24 @@ impl Action for CreateUser { }, } - tracing::trace!("Created user"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - let Self { - name, - uid, - groupname: _, - gid, - action_state: _, - } = self; + fn revert_description(&self) -> Vec { + let Self { + name, + uid, + groupname: _, + gid, + action_state: _, + } = self; - vec![ActionDescription::new( - format!("Delete user {name} with UID {uid} with group {gid}"), - vec![format!( - "The nix daemon requires system users it can act as in order to build" - )], - )] - } + vec![ActionDescription::new( + format!("Delete user {name} with UID {uid} with group {gid}"), + vec![format!( + "The nix daemon requires system users it can act as in order to build" + )], + )] } #[tracing::instrument(skip_all, fields( @@ -252,13 +236,8 @@ impl Action for CreateUser { uid: _, groupname: _, gid: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already completed: Deleting user"); - return Ok(()); - } - tracing::debug!("Deleting user"); use target_lexicon::OperatingSystem; match target_lexicon::OperatingSystem::host() { @@ -290,14 +269,16 @@ impl Action for CreateUser { }, }; - tracing::trace!("Deleted user"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/base/fetch_nix.rs b/src/action/base/fetch_nix.rs index e1f111b..0991df1 100644 --- a/src/action/base/fetch_nix.rs +++ b/src/action/base/fetch_nix.rs @@ -34,20 +34,18 @@ impl FetchNix { #[async_trait::async_trait] #[typetag::serde(name = "fetch_nix")] impl Action for FetchNix { - fn describe_execute(&self) -> Vec { - let Self { - url, - dest, - action_state: _, - } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Fetch Nix from `{url}`"), - vec![format!("Unpack it to `{}` (moved later)", dest.display())], - )] - } + fn tracing_synopsis(&self) -> String { + format!("Fetch Nix from `{}`", self.url) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new( + self.tracing_synopsis(), + vec![format!( + "Unpack it to `{}` (moved later)", + self.dest.display() + )], + )] } #[tracing::instrument(skip_all, fields( @@ -58,13 +56,8 @@ impl Action for FetchNix { let Self { url, dest, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Fetching Nix"); - return Ok(()); - } - tracing::debug!("Fetching Nix"); let res = reqwest::get(url.clone()) .await @@ -83,17 +76,11 @@ impl Action for FetchNix { .unpack(&dest_clone) .map_err(|e| FetchNixError::Unarchive(e).boxed())?; - tracing::trace!("Fetched Nix"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![/* Deliberately empty -- this is a noop */] - } + fn revert_description(&self) -> Vec { + vec![/* Deliberately empty -- this is a noop */] } #[tracing::instrument(skip_all, fields( @@ -104,21 +91,19 @@ impl Action for FetchNix { let Self { url: _, dest: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unfetch Nix (noop)"); - return Ok(()); - } - tracing::debug!("Unfetch Nix (noop)"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/base/move_unpacked_nix.rs b/src/action/base/move_unpacked_nix.rs index d545e23..68f857e 100644 --- a/src/action/base/move_unpacked_nix.rs +++ b/src/action/base/move_unpacked_nix.rs @@ -27,18 +27,18 @@ impl MoveUnpackedNix { #[async_trait::async_trait] #[typetag::serde(name = "mount_unpacked_nix")] impl Action for MoveUnpackedNix { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Move the downloaded Nix into `/nix`"), - vec![format!( - "Nix is being downloaded to `{}` and should be in `nix`", - self.src.display(), - )], - )] - } + fn tracing_synopsis(&self) -> String { + "Move the downloaded Nix into `/nix`".to_string() + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new( + format!("Move the downloaded Nix into `/nix`"), + vec![format!( + "Nix is being downloaded to `{}` and should be in `nix`", + self.src.display(), + )], + )] } #[tracing::instrument(skip_all, fields( @@ -64,9 +64,9 @@ impl Action for MoveUnpackedNix { "Did not expect to find multiple nix paths, please report this" ); let found_nix_path = found_nix_paths.into_iter().next().unwrap(); - tracing::trace!("Renaming"); let src_store = found_nix_path.join("store"); let dest = Path::new(DEST); + tracing::trace!(src = %src_store.display(), dest = %dest.display(), "Renaming"); tokio::fs::rename(src_store.clone(), dest) .await .map_err(|e| { @@ -81,12 +81,8 @@ impl Action for MoveUnpackedNix { Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![/* Deliberately empty -- this is a noop */] - } + fn revert_description(&self) -> Vec { + vec![/* Deliberately empty -- this is a noop */] } #[tracing::instrument(skip_all, fields( @@ -94,22 +90,17 @@ impl Action for MoveUnpackedNix { dest = DEST, ))] async fn revert(&mut self) -> Result<(), Box> { - let Self { - src: _, - action_state, - } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unmove Nix (noop)"); - return Ok(()); - } - tracing::debug!("Unmove Nix (noop)"); - *action_state = ActionState::Uncompleted; + // Noop Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/base/setup_default_profile.rs b/src/action/base/setup_default_profile.rs index 2b3e8b6..c9e70e9 100644 --- a/src/action/base/setup_default_profile.rs +++ b/src/action/base/setup_default_profile.rs @@ -25,15 +25,12 @@ impl SetupDefaultProfile { #[async_trait::async_trait] #[typetag::serde(name = "setup_default_profile")] impl Action for SetupDefaultProfile { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - "Setup the default Nix profile".to_string(), - vec!["TODO".to_string()], - )] - } + fn tracing_synopsis(&self) -> String { + "Setup the default Nix profile".to_string() + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -42,13 +39,8 @@ impl Action for SetupDefaultProfile { async fn execute(&mut self) -> Result<(), Box> { let Self { channels, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Setting up default profile"); - return Ok(()); - } - tracing::debug!("Setting up default profile"); // Find an `nix` package let nix_pkg_glob = "/nix/store/*-nix-*"; @@ -147,46 +139,32 @@ impl Action for SetupDefaultProfile { .map_err(|e| SetupDefaultProfileError::Command(e).boxed())?; } - tracing::trace!("Set up default profile"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - "Unset the default Nix profile".to_string(), - vec!["TODO".to_string()], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + "Unset the default Nix profile".to_string(), + vec!["TODO".to_string()], + )] } #[tracing::instrument(skip_all, fields( channels = %self.channels.join(","), ))] async fn revert(&mut self) -> Result<(), Box> { - let Self { - channels: _, - action_state, - } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unset default profile"); - return Ok(()); - } - tracing::debug!("Unsetting default profile (mostly noop)"); - std::env::remove_var("NIX_SSL_CERT_FILE"); - tracing::trace!("Unset default profile (mostly noop)"); - *action_state = ActionState::Completed; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/common/configure_nix.rs b/src/action/common/configure_nix.rs index ac2c561..61c3a27 100644 --- a/src/action/common/configure_nix.rs +++ b/src/action/common/configure_nix.rs @@ -2,7 +2,7 @@ use crate::{ action::{ base::{ConfigureNixDaemonService, SetupDefaultProfile}, common::{ConfigureShellProfile, PlaceChannelConfiguration, PlaceNixConfiguration}, - Action, ActionDescription, ActionState, + Action, ActionDescription, ActionImplementation, ActionState, }, channel_value::ChannelValue, BoxableError, CommonSettings, @@ -65,7 +65,11 @@ impl ConfigureNix { #[async_trait::async_trait] #[typetag::serde(name = "configure_nix")] impl Action for ConfigureNix { - fn describe_execute(&self) -> Vec { + fn tracing_synopsis(&self) -> String { + "Configure Nix".to_string() + } + + fn execute_description(&self) -> Vec { let Self { setup_default_profile, configure_nix_daemon_service, @@ -75,18 +79,14 @@ impl Action for ConfigureNix { action_state: _, } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - let mut buf = setup_default_profile.describe_execute(); - buf.append(&mut configure_nix_daemon_service.describe_execute()); - buf.append(&mut place_nix_configuration.describe_execute()); - buf.append(&mut place_channel_configuration.describe_execute()); - if let Some(configure_shell_profile) = configure_shell_profile { - buf.append(&mut configure_shell_profile.describe_execute()); - } - buf + let mut buf = setup_default_profile.execute_description(); + buf.append(&mut configure_nix_daemon_service.execute_description()); + buf.append(&mut place_nix_configuration.execute_description()); + buf.append(&mut place_channel_configuration.execute_description()); + if let Some(configure_shell_profile) = configure_shell_profile { + buf.append(&mut configure_shell_profile.execute_description()); } + buf } #[tracing::instrument(skip_all)] @@ -97,37 +97,29 @@ impl Action for ConfigureNix { place_nix_configuration, place_channel_configuration, configure_shell_profile, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Configuring nix"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Configuring nix"); if let Some(configure_shell_profile) = configure_shell_profile { tokio::try_join!( - async move { setup_default_profile.execute().await }, - async move { place_nix_configuration.execute().await }, - async move { place_channel_configuration.execute().await }, - async move { configure_shell_profile.execute().await }, + async move { setup_default_profile.try_execute().await }, + async move { place_nix_configuration.try_execute().await }, + async move { place_channel_configuration.try_execute().await }, + async move { configure_shell_profile.try_execute().await }, )?; } else { tokio::try_join!( - async move { setup_default_profile.execute().await }, - async move { place_nix_configuration.execute().await }, - async move { place_channel_configuration.execute().await }, + async move { setup_default_profile.try_execute().await }, + async move { place_nix_configuration.try_execute().await }, + async move { place_channel_configuration.try_execute().await }, )?; }; - configure_nix_daemon_service.execute().await?; + configure_nix_daemon_service.try_execute().await?; - tracing::trace!("Configured nix"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { + fn revert_description(&self) -> Vec { let Self { setup_default_profile, configure_nix_daemon_service, @@ -137,20 +129,16 @@ impl Action for ConfigureNix { action_state: _, } = &self; - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - let mut buf = Vec::default(); - if let Some(configure_shell_profile) = configure_shell_profile { - buf.append(&mut configure_shell_profile.describe_revert()); - } - buf.append(&mut place_channel_configuration.describe_revert()); - buf.append(&mut place_nix_configuration.describe_revert()); - buf.append(&mut configure_nix_daemon_service.describe_revert()); - buf.append(&mut setup_default_profile.describe_revert()); - - buf + let mut buf = Vec::default(); + if let Some(configure_shell_profile) = configure_shell_profile { + buf.append(&mut configure_shell_profile.revert_description()); } + buf.append(&mut place_channel_configuration.revert_description()); + buf.append(&mut place_nix_configuration.revert_description()); + buf.append(&mut configure_nix_daemon_service.revert_description()); + buf.append(&mut setup_default_profile.revert_description()); + + buf } #[tracing::instrument(skip_all)] @@ -161,29 +149,25 @@ impl Action for ConfigureNix { place_nix_configuration, place_channel_configuration, configure_shell_profile, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unconfiguring nix"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Unconfiguring nix"); - configure_nix_daemon_service.revert().await?; + configure_nix_daemon_service.try_revert().await?; if let Some(configure_shell_profile) = configure_shell_profile { - configure_shell_profile.revert().await?; + configure_shell_profile.try_revert().await?; } - place_channel_configuration.revert().await?; - place_nix_configuration.revert().await?; - setup_default_profile.revert().await?; + place_channel_configuration.try_revert().await?; + place_nix_configuration.try_revert().await?; + setup_default_profile.try_revert().await?; - tracing::trace!("Unconfigured nix"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } diff --git a/src/action/common/configure_shell_profile.rs b/src/action/common/configure_shell_profile.rs index b5b434d..5f8928d 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::{CreateOrAppendFile, CreateOrAppendFileError}; -use crate::action::{Action, ActionDescription, ActionState}; +use crate::action::{Action, ActionDescription, ActionImplementation, ActionState}; use crate::BoxableError; use std::path::Path; @@ -57,29 +57,23 @@ impl ConfigureShellProfile { #[async_trait::async_trait] #[typetag::serde(name = "configure_shell_profile")] impl Action for ConfigureShellProfile { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - "Configure the shell profiles".to_string(), - vec!["Update shell profiles to import Nix".to_string()], - )] - } + fn tracing_synopsis(&self) -> String { + "Configure the shell profiles".to_string() + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new( + self.tracing_synopsis(), + vec!["Update shell profiles to import Nix".to_string()], + )] } #[tracing::instrument(skip_all)] async fn execute(&mut self) -> Result<(), Box> { let Self { create_or_append_files, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Configuring shell profile"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Configuring shell profile"); let mut set = JoinSet::new(); let mut errors = Vec::default(); @@ -87,7 +81,7 @@ impl Action for ConfigureShellProfile { for (idx, create_or_append_file) in create_or_append_files.iter().enumerate() { let mut create_or_append_file_clone = create_or_append_file.clone(); let _abort_handle = set.spawn(async move { - create_or_append_file_clone.execute().await?; + create_or_append_file_clone.try_execute().await?; Result::<_, Box>::Ok(( idx, create_or_append_file_clone, @@ -113,34 +107,22 @@ impl Action for ConfigureShellProfile { } } - tracing::trace!("Configured shell profile"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - "Unconfigure the shell profiles".to_string(), - vec!["Update shell profiles to no longer import Nix".to_string()], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + "Unconfigure the shell profiles".to_string(), + vec!["Update shell profiles to no longer import Nix".to_string()], + )] } #[tracing::instrument(skip_all)] async fn revert(&mut self) -> Result<(), Box> { let Self { create_or_append_files, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unconfiguring shell profile"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Unconfiguring shell profile"); let mut set = JoinSet::new(); let mut errors = Vec::default(); @@ -174,14 +156,16 @@ impl Action for ConfigureShellProfile { } } - tracing::trace!("Unconfigured shell profile"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/common/create_nix_tree.rs b/src/action/common/create_nix_tree.rs index 3ebe209..e39724a 100644 --- a/src/action/common/create_nix_tree.rs +++ b/src/action/common/create_nix_tree.rs @@ -1,5 +1,5 @@ use crate::action::base::{CreateDirectory, CreateDirectoryError}; -use crate::action::{Action, ActionDescription, ActionState}; +use crate::action::{Action, ActionDescription, ActionImplementation, ActionState}; const PATHS: &[&str] = &[ "/nix/var", @@ -42,102 +42,86 @@ impl CreateNixTree { #[async_trait::async_trait] #[typetag::serde(name = "create_nix_tree")] impl Action for CreateNixTree { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Create a directory tree in `/nix`"), - vec![ - format!( - "Nix and the Nix daemon require a Nix Store, which will be stored at `/nix`" - ), - format!( - "Creates: {}", - PATHS - .iter() - .map(|v| format!("`{v}`")) - .collect::>() - .join(", ") - ), - ], - )] - } + fn tracing_synopsis(&self) -> String { + "Create a directory tree in `/nix`".to_string() + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new( + self.tracing_synopsis(), + vec![ + format!( + "Nix and the Nix daemon require a Nix Store, which will be stored at `/nix`" + ), + format!( + "Creates: {}", + PATHS + .iter() + .map(|v| format!("`{v}`")) + .collect::>() + .join(", ") + ), + ], + )] } #[tracing::instrument(skip_all)] async fn execute(&mut self) -> Result<(), Box> { let Self { create_directories, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating nix tree"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Creating nix tree"); // Just do sequential since parallelizing this will have little benefit for create_directory in create_directories { - create_directory.execute().await? + create_directory.try_execute().await? } - tracing::trace!("Created nix tree"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!("Remove the directory tree in `/nix`"), - vec![ - format!( - "Nix and the Nix daemon require a Nix Store, which will be stored at `/nix`" - ), - format!( - "Removes: {}", - PATHS - .iter() - .rev() - .map(|v| format!("`{v}`")) - .collect::>() - .join(", ") - ), - ], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + format!("Remove the directory tree in `/nix`"), + vec![ + format!( + "Nix and the Nix daemon require a Nix Store, which will be stored at `/nix`" + ), + format!( + "Removes: {}", + PATHS + .iter() + .rev() + .map(|v| format!("`{v}`")) + .collect::>() + .join(", ") + ), + ], + )] } #[tracing::instrument(skip_all)] async fn revert(&mut self) -> Result<(), Box> { let Self { create_directories, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Deleting nix tree"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Deleting nix tree"); // Just do sequential since parallelizing this will have little benefit for create_directory in create_directories.iter_mut().rev() { create_directory.revert().await? } - tracing::trace!("Deleted nix tree"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/common/create_users_and_group.rs b/src/action/common/create_users_and_group.rs index a4fea57..6608cdb 100644 --- a/src/action/common/create_users_and_group.rs +++ b/src/action/common/create_users_and_group.rs @@ -2,7 +2,7 @@ use crate::CommonSettings; use crate::{ action::{ base::{CreateGroup, CreateGroupError, CreateUser, CreateUserError}, - Action, ActionDescription, ActionState, + Action, ActionDescription, ActionImplementation, ActionState, }, BoxableError, }; @@ -55,7 +55,11 @@ impl CreateUsersAndGroup { #[async_trait::async_trait] #[typetag::serde(name = "create_users_and_group")] impl Action for CreateUsersAndGroup { - fn describe_execute(&self) -> Vec { + fn tracing_synopsis(&self) -> String { + "Create build users and group".to_string() + } + + fn execute_description(&self) -> Vec { let Self { daemon_user_count, nix_build_group_name, @@ -66,20 +70,17 @@ impl Action for CreateUsersAndGroup { create_users: _, action_state: _, } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ - ActionDescription::new( - format!("Create build users and group"), - vec![ - format!("The nix daemon requires system users (and a group they share) which it can act as in order to build"), - format!("Create group `{nix_build_group_name}` with uid `{nix_build_group_id}`"), - format!("Create {daemon_user_count} users with prefix `{nix_build_user_prefix}` starting at uid `{nix_build_user_id_base}`"), - ], - ) - ] - } + + vec![ + ActionDescription::new( + self.tracing_synopsis(), + vec![ + format!("The nix daemon requires system users (and a group they share) which it can act as in order to build"), + format!("Create group `{nix_build_group_name}` with uid `{nix_build_group_id}`"), + format!("Create {daemon_user_count} users with prefix `{nix_build_user_prefix}` starting at uid `{nix_build_user_id_base}`"), + ], + ) + ] } #[tracing::instrument(skip_all, fields( @@ -98,23 +99,17 @@ impl Action for CreateUsersAndGroup { nix_build_group_id: _, nix_build_user_prefix: _, nix_build_user_id_base: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating users and groups"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Creating users and groups"); // Create group - create_group.execute().await?; + create_group.try_execute().await?; // Mac is apparently not threadsafe here... for create_user in create_users.iter_mut() { // let mut create_user_clone = create_user.clone(); // let _abort_handle = set.spawn(async move { - create_user.execute().await?; + create_user.try_execute().await?; // Result::<_, CreateUserError>::Ok((idx, create_user_clone)) // }); } @@ -135,12 +130,10 @@ impl Action for CreateUsersAndGroup { // } // } - tracing::trace!("Created users and groups"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { + fn revert_description(&self) -> Vec { let Self { daemon_user_count, nix_build_group_name, @@ -183,15 +176,8 @@ impl Action for CreateUsersAndGroup { nix_build_group_id: _, nix_build_user_prefix: _, nix_build_user_id_base: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Delete users and groups"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Delete users and groups"); - let mut set = JoinSet::new(); let mut errors = Vec::default(); @@ -223,14 +209,16 @@ impl Action for CreateUsersAndGroup { // Create group create_group.revert().await?; - tracing::trace!("Deleted users and groups"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/common/place_channel_configuration.rs b/src/action/common/place_channel_configuration.rs index d8eba90..28d786f 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, CreateFileError}; use crate::{ - action::{Action, ActionDescription, ActionState}, + action::{Action, ActionDescription, ActionImplementation, ActionState}, BoxableError, }; use reqwest::Url; @@ -45,23 +45,15 @@ impl PlaceChannelConfiguration { #[async_trait::async_trait] #[typetag::serde(name = "place_channel_configuration")] impl Action for PlaceChannelConfiguration { - fn describe_execute(&self) -> Vec { - let Self { - channels: _, - create_file, - action_state: _, - } = self; - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!( - "Place channel configuration at `{}`", - create_file.path.display() - ), - vec![], - )] - } + fn tracing_synopsis(&self) -> String { + format!( + "Place channel configuration at `{}`", + self.create_file.path.display() + ) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -71,39 +63,22 @@ impl Action for PlaceChannelConfiguration { let Self { create_file, channels: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Placing channel configuration"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Placing channel configuration"); - create_file.execute().await?; + create_file.try_execute().await?; - tracing::trace!("Placed channel configuration"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - let Self { - channels: _, - create_file, - action_state: _, - } = self; - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!( - "Remove channel configuration at `{}`", - create_file.path.display() - ), - vec![], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + format!( + "Remove channel configuration at `{}`", + self.create_file.path.display() + ), + vec![], + )] } #[tracing::instrument(skip_all, fields( @@ -113,25 +88,21 @@ impl Action for PlaceChannelConfiguration { let Self { create_file, channels: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Removing channel configuration"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Removing channel configuration"); create_file.revert().await?; - tracing::debug!("Removed channel configuration"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/common/place_nix_configuration.rs b/src/action/common/place_nix_configuration.rs index f72c754..bad6e6f 100644 --- a/src/action/common/place_nix_configuration.rs +++ b/src/action/common/place_nix_configuration.rs @@ -1,5 +1,5 @@ use crate::action::base::{CreateDirectory, CreateDirectoryError, CreateFile, CreateFileError}; -use crate::action::{Action, ActionDescription, ActionState}; +use crate::action::{Action, ActionDescription, ActionImplementation, ActionState}; const NIX_CONF_FOLDER: &str = "/etc/nix"; const NIX_CONF: &str = "/etc/nix/nix.conf"; @@ -44,18 +44,18 @@ impl PlaceNixConfiguration { #[async_trait::async_trait] #[typetag::serde(name = "place_nix_configuration")] impl Action for PlaceNixConfiguration { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Place the nix configuration in `{NIX_CONF}`"), - vec![ - "This file is read by the Nix daemon to set its configuration options at runtime." - .to_string(), - ], - )] - } + fn tracing_synopsis(&self) -> String { + format!("Place the nix configuration in `{NIX_CONF}`") + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new( + self.tracing_synopsis(), + vec![ + "This file is read by the Nix daemon to set its configuration options at runtime." + .to_string(), + ], + )] } #[tracing::instrument(skip_all)] @@ -63,35 +63,23 @@ impl Action for PlaceNixConfiguration { let Self { create_file, create_directory, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Placing Nix configuration"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Placing Nix configuration"); - create_directory.execute().await?; - create_file.execute().await?; + create_directory.try_execute().await?; + create_file.try_execute().await?; - tracing::trace!("Placed Nix configuration"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!("Remove the nix configuration in `{NIX_CONF}`"), - vec![ - "This file is read by the Nix daemon to set its configuration options at runtime." - .to_string(), - ], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + format!("Remove the nix configuration in `{NIX_CONF}`"), + vec![ + "This file is read by the Nix daemon to set its configuration options at runtime." + .to_string(), + ], + )] } #[tracing::instrument(skip_all)] @@ -99,26 +87,22 @@ impl Action for PlaceNixConfiguration { let Self { create_file, create_directory, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Remove nix configuration"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Remove nix configuration"); create_file.revert().await?; create_directory.revert().await?; - tracing::trace!("Removed nix configuration"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/common/provision_nix.rs b/src/action/common/provision_nix.rs index 10fbbae..0568163 100644 --- a/src/action/common/provision_nix.rs +++ b/src/action/common/provision_nix.rs @@ -3,7 +3,7 @@ use crate::action::base::{ }; use crate::CommonSettings; use crate::{ - action::{Action, ActionDescription, ActionState}, + action::{Action, ActionDescription, ActionImplementation, ActionState}, BoxableError, }; use std::path::PathBuf; @@ -51,7 +51,11 @@ impl ProvisionNix { #[async_trait::async_trait] #[typetag::serde(name = "provision_nix")] impl Action for ProvisionNix { - fn describe_execute(&self) -> Vec { + fn tracing_synopsis(&self) -> String { + "Provision Nix".to_string() + } + + fn execute_description(&self) -> Vec { let Self { fetch_nix, create_users_and_group, @@ -59,17 +63,14 @@ impl Action for ProvisionNix { move_unpacked_nix, action_state: _, } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - let mut buf = Vec::default(); - buf.append(&mut fetch_nix.describe_execute()); - buf.append(&mut create_users_and_group.describe_execute()); - buf.append(&mut create_nix_tree.describe_execute()); - buf.append(&mut move_unpacked_nix.describe_execute()); - buf - } + let mut buf = Vec::default(); + buf.append(&mut fetch_nix.execute_description()); + buf.append(&mut create_users_and_group.execute_description()); + buf.append(&mut create_nix_tree.execute_description()); + buf.append(&mut move_unpacked_nix.execute_description()); + + buf } #[tracing::instrument(skip_all)] @@ -79,34 +80,26 @@ impl Action for ProvisionNix { create_nix_tree, create_users_and_group, move_unpacked_nix, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Provisioning Nix"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Provisioning Nix"); // We fetch nix while doing the rest, then move it over. let mut fetch_nix_clone = fetch_nix.clone(); let fetch_nix_handle = tokio::task::spawn(async { - fetch_nix_clone.execute().await?; + fetch_nix_clone.try_execute().await?; Result::<_, Box>::Ok(fetch_nix_clone) }); - create_users_and_group.execute().await?; - create_nix_tree.execute().await?; + create_users_and_group.try_execute().await?; + create_nix_tree.try_execute().await?; *fetch_nix = fetch_nix_handle.await.map_err(|e| e.boxed())??; - move_unpacked_nix.execute().await?; + move_unpacked_nix.try_execute().await?; - tracing::trace!("Provisioned Nix"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { + fn revert_description(&self) -> Vec { let Self { fetch_nix, create_users_and_group, @@ -114,16 +107,13 @@ impl Action for ProvisionNix { move_unpacked_nix, action_state: _, } = &self; - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - let mut buf = Vec::default(); - buf.append(&mut move_unpacked_nix.describe_revert()); - buf.append(&mut create_nix_tree.describe_revert()); - buf.append(&mut create_users_and_group.describe_revert()); - buf.append(&mut fetch_nix.describe_revert()); - buf - } + + let mut buf = Vec::default(); + buf.append(&mut move_unpacked_nix.revert_description()); + buf.append(&mut create_nix_tree.revert_description()); + buf.append(&mut create_users_and_group.revert_description()); + buf.append(&mut fetch_nix.revert_description()); + buf } #[tracing::instrument(skip_all)] @@ -133,27 +123,21 @@ impl Action for ProvisionNix { create_nix_tree, create_users_and_group, move_unpacked_nix, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unprovisioning nix"); - return Ok(()); - } - *action_state = ActionState::Progress; - tracing::debug!("Unprovisioning nix"); // We fetch nix while doing the rest, then move it over. let mut fetch_nix_clone = fetch_nix.clone(); let fetch_nix_handle = tokio::task::spawn(async { - fetch_nix_clone.revert().await?; + fetch_nix_clone.try_revert().await?; Result::<_, Box>::Ok(fetch_nix_clone) }); - if let Err(err) = create_users_and_group.revert().await { + if let Err(err) = create_users_and_group.try_revert().await { fetch_nix_handle.abort(); return Err(err); } - if let Err(err) = create_nix_tree.revert().await { + if let Err(err) = create_nix_tree.try_revert().await { fetch_nix_handle.abort(); return Err(err); } @@ -161,14 +145,16 @@ impl Action for ProvisionNix { *fetch_nix = fetch_nix_handle.await.map_err(|e| e.boxed())??; move_unpacked_nix.revert().await?; - tracing::trace!("Unprovisioned Nix"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/darwin/bootstrap_volume.rs b/src/action/darwin/bootstrap_volume.rs index a7e8772..03c153f 100644 --- a/src/action/darwin/bootstrap_volume.rs +++ b/src/action/darwin/bootstrap_volume.rs @@ -30,27 +30,22 @@ impl BootstrapVolume { #[async_trait::async_trait] #[typetag::serde(name = "bootstrap_volume")] impl Action for BootstrapVolume { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Bootstrap and kickstart `{}`", self.path.display()), - vec![], - )] - } + fn tracing_synopsis(&self) -> String { + format!("Bootstrap and kickstart `{}`", self.path.display()) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( path = %self.path.display(), ))] async fn execute(&mut self) -> Result<(), Box> { - let Self { path, action_state } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Bootstrapping volume"); - return Ok(()); - } - tracing::debug!("Bootstrapping volume"); + let Self { + path, + action_state: _, + } = self; execute_command( Command::new("launchctl") @@ -68,32 +63,24 @@ impl Action for BootstrapVolume { .await .map_err(|e| BootstrapVolumeError::Command(e).boxed())?; - tracing::trace!("Bootstrapped volume"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!("Stop `{}`", self.path.display()), - vec![], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + format!("Stop `{}`", self.path.display()), + vec![], + )] } #[tracing::instrument(skip_all, fields( path = %self.path.display(), ))] async fn revert(&mut self) -> Result<(), Box> { - let Self { path, action_state } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Stop volume"); - return Ok(()); - } - tracing::debug!("Stop volume"); + let Self { + path, + action_state: _, + } = self; execute_command( Command::new("launchctl") @@ -104,14 +91,16 @@ impl Action for BootstrapVolume { .await .map_err(|e| BootstrapVolumeError::Command(e).boxed())?; - tracing::trace!("Stopped volume"); - *action_state = ActionState::Completed; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/darwin/create_apfs_volume.rs b/src/action/darwin/create_apfs_volume.rs index f92f174..c662382 100644 --- a/src/action/darwin/create_apfs_volume.rs +++ b/src/action/darwin/create_apfs_volume.rs @@ -7,7 +7,7 @@ use crate::{ EnableOwnershipError, EncryptVolume, EncryptVolumeError, UnmountVolume, UnmountVolumeError, }, - Action, ActionDescription, ActionState, + Action, ActionDescription, ActionImplementation, ActionState, }, BoxableError, }; @@ -140,23 +140,19 @@ impl CreateApfsVolume { #[async_trait::async_trait] #[typetag::serde(name = "create_apfs_volume")] impl Action for CreateApfsVolume { - fn describe_execute(&self) -> Vec { + fn tracing_synopsis(&self) -> String { + format!( + "Create an APFS volume `{}` on `{}`", + self.name, + self.disk.display() + ) + } + + fn execute_description(&self) -> Vec { let Self { - disk, - name, - action_state: _, - .. + disk: _, name: _, .. } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Create an APFS volume `{name}` on `{}`", disk.display()), - vec![format!( - "Create a writable, persistent systemd system extension.", - )], - )] - } + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields(destination,))] @@ -175,25 +171,20 @@ impl Action for CreateApfsVolume { setup_volume_daemon, bootstrap_volume, enable_ownership, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating APFS volume"); - return Ok(()); - } - tracing::debug!("Creating APFS volume"); - create_or_append_synthetic_conf.execute().await?; - create_synthetic_objects.execute().await?; - unmount_volume.execute().await.ok(); // We actually expect this may fail. - create_volume.execute().await?; - create_or_append_fstab.execute().await?; + 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_or_append_fstab.try_execute().await?; if let Some(encrypt_volume) = encrypt_volume { - encrypt_volume.execute().await?; + encrypt_volume.try_execute().await?; } - setup_volume_daemon.execute().await?; + setup_volume_daemon.try_execute().await?; - bootstrap_volume.execute().await?; + bootstrap_volume.try_execute().await?; let mut retry_tokens: usize = 50; loop { @@ -213,30 +204,19 @@ impl Action for CreateApfsVolume { tokio::time::sleep(Duration::from_millis(100)).await; } - enable_ownership.execute().await?; + enable_ownership.try_execute().await?; - tracing::trace!("Created APFS volume"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - let Self { - disk, - name, - action_state, - .. - } = &self; - if *action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!("Remove the APFS volume `{name}` on `{}`", disk.display()), - vec![format!( - "Create a writable, persistent systemd system extension.", - )], - )] - } + fn revert_description(&self) -> Vec { + let Self { disk, name, .. } = &self; + vec![ActionDescription::new( + format!("Remove the APFS volume `{name}` on `{}`", disk.display()), + vec![format!( + "Create a writable, persistent systemd system extension.", + )], + )] } #[tracing::instrument(skip_all, fields(disk, name))] @@ -255,37 +235,34 @@ impl Action for CreateApfsVolume { setup_volume_daemon, bootstrap_volume, enable_ownership, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Removing APFS volume"); - return Ok(()); - } - tracing::debug!("Removing APFS volume"); - enable_ownership.revert().await?; - bootstrap_volume.revert().await?; - setup_volume_daemon.revert().await?; + 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.revert().await?; + encrypt_volume.try_revert().await?; } - create_or_append_fstab.revert().await?; + create_or_append_fstab.try_revert().await?; - unmount_volume.revert().await?; - create_volume.revert().await?; + unmount_volume.try_revert().await?; + create_volume.try_revert().await?; // Purposefully not reversed - create_or_append_synthetic_conf.revert().await?; - create_synthetic_objects.revert().await?; + create_or_append_synthetic_conf.try_revert().await?; + create_synthetic_objects.try_revert().await?; - tracing::trace!("Removed APFS volume"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/darwin/create_synthetic_objects.rs b/src/action/darwin/create_synthetic_objects.rs index 005ca7f..82c5629 100644 --- a/src/action/darwin/create_synthetic_objects.rs +++ b/src/action/darwin/create_synthetic_objects.rs @@ -21,26 +21,19 @@ impl CreateSyntheticObjects { #[async_trait::async_trait] #[typetag::serde(name = "create_synthetic_objects")] impl Action for CreateSyntheticObjects { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - "Create objects defined in `/etc/synthetic.conf`".to_string(), - vec!["Populates the `/nix` path".to_string()], - )] - } + fn tracing_synopsis(&self) -> String { + "Create objects defined in `/etc/synthetic.conf`".to_string() + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new( + self.tracing_synopsis(), + vec!["Populates the `/nix` path".to_string()], + )] } #[tracing::instrument(skip_all, fields())] async fn execute(&mut self) -> Result<(), Box> { - let Self { action_state } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating synthetic objects"); - return Ok(()); - } - tracing::debug!("Creating synthetic objects"); - // Yup we literally call both and ignore the error! Reasoning: https://github.com/NixOS/nix/blob/95331cb9c99151cbd790ceb6ddaf49fc1c0da4b3/scripts/create-darwin-volume.sh#L261 execute_command( Command::new("/System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util") @@ -57,31 +50,18 @@ impl Action for CreateSyntheticObjects { .await .ok(); // Deliberate - tracing::trace!("Created synthetic objects"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - "Refresh the objects defined in `/etc/synthetic.conf`".to_string(), - vec!["Will remove the `/nix` path".to_string()], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + "Refresh the objects defined in `/etc/synthetic.conf`".to_string(), + vec!["Will remove the `/nix` path".to_string()], + )] } #[tracing::instrument(skip_all, fields())] async fn revert(&mut self) -> Result<(), Box> { - let Self { action_state } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Refreshing synthetic objects"); - return Ok(()); - } - tracing::debug!("Refreshing synthetic objects"); - // Yup we literally call both and ignore the error! Reasoning: https://github.com/NixOS/nix/blob/95331cb9c99151cbd790ceb6ddaf49fc1c0da4b3/scripts/create-darwin-volume.sh#L261 execute_command( Command::new("/System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util") @@ -98,14 +78,16 @@ impl Action for CreateSyntheticObjects { .await .ok(); // Deliberate - tracing::trace!("Refreshed synthetic objects"); - *action_state = ActionState::Completed; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/darwin/create_volume.rs b/src/action/darwin/create_volume.rs index c1ca2be..51747ee 100644 --- a/src/action/darwin/create_volume.rs +++ b/src/action/darwin/create_volume.rs @@ -36,19 +36,16 @@ impl CreateVolume { #[async_trait::async_trait] #[typetag::serde(name = "create_volume")] impl Action for CreateVolume { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!( - "Create a volume on `{}` named `{}`", - self.disk.display(), - self.name - ), - vec![], - )] - } + fn tracing_synopsis(&self) -> String { + format!( + "Create a volume on `{}` named `{}`", + self.disk.display(), + self.name + ) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -61,13 +58,8 @@ impl Action for CreateVolume { disk, name, case_sensitive, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating volume"); - return Ok(()); - } - tracing::debug!("Creating volume"); execute_command( Command::new("/usr/sbin/diskutil") @@ -88,24 +80,18 @@ impl Action for CreateVolume { .await .map_err(|e| CreateVolumeError::Command(e).boxed())?; - tracing::trace!("Created volume"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!( - "Remove the volume on `{}` named `{}`", - self.disk.display(), - self.name - ), - vec![], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + format!( + "Remove the volume on `{}` named `{}`", + self.disk.display(), + self.name + ), + vec![], + )] } #[tracing::instrument(skip_all, fields( @@ -118,13 +104,8 @@ impl Action for CreateVolume { disk: _, name, case_sensitive: _, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Deleting volume"); - return Ok(()); - } - tracing::debug!("Deleting volume"); execute_command( Command::new("/usr/sbin/diskutil") @@ -134,14 +115,16 @@ impl Action for CreateVolume { .await .map_err(|e| CreateVolumeError::Command(e).boxed())?; - tracing::trace!("Deleted volume"); - *action_state = ActionState::Completed; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/darwin/enable_ownership.rs b/src/action/darwin/enable_ownership.rs index 349831e..ec7cf29 100644 --- a/src/action/darwin/enable_ownership.rs +++ b/src/action/darwin/enable_ownership.rs @@ -32,27 +32,22 @@ impl EnableOwnership { #[async_trait::async_trait] #[typetag::serde(name = "enable_ownership")] impl Action for EnableOwnership { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Enable ownership on {}", self.path.display()), - vec![], - )] - } + fn tracing_synopsis(&self) -> String { + format!("Enable ownership on {}", self.path.display()) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( path = %self.path.display(), ))] async fn execute(&mut self) -> Result<(), Box> { - let Self { path, action_state } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Enabling ownership"); - return Ok(()); - } - tracing::debug!("Enabling ownership"); + let Self { + path, + action_state: _, + } = self; let should_enable_ownership = { let buf = execute_command( @@ -79,41 +74,28 @@ impl Action for EnableOwnership { .map_err(|e| EnableOwnershipError::Command(e).boxed())?; } - tracing::trace!("Enabled ownership"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![] - } + fn revert_description(&self) -> Vec { + vec![] } #[tracing::instrument(skip_all, fields( path = %self.path.display(), ))] async fn revert(&mut self) -> Result<(), Box> { - let Self { - path: _, - action_state, - } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unenabling ownership (noop)"); - return Ok(()); - } - tracing::debug!("Unenabling ownership (noop)"); - - tracing::trace!("Unenabled ownership (noop)"); - *action_state = ActionState::Completed; + // noop Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/darwin/encrypt_volume.rs b/src/action/darwin/encrypt_volume.rs index f411b65..fc43813 100644 --- a/src/action/darwin/encrypt_volume.rs +++ b/src/action/darwin/encrypt_volume.rs @@ -31,15 +31,16 @@ impl EncryptVolume { #[async_trait::async_trait] #[typetag::serde(name = "encrypt_volume")] impl Action for EncryptVolume { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Encrypt volume `{}`", self.disk.display()), - vec![], - )] - } + fn tracing_synopsis(&self) -> String { + format!( + "Encrypt volume `{}` on disk `{}`", + self.name, + self.disk.display() + ) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -49,13 +50,8 @@ impl Action for EncryptVolume { let Self { disk, name, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Encrypting volume"); - return Ok(()); - } - tracing::debug!("Encrypting volume"); // Generate a random password. let password: String = { @@ -127,17 +123,17 @@ impl Action for EncryptVolume { ) .await?; - tracing::trace!("Encrypted volume"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + format!( + "Remove encryption keys for volume `{}`", + self.disk.display() + ), + vec![], + )] } #[tracing::instrument(skip_all, fields( @@ -147,13 +143,8 @@ impl Action for EncryptVolume { let Self { disk, name, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unencrypted volume"); - return Ok(()); - } - tracing::debug!("Unencrypted volume"); let disk_str = disk.to_str().expect("Could not turn disk into string"); /* Should not reasonably ever fail */ @@ -178,14 +169,16 @@ impl Action for EncryptVolume { ) .await?; - tracing::trace!("Unencrypted volume"); - *action_state = ActionState::Completed; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/darwin/kickstart_launchctl_service.rs b/src/action/darwin/kickstart_launchctl_service.rs index 9305dea..3041370 100644 --- a/src/action/darwin/kickstart_launchctl_service.rs +++ b/src/action/darwin/kickstart_launchctl_service.rs @@ -26,30 +26,23 @@ impl KickstartLaunchctlService { #[async_trait::async_trait] #[typetag::serde(name = "kickstart_launchctl_service")] impl Action for KickstartLaunchctlService { - fn describe_execute(&self) -> Vec { - let Self { unit, action_state } = self; - if *action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Kickstart the launchctl unit `{unit}`"), - vec![ - "The `nix` command line tool communicates with a running Nix daemon managed by your init system".to_string() - ] - )] - } + fn tracing_synopsis(&self) -> String { + let Self { unit, .. } = self; + format!("Kickstart the launchctl unit `{unit}`") + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( unit = %self.unit, ))] async fn execute(&mut self) -> Result<(), Box> { - let Self { unit, action_state } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Kickstarting launchctl unit"); - return Ok(()); - } - tracing::debug!("Kickstarting launchctl unit"); + let Self { + unit, + action_state: _, + } = self; execute_command( Command::new("launchctl") @@ -61,46 +54,28 @@ impl Action for KickstartLaunchctlService { .await .map_err(|e| KickstartLaunchctlServiceError::Command(e).boxed())?; - tracing::trace!("Kickstarted launchctl unit"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - "Kick".to_string(), - vec![ - "The `nix` command line tool communicates with a running Nix daemon managed by your init system".to_string() - ] - )] - } + fn revert_description(&self) -> Vec { + vec![] } #[tracing::instrument(skip_all, fields( unit = %self.unit, ))] async fn revert(&mut self) -> Result<(), Box> { - let Self { - unit: _, - action_state, - } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unkickstart launchctl unit (noop)"); - return Ok(()); - } - tracing::debug!("Unkickstart launchctl unit (noop)"); - - tracing::trace!("Unkickstart launchctl unit (noop)"); - *action_state = ActionState::Completed; + // noop Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/darwin/unmount_volume.rs b/src/action/darwin/unmount_volume.rs index 19067cc..8969289 100644 --- a/src/action/darwin/unmount_volume.rs +++ b/src/action/darwin/unmount_volume.rs @@ -34,17 +34,12 @@ impl UnmountVolume { #[async_trait::async_trait] #[typetag::serde(name = "unmount_volume")] impl Action for UnmountVolume { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - "Start the systemd Nix service and socket".to_string(), - vec![ - "The `nix` command line tool communicates with a running Nix daemon managed by your init system".to_string() - ] - )] - } + fn tracing_synopsis(&self) -> String { + format!("Unmount the `{}` volume", self.name) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -55,13 +50,8 @@ impl Action for UnmountVolume { let Self { disk: _, name, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Unmounting volume"); - return Ok(()); - } - tracing::debug!("Unmounting volume"); execute_command( Command::new("/usr/sbin/diskutil") @@ -72,22 +62,11 @@ impl Action for UnmountVolume { .await .map_err(|e| UnmountVolumeError::Command(e).boxed())?; - tracing::trace!("Unmounted volume"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - "Stop the systemd Nix service and socket".to_string(), - vec![ - "The `nix` command line tool communicates with a running Nix daemon managed by your init system".to_string() - ] - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -98,16 +77,11 @@ impl Action for UnmountVolume { let Self { disk: _, name, - action_state, + action_state: _, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Unmounting Nix Store volume"); - return Ok(()); - } - tracing::debug!("Unmounting Nix Store volume"); execute_command( - Command::new(" /usr/sbin/diskutil") + Command::new("/usr/sbin/diskutil") .args(["unmount", "force"]) .arg(name) .stdin(std::process::Stdio::null()), @@ -115,14 +89,16 @@ impl Action for UnmountVolume { .await .map_err(|e| UnmountVolumeError::Command(e).boxed())?; - tracing::trace!("Unmounted Nix Store volume"); - *action_state = ActionState::Completed; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/linux/create_systemd_sysext.rs b/src/action/linux/create_systemd_sysext.rs index b4029a7..4d9a634 100644 --- a/src/action/linux/create_systemd_sysext.rs +++ b/src/action/linux/create_systemd_sysext.rs @@ -1,6 +1,6 @@ use crate::action::base::{CreateDirectory, CreateDirectoryError, CreateFile, CreateFileError}; use crate::{ - action::{Action, ActionDescription, ActionState}, + action::{Action, ActionDescription, ActionImplementation, ActionState}, BoxableError, }; use std::path::{Path, PathBuf}; @@ -93,101 +93,79 @@ impl CreateSystemdSysext { #[async_trait::async_trait] #[typetag::serde(name = "create_systemd_sysext")] impl Action for CreateSystemdSysext { - fn describe_execute(&self) -> Vec { - let Self { - action_state: _, - destination, - create_bind_mount_unit: _, - create_directories: _, - create_extension_release: _, - } = &self; - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Create a systemd sysext at `{}`", destination.display()), - vec![format!( - "Create a writable, persistent systemd system extension.", - )], - )] - } + fn tracing_synopsis(&self) -> String { + format!( + "Create a systemd sysext at `{}`", + self.destination.display() + ) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new( + self.tracing_synopsis(), + vec![format!( + "Create a writable, persistent systemd system extension.", + )], + )] } #[tracing::instrument(skip_all, fields(destination,))] async fn execute(&mut self) -> Result<(), Box> { let Self { destination: _, - action_state, + action_state: _, create_directories, create_extension_release, create_bind_mount_unit, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Creating sysext"); - return Ok(()); - } - tracing::debug!("Creating sysext"); for create_directory in create_directories { - create_directory.execute().await?; + create_directory.try_execute().await?; } - create_extension_release.execute().await?; - create_bind_mount_unit.execute().await?; + create_extension_release.try_execute().await?; + create_bind_mount_unit.try_execute().await?; - tracing::trace!("Created sysext"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - let Self { - destination, - action_state: _, - create_directories: _, - create_extension_release: _, - create_bind_mount_unit: _, - } = &self; - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - format!("Remove the sysext located at `{}`", destination.display()), - vec![], - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + format!( + "Remove the sysext located at `{}`", + self.destination.display() + ), + vec![], + )] } #[tracing::instrument(skip_all, fields(destination,))] async fn revert(&mut self) -> Result<(), Box> { let Self { destination: _, - action_state, + action_state: _, create_directories, create_extension_release, create_bind_mount_unit, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Removing sysext"); - return Ok(()); - } - tracing::debug!("Removing sysext"); - create_bind_mount_unit.revert().await?; + create_bind_mount_unit.try_revert().await?; - create_extension_release.revert().await?; + create_extension_release.try_revert().await?; for create_directory in create_directories.iter_mut().rev() { - create_directory.revert().await?; + create_directory.try_revert().await?; } - tracing::trace!("Removed sysext"); - *action_state = ActionState::Uncompleted; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/linux/mod.rs b/src/action/linux/mod.rs index 58f683f..0211b9d 100644 --- a/src/action/linux/mod.rs +++ b/src/action/linux/mod.rs @@ -1,7 +1,5 @@ mod create_systemd_sysext; mod start_systemd_unit; -mod systemd_sysext_merge; pub use create_systemd_sysext::{CreateSystemdSysext, CreateSystemdSysextError}; pub use start_systemd_unit::{StartSystemdUnit, StartSystemdUnitError}; -pub use systemd_sysext_merge::{SystemdSysextMerge, SystemdSysextMergeError}; diff --git a/src/action/linux/start_systemd_unit.rs b/src/action/linux/start_systemd_unit.rs index 8886754..5e9afea 100644 --- a/src/action/linux/start_systemd_unit.rs +++ b/src/action/linux/start_systemd_unit.rs @@ -26,29 +26,19 @@ impl StartSystemdUnit { #[async_trait::async_trait] #[typetag::serde(name = "start_systemd_unit")] impl Action for StartSystemdUnit { - fn describe_execute(&self) -> Vec { - if self.action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - "Start the systemd Nix service and socket".to_string(), - vec![ - "The `nix` command line tool communicates with a running Nix daemon managed by your init system".to_string() - ] - )] - } + fn tracing_synopsis(&self) -> String { + format!("Enable (and start) the systemd unit {}", self.unit) + } + + fn execute_description(&self) -> Vec { + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( unit = %self.unit, ))] async fn execute(&mut self) -> Result<(), Box> { - let Self { unit, action_state } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Starting systemd unit"); - return Ok(()); - } - tracing::debug!("Starting systemd unit"); + let Self { unit, .. } = self; // TODO(@Hoverbear): Handle proxy vars execute_command( @@ -61,34 +51,21 @@ impl Action for StartSystemdUnit { .await .map_err(|e| StartSystemdUnitError::Command(e).boxed())?; - tracing::trace!("Started systemd unit"); - *action_state = ActionState::Completed; Ok(()) } - fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( - "Stop the systemd Nix service and socket".to_string(), - vec![ - "The `nix` command line tool communicates with a running Nix daemon managed by your init system".to_string() - ] - )] - } + fn revert_description(&self) -> Vec { + vec![ActionDescription::new( + format!("Disable (and stop) the systemd unit {}", self.unit), + vec![], + )] } #[tracing::instrument(skip_all, fields( unit = %self.unit, ))] async fn revert(&mut self) -> Result<(), Box> { - let Self { unit, action_state } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Stopping systemd unit"); - return Ok(()); - } - tracing::debug!("Stopping systemd unit"); + let Self { unit, .. } = self; // TODO(@Hoverbear): Handle proxy vars execute_command( @@ -100,14 +77,26 @@ impl Action for StartSystemdUnit { .await .map_err(|e| StartSystemdUnitError::Command(e).boxed())?; - tracing::trace!("Stopped systemd unit"); - *action_state = ActionState::Completed; + // We do both to avoid an error doing `disable --now` if the user did stop it already somehow. + execute_command( + Command::new("systemctl") + .arg("stop") + .arg(format!("{unit}")) + .stdin(std::process::Stdio::null()), + ) + .await + .map_err(|e| StartSystemdUnitError::Command(e).boxed())?; + Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/linux/systemd_sysext_merge.rs b/src/action/linux/systemd_sysext_merge.rs index 560c89a..7652652 100644 --- a/src/action/linux/systemd_sysext_merge.rs +++ b/src/action/linux/systemd_sysext_merge.rs @@ -25,19 +25,12 @@ impl SystemdSysextMerge { #[async_trait::async_trait] #[typetag::serde(name = "systemd_sysext_merge")] impl Action for SystemdSysextMerge { + fn tracing_synopsis(&self) -> String { + format!("Run `systemd-sysext merge `{}`", device.display()) + } + fn describe_execute(&self) -> Vec { - let Self { - action_state, - device, - } = self; - if *action_state == ActionState::Completed { - vec![] - } else { - vec![ActionDescription::new( - format!("Run `systemd-sysext merge `{}`", device.display()), - vec![], - )] - } + vec![ActionDescription::new(self.tracing_synopsis(), vec![])] } #[tracing::instrument(skip_all, fields( @@ -48,11 +41,6 @@ impl Action for SystemdSysextMerge { device, action_state, } = self; - if *action_state == ActionState::Completed { - tracing::trace!("Already completed: Merging systemd-sysext"); - return Ok(()); - } - tracing::debug!("Merging systemd-sysext"); execute_command( Command::new("systemd-sysext") @@ -63,22 +51,16 @@ impl Action for SystemdSysextMerge { .await .map_err(|e| SystemdSysextMergeError::Command(e).boxed())?; - tracing::trace!("Merged systemd-sysext"); - *action_state = ActionState::Completed; Ok(()) } fn describe_revert(&self) -> Vec { - if self.action_state == ActionState::Uncompleted { - vec![] - } else { - vec![ActionDescription::new( + vec![ActionDescription::new( "Stop the systemd Nix service and socket".to_string(), vec![ "The `nix` command line tool communicates with a running Nix daemon managed by your init system".to_string() ] )] - } } #[tracing::instrument(skip_all, fields( @@ -89,11 +71,6 @@ impl Action for SystemdSysextMerge { device, action_state, } = self; - if *action_state == ActionState::Uncompleted { - tracing::trace!("Already reverted: Stopping systemd unit"); - return Ok(()); - } - tracing::debug!("Unmrging systemd-sysext"); // TODO(@Hoverbear): Handle proxy vars execute_command( @@ -105,14 +82,16 @@ impl Action for SystemdSysextMerge { .await .map_err(|e| SystemdSysextMergeError::Command(e).boxed())?; - tracing::trace!("Unmerged systemd-sysext"); - *action_state = ActionState::Completed; Ok(()) } fn action_state(&self) -> ActionState { self.action_state } + + fn set_action_state(&mut self, action_state: ActionState) { + self.action_state = action_state; + } } #[derive(Debug, thiserror::Error)] diff --git a/src/action/mod.rs b/src/action/mod.rs index 62b3751..8276f32 100644 --- a/src/action/mod.rs +++ b/src/action/mod.rs @@ -5,18 +5,76 @@ pub mod linux; use serde::{Deserialize, Serialize}; +/// An action which can be reverted or completed, with an action state +/// +/// This trait interacts with [`ActionImplementation`] which does the [`ActionState`] manipulation and provides some tracing facilities. +/// +/// Instead of calling [`execute`][Action::execute] or [`revert`][Action::revert], you should prefer [`try_execute`][ActionImplementation::try_execute] and [`try_revert`][ActionImplementation::try_revert] #[async_trait::async_trait] #[typetag::serde(tag = "action")] pub trait Action: Send + Sync + std::fmt::Debug + dyn_clone::DynClone { - fn describe_execute(&self) -> Vec; - fn describe_revert(&self) -> Vec; - - // They should also have an `async fn plan(args...) -> Result, Box>;` + fn tracing_synopsis(&self) -> String; + fn execute_description(&self) -> Vec; + fn revert_description(&self) -> Vec; + /// Instead of calling [`execute`][Action::execute], you should prefer [`try_execute`][ActionImplementation::try_execute], so [`ActionState`] is handled correctly and tracing is done. async fn execute(&mut self) -> Result<(), Box>; + /// Instead of calling [`revert`][Action::revert], you should prefer [`try_revert`][ActionImplementation::try_revert], so [`ActionState`] is handled correctly and tracing is done. async fn revert(&mut self) -> Result<(), Box>; fn action_state(&self) -> ActionState; + fn set_action_state(&mut self, new_state: ActionState); + + // They should also have an `async fn plan(args...) -> Result, Box>;` } +/// The main wrapper around [`Action`], handling [`ActionState`] and tracing. +#[async_trait::async_trait] +pub trait ActionImplementation: Action { + fn describe_execute(&self) -> Vec { + if self.action_state() == ActionState::Completed { + return vec![]; + } + return self.execute_description(); + } + fn describe_revert(&self) -> Vec { + if self.action_state() == ActionState::Uncompleted { + return vec![]; + } + return self.revert_description(); + } + + /// You should prefer this ([`try_execute`][ActionImplementation::try_execute]) over [`execute`][Action::execute] as it handles [`ActionState`] and does tracing. + async fn try_execute(&mut self) -> Result<(), Box> { + if self.action_state() == ActionState::Completed { + tracing::trace!("Completed: (Already done) {}", self.tracing_synopsis()); + return Ok(()); + } + self.set_action_state(ActionState::Progress); + tracing::debug!("Executing: {}", self.tracing_synopsis()); + self.execute().await?; + self.set_action_state(ActionState::Completed); + tracing::debug!("Completed: {}", self.tracing_synopsis()); + Ok(()) + } + + /// You should prefer this ([`try_revert`][ActionImplementation::try_revert]) over [`revert`][Action::revert] as it handles [`ActionState`] and does tracing. + async fn try_revert(&mut self) -> Result<(), Box> { + if self.action_state() == ActionState::Uncompleted { + tracing::trace!("Reverted: (Already done) {}", self.tracing_synopsis()); + return Ok(()); + } + self.set_action_state(ActionState::Progress); + tracing::debug!("Reverting: {}", self.tracing_synopsis()); + self.revert().await?; + tracing::debug!("Reverted: {}", self.tracing_synopsis()); + self.set_action_state(ActionState::Uncompleted); + Ok(()) + } +} + +impl ActionImplementation for dyn Action {} + +impl ActionImplementation for A where A: Action {} + dyn_clone::clone_trait_object!(Action); #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Copy)] diff --git a/src/cli/subcommand/install.rs b/src/cli/subcommand/install.rs index 399a6dd..3e1fa33 100644 --- a/src/cli/subcommand/install.rs +++ b/src/cli/subcommand/install.rs @@ -120,7 +120,13 @@ impl CommandExecute for Install { let error = eyre!(err).wrap_err("Install failure"); if !no_confirm { tracing::error!("{:?}", error); - if !interaction::confirm(install_plan.describe_revert(explain)).await? { + if !interaction::confirm( + install_plan + .describe_revert(explain) + .map_err(|e| eyre!(e))?, + ) + .await? + { interaction::clean_exit_with_message("Okay, didn't do anything! Bye!").await; } let rx2 = tx.subscribe(); diff --git a/src/cli/subcommand/uninstall.rs b/src/cli/subcommand/uninstall.rs index 7088169..02fc5f2 100644 --- a/src/cli/subcommand/uninstall.rs +++ b/src/cli/subcommand/uninstall.rs @@ -53,7 +53,7 @@ impl CommandExecute for Uninstall { let mut plan: InstallPlan = serde_json::from_str(&install_receipt_string)?; if !no_confirm { - if !interaction::confirm(plan.describe_revert(explain)).await? { + if !interaction::confirm(plan.describe_revert(explain).map_err(|e| eyre!(e))?).await? { interaction::clean_exit_with_message("Okay, didn't do anything! Bye!").await; } } diff --git a/src/lib.rs b/src/lib.rs index 22f4981..5f5db9b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,8 +25,8 @@ use tokio::process::Command; async fn execute_command(command: &mut Command) -> Result { // TODO(@hoverbear): When tokio releases past 1.21.2, add a process group https://github.com/DeterminateSystems/harmonic/issues/41#issuecomment-1309513073 - tracing::trace!("Executing"); let command_str = format!("{:?}", command.as_std()); + tracing::trace!("Executing `{command_str}`"); let output = command.output().await?; match output.status.success() { true => Ok(output), diff --git a/src/plan.rs b/src/plan.rs index 264caf2..299fda1 100644 --- a/src/plan.rs +++ b/src/plan.rs @@ -2,10 +2,9 @@ use std::path::PathBuf; use crossterm::style::Stylize; use tokio::sync::broadcast::Receiver; -use tokio_util::sync::CancellationToken; use crate::{ - action::{Action, ActionDescription}, + action::{Action, ActionDescription, ActionImplementation}, planner::Planner, HarmonicError, }; @@ -103,7 +102,7 @@ impl InstallPlan { } } - if let Err(err) = action.execute().await { + if let Err(err) = action.try_execute().await { if let Err(err) = write_receipt(self.clone()).await { tracing::error!("Error saving receipt: {:?}", err); } @@ -116,23 +115,37 @@ impl InstallPlan { } #[tracing::instrument(skip_all)] - pub fn describe_revert(&self, explain: bool) -> String { + pub fn describe_revert( + &self, + explain: bool, + ) -> Result> { let Self { planner, actions } = self; - format!( + let buf = format!( "\ - This Nix uninstall is for:\n\ - Operating System: {os_type}\n\ - Init system: {init_type}\n\ - Nix channels: {nix_channels}\n\ + Nix uninstall plan\n\ \n\ - Created by planner: {planner:?} + Planner: {planner}\n\ \n\ - The following actions will be taken:\n\ - {actions} + Planner settings:\n\ + \n\ + {plan_settings}\n\ + \n\ + The following actions will be taken{maybe_explain}:\n\ + \n\ + {actions}\n\ ", - os_type = "Linux", - init_type = "systemd", - nix_channels = "todo", + maybe_explain = if !explain { + " (`--explain` for more context)" + } else { + "" + }, + planner = planner.typetag_name(), + plan_settings = planner + .settings()? + .into_iter() + .map(|(k, v)| format!("* {k}: {v}", k = k.bold().white())) + .collect::>() + .join("\n"), actions = actions .iter() .rev() @@ -145,17 +158,18 @@ impl InstallPlan { } = desc; let mut buf = String::default(); - buf.push_str(&format!("* {description}\n")); + buf.push_str(&format!("* {description}")); if explain { for line in explanation { - buf.push_str(&format!(" {line}\n")); + buf.push_str(&format!("\n {line}")); } } buf }) .collect::>() .join("\n"), - ) + ); + Ok(buf) } #[tracing::instrument(skip_all)] @@ -184,7 +198,7 @@ impl InstallPlan { } } - if let Err(err) = action.revert().await { + if let Err(err) = action.try_revert().await { if let Err(err) = write_receipt(self.clone()).await { tracing::error!("Error saving receipt: {:?}", err); }