General improvements, deprecate `activate` profile option in favor of executing $PROFILE/activate (Wrap It Yourself) to ensure successful rollback activations

4 files changed, 97 insertions, 117 deletions

diff --git a/src/activate.rs b/src/activate.rs
index f75303f..64baa4f 100644
--- a/src/activate.rs
+++ b/src/activate.rs
@@ -26,10 +26,6 @@ struct Opts {
     profile_path: String,
     closure: String,
 
-    /// Command for activating the given profile
-    #[clap(long)]
-    activate_cmd: Option<String>,
-
     /// Command for bootstrapping
     #[clap(long)]
     bootstrap_cmd: Option<String>,
@@ -42,21 +38,24 @@ struct Opts {
 pub async fn activate(
     profile_path: String,
     closure: String,
-    activate_cmd: Option<String>,
     bootstrap_cmd: Option<String>,
     auto_rollback: bool,
 ) -> Result<(), Box<dyn std::error::Error>> {
     info!("Activating profile");
 
-    Command::new("nix-env")
+    let nix_env_set_exit_status = Command::new("nix-env")
         .arg("-p")
         .arg(&profile_path)
         .arg("--set")
         .arg(&closure)
         .stdout(Stdio::null())
-        .spawn()?
+        .status()
         .await?;
 
+    if !nix_env_set_exit_status.success() {
+        good_panic!("Failed to update nix-env generation");
+    }
+
     if let (Some(bootstrap_cmd), false) = (bootstrap_cmd, !Path::new(&profile_path).exists()) {
         let bootstrap_status = Command::new("bash")
             .arg("-c")
@@ -76,87 +75,85 @@ pub async fn activate(
         }
     }
 
-    if let Some(activate_cmd) = activate_cmd {
-        let activate_status = Command::new("bash")
-            .arg("-c")
-            .arg(&activate_cmd)
-            .env("PROFILE", &profile_path)
-            .status()
-            .await;
+    let activate_status = Command::new(format!("{}/activate", profile_path))
+        .env("PROFILE", &profile_path)
+        .status()
+        .await;
+
+    match activate_status {
+        Ok(s) if s.success() => (),
+        _ if auto_rollback => {
+            error!("Failed to execute activation command");
+
+            let nix_env_rollback_exit_status = Command::new("nix-env")
+                .arg("-p")
+                .arg(&profile_path)
+                .arg("--rollback")
+                .stdout(Stdio::null())
+                .stderr(Stdio::null())
+                .status()
+                .await?;
+
+            if !nix_env_rollback_exit_status.success() {
+                good_panic!("`nix-env --rollback` failed");
+            }
 
-        match activate_status {
-            Ok(s) if s.success() => (),
-            _ if auto_rollback => {
-                error!("Failed to execute activation command");
-
-                let nix_env_rollback_exit_status = Command::new("nix-env")
-                    .arg("-p")
-                    .arg(&profile_path)
-                    .arg("--rollback")
-                    .stdout(Stdio::null())
-                    .stderr(Stdio::null())
-                    .status()
-                    .await?;
-
-                if !nix_env_rollback_exit_status.success() {
-                    good_panic!("`nix-env --rollback` failed");
-                }
-
-                let nix_env_list_generations_out = Command::new("nix-env")
-                    .arg("-p")
-                    .arg(&profile_path)
-                    .arg("--list-generations")
-                    .output()
-                    .await?;
-
-                if !nix_env_list_generations_out.status.success() {
-                    good_panic!("Listing `nix-env` generations failed");
-                }
-
-                let generations_list = String::from_utf8(nix_env_list_generations_out.stdout)?;
-
-                let last_generation_line = generations_list
-                    .lines()
-                    .last()
-                    .expect("Expected to find a generation in list");
-
-                let last_generation_id = last_generation_line
-                    .split_whitespace()
-                    .next()
-                    .expect("Expected to get ID from generation entry");
-
-                debug!("Removing generation entry {}", last_generation_line);
-                warn!("Removing generation by ID {}", last_generation_id);
-
-                let nix_env_delete_generation_exit_status = Command::new("nix-env")
-                    .arg("-p")
-                    .arg(&profile_path)
-                    .arg("--delete-generations")
-                    .arg(last_generation_id)
-                    .stdout(Stdio::null())
-                    .stderr(Stdio::null())
-                    .status()
-                    .await?;
-
-                if !nix_env_delete_generation_exit_status.success() {
-                    good_panic!("Failed to delete failed generation");
-                }
-
-                // TODO: Find some way to make sure this command never changes, otherwise this will not work
-                let re_activate_exit_status = Command::new("bash")
-                    .arg("-c")
-                    .arg(&activate_cmd)
-                    .status()
-                    .await?;
-
-                if !re_activate_exit_status.success() {
-                    good_panic!("Failed to re-activate the last generation");
-                }
-
-                std::process::exit(1);
+            debug!("Listing generations");
+
+            let nix_env_list_generations_out = Command::new("nix-env")
+                .arg("-p")
+                .arg(&profile_path)
+                .arg("--list-generations")
+                .output()
+                .await?;
+
+            if !nix_env_list_generations_out.status.success() {
+                good_panic!("Listing `nix-env` generations failed");
             }
-            _ => {}
+
+            let generations_list = String::from_utf8(nix_env_list_generations_out.stdout)?;
+
+            let last_generation_line = generations_list
+                .lines()
+                .last()
+                .expect("Expected to find a generation in list");
+
+            let last_generation_id = last_generation_line
+                .split_whitespace()
+                .next()
+                .expect("Expected to get ID from generation entry");
+
+            debug!("Removing generation entry {}", last_generation_line);
+            warn!("Removing generation by ID {}", last_generation_id);
+
+            let nix_env_delete_generation_exit_status = Command::new("nix-env")
+                .arg("-p")
+                .arg(&profile_path)
+                .arg("--delete-generations")
+                .arg(last_generation_id)
+                .stdout(Stdio::null())
+                .stderr(Stdio::null())
+                .status()
+                .await?;
+
+            if !nix_env_delete_generation_exit_status.success() {
+                good_panic!("Failed to delete failed generation");
+            }
+
+            info!("Attempting re-activate last generation");
+
+            let re_activate_exit_status = Command::new(format!("{}/activate", profile_path))
+                .env("PROFILE", &profile_path)
+                .status()
+                .await?;
+
+            if !re_activate_exit_status.success() {
+                good_panic!("Failed to re-activate the last generation");
+            }
+
+            std::process::exit(1);
         }
+        _ => {}
     }
 
     Ok(())
@@ -175,7 +172,6 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
     activate(
         opts.profile_path,
         opts.closure,
-        opts.activate_cmd,
         opts.bootstrap_cmd,
         opts.auto_rollback,
     )
diff --git a/src/utils/data.rs b/src/utils/data.rs
index d1dae5b..de6adfc 100644
--- a/src/utils/data.rs
+++ b/src/utils/data.rs
@@ -34,7 +34,6 @@ pub struct NodeSettings {
 #[derive(Deserialize, Debug, Clone)]
 pub struct ProfileSettings {
     pub path: String,
-    pub activate: Option<String>,
     pub bootstrap: Option<String>,
 }
 
diff --git a/src/utils/deploy.rs b/src/utils/deploy.rs
index f1f4210..7301967 100644
--- a/src/utils/deploy.rs
+++ b/src/utils/deploy.rs
@@ -9,10 +9,9 @@ fn build_activate_command(
     sudo: &Option<String>,
     profile_path: &str,
     closure: &str,
-    activate_cmd: &Option<String>,
     bootstrap_cmd: &Option<String>,
     auto_rollback: bool,
-) -> Result<String, Box<dyn std::error::Error>> {
+) -> String {
     let mut self_activate_command =
         format!("{} '{}' '{}'", activate_path_str, profile_path, closure);
 
@@ -27,18 +26,11 @@ fn build_activate_command(
         );
     }
 
-    if let Some(ref activate_cmd) = activate_cmd {
-        self_activate_command = format!(
-            "{} --activate-cmd '{}'",
-            self_activate_command, activate_cmd
-        );
-    }
-
     if auto_rollback {
         self_activate_command = format!("{} --auto-rollback", self_activate_command);
     }
 
-    Ok(self_activate_command)
+    self_activate_command
 }
 
 #[test]
@@ -47,22 +39,21 @@ fn test_activation_command_builder() {
     let sudo = Some("sudo -u test".to_string());
     let profile_path = "/blah/profiles/test";
     let closure = "/blah/etc";
-    let activate_cmd = Some("$THING/bin/aaaaaaa".to_string());
     let bootstrap_cmd = None;
     let auto_rollback = true;
 
-    match build_activate_command(
-        activate_path_str,
-        &sudo,
-        profile_path,
-        closure,
-        &activate_cmd,
-        &bootstrap_cmd,
-        auto_rollback,
-    ) {
-        Err(_) => panic!(""),
-        Ok(x) => assert_eq!(x, "sudo -u test /blah/bin/activate '/blah/profiles/test' '/blah/etc' --activate-cmd '$THING/bin/aaaaaaa' --auto-rollback".to_string()),
-    }
+    assert_eq!(
+        build_activate_command(
+            activate_path_str,
+            &sudo,
+            profile_path,
+            closure,
+            &bootstrap_cmd,
+            auto_rollback,
+        ),
+        "sudo -u test /blah/bin/activate '/blah/profiles/test' '/blah/etc' --auto-rollback"
+            .to_string(),
+    );
 }
 
 pub async fn deploy_profile(
@@ -81,10 +72,9 @@ pub async fn deploy_profile(
         &deploy_defs.sudo,
         &deploy_defs.profile_path,
         &deploy_data.profile.profile_settings.path,
-        &deploy_data.profile.profile_settings.activate,
         &deploy_data.profile.profile_settings.bootstrap,
         deploy_data.merged_settings.auto_rollback,
-    )?;
+    );
 
     let hostname = match deploy_data.cmd_overrides.hostname {
         Some(ref x) => x,
diff --git a/src/utils/push.rs b/src/utils/push.rs
index a973572..3f48d68 100644
--- a/src/utils/push.rs
+++ b/src/utils/push.rs
@@ -17,11 +17,6 @@ pub async fn push_profile(
         deploy_data.profile_name, deploy_data.node_name
     );
 
-    debug!(
-        "Building profile `{} for node `{}`",
-        deploy_data.profile_name, deploy_data.node_name
-    );
-
     let build_exit_status = if supports_flakes {
         Command::new("nix")
             .arg("build")