From fd510a7122d49cc1cbd72b9e70b1ae6b3c76c990 Mon Sep 17 00:00:00 2001
From: Michael Hoang <enzime@users.noreply.github.com>
Date: Sun, 3 Nov 2024 19:26:56 +1100
Subject: [PATCH] system: replace `for f in $(ls ...)` with `for f in .../*`

Fixes SC2045 but has one quirk which is if the bash glob doesn't match
anything it'll treat it as a string and run the loop once with
`f=.../*` so we need to check that `$f` actually exists.
---
 modules/system/launchd.nix | 41 ++++++++++++++++++++++++++------------
 modules/system/patches.nix | 15 ++++++++------
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/modules/system/launchd.nix b/modules/system/launchd.nix
index cdb6549f..c578dec3 100644
--- a/modules/system/launchd.nix
+++ b/modules/system/launchd.nix
@@ -105,19 +105,29 @@ in
       ${concatMapStringsSep "\n" (attr: launchdActivation "LaunchAgents" attr.target) launchAgents}
       ${concatMapStringsSep "\n" (attr: launchdActivation "LaunchDaemons" attr.target) launchDaemons}
 
-      for f in $(ls /run/current-system/Library/LaunchAgents 2> /dev/null); do
-        if test ! -e "${cfg.build.launchd}/Library/LaunchAgents/$f"; then
-          echo "removing service $(basename $f .plist)" >&2
+      for f in /run/current-system/Library/LaunchAgents/*; do
+        [[ -e "$f" ]] || break  # handle when directory is empty
+        f=''${f#/run/current-system/Library/LaunchAgents/}
+
+        if [[ ! -e "${cfg.build.launchd}/Library/LaunchAgents/$f" ]]; then
+          echo "removing service $(basename "$f" .plist)" >&2
           launchctl unload "/Library/LaunchAgents/$f" || true
-          if test -e "/Library/LaunchAgents/$f"; then rm -f "/Library/LaunchAgents/$f"; fi
+          if [[ -e "/Library/LaunchAgents/$f" ]]; then
+            rm -f "/Library/LaunchAgents/$f"
+          fi
         fi
       done
 
-      for f in $(ls /run/current-system/Library/LaunchDaemons 2> /dev/null); do
-        if test ! -e "${cfg.build.launchd}/Library/LaunchDaemons/$f"; then
-          echo "removing service $(basename $f .plist)" >&2
+      for f in /run/current-system/Library/LaunchDaemons/*; do
+        [[ -e "$f" ]] || break  # handle when directory is empty
+        f=''${f#/run/current-system/Library/LaunchDaemons/}
+
+        if [[ ! -e "${cfg.build.launchd}/Library/LaunchDaemons/$f" ]]; then
+          echo "removing service $(basename "$f" .plist)" >&2
           launchctl unload "/Library/LaunchDaemons/$f" || true
-          if test -e "/Library/LaunchDaemons/$f"; then rm -f "/Library/LaunchDaemons/$f"; fi
+          if [[ -e "/Library/LaunchDaemons/$f" ]]; then
+            rm -f "/Library/LaunchDaemons/$f"
+          fi
         fi
       done
     '';
@@ -133,11 +143,16 @@ in
       ''}
       ${concatMapStringsSep "\n" (attr: userLaunchdActivation attr.target) userLaunchAgents}
 
-      for f in $(ls /run/current-system/user/Library/LaunchAgents 2> /dev/null); do
-        if test ! -e "${cfg.build.launchd}/user/Library/LaunchAgents/$f"; then
-          echo "removing user service $(basename $f .plist)" >&2
-          launchctl unload ~/Library/LaunchAgents/$f || true
-          if test -e ~/Library/LaunchAgents/$f; then rm -f ~/Library/LaunchAgents/$f; fi
+      for f in /run/current-system/user/Library/LaunchAgents/*; do
+        [[ -e "$f" ]] || break  # handle when directory is empty
+        f=''${f#/run/current-system/user/Library/LaunchAgents/}
+
+        if [[ ! -e "${cfg.build.launchd}/user/Library/LaunchAgents/$f" ]]; then
+          echo "removing user service $(basename "$f" .plist)" >&2
+          launchctl unload ~/Library/LaunchAgents/"$f" || true
+          if [[ -e ~/Library/LaunchAgents/"$f" ]]; then
+            rm -f ~/Library/LaunchAgents/"$f"
+          fi
         fi
       done
     '';
diff --git a/modules/system/patches.nix b/modules/system/patches.nix
index 4f965014..7b192556 100644
--- a/modules/system/patches.nix
+++ b/modules/system/patches.nix
@@ -30,9 +30,9 @@ in
         Set of patches to apply to {file}`/`.
 
         ::: {.warning}
-        
+
         This can modify everything so use with caution.
-        
+
         :::
 
         Useful for safely changing system files.  Unlike the etc module this
@@ -56,10 +56,13 @@ in
       # Applying patches to /.
       echo "applying patches..." >&2
 
-      for f in $(ls /run/current-system/patches 2> /dev/null); do
-          if test ! -e "${config.system.build.patches}/patches/$f"; then
-              patch --force --reverse --backup -d / -p1 < "/run/current-system/patches/$f" || true
-          fi
+      for f in /run/current-system/patches/*; do
+        [[ -e "$f" ]] || break  # handle when directory is empty
+        f=''${f#/run/current-system/patches/}
+
+        if [[ ! -e "${config.system.build.patches}/patches/$f" ]]; then
+          patch --force --reverse --backup -d / -p1 < "/run/current-system/patches/$f" || true
+        fi
       done
 
       ${concatMapStringsSep "\n" (f: ''