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
Make the install script verify the checksum automatically #104
Conversation
@@ -1,13 +1,12 @@ | |||
#!/usr/bin/env bash | |||
set -eu | |||
|
|||
VERSION="v0.3.0" | |||
VERSION="v0.4.0" |
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.
This version doesn't exist but we need the path fix from #103, which will only be available in the next release.
install.sh
Outdated
|
||
[[ -e "${TMPFILE}" ]] && unlink "${TMPFILE}" | ||
rm -rf "${TMPDIR}" |
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.
Should we just leave the tmpdir to avoid doing a rm -rf
?
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.
I think that's quite common to keep those tempdir and anyway something should clean this up at one point automatically no?
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.
i don't like using -rf
in a script
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.
Removed
Why
The install script should not ask the user to compare the checksums, it should let
shasum
do that.How
Download the checksum file at the right place and run
shasum -c
, which would warn and fail the whole script in case of a mismatch.BLOCKED until the next release because we need to the new shasum files introduced by #103