-
Notifications
You must be signed in to change notification settings - Fork 24
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
fix(install): remove wget and sha256sum dependencies #1013
Conversation
function sha256sum() { | ||
shasum -a 256 "$@" | ||
} | ||
|
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 is the actual fix for sha256sum
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.
question: from what I understood of the actual problem, the call to sha256sum
works as this script used to work on Benjamin's Mac. It's the introduction of the function check_command_exists
that made the install fail. Isn't it?
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.
Not really, it never worked on Benjamin's Mac since the addition of the call to sha256sum
.
This binary seems not installed by default on MacOS.
The introduction of check_command_exists
stopped the script before the error occurred.
checksum_computed=$(sha256sum ${tmpdir}/${archive_name} | cut -d" " -f1) | ||
checksum_expected=$(wget --quiet --output-document - $checksums_url | grep $archive_name | cut -d" " -f1) | ||
checksum_computed=$(sha256sum "${tmpdir}/${archive_name}" | cut -d" " -f1) | ||
checksum_expected=$(curl --silent --location "$checksums_url" | grep "$archive_name" | cut -d" " -f1) |
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 is the actual fix for wget
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.
question: I have the same question as the one raised above. From what I understand of the issue, the problem is not the usage of wget
that is here for years according to the Git history. The problem is the call to check_command_exists
.
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.
Yes check_command_exists
blocked user that didn't have wget
installed.
But in the same way as this command failed due to missing binary.
I found it more consistent to use curl as we are already asking to use for the download https://doc.scalingo.com/platform/cli/start
function sha256sum() { | ||
shasum -a 256 "$@" | ||
} | ||
|
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.
question: from what I understood of the actual problem, the call to sha256sum
works as this script used to work on Benjamin's Mac. It's the introduction of the function check_command_exists
that made the install fail. Isn't it?
@@ -58,14 +54,14 @@ main() { | |||
echo | |||
} | |||
|
|||
if [ "x$DEBUG" = "xtrue" ] ; then | |||
if [ -n "$DEBUG" ] ; then |
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 not equivalent. Why don't we keep the equivalent?
if [ -n "$DEBUG" ] ; then | |
if [ "$DEBUG" = "true" ] ; then |
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 not equivalent, I change it to be more consistent with the usage of the DEBUG
of the CLI itself (which can be used with DEBUG=1
).
checksum_computed=$(sha256sum ${tmpdir}/${archive_name} | cut -d" " -f1) | ||
checksum_expected=$(wget --quiet --output-document - $checksums_url | grep $archive_name | cut -d" " -f1) | ||
checksum_computed=$(sha256sum "${tmpdir}/${archive_name}" | cut -d" " -f1) | ||
checksum_expected=$(curl --silent --location "$checksums_url" | grep "$archive_name" | cut -d" " -f1) |
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.
question: I have the same question as the one raised above. From what I understand of the issue, the problem is not the usage of wget
that is here for years according to the Git history. The problem is the call to check_command_exists
.
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.
Thanks for your answers :)
Fixes #1012
Tested on Benjamin's Mac