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

Allow creating MSI installers for Windows ARM64 #33689

Closed

Conversation

dennisameling
Copy link
Contributor

@dennisameling dennisameling commented Jun 1, 2020

Adds configuration to allow building an MSI installer for Windows ARM64.
MSI can be created by running vcbuild.bat release msi arm64

IMPORTANT: when building an MSI for ARM64, WIX 3.14 or higher is required, as that version includes support for ARM64.

I created a test release with a ZIP and MSI for Windows ARM64: https://github.com/dennisameling/node/releases/tag/15-arm64win

I only got Failed to generate license.rtf because the generated node.exe file is an ARM64 exe which can't be executed on a x64 host. When I skipped that part, the build was packaged successfully. We could try to find a solution for this either in this PR or in another PR.

Refs: #25998
Refs: #32582

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes --> did vcbuild test arm64 but didn't work on host x64 machine as it's not capable of running arm64 exe files.
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Adds configuration to allow building an MSI installer for Windows ARM64.
MSI can be created by running `vcbuild.bat release msi arm64`

Refs: nodejs#25998
Refs: nodejs#32582
@nodejs-github-bot nodejs-github-bot added install Issues and PRs related to the installers. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform. labels Jun 1, 2020
@addaleax
Copy link
Member

addaleax commented Jun 1, 2020

@nodejs/platform-windows

@joaocgreis
Copy link
Member

@dennisameling thanks for opening this PR and moving ARM64 support forward!

The approach here generally LGTM. We have WiX 3.11 in the machines that build Node, we get it from https://chocolatey.org/packages/wixtoolset. To move forward with ARM64 support I'll have to look into the best way to overcome this, but for now, since ARM64 is experimental, this does not block this PR unless there is some conflict with WiX 3.11 when building the supported architectures.

These files have been in place for a while, and for big changes we should test upgrade scenarios. To avoid that, I'd be more confident landing this if you could remove the unrelated changes you mentioned above, if building still works.

Minor nit, there's a change in the last line of the proj files that would be good to remove as well.

We need to start a test build of the final version of this before landing, to ensure there are no regressions with the current release system.

@dennisameling
Copy link
Contributor Author

@joaocgreis thanks for the feedback! Will update the PR in the coming days 😊

@dennisameling
Copy link
Contributor Author

@joaocgreis PR updated, deleted unrelated changes. The MSI build and installation still work as expected, so that's good 😊

I don't expect this to cause any issues with WiX 3.11 and the existing build process, because it should only try to find WiX's ARM64 files when an ARM64 build is explicitly requested. If you could start a test build of the final version that'd be fantastic, I'm curious if it will work as expected.

One last thing I'm not sure about, is the license.rtf, like I mentioned in the description of the PR. That's these lines in the vcbuild.bat. If an ARM64 build is created on a x64 host, the x64 host will try to run the ARM64 node.exe it just compiled, which doesn't work of course. One thing I could think of, is to add logic that checks if "%target_arch%"=="arm64", then downloads https://nodejs.org/dist/latest/win-x64/node.exe and runs tools\license2rtf.js using that x64 executable. How does that sound? Happy to add it to the vcbuild.bat if you're OK with that

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@joaocgreis joaocgreis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Test build completed successfully: https://nodejs.org/download/test/v15.0.0-test20200608f03c158f25/

About the changes in vcbuild, I'd rather leave that for another PR and land this PR as is. For node-chakracore, I once did nodejs/node-chakracore@efeaf88. Having a way to automatically download the latest node.exe would be good, but we need to make it very hard for that executable to be used unintentionally in the tests. So, download to a new subdirectory (to be added to .gitignore) and possibly rename the file. We still need a way like in the commit above to pass a variable and not download, so that the CI machines don't have to download every time. Something like this would go very well with our CI/release system:

if not defined native_node_exe set "native_node_exe=%~dp0native-bin\native-node.exe"
if not exist "%native_node_exe%" DOWNLOAD

@nodejs-github-bot
Copy link
Collaborator

@bzoz
Copy link
Contributor

bzoz commented Jun 9, 2020

The parallel/test-fs-stream-construct is failing constantly on ARM64, not related to this PR.

bzoz pushed a commit that referenced this pull request Jun 9, 2020
Adds configuration to allow building an MSI installer for Windows ARM64.
MSI can be created by running `vcbuild.bat release msi arm64`

Refs: #25998
Refs: #32582

PR-URL: #33689
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@bzoz
Copy link
Contributor

bzoz commented Jun 9, 2020

Landed in 35871c3

@bzoz bzoz closed this Jun 9, 2020
codebytere pushed a commit that referenced this pull request Jun 18, 2020
Adds configuration to allow building an MSI installer for Windows ARM64.
MSI can be created by running `vcbuild.bat release msi arm64`

Refs: #25998
Refs: #32582

PR-URL: #33689
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@codebytere codebytere mentioned this pull request Jun 28, 2020
codebytere pushed a commit that referenced this pull request Jun 30, 2020
Adds configuration to allow building an MSI installer for Windows ARM64.
MSI can be created by running `vcbuild.bat release msi arm64`

Refs: #25998
Refs: #32582

PR-URL: #33689
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
codebytere pushed a commit that referenced this pull request Jul 10, 2020
Adds configuration to allow building an MSI installer for Windows ARM64.
MSI can be created by running `vcbuild.bat release msi arm64`

Refs: #25998
Refs: #32582

PR-URL: #33689
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@codebytere codebytere mentioned this pull request Jul 13, 2020
codebytere pushed a commit that referenced this pull request Jul 14, 2020
Adds configuration to allow building an MSI installer for Windows ARM64.
MSI can be created by running `vcbuild.bat release msi arm64`

Refs: #25998
Refs: #32582

PR-URL: #33689
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
install Issues and PRs related to the installers. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants