- Notifications
You must be signed in to change notification settings - Fork928
fix(install.sh): remove extracted files after installation#12879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
06cf4a1 to0889bfbCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
| "$sh_c" cp "$CACHE_DIR/tmp/coder" "$STANDALONE_BINARY_LOCATION" | ||
| # Clean up the extracted files (note, not using sudo: $sh_c -> sh_c). | ||
| sh_c rm -rv "$CACHE_DIR/tmp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nit it's possible to perform cleanup using traps too. Would implementing this practice here be helpful, or would it add unnecessary complexity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's a good suggestion and definitely an option (unless shell compatibility is weak for some reason, haven't checked). Another option would be to split it up into two functions, where we:
mkdir .../tmpif ! install_bin; then rm -rf .../tmp echo install failed exit 1firm -rf .../tmpFor now though, I only want to make small changes to the script in stages/as needed, so we don't suddenly break it as there aren't really any tests I believe. 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Alright, thanks for the guidance!
Uh oh!
There was an error while loading.Please reload this page.
This PR creates a subdirectory in
CACHE_DIRnamedtmpthat is deleted after installation.This isimportant because the script executes
sudo unzipwhich leaves files with root permission in the users personal cache directory~/.cache/coder. Removing the extracted files frees up 200 MB on the users drive as well. We don't attempt to fix past mistakes, but future users will be happier.Note: We should probably downgrade the unzip to run as normal user, no point in doing it as root.Fixed!