edc / bass

Make Bash utilities usable in Fish shell
MIT License
2.2k stars 70 forks source link

`nvm current` reverts the used node version #13

Closed taktran closed 8 years ago

taktran commented 8 years ago

I noticed that when I create a nvm function, as suggested in the readme, it reverts the currently used node version (from an .nvmrc file)

ie,

In .config/fish/functions/nvm.fish

function nvm
    bass source ~/.nvm/nvm.sh ';' nvm $argv
end

In a directory where I have a .nvmrc file with iojs-v2.2.1 and I run

$ nvm use
Found /Users/ttt/Work/Dev/folder/.nvmrc with version <iojs-v2.2.1>
Now using io.js v2.2.1
$ which node
/Users/ttt/.nvm/versions/io.js/v2.2.1/bin/node

which is good. But then when I run nvm current:

$ nvm current
v4.1.1
$ which node
/Users/ttt/.nvm/versions/node/v4.1.1/bin/node

It reverts back to my default version, which in this case is v4.1.1. Same for other functions that read the status of nvm eg, nvm list

Is there a way to run current or other ready only functions without loading the default?

edc commented 8 years ago

I was not able to reproduce the bug:

~ $ cat .config/fish/functions/nvm.fish
function nvm
    bass source ~/.nvm/nvm.sh ';' nvm $argv
end

~ $ nvm use v4
Now using node v4.1.1 (npm v2.14.4)

~ $ which node
/Users/edc/.nvm/versions/node/v4.1.1/bin/node

~ $ cd tmp

~/tmp $ cat .nvmrc
v0.10.40

~/tmp $ nvm use
Found /Users/edc/tmp/.nvmrc with version <v0.10.40>
Now using node v0.10.40 (npm v1.4.28)

~/tmp $ which node
/Users/edc/.nvm/v0.10.40/bin/node

~/tmp $ nvm current
v0.10.40

~/tmp $ which node
/Users/edc/.nvm/v0.10.40/bin/node

~/tmp $ nvm --version
0.25.4

Have you tried the same flow in bash?

taktran commented 8 years ago

I got mostly what you got

~/tmp$ cat ~/.config/fish/functions/nvm.fish
function nvm
    bass source ~/.nvm/nvm.sh ';' nvm $argv
end

~$ nvm current
v4.1.1

~$ which node
/Users/ttt/.nvm/versions/node/v4.1.1/bin/node

~$ cd tmp
~/tmp$ nvm use
Found /Users/ttt/tmp/.nvmrc with version <v0.10.40>
Now using node v0.10.40 (npm v1.4.28)

~/tmp$ which node
/Users/ttt/.nvm/v0.10.40/bin/node

~/tmp$ nvm current
v4.1.1

~/tmp$ which node
/Users/ttt/.nvm/versions/node/v4.1.1/bin/node

~/tmp$ nvm --version
/Users/ttt/.nvm/nvm.sh: line 2188: echo: write error: Broken pipe
0.28.0

This bit might say something /Users/ttt/.nvm/nvm.sh: line 2188: echo: write error: Broken pipe, which points to the 2nd line here

nvm_supports_source_options() {
  [ "_$(echo 'echo $1' | . /dev/stdin yes 2> /dev/null)" = "_yes" ]
}

Any ideas?

edc commented 8 years ago

I suspect it's because of nvm. Is it possible for you to downgrade to 0.27.x? I will test with 0.28.0 if you still have the same problem after downgrade.

taktran commented 8 years ago

Nope, downgrading didn't work:

~$ nvm current
v4.1.1
~$ which node
/Users/ttt/.nvm/versions/node/v4.1.1/bin/node
~$ cd tmp
~/tmp$ nvm use
/Users/ttt/.nvm/nvm.sh: line 2111: echo: write error: Broken pipe
Found /Users/ttt/tmp/.nvmrc with version <v0.10.40>
Now using node v0.10.40 (npm v1.4.28)
~/tmp$ which node
/Users/ttt/.nvm/v0.10.40/bin/node
~/tmp$ nvm current
v4.1.1
~/tmp$ which node
/Users/ttt/.nvm/versions/node/v4.1.1/bin/node
~/tmp$ nvm --version
0.27.1

