From 05fb4bc3d2bfc501f5300f84d953178827af2544 Mon Sep 17 00:00:00 2001 From: picnoir Date: Fri, 6 Mar 2026 18:26:30 +0100 Subject: [PATCH 01/18] etc files activation: refactor This is pretty much a full refactor of the etc files activation system. This refactor is revolving around 2 core ideas: 1. Make the actual activation in two phase. A phase where we list all the static files contained in the various derivations. A second phase where we merge this list to the files explicitely mentionned in the configuration phase and create them. 2. Simplify the state data type. Giving up the recursive data structure and instead adopt a flat set of file paths. All the integration tests are now green, however, we should cover the state migration in a subsequent commit before merging this to main. --- crates/system-manager-engine/src/activate.rs | 24 +- .../src/activate/etc_files.rs | 706 ++++++++---------- .../src/activate/etc_files/etc_tree.rs | 571 -------------- .../src/activate/tmp_files.rs | 32 +- crates/system-manager-engine/src/lib.rs | 6 - nix/lib.nix | 2 +- 6 files changed, 335 insertions(+), 1006 deletions(-) delete mode 100644 crates/system-manager-engine/src/activate/etc_files/etc_tree.rs diff --git a/crates/system-manager-engine/src/activate.rs b/crates/system-manager-engine/src/activate.rs index fa59c32d..7d20193e 100644 --- a/crates/system-manager-engine/src/activate.rs +++ b/crates/system-manager-engine/src/activate.rs @@ -5,12 +5,12 @@ pub(crate) mod users; use anyhow::Result; use serde::{Deserialize, Serialize}; +use std::collections::HashSet; use std::fs::DirBuilder; use std::path::{Path, PathBuf}; use std::{fs, io, process}; use thiserror::Error; -use crate::activate::etc_files::FileTree; use crate::{StorePath, STATE_FILE_NAME, SYSTEM_MANAGER_STATE_DIR}; #[derive(Error, Debug)] @@ -33,10 +33,26 @@ impl ActivationError { pub type ActivationResult = Result>; +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum FileStatus { + Managed, + ManagedWithBackup, +} + +type EtcTree = HashSet; +type BackedUpFiles = HashSet; +#[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct EtcFilesState { + pub files: EtcTree, + pub backed_up_files: BackedUpFiles, +} + #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct State { - pub(crate) file_tree: FileTree, + pub(crate) file_tree: EtcFilesState, pub(crate) services: services::Services, } @@ -57,6 +73,7 @@ impl State { pub fn write_to_file(&self, state_file: &Path) -> Result<()> { log::info!("Writing state info into file: {}", state_file.display()); + log::debug!("State: {:?}", self); let writer = io::BufWriter::new(fs::File::create(state_file)?); serde_json::to_writer(writer, self)?; @@ -91,7 +108,7 @@ pub fn activate(store_path: &StorePath, ephemeral: bool) -> Result<()> { } log::info!("Activating tmp files..."); - let tmp_result = tmp_files::activate(&etc_tree); + let tmp_result = tmp_files::activate(&etc_tree.files); if let Err(e) = &tmp_result { log::error!("Error during activation of tmp files"); log::error!("{e}"); @@ -123,6 +140,7 @@ pub fn activate(store_path: &StorePath, ephemeral: bool) -> Result<()> { } Err(ActivationError::WithPartialResult { result, source }) => { log::error!("Error during activation: {source:?}"); + log::debug!("Resulting file tree: {:?}", result); let final_state = State { file_tree: result, ..old_state diff --git a/crates/system-manager-engine/src/activate/etc_files.rs b/crates/system-manager-engine/src/activate/etc_files.rs index 9edc1a80..0ea4df32 100644 --- a/crates/system-manager-engine/src/activate/etc_files.rs +++ b/crates/system-manager-engine/src/activate/etc_files.rs @@ -1,29 +1,20 @@ -mod etc_tree; - -use anyhow::anyhow; +use anyhow::{anyhow, Context}; use im::HashMap; -use itertools::Itertools; use regex; use serde::{Deserialize, Serialize}; -use std::fs::{DirBuilder, Permissions}; -use std::os::unix::fs as unixfs; +use std::collections::HashSet; +use std::fs::Permissions; use std::os::unix::prelude::PermissionsExt; -use std::path; +use std::os::unix::{self, fs as unixfs}; use std::path::{Path, PathBuf}; use std::sync::OnceLock; use std::{fs, io}; -use self::etc_tree::FileStatus; use super::ActivationResult; -use crate::activate::ActivationError; -use crate::{ - create_link, create_store_link, etc_dir, remove_dir, remove_file, remove_link, StorePath, - SYSTEM_MANAGER_STATIC_NAME, -}; - -pub use etc_tree::FileTree; +use crate::activate::{ActivationError, EtcFilesState}; +use crate::{etc_dir, remove_file, remove_link, StorePath}; -type EtcActivationResult = ActivationResult; +type EtcActivationResult = ActivationResult; static UID_GID_REGEX: OnceLock = OnceLock::new(); @@ -31,7 +22,7 @@ fn get_uid_gid_regex() -> &'static regex::Regex { UID_GID_REGEX.get_or_init(|| regex::Regex::new(r"^\+[0-9]+$").expect("could not compile regex")) } -const BACKUP_SUFFIX: &str = ".system-manager-backup"; +const BACKUP_SUFFIX: &str = "system-manager-backup"; #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -59,12 +50,12 @@ struct EtcFilesConfig { impl std::fmt::Display for EtcFilesConfig { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let out: String = itertools::intersperse( - self.entries.values().map(|entry| { + self.entries.iter().map(|entry| { format!( "target: {}, source:{}, mode:{}", - entry.target.display(), - entry.source, - entry.mode + entry.1.target.display(), + entry.1.source, + entry.1.mode ) }), "\n".to_owned(), @@ -87,9 +78,19 @@ fn read_config(store_path: &StorePath) -> anyhow::Result { Ok(config) } +/// Etc files activation +/// +/// The activation is performed in two main steps: +/// +/// 1. We list all the entries living in the nix-generated static env. These files +/// are generated by some Nix derivation, there is no way for them to appear in the +/// eval-time generated configuration. +/// 2. After merging the "static" entries listed in the previous step with the "copy" ones +/// coming from the system-manager state, we create all these files on the disk, backing +/// up the conflicts if necessary. pub fn activate( store_path: &StorePath, - old_state: FileTree, + old_state: EtcFilesState, ephemeral: bool, ) -> EtcActivationResult { let config = read_config(store_path) @@ -98,35 +99,53 @@ pub fn activate( let etc_dir = etc_dir(ephemeral); log::info!("Creating /etc entries in {}", etc_dir.display()); - let initial_state = FileTree::root_node(); - - let state = create_etc_static_link( - SYSTEM_MANAGER_STATIC_NAME, - &config.static_env, - &etc_dir, - initial_state, - )?; - - // Create the rest of the links - let final_state = create_etc_links(config.entries.values(), &etc_dir, state, &old_state) - .update_state(old_state, &try_delete_path) - .unwrap_or_default(); + let mut new_state = EtcFilesState::default(); - log::info!("Done"); - Ok(final_state) + // Walk through static link, list entries + let mut entries = match list_static_entries(&config) { + Ok(e) => e, + Err(e) => { + return Err(ActivationError::WithPartialResult { + result: new_state.clone(), + source: e, + }) + } + }; + let mut non_static_entries: Vec = config + .entries + .values() + .filter(|v| v.mode != "symlink") + .cloned() + .map(|mut v| { + v.source.store_path = v.source.store_path.join(&v.target); + v + }) + .collect(); + entries.append(&mut non_static_entries); + // Create dirs and link/copy entries + new_state = create_etc_files(entries, new_state.clone(), &old_state)?; + // Delete unecessary files + let files_to_delete: HashSet = old_state + .files + .difference(&new_state.files) + .map(|f| f.to_owned()) + .collect(); + new_state = delete_paths(&files_to_delete, new_state); + Ok(new_state) } -pub fn deactivate(old_state: FileTree) -> EtcActivationResult { - let final_state = old_state.deactivate(&try_delete_path).unwrap_or_default(); +pub fn deactivate(old_state: EtcFilesState) -> EtcActivationResult { + let files = old_state.files.clone(); + let final_state = delete_paths(&files, old_state); log::info!("Done"); Ok(final_state) } fn backup_path_for(path: &Path) -> PathBuf { - let mut s = path.as_os_str().to_owned(); - s.push(BACKUP_SUFFIX); - PathBuf::from(s) + let mut s = path.to_owned(); + s.add_extension(BACKUP_SUFFIX); + s } fn backup_existing_file(path: &Path) -> anyhow::Result<()> { @@ -158,388 +177,229 @@ fn restore_backup(path: &Path) -> anyhow::Result<()> { Ok(()) } -fn try_delete_path(path: &Path, status: &FileStatus) -> bool { - fn do_try_delete(path: &Path, status: &FileStatus) -> anyhow::Result<()> { - // exists() returns false for broken symlinks - if path.exists() || path.is_symlink() { +fn delete_paths(paths: &HashSet, mut state: EtcFilesState) -> EtcFilesState { + for path in paths { + if path.exists() { if path.is_symlink() { - remove_link(path)?; + let _ = remove_link(path).map(|_| state.files.remove(path)); } else if path.is_file() { - remove_file(path)?; - } else if path.is_dir() { - if path.read_dir()?.next().is_none() { - remove_dir(path)?; - } else { - if matches!(status, FileStatus::Managed | FileStatus::ManagedWithBackup) { - log::warn!("Managed directory not empty, ignoring: {}", path.display()); - } - return Ok(()); - } + let _ = remove_file(path).map(|_| state.files.remove(path)); } else { - anyhow::bail!("Unsupported file type! {}", path.display()) + log::warn!( + "Trying to delete {}, but it seems to be a directory", + path.display() + ); } } - - if *status == FileStatus::ManagedWithBackup { - restore_backup(path)?; + if state.backed_up_files.contains(path) { + let _ = restore_backup(path).map(|_| state.backed_up_files.remove(path)); } - - Ok(()) } - - log::debug!("Deactivating: {}", path.display()); - do_try_delete(path, status) - .map_err(|e| { - log::error!("Error deleting path: {}", path.display()); - log::error!("{e}"); - e - }) - .is_ok() + state } -fn create_etc_links<'a, E>( - entries: E, - etc_dir: &Path, - state: FileTree, - old_state: &FileTree, -) -> FileTree -where - E: Iterator, -{ - entries.fold(state, |state, entry| { - let new_state = create_etc_entry(entry, etc_dir, state, old_state); - match new_state { - Ok(new_state) => new_state, - Err(ActivationError::WithPartialResult { result, source }) => { - log::error!( - "Error while creating file in {}: {source:?}", - etc_dir.display() +/// List all the files contained in `config_entries` in a DFS fashion. +fn list_static_entries(config_entries: &EtcFilesConfig) -> anyhow::Result> { + let mut files = Vec::new(); + + /// Helper data structure used to keep track of the relative path + /// from the `static_env` directory. + #[derive(Clone)] + struct DirToVisit { + absolute_path: PathBuf, + path_from_root: PathBuf, + } + + let mut dirs_to_visit: Vec = vec![DirToVisit { + absolute_path: config_entries.static_env.store_path.clone(), + path_from_root: PathBuf::from(""), + }]; + let mut i = 0; + + while i < dirs_to_visit.len() { + let dir = dirs_to_visit + .get(i) + .context("ERROR: index error in dir loop")? + .clone(); + let dir_content = fs::read_dir(&dir.absolute_path)?; + for file in dir_content { + let file = file?; + let canon_path = fs::canonicalize(file.path()).context(format!( + "Failed to get the canonical path of {}", + file.path().display() + ))?; + if canon_path.is_dir() { + log::debug!("{} is a dir", canon_path.display()); + let dirname = file.file_name(); + let mut path_from_root = dir.path_from_root.clone(); + path_from_root.push(dirname); + dirs_to_visit.push(DirToVisit { + absolute_path: canon_path, + path_from_root, + }); + } else { + log::debug!("{} is a file", file.path().display()); + let target = dir.path_from_root.clone().join(file.file_name()); + // Is this file entry available in the config? If so, inherit replace_existing. + let replace_existing = config_entries + .entries + .iter() + .find(|e| e.1.target == target) + .map(|e| e.1.replace_existing) + .unwrap_or(false); + let etc_file = EtcFile { + source: StorePath { + store_path: canon_path, + }, + target: PathBuf::from("/etc").join(target), + uid: 0, + gid: 0, + group: "".to_string(), + user: "".to_string(), + mode: "symlink".to_string(), + replace_existing, + }; + log::debug!( + "add file: {:?}, path_from_root: {:?}, absolute_path: {:?}", + etc_file, + dir.path_from_root, + dir.absolute_path ); - result + files.push(etc_file); } } - }) + i += 1; + } + Ok(files) } -fn create_etc_static_link( - static_dir_name: &str, - store_path: &StorePath, - etc_dir: &Path, - state: FileTree, +/// Create a single etc file. +/// +/// We separated this from `create_etc_files` to catch any error on a file boundary +/// to make sure failing to link a file do not cancel the whole etc activation. +fn create_etc_file( + file: EtcFile, + mut state: EtcFilesState, + old_state: &EtcFilesState, ) -> EtcActivationResult { - let static_path = etc_dir.join(static_dir_name); - let new_state = create_dir_recursively(static_path.parent().unwrap(), state); - new_state.and_then(|new_state| { - create_store_link(store_path, &static_path).map_or_else( - |e| Err(ActivationError::with_partial_result(new_state.clone(), e)), - |_| Ok(new_state.clone().register_managed_entry(&static_path)), - ) - }) -} - -fn create_etc_link

