Revert "treat require(<builtin>) as a stdlib requirement" (#8548) — runtime-only link regression

[proggeramlug]

Revert "treat require(<builtin>) as a stdlib requirement" (#8548) — it caused a runtime-only link failure.

#8548 fixed a real bug (#8547: require('http') linked runtime-only, so every stdlib-backed builtin reached that way returned undefined, and issue_4903_listen_callback_deferred went 0/2 → 2/2). It also broke issue_5247_property_read_source_location, which had been passing:

compile ["--debug-symbols"] must succeed; stderr:
/usr/bin/ld: libperry_runtime.a(...): in function `js_webassembly_validate':
  undefined reference to `perry_wasm_host_validate'
  undefined reference to `perry_wasm_host_module_new'
  … collect2: error: ld returned 1 exit status

Attribution is solid, which is why this is a revert and not a follow-up patch. The suite passed on 1c2326554 (the commit immediately before) and fails on b00e261f8 (with #8548) in both sweep runs of that SHA — deterministic, not a flake. It does not reproduce locally, which is the tell for a feature-unification effect rather than a logic error: under cargo test --workspace cargo unifies features across the whole graph, so pulling perry-stdlib onto more compile paths changes what target/debug/libperry_runtime.a contains. The archive then carries webassembly.rs's code, whose extern declarations are satisfied only by the separate perry-wasm-host crate — which a runtime-only link line does not include.

So the honest position is that #8548's diagnosis stands and its link-mode consequence was not thought through: making a program need the stdlib is not free, and the runtime-only link path is not prepared for everything the stdlib's feature set drags into the runtime archive. Fixing #8547 properly needs that interaction handled — either by ensuring perry-wasm-host is on the link line whenever the runtime archive contains wasm code, or by keeping the wasm surface out of it.

Net effect of this revert: issue_4903 returns to failing (2 tests, the pre-existing #8547 symptom) and issue_5247 returns to passing (3 tests). Main is red either way; this restores the known red rather than a new link-level one, which is the safer state to leave a release branch in — a link regression can bite real builds, not just tests.

Reopens #8547.

Summary by CodeRabbit

• Bug Fixes

  • Reverted recent CommonJS standard-library linking changes to prevent runtime linker failures.
  • Restored compatibility for affected projects while preserving previously resolved behavior in other scenarios.

• Documentation

  • Updated release documentation to reflect the reverted behavior and its known impact.

• Chores

  • Incremented the application version to 0.5.1516.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b18b6a8-0167-49ca-bcad-a483fd067582

📥 Commits

Reviewing files that changed from the base of the PR and between b00e261 and 5e6aca4.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• CLAUDE.md
• Cargo.toml
• changelog.d/8548-require-stdlib-link.md
• changelog.d/8549-revert-require-stdlib-link.md
• crates/perry/src/commands/compile/cjs_wrap/mod.rs
• crates/perry/src/commands/compile/collect_modules.rs

---

📝 Walkthrough

Walkthrough

The compiler no longer links Node stdlib modules detected from CommonJS require() calls. The helper import is private. Release metadata now uses version 0.5.1516, with a changelog entry documenting the revert.

Changes

CommonJS stdlib linking revert

Layer / File(s)	Summary
Remove CommonJS stdlib linking crates/perry/src/commands/compile/cjs_wrap/mod.rs, crates/perry/src/commands/compile/collect_modules.rs	The compiler no longer detects stdlib specifiers for linking. extract_require_specifiers is imported privately.
Record release changes Cargo.toml, CLAUDE.md, changelog.d/8549-revert-require-stdlib-link.md	The workspace version and documented current version are 0.5.1516. The changelog records the revert.⁣

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: jdalton

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch revert-8548

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Correction — I attributed this to Cargo feature unification, and that was wrong.

I said the wasm link failure came from cargo test --workspace unifying perry-runtime/wasm-host on. That explanation does not survive checking: no workspace crate enables perry-runtime/wasm-host. The only occurrence in any Cargo.toml is the feature's own declaration (wasm-host = []). Unification cannot turn on a feature nothing requests.

The actual mechanism is shared-archive contamination between tests in the same job:

• optimized_libs/no_auto.rs builds perry-runtime-static with default features plus perry-runtime/wasm-host when ctx.needs_wasm_runtime is set — i.e. when the compiled program references WebAssembly.*. That is the PERRY_NO_AUTO_OPTIMIZE=1 path.
• issue_5247_property_read_source_location also runs with PERRY_NO_AUTO_OPTIMIZE=1 and points PERRY_RUNTIME_DIR at the same shared target/debug, rebuilding only -p perry-runtime (no wasm) via its own ensure_runtime_archive.

So under cargo test --workspace, any test that compiles a WebAssembly-using program rewrites the shared libperry_runtime.a with the wasm surface. A later runtime-only link from 5247 then hits undefined reference to perry_wasm_host_*, because the host archive is only put on the link line when that program sets needs_wasm_runtime. Whichever build wrote the archive last decides the outcome.

That reframes the situation in two ways worth being explicit about:

1. It is a pre-existing fragility, not something fix(compile): treat require(<builtin>) as a stdlib requirement (#8547) #8548 created. fix(compile): treat require(<builtin>) as a stdlib requirement (#8547) #8548 perturbed which tests rebuild the shared archive and in what order, which is why it flipped a latent race into a deterministic failure on that SHA. The revert is still correct — I would not leave a link-level red on a release branch — but "fix(compile): treat require(<builtin>) as a stdlib requirement (#8547) #8548 caused it" overstates my change's role, and I want that on the record before someone re-lands the detection and is surprised again.
2. The durable fix is archive isolation, not the link path. Two cargo invocations with different feature sets write the same target/debug/libperry_runtime.a; nothing arbitrates. Giving the runtime-archive-consuming tests their own PERRY_RUNTIME_DIR (or keying the archive path by feature set) removes the whole class. That is a smaller and safer change than either of the link-line options I listed earlier, and it would let require('http') links runtime-only, so stdlib builtins return undefined (ESM import works) #8547's detection re-land as-is.

I am not making that change myself: it touches test infrastructure I did not write, and I have already demonstrated on this issue that acting with an incomplete model of this link path produces regressions. Flagging it for an owner with the evidence above.