I've also tried with 0.27.0, and 0.25.4 (the one you were using)

I don't think it is nvm because it works fine when I switch to bash and run the exact commands.

I'm thinking it's something to do with the bass source ~/.nvm/nvm.sh line in the nvm function

taktran commented 8 years ago

I found the problem. It was because I had the default alias. Removing the default alias allows nvm to just use the previously set nvm version ie,

~$ nvm current
system
~$ which node
/usr/local/bin/node
~$ cd tmp
~/tmp$ nvm use
Found /Users/ttt/tmp/.nvmrc with version <v0.10.40>
Now using node v0.10.40 (npm v1.4.28)
~/tmp$ nvm current
v0.10.40
~/tmp$ which node
/Users/ttt/.nvm/v0.10.40/bin/node
~/tmp$ cd ..
~$ nvm current
v0.10.40
~$ which node
/Users/ttt/.nvm/v0.10.40/bin/node

Note how the nvm version is the same even after going to a different directory. In the previous case, the default would override this.

Bash does do what is expected though, so it's still kind of a bug in the bass version. However, it works for me without default, so I'm good =)

Up to you, what you want to do with this :bug: =)

edc commented 8 years ago

Glad to hear it works out!

I was able to reproduce it, and the bug was because nvm.sh switches to the default every time it is source'd. Bass has to source nvm.sh on every nvm command, so this is a limitation that's a bit hard to address. For now, I would suggest to not add a default alias as a workaround. I'll leave this open to see if I can come up with a fix.

ljharb commented 8 years ago

Can bass not do something along the lines of (pseudocode) "if nvm is already sourced, and nvm current works, store that value in a variable, and after sourcing nvm, nvm use --silent $previousValue"?

It's slightly inefficient but not that bad.

ljharb commented 8 years ago

Does fish + bass support sourcing with arguments? if so, I would consider adding (in addition to the --install argument that's already there) a --no-use argument that bypassed the auto-use behavior.

edc commented 8 years ago

Does fish + bass support sourcing with arguments

The short answer is yes. The sourcing is done by bash, so as long as bash supports it, it works. I tested with the following fish function:

$ cat ~/.config/fish/functions/nvm.fish
function nvm
    bass source ~/.nvm/nvm.sh --no-use ';' nvm $argv
end

Then I modified nvm.sh to add support of --no-use:

@@ -1791,9 +1791,9 @@
   elif nvm_rc_version >/dev/null 2>&1; then
     nvm install >/dev/null
   fi
-elif [ -n "$VERSION" ]; then
+elif [ -n "$VERSION" ] && [ "_$1" != "_--no-use" ]; then
   nvm use "$VERSION" >/dev/null
-elif nvm_rc_version >/dev/null 2>&1; then
+elif nvm_rc_version >/dev/null 2>&1 && [ "_$1" != "_--no-use" ]; then
   nvm use >/dev/null
 fi

Now it works flawlessly:

$ cat .nvmrc
v4.1.1
$ nvm use default
Now using node v0.10.41 (npm v1.4.29)
$ nvm current
v0.10.41
$ nvm use system
Now using system version of node: v0.12.2 (npm v2.7.5)
$ nvm current
system
$ nvm use
Found /private/tmp/.nvmrc with version <v4.1.1>
Now using node v4.1.1 (npm v2.14.4)
/private/tmp
$ nvm current
v4.1.1

I think it's a pretty good solution!

ljharb commented 8 years ago

Awesome. I'll add it as part of https://github.com/creationix/nvm/issues/972 (and the ordering of --install or --no-use won't actually matter, but --no-use will of course trump --install) and update here when it's released.

edc commented 8 years ago

Great! Thanks, @ljharb!

ljharb commented 8 years ago

Done and released in v0.30.2.

edc commented 8 years ago

Thanks, @ljharb!

@taktran, is this good to close?

taktran commented 8 years ago

Great, thanks!