feat(workflow): node-gyp rebuild for nodejs packages; re-apply gl/sharp .node injection (#858) - #867
feat(workflow): node-gyp rebuild for nodejs packages; re-apply gl/sharp .node injection (#858)#867looopmax wants to merge 4 commits into
Conversation
ced0a12 to
1222b9b
Compare
bbdf533 to
1222b9b
Compare
wuzhiming
left a comment
There was a problem hiding this comment.
Two items from reviewing the release-pipeline side of this. The node-gyp-rebuild.js one is blocking — the new nodejs rebuild step can't run at all as written. Details inline.
| @@ -0,0 +1,14 @@ | |||
| const { execSync } = require('child_process'); | |||
There was a problem hiding this comment.
Blocking: this file never reaches the release directory, so rebuild:node-gyp always fails
.vscodeignore whitelists only three .js files under workflow/:
workflow/!(postinstall|utils|electron-rebuild).js
release.js builds its copy list from exactly those patterns (readIgnorePatterns -> scanProjectFiles), so workflow/node-gyp-rebuild.js is filtered out and never lands in the publish dir. Running the pipeline's own globby call on this branch, these are the only workflow/ entries that get copied:
workflow/electron-rebuild.js
workflow/generate-dts-postprocess.ts
workflow/generate-dts-worker.mjs
workflow/generate-dts.ts
workflow/postinstall.js
workflow/utils.js
Repro:
node -e "(async()=>{const {globby}=await import('globby');const fs=require('fs');
const p=fs.readFileSync('.vscodeignore','utf8').split('\n').map(l=>l.trim()).filter(l=>l&&!l.startsWith('#'));
p.push('.publish/**');
const f=await globby(['**/*'],{cwd:process.cwd(),dot:true,ignore:p,onlyFiles:true});
console.log(f.filter(x=>x.startsWith('workflow/')));})()"So npm run rebuild:node-gyp in the release dir exits with MODULE_NOT_FOUND, and runCommand rejects on any non-zero exit (workflow/utils.js) — which fails the entire nodejs release pipeline, not just this step.
Two ways to fix:
- Whitelist the new file:
workflow/!(postinstall|utils|electron-rebuild|node-gyp-rebuild).js - Or drop the new file and add a flag to
electron-rebuild.js, which is already whitelisted — one less thing to keep in sync betweenworkflow/and.vscodeignore:withrun('npx --yes patch-package'); if (process.argv.includes('--node')) { run('npm rebuild'); } else { run(`npx @electron/rebuild --force --version ${electronVersion}`); }
"rebuild:node-gyp": "node workflow/electron-rebuild.js --node".
Either way, could you verify with an actual --nodejs release rather than npm run rebuild alone? The Verification section only exercises the electron path, which is the one that already worked — the new path is the one that's broken.
| @@ -0,0 +1,18 @@ | |||
| diff --git a/node_modules/sharp/lib/sharp.js b/node_modules/sharp/lib/sharp.js | |||
There was a problem hiding this comment.
sharp is a caret range, but the patch filename pins 0.32.6 — and a stale patch here fails silently
package.json declares "sharp": "^0.32.6", while patch-package resolves patches by exact <name>+<version> in the filename. Once a fresh install resolves 0.32.7+, two things can happen, and from patch-package@8.0.1's applyPatches.js neither is loud in our setup:
- Mismatch, patch still applies -> goes to
warnings. Exit code stays 0 unless--error-on-warnis passed. - Mismatch, patch fails to apply -> goes to
errorsviacreatePatchApplicationFailureError, but the exit code is still 0 unlessshouldExitWithErroris true, which requires--error-on-fail,NODE_ENV=test, orci-info'sisCI.
npm run release isn't invoked from any workflow in .github/workflows/, so releases are cut manually and isCI is false. Net effect: a patch can fail outright and the release still completes green — COCOS_CLI_SHARP_NODE would then be silently ignored at runtime, which is exactly the failure mode this PR exists to prevent.
Suggest both:
- Pin the exact version:
"sharp": "0.32.6" - Add
--error-on-failtopatch-packagein both rebuild scripts, so a stale or broken patch fails the release instead of shipping quietly
The second is worth doing independent of this PR — the same silent-failure exposure already covers gl (^9.0.0-rc.10) and nan. sharp just makes it a third patch riding on it.
Changes
1. feat(workflow): rebuild native node-gyp modules for nodejs packages (c1267fb)
During release, rebuild native modules for nodejs-type packages via patch-package + npm rebuild.
workflow/node-gyp-rebuild.jsrebuild:node-gypnpm script2. re-apply gl/sharp native .node injection via env vars (#858)
Re-applies the changes from #858 (previously reverted wholesale in #865; this is an exact revert-of-revert):
.gitignore: ignoreperf/docs/dev/native-modules-injection.md: new doc describing the gl/sharp native binding injection mechanismpatches/gl+9.0.0-rc.10.patch:native-gl.jssupports injection via theCOCOS_CLI_GL_NODEenv varpatches/sharp+0.32.6.patch: new patch, supports injection via theCOCOS_CLI_SHARP_NODEenv varDifference from the original PR: the #858 change to
workflow/release.js(unconditionalnpm run rebuild) is not re-applied, because this branch'srebuild:node-gypstep already covers that need, and both rebuild paths runpatch-package.Verification
npm run rebuild(patch-package + @electron/rebuild 42.3.0) passes; all three patches (gl / nan / sharp) apply successfullyCOCOS_CLI_GL_NODE/COCOS_CLI_SHARP_NODEset, requiring the corresponding module prints the injection log and loads the specified.node; without them, behavior is identical to before