From bb7867f1e5080de1dfff80e2f7d5a4be49889e23 Mon Sep 17 00:00:00 2001 From: Erik Arvstedt Date: Sat, 2 Jul 2022 16:12:16 +0200 Subject: [PATCH 1/2] vscode: simplify helper scripts This simplifies the following commit. - Remove `--` from `rm` because `$tmpDir` is guaranteed to never start with a `-`. We also don't use this pattern when removing tmpDirs at other places in nixpkgs. - Remove `unset tmpDir` because the variable `tmpDir` is never accessed after the exit handler is run. --- .../vscode/extensions/_maintainers/update-bin-srcs-lib.sh | 5 ++--- .../editors/vscode/extensions/cpptools/update_helper.sh | 4 +--- 2 files changed, 3 insertions(+), 6 deletions(-) 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 e3d1e5fb1397..6a8968024c88 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 @@ -37,9 +37,8 @@ prefetchExtensionUnpacked() { 1>&2 echo "zipStorePath='$zipStorePath'" function rm_tmpdir() { - 1>&2 printf "rm -rf -- %q\n" "$tmpDir" - rm -rf -- "$tmpDir" - unset tmpDir + 1>&2 printf "rm -rf %q\n" "$tmpDir" + rm -rf "$tmpDir" trap - INT TERM HUP EXIT } function make_trapped_tmpdir() { diff --git a/pkgs/applications/editors/vscode/extensions/cpptools/update_helper.sh b/pkgs/applications/editors/vscode/extensions/cpptools/update_helper.sh index 00ef77553242..3c1d68de79cd 100755 --- a/pkgs/applications/editors/vscode/extensions/cpptools/update_helper.sh +++ b/pkgs/applications/editors/vscode/extensions/cpptools/update_helper.sh @@ -44,9 +44,7 @@ extStoreName="${extPublisher}-${extName}" function rm_tmpdir() { - #echo "Removing \`tmpDir='$tmpDir'\`" - rm -rf -- "$tmpDir" - unset tmpDir + rm -rf "$tmpDir" trap - INT TERM HUP EXIT } function make_trapped_tmpdir() { From 3f54dfa4755613dbd3098814a9ab07cb977ca347 Mon Sep 17 00:00:00 2001 From: Erik Arvstedt Date: Sat, 2 Jul 2022 12:51:12 +0200 Subject: [PATCH 2/2] 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" }