Introduce an intermediate target for systemd services.

Restarting the active targets seems to cause issues on Ubuntu,
restarting certain targets causes the display manager to exit.

nix/modules/upstream/nixpkgs/nginx.nix

@@ -1,5 +1,13 @@
+{ lib, ... }:
 {
-  systemd.services.nginx.serviceConfig.DynamicUser = true;
+  systemd.services.nginx = {
+    serviceConfig.DynamicUser = true;
+
+    # TODO: can we handle this better?
+    wantedBy = lib.mkForce [
+      "system-manager.target"
+    ];
+  };
 
   # Disable this for now
   services.logrotate.settings.nginx = { };

src/activate/etc_files.rs

@@ -208,43 +208,35 @@ where
         upwards_path: &Path,
     ) -> EtcActivationResult {
         let link_path = etc_dir.join(link_target);
-        if link_path.is_dir() && absolute_target.is_dir() {
+        // Create the dir if it doesn't exist yet
-            log::debug!("Entering into directory {}...", link_path.display());
+        let dir_state = create_dir_recursively(&link_path, state)?;
-            Ok(absolute_target
+        log::debug!("Entering into directory {}...", link_path.display());
-                .read_dir()
+        Ok(absolute_target
-                .expect("Error reading the directory.")
+            .read_dir()
-                .fold(state, |state, entry| {
+            .expect("Error reading the directory.")
-                    let new_state = go(
+            .fold(dir_state, |state, entry| {
-                        &link_target.join(
+                let new_state = go(
-                            entry
+                    &link_target.join(
-                                .expect("Error reading the directory entry.")
+                        entry
-                                .file_name(),
+                            .expect("Error reading the directory entry.")
-                        ),
+                            .file_name(),
-                        etc_dir,
+                    ),
-                        state,
+                    etc_dir,
-                        old_state,
+                    state,
-                        &upwards_path.join(".."),
+                    old_state,
-                    );
+                    &upwards_path.join(".."),
-                    match new_state {
+                );
-                        Ok(new_state) => new_state,
+                match new_state {
-                        Err(ActivationError::WithPartialResult { result, source }) => {
+                    Ok(new_state) => new_state,
-                            log::error!(
+                    Err(ActivationError::WithPartialResult { result, source }) => {
-                                "Error while trying to link directory {}: {source:?}",
+                        log::error!(
-                                absolute_target.display()
+                            "Error while trying to link directory {}: {source:?}",
-                            );
+                            absolute_target.display()
-                            result
+                        );
-                        }
+                        result
                     }
-                }))
+                }
-        } else {
+            }))
-            Err(ActivationError::with_partial_result(
-                state,
-                anyhow::anyhow!(
-                    "Unmanaged file or directory {} already exists, ignoring...",
-                    link_path.display()
-                ),
-            ))
-        }
     }
 
     fn go(
@@ -260,15 +252,40 @@ where
             .join(SYSTEM_MANAGER_STATIC_NAME)
             .join(link_target);
         let absolute_target = etc_dir.join(SYSTEM_MANAGER_STATIC_NAME).join(link_target);
-        if link_path.exists() && !old_state.is_managed(&link_path) {
+
-            link_dir_contents(
+        // Some versions of systemd ignore .wants and .requires directories when they are symlinks.
-                link_target,
+        // We therefore create them as actual directories and link their contents instead.
-                &absolute_target,
+        let is_systemd_dependency_dir = absolute_target.is_dir()
-                etc_dir,
+            && absolute_target
-                dir_state,
+                .parent()
-                old_state,
+                .map(|p| p.ends_with("systemd/system"))
-                upwards_path,
+                .unwrap_or(false)
-            )
+            && link_target
+                .extension()
+                .filter(|ext| ["wants", "requires"].iter().any(|other| other == ext))
+                .is_some();
+
+        if (link_path.exists() && link_path.is_dir() && !old_state.is_managed(&link_path))
+            || is_systemd_dependency_dir
+        {
+            if absolute_target.is_dir() {
+                link_dir_contents(
+                    link_target,
+                    &absolute_target,
+                    etc_dir,
+                    dir_state,
+                    old_state,
+                    upwards_path,
+                )
+            } else {
+                Err(ActivationError::with_partial_result(
+                    dir_state,
+                    anyhow::anyhow!(
+                        "Unmanaged file or directory {} already exists, ignoring...",
+                        link_path.display()
+                    ),
+                ))
+            }
         } else if link_path.is_symlink()
             && link_path.read_link().expect("Error reading link.") == target
         {

src/activate/services.rs

@@ -66,7 +66,7 @@ pub fn activate(
     wait_for_jobs(
         &service_manager,
         &job_monitor,
-        stop_services(&service_manager, &services_to_stop),
+        stop_services(&service_manager, convert_services(&services_to_stop)),
         &timeout,
     )
     .map_err(|e| ActivationError::with_partial_result(services.clone(), e))?;
@@ -78,14 +78,11 @@ pub fn activate(
         .daemon_reload()
         .map_err(|e| ActivationError::with_partial_result(services.clone(), e))?;
 
-    let active_targets = get_active_targets(&service_manager)
-        .map_err(|e| ActivationError::with_partial_result(services.clone(), e))?;
-
     wait_for_jobs(
         &service_manager,
         &job_monitor,
-        reload_services(&service_manager, &services_to_reload)
+        reload_units(&service_manager, convert_services(&services_to_reload))
-            + start_units(&service_manager, &active_targets),
+            + start_units(&service_manager, ["system-manager.target"]),
         &timeout,
     )
     .map_err(|e| ActivationError::with_partial_result(services.clone(), e))?;
@@ -94,32 +91,6 @@ pub fn activate(
     Ok(services)
 }
 
-fn get_active_targets(
-    service_manager: &systemd::ServiceManager,
-) -> anyhow::Result<Vec<systemd::UnitStatus>> {
-    // We exclude some targets that we do not want to start
-    let excluded_targets: HashSet<String> =
-        ["suspend.target", "hibernate.target", "hybrid-sleep.target"]
-            .iter()
-            .map(ToOwned::to_owned)
-            .collect();
-    Ok(service_manager
-        .list_units_by_patterns(&["active", "activating"], &[])?
-        .into_iter()
-        .filter(|unit| {
-            unit.name.ends_with(".target")
-                && !excluded_targets.contains(&unit.name)
-                && !service_manager
-                    .unit_manager(unit)
-                    .refuse_manual_start()
-                    .unwrap_or_else(|e| {
-                        log::error!("Error communicating with DBus: {}", e);
-                        true
-                    })
-        })
-        .collect())
-}
-
 fn get_services_to_reload(services: Services, old_services: Services) -> Services {
     let mut services_to_reload = services.intersection(old_services.clone());
     services_to_reload.retain(|name, service| {
@@ -203,12 +174,14 @@ pub fn deactivate(old_services: Services) -> ServiceActivationResult {
             .map_err(|e| ActivationError::with_partial_result(old_services.clone(), e))?;
         let timeout = Some(Duration::from_secs(30));
 
+        let mut units_to_stop = convert_services(&old_services);
+        units_to_stop.push("system-manager.target");
         // We need to do this before we reload the systemd daemon, so that the daemon
         // still knows about these units.
         wait_for_jobs(
             &service_manager,
             &job_monitor,
-            stop_services(&service_manager, &old_services),
+            stop_services(&service_manager, units_to_stop),
             &timeout,
         )
         // We consider all jobs stopped now..
@@ -241,33 +214,32 @@ fn restore_ephemeral_system_dir() -> anyhow::Result<()> {
     Ok(())
 }
 
-fn stop_services(service_manager: &systemd::ServiceManager, services: &Services) -> HashSet<JobId> {
+fn stop_services<'a, U>(service_manager: &systemd::ServiceManager, units: U) -> HashSet<JobId>
-    for_each_unit(
+where
-        |s| service_manager.stop_unit(s),
+    U: AsRef<[&'a str]>,
-        convert_services(services),
+{
-        "stopping",
+    for_each_unit(|s| service_manager.stop_unit(s), units.as_ref(), "stopping")
-    )
 }
 
-fn reload_services(
+fn reload_units<'a, U>(service_manager: &systemd::ServiceManager, units: U) -> HashSet<JobId>
-    service_manager: &systemd::ServiceManager,
+where
-    services: &Services,
+    U: AsRef<[&'a str]>,
-) -> HashSet<JobId> {
+{
     for_each_unit(
         |s| service_manager.reload_unit(s),
-        convert_services(services),
+        units.as_ref(),
         "reloading",
     )
 }
 
-fn start_units(
+fn start_units<'a, U>(service_manager: &systemd::ServiceManager, units: U) -> HashSet<JobId>
-    service_manager: &systemd::ServiceManager,
+where
-    units: &[systemd::UnitStatus],
+    U: AsRef<[&'a str]>,
-) -> HashSet<JobId> {
+{
     for_each_unit(
         |unit| service_manager.start_unit(unit),
-        convert_units(units),
+        units.as_ref(),
-        "restarting",
+        "starting",
     )
 }
 
@@ -275,13 +247,6 @@ fn convert_services(services: &Services) -> Vec<&str> {
     services.keys().map(AsRef::as_ref).collect::<Vec<&str>>()
 }
 
-fn convert_units(units: &[systemd::UnitStatus]) -> Vec<&str> {
-    units
-        .iter()
-        .map(|unit| unit.name.as_ref())
-        .collect::<Vec<&str>>()
-}
-
 fn for_each_unit<'a, F, R, S>(action: F, units: S, log_action: &str) -> HashSet<JobId>
 where
     F: Fn(&str) -> anyhow::Result<R>,