feat(compile): auto-provision native ext bindings + wasm host - #7118
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThe compiler now routes supported extension symbols by prefix, builds missing CPU-only and Wasm host archives on demand, validates platform toolchains, and reuses resolved archives through linking. Node-forge and TLS API manifests, workspace metadata, tests, and release documentation are updated. ChangesAutomatic binding and Wasm provisioning
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FFI as record_ffi_call
participant Registry as EXT_PREFIX_REGISTRY
participant Cache as Module-capture replay
FFI->>Registry: Match supported extension symbol prefix
Registry-->>FFI: Return binding provider and prefix
FFI->>Cache: Persist prefix marker
Cache-->>FFI: Replay provider and prefix
sequenceDiagram
participant Pipeline as compile pipeline
participant Search as build_wasm_host_library
participant Cargo
participant Linker as static-library linking
Pipeline->>Search: Resolve perry-wasm-host archive
Search->>Cargo: Build release archive for target
Cargo-->>Search: Produce archive
Search-->>Pipeline: Return resolved archive
Pipeline->>Linker: Scan and link archive
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry/src/commands/compile/optimized_libs/driver.rs`:
- Around line 207-214: Update the missing-library auto-build call in the
optimized compilation flow to apply the target-specific Android or HarmonyOS
cross-compilation environment before invoking build_missing_prebuilt_ext_lib.
Reuse the same cross-env setup or target-specific invocation path used by
build_optimized_libs, while preserving existing behavior for other targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 898d2929-50d0-47ce-9118-6197d7efe94a
📒 Files selected for processing (7)
changelog.d/0000-auto-provision-ext-bindings.mdcrates/perry-codegen/src/ext_registry.rscrates/perry/src/commands/compile.rscrates/perry/src/commands/compile/library_search.rscrates/perry/src/commands/compile/optimized_libs/driver.rscrates/perry/src/commands/compile/optimized_libs/tests.rscrates/perry/src/commands/compile/run_pipeline.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry/src/commands/compile/optimized_libs/no_auto.rs`:
- Around line 179-212: Update the HarmonyOS branch in the auto-build flow to
require both a usable clang compiler and the corresponding sysroot before
applying harmonyos_cross_env. Validate the candidate returned by
find_harmonyos_sdk, and when either requirement is missing, return None so the
existing fallback message/path is used instead of configuring an incomplete
cross environment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b3ba8319-22c9-4cf2-b3ab-916013f9b16e
📒 Files selected for processing (5)
crates/perry-runtime/src/os_network.rscrates/perry/src/commands/compile/optimized_libs/no_auto.rscrates/perry/src/commands/compile/resolve.rscrates/perry/src/commands/compile/well_known.rsworkspace-architecture.json
05d3a38 to
410ffd3
Compare
Drive native-artifact provisioning off what a program actually references instead of the well-known iteration set alone. Link: codegen's ext_registry gains a prefix net so any emitted js_<binding>_* FFI (js_ioredis_*, js_undici_*, js_node_forge_*) flips its well-known wrapper onto the link line off provenance alone. An AOT-compiled iovalkey/undici/node-forge (a compilePackages member, never in any import set) now links its perry-ext-* staticlib instead of failing with Undefined symbols: _js_ioredis_new. Build: routed CPU-only ext staticlibs are auto-built from workspace source when missing (reusing build_missing_prebuilt_ext_lib), and perry-wasm-host is auto-built when the program uses WebAssembly.* so libperry_wasm_host.a-not-found can't happen on a normal compile. Tests: ext_registry prefix routing + replay markers, ext binding key resolution + build-routing split, and a fake-cargo wasm-host auto-build. Review + CI follow-ups folded in: - Android/HarmonyOS cross-env: build_missing_prebuilt_ext_lib now applies the OHOS SDK / Android NDK compile env (mirroring build_optimized_libs) before the auto-build, so a C-dependent CPU-only wrapper's cross build no longer fails in build.rs before it can fall back (CodeRabbit). - workspace-architecture baseline refreshed to record the audit facts the ext crates imply: perry-ext-node-forge classification, the intentional perry-ext-node-forge/perry-ext-undici -> perry-runtime edges (PerryTS#6303/PerryTS#6314), perry-container-compose default membership, and member/closure counts. - Cleared pre-existing -D warnings dead-code that had been red on main since PerryTS#7080/PerryTS#7028/PerryTS#7031: gate perry-runtime's format_mac_address to its real callers (Windows/macOS/BSD/test; Linux reads the MAC preformatted from /sys), and allow(dead_code) on resolve_exports and the well_known provenance fields (read by the review gate + tests, not the link path). - Regenerated docs/src/api/reference.md + docs/api/perry.d.ts for the node-forge manifest entries added in PerryTS#7033. - Dropped the now-passing test_gap_zlib_3285_params from the gap snapshot.
…yTS#7118) Route emitted extension FFI families to their well-known providers, build missing CPU-only bindings and the wasm host, and carry custom target-dir artifacts through scanning and linking. Validate HarmonyOS SDK inputs before applying cross-build environments.
410ffd3 to
c9186ba
Compare
Conflicts were confined to the two generated API-docs artifacts (docs/api/perry.d.ts, docs/src/api/reference.md), and only in their "Coverage:"/"Total:" header counts: this branch adds the iovalkey + dgram/sqlite manifest rows, while main's PerryTS#7118 (auto-provision native ext bindings) added its own. Both bodies merged cleanly; the counts are regenerated from the merged manifest in the follow-up commit.
changelog.d fragments are named <PR>-<slug>.md (changelog.d/README.md), so in-flight PRs never collide. This one was keyed to the issue it fixes (0513) rather than the PR. Also confirms the merge resolution: regenerating both API-docs artifacts from the merged manifest reproduces the committed files byte-for-byte, and their only delta against main is the "Coverage:"/"Total:" header count — main's PerryTS#7118 regenerated the artifacts without adding manifest rows, so nothing from main is dropped.
…ble (#7159) * fix(api-manifest): reconcile API_MANIFEST with the native dispatch table The manifest_consistency drift guards in perry-codegen were red on main: - every_native_module_has_at_least_one_manifest_entry / #513: iovalkey (a NATIVE_MODULES member and well_known_bindings entry that normalizes to ioredis in dispatch) had zero API_MANIFEST rows, so strict mode never engaged for it. Add the Redis class + createClient rows, mirroring the redis alias. - every_dispatch_entry_has_manifest_counterpart: dgram Socket.sendto and sqlite DatabaseSync.serialize/deserialize existed in NATIVE_MODULE_TABLE with no manifest counterpart. Add the matching receiver-method rows. - every_well_known_binding_has_manifest_entry: the test's inline TOML parser misread the new [bindings.*.upstream] provenance sub-tables as binding names (dotenv.upstream, iovalkey.upstream, ...). Teach it to skip nested sub-tables so it only checks real routed module names. Regenerate docs/api/perry.d.ts and docs/src/api/reference.md from the manifest (api-docs-drift gate); this also folds in the previously uncommitted node-forge and tls.convertALPNProtocols entries. * chore(changelog): key the fragment to PR 7159 changelog.d fragments are named <PR>-<slug>.md (changelog.d/README.md), so in-flight PRs never collide. This one was keyed to the issue it fixes (0513) rather than the PR. Also confirms the merge resolution: regenerating both API-docs artifacts from the merged manifest reproduces the committed files byte-for-byte, and their only delta against main is the "Coverage:"/"Total:" header count — main's #7118 regenerated the artifacts without adding manifest rows, so nothing from main is dropped. * fix(api-manifest): cover the dispatch rows main added after this branch `every_dispatch_entry_has_manifest_counterpart` listed 14 missing rows at origin/main; this branch as authored fixed 3 of them (dgram::sendto, sqlite::serialize/deserialize). The other 11 landed on main *after* the branch point, so merging main re-reds the very gate the branch exists to turn green: inspector::log/info/debug/warn/error, inspector/promises::SessionCall — #7090 (47040d5) inspector Node 26.5 parity cluster::listeners/rawListeners/setMaxListeners/getMaxListeners — #7105 (76fa7b4) cluster Node 26 semantics lru-cache::peek — #7136 (5f5006a) lru-cache faithful JS-value keys/values Rows mirror how each surface is already modelled: - the five `inspector.console.*` forwarders carry the dispatch table's `class_filter: Some("console")`, so they hang off the existing `property("inspector", "console")` instead of becoming named exports; - `inspector/promises::SessionCall` is the synthetic row for `Session(...)` called without `new` (js_node_inspector_session_call_without_new), so it is `internal_method` — the .d.ts keeps advertising `Session`; - the four `cluster` EventEmitter methods join their eleven siblings as `internal_method` + paired `internal_property`, matching the #3687 block directly above them. All rows use the plain `method`/`internal_method` helpers (params: &[], returns: Any), which `manifest_param_counts_match_dispatch_table` deliberately skips as the "no signature data" fallback — so no arity claim is invented for them. manifest_consistency: 5 passed / 0 failed (was 2 passed / 3 failed at origin/main, 4 passed / 1 failed with only the original branch fix). docs/src/api/reference.md regenerated: +6 rendered entries (the internal_* rows stay out of the public surface, as cluster's existing ones already do); docs/api/perry.d.ts is byte-unchanged, since nothing added is a named export. * docs(changelog): note the post-branch dispatch rows in the 7159 fragment --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
Running
perry compileon a real application could fail at the very last step, the link, with an error naming a symbol the user never wrote:There was nothing wrong with the program. Perry had generated a call into one of its own native bindings and then not linked the library that defines it. A second, separate failure hit programs that use
WebAssembly.*, which died withlibperry_wasm_host.a not found. Both had the same workaround: figure out which internal crate was missing and runcargo build --release -p <crate>by hand before compiling.After this change,
perry compileworks out which native libraries the compiled program actually references, builds any that are missing, and links them. No manual prebuild step and no environment variables. Both symptoms were measured against a downstream application on perry v0.5.1265.Some vocabulary used below
Ahead-of-time compilation, abbreviated AOT, means Perry compiles the package's own JavaScript into the binary rather than substituting a native replacement.
A foreign function interface call, abbreviated FFI, is a call from generated code into a native function such as
js_ioredis_new. Those functions live in static libraries that must be linked into the final binary or the link fails.A well-known binding is a native reimplementation Perry ships for a popular npm package. When Perry recognizes such a package, it "flips" to the binding instead of compiling the package's JavaScript.
Symptom 1: the link gap
The application listed
iovalkeyinperry.compilePackages, soiovalkeywas compiled ahead of time as ordinary JavaScript. But code generation still lowered its client usage to the nativejs_ioredis_newforeign function call.That symbol appeared in neither the exact
ext_registrytable nor in any import set, so the well-known flip never fired andperry-ext-iorediswas never linked. The result is the undefined-symbol error above.This is not specific to
iovalkey. It bites any well-known-backed package that also appears incompilePackages, and any package whose extension foreign function calls are emitted by code generation without the module being in the well-known iteration set.Symptom 2: the missing prebuild
The auto-optimize path never built the CPU-only extension static libraries,
perry-ext-node-forgeamong them, and never builtperry-wasm-hosteither.That meant a human had to run
cargo build --release -p …for the right crate before compiling, and a program usingWebAssembly.*failed withlibperry_wasm_host.a not founduntil someone knew to do it.Root cause
Native-artifact provisioning was driven off the well-known iteration set, meaning
ctx.native_module_importsplusPERRY_FORCE_WELL_KNOWN, rather than off what the program actually references.Those two sets come apart exactly where the bug lives. Members of
compilePackagesare compiled ahead of time, so they never enternative_module_importsat all, yet code generation still emits the extension binding'sjs_<binding>_*foreign function calls for them. The provisioning logic therefore never learned that the binding was needed.The exact-symbol
ext_registrytable only routed symbols someone had remembered to add to it, coveringhttp,net,ws,events,mysql2,streams, andcrypto. Wholejs_<binding>_*families, includingioredis,undici, andnode-forge, fell through it.Separately, the auto-optimize cargo invocation only rebuilt the shared-tokio extension crates. The CPU-only extension crates and
perry-wasm-hostwere built by nothing at all.The fix
Linking and building are now driven off the emitted foreign-function set, which is the ground truth for what the binary needs, in addition to the existing well-known routing.
For linking, in
crates/perry-codegen/src/ext_registry.rs, a prefix net calledEXT_PREFIX_REGISTRYroutes any emittedjs_<binding>_*symbol to its well-known binding key:js_ioredis_*toioredis,js_undici_*toundici,js_node_forge_*tonode-forge. It is checked after the exact table and the crypto net. The driver's existingWellKnown(key)tonative_module_importsrouting machinery does the rest, so there is no per-symbol maintenance and no import required. The per-module capture records the matched prefix as a stable object-cache replay marker, in the same shape as the existingjs_crypto_marker.For building CPU-only extensions, in
optimized_libs/driver.rs, a routed CPU-only binding whose static library is missing is now auto-built from workspace source through the existingbuild_missing_prebuilt_ext_libleaf build. The shared-tokio crates were already being rebuilt in the auto-optimize invocation. This is a no-op when the archive is already present.For the WebAssembly host, in
library_search.rsandrun_pipeline.rs, a newbuild_wasm_host_libraryis wired to the existingctx.needs_wasm_runtimeand--enable-wasm-runtimesignal. When a program that usesWebAssembly.*is missinglibperry_wasm_host.a,perry-wasm-hostis built from workspace source before the symbol-stub scan and the link, so both of those steps can resolve it. This is a no-op when the archive exists or when the program does not use WebAssembly.Acceptance: a bare compile with no manual steps
Run as a plain
perry compile, with noPERRY_FORCE_WELL_KNOWNand no manual prebuild. All three of the test application's command-line entry points link and run,_js_ioredis_newis gone, andioredis,http, andnetare auto-built:--versionworks)--versionworks)The WebAssembly host auto-provisioning was verified end to end on a minimal
new WebAssembly.Module(...)program. Withlibperry_wasm_host.aabsent, the compile auto-builds it, reportsUsing wasmi WebAssembly host runtime, links, and runs. Thenot founderror is gone.Test runs
In
perry-codegen: prefix routing for theioredis,undici, andnode-forgefamilies, a check that the prefix net does not over-match, and prefix-marker cache replay.In
perry: extension binding keys resolving to theirperry-ext-*crates, the split between tokio and CPU-only build routing, and aperry-wasm-hostauto-build driven through a fake cargo.cargo test -p perry --bin perryis green with 841 passing,cargo build --release -p perrysucceeds,cargo fmtis clean, and a changelog fragment is included.Summary by CodeRabbit
New Features
node-forgeAPIs andtls.convertALPNProtocolsto the public API reference.Bug Fixes
Documentation