FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Allow creating MSI installers for Windows ARM64 by dennisameling · Pull Request #33689 · nodejs/node · GitHub

/ node Public

Allow creating MSI installers for Windows ARM64 - #33689

Closed
dennisameling wants to merge 3 commits into
nodejs:masterfrom
dennisameling:add-windows-arm-msi-config
ConversationCommits (3)ChecksFiles changed
Closed

Allow creating MSI installers for Windows ARM64#33689
dennisameling wants to merge 3 commits into
nodejs:masterfrom
dennisameling:add-windows-arm-msi-config

Conversation

dennisameling commented Jun 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

win,msi: add arm64 config for windows msi
1fd576d 
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
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
dennisameling mentioned this pull request Jun 1, 2020
Closed

addaleax commented Jun 1, 2020

Copy link
Copy Markdown
Member

@nodejs/platform-windows

dennisameling commented Jun 1, 2020
View reviewed changes
Comment thread tools/msvs/msi/nodemsi.sln Outdated
dennisameling commented Jun 1, 2020
View reviewed changes
Comment thread tools/msvs/msi/nodemsi.wixproj Outdated
dennisameling commented Jun 1, 2020
View reviewed changes
Comment thread tools/msvs/msi/nodemsi.sln Outdated

joaocgreis commented Jun 8, 2020

Copy link
Copy Markdown
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 commented Jun 8, 2020

Copy link
Copy Markdown
Contributor Author

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

dennisameling commented Jun 8, 2020

Copy link
Copy Markdown
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 commented Jun 9, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31788/

joaocgreis approved these changes Jun 9, 2020
View reviewed changes

joaocgreis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Jun 9, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31791/

bzoz commented Jun 9, 2020

Copy link
Copy Markdown
Contributor

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
win,msi: add arm64 config for windows msi
35871c3 
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 commented Jun 9, 2020

Copy link
Copy Markdown
Contributor

Landed in 35871c3

bzoz closed this Jun 9, 2020
github-actions Bot mentioned this pull request Jun 10, 2020
Open
13 tasks
codebytere pushed a commit that referenced this pull request Jun 18, 2020
win,msi: add arm64 config for windows msi
3520a13 
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>
dennisameling mentioned this pull request Jun 22, 2020
Closed
4 tasks
codebytere mentioned this pull request Jun 28, 2020
Merged
codebytere pushed a commit that referenced this pull request Jun 30, 2020
win,msi: add arm64 config for windows msi
1da3baa 
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
win,msi: add arm64 config for windows msi
4388dad 
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 mentioned this pull request Jul 13, 2020
Merged
codebytere pushed a commit that referenced this pull request Jul 14, 2020
win,msi: add arm64 config for windows msi
7ee0b30 
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
joaocgreis joaocgreis approved these changes
bzoz bzoz approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL