Skip to content
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 nvm on SmartOS setups using 64 bits pkgsrc repository #1182

Merged
merged 1 commit into from
Jul 30, 2016

Conversation

misterdjules
Copy link

On SmartOS setups using 64 bits pkgsrc repositories, nvm_get_arch
would not handle pkg_info's output properly.

This would result in nvm not being to install any node binary when
running on SmartOS setups using a 64 bits pkgsrc repository.

This change fixes this problem, and fixes the tests suite on similar
setups.

Tested in separate SmartOS environments using a 32bits and 64bits pkgsrc repository respectively.

On SmartOS setups using 64 bits pkgsrc repositories, `nvm_get_arch`
would not handle pkg_info's output properly.

This would result in nvm not being to install any node binary when
running on SmartOS setups using a 64 bits pkgsrc repository.

This change fixes this problem, and fixes the tests suite on similar
setups.
@@ -1235,6 +1235,8 @@ nvm_get_arch() {
EXIT_CODE=$?
if [ $EXIT_CODE -ne 0 ]; then
HOST_ARCH=$(isainfo -n)
else
HOST_ARCH=$(echo "$HOST_ARCH" | tail -1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! do you think it's better to do the tail -1 on line 1234, when pkg_info is first called?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I was going for originally, but then it would mean that the EXIT_CODE value above would represent the exit code of tail -1, which would always be 0, and the rest of the current logic would be broken.

@ljharb ljharb added the bugs Oh no, something's broken :-( label Jul 29, 2016
@ljharb ljharb merged commit a32b914 into nvm-sh:master Jul 30, 2016
@ljharb
Copy link
Member

ljharb commented Jul 30, 2016

@misterdjules
Copy link
Author

@ljharb Thank you, very much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugs Oh no, something's broken :-(
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants