GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||
GeezerVison is showing github.com — msi: migrate to WiX4 by StefanStojanovic · Pull Request #45943 · nodejs/node Conversation
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
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
StefanStojanovic
force-pushed
the
mefi-wix-migration-wixproj
branch
2 times, most recently
from
5474db7 to
17cdadc
Compare
StefanStojanovic
added a commit
to JaneaSystems/build
that referenced
this pull request
This is required to migrate from WiX3 to WiX4 for building the Node msi. Refs: nodejs/node#45943
StefanStojanovic
force-pushed
the
mefi-wix-migration-wixproj
branch
from
17cdadc to
e3abcc7
Compare
Contributor
Author
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
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
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
approved these changes
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
|
There was an error while loading. Please reload this page.
|
Let me make a test release. If that works fine, I'm ready to rubberstamp this PR. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
https://nodejs.org/download/test/v20.0.0-test20230317e3abcc7400/ Can someone test with a Windows sandbox? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Is this semver-patch? Is there a risk to break upgrades from installs done with a previous MSI? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I also just tested it myself (x64) with an upgrade from v18. Looks good! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@targos, since neither Joao nor I are collaborators, can you please land this? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Landed in 0b66df6 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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]>
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]>
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]>
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
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
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]>
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
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
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]>
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]>
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]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.