-
Notifications
You must be signed in to change notification settings - Fork 210
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
Edge case platforms support (SmartOS, AIX, and gzip fallback) #117
Conversation
Discussion in the referenced issue doesn't appear to be settled, so it seems premature to drop all use of XZ packages. They are significantly smaller, so this would have a performance impact for all users. Also there is a CI failure on Git Bash on Windows, where it is trying to download |
Ack. I'll just exclude AIX.
Looking into that, I'm also waiting on access to AIX and SmartOS for more testing. |
4344aad
to
6527971
Compare
* also exclude AIX
6527971
to
3c9ee08
Compare
Both CIs are green, PTAL. |
@@ -75,6 +80,18 @@ nvs() { | |||
curl -L -# "${NODE_URI}" -o "${NODE_ARCHIVE}" | |||
fi | |||
|
|||
if [ ! -f "${NODE_ARCHIVE}" ] && [ "${NODE_ARCHIVE_EXT}" = ".tar.xz" ]; then | |||
NODE_ARCHIVE_EXT=".tar.xz" |
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 this was intended to switch to .tar.gz
?
@@ -75,6 +80,18 @@ nvs() { | |||
curl -L -# "${NODE_URI}" -o "${NODE_ARCHIVE}" | |||
fi | |||
|
|||
if [ ! -f "${NODE_ARCHIVE}" ] && [ "${NODE_ARCHIVE_EXT}" = ".tar.xz" ]; then | |||
NODE_ARCHIVE_EXT=".tar.xz" | |||
TAR_FLAGS="-Jxvf" |
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 it should be -zxvf
for gzip instead of xzip extraction.
local NODE_ARCH="$(uname -m | sed -e 's/x86_64/x64/;s/i86pc/x64/;s/i686/x86/')" | ||
# On AIX reports `uname -m` reports the machine ID number of the hardware running the system. | ||
if [ "${NVS_OS}" = "aix" ]; then | ||
local NODE_ARCH="ppc64" |
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.
nit: local
is not needed here since the local variable is already declared above.
Closing because this is superseded by #169 |
Fixes: #114
uname
parsing and workaround tar limitationsxz
detection, and add gzip download fallback