From 3f54dfa4755613dbd3098814a9ab07cb977ca347 Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Sat, 2 Jul 2022 12:51:12 +0200
Subject: [PATCH] treewide: fix bash exit handlers

Transform exit handlers of the form
trap cleanup EXIT [INT] [TERM] [QUIT] [HUP] [ERR]
  (where cleanup is idempotent)
to
trap cleanup EXIT

This fixes a common bash antipattern.

Each of the above signals causes the script to exit. For each signal,
bash first handles the signal by running `cleanup` and then runs
`cleanup` again when handling EXIT.
(Exception:  `vscode/*` prevents the second run of `cleanup` by removing
the trap in cleanup`).

Simplify the cleanup logic by just trapping exit, which is always run
when the script exits due to any of the above signals.

Note: In case of borgbackup, the exit handler is not idempotent, but just
trapping EXIT guarantees that it's only run once.
---
 maintainers/scripts/rebuild-amount.sh                       | 2 +-
 nixos/modules/services/backup/borgbackup.nix                | 4 +---
 nixos/modules/services/web-apps/keycloak.nix                | 2 +-
 .../vscode/extensions/_maintainers/update-bin-srcs-lib.sh   | 3 +--
 .../editors/vscode/extensions/cpptools/update_helper.sh     | 3 +--
 pkgs/build-support/fetchcvs/nix-prefetch-cvs                | 6 ++----
 6 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/maintainers/scripts/rebuild-amount.sh b/maintainers/scripts/rebuild-amount.sh
index bedd352db5f3..32810f6b98c0 100755
--- a/maintainers/scripts/rebuild-amount.sh
+++ b/maintainers/scripts/rebuild-amount.sh
@@ -35,7 +35,7 @@ toRemove=()
 cleanup() {
     rm -rf "${toRemove[@]}"
 }
-trap cleanup EXIT SIGINT SIGQUIT ERR
+trap cleanup EXIT
 
 MKTEMP='mktemp --tmpdir nix-rebuild-amount-XXXXXXXX'
 
diff --git a/nixos/modules/services/backup/borgbackup.nix b/nixos/modules/services/backup/borgbackup.nix
index 4c9ddfe4674b..7cf2c080f813 100644
--- a/nixos/modules/services/backup/borgbackup.nix
+++ b/nixos/modules/services/backup/borgbackup.nix
@@ -23,12 +23,10 @@ let
     on_exit()
     {
       exitStatus=$?
-      # Reset the EXIT handler, or else we're called again on 'exit' below
-      trap - EXIT
       ${cfg.postHook}
       exit $exitStatus
     }
-    trap 'on_exit' INT TERM QUIT EXIT
+    trap on_exit EXIT
 
     archiveName="${if cfg.archiveBaseName == null then "" else cfg.archiveBaseName + "-"}$(date ${cfg.dateFormat})"
     archiveSuffix="${optionalString cfg.appendFailedSuffix ".failed"}"
diff --git a/nixos/modules/services/web-apps/keycloak.nix b/nixos/modules/services/web-apps/keycloak.nix
index a1855e1c1a79..a4baad8e9213 100644
--- a/nixos/modules/services/web-apps/keycloak.nix
+++ b/nixos/modules/services/web-apps/keycloak.nix
@@ -569,7 +569,7 @@ in
             shopt -s inherit_errexit
 
             create_role="$(mktemp)"
-            trap 'rm -f "$create_role"' ERR EXIT
+            trap 'rm -f "$create_role"' EXIT
 
             db_password="$(<"$CREDENTIALS_DIRECTORY/db_password")"
             echo "CREATE ROLE keycloak WITH LOGIN PASSWORD '$db_password' CREATEDB" > "$create_role"
diff --git a/pkgs/applications/editors/vscode/extensions/_maintainers/update-bin-srcs-lib.sh b/pkgs/applications/editors/vscode/extensions/_maintainers/update-bin-srcs-lib.sh
index 6a8968024c88..4b0ca54da362 100755
--- a/pkgs/applications/editors/vscode/extensions/_maintainers/update-bin-srcs-lib.sh
+++ b/pkgs/applications/editors/vscode/extensions/_maintainers/update-bin-srcs-lib.sh
@@ -39,11 +39,10 @@ prefetchExtensionUnpacked() {
   function rm_tmpdir() {
     1>&2 printf "rm -rf %q\n" "$tmpDir"
     rm -rf "$tmpDir"
-    trap - INT TERM HUP EXIT
   }
   function make_trapped_tmpdir() {
     tmpDir=$(mktemp -d)
-    trap rm_tmpdir INT TERM HUP EXIT
+    trap rm_tmpdir EXIT
   }
 
   1>&2 echo
diff --git a/pkgs/applications/editors/vscode/extensions/cpptools/update_helper.sh b/pkgs/applications/editors/vscode/extensions/cpptools/update_helper.sh
index 3c1d68de79cd..d7bd307c92a0 100755
--- a/pkgs/applications/editors/vscode/extensions/cpptools/update_helper.sh
+++ b/pkgs/applications/editors/vscode/extensions/cpptools/update_helper.sh
@@ -45,11 +45,10 @@ extStoreName="${extPublisher}-${extName}"
 
 function rm_tmpdir() {
   rm -rf "$tmpDir"
-  trap - INT TERM HUP EXIT
 }
 function make_trapped_tmpdir() {
   tmpDir=$(mktemp -d)
-  trap rm_tmpdir INT TERM HUP EXIT
+  trap rm_tmpdir EXIT
 }
 
 echo
diff --git a/pkgs/build-support/fetchcvs/nix-prefetch-cvs b/pkgs/build-support/fetchcvs/nix-prefetch-cvs
index f9ed8ffa066f..b6a169f8b531 100755
--- a/pkgs/build-support/fetchcvs/nix-prefetch-cvs
+++ b/pkgs/build-support/fetchcvs/nix-prefetch-cvs
@@ -21,13 +21,11 @@ fi
 
 mkTempDir() {
     tmpPath="$(mktemp -d "${TMPDIR:-/tmp}/nix-prefetch-cvs-XXXXXXXX")"
-    trap removeTempDir EXIT SIGINT SIGQUIT
+    trap removeTempDir EXIT
 }
 
 removeTempDir() {
-    if test -n "$tmpPath"; then
-        rm -rf "$tmpPath" || true
-    fi
+    rm -rf "$tmpPath"
 }