Merge pull request #231 from serokell/rvem/#201-dont-hardcode-profile-directory

[#201] Deduce profile directory during activation

3 files changed, 166 insertions, 47 deletions

diff --git a/src/bin/activate.rs b/src/bin/activate.rs
index bf03538..4a2760b 100644
--- a/src/bin/activate.rs
+++ b/src/bin/activate.rs
@@ -15,9 +15,10 @@ use tokio::time::timeout;
 
 use std::time::Duration;
 
-use std::path::PathBuf;
+use std::env;
+use std::path::{Path, PathBuf};
 
-use notify::{RecommendedWatcher, RecursiveMode, Watcher, recommended_watcher};
+use notify::{recommended_watcher, RecommendedWatcher, RecursiveMode, Watcher};
 
 use thiserror::Error;
 
@@ -47,11 +48,24 @@ enum SubCommand {
 
 /// Activate a profile
 #[derive(Clap, Debug)]
+#[clap(group(
+    clap::ArgGroup::new("profile")
+        .required(true)
+        .multiple(false)
+        .args(&["profile-path","profile-user"])
+))]
 struct ActivateOpts {
     /// The closure to activate
     closure: String,
     /// The profile path to install into
-    profile_path: String,
+    #[clap(long)]
+    profile_path: Option<String>,
+    /// The profile user if explicit profile path is not specified
+    #[clap(long, requires = "profile-name")]
+    profile_user: Option<String>,
+    /// The profile name
+    #[clap(long, requires = "profile-user")]
+    profile_name: Option<String>,
 
     /// Maximum time to wait for confirmation after activation
     #[clap(long)]
@@ -78,7 +92,7 @@ struct ActivateOpts {
     temp_path: PathBuf,
 }
 
-/// Activate a profile
+/// Wait for profile activation
 #[derive(Clap, Debug)]
 struct WaitOpts {
     /// The closure to wait for
@@ -89,11 +103,18 @@ struct WaitOpts {
     temp_path: PathBuf,
 }
 
-/// Activate a profile
+/// Revoke profile activation
 #[derive(Clap, Debug)]
 struct RevokeOpts {
-    /// The profile path to revoke
-    profile_path: String,
+    /// The profile path to install into
+    #[clap(long)]
+    profile_path: Option<String>,
+    /// The profile user if explicit profile path is not specified
+    #[clap(long, requires = "profile-name")]
+    profile_user: Option<String>,
+    /// The profile name
+    #[clap(long, requires = "profile-user")]
+    profile_name: Option<String>,
 }
 
 #[derive(Error, Debug)]
@@ -315,8 +336,8 @@ pub async fn wait(temp_path: PathBuf, closure: String) -> Result<(), WaitError>
                             // 'lock_path' may not exist yet when some other files are created in 'temp_path'
                             // x is already supposed to be canonical path
                             Ok(lock_path) if x == &lock_path => created.try_send(Ok(())),
-                            _ => Ok (())
-                        }
+                            _ => Ok(()),
+                        },
                         _ => Ok(()),
                     }
                 }
@@ -459,6 +480,61 @@ async fn revoke(profile_path: String) -> Result<(), DeactivateError> {
     Ok(())
 }
 
