-
Notifications
You must be signed in to change notification settings - Fork 27
feat: use sha256 to check file integrity #81
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
base: main
Are you sure you want to change the base?
Conversation
|
@lispking Please check the review |
|
@luojiyin1987 thanks for this PR. I will have a look asap. |
jnaulty
left a comment
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.
Great feature addition to the suiup install.sh script.
I have some requested changes (primarily merging download_checksum + download_file)
|
Thanks for the fixes @luojiyin1987, I will have a look very soon (it's been busy and there's a lot of PRs from you to review, which is great but it takes a bit of time). |
- Make MD5 verification consistent with SHA256 behavior - Treat malformed checksum files as skipped verification instead of failure - Add clear warning messages and skip reasons - Improve user experience while maintaining security awareness
| ### From Script | ||
| ```bash | ||
| curl -sSfL https://raw.githubusercontent.com/Mystenlabs/suiup/main/install.sh | sh | ||
| curl -sSfL https://raw.githubusercontent.com/Mystenlabs/suiup/main/install.sh | bash |
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.
Why to change to bash?
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.
local is a keyword specific to bash, used to declare local variables. There is no local keyword in standard sh.
local ist nicht Teil des POSIX-Shell-Standards.
On Linux distributions (such as CentOS, Fedora, RHEL, openSUSE, macOS), /bin/sh is indeed a symbolic link (symlink) pointing to /bin/bash. So in most cases, there is no problem.
In Debian and its derivatives (such as Ubuntu), /bin/sh does not point to bash by default, but rather to a lighter-weight shell that more strictly adheres to POSIX standards—dash. This differs from bash support.
In order to detect problems earlier, it is recommended to use bash.
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.
You can use shellcheck to check compatibility between different shells @stefan-mysten


No description provided.