Cure APFS/Fstabs on Mac (#246)

* wip

* Do main editing portion

* Some more curing on fstab entries

* Overwrite fstab instead of append

* Add newline

* Improve --explain output for CreateNixVolume

* Tweak some permissions

* Fixup a few more permissions spots

* Improve encrypted volume handling

* Handle APFS volumes existing already to some degree

* Correct speeling

* More tweaking preparing for bootstrap/kickstart work

* Most of volume curing works

* Make kickstart use domain/service too

* Fixup nits

* Fix a missing format!

src/action/common/configure_init_service.rs

@@ -277,7 +277,7 @@ impl Action for ConfigureInitService {
             InitSystem::Launchd => {
                 vec![ActionDescription::new(
                     "Unconfigure Nix daemon related settings with launchctl".to_string(),
-                    vec!["Run `launchctl unload {DARWIN_NIX_DAEMON_DEST}`".to_string()],
+                    vec![format!("Run `launchctl unload {DARWIN_NIX_DAEMON_DEST}`")],
                 )]
             },
             #[cfg(not(target_os = "macos"))]

src/action/macos/bootstrap_apfs_volume.rs

@@ -1,103 +0,0 @@
-use std::path::{Path, PathBuf};
-
-use tokio::process::Command;
-use tracing::{span, Span};
-
-use crate::action::{ActionError, ActionTag, StatefulAction};
-use crate::execute_command;
-
-use crate::action::{Action, ActionDescription};
-
-/**
-Bootstrap and kickstart an APFS volume
-*/
-#[derive(Debug, serde::Deserialize, serde::Serialize, Clone)]
-pub struct BootstrapApfsVolume {
-    path: PathBuf,
-}
-
-impl BootstrapApfsVolume {
-    #[tracing::instrument(level = "debug", skip_all)]
-    pub async fn plan(path: impl AsRef<Path>) -> Result<StatefulAction<Self>, ActionError> {
-        Ok(Self {
-            path: path.as_ref().to_path_buf(),
-        }
-        .into())
-    }
-}
-
-#[async_trait::async_trait]
-#[typetag::serde(name = "bootstrap_apfs_volume")]
-impl Action for BootstrapApfsVolume {
-    fn action_tag() -> ActionTag {
-        ActionTag("bootstrap_apfs_volume")
-    }
-    fn tracing_synopsis(&self) -> String {
-        format!("Bootstrap and kickstart `{}`", self.path.display())
-    }
-
-    fn tracing_span(&self) -> Span {
-        span!(
-            tracing::Level::DEBUG,
-            "bootstrap_apfs_volume",
-            path = %self.path.display(),
-        )
-    }
-
-    fn execute_description(&self) -> Vec<ActionDescription> {
-        vec![ActionDescription::new(self.tracing_synopsis(), vec![])]
-    }
-
-    #[tracing::instrument(level = "debug", skip_all)]
-    async fn execute(&mut self) -> Result<(), ActionError> {
-        let Self { path } = self;
-
-        execute_command(
-            Command::new("launchctl")
-                .process_group(0)
-                .args(["bootstrap", "system"])
-                .arg(path)
-                .stdin(std::process::Stdio::null()),
-        )
-        .await?;
-        execute_command(
-            Command::new("launchctl")
-                .process_group(0)
-                .args(["kickstart", "-k", "system/org.nixos.darwin-store"])
-                .stdin(std::process::Stdio::null()),
-        )
-        .await?;
-
-        Ok(())
-    }
-
-    fn revert_description(&self) -> Vec<ActionDescription> {
-        vec![ActionDescription::new(
-            format!("Stop `{}`", self.path.display()),
-            vec![],
-        )]
-    }
-
-    #[tracing::instrument(level = "debug", skip_all)]
-    async fn revert(&mut self) -> Result<(), ActionError> {
-        let Self { path } = self;
-
-        execute_command(
-            Command::new("launchctl")
-                .process_group(0)
-                .args(["bootout", "system"])
-                .arg(path)
-                .stdin(std::process::Stdio::null()),
-        )
-        .await?;
-
-        Ok(())
-    }
-}
-
-#[non_exhaustive]
-#[derive(Debug, thiserror::Error)]
-pub enum BootstrapVolumeError {
-    #[error("Failed to execute command")]
-    Command(#[source] std::io::Error),
-}

src/action/macos/bootstrap_launchctl_service.rs

@@ -0,0 +1,141 @@
+use std::path::{Path, PathBuf};
+
+use tokio::process::Command;
+use tracing::{span, Span};
+
+use crate::action::{ActionError, ActionTag, StatefulAction};
+use crate::execute_command;
+
+use crate::action::{Action, ActionDescription};
+
+/**
+Bootstrap and kickstart an APFS volume
+*/
+#[derive(Debug, serde::Deserialize, serde::Serialize, Clone)]
+pub struct BootstrapLaunchctlService {
+    domain: String,
+    service: String,
+    path: PathBuf,
+}
+
+impl BootstrapLaunchctlService {
+    #[tracing::instrument(level = "debug", skip_all)]
+    pub async fn plan(
+        domain: impl AsRef<str>,
+        service: impl AsRef<str>,
+        path: impl AsRef<Path>,
+    ) -> Result<StatefulAction<Self>, ActionError> {
+        let domain = domain.as_ref().to_string();
+        let service = service.as_ref().to_string();
+        let path = path.as_ref().to_path_buf();
+
+        let mut command = Command::new("launchctl");
+        command.process_group(0);
+        command.arg("print");
+        command.arg(format!("{domain}/{service}"));
+        command.arg("-plist");
+        command.stdin(std::process::Stdio::null());
+        command.stdout(std::process::Stdio::piped());
+        command.stderr(std::process::Stdio::piped());
+        let output = command
+            .output()
+            .await
+            .map_err(|e| ActionError::command(&command, e))?;
+        if output.status.success() || output.status.code() == Some(37) {
+            // We presume that success means it's found
+            return Ok(StatefulAction::completed(Self {
+                service,
+                domain,
+                path,
+            }));
+        }
+
+        Ok(StatefulAction::uncompleted(Self {
+            domain,
+            service,
+            path,
+        }))
+    }
+}
+
+#[async_trait::async_trait]
+#[typetag::serde(name = "bootstrap_launchctl_service")]
+impl Action for BootstrapLaunchctlService {
+    fn action_tag() -> ActionTag {
+        ActionTag("bootstrap_launchctl_service")
+    }
+    fn tracing_synopsis(&self) -> String {
+        format!(
+            "Bootstrap the `{}` service via `launchctl bootstrap {} {}`",
+            self.service,
+            self.domain,
+            self.path.display()
+        )
+    }
+
+    fn tracing_span(&self) -> Span {
+        span!(
+            tracing::Level::DEBUG,
+            "bootstrap_launchctl_service",
+            domain = self.domain,
+            path = %self.path.display(),
+        )
+    }
+
+    fn execute_description(&self) -> Vec<ActionDescription> {
+        vec![ActionDescription::new(self.tracing_synopsis(), vec![])]
+    }
+
+    #[tracing::instrument(level = "debug", skip_all)]
+    async fn execute(&mut self) -> Result<(), ActionError> {
+        let Self {
+            domain,
+            service: _,
+            path,
+        } = self;
+
+        execute_command(
+            Command::new("launchctl")
+                .process_group(0)
+                .arg("bootstrap")
+                .arg(domain)
+                .arg(path)
+                .stdin(std::process::Stdio::null()),
+        )
+        .await?;
+
+        Ok(())
+    }
+
+    fn revert_description(&self) -> Vec<ActionDescription> {
+        vec![ActionDescription::new(
+            format!(
+                "Run `launchctl bootout {} {}`",
+                self.domain,
+                self.path.display()
+            ),
+            vec![],
+        )]
+    }
+
+    #[tracing::instrument(level = "debug", skip_all)]
+    async fn revert(&mut self) -> Result<(), ActionError> {
+        let Self {
+            path,
+            service: _,
+            domain,
+        } = self;
+
+        execute_command(
+            Command::new("launchctl")
+                .process_group(0)
+                .arg("bootout")
+                .arg(domain)
+                .arg(path)
+                .stdin(std::process::Stdio::null()),
+        )
+        .await?;
+
+        Ok(())
+    }
+}

src/action/macos/create_apfs_volume.rs

@@ -5,9 +5,9 @@ use tracing::{span, Span};
 
 use crate::action::{ActionError, ActionTag, StatefulAction};
 use crate::execute_command;
-use serde::Deserialize;
 
 use crate::action::{Action, ActionDescription};
+use crate::os::darwin::DiskUtilApfsListOutput;
 
 #[derive(Debug, serde::Deserialize, serde::Serialize, Clone)]
 pub struct CreateApfsVolume {
@@ -31,19 +31,20 @@ impl CreateApfsVolume {
         for container in parsed.containers {
             for volume in container.volumes {
                 if volume.name == name {
-                    return Err(ActionError::Custom(Box::new(
+                    return Ok(StatefulAction::completed(Self {
-                        CreateApfsVolumeError::ExistingVolume(name),
+                        disk: disk.as_ref().to_path_buf(),
-                    )));
+                        name,
+                        case_sensitive,
+                    }));
                 }
             }
         }
 
-        Ok(Self {
+        Ok(StatefulAction::uncompleted(Self {
             disk: disk.as_ref().to_path_buf(),
             name,
             case_sensitive,
-        }
+        }))
-        .into())
     }
 }
 
@@ -135,28 +136,3 @@ impl Action for CreateApfsVolume {
         Ok(())
     }
 }
-
-#[non_exhaustive]
-#[derive(Debug, thiserror::Error)]
-pub enum CreateApfsVolumeError {
-    #[error("Existing volume called `{0}` found in `diskutil apfs list`, delete it with `diskutil apfs deleteVolume \"{0}\"`")]
-    ExistingVolume(String),
-}
-
-#[derive(Deserialize, Clone, Debug)]
-#[serde(rename_all = "PascalCase")]
-struct DiskUtilApfsListOutput {
-    containers: Vec<DiskUtilApfsContainer>,
-}
-
-#[derive(Deserialize, Clone, Debug)]
-#[serde(rename_all = "PascalCase")]
-struct DiskUtilApfsContainer {
-    volumes: Vec<DiskUtilApfsListVolume>,
-}
-
-#[derive(Deserialize, Clone, Debug)]
-#[serde(rename_all = "PascalCase")]
-struct DiskUtilApfsListVolume {
-    name: String,
-}

src/action/macos/create_fstab_entry.rs

@@ -1,7 +1,8 @@
 use uuid::Uuid;
 
+use super::CreateApfsVolume;
 use crate::{
-    action::{Action, ActionDescription, ActionError, ActionTag, StatefulAction},
+    action::{Action, ActionDescription, ActionError, ActionState, ActionTag, StatefulAction},
     execute_command,
 };
 use serde::Deserialize;
@@ -15,8 +16,16 @@ use tracing::{span, Span};
 
 const FSTAB_PATH: &str = "/etc/fstab";
 
-/** Create an `/etc/fstab` entry for the given volume
+#[derive(Debug, serde::Deserialize, serde::Serialize, Clone, Copy)]
+enum ExistingFstabEntry {
+    /// Need to update the existing `nix-installer` made entry
+    NixInstallerEntry,
+    /// Need to remove old entry and add new entry
+    Foreign,
+    None,
+}
 
+/** Create an `/etc/fstab` entry for the given volume
 
 This action queries `diskutil info` on the volume to fetch it's UUID and
 add the relevant information to `/etc/fstab`.
@@ -26,34 +35,52 @@ add the relevant information to `/etc/fstab`.
 #[derive(Debug, serde::Deserialize, serde::Serialize, Clone)]
 pub struct CreateFstabEntry {
     apfs_volume_label: String,
+    existing_entry: ExistingFstabEntry,
 }
 
 impl CreateFstabEntry {
     #[tracing::instrument(level = "debug", skip_all)]
-    pub async fn plan(apfs_volume_label: String) -> Result<StatefulAction<Self>, ActionError> {
+    pub async fn plan(
+        apfs_volume_label: String,
+        planned_create_apfs_volume: &StatefulAction<CreateApfsVolume>,
+    ) -> Result<StatefulAction<Self>, ActionError> {
         let fstab_path = Path::new(FSTAB_PATH);
+
         if fstab_path.exists() {
             let fstab_buf = tokio::fs::read_to_string(&fstab_path)
                 .await
                 .map_err(|e| ActionError::Read(fstab_path.to_path_buf(), e))?;
             let prelude_comment = fstab_prelude_comment(&apfs_volume_label);
 
-            // See if the user already has a `/nix` related entry, if so, invite them to remove it.
-            if fstab_buf.split(&[' ', '\t']).any(|chunk| chunk == "/nix") {
-                return Err(ActionError::Custom(Box::new(
-                    CreateFstabEntryError::NixEntryExists,
-                )));
-            }
-
             // See if a previous install from this crate exists, if so, invite the user to remove it (we may need to change it)
             if fstab_buf.contains(&prelude_comment) {
-                return Err(ActionError::Custom(Box::new(
+                if planned_create_apfs_volume.state != ActionState::Completed {
-                    CreateFstabEntryError::VolumeEntryExists(apfs_volume_label.clone()),
+                    return Ok(StatefulAction::completed(Self {
-                )));
+                        apfs_volume_label,
+                        existing_entry: ExistingFstabEntry::NixInstallerEntry,
+                    }));
+                }
+
+                return Ok(StatefulAction::uncompleted(Self {
+                    apfs_volume_label,
+                    existing_entry: ExistingFstabEntry::NixInstallerEntry,
+                }));
+            } else if fstab_buf
+                .lines()
+                .any(|line| line.split(&[' ', '\t']).nth(2) == Some("/nix"))
+            {
+                // See if the user already has a `/nix` related entry, if so, invite them to remove it.
+                return Ok(StatefulAction::uncompleted(Self {
+                    apfs_volume_label,
+                    existing_entry: ExistingFstabEntry::Foreign,
+                }));
             }
         }
 
-        Ok(Self { apfs_volume_label }.into())
+        Ok(StatefulAction::uncompleted(Self {
+            apfs_volume_label,
+            existing_entry: ExistingFstabEntry::None,
+        }))
     }
 }
 
@@ -64,10 +91,16 @@ impl Action for CreateFstabEntry {
         ActionTag("create_fstab_entry")
     }
     fn tracing_synopsis(&self) -> String {
-        format!(
+        match self.existing_entry {
-            "Add a UUID based entry for the APFS volume `{}` to `/etc/fstab`",
+            ExistingFstabEntry::NixInstallerEntry | ExistingFstabEntry::Foreign => format!(
-            self.apfs_volume_label
+                "Update existing entry for the APFS volume `{}` to `/etc/fstab`",
-        )
+                self.apfs_volume_label
+            ),
+            ExistingFstabEntry::None => format!(
+                "Add a UUID based entry for the APFS volume `{}` to `/etc/fstab`",
+                self.apfs_volume_label
+            ),
+        }
     }
 
     fn tracing_span(&self) -> Span {
@@ -75,6 +108,7 @@ impl Action for CreateFstabEntry {
             tracing::Level::DEBUG,
             "create_fstab_entry",
             apfs_volume_label = self.apfs_volume_label,
+            existing_entry = ?self.existing_entry,
         );
 
         span
@@ -86,10 +120,12 @@ impl Action for CreateFstabEntry {
 
     #[tracing::instrument(level = "debug", skip_all)]
     async fn execute(&mut self) -> Result<(), ActionError> {
-        let Self { apfs_volume_label } = self;
+        let Self {
+            apfs_volume_label,
+            existing_entry,
+        } = self;
         let fstab_path = Path::new(FSTAB_PATH);
         let uuid = get_uuid_for_label(&apfs_volume_label).await?;
-        let fstab_entry = fstab_entry(&uuid, apfs_volume_label);
 
         let mut fstab = tokio::fs::OpenOptions::new()
             .create(true)
@@ -106,20 +142,79 @@ impl Action for CreateFstabEntry {
             .await
             .map_err(|e| ActionError::Read(fstab_path.to_owned(), e))?;
 
-        if fstab_buf.contains(&fstab_entry) {
+        let updated_buf = match existing_entry {
-            tracing::debug!("Skipped writing to `/etc/fstab` as the content already existed")
+            ExistingFstabEntry::NixInstallerEntry => {
-        } else {
+                // Update the entry
-            fstab
+                let mut current_fstab_lines = fstab_buf
-                .write_all(fstab_entry.as_bytes())
+                    .lines()
-                .await
+                    .map(|v| v.to_owned())
-                .map_err(|e| ActionError::Write(fstab_path.to_owned(), e))?;
+                    .collect::<Vec<String>>();
-        }
+                let mut updated_line = false;
+                let mut saw_prelude = false;
+                let prelude = fstab_prelude_comment(&apfs_volume_label);
+                for line in current_fstab_lines.iter_mut() {
+                    if line == &prelude {
+                        saw_prelude = true;
+                        continue;
+                    }
+                    if saw_prelude && line.split(&[' ', '\t']).nth(1) == Some("/nix") {
+                        *line = fstab_entry(&uuid);
+                        updated_line = true;
+                        break;
+                    }
+                }
+                if !(saw_prelude && updated_line) {
+                    return Err(ActionError::Custom(Box::new(
+                        CreateFstabEntryError::ExistingNixInstallerEntryDisappeared,
+                    )));
+                }
+                current_fstab_lines.join("\n")
+            },
+            ExistingFstabEntry::Foreign => {
+                // Overwrite the existing entry with our own
+                let mut current_fstab_lines = fstab_buf
+                    .lines()
+                    .map(|v| v.to_owned())
+                    .collect::<Vec<String>>();
+                let mut updated_line = false;
+                for line in current_fstab_lines.iter_mut() {
+                    if line.split(&[' ', '\t']).nth(2) == Some("/nix") {
+                        *line = fstab_lines(&uuid, apfs_volume_label);
+                        updated_line = true;
+                        break;
+                    }
+                }
+                if !updated_line {
+                    return Err(ActionError::Custom(Box::new(
+                        CreateFstabEntryError::ExistingForeignEntryDisappeared,
+                    )));
+                }
+                current_fstab_lines.join("\n")
+            },
+            ExistingFstabEntry::None => fstab_buf + "\n" + &fstab_lines(&uuid, apfs_volume_label),
+        };
+
+        fstab
+            .seek(SeekFrom::Start(0))
+            .await
+            .map_err(|e| ActionError::Seek(fstab_path.to_owned(), e))?;
+        fstab
+            .set_len(0)
+            .await
+            .map_err(|e| ActionError::Truncate(fstab_path.to_owned(), e))?;
+        fstab
+            .write_all(updated_buf.as_bytes())
+            .await
+            .map_err(|e| ActionError::Write(fstab_path.to_owned(), e))?;
 
         Ok(())
     }
 
     fn revert_description(&self) -> Vec<ActionDescription> {
-        let Self { apfs_volume_label } = &self;
+        let Self {
+            apfs_volume_label,
+            existing_entry: _,
+        } = &self;
         vec![ActionDescription::new(
             format!(
                 "Remove the UUID based entry for the APFS volume `{}` in `/etc/fstab`",
@@ -131,10 +226,13 @@ impl Action for CreateFstabEntry {
 
     #[tracing::instrument(level = "debug", skip_all)]
     async fn revert(&mut self) -> Result<(), ActionError> {
-        let Self { apfs_volume_label } = self;
+        let Self {
+            apfs_volume_label,
+            existing_entry: _,
+        } = self;
         let fstab_path = Path::new(FSTAB_PATH);
         let uuid = get_uuid_for_label(&apfs_volume_label).await?;
-        let fstab_entry = fstab_entry(&uuid, apfs_volume_label);
+        let fstab_entry = fstab_lines(&uuid, apfs_volume_label);
 
         let mut file = OpenOptions::new()
             .create(false)
@@ -188,27 +286,27 @@ async fn get_uuid_for_label(apfs_volume_label: &str) -> Result<Uuid, ActionError
     Ok(parsed.volume_uuid)
 }
 
+fn fstab_lines(uuid: &Uuid, apfs_volume_label: &str) -> String {
+    let prelude_comment = fstab_prelude_comment(apfs_volume_label);
+    let fstab_entry = fstab_entry(uuid);
+    prelude_comment + "\n" + &fstab_entry
+}
+
 fn fstab_prelude_comment(apfs_volume_label: &str) -> String {
     format!("# nix-installer created volume labelled `{apfs_volume_label}`")
 }
 
-fn fstab_entry(uuid: &Uuid, apfs_volume_label: &str) -> String {
+fn fstab_entry(uuid: &Uuid) -> String {
-    let prelude_comment = fstab_prelude_comment(apfs_volume_label);
+    format!("UUID={uuid} /nix apfs rw,noauto,nobrowse,suid,owners")
-    format!(
-        "\
-        {prelude_comment}\n\
-        UUID={uuid} /nix apfs rw,noauto,nobrowse,suid,owners\n\
-        "
-    )
 }
 
 #[non_exhaustive]
 #[derive(thiserror::Error, Debug)]
 pub enum CreateFstabEntryError {
-    #[error("An `/etc/fstab` entry for the `/nix` path already exists, consider removing the entry for `/nix`d from `/etc/fstab`")]
+    #[error("The `/etc/fstab` entry (previously created by a `nix-installer` install) detected during planning disappeared between planning and executing. Cannot update `/etc/fstab` as planned")]
-    NixEntryExists,
+    ExistingNixInstallerEntryDisappeared,
-    #[error("An `/etc/fstab` entry created by `nix-installer` already exists. If a volume named `{0}` already exists, it may need to be deleted with `diskutil apfs deleteVolume \"{0}\" and the entry for `/nix` should be removed from `/etc/fstab`")]
+    #[error("The `/etc/fstab` entry (previously created by the official install scripts) detected during planning disappeared between planning and executing. Cannot update `/etc/fstab` as planned")]
-    VolumeEntryExists(String),
+    ExistingForeignEntryDisappeared,
 }
 
 #[derive(Deserialize, Clone, Debug)]

src/action/macos/create_nix_volume.rs

@@ -1,7 +1,7 @@
 use crate::action::{
     base::{create_or_insert_into_file, CreateFile, CreateOrInsertIntoFile},
     macos::{
-        BootstrapApfsVolume, CreateApfsVolume, CreateSyntheticObjects, EnableOwnership,
+        BootstrapLaunchctlService, CreateApfsVolume, CreateSyntheticObjects, EnableOwnership,
         EncryptApfsVolume, UnmountApfsVolume,
     },
     Action, ActionDescription, ActionError, ActionTag, StatefulAction,
@@ -13,7 +13,7 @@ use std::{
 use tokio::process::Command;
 use tracing::{span, Span};
 
-use super::create_fstab_entry::CreateFstabEntry;
+use super::{create_fstab_entry::CreateFstabEntry, KickstartLaunchctlService};
 
 pub const NIX_VOLUME_MOUNTD_DEST: &str = "/Library/LaunchDaemons/org.nixos.darwin-store.plist";
 
@@ -31,7 +31,8 @@ pub struct CreateNixVolume {
     create_fstab_entry: StatefulAction<CreateFstabEntry>,
     encrypt_volume: Option<StatefulAction<EncryptApfsVolume>>,
     setup_volume_daemon: StatefulAction<CreateFile>,
-    bootstrap_volume: StatefulAction<BootstrapApfsVolume>,
+    bootstrap_volume: StatefulAction<BootstrapLaunchctlService>,
+    kickstart_launchctl_service: StatefulAction<KickstartLaunchctlService>,
     enable_ownership: StatefulAction<EnableOwnership>,
 }
 
@@ -67,12 +68,12 @@ impl CreateNixVolume {
             .await
             .map_err(|e| ActionError::Child(CreateApfsVolume::action_tag(), Box::new(e)))?;
 
-        let create_fstab_entry = CreateFstabEntry::plan(name.clone())
+        let create_fstab_entry = CreateFstabEntry::plan(name.clone(), &create_volume)
             .await
             .map_err(|e| ActionError::Child(CreateFstabEntry::action_tag(), Box::new(e)))?;
 
         let encrypt_volume = if encrypt {
-            Some(EncryptApfsVolume::plan(disk, &name).await?)
+            Some(EncryptApfsVolume::plan(disk, &name, &create_volume).await?)
         } else {
             None
         };
@@ -116,12 +117,20 @@ impl CreateNixVolume {
                 .await
                 .map_err(|e| ActionError::Child(CreateFile::action_tag(), Box::new(e)))?;
 
-        let bootstrap_volume = BootstrapApfsVolume::plan(NIX_VOLUME_MOUNTD_DEST)
+        let bootstrap_volume = BootstrapLaunchctlService::plan(
-            .await
+            "system",
-            .map_err(|e| ActionError::Child(BootstrapApfsVolume::action_tag(), Box::new(e)))?;
+            "org.nixos.darwin-store",
-        let enable_ownership = EnableOwnership::plan("/nix")
+            NIX_VOLUME_MOUNTD_DEST,
-            .await
+        )
-            .map_err(|e| ActionError::Child(EnableOwnership::action_tag(), Box::new(e)))?;
+        .await
+        .map_err(|e| ActionError::Child(BootstrapLaunchctlService::action_tag(), Box::new(e)))?;
+        let kickstart_launchctl_service =
+            KickstartLaunchctlService::plan("system", "org.nixos.darwin-store")
+                .await
+                .map_err(|e| {
+                    ActionError::Child(KickstartLaunchctlService::action_tag(), Box::new(e))
+                })?;
+        let enable_ownership = EnableOwnership::plan("/nix").await?;
 
         Ok(Self {
             disk: disk.to_path_buf(),
@@ -136,6 +145,7 @@ impl CreateNixVolume {
             encrypt_volume,
             setup_volume_daemon,
             bootstrap_volume,
+            kickstart_launchctl_service,
             enable_ownership,
         }
         .into())
@@ -150,9 +160,10 @@ impl Action for CreateNixVolume {
     }
     fn tracing_synopsis(&self) -> String {
         format!(
-            "Create an APFS volume `{}` for Nix on `{}`",
+            "Create an{maybe_encrypted} APFS volume `{name}` for Nix on `{disk}` and add it to `/etc/fstab` mounting on `/nix`",
-            self.name,
+            maybe_encrypted = if self.encrypt { " encrypted" } else { "" }, 
-            self.disk.display()
+            name = self.name,
+            disk = self.disk.display(),
         )
     }
 
@@ -166,10 +177,23 @@ impl Action for CreateNixVolume {
     }
 
     fn execute_description(&self) -> Vec<ActionDescription> {
-        let Self {
+        let mut explanation = vec![
-            disk: _, name: _, ..
+            self.create_or_append_synthetic_conf.tracing_synopsis(),
-        } = &self;
+            self.create_synthetic_objects.tracing_synopsis(),
-        vec![ActionDescription::new(self.tracing_synopsis(), vec![])]
+            self.unmount_volume.tracing_synopsis(),
+            self.create_volume.tracing_synopsis(),
+            self.create_fstab_entry.tracing_synopsis(),
+        ];
+        if let Some(encrypt_volume) = &self.encrypt_volume {
+            explanation.push(encrypt_volume.tracing_synopsis());
+        }
+        explanation.append(&mut vec![
+            self.setup_volume_daemon.tracing_synopsis(),
+            self.bootstrap_volume.tracing_synopsis(),
+            self.enable_ownership.tracing_synopsis(),
+        ]);
+
+        vec![ActionDescription::new(self.tracing_synopsis(), explanation)]
     }
 
     #[tracing::instrument(level = "debug", skip_all)]
@@ -202,7 +226,7 @@ impl Action for CreateNixVolume {
             encrypt_volume
                 .try_execute()
                 .await
-                .map_err(|e| ActionError::Child(encrypt_volume.action_tag(), Box::new(e)))?;
+                .map_err(|e| ActionError::Child(encrypt_volume.action_tag(), Box::new(e)))?
         }
         self.setup_volume_daemon
             .try_execute()
@@ -213,6 +237,12 @@ impl Action for CreateNixVolume {
             .try_execute()
             .await
             .map_err(|e| ActionError::Child(self.bootstrap_volume.action_tag(), Box::new(e)))?;
+        self.kickstart_launchctl_service
+            .try_execute()
+            .await
+            .map_err(|e| {
+                ActionError::Child(self.kickstart_launchctl_service.action_tag(), Box::new(e))
+            })?;
 
         let mut retry_tokens: usize = 50;
         loop {
@@ -242,34 +272,40 @@ impl Action for CreateNixVolume {
     }
 
     fn revert_description(&self) -> Vec<ActionDescription> {
-        let Self { disk, name, .. } = &self;
+        let mut explanation = vec![
+            self.create_or_append_synthetic_conf.tracing_synopsis(),
+            self.create_synthetic_objects.tracing_synopsis(),
+            self.unmount_volume.tracing_synopsis(),
+            self.create_volume.tracing_synopsis(),
+            self.create_fstab_entry.tracing_synopsis(),
+        ];
+        if let Some(encrypt_volume) = &self.encrypt_volume {
+            explanation.push(encrypt_volume.tracing_synopsis());
+        }
+        explanation.append(&mut vec![
+            self.setup_volume_daemon.tracing_synopsis(),
+            self.bootstrap_volume.tracing_synopsis(),
+            self.enable_ownership.tracing_synopsis(),
+        ]);
+
         vec![ActionDescription::new(
-            format!("Remove the APFS volume `{name}` on `{}`", disk.display()),
+            format!(
-            vec![format!(
+                "Remove the APFS volume `{}` on `{}`",
-                "Create a writable, persistent systemd system extension.",
+                self.name,
-            )],
+                self.disk.display()
+            ),
+            explanation,
         )]
     }
 
     #[tracing::instrument(level = "debug", skip_all)]
     async fn revert(&mut self) -> Result<(), ActionError> {
-        self.enable_ownership
+        self.enable_ownership.try_revert().await?;
-            .try_revert()
+        self.kickstart_launchctl_service.try_revert().await?;
-            .await
+        self.bootstrap_volume.try_revert().await?;
-            .map_err(|e| ActionError::Child(self.enable_ownership.action_tag(), Box::new(e)))?;
+        self.setup_volume_daemon.try_revert().await?;
-        self.bootstrap_volume
-            .try_revert()
-            .await
-            .map_err(|e| ActionError::Child(self.bootstrap_volume.action_tag(), Box::new(e)))?;
-        self.setup_volume_daemon
-            .try_revert()
-            .await
-            .map_err(|e| ActionError::Child(self.setup_volume_daemon.action_tag(), Box::new(e)))?;
         if let Some(encrypt_volume) = &mut self.encrypt_volume {
-            encrypt_volume
+            encrypt_volume.try_revert().await?;
-                .try_revert()
-                .await
-                .map_err(|e| ActionError::Child(encrypt_volume.action_tag(), Box::new(e)))?;
         }
         self.create_fstab_entry
             .try_revert()

src/action/macos/enable_ownership.rs

@@ -8,7 +8,7 @@ use crate::action::{ActionError, ActionTag, StatefulAction};
 use crate::execute_command;
 
 use crate::action::{Action, ActionDescription};
-use crate::os::darwin::DiskUtilOutput;
+use crate::os::darwin::DiskUtilInfoOutput;
 
 /**
 Enable ownership on a volume
@@ -35,7 +35,7 @@ impl Action for EnableOwnership {
         ActionTag("enable_ownership")
     }
     fn tracing_synopsis(&self) -> String {
-        format!("Enable ownership on {}", self.path.display())
+        format!("Enable ownership on `{}`", self.path.display())
     }
 
     fn tracing_span(&self) -> Span {
@@ -64,7 +64,7 @@ impl Action for EnableOwnership {
             )
             .await?
             .stdout;
-            let the_plist: DiskUtilOutput = plist::from_reader(Cursor::new(buf))?;
+            let the_plist: DiskUtilInfoOutput = plist::from_reader(Cursor::new(buf))?;
 
             the_plist.global_permissions_enabled
         };

src/action/macos/encrypt_apfs_volume.rs

@@ -1,15 +1,21 @@
 use crate::{
     action::{
-        macos::NIX_VOLUME_MOUNTD_DEST, Action, ActionDescription, ActionError, ActionTag,
+        macos::NIX_VOLUME_MOUNTD_DEST, Action, ActionDescription, ActionError, ActionState,
-        StatefulAction,
+        ActionTag, StatefulAction,
     },
     execute_command,
+    os::darwin::DiskUtilApfsListOutput,
 };
 use rand::Rng;
-use std::path::{Path, PathBuf};
+use std::{
+    path::{Path, PathBuf},
+    process::Stdio,
+};
 use tokio::process::Command;
 use tracing::{span, Span};
 
+use super::CreateApfsVolume;
+
 /**
 Encrypt an APFS volume
  */
@@ -24,13 +30,73 @@ impl EncryptApfsVolume {
     pub async fn plan(
         disk: impl AsRef<Path>,
         name: impl AsRef<str>,
+        planned_create_apfs_volume: &StatefulAction<CreateApfsVolume>,
     ) -> Result<StatefulAction<Self>, ActionError> {
         let name = name.as_ref().to_owned();
-        Ok(Self {
+        let disk = disk.as_ref().to_path_buf();
-            name,
+
-            disk: disk.as_ref().to_path_buf(),
+        let mut command = Command::new("/usr/bin/security");
+        command.args(["find-generic-password", "-a"]);
+        command.arg(&name);
+        command.arg("-s");
+        command.arg("Nix Store");
+        command.arg("-l");
+        command.arg(&format!("{} encryption password", disk.display()));
+        command.arg("-D");
+        command.arg("Encrypted volume password");
+        command.process_group(0);
+        command.stdin(Stdio::null());
+        command.stdout(Stdio::null());
+        command.stderr(Stdio::null());
+        if command
+            .status()
+            .await
+            .map_err(|e| ActionError::command(&command, e))?
+            .success()
+        {
+            // The user has a password matching what we would create.
+            if planned_create_apfs_volume.state == ActionState::Completed {
+                // We detected a created volume already, and a password exists, so we can keep using that and skip doing anything
+                return Ok(StatefulAction::completed(Self { name, disk }));
+            }
+
+            // Ask the user to remove it
+            return Err(ActionError::Custom(Box::new(
+                EncryptApfsVolumeError::ExistingPasswordFound(name, disk),
+            )));
+        } else {
+            if planned_create_apfs_volume.state == ActionState::Completed {
+                // The user has a volume already created, but a password not set. This means we probably can't decrypt the volume.
+                return Err(ActionError::Custom(Box::new(
+                    EncryptApfsVolumeError::MissingPasswordForExistingVolume(name, disk),
+                )));
+            }
         }
-        .into())
+
+        // Ensure if the disk already exists, that it's encrypted
+        let output =
+            execute_command(Command::new("/usr/sbin/diskutil").args(["apfs", "list", "-plist"]))
+                .await?;
+
+        let parsed: DiskUtilApfsListOutput = plist::from_bytes(&output.stdout)?;
+        for container in parsed.containers {
+            for volume in container.volumes {
+                if volume.name == name {
+                    match volume.encryption == false {
+                        true => {
+                            return Ok(StatefulAction::completed(Self { disk, name }));
+                        },
+                        false => {
+                            return Err(ActionError::Custom(Box::new(
+                                EncryptApfsVolumeError::ExistingVolumeNotEncrypted(name, disk),
+                            )));
+                        },
+                    }
+                }
+            }
+        }
+
+        Ok(StatefulAction::uncompleted(Self { name, disk }))
     }
 }
 
@@ -93,7 +159,7 @@ impl Action for EncryptApfsVolume {
                 "-a",
                 name.as_str(),
                 "-s",
-                name.as_str(),
+                "Nix Store",
                 "-l",
                 format!("{} encryption password", disk_str).as_str(),
                 "-D",
@@ -182,3 +248,13 @@ impl Action for EncryptApfsVolume {
         Ok(())
     }
 }
+
+#[derive(thiserror::Error, Debug)]
+pub enum EncryptApfsVolumeError {
+    #[error("The keychain has an existing password for a non-existing \"{0}\" volume on disk `{1}`, consider removing the password with `security delete-generic-password  -a \"{0}\" -s \"Nix Store\" -l \"{1} encryption password\" -D \"Encrypted volume password\"`")]
+    ExistingPasswordFound(String, PathBuf),
+    #[error("The keychain lacks a password for the already existing \"{0}\" volume on disk `{1}`, consider removing the volume with `diskutil apfs deleteVolume \"{0}\"` (if you receive error -69888, you may need to run `launchctl bootout system/org.nixos.darwin-store` and `launchctl bootout system/org.nixos.nix-daemon` first)")]
+    MissingPasswordForExistingVolume(String, PathBuf),
+    #[error("The existing APFS volume \"{0}\" on disk `{1}` is not encrypted but it should be, consider removing the volume with `diskutil apfs deleteVolume \"{0}\"` (if you receive error -69888, you may need to run `launchctl bootout system/org.nixos.darwin-store` and `launchctl bootout system/org.nixos.nix-daemon` first)")]
+    ExistingVolumeNotEncrypted(String, PathBuf),
+}

src/action/macos/kickstart_launchctl_service.rs

@@ -0,0 +1,150 @@
+use std::process::Output;
+
+use tokio::process::Command;
+use tracing::{span, Span};
+
+use crate::action::{ActionError, ActionTag, StatefulAction};
+use crate::execute_command;
+
+use crate::action::{Action, ActionDescription};
+
+/**
+Bootstrap and kickstart an APFS volume
+*/
+#[derive(Debug, serde::Deserialize, serde::Serialize, Clone)]
+pub struct KickstartLaunchctlService {
+    domain: String,
+    service: String,
+}
+
+impl KickstartLaunchctlService {
+    #[tracing::instrument(level = "debug", skip_all)]
+    pub async fn plan(
+        domain: impl AsRef<str>,
+        service: impl AsRef<str>,
+    ) -> Result<StatefulAction<Self>, ActionError> {
+        let domain = domain.as_ref().to_string();
+        let service = service.as_ref().to_string();
+
+        let mut service_exists = false;
+        let mut service_started = false;
+        let mut command = Command::new("launchctl");
+        command.process_group(0);
+        command.arg("print");
+        command.arg(&service);
+        command.arg("-plist");
+        command.stdin(std::process::Stdio::null());
+        command.stdout(std::process::Stdio::piped());
+        command.stderr(std::process::Stdio::piped());
+        let output = command
+            .output()
+            .await
+            .map_err(|e| ActionError::command(&command, e))?;
+        if output.status.success() {
+            service_exists = true;
+
+            let output_string = String::from_utf8(output.stdout)?;
+            // We are looking for a line containing "state = " with some trailing content
+            // The output is not a JSON or a plist
+            // MacOS's man pages explicitly tell us not to try to parse this output
+            // MacOS's man pages explicitly tell us this output is not stable
+            // Yet, here we are, doing exactly that.
+            for output_line in output_string.lines() {
+                let output_line_trimmed = output_line.trim();
+                if output_line_trimmed.starts_with("state") {
+                    if output_line_trimmed.contains("running") {
+                        service_started = true;
+                    }
+                    break;
+                }
+            }
+        }
+
+        if service_exists && service_started {
+            return Ok(StatefulAction::completed(Self { domain, service }));
+        }
+
+        // It's safe to assume the user does not have the service started
+        Ok(StatefulAction::uncompleted(Self { domain, service }))
+    }
+}
+
+#[async_trait::async_trait]
+#[typetag::serde(name = "kickstart_launchctl_service")]
+impl Action for KickstartLaunchctlService {
+    fn action_tag() -> ActionTag {
+        ActionTag("kickstart_launchctl_service")
+    }
+    fn tracing_synopsis(&self) -> String {
+        format!("Run `launchctl kickstart {}`", self.service)
+    }
+
+    fn tracing_span(&self) -> Span {
+        span!(
+            tracing::Level::DEBUG,
+            "kickstart_launchctl_service",
+            path = %self.service,
+        )
+    }
+
+    fn execute_description(&self) -> Vec<ActionDescription> {
+        vec![ActionDescription::new(self.tracing_synopsis(), vec![])]
+    }
+
+    #[tracing::instrument(level = "debug", skip_all)]
+    async fn execute(&mut self) -> Result<(), ActionError> {
+        let Self { domain, service } = self;
+
+        execute_command(
+            Command::new("launchctl")
+                .process_group(0)
+                .args(["kickstart", "-k"])
+                .arg(format!("{domain}/{service}"))
+                .stdin(std::process::Stdio::null()),
+        )
+        .await?;
+
+        Ok(())
+    }
+
+    fn revert_description(&self) -> Vec<ActionDescription> {
+        vec![ActionDescription::new(
+            format!("Run `launchctl stop {}`", self.service),
+            vec![],
+        )]
+    }
+
+    #[tracing::instrument(level = "debug", skip_all)]
+    async fn revert(&mut self) -> Result<(), ActionError> {
+        let Self { domain, service } = self;
+
+        // MacOs doesn't offer an "ensure-stopped" like they do with Kickstart
+        let mut command = Command::new("launchctl");
+        command.process_group(0);
+        command.arg("stop");
+        command.arg(format!("{domain}/{service}"));
+        command.stdin(std::process::Stdio::null());
+        let command_str = format!("{:?}", command.as_std());
+        let output = command
+            .output()
+            .await
+            .map_err(|e| ActionError::command(&command, e))?;
+        // On our test Macs, a status code of `3` was reported if the service was stopped while not running.
+        match output.status.code() {
+            Some(3) | Some(0) | None => (),
+            _ => {
+                return Err(ActionError::Custom(Box::new(
+                    KickstartLaunchctlServiceError::CannotStopService(command_str, output),
+                )))
+            },
+        }
+
+        Ok(())
+    }
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum KickstartLaunchctlServiceError {
+    #[error("Command `{0}` failed, stderr: {}", String::from_utf8(.1.stderr.clone()).unwrap_or_else(|_e| String::from("<Non-UTF-8>")))]
+    CannotStopService(String, Output),
+}

src/action/macos/mod.rs

@@ -1,19 +1,21 @@
 /*!  [`Action`](crate::action::Action)s for Darwin based systems
 */
 
-pub(crate) mod bootstrap_apfs_volume;
+pub(crate) mod bootstrap_launchctl_service;
 pub(crate) mod create_apfs_volume;
 pub(crate) mod create_fstab_entry;
 pub(crate) mod create_nix_volume;
 pub(crate) mod create_synthetic_objects;
 pub(crate) mod enable_ownership;
 pub(crate) mod encrypt_apfs_volume;
+pub(crate) mod kickstart_launchctl_service;
 pub(crate) mod unmount_apfs_volume;
 
-pub use bootstrap_apfs_volume::{BootstrapApfsVolume, BootstrapVolumeError};
+pub use bootstrap_launchctl_service::BootstrapLaunchctlService;
-pub use create_apfs_volume::{CreateApfsVolume, CreateApfsVolumeError};
+pub use create_apfs_volume::CreateApfsVolume;
 pub use create_nix_volume::{CreateNixVolume, NIX_VOLUME_MOUNTD_DEST};
 pub use create_synthetic_objects::{CreateSyntheticObjects, CreateSyntheticObjectsError};
 pub use enable_ownership::{EnableOwnership, EnableOwnershipError};
 pub use encrypt_apfs_volume::EncryptApfsVolume;
+pub use kickstart_launchctl_service::KickstartLaunchctlService;
 pub use unmount_apfs_volume::UnmountApfsVolume;

src/action/macos/unmount_apfs_volume.rs

@@ -1,3 +1,4 @@
+use std::io::Cursor;
 use std::path::{Path, PathBuf};
 
 use tokio::process::Command;
@@ -7,6 +8,7 @@ use crate::action::{ActionError, ActionTag, StatefulAction};
 use crate::execute_command;
 
 use crate::action::{Action, ActionDescription};
+use crate::os::darwin::DiskUtilInfoOutput;
 
 /**
 Unmount an APFS volume
@@ -55,14 +57,33 @@ impl Action for UnmountApfsVolume {
     async fn execute(&mut self) -> Result<(), ActionError> {
         let Self { disk: _, name } = self;
 
-        execute_command(
+        let currently_mounted = {
-            Command::new("/usr/sbin/diskutil")
+            let buf = execute_command(
-                .process_group(0)
+                Command::new("/usr/sbin/diskutil")
-                .args(["unmount", "force"])
+                    .process_group(0)
-                .arg(name)
+                    .args(["info", "-plist"])
-                .stdin(std::process::Stdio::null()),
+                    .arg(&name)
-        )
+                    .stdin(std::process::Stdio::null()),
-        .await?;
+            )
+            .await?
+            .stdout;
+            let the_plist: DiskUtilInfoOutput = plist::from_reader(Cursor::new(buf))?;
+
+            the_plist.mount_point.is_some()
+        };
+
+        if !currently_mounted {
+            execute_command(
+                Command::new("/usr/sbin/diskutil")
+                    .process_group(0)
+                    .args(["unmount", "force"])
+                    .arg(name)
+                    .stdin(std::process::Stdio::null()),
+            )
+            .await?;
+        } else {
+            tracing::debug!("Volume was already unmounted, can skip unmounting")
+        }
 
         Ok(())
     }

src/os/darwin.rs

@@ -1,6 +1,28 @@
+use std::path::PathBuf;
+
 #[derive(serde::Deserialize)]
 #[serde(rename_all = "PascalCase")]
-pub struct DiskUtilOutput {
+pub struct DiskUtilInfoOutput {
     pub parent_whole_disk: String,
     pub global_permissions_enabled: bool,
+    pub mount_point: Option<PathBuf>,
+}
+
+#[derive(serde::Deserialize, Clone, Debug)]
+#[serde(rename_all = "PascalCase")]
+pub struct DiskUtilApfsListOutput {
+    pub containers: Vec<DiskUtilApfsContainer>,
+}
+
+#[derive(serde::Deserialize, Clone, Debug)]
+#[serde(rename_all = "PascalCase")]
+pub struct DiskUtilApfsContainer {
+    pub volumes: Vec<DiskUtilApfsListVolume>,
+}
+
+#[derive(serde::Deserialize, Clone, Debug)]
+#[serde(rename_all = "PascalCase")]
+pub struct DiskUtilApfsListVolume {
+    pub name: String,
+    pub encryption: bool,
 }

src/planner/macos.rs

@@ -12,7 +12,7 @@ use crate::{
         StatefulAction,
     },
     execute_command,
-    os::darwin::DiskUtilOutput,
+    os::darwin::DiskUtilInfoOutput,
     planner::{Planner, PlannerError},
     settings::InstallSettingsError,
     settings::{CommonSettings, InitSystem},
@@ -68,7 +68,7 @@ async fn default_root_disk() -> Result<String, PlannerError> {
     .await
     .unwrap()
     .stdout;
-    let the_plist: DiskUtilOutput = plist::from_reader(Cursor::new(buf))?;
+    let the_plist: DiskUtilInfoOutput = plist::from_reader(Cursor::new(buf))?;
 
     Ok(the_plist.parent_whole_disk)
 }
@@ -98,7 +98,7 @@ impl Planner for Macos {
                 .await
                 .unwrap()
                 .stdout;
-                let the_plist: DiskUtilOutput = plist::from_reader(Cursor::new(buf)).unwrap();
+                let the_plist: DiskUtilInfoOutput = plist::from_reader(Cursor::new(buf)).unwrap();
 
                 Some(the_plist.parent_whole_disk)
             },

tests/fixtures/macos/macos.json

@@ -40,7 +40,8 @@
         },
         "create_fstab_entry": {
           "action": {
-            "apfs_volume_label": "Nix Store"
+            "apfs_volume_label": "Nix Store",
+            "existing_entry": "None"
           },
           "state": "Uncompleted"
         },
@@ -64,10 +65,19 @@
         },
         "bootstrap_volume": {
           "action": {
+            "domain": "system",
+            "service": "org.nixos.darwin-store",
             "path": "/Library/LaunchDaemons/org.nixos.darwin-store.plist"
           },
           "state": "Uncompleted"
         },
+        "kickstart_launchctl_service": {
+          "action": {
+            "domain": "system",
+            "service": "org.nixos.darwin-store"
+          },
+          "state": "Uncompleted"
+        },
         "enable_ownership": {
           "action": {
             "path": "/nix"