Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 5, 2024

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Apr 5, 2024

This PR creates a subdirectory in CACHE_DIR named tmp that is deleted after installation.

This is important because the script executes sudo unzip which 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.

cat install.sh | sh -s -- --method=standalone --version 2.9.1
macOS v14.4.1
Installing v2.9.1 of the arm64 release from GitHub.

+ mkdir -p ~/.cache/coder
+ curl -#fL -o ~/.cache/coder/coder_2.9.1_darwin_arm64.zip.incomplete -C - https://github.com/coder/coder/releases/download/v2.9.1/coder_2.9.1_darwin_arm64.zip
######################################################################## 100.0%
+ mv ~/.cache/coder/coder_2.9.1_darwin_arm64.zip.incomplete ~/.cache/coder/coder_2.9.1_darwin_arm64.zip
+ mkdir -p /usr/local
+ sudo mkdir -p /usr/local/bin
+ sudo mkdir -p ~/.cache/coder/tmp
+ sudo unzip -d ~/.cache/coder/tmp -o ~/.cache/coder/coder_2.9.1_darwin_arm64.zip
Archive:  /Users/maf/.cache/coder/coder_2.9.1_darwin_arm64.zip
  inflating: /Users/maf/.cache/coder/tmp/LICENSE
  inflating: /Users/maf/.cache/coder/tmp/LICENSE.enterprise
  inflating: /Users/maf/.cache/coder/tmp/README.md
  inflating: /Users/maf/.cache/coder/tmp/coder
+ sudo rm /usr/local/bin/coder
+ sudo cp ~/.cache/coder/tmp/coder /usr/local/bin/coder
+ sudo rm -rv ~/.cache/coder/tmp
/Users/maf/.cache/coder/tmp/LICENSE
/Users/maf/.cache/coder/tmp/README.md
/Users/maf/.cache/coder/tmp/coder
/Users/maf/.cache/coder/tmp/LICENSE.enterprise
/Users/maf/.cache/coder/tmp

Coder stable release v2.9.1 installed. See our releases documentation or GitHub for more information on versioning.

Note: We should probably downgrade the unzip to run as normal user, no point in doing it as root. Fixed!

@mafredri mafredri force-pushed the mafredri/fix-install.sh-cache-cleanup branch from 06cf4a1 to 0889bfb Compare April 5, 2024 12:57
Copy link
Member

@mtojek mtojek left a 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"
Copy link
Member

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?

Copy link
Member Author

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 .../tmp
if ! install_bin; then
  rm -rf .../tmp
  echo install failed
  exit 1
fi
rm -rf .../tmp

For 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. 😅

Copy link
Member

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!

@mafredri mafredri merged commit b06452e into main Apr 5, 2024
23 checks passed
@mafredri mafredri deleted the mafredri/fix-install.sh-cache-cleanup branch April 5, 2024 16:04
@github-actions github-actions bot locked and limited conversation to collaborators Apr 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants