Skip to content

install: use EXIT trap for temp directory cleanup#2380

Merged
devm33 merged 1 commit intogithub:mainfrom
marcelsafin:fix/install-trap-cleanup
Mar 29, 2026
Merged

install: use EXIT trap for temp directory cleanup#2380
devm33 merged 1 commit intogithub:mainfrom
marcelsafin:fix/install-trap-cleanup

Conversation

@marcelsafin
Copy link
Copy Markdown
Contributor

The install script creates a temp directory with mktemp -d but relies on manual rm -rf calls scattered across individual error handlers to clean it up. Several exit paths are missing cleanup entirely:

  • curl/wget download failure (set -e exits before any cleanup)
  • tar extraction failure
  • chmod failure
  • mkdir -p failure (handled error path, but no rm -rf)

This replaces all five manual rm -rf "$TMP_DIR" calls with a single trap ... EXIT registered immediately after the temp directory is created. The trap fires on any exit—whether from set -e, an explicit exit 1, or normal completion—so every path is covered with one line.

Before: 5 cleanup calls, 4+ uncovered exit paths
After: 1 trap, 0 uncovered exit paths

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the installer’s temporary directory cleanup to be centralized and reliable across all exit paths, aligning with the script’s set -e behavior and reducing duplicated cleanup logic.

Changes:

  • Registers a single trap ... EXIT immediately after creating the mktemp -d directory.
  • Removes multiple scattered rm -rf "$TMP_DIR" calls from individual error/success paths.
  • Ensures cleanup occurs for failures during download, validation, extraction, chmod, and install directory creation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Replace scattered rm -rf calls with a single EXIT trap to ensure the
temp directory is always cleaned up on exit. Previously, failures in
curl/wget download, tar extraction, chmod, or mkdir left temp files
behind because set -e would exit before reaching manual cleanup calls.

The trap fires on any exit (success or failure), so it covers all
paths with one line while removing five redundant cleanup calls.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Member

@devm33 devm33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@devm33 devm33 merged commit a503500 into github:main Mar 29, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants