GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — msi: migrate to WiX4 by StefanStojanovic · Pull Request #45943 · nodejs/node

Conversation

@StefanStojanovic

@StefanStojanovic StefanStojanovic commented Dec 22, 2022

Copy link
Copy Markdown
Contributor

Currently, two versions of the WiX Toolset are used to build the MSI installers. WiX v3.11 is used for x86 and x64, and WiX v3.14 is used for ARM64, which is not a stable release. This PR aims to migrate Node's MSI installer to WiX4 to unify all installers under a single version and remove all dependencies to WiX3, e.g. it can be removed from the CI machines.

In addition, using any WiX3 version requires it to be installed on the machine. Also, the WiX team is dropping support for v3 and they are encouraging users to move to v4.

This PR uses WiX rc-1, released on 16th December 2022. No big changes are expected until GA other than documentation updates and minor fixes. I've tested this extensively, including upgrade scenarios, and I believe this is good to be used on Node as is. Anyway, I plan to update to the stable version when it is released.

In this PR, the way to download WiX is changed to leverage MSBuild, as recommended for WiX v4. This includes an update to BUILDING.md, making it simpler, and is expected to work out of the box in CI machines.

About semver, this should only add support for ARM64. Everything else should stay the same. So, I think this should be semver-minor.

Refs: nodejs/build#2540

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. doc Issues and PRs related to Node.js documentation. install Issues and PRs related to installing Node.js and its installers. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform. labels Dec 22, 2022
@StefanStojanovic StefanStojanovic mentioned this pull request Dec 22, 2022
msi: migrate to WiX4 (WIP) #45456
Closed
@lpinca

lpinca commented Dec 24, 2022

Copy link
Copy Markdown
Member

@nodejs/platform-windows

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@p-b-o

p-b-o commented Jan 16, 2023

Copy link
Copy Markdown
Contributor

@StefanStojanovic We can help with testing on this. Do you have an installer to share (built from that PR) that would could try?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@StefanStojanovic

StefanStojanovic commented Jan 17, 2023

Copy link
Copy Markdown
Contributor Author

@StefanStojanovic We can help with testing on this. Do you have an installer to share (built from that PR) that would could try?

I made a release with the installer on JaneaSystems/node fork here. Anyone from @nodejs/platform-windows-arm, feel free to chip in.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@p-b-o

p-b-o commented Jan 18, 2023

Copy link
Copy Markdown
Contributor

I tried your installer on a Windows on Arm machine, and it works great. Node is added to PATH, and is accessible from any prompt. Great work 👍

StefanStojanovic reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@StefanStojanovic

StefanStojanovic commented Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

An update on WiX v4: release candidate 2 was released on January 20th. I've tried it and it generated the same MSI file as release candidate 1. Since there are no notable changes for the Node.js installer, this PR will keep relying on the RC1.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@StefanStojanovic
StefanStojanovic force-pushed the mefi-wix-migration-wixproj branch 2 times, most recently from 5474db7 to 17cdadc Compare March 1, 2023 20:23
StefanStojanovic added a commit to JaneaSystems/build that referenced this pull request Mar 2, 2023
@StefanStojanovic
ansible: add .NET SDK on Windows
d7bf6c5
This is required to migrate from WiX3 to WiX4 for building the Node msi.

Refs: nodejs/node#45943
@StefanStojanovic StefanStojanovic mentioned this pull request Mar 2, 2023
Windows ansible updates nodejs/build#3211
Merged
@StefanStojanovic
StefanStojanovic force-pushed the mefi-wix-migration-wixproj branch from 17cdadc to e3abcc7 Compare March 2, 2023 14:36
@StefanStojanovic

StefanStojanovic commented Mar 2, 2023

Copy link
Copy Markdown
Contributor Author

Just a quick update - I've made a few changes, the most important one regarding the NuGet.Config file, which makes sure nodemsi.sln is able to resolve WixToolset.SDK regardless of the NuGet configuration on the machine it's being built on. I've also tested everything in the CI and the job passed https://ci.nodejs.org/job/node-test-commit-windows-fanned/53633/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

