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

Add support for nvm for Windows #3748

Merged
merged 1 commit into from
Jul 4, 2018
Merged

Add support for nvm for Windows #3748

merged 1 commit into from
Jul 4, 2018

Conversation

JimiC
Copy link
Contributor

@JimiC JimiC commented Jun 28, 2018

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? N/A
Fixed tickets #3739
License MIT

These are the minimum required changes to support nvm for Windows.

@CLAassistant
Copy link

CLAassistant commented Jun 28, 2018

CLA assistant check
All committers have signed the CLA.

@JimiC
Copy link
Contributor Author

JimiC commented Jun 28, 2018

I'd like to explain lines 380-381 a bit as nvm for Windows has a quirk.
When it first downloads node.exe for a version, it renames it to node64.exe for x64 platforms and node32.exe for x32 ones, in order to support both arch side by side.
Upon activation via nvm use it renames nodeX.exe to node.exe and remains so if no other arch version for the same node version is downloaded.

Now, I wanted to refrain from making the codebase complex by introducing filename checks, so I kept it at a bare minimum which leads to a quirky flow.

Explaining.

pm2 will always look for a node.exe filename when trying to resolve the specified interpreter.

  • If the user has already activated the node version, pm2 will see the node.exe file and proceed as normal.
  • If the user has not downloaded the node version specified, pm2 will trigger an nvm install. As mentioned above nvm for Windows has that renaming quirk, so after the download is finished, pm2 will look for a node64.exe or node32.exe instead of node.exe, as the user has not yet activated that node version.
  • If the user has previously downloaded the node version specified but has not activated it yet, the filename will still be nodeX.exe. pm2 will look for a node.exe filename but will not find it and will try to download that version again. Now, nvm for Windows will see that that node version is already there and prompt that the version already exists and skips download. This is when lines 380-381 come to the rescue as they tell pm2 to look for a nodeX.exe filename instead, which is true and pm2 then proceeds as normal.

If you think that this flow will cause maintainability issues, and you prefer a filename check function to be added instead, please let me know, so to update the PR.

@JimiC
Copy link
Contributor Author

JimiC commented Jun 28, 2018

Unfortunately, no tests have been added, as you don't have a Windows testing environment, yet.

@wallet77 wallet77 merged commit 2dac235 into Unitech:development Jul 4, 2018
@JimiC JimiC deleted the support_nvm4win branch July 4, 2018 13:41
inerc pushed a commit to inerc/pm2 that referenced this pull request Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants