Repository navigation
Remove the node-gyp source-build path from the npm package - #1430
Conversation
The opencc npm package now installs its native addon exclusively through
the Bazel-built @opencc/opencc-<platform>-<arch> scoped packages. The
binding.gyp source-build fallback, its gypi definitions, and the npm gyp
dependency chain are removed; the tarball no longer ships C++ sources and
shrinks from several megabytes to ~600 KB. Platforms without a scoped
binary package are no longer supported by npm install.
Detailed Changes:
- **package.json / package-lock.json**:
- Drop the install and prebuild scripts and the node-gyp, node-gyp-build,
node-addon-api, and prebuildify dependencies (node-addon-api comes from
the Bazel Central Registry for the Bazel build).
- Prune the files whitelist to the JavaScript API, CLI, typings,
prebuilds/assets, and the two publish verification scripts.
- Drop the overrides block; the overridden packages left the tree with
the gyp dependency chain.
- **Deleted files**:
- binding.gyp, node/global.gypi, node/configs.gypi, node/dicts.gypi,
node/node_opencc.gypi, node/dict.js, scripts/install.js.
- **scripts/verify-node-main-package.js**:
- Replace the gyp source and #include closure walk with a static
required-files check against the files whitelist.
- **.github/workflows/nodejs.yml**:
- Replace the pack-source-install job with pack-install: pack the main
tarball plus the Bazel-built scoped binary package for the host
platform, install both, and smoke test.
- **.appveyor.yml**:
- The npm packaging job builds the win32-x64 prebuild with Bazel
(bazelisk via choco) on a VS2022 worker instead of the node-gyp
install build and prebuildify, and tests with PREBUILDS_ONLY=1.
- **CMakeLists.txt**:
- Drop the /binding.gyp source-package exclusion.
- **Documentation**:
- Update README.md, node/README.md, node/README.zh.md,
node/PUBLISHING.md, CONTRIBUTING.md, and AGENTS.md to describe the
prebuilt-only install model; record the change in NEWS.md; refresh
comments in node/opencc.js, node/BUILD.bazel, node/test/BUILD.bazel,
and node/test/test_entry.js that referenced node-gyp-build.
Install bazelisk via npm (@bazel/bazelisk provides both `bazel` and `bazelisk` on the npm global bin directory, which is already on PATH, unlike choco shims installed mid-build), verify with `bazel --version`, and invoke the build script through Git Bash explicitly since bare `bash` may resolve to the WSL launcher on AppVeyor Windows images.
The dictionary-generation py_binary genrules failed on AppVeyor because rules_python's venv bootstrap calls os.symlink, which needs Developer Mode (enabled on GitHub runner images but not AppVeyor workers). Enable the Developer Mode registry key like the GitHub images do, and pin bootstrap_impl=system_python via .bazelrc.user so py_binary targets run without creating a venv at all.
|
@codex review |
There was a problem hiding this comment.
Pull request overview
此 PR 延續 #1426，完成 opencc npm 套件移除 node-gyp / gyp source-build fallback 的計畫，改為 只 透過 Bazel 產出的 @opencc/opencc-<platform>-<arch> scoped binary packages 提供原生 addon，並同步調整打包驗證、CI 與文件以符合「prebuilt-only」安裝模型。
Changes:
- 移除 npm 套件中的 node-gyp 來源編譯路徑與相關檔案/相依（
binding.gyp、多個.gypi、scripts/install.js、node-gyp 依賴鏈）。 - 更新 npm 打包驗證與資產準備腳本，聚焦於
files白名單與 Bazel 產物（prebuilds/assets）。 - 更新 GitHub Actions / AppVeyor 的 npm 打包安裝 smoke test 流程與多份文件，說明不再支援無 scoped binary 的平台。
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/verify-node-main-package.js | 改以靜態 required-files + package.json#files 白名單檢查取代 gyp include closure 驗證。 |
| scripts/prepare-node-prebuild-artifacts.js | 更新錯誤訊息與資產準備流程，指向 Bazel dictionary 產物。 |
| scripts/install.js | 刪除：移除 npm install 階段的 node-gyp-build / fallback 邏輯。 |
| README.md | 更新 npm 安裝與平台支援敘述（prebuilt-only）。 |
| package.json | 移除 install/prebuild scripts 與 node-gyp 依賴鏈；縮減 files 白名單並保留 scoped optionalDependencies。 |
| package-lock.json | 移除 node-gyp / node-gyp-build / node-addon-api / prebuildify 等依賴後的 lockfile 縮減與更新。 |
| node/test/test_entry.js | 更新註解，移除 node-gyp-build 相關描述。 |
| node/test/BUILD.bazel | 更新註解，移除 node-gyp-build 相關描述。 |
| node/README.zh.md | 更新安裝與平台支援敘述（prebuilt-only）。 |
| node/README.md | 更新安裝與平台支援敘述（prebuilt-only）。 |
| node/PUBLISHING.md | 更新發佈流程與 CI 形狀說明，明確表示已移除 source-build fallback。 |
| node/opencc.js | 更新註解，反映不再依賴 node-gyp-build；維持現有 addon 查找策略。 |
| node/node_opencc.gypi | 刪除：移除 gyp 編譯定義。 |
| node/global.gypi | 刪除：移除 gyp 全域設定。 |
| node/dicts.gypi | 刪除：移除 gyp 字典建置 actions。 |
| node/dict.js | 刪除：移除僅供 gyp actions 使用的字典產生入口。 |
| node/configs.gypi | 刪除：移除 gyp config copy 定義。 |
| node/BUILD.bazel | 更新註解，移除與 node-gyp 的對照敘述。 |
| NEWS.md | 新增 ## Unreleased，記錄 Node.js npm 安裝模型的 breaking change。 |
| CONTRIBUTING.md | 更新 Node.js 開發/測試指引，移除 node-gyp fallback 描述。 |
| CMakeLists.txt | 移除 source package ignore 中對 /binding.gyp 的排除項（檔案已刪）。 |
| binding.gyp | 刪除：移除頂層 gyp 入口檔。 |
| AGENTS.md | 更新專案概要，反映 Node addon 以 Bazel 建置且無 node-gyp fallback。 |
| .github/workflows/nodejs.yml | 將 pack-source-install 改為 pack-install：同時 pack 主套件與 scoped binary，模擬 release 安裝。 |
| .appveyor.yml | Windows npm 包裝 job 改為以 Bazel（bazelisk）建置 prebuild，並用 PREBUILDS_ONLY=1 測試。 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
npm install itself succeeds on platforms without a scoped binary package (there is no install script anymore); the failure surfaces when requiring opencc. Reword the READMEs, NEWS.md, PUBLISHING.md, AGENTS.md, and the workflow comment accordingly, and add TWPhrases.ocd2 / TWVariants.ocd2 (referenced by the spot-checked s2twp.json) to the prepack required-files list.
The Bazel dictionary genrules run rules_python py_binary tools whose venv bootstrap calls os.symlink, which keeps failing on AppVeyor workers (WinError 87) even with the Developer Mode registry key, and the bootstrap_impl override does not reach the exec configuration. Build only the addon (//node:opencc) with Bazel, move the jieba CMake build from after_test to build_script, and assemble prebuilds/assets from its install tree, where data/scripts run on the system Python directly — the same dictionary pipeline the AppVeyor winget and Python jobs already use.
On platforms without an @opencc/opencc-<platform>-<arch> scoped binary
package, npm install now builds //node:opencc from source with Bazel and
stages it under prebuilds/<platform>-<arch>/, instead of leaving the
package unusable. Bazel is taken from PATH (bazel or bazelisk) or fetched
automatically through npx; dictionaries always ship prebuilt in the
tarball, so the source build compiles only the addon and needs no Python.
Detailed Changes:
- **scripts/install.js**:
- Reinstate the install script: skip in source checkouts, accept an
installed scoped binary package, otherwise run
`bazel --max_idle_secs=10 build -c opt //node:opencc` and stage the
addon; `npm install --build-from-source` forces the local compile.
- **package.json**:
- Restore the install script and ship the Bazel workspace in the
tarball: .bazelignore/.bazelrc/.bazelversion, MODULE.bazel(.lock),
patches/, the whole node/ and src/ directories (all C++ dependencies
come from the Bazel Central Registry, so deps/ stays excluded).
- **scripts/verify-node-main-package.js**:
- Require the source-build files in the prepack check.
- **.github/workflows/nodejs.yml**:
- Add a pack-source-install job installing the packed tarball without
any scoped package, exercising the Bazel install-time build on
ubuntu/macos/macos-intel.
- **Documentation**:
- Update README.md, node/README.md, node/README.zh.md,
node/PUBLISHING.md, AGENTS.md, and the NEWS.md entry to describe the
Bazel source-build fallback.
The Bazel source-build fallback now mirrors a full prebuild instead of compiling only the addon: npm install builds //data/dictionary:binary_dictionaries alongside //node:opencc and refreshes prebuilds/assets from the freshly generated dictionaries, with Bazel providing the hermetic Python toolchain for the generation scripts. The tarball ships data/config, data/dictionary, and data/scripts for this; prebuilt assets still ship for the scoped-package fast path.
The install-time source build failed from the npm tarball because //src/tools:command_line_main_lib (in the dictionary-generation graph via //src/tools:command_line) still depended on the vendored //deps/rapidjson-1.1.0 package, which the tarball does not ship. Switch it and the data/config validation tests to @rapidjson from the Bazel Central Registry, matching //src:config_lib; the only remaining vendored-deps references in Bazel are the Windows-Zig genrule sources.
Automated code reviewReviewed PR #1430, which removes the node-gyp FindingsNo actionable defects found. Files inspected
Reviewed head |
|
上方自動 review 的行為描述有兩處與本 PR 實際內容不符,為免誤導 reviewer 澄清一下:
其「無缺陷」的結論與本 PR 的 CI 結果一致。 Generated by Claude Code |
|
使用者在無 scoped prebuilt binary 平台安裝時，解開的 tarball 裡已經有一份 prebuilds/assets/。因此重新 build ocd2 檔案是冗餘的（ 如果使用者需要從 txt 重新構建詞典，可以使用 opencc_dict 工具。 另外：合併前建議重新用 npm install --package-lock-only 生成一次 package-lock.json，以防其中的 hasInstallScript: true 與 package.json 的定義发生冲突。 |
Regenerate package-lock.json so the root entry carries hasInstallScript: true, matching the reinstated install script, and note in scripts/install.js why the source build regenerates dictionaries: the .ocd2 format is endianness-dependent (marisa trie dump and SerializedValues both use native-endian integers), so the shipped little-endian assets cannot serve big-endian fallback platforms.
|
@frankslin 謝謝 review,兩點分別處理如下: package-lock.json:確認屬實——根條目缺 安裝時重建字典:查證後這一步不是冗餘,關鍵在字節序:
Generated by Claude Code |
|
其實現今基本上很少見 big-endian 的平台了，所以只支持 little-endian 或許沒有太大的問題，也不值得為這樣的小眾平台作單獨適配。如果確有 endianness 這方面的顧慮，不論 ocd2 還是 ocd 均是不可跨平台使用的。 或許作為 distribution format，直接改用 txt 格式更理想？也可以考慮將 swappable config+dictionary assets 單獨打包，讓 npm opencc 安裝者每次都能獲取最新的 opencc-data？當然這些架構問題可以後續再考慮，不屬於本 PR 範疇。 |
|
同意這些屬於後續架構討論、不在本 PR 範疇。本 PR 維持現狀:找不到 scoped 預編譯包時完整源碼構建(addon + 從 txt 重建字典),以正確性優先——重建成本只落在需要 fallback 的小眾平台上,常見平台走 scoped 包快路徑不受影響。 兩個方向都很有價值,適合後續獨立 issue 追蹤:
需要的話我可以開 issue 記錄這兩點。本 PR 就聚焦在移除 node-gyp / 切換到 Bazel 這件事上收尾。 Generated by Claude Code |
Summary
Follow-up to #1426, completing the gyp removal plan (steps 2–5): the node-gyp source-build path is fully removed. The
openccnpm package installs its native addon through the Bazel-built@opencc/opencc-<platform>-<arch>scoped packages, and on platforms without one,npm installruns a full Bazel source build (scripts/install.js): it compiles//node:opencc, regenerates the dictionaries (//data/dictionary:binary_dictionaries), stages the addon underprebuilds/<platform>-<arch>/, and refreshesprebuilds/assets.binding.gyp, the.gypibuild definitions, and thenode-gyp/node-gyp-build/node-addon-api/prebuildifynpm dependency chain are all removed.Behavior changes:
bazel/bazeliskfrom PATH, or bazelisk fetched automatically vianpx; plus a C++ toolchain and network access. Python for the dictionary scripts comes from Bazel's hermetic toolchain — no system Python required (node-gyp needed one). Without Bazel available, the install fails with instructions.MODULE.bazel(.lock),.bazelrc,.bazelversion,patches/), thenode/+src/sources, and thedata/config/dictionary sources instead of gyp files anddeps/(all C++ dependencies come from the Bazel Central Registry). Prebuilt assets still ship for the scoped-package fast path. Size: ~1.4 MB packed.Changes
scripts/install.js: rewritten around Bazel — skip in source checkouts, accept an installed scoped binary package, otherwisebazel --max_idle_secs=10 build -c opt //data/dictionary:binary_dictionaries //node:opencc, stage the addon, and regenerateprebuilds/assets;npm install --build-from-sourceforces the local build.package.json/package-lock.json: drop theprebuildscript and the gyp dependency chain;filesships runtime JS, typings,prebuilds/assets, and the Bazel workspace +src/+data/sources; drop the now-unusedoverridesblock. The lockfile shrinks to ~20 packages.binding.gyp,node/global.gypi,node/configs.gypi,node/dicts.gypi,node/node_opencc.gypi,node/dict.js.scripts/verify-node-main-package.js: static prepack check covering the runtime files, spot-checked dictionaries, and the source-build files (Bazel workspace, sources, dictionary scripts)..github/workflows/nodejs.yml:pack-installpacks the tarball plus the host scoped binary package and smoke-tests the prebuilt path;pack-source-installinstalls the tarball without any scoped package, exercising the full install-time Bazel build (addon + dictionaries) on ubuntu / macos / macos-intel..appveyor.yml: the npm packaging job moves to a Visual Studio 2022 worker, builds the addon with Bazel (//node:opencconly) and takes dictionaries/configs from the jieba CMake install — the Bazel dictionary genrules need rules_python venv symlinks AppVeyor workers lack. Tests run withPREBUILDS_ONLY=1.CMakeLists.txt: drop the/binding.gypsource-package exclusion.## Unreleased.Testing
npm test: 1515 pass / 0 fail, default and withPREBUILDS_ONLY=1.scripts/install.jsbranch coverage verified locally: source-checkout skip, scoped-package short-circuit, and the clear error when nobazel/bazelisk/npxis available; the full install-time Bazel build (addon + dictionary regeneration) is covered by thepack-source-installCI job.npm packinspected: ~1.4 MB packed, contains the Bazel workspace,node/,src/,data/, assets; no gyp files, nodeps/.pack-install(scoped path), and the addon-only variant ofpack-source-installon all three platforms.Review notes
## Unreleasedheading; rename to the release version when cutting it.🤖 Generated with Claude Code
https://claude.ai/code/session_011sQk5XBhgd3BcgbfMWUmva