( - link_target: &P, - etc_dir: &Path, - state: FileTree, - old_state: &FileTree, - replace_existing: bool, -) -> EtcActivationResult -where - P: AsRef, -{ - fn link_dir_contents( - link_target: &Path, - absolute_target: &Path, - etc_dir: &Path, - state: FileTree, - old_state: &FileTree, - upwards_path: &Path, - replace_existing: bool, - ) -> EtcActivationResult { - let link_path = etc_dir.join(link_target); - // Create the dir if it doesn't exist yet - let dir_state = if !link_path.exists() { - create_dir_recursively(&link_path, state)? - } else { - state - }; - log::debug!("Entering into directory {}...", link_path.display()); - Ok(absolute_target - .read_dir() - .expect("Error reading the directory.") - .fold(dir_state, |state, entry| { - let new_state = go( - &link_target.join( - entry - .expect("Error reading the directory entry.") - .file_name(), - ), - etc_dir, - state, - old_state, - &upwards_path.join(".."), - replace_existing, + let target = PathBuf::from("/etc").join(&file.target); + log::debug!( + "Creating {} to {} ({})", + file.source, + target.display(), + file.target.display() + ); + // Create all dirs + log::debug!("Creating all dirs up to {:?}", target.parent()); + target.parent().map(fs::create_dir_all); + + // We want to override all the Ubuntu systemd .wants and .requires entries. + // We did not find a proper way to do that from the Nix static env, + // hardcoding this condition in the activation instead. + let target_is_in_systemd_dir = is_inside_systemd_dependency_dir(&target); + if file.mode == "symlink" { + // On some symlinks, target.exists() returns false. Not sure why. + let exists = target.exists() || target.is_symlink(); + if exists { + // If the target exists and has been created by a previous system-manager activation, + // replace it. + if old_state.files.contains(&target) { + log::debug!( + "{} is managed by system-manager. Deleting.", + &target.display() ); - match new_state { - Ok(new_state) => new_state, - Err(ActivationError::WithPartialResult { result, source }) => { - log::error!( - "Error while trying to link directory {}: {source:?}", - absolute_target.display() - ); - result + fs::remove_file(&target).map_err(|e| ActivationError::WithPartialResult { + result: state.clone(), + source: e.into(), + })?; + log::debug!("{} is managed by system-manager.", &target.display()); + unix::fs::symlink(file.source.store_path, &target).map_err(|e| { + ActivationError::WithPartialResult { + result: state.clone(), + source: e.into(), } - } - })) - } - - // Some versions of systemd ignore .wants and .requires directories when they are symlinks. - // We therefore create them as actual directories and link their contents instead. - fn is_systemd_dependency_dir(path: &Path) -> bool { - path.is_dir() - && path - .parent() - .map(|p| p.ends_with("systemd/system")) - .unwrap_or(false) - && path - .extension() - .filter(|ext| ["wants", "requires"].iter().any(|other| other == ext)) - .is_some() - } - - /// Check whether link_path is inside a systemd .wants or .requires directory. - fn is_inside_systemd_dependency_dir(link_path: &Path) -> bool { - link_path - .parent() - .map(|parent| { - parent - .extension() - .filter(|ext| ["wants", "requires"].iter().any(|other| other == ext)) - .is_some() - && parent - .parent() - .map(|pp| pp.ends_with("systemd/system")) - .unwrap_or(false) - }) - .unwrap_or(false) - } - - fn backup_and_link( - target: &Path, - link_path: &Path, - dir_state: FileTree, - ) -> EtcActivationResult { - backup_existing_file(link_path) - .map_err(|e| ActivationError::with_partial_result(dir_state.clone(), e))?; - create_link(target, link_path) - .map_err(|e| ActivationError::with_partial_result(dir_state.clone(), e))?; - Ok(dir_state.register_backed_up_entry(link_path)) - } - - fn go( - link_target: &Path, - etc_dir: &Path, - state: FileTree, - old_state: &FileTree, - upwards_path: &Path, - replace_existing: bool, - ) -> EtcActivationResult { - let link_path = etc_dir.join(link_target); - let mut dir_state = create_dir_recursively(link_path.parent().unwrap(), state)?; - let target = upwards_path - .join(SYSTEM_MANAGER_STATIC_NAME) - .join(link_target); - let absolute_target = etc_dir.join(SYSTEM_MANAGER_STATIC_NAME).join(link_target); - log::debug!( - "GO iteration on entry {} from {}", - link_path.display(), - absolute_target.display() - ); - // The target is a directory. Let's create the directory to /etc. - // Note: if we were not doing that, we'd end up linking the whole directory to SYSTEM_MANAGER_STATIC_NAME (itself linked in the nix-store) and we'd end up trying creating the children on themselves via the symlink indirection. - if absolute_target.is_dir() { - if !link_path.exists() { - if let Err(err) = fs::create_dir(&link_path) { - return Err(ActivationError::with_partial_result( - dir_state, - anyhow!( - "Cannot create dir {}: {}, ignoring...", - link_path.display(), - err - ), - )); - } - } - dir_state = dir_state.register_managed_entry(&link_path); - }; - // The link is a directory or a systemd dependency. Recurse into it and link its content. - if (link_path.exists() && link_path.is_dir()) || is_systemd_dependency_dir(&absolute_target) - { - if absolute_target.is_dir() { - // Auto-replace inside .wants/.requires directories - let effective_replace = - replace_existing || is_systemd_dependency_dir(&absolute_target); - link_dir_contents( - link_target, - &absolute_target, - etc_dir, - dir_state, - old_state, - upwards_path, - effective_replace, - ) - } else if replace_existing || is_inside_systemd_dependency_dir(&link_path) { - backup_and_link(&target, &link_path, dir_state) - } else { - Err(ActivationError::with_partial_result( - dir_state, - anyhow::anyhow!( - "Unmanaged file or directory {} already exists, ignoring...", - link_path.display() - ), - )) - } - // The link is a symlink and is up to date. No-op. - } else if link_path.is_symlink() - && link_path.read_link().expect("Error reading link.") == target - { - log::debug!("Link {} up to date.", link_path.display()); - Ok(dir_state.register_managed_entry(&link_path)) - // The link exists but is not currently managed by system-manager. - // Override it if it's a systemd dependency or if we're in a replace mode. - } else if (link_path.exists() || link_path.is_symlink()) - && !old_state.is_managed(&link_path) - { - if replace_existing || is_inside_systemd_dependency_dir(&link_path) { - backup_and_link(&target, &link_path, dir_state) + })?; + state.files.insert(target); + } else if file.replace_existing || target_is_in_systemd_dir { + log::debug!( + "{} already exists but it's set to replace. Backup and link again.", + file.source + ); + state = backup_and_link(&target, &file.source.store_path, state)?; } else { - Err(ActivationError::with_partial_result( - dir_state, - anyhow::anyhow!("Unmanaged path already exists in filesystem, please remove it and run system-manager again: {}", - link_path.display()), - )) + log::warn!( + "{} already exists. Set replaceExisting if you're willing to override it.", + target.display() + ); } - // There's no directory to create or recurse on. Let's try to create the symlink and potentially remove the old one. } else { - let result = if link_path.exists() || link_path.is_symlink() { - fs::remove_file(&link_path) - .map_err(|e| ActivationError::with_partial_result(dir_state.clone(), e)) - } else { - Ok(()) - }; - - match result.and_then(|_| { - create_link(&target, &link_path) - .map_err(|e| ActivationError::with_partial_result(dir_state.clone(), e)) - }) { - Ok(_) => Ok(dir_state.register_managed_entry(&link_path)), - Err(e) => Err(e), - } + log::debug!("Symlink {} => {}", file.source, target.display()); + unix::fs::symlink(file.source.store_path, &target).map_err(|e| { + ActivationError::WithPartialResult { + result: state.clone(), + source: e.into(), + } + })?; + state.files.insert(target); } + } else { + log::debug!("{} is a regular file", file.source); + state = copy_file(&file.source.store_path, &target, &file, old_state, state)?; } - - go( - link_target.as_ref(), - etc_dir, - state, - old_state, - Path::new("."), - replace_existing, - ) + Ok(state) } -fn create_etc_entry( - entry: &EtcFile, - etc_dir: &Path, - state: FileTree, - old_state: &FileTree, +fn create_etc_files( + mut files: Vec, + mut state: EtcFilesState, + old_state: &EtcFilesState, ) -> EtcActivationResult { - if entry.mode == "symlink" { - if let Some(path::Component::Normal(link_target)) = entry.target.components().next() { - create_etc_link( - &link_target, - etc_dir, - state, - old_state, - entry.replace_existing, - ) - } else { - Err(ActivationError::with_partial_result( - state, - anyhow::anyhow!("Cannot create link: {}", entry.target.display()), - )) - } - } else { - let target_path = etc_dir.join(&entry.target); - let new_state = create_dir_recursively(target_path.parent().unwrap(), state)?; - match copy_file( - &entry.source.store_path.join(&entry.target), - &target_path, - entry, - old_state, - ) { - Ok(backed_up) => { - let register = if backed_up { - FileTree::register_backed_up_entry - } else { - FileTree::register_managed_entry - }; - Ok(register(new_state, &target_path)) + files.sort_by(|a, b| a.target.cmp(&b.target)); + for file in files { + let target = file.target.clone(); + state = match create_etc_file(file, state, old_state) { + Ok(state) => state, + Err(ActivationError::WithPartialResult { result, source }) => { + log::warn!("Can't link/copy {} to : {}", target.display(), source); + result } - Err(e) => Err(ActivationError::with_partial_result(new_state, e)), } } + Ok(state) } -fn create_dir_recursively(dir: &Path, state: FileTree) -> EtcActivationResult { - use itertools::FoldWhile::{Continue, Done}; - use path::Component; - log::debug!("create_dir_recursively: {}", dir.display()); - let dirbuilder = DirBuilder::new(); - let (new_state, _) = dir - .components() - .fold_while( - (Ok(state), PathBuf::from(path::MAIN_SEPARATOR_STR)), - |(state, path), component| match (state, component) { - (Ok(state), Component::RootDir) => Continue((Ok(state), path)), - (Ok(state), Component::Normal(dir)) => { - let new_path = path.join(dir); - if !new_path.exists() { - log::debug!("Creating path: {}", new_path.display()); - match dirbuilder.create(&new_path) { - Ok(_) => { - let new_state = state.register_managed_entry(&new_path); - Continue((Ok(new_state), new_path)) - } - Err(e) => Done(( - Err(ActivationError::with_partial_result( - state, - anyhow::anyhow!(e).context(format!( - "Error creating directory {}", - new_path.display() - )), - )), - path, - )), - } - } else { - Continue((Ok(state), new_path)) - } - } - (Ok(state), otherwise) => Done(( - Err(ActivationError::with_partial_result( - state, - anyhow::anyhow!("Unexpected path component encountered: {:?}", otherwise), - )), - path, - )), - (Err(e), _) => { - panic!("Something went horribly wrong! We should not get here: {e:?}.") - } - }, - ) - .into_inner(); - new_state +fn backup_and_link( + target: &Path, + link_path: &Path, + mut dir_state: EtcFilesState, +) -> EtcActivationResult { + backup_existing_file(target) + .map_err(|e| ActivationError::with_partial_result(dir_state.clone(), e))?; + log::debug!("Symlink {} => {}", link_path.display(), target.display()); + unix::fs::symlink(link_path, target).map_err(|e| ActivationError::WithPartialResult { + result: dir_state.clone(), + source: e.into(), + })?; + dir_state.backed_up_files.insert(target.to_owned()); + dir_state.files.insert(target.to_owned()); + Ok(dir_state) +} + +/// Check whether link_path is inside a systemd .wants or .requires directory. +fn is_inside_systemd_dependency_dir(link_path: &Path) -> bool { + link_path + .parent() + .map(|parent| { + parent + .extension() + .filter(|ext| ["wants", "requires"].iter().any(|other| other == ext)) + .is_some() + && parent + .parent() + .map(|pp| pp.ends_with("systemd/system")) + .unwrap_or(false) + }) + .unwrap_or(false) } fn find_uid(entry: &EtcFile) -> anyhow::Result { @@ -584,33 +444,57 @@ fn find_gid(entry: &EtcFile) -> anyhow::Result { } } -/// Copy a file from source to target. Returns `Ok(true)` if a pre-existing -/// file was backed up, `Ok(false)` if no backup was needed. +/// Copy a file from source to target. +/// Failing to copy a file shouldn't stop the overall activation, hence the anyhow return. fn copy_file( source: &Path, - target: &Path, + target: &PathBuf, entry: &EtcFile, - old_state: &FileTree, -) -> anyhow::Result { - let exists = target.try_exists()?; - let backed_up = if exists && !old_state.is_managed(target) { + old_state: &EtcFilesState, + mut new_state: EtcFilesState, +) -> EtcActivationResult { + let exists = target.exists(); + if exists && !old_state.files.contains(target) { if entry.replace_existing { - backup_existing_file(target)?; - true + backup_existing_file(target).map_err(|e| ActivationError::WithPartialResult { + result: new_state.clone(), + source: e, + })?; + new_state.backed_up_files.insert(target.clone()); } else { - anyhow::bail!("File {} already exists, ignoring.", target.display()); + let error = anyhow!("File {} already exists, ignoring. Set replaceExisting if you want to back it up and override it.", target.display()); + return Err(ActivationError::WithPartialResult { + result: new_state, + source: error, + }); } - } else { - false }; log::debug!( "Copying file {} to {}...", source.display(), target.display() ); - fs::copy(source, target)?; - let mode_int = u32::from_str_radix(&entry.mode, 8)?; - fs::set_permissions(target, Permissions::from_mode(mode_int))?; - unixfs::chown(target, Some(find_uid(entry)?), Some(find_gid(entry)?))?; - Ok(backed_up) + fn to_activation_result>( + e: E, + state: &EtcFilesState, + ) -> ActivationError { + ActivationError::WithPartialResult { + result: state.clone(), + source: e.into(), + } + } + log::warn!("copy {} to {}", source.display(), target.display()); + fs::copy(source, target).map_err(|e| to_activation_result(e, &new_state))?; + let mode_int = + u32::from_str_radix(&entry.mode, 8).map_err(|e| to_activation_result(e, &new_state))?; + fs::set_permissions(target, Permissions::from_mode(mode_int)) + .map_err(|e| to_activation_result(e, &new_state))?; + unixfs::chown( + target, + Some(find_uid(entry).map_err(|e| to_activation_result(e, &new_state))?), + Some(find_gid(entry).map_err(|e| to_activation_result(e, &new_state))?), + ) + .map_err(|e| to_activation_result(e, &new_state))?; + new_state.files.insert(target.clone()); + Ok(new_state) } diff --git a/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs b/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs deleted file mode 100644 index 6728246f..00000000 --- a/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs +++ /dev/null @@ -1,571 +0,0 @@ -use im::HashMap; -use serde::{Deserialize, Serialize}; -use std::cmp::Eq; -use std::iter::Peekable; -use std::path; -use std::path::{Path, PathBuf}; - -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub enum FileStatus { - Managed, - ManagedWithBackup, - Unmanaged, -} - -impl FileStatus { - fn merge(&self, other: &Self) -> Self { - use FileStatus::*; - - match (self, other) { - (Unmanaged, Unmanaged) => Unmanaged, - (ManagedWithBackup, _) | (_, ManagedWithBackup) => ManagedWithBackup, - _ => Managed, - } - } -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct FileTree { - status: FileStatus, - pub(crate) path: PathBuf, - // TODO directories and files are now both represented as a string associated with a nested - // map. For files the nested map is simple empty. - // We could potentially optimise this. - pub(crate) nested: HashMap, -} - -impl AsRef for FileTree { - fn as_ref(&self) -> &FileTree { - self - } -} - -impl Default for FileTree { - fn default() -> Self { - Self::root_node() - } -} - -/// Data structure to represent files that are managed by system-manager. -/// -/// This data will be serialised to disk and read on the next run. -/// -/// We need these basic operations: -/// 1. Create a new root structure -/// 2. Persist to a file -/// 3. Import from a file -/// 4. Add a path to the tree, that will from then on be considered as managed -/// 5. -impl FileTree { - fn new(path: PathBuf) -> Self { - Self::with_status(path, FileStatus::Unmanaged) - } - - fn with_status(path: PathBuf, status: FileStatus) -> Self { - Self { - status, - path, - nested: HashMap::new(), - } - } - - pub fn root_node() -> Self { - Self::new(PathBuf::from(path::MAIN_SEPARATOR_STR)) - } - - pub fn get_status<'a>(&'a self, path: &Path) -> &'a FileStatus { - fn go<'a, 'b, C>(tree: &'a FileTree, mut components: C, path: &Path) -> &'a FileStatus - where - C: Iterator>, - { - if let Some(component) = components.next() { - match component { - path::Component::Normal(name) => tree - .nested - .get(name.to_string_lossy().as_ref()) - .map(|subtree| go(subtree, components, path)) - .unwrap_or(&FileStatus::Unmanaged), - path::Component::RootDir => go(tree, components, path), - _ => todo!(), - } - } else { - debug_assert!(tree.path == path); - &tree.status - } - } - go(self, path.components(), path) - } - - pub fn is_managed(&self, path: &Path) -> bool { - matches!( - self.get_status(path), - FileStatus::Managed | FileStatus::ManagedWithBackup - ) - } - - // TODO is recursion OK here? - // Should we convert to CPS and use a crate like tramp to TCO this? - pub fn register_managed_entry(self, path: &Path) -> Self { - self.register_entry(path, FileStatus::Managed) - } - - pub fn register_backed_up_entry(self, path: &Path) -> Self { - self.register_entry(path, FileStatus::ManagedWithBackup) - } - - fn register_entry(self, path: &Path, leaf_status: FileStatus) -> Self { - fn go<'a, C>( - mut tree: FileTree, - mut components: Peekable, - path: PathBuf, - leaf_status: &FileStatus, - ) -> FileTree - where - C: Iterator>, - { - if let Some(component) = components.next() { - match component { - path::Component::Normal(name) => { - let new_path = path.join(component); - tree.nested = tree.nested.alter( - |maybe_subtree| { - Some(go( - maybe_subtree.unwrap_or_else(|| { - FileTree::with_status( - new_path.clone(), - // We only label with the leaf status the final path - // entry, to label intermediate nodes as managed, we - // should call this function for every one of them - // separately. - components.peek().map_or(leaf_status.clone(), |_| { - FileStatus::Unmanaged - }), - ) - }), - components, - new_path, - leaf_status, - )) - }, - name.to_string_lossy().to_string(), - ); - tree - } - path::Component::RootDir => go( - tree, - components, - path.join(path::MAIN_SEPARATOR_STR), - leaf_status, - ), - _ => panic!( - "Unsupported path provided! At path component: {:?}", - component - ), - } - } else { - tree - } - } - - go( - self, - path.components().peekable(), - PathBuf::new(), - &leaf_status, - ) - } - - pub fn deactivate(self, delete_action: &F) -> Option - where - F: Fn(&Path, &FileStatus) -> bool, - { - let new_tree = self.nested.keys().fold(self.clone(), |mut new_tree, name| { - new_tree.nested = new_tree.nested.alter( - |subtree| subtree.and_then(|subtree| subtree.deactivate(delete_action)), - name.to_owned(), - ); - new_tree - }); - - // We clean up nodes that are empty and unmanaged. - // These represent intermediate directories that already existed, so we - // are not responsible for cleaning them up (we don't run the delete_action - // closure on their paths). - if new_tree.nested.is_empty() { - if matches!( - new_tree.status, - FileStatus::Managed | FileStatus::ManagedWithBackup - ) { - if delete_action(&new_tree.path, &new_tree.status) { - None - } else { - Some(new_tree) - } - } else { - None - } - } else { - Some(new_tree) - } - } - - pub fn update_state(self, other: Self, delete_action: &F) -> Option - where - F: Fn(&Path, &FileStatus) -> bool, - { - let to_deactivate = other - .nested - .clone() - .relative_complement(self.nested.clone()); - let to_merge = other.nested.intersection(self.nested.clone()); - - let deactivated = to_deactivate - .into_iter() - .fold(self, |mut new_tree, (name, subtree)| { - subtree - .deactivate(delete_action) - .into_iter() - .for_each(|subtree| { - new_tree.nested.insert(name.to_owned(), subtree); - }); - new_tree - }); - - let merged = to_merge - .into_iter() - .fold(deactivated, |mut new_tree, (name, other_tree)| { - new_tree.nested = new_tree.nested.alter( - |subtree| { - subtree.and_then(|subtree| { - subtree.update_state(other_tree.clone(), delete_action).map( - |mut new_tree| { - new_tree.status = new_tree.status.merge(&other_tree.status); - new_tree - }, - ) - }) - }, - name, - ); - new_tree - }); - - // If our invariants are properly maintained, then we should never end up - // here with dangling unmanaged nodes. - debug_assert!( - !merged.nested.is_empty() - || matches!( - merged.status, - FileStatus::Managed | FileStatus::ManagedWithBackup - ) - ); - - Some(merged) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use itertools::Itertools; - - impl FileTree { - pub fn deactivate_managed_entry(self, path: &Path, delete_action: &F) -> Self - where - F: Fn(&Path, &FileStatus) -> bool, - { - fn go<'a, C, F>( - mut tree: FileTree, - path: PathBuf, - mut components: Peekable, - delete_action: &F, - ) -> FileTree - where - C: Iterator>, - F: Fn(&Path, &FileStatus) -> bool, - { - log::debug!("Deactivating {}", path.display()); - - if let Some(component) = components.next() { - match component { - path::Component::Normal(name) => { - let new_path = path.join(name); - tree.nested = tree.nested.alter( - |maybe_subtree| { - maybe_subtree.and_then(|subtree| { - if components.peek().is_some() { - Some(go(subtree, new_path, components, delete_action)) - } else { - subtree.deactivate(delete_action) - } - }) - }, - name.to_string_lossy().to_string(), - ); - tree - } - path::Component::RootDir => go( - tree, - path.join(path::MAIN_SEPARATOR.to_string()), - components, - delete_action, - ), - _ => panic!( - "Unsupported path provided! At path component: {:?}", - component - ), - } - } else { - tree - } - } - go( - self, - PathBuf::new(), - path.components().peekable(), - delete_action, - ) - } - } - - #[test] - fn get_status() { - let tree1 = FileTree::root_node() - .register_managed_entry(&PathBuf::from("/").join("foo").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo2")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz2")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz2").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo3").join("baz2").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo4")) - .register_managed_entry(&PathBuf::from("/").join("foo4").join("baz")) - .register_managed_entry(&PathBuf::from("/").join("foo4").join("baz").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo5")) - .register_managed_entry(&PathBuf::from("/").join("foo5").join("baz")) - .register_managed_entry(&PathBuf::from("/").join("foo5").join("baz2")) - .register_managed_entry(&PathBuf::from("/").join("foo5").join("baz").join("bar")); - - assert!(tree1.is_managed(&PathBuf::from("/").join("foo5").join("baz").join("bar"))); - assert!(!tree1.is_managed(&PathBuf::from("/").join("foo"))); - assert!(!tree1.is_managed(&PathBuf::from("/").join("foo").join("nonexistent"))); - } - - #[test] - fn register() { - let tree = FileTree::root_node() - .register_managed_entry(&PathBuf::from("/").join("foo").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz2").join("bar")); - dbg!(&tree); - assert_eq!( - tree.nested.keys().sorted().collect::>(), - ["foo", "foo2"] - ); - assert_eq!( - tree.nested - .get("foo2") - .unwrap() - .nested - .get("baz") - .unwrap() - .nested - .get("bar") - .unwrap() - .path, - PathBuf::from("/").join("foo2").join("baz").join("bar") - ); - } - - #[test] - fn deactivate() { - let tree1 = FileTree::root_node() - .register_managed_entry(&PathBuf::from("/").join("foo").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo2")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz2")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz2").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo3").join("baz2").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo4")) - .register_managed_entry(&PathBuf::from("/").join("foo4").join("baz")) - .register_managed_entry(&PathBuf::from("/").join("foo4").join("baz").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo5")) - .register_managed_entry(&PathBuf::from("/").join("foo5").join("baz")) - .register_managed_entry(&PathBuf::from("/").join("foo5").join("baz2")) - .register_managed_entry(&PathBuf::from("/").join("foo5").join("baz").join("bar")); - let tree2 = tree1 - .clone() - .deactivate_managed_entry(&PathBuf::from("/").join("foo4"), &|path, _status| { - println!("Deactivating: {}", path.display()); - false - }) - .deactivate_managed_entry(&PathBuf::from("/").join("foo2"), &|path, _status| { - println!("Deactivating: {}", path.display()); - true - }) - .deactivate_managed_entry(&PathBuf::from("/").join("foo3"), &|path, _status| { - println!("Deactivating: {}", path.display()); - true - }) - .deactivate_managed_entry( - &PathBuf::from("/").join("foo5").join("baz"), - &|path, _status| { - println!("Deactivating: {}", path.display()); - true - }, - ); - dbg!(&tree1); - assert_eq!( - tree2.nested.keys().sorted().collect::>(), - ["foo", "foo4", "foo5"] - ); - assert!(tree2 - .nested - .get("foo5") - .unwrap() - .nested - .get("baz2") - .unwrap() - .nested - .keys() - .sorted() - .collect::>() - .is_empty()); - assert_eq!( - tree1.nested.keys().sorted().collect::>(), - ["foo", "foo2", "foo3", "foo4", "foo5"] - ); - } - - #[test] - fn managed_with_backup_is_managed() { - let tree = FileTree::root_node() - .register_backed_up_entry(&PathBuf::from("/").join("foo").join("bar")); - - assert!(tree.is_managed(&PathBuf::from("/").join("foo").join("bar"))); - assert!(!tree.is_managed(&PathBuf::from("/").join("foo"))); - } - - #[test] - fn register_backed_up_entry_sets_status() { - let tree = FileTree::root_node() - .register_backed_up_entry(&PathBuf::from("/").join("etc").join("nix.conf")); - - assert_eq!( - *tree.get_status(&PathBuf::from("/").join("etc").join("nix.conf")), - FileStatus::ManagedWithBackup, - ); - assert_eq!( - *tree.get_status(&PathBuf::from("/").join("etc")), - FileStatus::Unmanaged, - ); - } - - #[test] - fn merge_preserves_managed_with_backup() { - assert_eq!( - FileStatus::ManagedWithBackup.merge(&FileStatus::Unmanaged), - FileStatus::ManagedWithBackup, - ); - assert_eq!( - FileStatus::ManagedWithBackup.merge(&FileStatus::Managed), - FileStatus::ManagedWithBackup, - ); - assert_eq!( - FileStatus::Managed.merge(&FileStatus::ManagedWithBackup), - FileStatus::ManagedWithBackup, - ); - assert_eq!( - FileStatus::Unmanaged.merge(&FileStatus::ManagedWithBackup), - FileStatus::ManagedWithBackup, - ); - assert_eq!( - FileStatus::ManagedWithBackup.merge(&FileStatus::ManagedWithBackup), - FileStatus::ManagedWithBackup, - ); - } - - #[test] - fn deactivate_passes_backup_status_to_action() { - let tree = FileTree::root_node() - .register_backed_up_entry(&PathBuf::from("/").join("etc").join("nix.conf")) - .register_managed_entry(&PathBuf::from("/").join("etc").join("other")); - - let statuses = std::cell::RefCell::new(Vec::<(PathBuf, FileStatus)>::new()); - tree.deactivate(&|path: &Path, status: &FileStatus| { - statuses - .borrow_mut() - .push((path.to_owned(), status.clone())); - true - }); - - let statuses = statuses.into_inner(); - let backup_entries: Vec<_> = statuses - .iter() - .filter(|(_, s)| *s == FileStatus::ManagedWithBackup) - .collect(); - assert_eq!(backup_entries.len(), 1); - assert_eq!( - backup_entries[0].0, - PathBuf::from("/").join("etc").join("nix.conf") - ); - - let managed_entries: Vec<_> = statuses - .iter() - .filter(|(_, s)| *s == FileStatus::Managed) - .collect(); - assert_eq!(managed_entries.len(), 1); - assert_eq!( - managed_entries[0].0, - PathBuf::from("/").join("etc").join("other") - ); - } - - #[test] - fn mixed_managed_and_backed_up() { - let tree = FileTree::root_node() - .register_managed_entry(&PathBuf::from("/").join("foo").join("bar")) - .register_backed_up_entry(&PathBuf::from("/").join("foo").join("baz")); - - assert!(tree.is_managed(&PathBuf::from("/").join("foo").join("bar"))); - assert!(tree.is_managed(&PathBuf::from("/").join("foo").join("baz"))); - assert_eq!( - *tree.get_status(&PathBuf::from("/").join("foo").join("bar")), - FileStatus::Managed, - ); - assert_eq!( - *tree.get_status(&PathBuf::from("/").join("foo").join("baz")), - FileStatus::ManagedWithBackup, - ); - } - - #[test] - fn update_state() { - let tree1 = FileTree::root_node() - .register_managed_entry(&PathBuf::from("/").join("foo").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo2")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz2")) - .register_managed_entry(&PathBuf::from("/").join("foo2").join("baz2").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo3").join("baz2").join("bar")); - let tree2 = FileTree::root_node() - .register_managed_entry(&PathBuf::from("/").join("foo").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo3").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo4")) - .register_managed_entry(&PathBuf::from("/").join("foo4").join("bar")) - .register_managed_entry(&PathBuf::from("/").join("foo5")) - .register_managed_entry(&PathBuf::from("/").join("foo5").join("bar")); - let new_tree = tree1.update_state(tree2, &|path, _status| { - println!("Deactivating path: {}", path.display()); - *path != PathBuf::from("/").join("foo5").join("bar") - }); - assert_eq!( - new_tree.unwrap().nested.keys().sorted().collect::>(), - ["foo", "foo2", "foo3", "foo5"] - ); - } -} diff --git a/crates/system-manager-engine/src/activate/tmp_files.rs b/crates/system-manager-engine/src/activate/tmp_files.rs index aa03b92a..b4543476 100644 --- a/crates/system-manager-engine/src/activate/tmp_files.rs +++ b/crates/system-manager-engine/src/activate/tmp_files.rs @@ -1,25 +1,29 @@ use crate::activate; -use crate::activate::etc_files::FileTree; use super::ActivationResult; +use std::collections::HashSet; +use std::path::PathBuf; use std::process; type TmpFilesActivationResult = ActivationResult<()>; -pub fn activate(etc_tree: &FileTree) -> TmpFilesActivationResult { - let conf_files = etc_tree - .nested - .get("etc") - .and_then(|etc| etc.nested.get("tmpfiles.d")) - .map_or(vec![], |tmpfiles_d| { - tmpfiles_d - .nested - .iter() - .map(|(_, node)| node.path.to_string_lossy().to_string()) - .collect::>() - }); +pub fn activate(etc_tree: &HashSet) -> TmpFilesActivationResult { + let tmp_files_prefix = PathBuf::from("/etc/tmpfiles.d"); + // List and collect managed files under /etc/tmpFiles.d + let tmpfiles_conf_files: Vec<&str> = etc_tree + .iter() + .filter_map(|p| { + if p.starts_with(&tmp_files_prefix) { + p.to_str() + } else { + None + } + }) + .collect(); let mut cmd = process::Command::new("systemd-tmpfiles"); - cmd.arg("--create").arg("--remove").args(conf_files); + cmd.arg("--create") + .arg("--remove") + .args(tmpfiles_conf_files); log::debug!("running {:#?}", cmd); let output = cmd .stdout(process::Stdio::inherit()) diff --git a/crates/system-manager-engine/src/lib.rs b/crates/system-manager-engine/src/lib.rs index 043d6a96..07b90e0e 100644 --- a/crates/system-manager-engine/src/lib.rs +++ b/crates/system-manager-engine/src/lib.rs @@ -143,12 +143,6 @@ fn remove_file(from: &Path) -> Result<()> { Ok(()) } -fn remove_dir(from: &Path) -> Result<()> { - log::info!("Removing directory: {}", from.display()); - fs::remove_dir(from)?; - Ok(()) -} - pub fn etc_dir(ephemeral: bool) -> PathBuf { if ephemeral { Path::new("/run").join("etc") diff --git a/nix/lib.nix b/nix/lib.nix index d61917f9..6e5c327e 100644 --- a/nix/lib.nix +++ b/nix/lib.nix @@ -195,7 +195,7 @@ let action, }: '' - ${node}.succeed("${profile}/bin/${action} 2>&1 | tee /tmp/output.log") + ${node}.succeed("RUST_LOG=debug ${profile}/bin/${action} 2>&1 | tee /tmp/output.log") ${node}.succeed("! grep -F 'ERROR' /tmp/output.log") ''; From bc60f5103839b4b3770260076dbe810acca65061 Mon Sep 17 00:00:00 2001 From: picnoir Date: Tue, 31 Mar 2026 16:07:56 +0200 Subject: [PATCH 02/18] Etc files refactoring: implement state migration mechanism Implementing a mechanism allowing us to migrate the old style etc files recursive datastructure state into the new flat one. --- crates/system-manager-engine/src/activate.rs | 83 ++++++++++++++----- .../src/activate/etc_files.rs | 28 +++++-- .../src/activate/etc_files/etc_tree.rs | 75 +++++++++++++++++ .../system-manager-engine/src/deactivate.rs | 12 +-- 4 files changed, 166 insertions(+), 32 deletions(-) create mode 100644 crates/system-manager-engine/src/activate/etc_files/etc_tree.rs diff --git a/crates/system-manager-engine/src/activate.rs b/crates/system-manager-engine/src/activate.rs index 7d20193e..1130beb8 100644 --- a/crates/system-manager-engine/src/activate.rs +++ b/crates/system-manager-engine/src/activate.rs @@ -3,14 +3,17 @@ pub(crate) mod services; mod tmp_files; pub(crate) mod users; -use anyhow::Result; +use anyhow::{anyhow, Result}; use serde::{Deserialize, Serialize}; +use serde_json::error::Category; use std::collections::HashSet; use std::fs::DirBuilder; +use std::io::Seek; use std::path::{Path, PathBuf}; use std::{fs, io, process}; use thiserror::Error; +use crate::activate::etc_files::etc_tree::StateV0; use crate::{StorePath, STATE_FILE_NAME, SYSTEM_MANAGER_STATE_DIR}; #[derive(Error, Debug)] @@ -42,30 +45,68 @@ pub enum FileStatus { type EtcTree = HashSet; type BackedUpFiles = HashSet; -#[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct EtcFilesState { pub files: EtcTree, pub backed_up_files: BackedUpFiles, } -#[derive(Debug, Clone, Default, Serialize, Deserialize)] +impl EtcFilesState { + pub fn contains(&self, path: &Path) -> bool { + // Union of backed up files + regular files appearing in old state. + let mut all_files_old_state = self.files.clone(); + all_files_old_state.extend(self.backed_up_files.clone()); + all_files_old_state.contains(path) + } +} + +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct State { +pub struct StateV1 { pub(crate) file_tree: EtcFilesState, pub(crate) services: services::Services, + pub(crate) version: u32, +} + +impl Default for StateV1 { + fn default() -> Self { + Self { + file_tree: EtcFilesState::default(), + services: services::Services::default(), + version: 1, + } + } } -impl State { +impl StateV1 { pub fn from_file(state_file: &Path) -> Result { if state_file.is_file() { log::info!("Reading state info from {}", state_file.display()); - let reader = io::BufReader::new(fs::File::open(state_file)?); - serde_json::from_reader(reader).or_else(|e| { - log::error!("Error reading the state file, ignoring."); - log::error!("{e:?}"); - Ok(Self::default()) - }) + let mut reader = io::BufReader::new(fs::File::open(state_file)?); + // if state is v1 + let rv1: serde_json::Result = serde_json::from_reader(&mut reader); + match rv1 { + Ok(v1) => Ok(v1), + Err(e) => { + // State might be v0. Let's try to parse it. + if e.classify() == Category::Data { + reader.rewind()?; + let filetree: StateV0 = + serde_json::from_reader(&mut reader).map_err(|e| { + anyhow!( + "Cannot parse state, it doesn't match any supported format: {}", + e + ) + })?; + Ok(filetree.into()) + } else { + // State is + Err(anyhow!("Unexpected serde_json error: {}", e)) + } + } + } + // else parse v0 then migrate } else { Ok(Self::default()) } @@ -93,7 +134,7 @@ pub fn activate(store_path: &StorePath, ephemeral: bool) -> Result<()> { } let state_file = &get_state_file()?; - let old_state = State::from_file(state_file)?; + let old_state = StateV1::from_file(state_file)?; log::info!("Activating etc files..."); @@ -118,15 +159,17 @@ pub fn activate(store_path: &StorePath, ephemeral: bool) -> Result<()> { log::info!("Activating systemd services..."); let final_state = match services::activate(store_path, old_state.services, ephemeral) { - Ok(services) => State { + Ok(services) => StateV1 { file_tree: etc_tree, services, + version: Default::default(), }, Err(ActivationError::WithPartialResult { result, source }) => { log::error!("Error during activation: {source:?}"); - State { + StateV1 { file_tree: etc_tree, services: result, + version: Default::default(), } } }; @@ -141,7 +184,7 @@ pub fn activate(store_path: &StorePath, ephemeral: bool) -> Result<()> { Err(ActivationError::WithPartialResult { result, source }) => { log::error!("Error during activation: {source:?}"); log::debug!("Resulting file tree: {:?}", result); - let final_state = State { + let final_state = StateV1 { file_tree: result, ..old_state }; @@ -163,7 +206,7 @@ pub fn prepopulate(store_path: &StorePath, ephemeral: bool) -> Result<()> { } let state_file = &get_state_file()?; - let old_state = State::from_file(state_file)?; + let old_state = StateV1::from_file(state_file)?; log::info!("Activating etc files..."); @@ -171,22 +214,24 @@ pub fn prepopulate(store_path: &StorePath, ephemeral: bool) -> Result<()> { Ok(etc_tree) => { log::info!("Registering systemd services..."); match services::get_active_services(store_path, old_state.services) { - Ok(services) => State { + Ok(services) => StateV1 { file_tree: etc_tree, services, + version: Default::default(), }, Err(ActivationError::WithPartialResult { result, source }) => { log::error!("Error during activation: {source:?}"); - State { + StateV1 { file_tree: etc_tree, services: result, + version: Default::default(), } } } } Err(ActivationError::WithPartialResult { result, source }) => { log::error!("Error during activation: {source:?}"); - State { + StateV1 { file_tree: result, ..old_state } diff --git a/crates/system-manager-engine/src/activate/etc_files.rs b/crates/system-manager-engine/src/activate/etc_files.rs index 0ea4df32..1cd906da 100644 --- a/crates/system-manager-engine/src/activate/etc_files.rs +++ b/crates/system-manager-engine/src/activate/etc_files.rs @@ -1,3 +1,4 @@ +pub mod etc_tree; use anyhow::{anyhow, Context}; use im::HashMap; use regex; @@ -297,13 +298,14 @@ fn create_etc_file( // We did not find a proper way to do that from the Nix static env, // hardcoding this condition in the activation instead. let target_is_in_systemd_dir = is_inside_systemd_dependency_dir(&target); + if file.mode == "symlink" { // On some symlinks, target.exists() returns false. Not sure why. let exists = target.exists() || target.is_symlink(); if exists { // If the target exists and has been created by a previous system-manager activation, // replace it. - if old_state.files.contains(&target) { + if old_state.contains(&target) { log::debug!( "{} is managed by system-manager. Deleting.", &target.display() @@ -319,7 +321,12 @@ fn create_etc_file( source: e.into(), } })?; - state.files.insert(target); + // Check whether this file is a backup or plain link in the old state. + if old_state.files.contains(&target) { + state.files.insert(target); + } else { + state.backed_up_files.insert(target); + } } else if file.replace_existing || target_is_in_systemd_dir { log::debug!( "{} already exists but it's set to replace. Backup and link again.", @@ -333,6 +340,7 @@ fn create_etc_file( ); } } else { + // Target do not exist on the filesystem log::debug!("Symlink {} => {}", file.source, target.display()); unix::fs::symlink(file.source.store_path, &target).map_err(|e| { ActivationError::WithPartialResult { @@ -381,7 +389,6 @@ fn backup_and_link( source: e.into(), })?; dir_state.backed_up_files.insert(target.to_owned()); - dir_state.files.insert(target.to_owned()); Ok(dir_state) } @@ -453,14 +460,14 @@ fn copy_file( old_state: &EtcFilesState, mut new_state: EtcFilesState, ) -> EtcActivationResult { - let exists = target.exists(); - if exists && !old_state.files.contains(target) { - if entry.replace_existing { + let exists = target.exists() || target.is_symlink(); + let exists_and_need_backup = exists && !old_state.contains(target) && entry.replace_existing; + if exists && !old_state.contains(target) { + if exists_and_need_backup { backup_existing_file(target).map_err(|e| ActivationError::WithPartialResult { result: new_state.clone(), source: e, })?; - new_state.backed_up_files.insert(target.clone()); } else { let error = anyhow!("File {} already exists, ignoring. Set replaceExisting if you want to back it up and override it.", target.display()); return Err(ActivationError::WithPartialResult { @@ -495,6 +502,11 @@ fn copy_file( Some(find_gid(entry).map_err(|e| to_activation_result(e, &new_state))?), ) .map_err(|e| to_activation_result(e, &new_state))?; - new_state.files.insert(target.clone()); + // Update the state depending whether or not we backed up a file before + if exists_and_need_backup || (old_state.backed_up_files.contains(target)) { + new_state.backed_up_files.insert(target.clone()); + } else { + new_state.files.insert(target.clone()); + } Ok(new_state) } diff --git a/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs b/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs new file mode 100644 index 00000000..91520db8 --- /dev/null +++ b/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs @@ -0,0 +1,75 @@ +use im::HashMap; +use serde::{Deserialize, Serialize}; +use std::path::PathBuf; + +use crate::activate::{services, EtcFilesState, StateV1}; + +/// Legacy datatype used to migrate to the new state format. +/// +/// This should be deleted from the codebase at some point. Once we assume most users migrated to the new version. +/// It cannot be before next release. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct StateV0 { + pub(crate) file_tree: FileTree, + pub(crate) services: services::Services, +} + +impl From for StateV1 { + fn from(v0: StateV0) -> StateV1 { + let services = v0.services; + let file_tree: EtcFilesState = v0.file_tree.into(); + StateV1 { + file_tree, + services, + version: Default::default(), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum FileStatus { + Managed, + ManagedWithBackup, + Unmanaged, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct FileTree { + status: FileStatus, + pub(crate) path: PathBuf, + // TODO directories and files are now both represented as a string associated with a nested + // map. For files the nested map is simple empty. + // We could potentially optimise this. + pub(crate) nested: HashMap, +} + +impl From for EtcFilesState { + fn from(ft: FileTree) -> EtcFilesState { + let mut paths_to_go: Vec = vec![ft]; + let mut etc_files = EtcFilesState::default(); + let mut i = 0; + while i < paths_to_go.len() { + let elem = paths_to_go + .get(i) + .expect("ERROR: index error in paths_to_go loop") + .clone(); + for nested in elem.nested.clone() { + paths_to_go.push(nested.1); + } + match elem.status { + FileStatus::Managed => { + etc_files.files.insert(elem.path); + } + FileStatus::ManagedWithBackup => { + etc_files.backed_up_files.insert(elem.path); + } + FileStatus::Unmanaged => {} + }; + i += 1; + } + etc_files + } +} diff --git a/crates/system-manager-engine/src/deactivate.rs b/crates/system-manager-engine/src/deactivate.rs index 09e533a2..3b80f2bc 100644 --- a/crates/system-manager-engine/src/deactivate.rs +++ b/crates/system-manager-engine/src/deactivate.rs @@ -3,14 +3,14 @@ use anyhow::Result; use crate::activate::etc_files; use crate::activate::services; use crate::activate::users; -use crate::activate::{get_state_file, ActivationError, State}; +use crate::activate::{get_state_file, ActivationError, StateV1}; /// Deactivates system-manager by locking managed users, removing etc files, /// and stopping systemd services. pub fn deactivate() -> Result<()> { log::info!("Deactivating system-manager"); let state_file = &get_state_file()?; - let old_state = State::from_file(state_file)?; + let old_state = StateV1::from_file(state_file)?; log::debug!("{old_state:?}"); if let Err(e) = users::lock_managed_users() { @@ -25,22 +25,24 @@ pub fn deactivate() -> Result<()> { Ok(etc_tree) => { log::info!("Deactivating systemd services..."); match services::deactivate(old_state.services) { - Ok(services) => State { + Ok(services) => StateV1 { file_tree: etc_tree, services, + version: Default::default(), }, Err(ActivationError::WithPartialResult { result, source }) => { log::error!("Error during deactivation: {source:?}"); - State { + StateV1 { file_tree: etc_tree, services: result, + version: Default::default(), } } } } Err(ActivationError::WithPartialResult { result, source }) => { log::error!("Error during deactivation: {source:?}"); - State { + StateV1 { file_tree: result, ..old_state } From d68a1243aef339c52d96f6b7d6279318d542aaea Mon Sep 17 00:00:00 2001 From: picnoir Date: Tue, 31 Mar 2026 19:39:22 +0200 Subject: [PATCH 03/18] /etc files refactoring: fix deactivation bug The refactoring introduced a regression in the deactivation stage: the backed up files are not appearing anymore in state.files. Fixing it. --- crates/system-manager-engine/src/activate/etc_files.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/system-manager-engine/src/activate/etc_files.rs b/crates/system-manager-engine/src/activate/etc_files.rs index 1cd906da..39e281de 100644 --- a/crates/system-manager-engine/src/activate/etc_files.rs +++ b/crates/system-manager-engine/src/activate/etc_files.rs @@ -137,8 +137,11 @@ pub fn activate( pub fn deactivate(old_state: EtcFilesState) -> EtcActivationResult { let files = old_state.files.clone(); - let final_state = delete_paths(&files, old_state); - + let mut final_state = delete_paths(&files, old_state); + for file_to_restore in &final_state.backed_up_files.clone() { + let _ = restore_backup(file_to_restore) + .map(|_| final_state.backed_up_files.remove(&file_to_restore.clone())); + } log::info!("Done"); Ok(final_state) } From 2c025e31127b1aa75f55e5cb91ec2e32a13d3cbc Mon Sep 17 00:00:00 2001 From: picnoir Date: Tue, 31 Mar 2026 20:07:34 +0200 Subject: [PATCH 04/18] etc files refactoring: introduce new state migration test First container test: 1. activates a system-manager profile with an old version generating a v0 state file. 2. deactivate with a new system-manager, testing the state migration. --- .../container-tests/state-v0-v1-migration.nix | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 testFlake/container-tests/state-v0-v1-migration.nix diff --git a/testFlake/container-tests/state-v0-v1-migration.nix b/testFlake/container-tests/state-v0-v1-migration.nix new file mode 100644 index 00000000..63d4a4bb --- /dev/null +++ b/testFlake/container-tests/state-v0-v1-migration.nix @@ -0,0 +1,88 @@ +{ + forEachDistro, + system, + ... +}: + +forEachDistro "state-v0-v1-migration" ( + let + module = { + environment.etc = { + "a/bar" = { + text = "bar"; + mode = "0700"; + user = "user"; + group = "root"; + }; + "a/link" = { + text = "link"; + mode = "symlink"; + }; + "b/bar" = { + text = "bar"; + mode = "0700"; + user = "user"; + group = "root"; + replaceExisting = true; + }; + "b/link" = { + text = "link"; + mode = "symlink"; + replaceExisting = true; + }; + }; + }; + v0SystemManagerFlake = builtins.getFlake "github:numtide/system-manager/62a1be911f9e24bdb357377587c73544c2aec719"; + v0TopLevel = v0SystemManagerFlake.lib.makeSystemConfig { + modules = [ + module + { + nixpkgs.hostPlatform = system; + system-manager.allowAnyDistro = true; + } + + ]; + }; + + in + { + modules = [ + module + ]; + extraPathsToRegister = [ v0TopLevel ]; + testScriptFunction = + { toplevel, hostPkgs, ... }: + '' + # Start the container + start_all() + + # Wait for systemd to be ready + machine.wait_for_unit("multi-user.target") + machine.execute('mkdir -p /etc/b') + machine.execute('echo "tobackup" > /etc/b/link') + machine.execute('echo "tobackup" > /etc/b/bar') + + def check_file(path, content): + file = machine.file(path) + assert file.exists, f"{path} should exist" + assert file.is_file, f"{path} should be a file" + assert file.contains(content), f"{path} should contain {content}" + + # Let's activate the profile with a v0 state file (using an old system-manager checkout) + machine.succeed("${v0TopLevel}/bin/activate") + with subtest("Verify correct files are created"): + check_file("/etc/a/bar", "bar") + check_file("/etc/a/link", "link") + check_file("/etc/b/bar", "bar") + check_file("/etc/b/link", "link") + + # Let's try to deactivate the machine with the new binary, making sure the state migration works. + machine.succeed("${toplevel}/bin/deactivate") + with subtest("v1 deactivation restores the backups from a v0 generated state"): + machine.succeed("test -f /etc/b/bar") + machine.succeed("test -f /etc/b/link") + check_file("/etc/b/bar", "tobackup") + check_file("/etc/b/link", "tobackup") + ''; + } +) From 28ce62660d75dde270fb4cd42e1572ca4c9dc543 Mon Sep 17 00:00:00 2001 From: picnoir Date: Wed, 1 Apr 2026 16:57:05 +0200 Subject: [PATCH 05/18] etc files tests: remove check on /etc/.system-manager-static We're not linking this path anymore. --- testFlake/container-tests/empty-config.nix | 4 ---- 1 file changed, 4 deletions(-) diff --git a/testFlake/container-tests/empty-config.nix b/testFlake/container-tests/empty-config.nix index c6720813..05834241 100644 --- a/testFlake/container-tests/empty-config.nix +++ b/testFlake/container-tests/empty-config.nix @@ -102,10 +102,6 @@ forEachDistro "empty-config" { machine.wait_for_unit("system-manager.target") - with subtest("Static environment symlink exists"): - assert machine.file("/etc/.system-manager-static").is_symlink, \ - "/etc/.system-manager-static should be a symlink after activation" - with subtest("No unexpected changes to /etc after activation"): after = snapshot_etc() added = [p for p in (set(after) - set(before)) if not is_expected(p)] From 28f7c3f774eb87e6f485a2ee07f5ed862f9e3ba9 Mon Sep 17 00:00:00 2001 From: picnoir Date: Wed, 1 Apr 2026 18:26:30 +0200 Subject: [PATCH 06/18] etc files: align refactored text to the previous one Some tests are relying on this warn message. Let's rather align the code to the test to ensure continuity. --- crates/system-manager-engine/src/activate/etc_files.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/system-manager-engine/src/activate/etc_files.rs b/crates/system-manager-engine/src/activate/etc_files.rs index 39e281de..ba1474d8 100644 --- a/crates/system-manager-engine/src/activate/etc_files.rs +++ b/crates/system-manager-engine/src/activate/etc_files.rs @@ -338,7 +338,7 @@ fn create_etc_file( state = backup_and_link(&target, &file.source.store_path, state)?; } else { log::warn!( - "{} already exists. Set replaceExisting if you're willing to override it.", + "Error while creating file in /etc: Unmanaged path already exists in filesystem, please remove it and run system-manager again: {}\nSet replaceExisting if you're willing to override it.", target.display() ); } From 702c93d9c4e174f751d024d3ac1a13df7692534a Mon Sep 17 00:00:00 2001 From: picnoir Date: Thu, 2 Apr 2026 09:54:34 +0200 Subject: [PATCH 07/18] EtcFilesState::contains: remove unecessary allocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No need to allocate a new hashset 🤦 --- crates/system-manager-engine/src/activate.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/system-manager-engine/src/activate.rs b/crates/system-manager-engine/src/activate.rs index 1130beb8..ae4971c2 100644 --- a/crates/system-manager-engine/src/activate.rs +++ b/crates/system-manager-engine/src/activate.rs @@ -54,10 +54,7 @@ pub struct EtcFilesState { impl EtcFilesState { pub fn contains(&self, path: &Path) -> bool { - // Union of backed up files + regular files appearing in old state. - let mut all_files_old_state = self.files.clone(); - all_files_old_state.extend(self.backed_up_files.clone()); - all_files_old_state.contains(path) + self.files.contains(path) || self.backed_up_files.contains(path) } } From d7598e40c9eeccc606d26ccae1bb7b828855674e Mon Sep 17 00:00:00 2001 From: picnoir Date: Thu, 2 Apr 2026 10:25:49 +0200 Subject: [PATCH 08/18] testFlake: add activate state migration test This test is making sure re-activating a profile originally activated using an old system manager version that uses the v0 format works as expected. --- .../state-v0-v1-activate-migration.nix | 104 ++++++++++++++++++ ...x => state-v0-v1-deactivate-migration.nix} | 2 +- 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 testFlake/container-tests/state-v0-v1-activate-migration.nix rename testFlake/container-tests/{state-v0-v1-migration.nix => state-v0-v1-deactivate-migration.nix} (98%) diff --git a/testFlake/container-tests/state-v0-v1-activate-migration.nix b/testFlake/container-tests/state-v0-v1-activate-migration.nix new file mode 100644 index 00000000..728a8437 --- /dev/null +++ b/testFlake/container-tests/state-v0-v1-activate-migration.nix @@ -0,0 +1,104 @@ +{ + forEachDistro, + system, + ... +}: + +forEachDistro "state-v0-v1-migration-activate" ( + let + module = { + environment.etc = { + "a/bar" = { + text = "bar"; + mode = "0700"; + user = "user"; + group = "root"; + }; + "a/link" = { + text = "link"; + mode = "symlink"; + }; + "b/bar" = { + text = "bar"; + mode = "0700"; + user = "user"; + group = "root"; + replaceExisting = true; + }; + "b/link" = { + text = "link"; + mode = "symlink"; + replaceExisting = true; + }; + }; + }; + v0SystemManagerFlake = builtins.getFlake "github:numtide/system-manager/62a1be911f9e24bdb357377587c73544c2aec719"; + v0TopLevel = v0SystemManagerFlake.lib.makeSystemConfig { + modules = [ + module + { + nixpkgs.hostPlatform = system; + system-manager.allowAnyDistro = true; + } + + ]; + }; + + in + { + modules = [ + module + ]; + extraPathsToRegister = [ v0TopLevel ]; + testScriptFunction = + { toplevel, hostPkgs, ... }: + '' + # Start the container + start_all() + + # Wait for systemd to be ready + machine.wait_for_unit("multi-user.target") + machine.execute('mkdir -p /etc/b') + machine.execute('echo "tobackup" > /etc/b/link') + machine.execute('echo "tobackup" > /etc/b/bar') + + def check_file(path, content): + file = machine.file(path) + assert file.exists, f"{path} should exist" + assert file.is_file, f"{path} should be a file" + assert file.contains(content), f"{path} should contain {content}" + + # Let's activate the profile with a v0 state file (using an old system-manager checkout) + machine.succeed("${v0TopLevel}/bin/activate") + with subtest("Verify correct files are created"): + check_file("/etc/a/bar", "bar") + check_file("/etc/a/link", "link") + check_file("/etc/b/bar", "bar") + check_file("/etc/b/link", "link") + + # Let's try to deactivate the machine with the new binary, making sure the state migration works. + machine.succeed("${toplevel}/bin/activate") + with subtest("v1 activation keeps the file and migrate the state to v1"): + check_file("/etc/a/bar", "bar") + check_file("/etc/a/link", "link") + check_file("/etc/b/bar", "bar") + check_file("/etc/b/link", "link") + + with subtest("Check state content and make sure it's correctly migrated"): + # Test state + import json + file = machine.file("/var/lib/system-manager/state/system-manager-state.json") + state = json.loads(file.content_string) + files = state['fileTree']['files'] + assert "/etc/a/bar" in files, "/etc/a/bar should appear in the state as a non backup file" + assert "/etc/a/link" in files, "/etc/a/link should appear in the state as a non backup file" + assert not ("/etc/b/bar" in files), "/etc/b/bar should be a backup and not appear in files" + assert not ("/etc/b/link" in files), "/etc/b/link should be a backup and not appear in files" + backups = state['fileTree']['backedUpFiles'] + assert "/etc/b/bar" in backups, "/etc/b/bar should appear in the state as a backup file" + assert "/etc/b/link" in backups, "/etc/b/link should appear in the state as a backup file" + assert not ("/etc/a/bar" in backups), "/etc/a/bar should not appear in backups" + assert not ("/etc/a/link" in backups), "/etc/a/link should not appear in backups" + ''; + } +) diff --git a/testFlake/container-tests/state-v0-v1-migration.nix b/testFlake/container-tests/state-v0-v1-deactivate-migration.nix similarity index 98% rename from testFlake/container-tests/state-v0-v1-migration.nix rename to testFlake/container-tests/state-v0-v1-deactivate-migration.nix index 63d4a4bb..dd3d229e 100644 --- a/testFlake/container-tests/state-v0-v1-migration.nix +++ b/testFlake/container-tests/state-v0-v1-deactivate-migration.nix @@ -4,7 +4,7 @@ ... }: -forEachDistro "state-v0-v1-migration" ( +forEachDistro "state-v0-v1-migration-deactivate" ( let module = { environment.etc = { From 95cfd79dc5b5f2ed1f86d7f570b6fd1fcb635720 Mon Sep 17 00:00:00 2001 From: picnoir Date: Fri, 3 Apr 2026 10:58:44 +0200 Subject: [PATCH 09/18] tests: move system-manager v1.1.0 pin to test flake --- testFlake/container-tests/default.nix | 2 + .../state-v0-v1-activate-migration.nix | 4 +- .../state-v0-v1-deactivate-migration.nix | 4 +- testFlake/flake.lock | 173 +++++++++++++++++- testFlake/flake.nix | 3 + 5 files changed, 181 insertions(+), 5 deletions(-) diff --git a/testFlake/container-tests/default.nix b/testFlake/container-tests/default.nix index 37f8acce..59936c43 100644 --- a/testFlake/container-tests/default.nix +++ b/testFlake/container-tests/default.nix @@ -6,6 +6,7 @@ hostPkgs, nixpkgs, sops-nix, + system-manager-v1-1-0, }: let @@ -69,6 +70,7 @@ let lib hostPkgs sops-nix + system-manager-v1-1-0 ; }; testFiles = lib.filterAttrs (name: type: name != "default.nix" && lib.hasSuffix ".nix" name) ( diff --git a/testFlake/container-tests/state-v0-v1-activate-migration.nix b/testFlake/container-tests/state-v0-v1-activate-migration.nix index 728a8437..37e05566 100644 --- a/testFlake/container-tests/state-v0-v1-activate-migration.nix +++ b/testFlake/container-tests/state-v0-v1-activate-migration.nix @@ -1,6 +1,7 @@ { forEachDistro, system, + system-manager-v1-1-0, ... }: @@ -32,8 +33,7 @@ forEachDistro "state-v0-v1-migration-activate" ( }; }; }; - v0SystemManagerFlake = builtins.getFlake "github:numtide/system-manager/62a1be911f9e24bdb357377587c73544c2aec719"; - v0TopLevel = v0SystemManagerFlake.lib.makeSystemConfig { + v0TopLevel = system-manager-v1-1-0.lib.makeSystemConfig { modules = [ module { diff --git a/testFlake/container-tests/state-v0-v1-deactivate-migration.nix b/testFlake/container-tests/state-v0-v1-deactivate-migration.nix index dd3d229e..4d3301c0 100644 --- a/testFlake/container-tests/state-v0-v1-deactivate-migration.nix +++ b/testFlake/container-tests/state-v0-v1-deactivate-migration.nix @@ -1,6 +1,7 @@ { forEachDistro, system, + system-manager-v1-1-0, ... }: @@ -32,8 +33,7 @@ forEachDistro "state-v0-v1-migration-deactivate" ( }; }; }; - v0SystemManagerFlake = builtins.getFlake "github:numtide/system-manager/62a1be911f9e24bdb357377587c73544c2aec719"; - v0TopLevel = v0SystemManagerFlake.lib.makeSystemConfig { + v0TopLevel = system-manager-v1-1-0.lib.makeSystemConfig { modules = [ module { diff --git a/testFlake/flake.lock b/testFlake/flake.lock index ceedd2f9..5418b47d 100644 --- a/testFlake/flake.lock +++ b/testFlake/flake.lock @@ -16,6 +16,22 @@ "type": "github" } }, + "flake-compat_2": { + "flake": false, + "locked": { + "lastModified": 1767039857, + "narHash": "sha256-vNpUSpF5Nuw8xvDLj2KCwwksIbjua2LZCqhV1LNRDns=", + "owner": "edolstra", + "repo": "flake-compat", + "rev": "5edf11c44bc78a0d334f6334cdaf7d60d732daab", + "type": "github" + }, + "original": { + "owner": "edolstra", + "repo": "flake-compat", + "type": "github" + } + }, "flake-parts": { "inputs": { "nixpkgs-lib": [ @@ -38,6 +54,28 @@ "type": "github" } }, + "flake-parts_2": { + "inputs": { + "nixpkgs-lib": [ + "system-manager-v1-1-0", + "userborn", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1768135262, + "narHash": "sha256-PVvu7OqHBGWN16zSi6tEmPwwHQ4rLPU9Plvs8/1TUBY=", + "owner": "hercules-ci", + "repo": "flake-parts", + "rev": "80daad04eddbbf5a4d883996a73f3f542fa437ac", + "type": "github" + }, + "original": { + "owner": "hercules-ci", + "repo": "flake-parts", + "type": "github" + } + }, "gitignore": { "inputs": { "nixpkgs": [ @@ -61,6 +99,29 @@ "type": "github" } }, + "gitignore_2": { + "inputs": { + "nixpkgs": [ + "system-manager-v1-1-0", + "userborn", + "pre-commit-hooks-nix", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1709087332, + "narHash": "sha256-HG2cCnktfHsKV0s4XW83gU3F57gaTljL9KNSuG6bnQs=", + "owner": "hercules-ci", + "repo": "gitignore.nix", + "rev": "637db329424fd7e46cf4185293b9cc8c88c95394", + "type": "github" + }, + "original": { + "owner": "hercules-ci", + "repo": "gitignore.nix", + "type": "github" + } + }, "nix-vm-test": { "inputs": { "nixpkgs": [ @@ -97,6 +158,22 @@ "type": "github" } }, + "nixpkgs_2": { + "locked": { + "lastModified": 1772773019, + "narHash": "sha256-E1bxHxNKfDoQUuvriG71+f+s/NT0qWkImXsYZNFFfCs=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "aca4d95fce4914b3892661bcb80b8087293536c6", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, "pre-commit-hooks-nix": { "inputs": { "flake-compat": [ @@ -125,6 +202,34 @@ "type": "github" } }, + "pre-commit-hooks-nix_2": { + "inputs": { + "flake-compat": [ + "system-manager-v1-1-0", + "userborn", + "flake-compat" + ], + "gitignore": "gitignore_2", + "nixpkgs": [ + "system-manager-v1-1-0", + "userborn", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1769069492, + "narHash": "sha256-Efs3VUPelRduf3PpfPP2ovEB4CXT7vHf8W+xc49RL/U=", + "owner": "cachix", + "repo": "pre-commit-hooks.nix", + "rev": "a1ef738813b15cf8ec759bdff5761b027e3e1d23", + "type": "github" + }, + "original": { + "owner": "cachix", + "repo": "pre-commit-hooks.nix", + "type": "github" + } + }, "root": { "inputs": { "nix-vm-test": "nix-vm-test", @@ -133,7 +238,8 @@ "nixpkgs" ], "sops-nix": "sops-nix", - "system-manager": "system-manager" + "system-manager": "system-manager", + "system-manager-v1-1-0": "system-manager-v1-1-0" } }, "sops-nix": { @@ -172,6 +278,27 @@ }, "parent": [] }, + "system-manager-v1-1-0": { + "inputs": { + "flake-compat": "flake-compat_2", + "nixpkgs": "nixpkgs_2", + "userborn": "userborn_2" + }, + "locked": { + "lastModified": 1773316396, + "narHash": "sha256-r0/UDbEeYmVqhtxiuJSUfYhjBjtLKHDWhMScpe1RkOA=", + "owner": "numtide", + "repo": "system-manager", + "rev": "96cfa2041673dcc093cd06b1fe49a96a28203a13", + "type": "github" + }, + "original": { + "owner": "numtide", + "ref": "v1.1.0", + "repo": "system-manager", + "type": "github" + } + }, "systems": { "locked": { "lastModified": 1681028828, @@ -187,6 +314,21 @@ "type": "github" } }, + "systems_2": { + "locked": { + "lastModified": 1681028828, + "narHash": "sha256-Vy1rq5AaRuLzOxct8nz4T6wlgyUR7zLU309k9mBC768=", + "owner": "nix-systems", + "repo": "default", + "rev": "da67096a3b9bf56a91d16901293e51ba5b49a27e", + "type": "github" + }, + "original": { + "owner": "nix-systems", + "repo": "default", + "type": "github" + } + }, "userborn": { "inputs": { "flake-compat": [ @@ -215,6 +357,35 @@ "repo": "userborn", "type": "github" } + }, + "userborn_2": { + "inputs": { + "flake-compat": [ + "system-manager-v1-1-0", + "flake-compat" + ], + "flake-parts": "flake-parts_2", + "nixpkgs": [ + "system-manager-v1-1-0", + "nixpkgs" + ], + "pre-commit-hooks-nix": "pre-commit-hooks-nix_2", + "systems": "systems_2" + }, + "locked": { + "lastModified": 1770377964, + "narHash": "sha256-q2pnlX2IW0kg80GLFnwWd/GigIpkuZnyKPLhrgJql3E=", + "owner": "jfroche", + "repo": "userborn", + "rev": "55c2cd7952c207a62736a5bbd9499ea73da18d24", + "type": "github" + }, + "original": { + "owner": "jfroche", + "ref": "system-manager", + "repo": "userborn", + "type": "github" + } } }, "root": "root", diff --git a/testFlake/flake.nix b/testFlake/flake.nix index b717440e..d86837d8 100644 --- a/testFlake/flake.nix +++ b/testFlake/flake.nix @@ -8,6 +8,7 @@ inputs = { system-manager.url = "path:.."; + system-manager-v1-1-0.url = "github:numtide/system-manager/v1.1.0"; nixpkgs.follows = "system-manager/nixpkgs"; nix-vm-test = { url = "github:numtide/nix-vm-test"; @@ -24,6 +25,7 @@ nixpkgs, nix-vm-test, sops-nix, + system-manager-v1-1-0, }: let testedSystems = [ @@ -54,6 +56,7 @@ hostPkgs = nixpkgs.legacyPackages.${system}; inherit system-manager; inherit sops-nix; + inherit system-manager-v1-1-0; }; in { From e06811636eb6572807dd5b7252b978692f75e3a0 Mon Sep 17 00:00:00 2001 From: picnoir Date: Fri, 3 Apr 2026 10:59:01 +0200 Subject: [PATCH 10/18] Remove unused SYSTEM_MANAGER_STATIC_NAME --- crates/system-manager-engine/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/system-manager-engine/src/lib.rs b/crates/system-manager-engine/src/lib.rs index 07b90e0e..95a94c10 100644 --- a/crates/system-manager-engine/src/lib.rs +++ b/crates/system-manager-engine/src/lib.rs @@ -16,7 +16,6 @@ pub const PROFILE_NAME: &str = "system-manager"; pub const GCROOT_PATH: &str = "/nix/var/nix/gcroots/system-manager-current"; pub const SYSTEM_MANAGER_STATE_DIR: &str = "/var/lib/system-manager/state"; pub const STATE_FILE_NAME: &str = "system-manager-state.json"; -pub const SYSTEM_MANAGER_STATIC_NAME: &str = ".system-manager-static"; #[derive(PartialEq, Debug, Clone, Serialize, Deserialize)] #[serde(from = "String", into = "String", rename_all = "camelCase")] From acb1120f36d1e3c8941ec8da6d3094f177b99389 Mon Sep 17 00:00:00 2001 From: picnoir Date: Fri, 3 Apr 2026 11:18:12 +0200 Subject: [PATCH 11/18] tests: check activation logs for migration tests --- .../state-v0-v1-activate-migration.nix | 22 +++++++++++++++---- .../state-v0-v1-deactivate-migration.nix | 17 ++++++++++---- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/testFlake/container-tests/state-v0-v1-activate-migration.nix b/testFlake/container-tests/state-v0-v1-activate-migration.nix index 37e05566..395a89ff 100644 --- a/testFlake/container-tests/state-v0-v1-activate-migration.nix +++ b/testFlake/container-tests/state-v0-v1-activate-migration.nix @@ -8,11 +8,14 @@ forEachDistro "state-v0-v1-migration-activate" ( let module = { + # Required for v1.1.0. + nix.enable = false; + environment.etc = { "a/bar" = { text = "bar"; mode = "0700"; - user = "user"; + user = "root"; group = "root"; }; "a/link" = { @@ -22,7 +25,7 @@ forEachDistro "state-v0-v1-migration-activate" ( "b/bar" = { text = "bar"; mode = "0700"; - user = "user"; + user = "root"; group = "root"; replaceExisting = true; }; @@ -69,7 +72,11 @@ forEachDistro "state-v0-v1-migration-activate" ( assert file.contains(content), f"{path} should contain {content}" # Let's activate the profile with a v0 state file (using an old system-manager checkout) - machine.succeed("${v0TopLevel}/bin/activate") + activation_logs = machine.succeed("${v0TopLevel}/bin/activate") + for line in activation_logs.split("\n"): + assert not "ERROR" in line, line + machine.wait_for_unit("system-manager.target") + with subtest("Verify correct files are created"): check_file("/etc/a/bar", "bar") check_file("/etc/a/link", "link") @@ -77,7 +84,14 @@ forEachDistro "state-v0-v1-migration-activate" ( check_file("/etc/b/link", "link") # Let's try to deactivate the machine with the new binary, making sure the state migration works. - machine.succeed("${toplevel}/bin/activate") + activation_logs = machine.succeed("${toplevel}/bin/activate") + for line in activation_logs.split("\n"): + assert ((not "ERROR" in line) and (not "WARN" in line)), line + + # Check the state backup works + backup = machine.file("/var/lib/system-manager/state/system-manager-state.json.v0back") + assert backup.exists, "the v0 state should be backed up" + with subtest("v1 activation keeps the file and migrate the state to v1"): check_file("/etc/a/bar", "bar") check_file("/etc/a/link", "link") diff --git a/testFlake/container-tests/state-v0-v1-deactivate-migration.nix b/testFlake/container-tests/state-v0-v1-deactivate-migration.nix index 4d3301c0..db770b0f 100644 --- a/testFlake/container-tests/state-v0-v1-deactivate-migration.nix +++ b/testFlake/container-tests/state-v0-v1-deactivate-migration.nix @@ -8,11 +8,14 @@ forEachDistro "state-v0-v1-migration-deactivate" ( let module = { + # Required for v1.1.0. + nix.enable = false; + environment.etc = { "a/bar" = { text = "bar"; mode = "0700"; - user = "user"; + user = "root"; group = "root"; }; "a/link" = { @@ -22,7 +25,7 @@ forEachDistro "state-v0-v1-migration-deactivate" ( "b/bar" = { text = "bar"; mode = "0700"; - user = "user"; + user = "root"; group = "root"; replaceExisting = true; }; @@ -69,7 +72,11 @@ forEachDistro "state-v0-v1-migration-deactivate" ( assert file.contains(content), f"{path} should contain {content}" # Let's activate the profile with a v0 state file (using an old system-manager checkout) - machine.succeed("${v0TopLevel}/bin/activate") + activation_logs = machine.succeed("${v0TopLevel}/bin/activate") + for line in activation_logs.split("\n"): + assert not "ERROR" in line, line + machine.wait_for_unit("system-manager.target") + with subtest("Verify correct files are created"): check_file("/etc/a/bar", "bar") check_file("/etc/a/link", "link") @@ -77,7 +84,9 @@ forEachDistro "state-v0-v1-migration-deactivate" ( check_file("/etc/b/link", "link") # Let's try to deactivate the machine with the new binary, making sure the state migration works. - machine.succeed("${toplevel}/bin/deactivate") + deactivation_logs = machine.succeed("${toplevel}/bin/deactivate") + for line in activation_logs.split("\n"): + assert ((not "ERROR" in line) and (not "WARN" in line)), line with subtest("v1 deactivation restores the backups from a v0 generated state"): machine.succeed("test -f /etc/b/bar") machine.succeed("test -f /etc/b/link") From 8e0b84d44f48399acff28464c01a5c1293589969 Mon Sep 17 00:00:00 2001 From: picnoir Date: Fri, 3 Apr 2026 11:27:05 +0200 Subject: [PATCH 12/18] fix: remove existing links/files we're managing --- crates/system-manager-engine/src/activate/etc_files.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/system-manager-engine/src/activate/etc_files.rs b/crates/system-manager-engine/src/activate/etc_files.rs index ba1474d8..a2431526 100644 --- a/crates/system-manager-engine/src/activate/etc_files.rs +++ b/crates/system-manager-engine/src/activate/etc_files.rs @@ -493,7 +493,11 @@ fn copy_file( source: e.into(), } } - log::warn!("copy {} to {}", source.display(), target.display()); + if exists && old_state.contains(target) { + log::debug!("remove {}, we're managing it.", target.display()); + fs::remove_file(target).map_err(|e| to_activation_result(e, &new_state))?; + } + log::debug!("copy {} to {}", source.display(), target.display()); fs::copy(source, target).map_err(|e| to_activation_result(e, &new_state))?; let mode_int = u32::from_str_radix(&entry.mode, 8).map_err(|e| to_activation_result(e, &new_state))?; From 6ca765f5d3a44a935431ca56530a259baa983b62 Mon Sep 17 00:00:00 2001 From: picnoir Date: Fri, 3 Apr 2026 11:38:39 +0200 Subject: [PATCH 13/18] chrore: write log message after state migration --- crates/system-manager-engine/src/activate.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/system-manager-engine/src/activate.rs b/crates/system-manager-engine/src/activate.rs index ae4971c2..13e35e7f 100644 --- a/crates/system-manager-engine/src/activate.rs +++ b/crates/system-manager-engine/src/activate.rs @@ -96,9 +96,10 @@ impl StateV1 { e ) })?; + log::info!("The state is in the V0 format. Migrating it to the V1 format."); Ok(filetree.into()) } else { - // State is + // We don't know what that state is. Err(anyhow!("Unexpected serde_json error: {}", e)) } } From 9f8e80954d0492d0c9cb38e13a41cf02829f68a9 Mon Sep 17 00:00:00 2001 From: picnoir Date: Fri, 3 Apr 2026 11:38:58 +0200 Subject: [PATCH 14/18] fix: ignore directories during state migration We're not tracking those anymore. --- .../src/activate/etc_files/etc_tree.rs | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs b/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs index 91520db8..20fc32cd 100644 --- a/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs +++ b/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs @@ -59,15 +59,17 @@ impl From for EtcFilesState { for nested in elem.nested.clone() { paths_to_go.push(nested.1); } - match elem.status { - FileStatus::Managed => { - etc_files.files.insert(elem.path); - } - FileStatus::ManagedWithBackup => { - etc_files.backed_up_files.insert(elem.path); - } - FileStatus::Unmanaged => {} - }; + if !elem.path.is_dir() { + match elem.status { + FileStatus::Managed => { + etc_files.files.insert(elem.path); + } + FileStatus::ManagedWithBackup => { + etc_files.backed_up_files.insert(elem.path); + } + FileStatus::Unmanaged => {} + }; + } i += 1; } etc_files From a7d0c2abc877d4f9663c27d851d7fef6ccdac279 Mon Sep 17 00:00:00 2001 From: picnoir Date: Fri, 3 Apr 2026 11:43:34 +0200 Subject: [PATCH 15/18] fix: properly default statev1 version to 1. --- crates/system-manager-engine/src/activate.rs | 8 ++++---- .../src/activate/etc_files/etc_tree.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/system-manager-engine/src/activate.rs b/crates/system-manager-engine/src/activate.rs index 13e35e7f..39c481a1 100644 --- a/crates/system-manager-engine/src/activate.rs +++ b/crates/system-manager-engine/src/activate.rs @@ -160,14 +160,14 @@ pub fn activate(store_path: &StorePath, ephemeral: bool) -> Result<()> { Ok(services) => StateV1 { file_tree: etc_tree, services, - version: Default::default(), + version: 1, }, Err(ActivationError::WithPartialResult { result, source }) => { log::error!("Error during activation: {source:?}"); StateV1 { file_tree: etc_tree, services: result, - version: Default::default(), + version: 1, } } }; @@ -215,14 +215,14 @@ pub fn prepopulate(store_path: &StorePath, ephemeral: bool) -> Result<()> { Ok(services) => StateV1 { file_tree: etc_tree, services, - version: Default::default(), + version: 1, }, Err(ActivationError::WithPartialResult { result, source }) => { log::error!("Error during activation: {source:?}"); StateV1 { file_tree: etc_tree, services: result, - version: Default::default(), + version: 1, } } } diff --git a/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs b/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs index 20fc32cd..7b1ba5f4 100644 --- a/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs +++ b/crates/system-manager-engine/src/activate/etc_files/etc_tree.rs @@ -22,7 +22,7 @@ impl From for StateV1 { StateV1 { file_tree, services, - version: Default::default(), + version: 1, } } } From db15427f70547e81c51a4a5690689c6064767a4a Mon Sep 17 00:00:00 2001 From: picnoir Date: Fri, 3 Apr 2026 15:31:36 +0200 Subject: [PATCH 16/18] state: backup v0 state before migration Better be safe than sorry. --- crates/system-manager-engine/src/activate.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/system-manager-engine/src/activate.rs b/crates/system-manager-engine/src/activate.rs index 39c481a1..fce3d8da 100644 --- a/crates/system-manager-engine/src/activate.rs +++ b/crates/system-manager-engine/src/activate.rs @@ -97,6 +97,14 @@ impl StateV1 { ) })?; log::info!("The state is in the V0 format. Migrating it to the V1 format."); + // Backup the old state, just in case. Better be safe than sorry. + let mut backup_path = state_file.to_owned(); + backup_path.add_extension("v0back"); + log::info!( + "Create a backup of the v0 state at {}.", + &backup_path.display() + ); + fs::copy(state_file, backup_path)?; Ok(filetree.into()) } else { // We don't know what that state is. From 86d8059c76c5a36858dbe6d415636862c7d54ce6 Mon Sep 17 00:00:00 2001 From: picnoir Date: Tue, 7 Apr 2026 13:21:34 +0200 Subject: [PATCH 17/18] etc_files: support again --ephemeral I forgot to wire the ephemeral flag through the etc files creation. When this flag is set, we're linking the etc files to /run/etc instead of /etc. --- .../src/activate/etc_files.rs | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/crates/system-manager-engine/src/activate/etc_files.rs b/crates/system-manager-engine/src/activate/etc_files.rs index a2431526..96bf503d 100644 --- a/crates/system-manager-engine/src/activate/etc_files.rs +++ b/crates/system-manager-engine/src/activate/etc_files.rs @@ -124,7 +124,7 @@ pub fn activate( .collect(); entries.append(&mut non_static_entries); // Create dirs and link/copy entries - new_state = create_etc_files(entries, new_state.clone(), &old_state)?; + new_state = create_etc_files(entries, new_state.clone(), &old_state, &etc_dir)?; // Delete unecessary files let files_to_delete: HashSet = old_state .files @@ -277,6 +277,26 @@ fn list_static_entries(config_entries: &EtcFilesConfig) -> anyhow::Result, + mut state: EtcFilesState, + old_state: &EtcFilesState, + etc_dir: &Path, +) -> EtcActivationResult { + files.sort_by(|a, b| a.target.cmp(&b.target)); + for file in files { + let target = file.target.clone(); + state = match create_etc_file(file, state, old_state, etc_dir) { + Ok(state) => state, + Err(ActivationError::WithPartialResult { result, source }) => { + log::warn!("Can't link/copy {} to : {}", target.display(), source); + result + } + } + } + Ok(state) +} + /// Create a single etc file. /// /// We separated this from `create_etc_files` to catch any error on a file boundary @@ -285,8 +305,9 @@ fn create_etc_file( file: EtcFile, mut state: EtcFilesState, old_state: &EtcFilesState, + etc_dir: &Path, ) -> EtcActivationResult { - let target = PathBuf::from("/etc").join(&file.target); + let target = PathBuf::from(etc_dir).join(&file.target); log::debug!( "Creating {} to {} ({})", file.source, @@ -360,25 +381,6 @@ fn create_etc_file( Ok(state) } -fn create_etc_files( - mut files: Vec, - mut state: EtcFilesState, - old_state: &EtcFilesState, -) -> EtcActivationResult { - files.sort_by(|a, b| a.target.cmp(&b.target)); - for file in files { - let target = file.target.clone(); - state = match create_etc_file(file, state, old_state) { - Ok(state) => state, - Err(ActivationError::WithPartialResult { result, source }) => { - log::warn!("Can't link/copy {} to : {}", target.display(), source); - result - } - } - } - Ok(state) -} - fn backup_and_link( target: &Path, link_path: &Path, From c8046c56d48265b6bb4d248deaf9c4df54f08acb Mon Sep 17 00:00:00 2001 From: picnoir Date: Tue, 7 Apr 2026 13:29:20 +0200 Subject: [PATCH 18/18] etc_files tests: activate profile twice in deactivation test Activating the profile twice, we make sure the backed up files are correctly saved in the state. --- testFlake/container-tests/state-v0-v1-deactivate-migration.nix | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testFlake/container-tests/state-v0-v1-deactivate-migration.nix b/testFlake/container-tests/state-v0-v1-deactivate-migration.nix index db770b0f..9318e2cd 100644 --- a/testFlake/container-tests/state-v0-v1-deactivate-migration.nix +++ b/testFlake/container-tests/state-v0-v1-deactivate-migration.nix @@ -83,6 +83,9 @@ forEachDistro "state-v0-v1-migration-deactivate" ( check_file("/etc/b/bar", "bar") check_file("/etc/b/link", "link") + # Let's activate the profile with a v0 state file (using an old system-manager checkout) + activation_logs = machine.succeed("${v0TopLevel}/bin/activate") + # Let's try to deactivate the machine with the new binary, making sure the state migration works. deactivation_logs = machine.succeed("${toplevel}/bin/deactivate") for line in activation_logs.split("\n"):