-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Curl --compressed handling #1550
Conversation
curl < 7.21.0 will output redirection reponse body to the output when compressed, which will cause tarball is prepended with redirection reponse body and leads to checksum mismatch. - add `nvm_curl_use_compression` and `nvm_curl_version`
nvm.sh
Outdated
@@ -79,11 +79,18 @@ nvm_curl_libz_support() { | |||
curl -V 2>/dev/null | nvm_grep "^Features:" | nvm_grep -q "libz" | |||
} | |||
|
|||
nvm_curl_use_compression() { | |||
if nvm_curl_libz_support && nvm_version_greater_than_or_equal_to nvm_curl_version 7.21.0; then | |||
return 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.
It looks amature here to return 0/1, any suggestions from bash pro is welcome.
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.
The body of the function can just be nvm_curl_libz_support && nvm_version_greater_than_or_equal_to nvm_curl_version 7.21.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.
Looks great!
Please also add a unit test for nvm_curl_use_compression
:-)
nvm.sh
Outdated
@@ -79,11 +79,18 @@ nvm_curl_libz_support() { | |||
curl -V 2>/dev/null | nvm_grep "^Features:" | nvm_grep -q "libz" | |||
} | |||
|
|||
nvm_curl_use_compression() { | |||
if nvm_curl_libz_support && nvm_version_greater_than_or_equal_to nvm_curl_version 7.21.0; then | |||
return 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.
The body of the function can just be nvm_curl_libz_support && nvm_version_greater_than_or_equal_to nvm_curl_version 7.21.0
nvm.sh
Outdated
@@ -221,6 +228,10 @@ nvm_clang_version() { | |||
clang --version | command awk '{ if ($2 == "version") print $3; else if ($3 == "version") print $4 }' | command sed 's/-.*$//g' | |||
} | |||
|
|||
nvm_curl_version() { | |||
curl --version | command awk '{ if ($1 == "curl") print $2 }' | command sed 's/-.*$//g' |
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.
nvm_curl_libz_support
uses curl -V
; are those identical across all curl versions?
If you're not certain, let's stick with curl -V
to be consistent.
6460eae
to
cb20548
Compare
Added unit tests of nvm_curl_version as well as nvm_curl_use_compression. |
cb20548
to
e199e5f
Compare
|
||
if type "curl" > /dev/null 2>&1 ; then | ||
curl_exec="$(type "curl")" | ||
sudo rm -rf "${curl_exec}" |
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 think this is necessary (or safe, since this runs on developer's systems); the function below should mask it sufficiently.
|
||
if type "curl" > /dev/null 2>&1 ; then | ||
curl_exec="$(type "curl")" | ||
sudo rm -rf "${curl_exec}" |
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.
same here
Removed unnecessary curl mask. |
b8c80a5
to
7965796
Compare
Thanks! |
Added
curl_use_compression
routine to determine whether we should add compression flag to curl routine. Fixed #1549