worker: parse NODE_OPTIONS when env option is not provided#62303
Closed
semimikoh wants to merge 3753 commits intonodejs:mainfrom
Closed
worker: parse NODE_OPTIONS when env option is not provided#62303semimikoh wants to merge 3753 commits intonodejs:mainfrom
semimikoh wants to merge 3753 commits intonodejs:mainfrom
Conversation
PR-URL: nodejs#60520 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
PR-URL: nodejs#60422 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#60348 Reviewed-By: Daeyeon Jeong <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Jason Zhang <[email protected]>
Bumps [actions/stale](https://github.com/actions/stale) from 10.0.0 to 10.1.0. - [Release notes](https://github.com/actions/stale/releases) - [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md) - [Commits](actions/stale@3a9db7e...5f858e3) --- updated-dependencies: - dependency-name: actions/stale dependency-version: 10.1.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> PR-URL: nodejs#60528 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: MichaΓ«l Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 5.0.0 to 6.0.0. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](actions/setup-node@a0853c2...2028fbc) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: 6.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> PR-URL: nodejs#60529 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Ulises GascΓ³n <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.30.5 to 4.31.2. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@3599b3b...0499de3) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.31.2 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> PR-URL: nodejs#60533 Reviewed-By: MichaΓ«l Zasso <[email protected]> Reviewed-By: Ulises GascΓ³n <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
The previous .devcontainer.json configuration was outdated and contained personal configurations that were not needed to run a dev container. This updates the structure so that it's put in .devcontainer/base/devcontainer.json based on the recommended setup in GitHub's documentation. The official image now publishes both arm64 and amd64 images, and devcontainer tools should be able to pick up the right one without extra arguments. This also adds documentation on how to use the container. Refs: https://docs.github.com/en/codespaces/setting-up-your-project-for-codespaces/adding-a-dev-container-configuration/introduction-to-dev-containers#devcontainerjson PR-URL: nodejs#60472 Refs: https://github.com/nodejs/devcontainer Refs: https://hub.docker.com/r/nodejs/devcontainer Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#60541 Reviewed-By: MichaΓ«l Zasso <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#60542 Reviewed-By: MichaΓ«l Zasso <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#60385 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#59354 Reviewed-By: Tim Perry <[email protected]> Reviewed-By: theanarkh <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]>
PR-URL: nodejs#60565 Refs: nodejs#60494 Refs: https://github.com/nodejs/reliability/blob/main/reports/2025-11-03.md Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: James M Snell <[email protected]>
This provides a bit more information about where & when the child processes crashes when it crashes / flakes in the CI. PR-URL: nodejs#60466 Refs: nodejs#54346 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
Previously when the child process helpers are used to print
information about the failed expectations and the env of the
child process was overridden, it printed the entire env object,
which may be too much if the caller does something like
{ env: { ENV: 'var', ...process.env } } (which tend to be always
the case for specifying env because we need to copy the
process.env for dynamic library loading in the CI).
This updates it to only show the env vars that differ from
process.env for a cleaner log in the case of failure.
PR-URL: nodejs#60556
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
This allows us to set up fixtures for the benchmark suite only once, which can save quite a bit of time when running benchmarks that require tens of thousands of fixture files or more (e.g. the module benchmarks). PR-URL: nodejs#60574 Fixes: nodejs#58488 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#53864 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
`node:http`' `request.reusedSocket` documentation sample code was referencing an undeclared `agent` identifier. PR-URL: nodejs#55478 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
This problem was introduced in PR nodejs#38837. PR-URL: nodejs#57677 Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#58205 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#59277 Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#59698 Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#59815 Reviewed-By: Chengzhong Wu <[email protected]>
`events.once` is a great way to manage the `close` / `error` / `exit` events of a child process. It creates a Promise that is fulfilled when the EventEmitter emits the given event or that is rejected if the EventEmitter emits 'error' while waiting. I think a lot of people wrongly think that managing a spawned child process requires writing boilerplate handlers for `close` and `error`, and that there's no `promisify` solution for spawned child processes. In fact, `events.once` is that solution. The docs should explicitly recommend it in examples. PR-URL: nodejs#60017 Reviewed-By: Chemi Atlow <[email protected]>
PR-URL: nodejs#60305 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
- Update primitive types to lower case, so the anchor of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Data_structures will work correctly. - Add `bigint` to primitive types to match MDN Web Doc. PR-URL: nodejs#60581 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Claudio Wunder <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Instead of measuring the performance of the entire module initialization, focus only on the import.meta initialization. PR-URL: nodejs#60603 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
The sea tests are only run sequentially to avoid taking too much disk space. When there is enough disk space, however, it's better to run them in parallel, as these tests tend to be slow. PR-URL: nodejs#60604 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#60636 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs#44878 Fixes: nodejs#38685 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#60650 Reviewed-By: MichaΓ«l Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: nodejs#58324 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Stefan Stojanovic <[email protected]>
They were missing from the skip pattern due to different naming. PR-URL: nodejs#61504 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#61528 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
To reduce cache thrashing. PR-URL: nodejs#61790 Refs: nodejs#61436 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: MichaΓ«l Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Signed-off-by: Tierney Cyren <[email protected]> PR-URL: nodejs#61663 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#61742 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: nodejs#61734 Reviewed-By: RenΓ© <[email protected]> Reviewed-By: MichaΓ«l Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#61759 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#61899 Reviewed-By: MichaΓ«l Zasso <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#61903 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#61921 Reviewed-By: Stewart X Addison <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#61451 Refs: nodejs#60869 Refs: nodejs#61449 Reviewed-By: Stewart X Addison <[email protected]> Reviewed-By: Stefan Stojanovic <[email protected]>
PR-URL: nodejs#61789 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Original commit message:
[riscv] Fix sp handling in MacroAssembler::LeaveFrame
Keep sp <= fp to ensure that data right above fp doesn't get clobbered
by an inopportune signal and its handler.
Such clobbering can happen in e.g. Node.js when JIT-compiled code is
interrupted by a SIGCHLD handler.
Bug: None
Change-Id: Ief0836032ada7942e89f081f7605f61632c4d414
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7540554
Reviewed-by: Ji Qiu <[email protected]>
Commit-Queue: Yahan Lu (LuYahan) <[email protected]>
Reviewed-by: Rezvan Mahdavi Hezaveh <[email protected]>
Cr-Commit-Position: refs/heads/main@{#105069}
Refs: v8/v8@6a0a25a
Co-authored-by: kxxt <[email protected]>
PR-URL: nodejs#61688
Reviewed-By: Antoine du Hamel <[email protected]>
`parallel/test-strace-openat-openssl` was added to check explicitly
for a list of known files that would be opened for a set workload
(`require("crypto")`). This is not reliable when Node.js is linked
to an external/shared OpenSSL library (e.g. it might be configured
to load configuration files from a different default location and/or
load more than one configuration file) so skip this test when Node.js
is built in that way.
PR-URL: nodejs#61987
Fixes: nodejs#61966
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#61732 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: MichaΓ«l Zasso <[email protected]>
PR-URL: nodejs#61830 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Ulises GascΓ³n <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: nodejs#62016 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: GΓΌrgΓΌn DayΔ±oΔlu <[email protected]>
build: * test on Python 3.14 (Christian Clauss) nodejs#59983 cli: * mark `--heapsnapshot-near-heap-limit` as stable (Joyee Cheung) nodejs#60956 crypto: * update root certificates to NSS 3.119 (Node.js GitHub Bot) nodejs#61419 * update root certificates to NSS 3.117 (Node.js GitHub Bot) nodejs#60741 doc: * add avivkeller to collaborators (Aviv Keller) nodejs#61115 * add gurgunday to collaborators (GΓΌrgΓΌn DayΔ±oΔlu) nodejs#61094 meta: * add Renegade334 to collaborators (Renegade334) nodejs#60714 PR-URL: nodejs#61947
PR-URL: nodejs#61947
Collaborator
|
Review requested:
|
nodejs-github-bot
added
meta
Contributor
|
Very suspect PR, changing more than 3000 files! |
Contributor
Author
|
@MikeMcC399 |
Contributor
Author
|
Closed in favor of #62306 β the previous PR included unrelated commits due to a fork sync issue. Sorry for the noise! |
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
20 participants
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.
When
envoption is not explicitly provided tonew Worker(),runtime changes to
process.env.NODE_OPTIONS(e.g. adding--import)are not picked up by the worker thread.
This happens because in
node_worker.cc, theNODE_OPTIONSparsingblock is gated behind
args[1]->IsObject() || args[2]->IsArray().When
envis not provided,args[1]isnull(cloned fromprocess.env), so the condition is false andNODE_OPTIONSparsingis skipped entirely.
This fix adds
args[1]->IsNull()to the condition so thatNODE_OPTIONSis also parsed when the env is inherited (cloned)from the parent process.