chore(pg): remove native pg binding, compile real package from source - #10677
proggeramlug wants to merge 5 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (28)
💤 Files with no reviewable changes (17)
Included review availability: Your plan provides up to 8 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe PR removes Perry’s native PostgreSQL binding, including its extension crate, bundled standard-library implementation, manifest entries, dispatch rows, feature gates, workspace records, and generated documentation. ChangesNative PostgreSQL binding removal
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Other 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
|
Heads-up before this is queued: this PR's recorded workspace baseline is stale and will fail It records Full table and reasoning in #10739. The short version, for whoever rebases this:
Also relevant to the acceptance run whenever it happens: #10735 is live on main — |
|
This PR reports It records Nothing about this is visible in the diff: the counts sit on different JSON lines from the deleted crate entry, so git auto-merges cleanly and GitHub reports the PR mergeable. See #10739 for the general mechanism. A distinction worth drawing, because it caught me outThis PR's file is internally consistent: So the three-way check is necessary but not sufficient. It validates the file against itself, not against the tree the PR will land on. A file can be perfectly self-consistent and still describe a workspace that no longer exists. Both checks are needed:
This PR passes (1) and fails (2). #10691 earlier failed (1) while looking plausible on (2). They are independent failure modes and neither check subsumes the other. What this needsA rebase onto current
|
Removes crates/perry-ext-pg (sqlx::postgres + tokio bridge) and the duplicate pre-#466 in-tree pg implementation in crates/perry-stdlib/src/pg/ (bundled-pg feature), plus every registry entry that pointed at them. import ... from "pg" now falls through to real-source compilation instead of the native binding. wip, base = PR #10674 (fix/10437-cjs-conditional-require) since pg does not run without that fix. # Conflicts: # Cargo.lock # crates/perry-api-manifest/src/entries.rs # workspace-architecture.json
…elines - crates/perry-codegen/src/lower_call/native_table/databases.rs: drop the 7 pg NativeModSig rows (js_pg_* runtime symbols that no longer exist). Caught by perry-codegen's every_dispatch_entry_has_manifest_counterpart test, which fails on drift between this table and API_MANIFEST. - docs/api/perry.d.ts, docs/src/api/reference.md: regenerated via --print-api-manifest (drops the pg module section). - docs/src/native-libraries/governance.md: regenerated via binding_governance.py --table (drops the perry-ext-pg row). - docs/src/native-libraries/overview.md: pg no longer routes to an in-tree native wrapper; updated the well-known-binding description. - workspace-architecture.json: refreshed the recorded baseline (workspace_members 83->82, externalize 33->32) that workspace_architecture.py --check compares against. - scripts/string_payload_access_baseline.txt, scripts/unrooted_local_shape_baseline.json: refreshed ratchet baselines now that perry-ext-pg/perry-stdlib/src/pg no longer contribute findings. # Conflicts: # docs/api/perry.d.ts # docs/src/api/reference.md # docs/src/native-libraries/governance.md # scripts/string_payload_access_baseline.txt # scripts/unrooted_local_shape_baseline.json # workspace-architecture.json
…g-removal rebase Fixes a rebase artifact: literal conflict markers survived the previous resolution commit in docs/api/perry.d.ts and docs/src/api/reference.md. Also resyncs Cargo.lock, workspace-architecture.json's baseline block, native_result_ledger EXPECTED_ROWS/EXPECTED_PROVIDERS, the unrooted-local-shape and string-payload-access baselines, and the generated binding-governance table -- all recomputed from the resolved tree, not carried over from either side of the rebase.
…mands Found during the #10677 rebase reconnaissance: the original PR removed crates/perry-ext-pg but never touched .github/workflows/test.yml, which still names -p perry-ext-pg in the compile-smoke and per-UI-backend build steps (lines ~3029/3881 on current main). Left alone, those two CI jobs would fail to resolve a crate that no longer exists once this PR merges.
77e5e1e to
624b9d6
Compare
…#10677) Squashed rebase of #10677 onto main. Generated/absolute-count files (Cargo.lock, docs/api/perry.d.ts, docs/src/api/reference.md, scripts/native_result_ledger.py, scripts/string_payload_access_baseline.txt, scripts/unrooted_local_shape_baseline.json, workspace-architecture.json) are left at main's values here and regenerated from their owning scripts in a later commit (#10739).
Squashed rebase of #10680 on top of #10677. Generated/absolute-count files are left at main's values here and regenerated from their owning scripts in a later commit (#10739). Non-obvious conflict call: #10680's side of crates/perry-hir/src/lower/expr_call/native_module.rs still contained native_module_member_path(), which main deleted with the node-forge binding. Taking 'theirs' would have resurrected it as dead code, so the whole hunk resolves to empty.
#10677 deleted perry-ext-pg and perry-stdlib/src/pg but left the call sites that reference their symbols: - lower_call/builtin.rs still lowered `new Client(cfg)`/`new Pool(cfg)` from an `import ... from "pg"` to `js_pg_client_new`/`js_pg_pool_new`. With no provider those are undefined at link time, which is exactly the failure the real `pg` package would hit (it constructs `new Client`). Dropped the two arms and the `"Client" | "Pool" => Some(&["pg"])" import gate together, so a user-defined Pool/Client falls through to the generic path (#536). - runtime_decls/stdlib_ffi/data_stores.rs declared the ten js_pg_* externs. - perry-ui-android/src/stdlib_stubs.rs defined seven js_pg_* stubs (#10680 had already dropped the matching mysql2 ones). - perry-codegen-js browser-emit list and a native_table/mod.rs comment. - scripts/run_doc_tests.sh / .ps1 still passed -p perry-ext-pg to cargo build, which no longer resolves.
|
Landed via merge train 245 (#10845), released as v0.5.1624 — merge commit Carried together with its sibling removal, deliberately: #10678's duplicate-symbol hazard is only closed by removing both bindings, because each existed twice (the Two things were redone rather than carried through the rebase onto v0.5.1623:
Validation on the union: Closing here rather than merging — a train lands the commits directly, so the source PR has nothing left to merge. |
Summary
Deletes the native
pgbinding (crates/perry-ext-pg, sqlx::postgres + tokio bridge) and the duplicate pre-#466 in-treepgimplementation living insidecrates/perry-stdlib/src/pg/(thebundled-pgCargo feature), plus every registry entry that pointed at either one.import ... from "pg"no longer resolves as a native module at all — it now compiles the real npmpgpackage from source, same as any other TypeScript/JavaScript dependency.Must not merge before #10674 (
fix/10437-cjs-conditional-require) —pgdoes not run without that fix: a literalrequire('pg-native')inside a conditional guard inpg/lib/native/client.jswas hoisted to an eager static import and threw at program start. This branch is based on #10674's head (b1ba0caf5), notmain.Why two implementations
crates/perry-ext-pg(the registered[bindings.pg]binding,sqlx::postgres+perry-ffi) was the one actually wired up. Butcrates/perry-stdlib/src/pg/turned out to be a second, older, full native reimplementation ofpg(its own module doc: "pg compatible native implementation... drop-in replacement for the pg npm package using sqlx"), kept around behind abundled-pgfeature since before the #466 migration to a separate ext crate. It defined the exact sameextern "C"symbol names (js_pg_client_new,js_pg_client_query, …) asperry-ext-pg, so in any build that linked both, the linker's link-order (perry-extwins) silently discarded the stdlib copy — but it still compiled into every build that also usedmysql2(its#[cfg]wasany(bundled-pg, bundled-mysql2)). Verified zero cross-references frommysql2's module before deleting it, so this is a clean removal, not a partial one.What was deleted
crates/perry-ext-pg/(Cargo.toml + 750-line lib.rs)crates/perry-stdlib/src/pg/(929 lines: connection.rs, pool.rs, result.rs, types.rs, mod.rs) and thebundled-pg/database-postgresCargo features that gated it"postgres"sqlx feature onperry-stdlib's sharedsqlxdependency (verified nothing outside the deletedpg/module referencedsqlx::postgres/PgPool/etc. —mysql2's own sqlx dependency never requested it)crates/perry/well_known_bindings.toml([bindings.pg]+ upstream pin),NATIVE_MODULESincrates/perry-api-manifest/src/entries.rs, the manifest method/class rows inentries/part_1.rsandpart_3.rs, the pg row incrates/perry-codegen/src/lower_call/native_table/databases.rs(7NativeModSigrows — caught byevery_dispatch_entry_has_manifest_counterpart, which fails on drift between this table and the manifest),crates/perry/src/commands/stdlib_features.rs, thebundled-pg/pgentries inoptimized_libs/driver.rsandoptimized_libs/freshness.rs, the workspaceCargo.tomlmember + path-dependency entries, and theperry-ext-pgentry inworkspace-architecture.jsonWhat was deliberately left alone
A grep sweep turned up roughly a dozen more
"pg"-literal matches deep inperry-hir(local_natives.rs,native_new.rs,native_fetch.rs,module_decl.rs,stmt.rs,expr_call/static_and_instance.rs,expr_assign.rs) andperry-codegen(lower_call/builtin.rs'slower_builtin_newdisambiguation,codegen/opts.rs's doc comment) — all("pg", "connect") => Some("Client")-shaped type-narrowing heuristics for the removed native binding. Checked each one: every arm is reached only throughctx.lookup_native_module()/ctx.imported_class_sourceslookups that require the module to have actually been classified native at import time. Sincepgcan no longer classify as native, these arms are unreachable dead code, not live landmines — confirmed both by reading the gating code and empirically, since the original compilability probe (which forced real-sourcepgviacompilePackageswhilepgwas still registered native) already exercised this exact "real pg source, non-native path" combination successfully.perry-codegen-js/src/emit/native.rs's browser-target"pg" => throw(...)arm is similarly dead (only reached for calls already lowered as native). Left as-is per the brief's "keep the diff tight" scope — noting them here for anyone doing a future dead-code sweep.The real post-removal experience (no
compilePackagesentry for pg)Tested with a from-scratch
node_modules(npm installunder Node 26.5.1) and apackage.jsoncontaining only:{ "dependencies": { "pg": "^8" } }No
perry.compilePackageskey at all.perry compile pg_test.tsprintedCompile package wildcard: expanded to 14 installed package(s)— whenperry.compilePackagesis entirely absent, Perry's default behavior sweeps every installednode_modulespackage as a compile-from-source candidate, which coveredpgand all 13 of its transitive deps (pg-cloudflare,pg-connection-string,pg-pool,pg-protocol,pg-types,pgpass,pg-int8,postgres-array,postgres-date,postgres-interval,postgres-bytea,split2,xtend) automatically. It compiled all 43 modules, linked, and produced a 24.7 MB binary.So: for a project where
pgis the only (or main) native-ish dependency, noperry.compilePackagesconfiguration is needed at all — just the ordinary npm"pg": "^8"entry. For a project with a larger, more heterogeneousnode_modules(where blindly wildcard-compiling everything installed isn't desirable — some packages may not be TypeScript-subset-compatible), the recommended path is still an explicitperry.compilePackageslist namingpgand the same 13 transitive packages above, scoping compilation precisely instead of relying on the wildcard.Running the compiled binary against a
Client({ host: "127.0.0.1", port: 5432, ... })/client.connect()(no Postgres listening on this host):RESULT: ERROR Connection refused (os error 111)— a genuine OS-levelnet.connect()failure from the real npmpgsource, reached with zero native-pg code anywhere in the tree. This is the same milestone the original compilability probe hit (which neededcompilePackagesforcing while the native binding still existed); this PR reaches it as the default, unforced behavior. No live Postgres was available to test a real query round-trip — that remains unverified, stated plainly rather than implied.Validation (host: perrymaster, Node 26.5.1 at
/opt/node-v26.5.1-linux-x64; box default is 26.8.1)cargo build --profile perry-dev -p perry -p perry-runtime-static -p perry-stdlib-static— clean, single invocation, confirmed the.a/binary mtimes moved after the final edit (an earlier E2E attempt caught a real coherence bug from buildingperrybefore the last source edit landed — a stale/current source-fingerprint mismatch between the compiler and a freshly auto-optimize-built runtime archive; rebuilding both together in one invocation fixed it, consistent with the Archives from separate cargo invocations can bundle different tokio builds off one Cargo.lock; the link guard catches it but two agents hit it today in unrelated work #10671 archive-coherence warning in the brief).perry-dev:perry-api-manifest: 39/39perry-hir(lib + every integration test file, includingunimplemented_api_check.rs): 459 lib tests + all integration suites, 0 failuresperry-codegen: all 36 integration test targets green (includingmanifest_consistency'severy_dispatch_entry_has_manifest_counterpart, which initially caught the dispatch-table drift this PR now fixes).perry-codegen's own lib unit tests could not be compiled — pre-existing on the fix(cjs): defer conditional CommonJS require() init instead of hoisting (#10437) #10674 base commit itself (b1ba0caf5), unrelated to this change:instanceof_imported_rhs_tests.rs/new_builtin_shadow_tests.rsconstruct anImportedClassliteral missing theconstructor_has_synthetic_argumentsfield, confirmed present atb1ba0caf5before my first commit. Not mine to fix; named explicitly here as something I did not get a pass/fail count for.perry-stdlib: 139/139perry --bin perry(stdlib_features + optimized_libs filtered): 52/52 (matches chore: remove Tier A native bindings (fetch alias, tursodb, iroh) #10618's precedent number). Full unfiltered suite: 1129/1130 on the first run, with onecommands::compile::geisterhand::…warm_archives_are_rebuilt_as_one_runtime_graphfailure that re-ran green in isolation — a transient resource-contention artifact from a heavily shared host (multiple concurrent agents running their owncargo build --release, disk at 96-99% used throughout), not a pg regression; that test doesn't touch pg/mysql2/well_known_bindings at all.perry-runtime(RUST_TEST_THREADS=1, untouched by this diff): 4039 passed, 2 failed, 4 ignored. Both failures are the two known pre-existingdebug_assert!-gated cases named in the campaign brief (gc::tests::copy_slot_decode::…,gc::tests::heap_generation::…), which fail underperry-dev/releaseby construction (both profiles compile outdebug_assert!). Confirmed unrelated to this change — perry-runtime is untouched by this diff.SKIP_COMPILE_GATES=1 ./scripts/run_lint_gates.sh): 76 of 77 passed (compile tier skipped per host convention, 2 CI-only skipped). The one red, "Public benchmark evidence freshness", is the pre-existing, repo-wide red named in the brief — not chased. Along the way this also caught and fixed two gates this removal touches that PR chore: remove Tier A native bindings (fetch alias, tursodb, iroh) #10618's tier-A removal never exercised:workspace_architecture.py --check(the recordedbaselineobject inworkspace-architecture.jsonneededworkspace_members/decision_countsrefreshed — no script flag does this automatically, recomputed via the module's own helpers) andstring_payload_access_inventory.py(a per-file ratchet baseline with a staleperry-ext-pgentry).binding_governance.py --checkOK,binding_pins.mjs --checkOK,unrooted_local_shape.py --checkOK (baseline refreshed),check_file_size.shOK.package.jsona user now needs{ "dependencies": { "pg": "^8" } }No
perry.compilePackagesentry required whenpgis the project's only real dependency (Perry's no-config wildcard covers it and its transitive deps). For a project with other, unrelated npm packages installed, listpgplus its transitive deps explicitly inperry.compilePackagesinstead of relying on the wildcard sweeping everything:pg,pg-cloudflare,pg-connection-string,pg-pool,pg-protocol,pg-types,pgpass,pg-int8,postgres-array,postgres-date,postgres-interval,postgres-bytea,split2,xtend.Scope
Removal only — no other binding touched. A sibling agent is concurrently removing the
axiosbinding on its own branch against the same shared registry files (well_known_bindings.toml,entries.rs,stdlib_features.rs, workspaceCargo.toml,workspace-architecture.json); expect a merge conflict there, resolved by the merge train, not by either PR individually.Rebase note (2026-09-20)
Rebased onto
main@b9ba951ff861c61afb845bfbdfa574cb0fa4080e(train 239) as part of a4-PR sequential rebase campaign together with #10795, #10680, #10704 — all four
independently rebased onto this same main SHA and pushed together. This PR's base is now
main, notfix/10437-cjs-conditional-require: that branch squash-merged into mainsome time ago (its content lives on main as
e52aae9947/077140b8eb), so this needed thetwo-step unstack —
git rebase --onto origin/main <fix/10437 tip> pr-10677followed bygh pr edit --base main— not a plain rebase, which would have reported CONFLICTINGagainst a stale, no-longer-reachable base.
Conflicts:
Cargo.lock(resynced viacargo metadata --offline),crates/perry-api-manifest/ src/entries.rsandworkspace-architecture.json(both the ordinary "two unrelateddeletions landed at the same list position" shape — main had already dropped
uuid/qsentries adjacent to
pg's; resolved by removing onlypg's own lines, keeping everythingelse exactly as it stands on current main), then a second round on
docs/api/perry.d.ts,docs/src/api/reference.md,docs/src/native-libraries/governance.md,scripts/string_payload_access_baseline.txt,scripts/unrooted_local_shape_baseline.jsonand
workspace-architecture.jsonagain — all fully regenerated from the resolved treevia their own tools rather than hand-merged.
Found and fixed a gap in the original PR while rebasing:
.github/workflows/test.ymlstill named
-p perry-ext-pgin twocargo build --releasesteps (the compile-smoke job andthe per-UI-backend build job, ~lines 3029/3881 on current main) — the original PR never
touched
test.yml. Left alone, both jobs would fail to resolve a crate this PR deletes.Removed
-p perry-ext-pgfrom both lines; left-p perry-ext-mysql2on those same linesuntouched (that's #10680's job, landing independently).
Recomputed triple:
workspace_members=69(decision_counts: externalize=20, keep=44,merge=1, remove=1, review=3) — the committed baseline was stale (recorded from an earlier
main) and needed a full recompute, plus a stray
"perry-ext-pg"entry survived theauto-merge in the per-crate map itself (baseline alone would have looked consistent while
still naming a deleted crate —
--checkcatches this, which is why it's not enough to trusta clean auto-merge);
native_result_ledger: 349 rows / 300 providers (main's real ledgerminus pg's 7 rows / 7 providers);
unrooted-local-shapetotal: 520 (main's real total minuspg's 7-line
perry-ext-pg/src/lib.rsentry; one unrelated file,tls.rs, also drifted +1between this branch's fork point and current main — real tree drift, not something this PR
touched).
These numbers assume main is still at the stated SHA — #10795/#10680/#10704 remove
different crates from the same starting point; whichever of the four lands first moves the
ground under the other three's counts.
Gates:
cargo fmt --all -- --checkOK (after onecargo fmt --allpass to fix acomment-alignment mismatch left by the merge resolution);
cargo check --workspace --all-targetsunder-D warningson the defaultdevprofile (excl.perry-ui-gtk4) —clean;
run_lint_gates.sh SKIP_COMPILE_GATES=1— 78 of 79 passed (1 pre-existing, #10707,not chased);
binding_governance.py --checkOK;binding_pins.mjs --checkunder Node26.5.1 OK;
check_file_size.shOK. Compile tier not run. No gap sweep run. No acceptancere-run — pg's behavior is unchanged by this rebase.
Summary by CodeRabbit
Breaking Changes
pg) binding.import ... from "pg"now uses the npmpgpackage compiled from source instead of native support.Client,Pool,connect,query, and related methods, are no longer provided.Documentation