From 8f4f1ce005ceba182805d5a8866e39269b9a23a5 Mon Sep 17 00:00:00 2001
From: Bruce Toll <4109762+tollb@users.noreply.github.com>
Date: Fri, 30 Jun 2023 10:11:23 -0400
Subject: [PATCH] nixos/atop: Fix regression in enabling atop units

Fix regression where the systemd units for atop are no longer
automatically started at boot when programs.atop.enable = true.

Regression was introduced in commit: 09350ff7d424
  nixos/atop: Convert log format to fix service start

This commit restructures the atop systemd service config so that the
code to convert the log format gets configured as a preStart script
along with the addition of the wantedBy rule.
---
 nixos/modules/programs/atop.nix | 72 +++++++++++++++++----------------
 1 file changed, 37 insertions(+), 35 deletions(-)

diff --git a/nixos/modules/programs/atop.nix b/nixos/modules/programs/atop.nix
index 9d5843bd670e2..a5f4d990bdbe1 100644
--- a/nixos/modules/programs/atop.nix
+++ b/nixos/modules/programs/atop.nix
@@ -123,8 +123,8 @@ in
       boot.extraModulePackages = [ (lib.mkIf cfg.netatop.enable cfg.netatop.package) ];
       systemd =
         let
-          mkSystemd = type: cond: name: restartTriggers: {
-            ${name} = lib.mkIf cond {
+          mkSystemd = type: name: restartTriggers: {
+            ${name} = {
               inherit restartTriggers;
               wantedBy = [ (if type == "services" then "multi-user.target" else if type == "timers" then "timers.target" else null) ];
             };
@@ -134,42 +134,44 @@ in
         in
         {
           packages = [ atop (lib.mkIf cfg.netatop.enable cfg.netatop.package) ];
-          services =
-            mkService cfg.atopService.enable "atop" [ atop ]
-            // lib.mkIf cfg.atopService.enable {
-              # always convert logs to newer version first
-              # XXX might trigger TimeoutStart but restarting atop.service will
-              # convert remainings logs and start eventually
-              atop.serviceConfig.ExecStartPre = pkgs.writeShellScript "atop-update-log-format" ''
-                set -e -u
-                shopt -s nullglob
-                for logfile in "$LOGPATH"/atop_*
-                do
-                  ${atop}/bin/atopconvert "$logfile" "$logfile".new
-                  # only replace old file if version was upgraded to avoid
-                  # false positives for atop-rotate.service
-                  if ! ${pkgs.diffutils}/bin/cmp -s "$logfile" "$logfile".new
-                  then
-                    ${pkgs.coreutils}/bin/mv -v -f "$logfile".new "$logfile"
-                  else
-                    ${pkgs.coreutils}/bin/rm -f "$logfile".new
-                  fi
-                done
-              '';
-            }
-            // mkService cfg.atopacctService.enable "atopacct" [ atop ]
-            // mkService cfg.netatop.enable "netatop" [ cfg.netatop.package ]
-            // mkService cfg.atopgpu.enable "atopgpu" [ atop ];
-          timers = mkTimer cfg.atopRotateTimer.enable "atop-rotate" [ atop ];
+          services = lib.mkMerge [
+            (lib.mkIf cfg.atopService.enable (lib.recursiveUpdate
+              (mkService "atop" [ atop ])
+              {
+                # always convert logs to newer version first
+                # XXX might trigger TimeoutStart but restarting atop.service will
+                # convert remainings logs and start eventually
+                atop.preStart = ''
+                  set -e -u
+                  shopt -s nullglob
+                  for logfile in "$LOGPATH"/atop_*
+                  do
+                    ${atop}/bin/atopconvert "$logfile" "$logfile".new
+                    # only replace old file if version was upgraded to avoid
+                    # false positives for atop-rotate.service
+                    if ! ${pkgs.diffutils}/bin/cmp -s "$logfile" "$logfile".new
+                    then
+                      ${pkgs.coreutils}/bin/mv -v -f "$logfile".new "$logfile"
+                    else
+                      ${pkgs.coreutils}/bin/rm -f "$logfile".new
+                    fi
+                  done
+                '';
+              }))
+            (lib.mkIf cfg.atopacctService.enable (mkService "atopacct" [ atop ]))
+            (lib.mkIf cfg.netatop.enable (mkService "netatop" [ cfg.netatop.package ]))
+            (lib.mkIf cfg.atopgpu.enable (mkService "atopgpu" [ atop ]))
+          ];
+          timers = lib.mkIf cfg.atopRotateTimer.enable (mkTimer "atop-rotate" [ atop ]);
         };
 
       security.wrappers = lib.mkIf cfg.setuidWrapper.enable {
-        atop =
-          { setuid = true;
-            owner = "root";
-            group = "root";
-            source = "${atop}/bin/atop";
-          };
+        atop = {
+          setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${atop}/bin/atop";
+        };
       };
     }
   );