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.

Sign up for GitHub

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

Jump to bottom

[New] Supercharge nvm debug output #1453

Merged
merged 1 commit into from
Mar 27, 2017
Merged

[New] Supercharge nvm debug output #1453

merged 1 commit into from
Mar 27, 2017

Conversation

PeterDaveHello
Copy link
Collaborator

Try to get shell version, OS and its version, curl/wget version.

@PeterDaveHello @ljharb
[New] Supercharge nvm debug output
7b253c8 
Try to get shell version, OS and its version, curl/wget/git version.
@PeterDaveHello PeterDaveHello force-pushed the nvm-debug-improvement branch 2 times, most recently from 70693bd to d113640 Compare March 24, 2017 09:44
@PeterDaveHello
Copy link
Collaborator Author

PeterDaveHello commented Mar 24, 2017 •
edited
Loading

Output examples on different systems:

nvm --version: v0.33.1
$SHELL: /usr/local/bin/bash
$HOME: /Users/peterdavehello
$NVM_DIR: '$HOME/.nvm'
$PREFIX: ''
$NPM_CONFIG_PREFIX: ''
$NVM_NODEJS_ORG_MIRROR: ''
$NVM_IOJS_ORG_MIRROR: ''
shell version: 'GNU bash, version 4.4.12(1)-release (x86_64-apple-darwin16.3.0)'
uname -a: 'Darwin 16.4.0 Darwin Kernel Version 16.4.0: Thu Dec 22 22:53:21 PST 2016; root:xnu-3789.41.3~3/RELEASE_X86_64 x86_64'
OS version: Mac 10.12.3 16D32
curl: curl is /usr/bin/curl, curl 7.51.0 (x86_64-apple-darwin16.0) libcurl/7.51.0 SecureTransport zlib/1.2.8
wget: wget is /usr/local/bin/wget, GNU Wget 1.19.1 built on darwin16.4.0.
nvm current: v7.6.0
which node: $NVM_DIR/versions/node/v7.6.0/bin/node
which iojs:
which npm: $NVM_DIR/versions/node/v7.6.0/bin/npm
npm config get prefix: $NVM_DIR/versions/node/v7.6.0
npm root -g: $NVM_DIR/versions/node/v7.6.0/lib/node_modules

nvm --version: v0.33.1
$SHELL: /bin/bash
$HOME: /home/peter
$NVM_DIR: '$HOME/.nvm'
$PREFIX: ''
$NPM_CONFIG_PREFIX: ''
$NVM_NODEJS_ORG_MIRROR: 'https://nodejs.org/dist'
$NVM_IOJS_ORG_MIRROR: 'https://iojs.org/dist'
shell version: 'GNU bash, version 4.4.12(1)-release (x86_64-unknown-linux-gnu)'
uname -a: 'Linux 4.9.11-1-ARCH #1 SMP PREEMPT Sun Feb 19 13:45:52 UTC 2017 x86_64 GNU/Linux'
OS version: Arch Linux  ()
curl: curl is /usr/sbin/curl, curl 7.53.1 (x86_64-pc-linux-gnu) libcurl/7.53.1 OpenSSL/1.0.2k zlib/1.2.11 libpsl/0.17.0 (+libicu/58.2) libssh2/1.8.0
wget: wget is /usr/sbin/wget, GNU Wget 1.19.1 built on linux-gnu.
nvm current: v0.12.18
which node: $NVM_DIR/versions/node/v0.12.18/bin/node
which iojs: which: no iojs in ($NVM_DIR/versions/node/v0.12.18/bin:~/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/local/bin:/usr/bin:/bin)
which npm: $NVM_DIR/versions/node/v0.12.18/bin/npm
npm config get prefix: $NVM_DIR/versions/node/v0.12.18
npm root -g: $NVM_DIR/versions/node/v0.12.18/lib/node_modules

nvm --version: v0.33.1
$SHELL: /usr/local/bin/bash
$HOME: /home/peter
$NVM_DIR: '$HOME/.nvm'
$PREFIX: ''
$NPM_CONFIG_PREFIX: ''
$NVM_NODEJS_ORG_MIRROR: ''
$NVM_IOJS_ORG_MIRROR: ''
shell version: 'GNU bash, version 4.3.39(2)-release (amd64-portbld-freebsd9.3)'
uname -a: 'FreeBSD 10.1-RELEASE-p26 FreeBSD 10.1-RELEASE-p26 #0: Wed Jan 13 20:59:29 UTC 2016 root@amd64-builder.daemonology.net:/usr/obj/usr/src/sys/GENERIC amd64'
curl: curl is /usr/local/bin/curl, curl 7.43.0 (amd64-portbld-freebsd10.1) libcurl/7.43.0 OpenSSL/1.0.2d zlib/1.2.8 libssh2/1.4.3
wget: wget is /usr/local/bin/wget, GNU Wget 1.16.3 built on freebsd9.3.
nvm current: system
which node: /usr/local/bin/node
which iojs:
which npm: /usr/local/bin/npm
npm config get prefix: /usr/local
npm root -g: /usr/local/lib/node_modules

nvm --version: v0.33.1
$SHELL: /bin/bash
$HOME: /home/peterdavehello
$NVM_DIR: '$HOME/nvm'
$PREFIX: ''
$NPM_CONFIG_PREFIX: ''
$NVM_NODEJS_ORG_MIRROR: ''
$NVM_IOJS_ORG_MIRROR: ''
shell version: 'GNU bash, version 4.3.11(1)-release (x86_64-pc-linux-gnu)'
uname -a: 'Linux 4.2.0-27-generic #32~14.04.1-Ubuntu SMP Fri Jan 22 15:32:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux'
OS version: Ubuntu 14.04.5 LTS
curl: curl is /usr/bin/curl, curl 7.35.0 (x86_64-pc-linux-gnu) libcurl/7.35.0 OpenSSL/1.0.1f zlib/1.2.8 libidn/1.28 librtmp/2.3
wget: wget is /usr/bin/wget, GNU Wget 1.15 built on linux-gnu.
nvm current: v4.8.1
which node: $NVM_DIR/versions/node/v4.8.1/bin/node
which iojs:
which npm: $NVM_DIR/versions/node/v4.8.1/bin/npm
npm config get prefix: $NVM_DIR/versions/node/v4.8.1
npm root -g: $NVM_DIR/versions/node/v4.8.1/lib/node_modules

ljharb
ljharb approved these changes Mar 24, 2017
View reviewed changes
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Nice, I like it. Just one change.

nvm.sh Outdated
@@ -2249,6 +2249,35 @@ nvm() {
nvm_err "\$NPM_CONFIG_PREFIX: '$(nvm_sanitize_path "$NPM_CONFIG_PREFIX")'"
nvm_err "\$NVM_NODEJS_ORG_MIRROR: '${NVM_NODEJS_ORG_MIRROR}'"
nvm_err "\$NVM_IOJS_ORG_MIRROR: '${NVM_IOJS_ORG_MIRROR}'"
nvm_err "shell version: '$(${SHELL} --version | command head -n 1)'"
nvm_err "uname -a: '$(uname -a | awk '{$2=""; print}' | xargs)'"
if [ "$(nvm_get_os)" = "darwin" ] && type sw_vers > /dev/null; then
Copy link
Member

Choose a reason for hiding this comment

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

can this use nvm_has instead of type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure

@PeterDaveHello PeterDaveHello force-pushed the nvm-debug-improvement branch 2 times, most recently from 2ec54e2 to 5f18eaa Compare March 25, 2017 08:48
@PeterDaveHello
Copy link
Collaborator Author

Also added git and improved output for aliased commands

ljharb
ljharb reviewed Mar 25, 2017
View reviewed changes
nvm.sh Outdated
if nvm_has "curl"; then
local CURL
if type curl | command grep -q hashed; then
CURL="$(type curl | command sed -E 's/\(|)//g' | command awk '{print $4}')"
Copy link
Member

Choose a reason for hiding this comment

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

it seems like this logic is repeated three times; can we consolidate it into a nvm_command_info function? I'm thinking CURL="$(nvm_command_info curl)"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@PeterDaveHello PeterDaveHello force-pushed the nvm-debug-improvement branch 4 times, most recently from cea67b7 to 97a3adf Compare March 25, 2017 22:08
@PeterDaveHello
Copy link
Collaborator Author

Should be good now.

ljharb
ljharb approved these changes Mar 26, 2017
View reviewed changes
test/fast/Unit tests/nvm_command_info Outdated
[ "${WGET_COMMAND_INFO}" = "${WGET_EXPECTED_INFO}" ] || die "wget command info wrong(stage 2), expected: '${WGET_EXPECTED_INFO}', got '${WGET_COMMAND_INFO}'"

# clean up this stage
unalias wget
Copy link
Member

Choose a reason for hiding this comment

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

these two lines should probably just call cleanup, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Clean up will hide the output, I think we may not want to do this before the test finished.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, good call. I'll update cleanup itself to not hide the output.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ljharb I don't want the error message since if the alias is not there the cleanup will give error message.

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer maximal output in tests; it helps when debugging a failure.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we can treat the cleanup() as the final process to do before exit but before that the cleanup process is a little bit different?

Copy link
Member

Choose a reason for hiding this comment

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

Technically these three tests should each be in a separate file, each with their own cleanup.

test/fast/Unit tests/nvm_command_info Outdated
[ "${WGET_COMMAND_INFO}" = "${WGET_EXPECTED_INFO}" ] || die "wget command info wrong(stage 1), expected: '${WGET_EXPECTED_INFO}', got '${WGET_COMMAND_INFO}'"

# clean up this stage
unset WGET_EXPECTED_INFO WGET_COMMAND_INFO
Copy link
Member

Choose a reason for hiding this comment

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

this line should just call cleanup, no?

@ljharb ljharb force-pushed the nvm-debug-improvement branch 3 times, most recently from c7e6b22 to 7b253c8 Compare March 27, 2017 00:51
@ljharb ljharb merged commit 7b253c8 into nvm-sh:master Mar 27, 2017
@PeterDaveHello PeterDaveHello deleted the nvm-debug-improvement branch March 27, 2017 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljharb ljharb ljharb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@PeterDaveHello @ljharb