diff --git a/src/action/macos/create_determinate_nix_volume.rs b/src/action/macos/create_determinate_nix_volume.rs index f25d9f570..219080662 100644 --- a/src/action/macos/create_determinate_nix_volume.rs +++ b/src/action/macos/create_determinate_nix_volume.rs @@ -86,7 +86,7 @@ impl CreateDeterminateNixVolume { .map_err(Self::error)? }; - let create_fstab_entry = CreateFstabEntry::plan(name.clone(), &create_volume) + let create_fstab_entry = CreateFstabEntry::plan(name.clone()) .await .map_err(Self::error)?; diff --git a/src/action/macos/create_fstab_entry.rs b/src/action/macos/create_fstab_entry.rs index b44fae694..16dae4de3 100644 --- a/src/action/macos/create_fstab_entry.rs +++ b/src/action/macos/create_fstab_entry.rs @@ -1,27 +1,15 @@ +use std::path::Path; + +use tracing::{span, Span}; use uuid::Uuid; -use super::{get_disk_info_for_label, CreateApfsVolume}; +use super::get_disk_info_for_label; use crate::action::{ - Action, ActionDescription, ActionError, ActionErrorKind, ActionState, ActionTag, StatefulAction, + Action, ActionDescription, ActionError, ActionErrorKind, ActionTag, StatefulAction, }; -use std::{io::SeekFrom, path::Path}; -use tokio::{ - fs::OpenOptions, - io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt}, -}; -use tracing::{span, Span}; const FSTAB_PATH: &str = "/etc/fstab"; -#[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 @@ -33,52 +21,12 @@ add the relevant information to `/etc/fstab`. #[serde(tag = "action_name", rename = "create_fstab_entry")] 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, - planned_create_apfs_volume: &StatefulAction, - ) -> Result, 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| Self::error(ActionErrorKind::Read(fstab_path.to_path_buf(), e)))?; - let prelude_comment = fstab_prelude_comment(&apfs_volume_label); - - // 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) { - if planned_create_apfs_volume.state != ActionState::Completed { - 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(StatefulAction::uncompleted(Self { - apfs_volume_label, - existing_entry: ExistingFstabEntry::None, - })) + pub async fn plan(apfs_volume_label: String) -> Result, ActionError> { + Ok(StatefulAction::uncompleted(Self { apfs_volume_label })) } } @@ -89,16 +37,10 @@ impl Action for CreateFstabEntry { ActionTag("create_fstab_entry") } fn tracing_synopsis(&self) -> String { - match self.existing_entry { - ExistingFstabEntry::NixInstallerEntry | ExistingFstabEntry::Foreign => format!( - "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 - ), - } + format!( + "Update `{FSTAB_PATH}` to mount the APFS volume `{}`", + self.apfs_volume_label + ) } fn tracing_span(&self) -> Span { @@ -106,7 +48,6 @@ impl Action for CreateFstabEntry { tracing::Level::DEBUG, "create_fstab_entry", apfs_volume_label = self.apfs_volume_label, - existing_entry = ?self.existing_entry, ); span @@ -118,112 +59,68 @@ impl Action for CreateFstabEntry { #[tracing::instrument(level = "debug", skip_all)] async fn execute(&mut self) -> Result<(), ActionError> { - let Self { - apfs_volume_label, - existing_entry, - } = self; let fstab_path = Path::new(FSTAB_PATH); - let uuid = match get_disk_info_for_label(apfs_volume_label) + let uuid = match get_disk_info_for_label(&self.apfs_volume_label) .await .map_err(Self::error)? { Some(diskutil_info) => diskutil_info.volume_uuid, None => { return Err(Self::error(CreateFstabEntryError::CannotDetermineUuid( - apfs_volume_label.clone(), + self.apfs_volume_label.clone(), )))? }, }; - let mut fstab = tokio::fs::OpenOptions::new() - .create(true) - .truncate(false) - .write(true) - .read(true) - .open(fstab_path) - .await - .map_err(|e| Self::error(ActionErrorKind::Open(fstab_path.to_path_buf(), e)))?; - - // Make sure it doesn't already exist before we write to it. - let mut fstab_buf = String::new(); - fstab - .read_to_string(&mut fstab_buf) + let fstab_buf = tokio::fs::read_to_string(FSTAB_PATH) .await + .or_else(|e| match e.kind() { + std::io::ErrorKind::NotFound => Ok(String::new()), + _ => Err(e), + }) .map_err(|e| Self::error(ActionErrorKind::Read(fstab_path.to_owned(), e)))?; - let updated_buf = match existing_entry { - ExistingFstabEntry::NixInstallerEntry => { - // Update the entry - let mut current_fstab_lines = fstab_buf - .lines() - .map(|v| v.to_owned()) - .collect::>(); - 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(Self::error( - CreateFstabEntryError::ExistingNixInstallerEntryDisappeared, - )); + let mut line_present = false; + let mut current_fstab_lines = fstab_buf + .lines() + .filter_map(|line| { + // Delete nix-installer entries with a "prelude" comment + if line.starts_with("# nix-installer created volume labelled") { + None + } else { + Some(line) } - 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::>(); - 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(Self::error( - CreateFstabEntryError::ExistingForeignEntryDisappeared, - )); + }) + .map(|line| { + if line.split(&[' ', '\t']).nth(1) == Some("/nix") { + // Replace the existing line with an updated version + line_present = true; + fstab_entry(&uuid) + } else { + line.to_owned() } - current_fstab_lines.join("\n") - }, - ExistingFstabEntry::None => fstab_buf + "\n" + &fstab_lines(&uuid, apfs_volume_label), - }; + }) + .collect::>(); - fstab - .seek(SeekFrom::Start(0)) - .await - .map_err(|e| Self::error(ActionErrorKind::Seek(fstab_path.to_owned(), e)))?; - fstab - .set_len(0) - .await - .map_err(|e| Self::error(ActionErrorKind::Truncate(fstab_path.to_owned(), e)))?; - fstab - .write_all(updated_buf.as_bytes()) - .await - .map_err(|e| Self::error(ActionErrorKind::Write(fstab_path.to_owned(), e)))?; + if !line_present { + current_fstab_lines.push(fstab_entry(&uuid)) + } + + if current_fstab_lines.last().map(|s| s.as_ref()) != Some("") { + // Don't leave the file without a trailing newline + current_fstab_lines.push("".into()); + } + + let updated_buf = current_fstab_lines.join("\n"); + write_atomic(fstab_path, &updated_buf) + .await + .map_err(Self::error)?; Ok(()) } fn revert_description(&self) -> Vec { - let Self { - apfs_volume_label, - existing_entry: _, - } = &self; + let Self { apfs_volume_label } = &self; vec![ActionDescription::new( format!( "Remove the UUID based entry for the APFS volume `{}` in `/etc/fstab`", @@ -237,75 +134,56 @@ impl Action for CreateFstabEntry { async fn revert(&mut self) -> Result<(), ActionError> { let fstab_path = Path::new(FSTAB_PATH); - if let Some(diskutil_info) = get_disk_info_for_label(&self.apfs_volume_label) + let fstab_buf = tokio::fs::read_to_string(FSTAB_PATH) .await - .map_err(Self::error)? - { - let fstab_entry = fstab_lines(&diskutil_info.volume_uuid, &self.apfs_volume_label); - - let mut file = OpenOptions::new() - .create(false) - .write(true) - .read(true) - .open(&fstab_path) - .await - .map_err(|e| Self::error(ActionErrorKind::Open(fstab_path.to_owned(), e)))?; - - let mut file_contents = String::default(); - file.read_to_string(&mut file_contents) - .await - .map_err(|e| Self::error(ActionErrorKind::Read(fstab_path.to_owned(), e)))?; + .or_else(|e| match e.kind() { + std::io::ErrorKind::NotFound => Ok(String::new()), + _ => Err(e), + }) + .map_err(|e| Self::error(ActionErrorKind::Read(fstab_path.to_owned(), e)))?; - if let Some(start) = file_contents.rfind(fstab_entry.as_str()) { - let end = start + fstab_entry.len(); - file_contents.replace_range(start..end, "") - } + let mut current_fstab_lines = fstab_buf + .lines() + .filter_map(|line| { + // Delete nix-installer entries with a "prelude" comment + if line.starts_with("# nix-installer created volume labelled") { + None + } else { + Some(line) + } + }) + .filter_map(|line| { + if line.split(&[' ', '\t']).nth(1) == Some("/nix") { + // Delete the mount line for /nix + None + } else { + Some(line) + } + }) + .collect::>(); - file.seek(SeekFrom::Start(0)) - .await - .map_err(|e| Self::error(ActionErrorKind::Seek(fstab_path.to_owned(), e)))?; - file.set_len(0) - .await - .map_err(|e| Self::error(ActionErrorKind::Truncate(fstab_path.to_owned(), e)))?; - file.write_all(file_contents.as_bytes()) - .await - .map_err(|e| Self::error(ActionErrorKind::Write(fstab_path.to_owned(), e)))?; - file.flush() - .await - .map_err(|e| Self::error(ActionErrorKind::Flush(fstab_path.to_owned(), e)))?; - } else { - return Err(Self::error(CreateFstabEntryError::CannotDetermineFstabLine)); + if current_fstab_lines.last() != Some(&"") { + // Don't leave the file without a trailing newline + current_fstab_lines.push(""); } + write_atomic(fstab_path, ¤t_fstab_lines.join("\n")) + .await + .map_err(Self::error)?; + Ok(()) } } -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) -> String { - format!("UUID={uuid} /nix apfs rw,noauto,nobrowse,suid,owners") + format!("UUID={uuid} /nix apfs rw,noatime,noauto,nobrowse,nosuid,owners # Added by the Determinate Nix Installer") } #[non_exhaustive] #[derive(thiserror::Error, Debug)] pub enum CreateFstabEntryError { - #[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")] - ExistingNixInstallerEntryDisappeared, - #[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")] - ExistingForeignEntryDisappeared, #[error("Unable to determine how to add APFS volume `{0}` the `/etc/fstab` line, likely the volume is not yet created or there is some synchronization issue, please report this")] CannotDetermineUuid(String), - #[error("Unable to reliably determine which `/etc/fstab` line to remove, the volume is likely already deleted, the line involving `/nix` in `/etc/fstab` should be removed manually")] - CannotDetermineFstabLine, } impl From for ActionErrorKind { @@ -313,3 +191,17 @@ impl From for ActionErrorKind { ActionErrorKind::Custom(Box::new(val)) } } + +async fn write_atomic(destination: &Path, body: &str) -> Result<(), ActionErrorKind> { + let temp = destination.with_extension("tmp"); + + tokio::fs::write(&temp, body) + .await + .map_err(|e| ActionErrorKind::Write(temp.to_owned(), e))?; + + tokio::fs::rename(&temp, &destination) + .await + .map_err(|e| ActionErrorKind::Rename(temp, destination.into(), e))?; + + Ok(()) +} diff --git a/src/action/macos/create_nix_volume.rs b/src/action/macos/create_nix_volume.rs index ce4eb8c40..480dfe622 100644 --- a/src/action/macos/create_nix_volume.rs +++ b/src/action/macos/create_nix_volume.rs @@ -76,7 +76,7 @@ impl CreateNixVolume { .map_err(Self::error)? }; - let create_fstab_entry = CreateFstabEntry::plan(name.clone(), &create_volume) + let create_fstab_entry = CreateFstabEntry::plan(name.clone()) .await .map_err(Self::error)?; diff --git a/src/lib.rs b/src/lib.rs index 0db472c27..2fca40d3a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,6 @@ +// .filter_map() predicates returns Some/None, which is more clear than .filter()'s -> bool predicates. +#![allow(clippy::unnecessary_filter_map)] + /*! The Determinate [Nix](https://github.com/NixOS/nix) Installer `nix-installer` breaks down into three main concepts: