Kikobeats / nodengine

Node.js version switcher based on engines at `package.json`.
https://nicedoc.io/Kikobeats/nodengine
MIT License
68 stars 8 forks source link

bash nvm switching doesn't alter parent shell environment #11

Open rosskevin opened 8 years ago

rosskevin commented 8 years ago

Very strange, this stopped working around either nodengine 2.0.0 or node 6.2.0 release in the last few days.

# change into a directory that should switch to 6.2.0
kross:~ kross$ source .bash_profile 
kross:~ kross$ cd projects/relay/examples/todo/
v6.2.0 is already installed.
Now using node v6.2.0 (npm v3.8.9)

# hmmm, still 5.11.1....
kross:todo kross$ node -v
v5.11.1

kross:todo kross$ npm ls -g nodengine
/Users/kross/.nvm/versions/node/v5.11.1/lib
└── nodengine@2.0.0 

kross:todo kross$ nvm list
->      v5.11.1
         v6.1.0
         v6.2.0
default -> 5 (-> v5.11.1)
node -> stable (-> v6.2.0) (default)
stable -> 6.2 (-> v6.2.0) (default)
iojs -> N/A (default)

It seems that after my switch to 2.0.0 (just a guess on timing), my nvm is not using my 6.2 version as it says.

I'll debug this further and report back.

rosskevin commented 8 years ago

This is what is executed:

# reset to 5
kross:projects kross$ nvm use 5
Now using node v5.11.1 (npm v3.8.6)
kross:projects kross$ node -v
v5.11.1

# should update to 6.2
kross:projects kross$ cd nodengine/
v6.2.0 is already installed.
Now using node v6.2.0 (npm v3.8.9)
kross:nodengine kross$ node -v
v5.11.1

# exec the same _exact_ command manually
kross:nodengine kross$ bash -c 'source $NVM_DIR/nvm.sh; nvm install 6'
v6.2.0 is already installed.
Now using node v6.2.0 (npm v3.8.9)
kross:nodengine kross$ node -v
v5.11.1

So, I'm not up on bash semantics but the -c is not allowing it to stick. I'll look further.

rosskevin commented 8 years ago

It's certainly possible I never had bash autoswitching working. While it execs the command and gives good output, the node version isn't sticking in the environment. I haven't yet found out why, and my current experiments haven't worked (yet).

This is confirmed to be a bug with bash nvm switching, whether it is auto or manual.

rosskevin commented 8 years ago

It seems a child_process cannot change a parent process's environment: http://stackoverflow.com/a/22681200/2363935

And: https://github.com/nodejs/node-v0.x-archive/issues/7411

So the stdout messages make you think you've got it right, but if you check with node -v, it will show the default. It seems nvm takes effect by altering NVM_BIN, NVM_PATH, PATH.

So far, I'm not sure this could ever work if we are spawning this command from node.

rosskevin commented 8 years ago

I have a really terrible hack to get switching working. https://github.com/rosskevin/nodengine/commit/b3e1f1d2e0e033ee3757b27ed47c13e89b0640df It is not recommended and I could not find another way. To use it you have to do one of the following:

  1. via command line

    V=`nodengine` && nvm install $V
  2. via autoswitching in .bash_profile

    # Enable nodengine autoswitching
    # http://unix.stackexchange.com/a/21364
    cd () { builtin cd "$@" && chpwd; }
    pushd () { builtin pushd "$@" && chpwd; }
    popd () { builtin popd "$@" && chpwd; }
    chpwd () {
    FILE=$PWD/package.json
    
    if [ -f $FILE ];
    then
     V=`nodengine` && nvm install $V
    fi
    }

Perhaps someone savvier with bash can make something work, but what I found is that the child process cannot affect the parent environment, so nodengine could only do the logic to figure out what to exec, the parent shell must exec the change. This hack (again, not recommended) prints the version to std out and my autoswitcher changes the node version.

So, definite bug using this with bash, no good workaround present, and my fork is not recommended. I'll need someone else to help out with this.

Kikobeats commented 8 years ago

We need a clean way to fix it or I could drop nvm support in favour of n.

I'm using n and seriously, all works fine. so nvm always have problem with something.

The point is, can you confirm me that is a problem related with nodengine@2.0.0 or a problem with last node version?

npm install -g nodengine@1.3.1 and say me what happens please 😄 .

rosskevin commented 8 years ago

This is confirmed a problem with node + bash + nvm. I don't think it ever worked, even though stdout reported it ran.

Kikobeats commented 8 years ago

what do you think about use this

https://github.com/wbyoung/avn-nvm/blob/master/index.js#L20

Kikobeats commented 8 years ago

sounds strange because as you as seeing avn-nvm is using similar strategy

rosskevin commented 8 years ago

Yes this looks virtually identical. I don't discount the idea that I've got something strange in my environment...and the fact that I thought it worked last week.

BUT, based on the links I posted above about the inability for a child process to affect a parent process, the behavior I am currently seeing looks correct. With that said, the avn-nvm you shared wouldn't work either!

So...I don't know. Do we have other bash users? Is it working for them?

rosskevin commented 8 years ago

I'm switching to zsh and n. I'm going to close this, just guessing it is something odd in my environment.

If someone else bumps into this, please speak up.

Kikobeats commented 8 years ago

Hey @rosskevin, I have the same report from another user:

nodengine

so finally spawn process is not affected parent in the nvm version, any idea?

Kikobeats commented 8 years ago

Read this: https://github.com/sylvaindethier/nvm-test/blob/27d1ab2b07aebd970251de67365acd91fc9aa782/src/api/nvm.js#L14-L15

I think that could be possible... can you try?

Kikobeats commented 8 years ago

I think that also could be works using https://github.com/sindresorhus/execa

rosskevin commented 8 years ago

Both using 5.7 and greater with the shell option and execa look promising. I've become a zsh user now so I don't want to spend any more time messing with bash.

clementprevot commented 7 years ago

After @Kikobeats pointed it to me, I also encounter this issue. I'll give some of the solution you gave a try and open a pull request if I ever manage to solve this problem.

I'll let you know.

clementprevot commented 7 years ago

@Kikobeats I tried many solutions (including the two mentionned in this issue) and I didn't managed to get this work. So I don't see any other way than the one pointed out by @rosskevin here. As my use case is indeed auto-switching via the bashrc thing, I don't really mind that this is kind of a dirty solution.

But please, don't drop support of NVM because of this :smile:

Do you want me to try to implement something like this?