jorgebucaran / nvm.fish

The Node.js version manager you'll adore, crafted just for Fish
https://git.io/nvm.fish
MIT License
2.06k stars 69 forks source link

install command can fail if run at the same time in multiple terminals #204

Closed GrantASL19 closed 1 year ago

GrantASL19 commented 1 year ago

(Forgive me if this is simply outside of the scope of this project!)

In fish.config I have the following code to automatically install/activate the Node version specified in .node-version/.nvmrc when starting a new terminal or changing directories:

# Via https://github.com/jorgebucaran/nvm.fish/issues/122#issuecomment-727248662
function activate_directory_node_version --on-variable PWD
    if type -q nvm
        test -e $PWD/.nvmrc || test -e $PWD/.node-version && nvm install
    else
        echo "NOTE: Install nvm.fish to automatically source activate the Node version specified in `.node-version` when entering a directory"
    end
end

# Call immediately in case shell opened to directory containing `.node-version`
activate_directory_node_version

This works, but appears to exhibit a race condition if I e.g. restore a VS Code session with multiple terminals open. The first one will output e.g.

Now using Node v18.14.0 (npm 9.3.1) ~/.local/share/nvm/v18.14.0/bin/node"

But the second will sometimes (but not always — seemingly dependent on timing?) output:

awk: can't open file /Users/grant/.local/share/nvm/.index.temp source line number 6 nvm: Invalid version number or alias: "18.14.0"

This seems to be caused by 2+ terminals running nvm install at the same time.

In these cases I’ll just re-open the second terminal so this isn’t a big deal, but it can be confusing if I miss the error and run into unexpected behaviour due to the running the project with the my default system Node version.

Thanks for the project btw — it makes my life better dozens of times a day!

jorgebucaran commented 1 year ago

I see. But you still should be able to use multiple nodes at the same time in different terminals, since path changes are local, yes?

GrantASL19 commented 1 year ago

I see. But you still should be able to use multiple nodes at the same time in different terminals, since path changes are local, yes?

Yes I can, but when this happens the terminal in which the failure happened will have the Homebrew node in the path until I re-run activate_directory_node_version:

awk: can't open file /Users/grant/.local/share/nvm/.index.temp
 source line number 6
nvm: Invalid version number or alias: "18.14.2"
Welcome to fish, the friendly interactive shell
Type `help` for instructions on how to use fish
~/c/a/s/web> node --version
v19.6.1
~/c/a/s/web> which node
/usr/local/bin/node
~/c/a/s/web> activate_directory_node_version
Now using Node v18.14.2 (npm 9.5.0) ~/.local/share/nvm/v18.14.2/bin/node
~/c/a/s/web> node --version
v18.14.2
~/c/a/s/web> which node
/Users/grant/.local/share/nvm/v18.14.2/bin/node

While the other terminal will (correctly) have the NVM node in the path from the beginning:

Now using Node v18.14.2 (npm 9.5.0) ~/.local/share/nvm/v18.14.2/bin/node
Welcome to fish, the friendly interactive shell
Type `help` for instructions on how to use fish
~/c/a/s/web> node --version
v18.14.2
~/c/a/s/web> which node
/Users/grant/.local/share/nvm/v18.14.2/bin/node
jorgebucaran commented 1 year ago

To clarify, this only happens within VS Code?

GrantASL19 commented 1 year ago

To clarify, this only happens within VS Code?

It happens in VS Code but it’s not a VS Code-specific bug.

I can also trigger this by opening two Terminal.app tabs to directories containing .node-version files, saving them with “Window -> Save Windows as Group…”, then opening that group with “Window -> Open Window Group” (might not happen every time, but happened for me in 2/3 attempts). So I think it’s safe to say this can happen in any context where nvm install is called simultaneously in two or more terminals.

jorgebucaran commented 1 year ago

May I ask why you are trying to run the install command simultaneously in multiple terminals? Given that this is outside the scope of the project, I am going to close this issue (at least for now).

GrantASL19 commented 1 year ago

May I ask why you are trying to run the install command simultaneously in multiple terminals? Given that this is outside the scope of the project, I am going to close this issue (at least for now).

I’ll often have two terminal tabs open in VS Code, and when I reopen the project they’re restored simultaneously.

I understand this is an unusual use case — for now I’ve been using fnm to solve this. Thanks for this project, it’s served me well for years!

jorgebucaran commented 1 year ago

Well, I've never encountered this particular issue, but I suppose anything is possible. Maybe you stumbled upon some extremely rare edge case that only a select few have ever experienced. Or maybe it's just the limitations of Fish's subpar concurrency. I may circle back to this later, but I'll keep it closed for now.