fix hot reloading /etc/niri/config.kdl (#1907)

* refactor config load logic, and properly watch the system config path

* move config creation to niri-config, and make the errors a bit nicer

notably, "error creating config" is now a cause for "error loading
config", instead of it being one error and then "error loading config:
no such file or directory". also, failure to load a config is now
printed as an error level diagnostic (because it is indeed an error, not
just a warning you can shrug off)

* refactor watcher tests; add some new ones

now they check for the file contents too! and i added some tests for
ConfigPath::Regular, including a messy one with many symlink swaps

* fixes

---------

Co-authored-by: Ivan Molodetskikh <yalterz@gmail.com>

niri-config/src/lib.rs

@@ -3,6 +3,8 @@ extern crate tracing;
 
 use std::collections::HashSet;
 use std::ffi::OsStr;
+use std::fs::{self, File};
+use std::io::Write;
 use std::ops::{Mul, MulAssign};
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
@@ -2368,14 +2370,118 @@ pub enum PreviewRender {
     ScreenCapture,
 }
 
-impl Config {
+#[derive(Debug, Clone)]
-    pub fn load(path: &Path) -> miette::Result<Self> {
+pub enum ConfigPath {
-        let _span = tracy_client::span!("Config::load");
+    /// Explicitly set config path.
-        Self::load_internal(path).context("error loading config")
+    ///
+    /// Load the config only from this path, never create it.
+    Explicit(PathBuf),
+
+    /// Default config path.
+    ///
+    /// Prioritize the user path, fallback to the system path, fallback to creating the user path
+    /// at compositor startup.
+    Regular {
+        /// User config path, usually `$XDG_CONFIG_HOME/niri/config.kdl`.
+        user_path: PathBuf,
+        /// System config path, usually `/etc/niri/config.kdl`.
+        system_path: PathBuf,
+    },
 }
 
-    fn load_internal(path: &Path) -> miette::Result<Self> {
+impl ConfigPath {
-        let contents = std::fs::read_to_string(path)
+    /// Load the config, or return an error if it doesn't exist.
+    pub fn load(&self) -> miette::Result<Config> {
+        let _span = tracy_client::span!("ConfigPath::load");
+
+        self.load_inner(|user_path, system_path| {
+            Err(miette::miette!(
+                "no config file found; create one at {user_path:?} or {system_path:?}",
+            ))
+        })
+        .context("error loading config")
+    }
+
+    /// Load the config, or create it if it doesn't exist.
+    ///
+    /// Returns a tuple containing the path that was created, if any, and the loaded config.
+    ///
+    /// If the config was created, but for some reason could not be read afterwards,
+    /// this may return `(Some(_), Err(_))`.
+    pub fn load_or_create(&self) -> (Option<&Path>, miette::Result<Config>) {
+        let _span = tracy_client::span!("ConfigPath::load_or_create");
+
+        let mut created_at = None;
+
+        let result = self
+            .load_inner(|user_path, _| {
+                Self::create(user_path, &mut created_at)
+                    .map(|()| user_path)
+                    .with_context(|| format!("error creating config at {user_path:?}"))
+            })
+            .context("error loading config");
+
+        (created_at, result)
+    }
+
+    fn load_inner<'a>(
+        &'a self,
+        maybe_create: impl FnOnce(&'a Path, &'a Path) -> miette::Result<&'a Path>,
+    ) -> miette::Result<Config> {
+        let path = match self {
+            ConfigPath::Explicit(path) => path.as_path(),
+            ConfigPath::Regular {
+                user_path,
+                system_path,
+            } => {
+                if user_path.exists() {
+                    user_path.as_path()
+                } else if system_path.exists() {
+                    system_path.as_path()
+                } else {
+                    maybe_create(user_path.as_path(), system_path.as_path())?
+                }
+            }
+        };
+        Config::load(path)
+    }
+
+    fn create<'a>(path: &'a Path, created_at: &mut Option<&'a Path>) -> miette::Result<()> {
+        if let Some(default_parent) = path.parent() {
+            fs::create_dir_all(default_parent)
+                .into_diagnostic()
+                .with_context(|| format!("error creating config directory {default_parent:?}"))?;
+        }
+
+        // Create the config and fill it with the default config if it doesn't exist.
+        let mut new_file = match File::options()
+            .read(true)
+            .write(true)
+            .create_new(true)
+            .open(path)
+        {
+            Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => return Ok(()),
+            res => res,
+        }
+        .into_diagnostic()
+        .with_context(|| format!("error opening config file at {path:?}"))?;
+
+        *created_at = Some(path);
+
+        let default = include_bytes!("../../resources/default-config.kdl");
+
+        new_file
+            .write_all(default)
+            .into_diagnostic()
+            .with_context(|| format!("error writing default config to {path:?}"))?;
+
+        Ok(())
+    }
+}
+
+impl Config {
+    pub fn load(path: &Path) -> miette::Result<Self> {
+        let contents = fs::read_to_string(path)
             .into_diagnostic()
             .with_context(|| format!("error reading {path:?}"))?;
 

src/main.rs

@@ -2,10 +2,10 @@
 extern crate tracing;
 
 use std::fmt::Write as _;
-use std::fs::{self, File};
+use std::fs::File;
 use std::io::{self, Write};
 use std::os::fd::FromRawFd;
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 use std::process::Command;
 use std::{env, mem};
 
@@ -25,7 +25,7 @@ use niri::utils::spawning::{
 };
 use niri::utils::watcher::Watcher;
 use niri::utils::{cause_panic, version, xwayland, IS_SYSTEMD_SERVICE};
-use niri_config::Config;
+use niri_config::ConfigPath;
 use niri_ipc::socket::SOCKET_PATH_ENV;
 use portable_atomic::Ordering;
 use sd_notify::NotifyState;
@@ -99,8 +99,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             Sub::Validate { config } => {
                 tracy_client::Client::start();
 
-                let (path, _, _) = config_path(config);
+                config_path(config).load()?;
-                Config::load(&path)?;
                 info!("config is valid");
                 return Ok(());
             }
@@ -144,47 +143,9 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     info!("starting version {}", &version());
 
     // Load the config.
-    let mut config_created = false;
+    let config_path = config_path(cli.config);
-    let (path, watch_path, create_default) = config_path(cli.config);
     env::remove_var("NIRI_CONFIG");
-    if create_default {
+    let (config_created_at, config_load_result) = config_path.load_or_create();
-        let default_parent = path.parent().unwrap();
-
-        match fs::create_dir_all(default_parent) {
-            Ok(()) => {
-                // Create the config and fill it with the default config if it doesn't exist.
-                let new_file = File::options()
-                    .read(true)
-                    .write(true)
-                    .create_new(true)
-                    .open(&path);
-                match new_file {
-                    Ok(mut new_file) => {
-                        let default = include_bytes!("../resources/default-config.kdl");
-                        match new_file.write_all(default) {
-                            Ok(()) => {
-                                config_created = true;
-                                info!("wrote default config to {:?}", &path);
-                            }
-                            Err(err) => {
-                                warn!("error writing config file at {:?}: {err:?}", &path)
-                            }
-                        }
-                    }
-                    Err(err) if err.kind() == io::ErrorKind::AlreadyExists => {}
-                    Err(err) => warn!("error creating config file at {:?}: {err:?}", &path),
-                }
-            }
-            Err(err) => {
-                warn!(
-                    "error creating config directories {:?}: {err:?}",
-                    default_parent
-                );
-            }
-        }
-    }
-
-    let config_load_result = Config::load(&path);
     let config_errored = config_load_result.is_err();
     let mut config = config_load_result
         .map_err(|err| warn!("{err:?}"))
@@ -273,14 +234,14 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     let _watcher = {
         // Parsing the config actually takes > 20 ms on my beefy machine, so let's do it on the
         // watcher thread.
-        let process = |path: &Path| {
+        let process = |path: &ConfigPath| {
-            Config::load(path).map_err(|err| {
+            path.load().map_err(|err| {
-                warn!("{:?}", err.context("error loading config"));
+                warn!("{err:?}");
             })
         };
 
         let (tx, rx) = calloop::channel::sync_channel(1);
-        let watcher = Watcher::new(watch_path.clone(), process, tx);
+        let watcher = Watcher::new(config_path.clone(), process, tx);
         event_loop
             .handle()
             .insert_source(rx, |event, _, state| match event {
@@ -301,7 +262,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     // Show the config error notification right away if needed.
     if config_errored {
         state.niri.config_error_notification.show();
-    } else if config_created {
+    } else if let Some(path) = config_created_at {
         state.niri.config_error_notification.show_created(path);
     }
 
@@ -385,26 +346,21 @@ fn system_config_path() -> PathBuf {
     PathBuf::from("/etc/niri/config.kdl")
 }
 
-/// Resolves and returns the config path to load, the config path to watch, and whether to create
+fn config_path(cli_path: Option<PathBuf>) -> ConfigPath {
-/// the default config at the path to load.
-fn config_path(cli_path: Option<PathBuf>) -> (PathBuf, PathBuf, bool) {
     if let Some(explicit) = cli_path.or_else(env_config_path) {
-        return (explicit.clone(), explicit, false);
+        return ConfigPath::Explicit(explicit);
     }
 
     let system_path = system_config_path();
-    if let Some(path) = default_config_path() {
-        if path.exists() {
-            return (path.clone(), path, false);
-        }
 
-        if system_path.exists() {
+    if let Some(user_path) = default_config_path() {
-            (system_path, path, false)
+        ConfigPath::Regular {
-        } else {
+            user_path,
-            (path.clone(), path, true)
+            system_path,
         }
     } else {
-        (system_path.clone(), system_path, false)
+        // Couldn't find the home directory, or whatever.
+        ConfigPath::Explicit(system_path)
     }
 }
 

src/ui/config_error_notification.rs

@@ -68,10 +68,9 @@ impl ConfigErrorNotification {
         )
     }
 
-    pub fn show_created(&mut self, created_path: PathBuf) {
+    pub fn show_created(&mut self, created_path: &Path) {
-        let created_path = Some(created_path);
+        if self.created_path.as_deref() != Some(created_path) {
-        if self.created_path != created_path {
+            self.created_path = Some(created_path.to_owned());
-            self.created_path = created_path;
             self.buffers.borrow_mut().clear();
         }
 

src/utils/watcher.rs

@@ -3,9 +3,10 @@
 use std::path::{Path, PathBuf};
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{mpsc, Arc};
-use std::thread;
+use std::time::{Duration, SystemTime};
-use std::time::Duration;
+use std::{io, thread};
 
+use niri_config::ConfigPath;
 use smithay::reexports::calloop::channel::SyncSender;
 
 pub struct Watcher {
@@ -20,58 +21,76 @@ impl Drop for Watcher {
 
 impl Watcher {
     pub fn new<T: Send + 'static>(
-        path: PathBuf,
+        path: ConfigPath,
-        process: impl FnMut(&Path) -> T + Send + 'static,
+        process: impl FnMut(&ConfigPath) -> T + Send + 'static,
         changed: SyncSender<T>,
     ) -> Self {
-        Self::with_start_notification(path, process, changed, None)
+        let interval = Duration::from_millis(500);
+        Self::with_start_notification(path, process, changed, None, interval)
     }
 
     pub fn with_start_notification<T: Send + 'static>(
-        path: PathBuf,
+        config_path: ConfigPath,
-        mut process: impl FnMut(&Path) -> T + Send + 'static,
+        mut process: impl FnMut(&ConfigPath) -> T + Send + 'static,
         changed: SyncSender<T>,
         started: Option<mpsc::SyncSender<()>>,
+        polling_interval: Duration,
     ) -> Self {
         let should_stop = Arc::new(AtomicBool::new(false));
 
         {
             let should_stop = should_stop.clone();
             thread::Builder::new()
-                .name(format!("Filesystem Watcher for {}", path.to_string_lossy()))
+                .name(format!("Filesystem Watcher for {config_path:?}"))
                 .spawn(move || {
-                    // this "should" be as simple as mtime, but it does not quite work in practice;
+                    // this "should" be as simple as storing the last seen mtime,
-                    // it doesn't work if the config is a symlink, and its target changes but the
+                    // and if the contents change without updating mtime, we ignore it.
-                    // new target and old target have identical mtimes.
                     //
-                    // in practice, this does not occur on any systems other than nix.
+                    // but that breaks if the config is a symlink, and its target
-                    // because, on nix practically everything is a symlink to /nix/store
+                    // changes but the new target and old target have identical mtimes.
-                    // and due to reproducibility, /nix/store keeps no mtime (= 1970-01-01)
+                    // in which case we should *not* ignore it; this is an entirely different file.
+                    //
+                    // in practice, this edge case does not occur on systems other than nix.
+                    // because, on nix, everything is a symlink to /nix/store
+                    // and /nix/store keeps no mtime (= 1970-01-01)
                     // so, symlink targets change frequently when mtime doesn't.
-                    let mut last_props = path
+                    //
-                        .canonicalize()
+                    // therefore, we must also store the canonical path, along with its mtime
-                        .and_then(|canon| Ok((canon.metadata()?.modified()?, canon)))
+
-                        .ok();
+                    fn see_path(path: &Path) -> io::Result<(SystemTime, PathBuf)> {
+                        let canon = path.canonicalize()?;
+                        let mtime = canon.metadata()?.modified()?;
+                        Ok((mtime, canon))
+                    }
+
+                    fn see(config_path: &ConfigPath) -> io::Result<(SystemTime, PathBuf)> {
+                        match config_path {
+                            ConfigPath::Explicit(path) => see_path(path),
+                            ConfigPath::Regular {
+                                user_path,
+                                system_path,
+                            } => see_path(user_path).or_else(|_| see_path(system_path)),
+                        }
+                    }
+
+                    let mut last_props = see(&config_path).ok();
 
                     if let Some(started) = started {
                         let _ = started.send(());
                     }
 
                     loop {
-                        thread::sleep(Duration::from_millis(500));
+                        thread::sleep(polling_interval);
 
                         if should_stop.load(Ordering::SeqCst) {
                             break;
                         }
 
-                        if let Ok(new_props) = path
+                        if let Ok(new_props) = see(&config_path) {
-                            .canonicalize()
-                            .and_then(|canon| Ok((canon.metadata()?.modified()?, canon)))
-                        {
                             if last_props.as_ref() != Some(&new_props) {
-                                trace!("file changed: {}", path.to_string_lossy());
+                                trace!("config file changed");
 
-                                let rv = process(&path);
+                                let rv = process(&config_path);
 
                                 if let Err(err) = changed.send(rv) {
                                     warn!("error sending change notification: {err:?}");
@@ -83,7 +102,7 @@ impl Watcher {
                         }
                     }
 
-                    debug!("exiting watcher thread for {}", path.to_string_lossy());
+                    debug!("exiting watcher thread for {config_path:?}");
                 })
                 .unwrap();
         }
@@ -95,262 +114,489 @@ impl Watcher {
 #[cfg(test)]
 mod tests {
     use std::error::Error;
-    use std::fs::File;
+    use std::fs::{self, File, FileTimes};
     use std::io::Write;
-    use std::sync::atomic::AtomicU8;
 
-    use calloop::channel::sync_channel;
+    use calloop::channel::{sync_channel, Event};
     use calloop::EventLoop;
-    use smithay::reexports::rustix::fs::{futimens, Timestamps};
+    use xshell::{cmd, Shell, TempDir};
-    use smithay::reexports::rustix::time::Timespec;
-    use xshell::{cmd, Shell};
 
     use super::*;
 
-    fn check(
+    type Result<T = (), E = Box<dyn Error>> = std::result::Result<T, E>;
-        setup: impl FnOnce(&Shell) -> Result<(), Box<dyn Error>>,
+
-        change: impl FnOnce(&Shell) -> Result<(), Box<dyn Error>>,
+    fn canon(config_path: &ConfigPath) -> &PathBuf {
-    ) {
+        match config_path {
+            ConfigPath::Explicit(path) => path,
+            ConfigPath::Regular {
+                user_path,
+                system_path,
+            } => {
+                if user_path.exists() {
+                    user_path
+                } else {
+                    system_path
+                }
+            }
+        }
+    }
+
+    enum TestPath<P> {
+        Explicit(P),
+        Regular { user_path: P, system_path: P },
+    }
+
+    impl<P: AsRef<Path>> TestPath<P> {
+        fn setup<Discard>(
+            self,
+            setup: impl FnOnce(&Shell) -> xshell::Result<Discard>,
+        ) -> TestSetup {
+            self.setup_any(|sh| {
+                _ = setup(sh)?;
+                Ok(())
+            })
+        }
+
+        fn without_setup(self) -> TestSetup {
+            self.setup_any(|_| Ok(())).assert_initial_not_exists()
+        }
+
+        fn setup_any(self, setup: impl FnOnce(&Shell) -> Result) -> TestSetup {
             let sh = Shell::new().unwrap();
             let temp_dir = sh.create_temp_dir().unwrap();
             sh.change_dir(temp_dir.path());
-        // let dir = sh.create_dir("xshell").unwrap();
-        // sh.change_dir(dir);
 
-        let mut config_path = sh.current_dir();
+            let dir = sh.current_dir();
-        config_path.push("niri");
+            let config_path = match self {
-        config_path.push("config.kdl");
+                TestPath::Explicit(path) => ConfigPath::Explicit(dir.join(path)),
+                TestPath::Regular {
+                    user_path,
+                    system_path,
+                } => ConfigPath::Regular {
+                    user_path: dir.join(user_path),
+                    system_path: dir.join(system_path),
+                },
+            };
 
             setup(&sh).unwrap();
 
-        let changed = AtomicU8::new(0);
+            TestSetup {
+                sh,
+                config_path,
+                _temp_dir: temp_dir,
+            }
+        }
+    }
 
-        let mut event_loop = EventLoop::try_new().unwrap();
+    struct TestSetup {
-        let loop_handle = event_loop.handle();
+        sh: Shell,
+        config_path: ConfigPath,
+        _temp_dir: TempDir,
+    }
+
+    impl TestSetup {
+        fn assert_initial_not_exists(self) -> Self {
+            let canon = canon(&self.config_path);
+            assert!(!canon.exists(), "initial should not exist");
+            self
+        }
+
+        fn assert_initial(self, expected: &str) -> Self {
+            let canon = canon(&self.config_path);
+            assert!(canon.exists(), "initial should exist at {canon:?}");
+            let actual = fs::read_to_string(canon).unwrap();
+            assert_eq!(actual, expected, "initial file contents do not match");
+            self
+        }
+
+        fn run(self, body: impl FnOnce(&Shell, &mut TestUtil) -> Result) -> Result {
+            let TestSetup {
+                sh, config_path, ..
+            } = self;
 
             let (tx, rx) = sync_channel(1);
             let (started_tx, started_rx) = mpsc::sync_channel(1);
-        let _watcher =
-            Watcher::with_start_notification(config_path.clone(), |_| (), tx, Some(started_tx));
-        loop_handle
-            .insert_source(rx, |_, _, _| {
-                changed.fetch_add(1, Ordering::SeqCst);
-            })
-            .unwrap();
-        started_rx.recv().unwrap();
 
-        // HACK: if we don't sleep, files might have the same mtime.
+            let _watcher = Watcher::with_start_notification(
-        thread::sleep(Duration::from_millis(100));
+                config_path,
+                |config_path| canon(config_path).clone(),
+                tx,
+                Some(started_tx),
+                Duration::from_millis(100),
+            );
 
-        change(&sh).unwrap();
+            started_rx.recv()?;
 
+            let event_loop = EventLoop::try_new()?;
             event_loop
-            .dispatch(Duration::from_millis(750), &mut ())
+                .handle()
+                .insert_source(rx, |event, (), latest_path| {
+                    if let Event::Msg(path) = event {
+                        *latest_path = Some(path);
+                    }
+                })?;
+
+            let mut test = TestUtil { event_loop };
+
+            // don't trigger before we start
+            test.assert_unchanged();
+            // pass_time() inside assert_unchanged() ensures that mtime
+            // isn't the same as the initial time
+
+            body(&sh, &mut test)?;
+
+            // nothing should trigger after the test runs
+            test.assert_unchanged();
+
+            Ok(())
+        }
+    }
+
+    struct TestUtil<'a> {
+        event_loop: EventLoop<'a, Option<PathBuf>>,
+    }
+
+    impl<'a> TestUtil<'a> {
+        fn pass_time(&self) {
+            thread::sleep(Duration::from_millis(50));
+        }
+
+        fn assert_unchanged(&mut self) {
+            let mut new_path = None;
+            self.event_loop
+                .dispatch(Duration::from_millis(150), &mut new_path)
                 .unwrap();
+            assert_eq!(
+                new_path, None,
+                "watcher should not have noticed any changes"
+            );
 
-        assert_eq!(changed.load(Ordering::SeqCst), 1);
+            self.pass_time();
+        }
 
-        // Verify that the watcher didn't break.
+        fn assert_changed_to(&mut self, expected: &str) {
-        sh.write_file(&config_path, "c").unwrap();
+            let mut new_path = None;
-
+            self.event_loop
-        event_loop
+                .dispatch(Duration::from_millis(150), &mut new_path)
-            .dispatch(Duration::from_millis(750), &mut ())
                 .unwrap();
+            let Some(new_path) = new_path else {
+                panic!("watcher should have noticed a change, but it didn't");
+            };
+            let actual = fs::read_to_string(&new_path).unwrap();
+            assert_eq!(actual, expected, "watcher gave the wrong file");
 
-        assert_eq!(changed.load(Ordering::SeqCst), 2);
+            self.pass_time();
+        }
     }
 
     #[test]
-    fn change_file() {
+    fn change_file() -> Result {
-        check(
+        TestPath::Explicit("niri/config.kdl")
-            |sh| {
+            .setup(|sh| sh.write_file("niri/config.kdl", "a"))
-                sh.write_file("niri/config.kdl", "a")?;
+            .assert_initial("a")
-                Ok(())
+            .run(|sh, test| {
-            },
-            |sh| {
                 sh.write_file("niri/config.kdl", "b")?;
+                test.assert_changed_to("b");
+
                 Ok(())
-            },
+            })
-        );
     }
 
     #[test]
-    fn create_file() {
+    fn overwrite_but_dont_change_file() -> Result {
-        check(
+        TestPath::Explicit("niri/config.kdl")
-            |sh| {
+            .setup(|sh| sh.write_file("niri/config.kdl", "a"))
-                sh.create_dir("niri")?;
+            .assert_initial("a")
-                Ok(())
+            .run(|sh, test| {
-            },
-            |sh| {
                 sh.write_file("niri/config.kdl", "a")?;
+                test.assert_changed_to("a");
+
                 Ok(())
-            },
+            })
-        );
     }
 
     #[test]
-    fn create_dir_and_file() {
+    fn touch_file() -> Result {
-        check(
+        TestPath::Explicit("niri/config.kdl")
-            |_sh| Ok(()),
+            .setup(|sh| sh.write_file("niri/config.kdl", "a"))
-            |sh| {
+            .assert_initial("a")
+            .run(|sh, test| {
+                cmd!(sh, "touch niri/config.kdl").run()?;
+                test.assert_changed_to("a");
+
+                Ok(())
+            })
+    }
+
+    #[test]
+    fn create_file() -> Result {
+        TestPath::Explicit("niri/config.kdl")
+            .setup(|sh| sh.create_dir("niri"))
+            .assert_initial_not_exists()
+            .run(|sh, test| {
                 sh.write_file("niri/config.kdl", "a")?;
+                test.assert_changed_to("a");
+
                 Ok(())
-            },
+            })
-        );
     }
 
     #[test]
-    fn change_linked_file() {
+    fn create_dir_and_file() -> Result {
-        check(
+        TestPath::Explicit("niri/config.kdl")
-            |sh| {
+            .without_setup()
+            .run(|sh, test| {
+                sh.write_file("niri/config.kdl", "a")?;
+                test.assert_changed_to("a");
+
+                Ok(())
+            })
+    }
+
+    #[test]
+    fn change_linked_file() -> Result {
+        TestPath::Explicit("niri/config.kdl")
+            .setup(|sh| {
                 sh.write_file("niri/config2.kdl", "a")?;
-                cmd!(sh, "ln -s config2.kdl niri/config.kdl").run()?;
+                cmd!(sh, "ln -sf config2.kdl niri/config.kdl").run()
-                Ok(())
+            })
-            },
+            .assert_initial("a")
-            |sh| {
+            .run(|sh, test| {
                 sh.write_file("niri/config2.kdl", "b")?;
+                test.assert_changed_to("b");
+
                 Ok(())
-            },
+            })
-        );
     }
 
     #[test]
-    fn change_file_in_linked_dir() {
+    fn change_file_in_linked_dir() -> Result {
-        check(
+        TestPath::Explicit("niri/config.kdl")
-            |sh| {
+            .setup(|sh| {
                 sh.write_file("niri2/config.kdl", "a")?;
-                cmd!(sh, "ln -s niri2 niri").run()?;
+                cmd!(sh, "ln -s niri2 niri").run()
-                Ok(())
+            })
-            },
+            .assert_initial("a")
-            |sh| {
+            .run(|sh, test| {
                 sh.write_file("niri2/config.kdl", "b")?;
+                test.assert_changed_to("b");
+
                 Ok(())
-            },
+            })
-        );
     }
 
     #[test]
-    fn recreate_file() {
+    fn remove_file() -> Result {
-        check(
+        TestPath::Explicit("niri/config.kdl")
-            |sh| {
+            .setup(|sh| sh.write_file("niri/config.kdl", "a"))
-                sh.write_file("niri/config.kdl", "a")?;
+            .assert_initial("a")
+            .run(|sh, test| {
+                sh.remove_path("niri/config.kdl")?;
+                test.assert_unchanged();
+
                 Ok(())
-            },
+            })
-            |sh| {
+    }
+
+    #[test]
+    fn remove_dir() -> Result {
+        TestPath::Explicit("niri/config.kdl")
+            .setup(|sh| sh.write_file("niri/config.kdl", "a"))
+            .assert_initial("a")
+            .run(|sh, test| {
+                sh.remove_path("niri")?;
+                test.assert_unchanged();
+
+                Ok(())
+            })
+    }
+
+    #[test]
+    fn recreate_file() -> Result {
+        TestPath::Explicit("niri/config.kdl")
+            .setup(|sh| sh.write_file("niri/config.kdl", "a"))
+            .assert_initial("a")
+            .run(|sh, test| {
                 sh.remove_path("niri/config.kdl")?;
                 sh.write_file("niri/config.kdl", "b")?;
+                test.assert_changed_to("b");
+
                 Ok(())
-            },
+            })
-        );
     }
 
     #[test]
-    fn recreate_dir() {
+    fn recreate_dir() -> Result {
-        check(
+        TestPath::Explicit("niri/config.kdl")
-            |sh| {
+            .setup(|sh| {
                 sh.write_file("niri/config.kdl", "a")?;
                 Ok(())
-            },
+            })
-            |sh| {
+            .assert_initial("a")
+            .run(|sh, test| {
                 sh.remove_path("niri")?;
                 sh.write_file("niri/config.kdl", "b")?;
+                test.assert_changed_to("b");
+
                 Ok(())
-            },
+            })
-        );
     }
 
     #[test]
-    fn swap_dir() {
+    fn swap_dir() -> Result {
-        check(
+        TestPath::Explicit("niri/config.kdl")
-            |sh| {
+            .setup(|sh| sh.write_file("niri/config.kdl", "a"))
-                sh.write_file("niri/config.kdl", "a")?;
+            .assert_initial("a")
-                Ok(())
+            .run(|sh, test| {
-            },
-            |sh| {
                 sh.write_file("niri2/config.kdl", "b")?;
                 sh.remove_path("niri")?;
                 cmd!(sh, "mv niri2 niri").run()?;
+                test.assert_changed_to("b");
+
                 Ok(())
-            },
+            })
-        );
     }
 
     #[test]
-    fn swap_just_link() {
+    fn swap_dir_link() -> Result {
-        // NixOS setup: link path changes, mtime stays constant.
+        TestPath::Explicit("niri/config.kdl")
-        check(
+            .setup(|sh| {
-            |sh| {
+                sh.write_file("niri2/config.kdl", "a")?;
-                let mut dir = sh.current_dir();
+                cmd!(sh, "ln -s niri2 niri").run()
-                dir.push("niri");
+            })
+            .assert_initial("a")
+            .run(|sh, test| {
+                sh.write_file("niri3/config.kdl", "b")?;
+                sh.remove_path("niri")?;
+                cmd!(sh, "ln -s niri3 niri").run()?;
+                test.assert_changed_to("b");
+
+                Ok(())
+            })
+    }
+
+    // Important: On systems like NixOS, mtime is not kept for config files.
+    // So, this is testing that the watcher handles that correctly.
+    fn create_epoch(path: impl AsRef<Path>, content: &str) -> Result {
+        let mut file = File::create(path)?;
+        file.write_all(content.as_bytes())?;
+        file.set_times(
+            FileTimes::new()
+                .set_accessed(SystemTime::UNIX_EPOCH)
+                .set_modified(SystemTime::UNIX_EPOCH),
+        )?;
+        file.sync_all()?;
+        Ok(())
+    }
+
+    #[test]
+    fn swap_just_link() -> Result {
+        TestPath::Explicit("niri/config.kdl")
+            .setup_any(|sh| {
+                let dir = sh.current_dir().join("niri");
+
                 sh.create_dir(&dir)?;
 
-                let mut d2 = dir.clone();
+                create_epoch(dir.join("config2.kdl"), "a")?;
-                d2.push("config2.kdl");
+                create_epoch(dir.join("config3.kdl"), "b")?;
-                let mut c2 = File::create(d2).unwrap();
-                write!(c2, "a")?;
-                c2.flush()?;
-                futimens(
-                    &c2,
-                    &Timestamps {
-                        last_access: Timespec {
-                            tv_sec: 0,
-                            tv_nsec: 0,
-                        },
-                        last_modification: Timespec {
-                            tv_sec: 0,
-                            tv_nsec: 0,
-                        },
-                    },
-                )?;
-                c2.sync_all()?;
-                drop(c2);
-
-                let mut d3 = dir.clone();
-                d3.push("config3.kdl");
-                let mut c3 = File::create(d3).unwrap();
-                write!(c3, "b")?;
-                c3.flush()?;
-                futimens(
-                    &c3,
-                    &Timestamps {
-                        last_access: Timespec {
-                            tv_sec: 0,
-                            tv_nsec: 0,
-                        },
-                        last_modification: Timespec {
-                            tv_sec: 0,
-                            tv_nsec: 0,
-                        },
-                    },
-                )?;
-                c3.sync_all()?;
-                drop(c3);
 
                 cmd!(sh, "ln -s config2.kdl niri/config.kdl").run()?;
+
                 Ok(())
-            },
+            })
-            |sh| {
+            .assert_initial("a")
-                cmd!(sh, "unlink niri/config.kdl").run()?;
+            .run(|sh, test| {
-                cmd!(sh, "ln -s config3.kdl niri/config.kdl").run()?;
+                cmd!(sh, "ln -sf config3.kdl niri/config.kdl").run()?;
+                test.assert_changed_to("b");
+
                 Ok(())
-            },
+            })
-        );
     }
 
     #[test]
-    fn swap_dir_link() {
+    fn swap_many_regular() -> Result {
-        check(
+        TestPath::Regular {
-            |sh| {
+            user_path: "user-niri/config.kdl",
-                sh.write_file("niri2/config.kdl", "a")?;
+            system_path: "system-niri/config.kdl",
-                cmd!(sh, "ln -s niri2 niri").run()?;
+        }
+        .setup(|sh| sh.write_file("system-niri/config.kdl", "system config"))
+        .assert_initial("system config")
+        .run(|sh, test| {
+            sh.write_file("user-niri/config.kdl", "user config")?;
+            test.assert_changed_to("user config");
+
+            cmd!(sh, "touch system-niri/config.kdl").run()?;
+            test.assert_unchanged();
+
+            sh.remove_path("system-niri")?;
+            test.assert_unchanged();
+
+            sh.write_file("system-niri/config.kdl", "new system config")?;
+            test.assert_unchanged();
+
+            sh.remove_path("user-niri")?;
+            test.assert_changed_to("new system config");
+
+            sh.write_file("system-niri/config.kdl", "updated system config")?;
+            test.assert_changed_to("updated system config");
+
+            sh.write_file("user-niri/config.kdl", "new user config")?;
+            test.assert_changed_to("new user config");
+
             Ok(())
-            },
+        })
-            |sh| {
+    }
-                sh.write_file("niri3/config.kdl", "b")?;
+
-                cmd!(sh, "unlink niri").run()?;
+    #[test]
-                cmd!(sh, "ln -s niri3 niri").run()?;
+    fn swap_many_links_regular_like_nix() -> Result {
+        TestPath::Regular {
+            user_path: "user-niri/config.kdl",
+            system_path: "system-niri/config.kdl",
+        }
+        .setup_any(|sh| {
+            let store = sh.current_dir().join("store");
+
+            sh.create_dir(&store)?;
+
+            create_epoch(store.join("gen1"), "gen 1")?;
+            create_epoch(store.join("gen2"), "gen 2")?;
+            create_epoch(store.join("gen3"), "gen 3")?;
+
+            sh.create_dir("user-niri")?;
+            sh.create_dir("system-niri")?;
+
             Ok(())
-            },
+        })
-        );
+        .assert_initial_not_exists()
+        .run(|sh, test| {
+            let store = sh.current_dir().join("store");
+            test.assert_unchanged();
+
+            cmd!(sh, "ln -s {store}/gen1 user-niri/config.kdl").run()?;
+            test.assert_changed_to("gen 1");
+
+            cmd!(sh, "ln -s {store}/gen2 system-niri/config.kdl").run()?;
+            test.assert_unchanged();
+
+            cmd!(sh, "unlink user-niri/config.kdl").run()?;
+            test.assert_changed_to("gen 2");
+
+            cmd!(sh, "ln -s {store}/gen3 user-niri/config.kdl").run()?;
+            test.assert_changed_to("gen 3");
+
+            cmd!(sh, "ln -sf {store}/gen1 system-niri/config.kdl").run()?;
+            test.assert_unchanged();
+
+            cmd!(sh, "unlink system-niri/config.kdl").run()?;
+            test.assert_unchanged();
+
+            cmd!(sh, "ln -s {store}/gen1 system-niri/config.kdl").run()?;
+            test.assert_unchanged();
+
+            cmd!(sh, "unlink user-niri/config.kdl").run()?;
+            test.assert_changed_to("gen 1");
+
+            Ok(())
+        })
     }
 }