From 99b70cdb1dbd54af799d2839f0081e7bdfd3fede Mon Sep 17 00:00:00 2001
From: r-vdp <ramses@well-founded.dev>
Date: Thu, 30 Apr 2026 11:27:30 +0200
Subject: [PATCH] switch-to-configuration-ng: replace is_unit_disabled with
 UnitFileState

is_unit_disabled mapped any canonicalize() error to "disabled" via
unwrap_or(true), conflating ENOENT with transient I/O failures. For
new_dropins.all(is_unit_disabled) that meant a transient error on a
still-present drop-in could read as "all masked" and stop a unit that
is still configured.

Replace the boolean helper with a tri-state UnitFileState (Present /
Masked / Missing) returned from unit_file_state(). Only ENOENT maps to
Missing; other errors are propagated so callers cannot accidentally
pick the wrong default.
---
 .../sw/switch-to-configuration-ng/src/main.rs | 75 ++++++++++++++-----
 1 file changed, 55 insertions(+), 20 deletions(-)

diff --git a/pkgs/by-name/sw/switch-to-configuration-ng/src/main.rs b/pkgs/by-name/sw/switch-to-configuration-ng/src/main.rs
index 63867a9b9322..af3771561055 100644
--- a/pkgs/by-name/sw/switch-to-configuration-ng/src/main.rs
+++ b/pkgs/by-name/sw/switch-to-configuration-ng/src/main.rs
@@ -1002,13 +1002,35 @@ fn remove_file_if_exists(p: impl AsRef<Path>) -> std::io::Result<()> {
     }
 }
 
-/// Checks if a unit has been disabled in configuration
-fn is_unit_disabled(unit_file: impl AsRef<Path>) -> bool {
-    unit_file
-        .as_ref()
-        .canonicalize()
-        .map(|full_path| full_path == Path::new("/dev/null"))
-        .unwrap_or(true)
+#[derive(Debug, PartialEq)]
+enum UnitFileState {
+    /// The file exists and resolves to a real unit file.
+    Present,
+    /// The file is a (chain of) symlink(s) to /dev/null, i.e. masked.
+    Masked,
+    /// The file does not exist, or is a dangling symlink.
+    Missing,
+}
+
+impl UnitFileState {
+    /// Whether the unit file is absent from the configuration, either because
+    /// it does not exist or because it has been masked to /dev/null.
+    fn is_gone(&self) -> bool {
+        matches!(self, UnitFileState::Masked | UnitFileState::Missing)
+    }
+}
+
+/// Classify a unit-file path. Unexpected I/O errors are propagated.
+fn unit_file_state(unit_file: impl AsRef<Path>) -> Result<UnitFileState> {
+    let path = unit_file.as_ref();
+    match path.canonicalize() {
+        Ok(target) if target == Path::new("/dev/null") => Ok(UnitFileState::Masked),
+        Ok(_) => Ok(UnitFileState::Present),
+        Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(UnitFileState::Missing),
+        Err(err) => {
+            Err(err).with_context(|| format!("Failed to canonicalize unit file {}", path.display()))
+        }
+    }
 }
 
 /// Iterate over currently active units in the given scope, compare the unit
@@ -1079,19 +1101,32 @@ fn collect_unit_changes(
                 current_base_unit_file = old_unit_dir.join(&base_unit);
                 new_base_unit_file = new_unit_dir.join(&base_unit);
 
-                // Handle instances defined as drop-ins
-                let mut current_dropins =
-                    unit_dropin_glob(&current_unit_file)?.filter_map(|v| v.ok());
-                let mut new_dropins = unit_dropin_glob(&new_unit_file)?.filter_map(|v| v.ok());
+                // Handle instances defined as drop-ins. When the unit is
+                // disabled, the override files will be a symlink to
+                // /dev/null instead.
 
-                // When the unit is disabled, the override files will be a symlink to /dev/null instead.
-                dropins_removed =
-                    // True iff at least one current drop-in is not /dev/null,
-                    // i.e. the instance was NixOS-managed in the old generation.
-                    !current_dropins.all(is_unit_disabled)
-                    // True iff there are no new drop-ins, or all of them are
-                    // /dev/null, i.e. the instance is gone in the new generation.
-                    && new_dropins.all(is_unit_disabled);
+                // The instance was NixOS-managed in the old generation iff
+                // at least one current drop-in is a real file (not masked).
+                let mut currently_managed = false;
+                for entry in unit_dropin_glob(&current_unit_file)?.flatten() {
+                    if unit_file_state(&entry)? == UnitFileState::Present {
+                        currently_managed = true;
+                        break;
+                    }
+                }
+
+                // If the instance was managed before, check whether it
+                // still is: gone iff no new drop-ins remain (or all are
+                // masked to /dev/null).
+                if currently_managed {
+                    dropins_removed = true;
+                    for entry in unit_dropin_glob(&new_unit_file)?.flatten() {
+                        if unit_file_state(&entry)? == UnitFileState::Present {
+                            dropins_removed = false;
+                            break;
+                        }
+                    }
+                }
             }
         }
 
@@ -1108,7 +1143,7 @@ fn collect_unit_changes(
         {
             // Account for template unit instances where overrideStrategy == "asDropin"
             // whilst also allowing manual instances to keep running.
-            if dropins_removed || is_unit_disabled(&new_base_unit_file) {
+            if dropins_removed || unit_file_state(&new_base_unit_file)?.is_gone() {
                 let current_unit_info = parse_unit(&current_unit_file, &current_base_unit_file)?;
                 if parse_systemd_bool(Some(&current_unit_info), "Unit", "X-StopOnRemoval", true) {
                     _ = units_to_stop.insert(unit.to_string(), ());