StefanStojanovic added a commit to JaneaSystems/build that referenced this pull request Mar 8, 2023
@StefanStojanovic
ansible: add .NET SDK on Windows
a7aa3c6
This is required to migrate from WiX3 to WiX4 for building the Node msi.

Refs: nodejs/node#45943
PR-URL: nodejs#3211
Reviewed-By: Richard Lau <[email protected]>
StefanStojanovic added a commit to nodejs/build that referenced this pull request Mar 8, 2023
@StefanStojanovic
ansible: add .NET SDK on Windows
7731bb7
This is required to migrate from WiX3 to WiX4 for building the Node msi.

Refs: nodejs/node#45943
PR-URL: #3211
Reviewed-By: Richard Lau <[email protected]>
joaocgreis
joaocgreis approved these changes Mar 17, 2023
View reviewed changes

@joaocgreis joaocgreis left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I did discuss the implementation with @StefanStojanovic, but reviewed this independently and I believe this is correct.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@joaocgreis

joaocgreis commented Mar 17, 2023

Copy link
Copy Markdown
Member

@nodejs/tsc what is the best way of moving this forward? How can we help?

We would like to bring ARM64 Windows to tier 2 support, and this is needed to be able to build an MSI installer.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos

targos commented Mar 17, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Let me make a test release. If that works fine, I'm ready to rubberstamp this PR.

niyas-sait reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos

targos commented Mar 17, 2023

Copy link
Copy Markdown
Member

https://nodejs.org/download/test/v20.0.0-test20230317e3abcc7400/

Can someone test with a Windows sandbox?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos

targos commented Mar 17, 2023

Copy link
Copy Markdown
Member

Is this semver-patch? Is there a risk to break upgrades from installs done with a previous MSI?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joaocgreis

joaocgreis commented Mar 17, 2023

Copy link
Copy Markdown
Member

Per semver definition, this should be minor. The only visible change is to add support for ARM64 MSI, so it only adds functionality. Though the change is contained to the ARM64 Windows platform, so could it be patch?

As with any change, there is risk that there is some configuration out there where this breaks. However, this only changes the software used to build the MSI. The MSI itself should be very similar (we checked). There were no changes in the mechanism that handles upgrades. We tested the upgrade scenario and found no issues.

@targos I checked the test release and it worked well for me (upgraded from latest v19). Would be good to have other people test it as well!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos

targos commented Mar 17, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I also just tested it myself (x64) with an upgrade from v18. Looks good!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos
targos approved these changes Mar 18, 2023
View reviewed changes
@StefanStojanovic

StefanStojanovic commented Mar 20, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@targos, since neither Joao nor I are collaborators, can you please land this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

richardlau
richardlau approved these changes Mar 22, 2023
View reviewed changes
@richardlau richardlau added commit-queue PRs queued for automated landing through the Commit Queue. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Mar 22, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Mar 22, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 0b66df6 into nodejs:main Mar 22, 2023
@nodejs-github-bot

nodejs-github-bot commented Mar 22, 2023

Copy link
Copy Markdown
Collaborator

