-
-
Notifications
You must be signed in to change notification settings - Fork 161
fix(http): rebuild client-pump stdlib for node:http under PERRY_NO_AUTO_OPTIMIZE #10667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
+234
−17
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
33ffbda
fix(http): rebuild client-pump stdlib for node:http under PERRY_NO_AU…
perry-bot e7d8d6a
fix(http): rebuild perry-ext-http alongside the http-client-pump stdl…
perry-bot 18e6f28
changelog: #10667
perry-bot 796f336
fix(test): update no-auto well-known test for the http-client-pump re…
perry-bot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| Fixed `node:http`/`node:https` client dynamic dispatch (`res.pipe()`, `req.setHeader()`, `req.setTimeout()`, and | ||
| the rest of the client `IncomingMessage`/`ClientRequest` fallback surface) being silently absent under | ||
| `PERRY_NO_AUTO_OPTIMIZE=1`: the prebuilt stdlib archive is built with the default `full` feature set, which | ||
| deliberately excludes `external-http-client-pump` (folding it into `full` would force every no-auto program to | ||
| carry `libperry_ext_http.a`). When the program imports `http`/`https`, the no-auto path now rebuilds | ||
| `perry-stdlib-static` with that feature on top of `full`, in the same cargo invocation as `perry-ext-http` itself | ||
| (two archives built in separate invocations can carry different tokio compilations even off an identical | ||
| `Cargo.lock`, which the existing link-time guard in `runtime_compat.rs` refuses to link). Mirrors the on-demand | ||
| `wasm-host` rebuild `build_optional_runtime` already does for `WebAssembly.*` support (#10466). |
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 24770
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 39033
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 50370
Build one compatible archive set when both rebuild conditions apply.
When a Windows target needs
wasm-hostor a native addon and importsnode:httpornode:https,build_optional_runtimereturns a Windowsperry_stdlib.lib. Its Cargo invocation builds onlyperry-runtime-staticandperry-stdlib-staticwith the runtime features. The defaultfullstdlib excludesexternal-http-client-pump, so thisor_elseskips the HTTP rebuild.resolve_prebuilt_ext_libscan still provideperry-ext-http, but it is not built with this stdlib in the same Cargo invocation.The linked stdlib therefore omits the client dispatch fallbacks compiled behind
external-http-client-pump, and the selected stdlib/ext-http pair does not guarantee the shared Tokio build required by the HTTP integration.Extend the Windows optional-runtime build to include
perry-ext-httpandperry-stdlib/external-http-client-pumpwhen HTTP client imports exist. Alternatively, use one unified rebuild helper for this combination.🤖 Prompt for AI Agents