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

Install-NodeVersion 20 Fails #101

Closed
drekuru opened this issue Jun 12, 2023 · 4 comments
Closed

Install-NodeVersion 20 Fails #101

drekuru opened this issue Jun 12, 2023 · 4 comments

Comments

@drekuru
Copy link
Contributor

drekuru commented Jun 12, 2023

Issue:
Running the following command Install-NodeVersion 20 fails with a path issue. Per the screenshot
image

it's looking for a nodejs dir which is no longer sitting in the .u directory but is nested inside of a new dir PFiles64.

image

Adding a check for this unpack should fix the problem

System: Windows 10
Environment: Windows Powershell, Powershell 7.3.4 (tested in both)

As a side note:
Powershell v6+ Supports chaining params in the Join-Path command like Join-Path "str1 "str2" "str3" and so on, (beyond 2 params).
However this syntax doesn't work with windows powershell

@aaronpowell
Copy link
Owner

There must be something different in the MSI structure, or how the MSI unpacks. I wonder if this is only in Node.js v20 and above or a change in Windows MSI handling.

I'll look at getting to this at some point, but it's a low priority for me at present as I'm not really using the module myself these days, I primarily use WSL for development, so happy for someone to take a crack and I can review the changes.

@drekuru
Copy link
Contributor Author

drekuru commented Jun 13, 2023

@aaronpowell
For me it was just erroring out for Node 20.
Installing Node 18 and 16 was no problem.

I am down to take a crack at it, to pretty much add a path check to see if nodejs is there otherwise try to look for a PFiles64 dir, however I am not sure how this will affect, non-Windows installations, not sure if any of your tests account for this, nor do I have a way of manually testing against Mac and/or Linux.

If you don't see any issues with this approach, I will gladly make a PR.

@aaronpowell
Copy link
Owner

Check for existence of the folder and append it probably is the simplest then. We have test coverage that runs on Mac and Linux so assuming they all pass still it should be fine

@aaronpowell
Copy link
Owner

I've done some digging into the root cause of the issue and why it's only just cropped up.

In Node.js 19 there was a refactor to how the MSI is built on Windows, and this saw a migration from Wix 3.x to Wix 4.x, which happened in nodejs/node#45943 (relevant changelog: https://github.com/nodejs/node/blob/main/doc/changelogs/CHANGELOG_V19.md#other-notable-changes).

I'm not familiar with how Wix works or how the MSI is created, but likely something was changed to better handle the support for Program Files paths across 32bit and 64bit architectures, and that sees the bundling into a subfolder.

I confirmed this by using https://github.com/activescott/lessmsi to have a look into MSI packages from v18, v19 and v20 and you can see the folder path change in the internals. It can also be observed that 32bit and 64bit have some slightly different paths, as they target different Program Files locations:

32bit vs 64bit MSI internal comparisons

As a fix, we should check for the existence of the PFiles folder with the architecture suffix (blank for 32bit, 64 for 64bit) so that we can unpack appropriately.

There might be a better way to unpack with MSI command line flags, but this work in #102 should do as an initial solution.

@drekuru drekuru closed this as completed Jun 30, 2023
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

No branches or pull requests

2 participants