+#[derive(Error, Debug)]
+pub enum GetProfilePathError {
+    #[error("Failed to deduce HOME directory for user {0}")]
+    NoUserHome(String),
+}
+
+fn get_profile_path(
+    profile_path: Option<String>,
+    profile_user: Option<String>,
+    profile_name: Option<String>,
+) -> Result<String, GetProfilePathError> {
+    match (profile_path, profile_user, profile_name) {
+        (Some(profile_path), None, None) => Ok(profile_path),
+        (None, Some(profile_user), Some(profile_name)) => {
+            let nix_state_dir = env::var("NIX_STATE_DIR").unwrap_or("/nix/var/nix".to_string());
+            // As per https://nixos.org/manual/nix/stable/command-ref/files/profiles#profiles
+            match &profile_user[..] {
+                "root" => {
+                    match &profile_name[..] {
+                        // NixOS system profile belongs to the root user, but isn't stored in the 'per-user/root'
+                        "system" => Ok(format!("{}/profiles/system", nix_state_dir)),
+                        _ => Ok(format!(
+                            "{}/profiles/per-user/root/{}",
+                            nix_state_dir, profile_name
+                        )),
+                    }
+                }
+                _ => {
+                    let old_user_profiles_dir =
+                        format!("{}/profiles/per-user/{}", nix_state_dir, profile_user);
+                    // To stay backward compatible
+                    if Path::new(&old_user_profiles_dir).exists() {
+                        Ok(format!("{}/{}", old_user_profiles_dir, profile_name))
+                    } else {
+                        // https://github.com/NixOS/nix/blob/2.17.0/src/libstore/profiles.cc#L308
+                        // This is basically the equivalent of calling 'dirs::state_dir()'.
+                        // However, this function returns 'None' on macOS, while nix will actually
+                        // check env variables, so we imitate nix implementation below instead of
+                        // using 'dirs::state_dir()' directly.
+                        let state_dir = env::var("XDG_STATE_HOME").or_else(|_| {
+                            dirs::home_dir()
+                                .map(|h| {
+                                    format!("{}/.local/state", h.as_path().display().to_string())
+                                })
+                                .ok_or(GetProfilePathError::NoUserHome(profile_user))
+                        })?;
+                        Ok(format!("{}/nix/profiles/{}", state_dir, profile_name))
+                    }
+                }
+            }
+        }
+        _ => panic!("impossible"),
+    }
+}
+
 #[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
     // Ensure that this process stays alive after the SSH connection dies
@@ -483,7 +559,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 
     let r = match opts.subcmd {
         SubCommand::Activate(activate_opts) => activate(
-            activate_opts.profile_path,
+            get_profile_path(
+                activate_opts.profile_path,
+                activate_opts.profile_user,
+                activate_opts.profile_name,
+            )?,
             activate_opts.closure,
             activate_opts.auto_rollback,
             activate_opts.temp_path,
@@ -499,9 +579,13 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
             .await
             .map_err(|x| Box::new(x) as Box<dyn std::error::Error>),
 
-        SubCommand::Revoke(revoke_opts) => revoke(revoke_opts.profile_path)
-            .await
-            .map_err(|x| Box::new(x) as Box<dyn std::error::Error>),
+        SubCommand::Revoke(revoke_opts) => revoke(get_profile_path(
+            revoke_opts.profile_path,
+            revoke_opts.profile_user,
+            revoke_opts.profile_name,
+        )?)
+        .await
+        .map_err(|x| Box::new(x) as Box<dyn std::error::Error>),
     };
 
     match r {
diff --git a/src/deploy.rs b/src/deploy.rs
index 574e9b2..41cd58b 100644
--- a/src/deploy.rs
+++ b/src/deploy.rs
@@ -9,11 +9,11 @@ use std::path::Path;
 use thiserror::Error;
 use tokio::process::Command;
 
-use crate::DeployDataDefsError;
+use crate::{DeployDataDefsError, ProfileInfo};
 
 struct ActivateCommandData<'a> {
     sudo: &'a Option<String>,
-    profile_path: &'a str,
+    profile_info: &'a ProfileInfo,
     closure: &'a str,
     auto_rollback: bool,
     temp_path: &'a Path,
@@ -37,8 +37,21 @@ fn build_activate_command(data: &ActivateCommandData) -> String {
     }
 
     self_activate_command = format!(
-        "{} activate '{}' '{}' --temp-path '{}'",
-        self_activate_command, data.closure, data.profile_path, data.temp_path.display()
+        "{} activate '{}' {} --temp-path '{}'",
+        self_activate_command,
+        data.closure,
+        match data.profile_info {
+            ProfileInfo::ProfilePath { profile_path } =>
+                format!("--profile-path '{}'", profile_path),
+            ProfileInfo::ProfileUserAndName {
+                profile_user,
+                profile_name,
+            } => format!(
+                "--profile-user {} --profile-name {}",
+                profile_user, profile_name
+            ),
+        },
+        data.temp_path.display()
     );
 
     self_activate_command = format!(
@@ -72,7 +85,9 @@ fn build_activate_command(data: &ActivateCommandData) -> String {
 #[test]
 fn test_activation_command_builder() {
     let sudo = Some("sudo -u test".to_string());
-    let profile_path = "/blah/profiles/test";
+    let profile_info = &ProfileInfo::ProfilePath {
+        profile_path: "/blah/profiles/test".to_string(),
+    };
     let closure = "/nix/store/blah/etc";
     let auto_rollback = true;
     let dry_activate = false;
@@ -86,7 +101,7 @@ fn test_activation_command_builder() {
     assert_eq!(
         build_activate_command(&ActivateCommandData {
             sudo: &sudo,
-            profile_path,
+            profile_info,
             closure,
             auto_rollback,
             temp_path,
@@ -97,7 +112,7 @@ fn test_activation_command_builder() {
             dry_activate,
             boot,
         }),
-        "sudo -u test /nix/store/blah/etc/activate-rs --debug-logs --log-dir /tmp/something.txt activate '/nix/store/blah/etc' '/blah/profiles/test' --temp-path '/tmp' --confirm-timeout 30 --magic-rollback --auto-rollback"
+        "sudo -u test /nix/store/blah/etc/activate-rs --debug-logs --log-dir /tmp/something.txt activate '/nix/store/blah/etc' --profile-path '/blah/profiles/test' --temp-path '/tmp' --confirm-timeout 30 --magic-rollback --auto-rollback"
             .to_string(),
     );
 }
@@ -123,7 +138,9 @@ fn build_wait_command(data: &WaitCommandData) -> String {
 
     self_activate_command = format!(
         "{} wait '{}' --temp-path '{}'",
-        self_activate_command, data.closure, data.temp_path.display(),
+        self_activate_command,
+        data.closure,
+        data.temp_path.display(),
     );
 
     if let Some(sudo_cmd) = &data.sudo {
@@ -157,7 +174,7 @@ fn test_wait_command_builder() {
 struct RevokeCommandData<'a> {
     sudo: &'a Option<String>,
     closure: &'a str,
-    profile_path: &'a str,
+    profile_info: ProfileInfo,
     debug_logs: bool,
     log_dir: Option<&'a str>,
 }
@@ -173,7 +190,21 @@ fn build_revoke_command(data: &RevokeCommandData) -> String {
         self_activate_command = format!("{} --log-dir {}", self_activate_command, log_dir);
     }
 
-    self_activate_command = format!("{} revoke '{}'", self_activate_command, data.profile_path);
+    self_activate_command = format!(
+        "{} revoke {}",
+        self_activate_command,
+        match &data.profile_info {
+            ProfileInfo::ProfilePath { profile_path } =>
+                format!("--profile-path '{}'", profile_path),
+            ProfileInfo::ProfileUserAndName {
+                profile_user,
+                profile_name,
+            } => format!(
+                "--profile-user {} --profile-name {}",
+                profile_user, profile_name
+            ),
+        }
+    );
 
     if let Some(sudo_cmd) = &data.sudo {
         self_activate_command = format!("{} {}", sudo_cmd, self_activate_command);
@@ -186,7 +217,9 @@ fn build_revoke_command(data: &RevokeCommandData) -> String {
 fn test_revoke_command_builder() {
     let sudo = Some("sudo -u test".to_string());
     let closure = "/nix/store/blah/etc";
-    let profile_path = "/nix/var/nix/per-user/user/profile";
+    let profile_info = ProfileInfo::ProfilePath {
+        profile_path: "/nix/var/nix/per-user/user/profile".to_string(),
+    };
     let debug_logs = true;
     let log_dir = Some("/tmp/something.txt");
 
@@ -194,11 +227,11 @@ fn test_revoke_command_builder() {
         build_revoke_command(&RevokeCommandData {
             sudo: &sudo,
             closure,
-            profile_path,
+            profile_info,
             debug_logs,
             log_dir
         }),
-        "sudo -u test /nix/store/blah/etc/activate-rs --debug-logs --log-dir /tmp/something.txt revoke '/nix/var/nix/per-user/user/profile'"
+        "sudo -u test /nix/store/blah/etc/activate-rs --debug-logs --log-dir /tmp/something.txt revoke --profile-path '/nix/var/nix/per-user/user/profile'"
             .to_string(),
     );
 }
@@ -271,6 +304,8 @@ pub enum DeployProfileError {
 
     #[error("Error confirming deployment: {0}")]
     Confirm(#[from] ConfirmProfileError),
+    #[error("Deployment data invalid: {0}")]
+    InvalidDeployDataDefs(#[from] DeployDataDefsError),
 }
 
 pub async fn deploy_profile(
@@ -299,7 +334,7 @@ pub async fn deploy_profile(
 
     let self_activate_command = build_activate_command(&ActivateCommandData {
         sudo: &deploy_defs.sudo,
-        profile_path: &deploy_defs.profile_path,
+        profile_info: &deploy_data.get_profile_info()?,
         closure: &deploy_data.profile.profile_settings.path,
         auto_rollback,
         temp_path: temp_path,
@@ -439,7 +474,7 @@ pub async fn revoke(
     let self_revoke_command = build_revoke_command(&RevokeCommandData {
         sudo: &deploy_defs.sudo,
         closure: &deploy_data.profile.profile_settings.path,
-        profile_path: &deploy_data.get_profile_path()?,
+        profile_info: deploy_data.get_profile_info()?,
         debug_logs: deploy_data.debug_logs,
         log_dir: deploy_data.log_dir,
     });
diff --git a/src/lib.rs b/src/lib.rs
index c6f8e03..0e5d817 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -332,9 +332,17 @@ pub struct DeployData<'a> {
 pub struct DeployDefs {
     pub ssh_user: String,
     pub profile_user: String,
-    pub profile_path: String,
     pub sudo: Option<String>,
 }
+enum ProfileInfo {
+    ProfilePath {
+        profile_path: String,
+    },
+    ProfileUserAndName {
+        profile_user: String,
+        profile_name: String,
+    },
+}
 
 #[derive(Error, Debug)]
 pub enum DeployDataDefsError {
@@ -351,8 +359,6 @@ impl<'a> DeployData<'a> {
 
         let profile_user = self.get_profile_user()?;
 
-        let profile_path = self.get_profile_path()?;
-
         let sudo: Option<String> = match self.merged_settings.user {
             Some(ref user) if user != &ssh_user => Some(format!("{} {}", self.get_sudo(), user)),
             _ => None,
@@ -361,26 +367,10 @@ impl<'a> DeployData<'a> {
         Ok(DeployDefs {
             ssh_user,
             profile_user,
-            profile_path,
             sudo,
         })
     }
 
-    fn get_profile_path(&'a self) -> Result<String, DeployDataDefsError> {
-        let profile_user = self.get_profile_user()?;
-        let profile_path = match self.profile.profile_settings.profile_path {
-            None => match &profile_user[..] {
-                "root" => format!("/nix/var/nix/profiles/{}", self.profile_name),
-                _ => format!(
-                    "/nix/var/nix/profiles/per-user/{}/{}",
-                    profile_user, self.profile_name
-                ),
-            },
-            Some(ref x) => x.clone(),
-        };
-        Ok(profile_path)
-    }
-
     fn get_profile_user(&'a self) -> Result<String, DeployDataDefsError> {
         let profile_user = match self.merged_settings.user {
             Some(ref x) => x.clone(),
@@ -403,6 +393,16 @@ impl<'a> DeployData<'a> {
             None => "sudo -u".to_string(),
         }
     }
+
+    fn get_profile_info(&'a self) -> Result<ProfileInfo, DeployDataDefsError> {
+        match self.profile.profile_settings.profile_path {
+            Some(ref profile_path) => Ok(ProfileInfo::ProfilePath { profile_path: profile_path.to_string() }),
+            None => {
+                let profile_user = self.get_profile_user()?;
+                Ok(ProfileInfo::ProfileUserAndName { profile_user, profile_name: self.profile_name.to_string() })
+            },
+        }
+    }
 }
 
 pub fn make_deploy_data<'a, 's>(