Replace prebuild/prebuild-install with prebuildify + node-gyp-build#44
Open
arv wants to merge 3 commits into
Open
Replace prebuild/prebuild-install with prebuildify + node-gyp-build#44arv wants to merge 3 commits into
arv wants to merge 3 commits into
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
arv
marked this pull request as ready for review
July 22, 2026 09:40
There was a problem hiding this comment.
Pull request overview
This PR migrates the native addon packaging and installation flow away from prebuild-install (and its large/deprecated dependency tree) to the recommended prebuildify + node-gyp-build approach, and updates the shell wrapper + CI to ship and publish prebuilds inside the npm tarball.
Changes:
- Replace runtime native binding resolution (
bindings) withnode-gyp-buildin library code and tests. - Add a
prebuilds/**package payload and updateshell.jsto fall back to a packaged shell binary when present. - Rework the release CI to build
prebuilds/, upload them as artifacts, and merge them into the publish job (instead of uploading/downloading release assets).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test/10.database.open.js | Updates the nativeBinding test to use node-gyp-build’s resolved binding path. |
| shell.js | Adds a prebuilds-based fallback lookup for the packaged shell binary. |
| package.json | Switches deps/scripts to node-gyp-build + prebuildify and includes prebuilds/** in published files. |
| lib/database.js | Loads the addon via node-gyp-build instead of bindings. |
| deps/copy-shell.js | New helper to copy the built shell executable into prebuilds/<platform>-<arch>/. |
| .gitignore | Ignores generated prebuilds/ output. |
| .github/workflows/build.yml | Updates prebuild commands and publishes merged prebuilds/ artifacts on release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Prompted by WiseLibs/better-sqlite3#655: prebuild-install pulls in a large tree of deprecated transitive dependencies (npmlog, gauge, rc, request, tar-fs 2, ...) and downloads binaries from GitHub releases at install time. With prebuildify, all prebuilt binaries ship inside the npm package under prebuilds/<platform>-<arch>/ and node-gyp-build (zero dependencies) resolves the right one at runtime, so installs need no network access and no install scripts on supported platforms. - runtime deps drop from bindings + prebuild-install (~40 packages) to just node-gyp-build (0 transitive deps) - install script is now plain node-gyp-build: use prebuild if compatible, else reuse existing build/Release, else compile (same fallback chain) - binaries stay per-ABI (--no-napi) since better-sqlite3 uses V8 APIs directly; --name node emits canonical node.abi<N>[.<libc>].node names - linux builds pass --tag-libc so glibc and musl coexist per arch - the zero_sqlite3 shell (previously included in prebuild tarballs via --include-regex) is copied into prebuilds/ by deps/copy-shell.js, and shell.js falls back to the prebuilt copy - CI prebuild jobs upload prebuilds/ artifacts instead of release assets; the publish job merges them into the package before npm publish
Removing prebuild dropped its modern node-gyp from node_modules/.bin, leaving the ancient node-gyp 8.4.1 hoisted from the sqlite3 benchmark devDependency. That version emits broken llvm-lib /LTCG flags with VS2022 on Node 24 (CI: Testing Node 24 on windows-2022). Depend on node-gyp ^11 explicitly so builds use a deterministic, current version.
- Pin actions/upload-artifact (v4.6.2) and actions/download-artifact (v4.3.0) to commit SHAs, consistent with the other actions in the workflow. - Fail fast in deps/copy-shell.js when --arch is passed without a value instead of silently writing to prebuilds/<platform>-undefined/.
This file contains hidden or 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
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.
Prompted by WiseLibs/better-sqlite3#655:
prebuild-installdrags in ~40 transitive packages, most of them deprecated (npmlog,gauge,rc,request,tar-fs@2, …), and downloads binaries from GitHub releases at install time. This switches to the recommendedprebuildify+node-gyp-buildpair.What changes
bindings+prebuild-install→ justnode-gyp-build(zero transitive deps). A fresh install (including all devDeps) drops from 461 to 282 packages.prebuilds/<platform>-<arch>/;node-gyp-buildpicks the right one at require time. No network access, no GitHub-release download step. The install script keeps the exact same fallback chain as before (matching prebuild → existingbuild/Release→node-gyp rebuild).--no-napi --name node, producingnode.abi<N>.nodefiles (better-sqlite3 uses V8 APIs directly, so binaries are ABI-specific). Linux builds add--tag-libcso glibc and musl coexist in the sameprebuilds/linux-<arch>/dir;node-gyp-buildmatches the libc tag at runtime.zero_sqlite3shell via--include-regex. prebuildify only ships.nodefiles, sodeps/copy-shell.jscopies the shell intoprebuilds/andshell.jsfalls back to that copy (skipped on musl, where the shell target isn't built — same as today).prebuilds/as artifacts (actions pinned to SHAs); the publish job merges all of them into the package beforenpm publish. The jobs no longer needcontents: write.prebuildalso removed its modernnode-gypfromnode_modules/.bin, leaving the ancient node-gyp 8.4.1 hoisted from thesqlite3benchmark devDep — which emits brokenllvm-lib /LTCGflags with VS2022 on Node 24. Depending onnode-gyp ^11directly makes the build toolchain deterministic.Verified locally (linux-x64, Node 22)
prebuildifyemitsprebuilds/linux-x64/node.abi127.glibc.node; withbuild/removed, the addon, the shell wrapper, and thenode-gyp-buildinstall script all resolve fromprebuilds/alone.overrides: { prebuildify: { node-abi: ^4.25.0 } }gives the same ABI set as the old prebuild flow (115/127/131/137/141/147 for Node 20/22/23/24/25/26).Trade-offs / notes
optionalDependenciespackages (esbuild-style).Statement#toString(),Database#explain()), we could drop--no-napiand ship one ABI-stable binary per platform (~9 binaries, no per-Node-version builds, no GCC bullseye/bookworm split), bringing the tarball back to roughly today's size.undefined symbol: v8::FunctionTemplate::InstanceTemplate) — its V8 shim doesn't cover the symbols our per-ABI binaries reference. This is not a regression: the current prebuild-install binaries are compiled identically and hit the same wall; Bun effectively requires a source build either way (which is what thetest-bunCI job does). Upstream's N-API refactor would fix this properly.releaseevents), so the artifact merge flow is untested until a release dry-run.