Landed in 0b66df6

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot mentioned this pull request Mar 23, 2023
CI Reliability 2023-03-23 nodejs/reliability#533
Open
33 tasks
StefanStojanovic added a commit to JaneaSystems/build that referenced this pull request Mar 23, 2023
@StefanStojanovic
jenkins: add ARM64 Windows MSI
c64b51c
Refs: nodejs/node#45943
StefanStojanovic added a commit to JaneaSystems/build that referenced this pull request Mar 23, 2023
@StefanStojanovic
jenkins: add ARM64 Windows MSI
c6570ba
Refs: nodejs/node#45943
@StefanStojanovic StefanStojanovic mentioned this pull request Mar 23, 2023
Adding ARM64 Windows MSI to Jenkins nodejs/build#3251
Merged
StefanStojanovic added a commit to JaneaSystems/build that referenced this pull request Mar 27, 2023
@StefanStojanovic
jenkins: add ARM64 Windows MSI
1ace7a4
Refs: nodejs/node#45943
PR-URL: nodejs#3251
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
StefanStojanovic added a commit to nodejs/build that referenced this pull request Mar 27, 2023
@StefanStojanovic
jenkins: add ARM64 Windows MSI
3aa5e1b
Refs: nodejs/node#45943
PR-URL: #3251
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Apr 3, 2023
2023-04-18, Version 20.0.0 (Current) #47381
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@StefanStojanovic @RafaelGSS
msi: migrate to WiX4
915f34d
To be able to build x86, x64, and ARM64 MSI installers with the same WiX
version, migration to WiX4 is required.

PR-URL: #45943
Refs: nodejs/build#2540
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
v19.9.0 proposal #47441
Merged
RafaelGSS added a commit that referenced this pull request Apr 6, 2023
@RafaelGSS
2023-04-07, Version 19.9.0 (Current)
75642ba
Notable changes:

events:
  * (SEMVER-MINOR) add getMaxListeners method (Khafra) #47039
lib:
  * (SEMVER-MINOR) add tracing channel to diagnostics\_channel (Stephen Belanger)
msi:
  * (SEMVER-MINOR) migrate to WiX4 (Stefan Stojanovic) #45943
node-api:
  * (SEMVER-MINOR) deprecate napi\_module\_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (Robert Nagy) #46929
url:
  * (SEMVER-MINOR) implement URL.canParse (Khafra) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: TBD
RafaelGSS added a commit that referenced this pull request Apr 6, 2023
@RafaelGSS
2023-04-07, Version 19.9.0 (Current)
c68eb32
Notable changes:

events:
  * (SEMVER-MINOR) add getMaxListeners method (Khafra) #47039
lib:
  * (SEMVER-MINOR) add tracing channel to diagnostics\_channel (Stephen Belanger)
msi:
  * (SEMVER-MINOR) migrate to WiX4 (Stefan Stojanovic) #45943
node-api:
  * (SEMVER-MINOR) deprecate napi\_module\_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (Robert Nagy) #46929
url:
  * (SEMVER-MINOR) implement URL.canParse (Khafra) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #47441
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
@StefanStojanovic @RafaelGSS
msi: migrate to WiX4
c03e79b
To be able to build x86, x64, and ARM64 MSI installers with the same WiX
version, migration to WiX4 is required.

PR-URL: #45943
Refs: nodejs/build#2540
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
RafaelGSS added a commit that referenced this pull request Apr 7, 2023
@RafaelGSS
2023-04-07, Version 19.9.0 (Current)
5402535
Notable changes:

events:
  * (SEMVER-MINOR) add getMaxListeners method (Khafra) #47039
lib:
  * (SEMVER-MINOR) add tracing channel to diagnostics\_channel (Stephen Belanger)
msi:
  * (SEMVER-MINOR) migrate to WiX4 (Stefan Stojanovic) #45943
node-api:
  * (SEMVER-MINOR) deprecate napi\_module\_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (Robert Nagy) #46929
url:
  * (SEMVER-MINOR) implement URL.canParse (Khafra) #47179
wasi:
  * (SEMVER-MINOR) no longer require flag to enable wasi (Michael Dawson) #47286

PR-URL: #47441
RafaelGSS added a commit that referenced this pull request Apr 8, 2023
@RafaelGSS
2023-04-10, Version 19.9.0 (Current)
d2a7e91
Notable changes:

events:
  * (SEMVER-MINOR) add getMaxListeners method (Khafra) #47039
lib:
  * (SEMVER-MINOR) add tracing channel to diagnostics\_channel (Stephen Belanger)
msi:
  * (SEMVER-MINOR) migrate to WiX4 (Stefan Stojanovic) #45943
node-api:
  * (SEMVER-MINOR) deprecate napi\_module\_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (Robert Nagy) #46929
url:
  * (SEMVER-MINOR) implement URL.canParse (Khafra) #47179

PR-URL: #47441
RafaelGSS added a commit that referenced this pull request Apr 10, 2023
@RafaelGSS
2023-04-10, Version 19.9.0 (Current)
8085bcf
Notable changes:

events:
  * (SEMVER-MINOR) add getMaxListeners method (Khafra) #47039
lib:
  * (SEMVER-MINOR) add tracing channel to diagnostics\_channel (Stephen Belanger)
msi:
  * (SEMVER-MINOR) migrate to WiX4 (Stefan Stojanovic) #45943
node-api:
  * (SEMVER-MINOR) deprecate napi\_module\_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (Robert Nagy) #46929
url:
  * (SEMVER-MINOR) implement URL.canParse (Khafra) #47179
test_runner:
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238

PR-URL: #47441
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS added a commit that referenced this pull request Apr 11, 2023
@RafaelGSS
2023-04-10, Version 19.9.0 (Current)
c8f6f85
Notable changes:

events:
  * (SEMVER-MINOR) add getMaxListeners method (Khafra) #47039
lib:
  * (SEMVER-MINOR) add tracing channel to diagnostics\_channel (Stephen Belanger)
msi:
  * (SEMVER-MINOR) migrate to WiX4 (Stefan Stojanovic) #45943
node-api:
  * (SEMVER-MINOR) deprecate napi\_module\_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (Robert Nagy) #46929
url:
  * (SEMVER-MINOR) implement URL.canParse (Khafra) #47179
test_runner:
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238

PR-URL: #47441
Signed-off-by: RafaelGSS <[email protected]>
RafaelGSS added a commit that referenced this pull request Apr 13, 2023
@RafaelGSS
2023-04-10, Version 19.9.0 (Current)
4c8a25b
Notable changes:

events:
  * (SEMVER-MINOR) add getMaxListeners method (Khafra) #47039
lib:
  * (SEMVER-MINOR) add tracing channel to diagnostics\_channel (Stephen Belanger)
msi:
  * (SEMVER-MINOR) migrate to WiX4 (Stefan Stojanovic) #45943
node-api:
  * (SEMVER-MINOR) deprecate napi\_module\_register (Vladimir Morozov) #46319
stream:
  * (SEMVER-MINOR) add setter & getter for default highWaterMark (Robert Nagy) #46929
url:
  * (SEMVER-MINOR) implement URL.canParse (Khafra) #47179
test_runner:
  * (SEMVER-MINOR) expose reporter for use in run api (Chemi Atlow) #47238

PR-URL: #47441
Signed-off-by: RafaelGSS <[email protected]>
@danielleadams danielleadams added the backport-requested-v18.x label Jun 12, 2023
@aaronpowell aaronpowell mentioned this pull request Jun 19, 2023
Install-NodeVersion 20 Fails aaronpowell/ps-nvm#101
Closed
@richardlau richardlau mentioned this pull request Aug 10, 2023
vs2019 failing nodejs/build#3442
Closed
@asklar asklar mentioned this pull request May 26, 2026
MSI installer strips inherited "ALL APPLICATION PACKAGES" ACE from install directory #63590
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@richardlau richardlau richardlau approved these changes
+2 more reviewers
@joaocgreis joaocgreis joaocgreis approved these changes
@targos targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

build Issues and PRs related to Node.js builds or CI infrastructure. commit-queue-squash PRs the Commit Queue should land as one squashed commit. doc Issues and PRs related to Node.js documentation. install Issues and PRs related to installing Node.js and its installers. meta Issues and PRs related to the general management of the project. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version. 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.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@StefanStojanovic @lpinca @p-b-o @joaocgreis @targos @nodejs-github-bot @richardlau @danielleadams
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.