CI (Windows): Use npm (not yarn) to install ppm #185
Merged
Conversation
This file contains 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
Should fix an obscure issue where ppm's postinstall script exits out half-way through without resuming under Yarn, but completes fully under NPM. Probably has something to do with Yarn handling process.exit() differently than NPM does, in the postinstall script/its sub-scripts. (Perhaps Yarn avoids running sub-processes of Node nested as deeply as NPM does? Thus process.exit() exiting a higher Node process that we expect to still have work to do later in the main postinstall script? Not totally sure. This is just a guess.)
confused-Techie
approved these changes
Dec 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Requirements for Contributing a Bug Fix (from template, click to expand):
Identify the Bug
ppm has its dependencies built for Node 14 in Windows CI (and CI artifacts). This makes ppm fail, because the bundled copy of Node in ppm is Node 16 now.
Those dependencies should be built for Node 16 now, after the recent bump to bundled Node 16 in the ppm repo, which was done for Apple Silicon compatibility reasons.
Details of the error message (click to expand):
This PR should fix this particular occurrence of the semi-infamous
NODE_MODULE_VERSIONerror -- which indicates when you're trying to run native C/C++ code compiled against the wrong version of Node/Electron -- a different version than you're currently running.This is saying the module was built for Node 14, but you're attempting to run it with Node 16.
(To decode the meaning of the different
NODE_ABI_VERSIONnumbers, refer to this metadata file: https://github.com/electron/node-abi/blob/main/abi_registry.json. ABI 83 is Node 14, ABI 93 is Node 16.)Description of the Change
Use npm, not Yarn, to install ppm's dependencies.
(Fixes an obscure thing where Yarn exits the postinstall script early, earlier than NPM does.)
Speculating on why this happens in Yarn but not NPM? (Click to expand):
(Probably has something to do with Yarn handling process.exit() differently than NPM does, in ppm's
postinstall.cmd. Specifically, ppm'sscript/download-node.jshas a bit where it callsprocess.exit(), which I assume is handled differently when run under Yarn compared to running it under NPM? Since that's right where postinstall.js exits "early" under Yarn. Npm keeps going till the later part ofpostinstall.cmd.)(Perhaps Yarn avoids running sub-processes of Node nested as deeply as NPM does? Thus process.exit() exiting a higher Node process that we expect to still have work to do later in the main postinstall script? Not totally sure. This is just a guess.)
Alternate Designs
N/A. I didn't consider any alternatives, running it under Yarn didn't work for me locally, but running it under NPM did work for me locally.
Possible Drawbacks
None expected.
The ppm (previously apm) repo was traditionally developed with NPM, and I don't think it has any particular thing that necessitates the use of Yarn, unlike the workarounds Yarn was able to provide for installing the main Pulsar dependencies.
Verification Process
Worked for me locally, hopefully CI produces an artifact with working ppm.
Update:
Noice. (The "Rebuilding apm dependencies with bundled Node" part is working with NPM now in Cirrus CI Windows.)
Release Notes
Fixed ppm (package manager) in Windows CI builds