| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Review requested:
|
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
|
Looks like the recent flakes are also present in v18.x staging now, we should probably backport those test fixes there too.. |
Sorry, something went wrong.
|
@RafaelGSS Are the failures here expected? #44473 was closed but this still doesn't seem like it's related to the backport in any way. |
Sorry, something went wrong.
I'm not sure. The #44473 didn't land. @joyeecheung are you sure all the patches are in v18.x-staging? Basically, the proposal contains everything from this list: branch-diff upstream/v18.x-staging upstream/main --exclude-label=semver-major,dont-land-on-v18.x,backport-requested-v18.x,backport-blocked-v18.x,backport-open-v18.x,backported-to-v18.x --filter-release --format=simple --reverse |
Sorry, something went wrong.
Sorry, something went wrong.
|
@RafaelGSS Yes, I think the tests fixes have been landed. The new test failures were caused by #44402 and #44366, those probably need some proper fix (maybe #44571 does it for the watch mode failure) |
Sorry, something went wrong.
|
@nodejs/backporters Can this be merged? As in #44571 this is something I’d leave to y’all unless you’d prefer for me to land this. |
Sorry, something went wrong.
|
I think so. I'll do the next regular release, in case it conflicts somehow I'll let you know. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM.
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
|
@addaleax I'm preparing the next v18 release. Could you please rebase to solve the conflicts? |
Sorry, something went wrong.
So far, process initialization has been a bit all over the place in Node.js. `InitializeNodeWithArgs()` is our main public API for this, but inclusion of items in it vs. `InitializeOncePerProcess()` and `PlatformInit()` has been random at best. Likewise, some pieces of initialization have been guarded by `NODE_SHARED_MODE`, but also fairly randomly and without any meaningful connection to shared library usage. This leaves embedders in a position to cherry-pick some of the initialization code into their own code to make their application behave like typical Node.js applications to the degree to which they desire it. Electron takes an alternative route and makes direct use of `InitializeOncePerProcess()` already while it is a private API, with a `TODO` to add it to the public API in Node.js. This commit addresses that `TODO`, and `TODO`s around the `NODE_SHARED_MODE` usage. Specifically: - `InitializeOncePerProcess()` and `TearDownOncePerProcess()` are added to the public API. - The `flags` option of these functions are merged with the `flags` option for `InitializeNodeWithArgs()`, since they essentially share the same semantics. - The return value of the function is made an abstract class, rather than a struct, for easier API/ABI stability. - Initialization code from `main()` is brought into these functions (since that makes sense in general). - Add a `TODO` for turning `InitializeNodeWithArgs()` into a small wrapper around `InitializeOncePerProcess()` and eventually removing it (at least one major release cycle each, presumably). - Remove `NODE_SHARED_MODE` guards and replace them with runtime options. PR-URL: nodejs#44121 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
|
@RafaelGSS Yes, done. It’s highly frustrating that backport PRs just lay around as open PRs with no conflicts and green CI for weeks, and then miss a release because they’ve started having a conflict with a later backport and I happened to have been unavailable for a work week. This sucks. |
Sorry, something went wrong.
Sorry, something went wrong.
So far, process initialization has been a bit all over the place in Node.js. `InitializeNodeWithArgs()` is our main public API for this, but inclusion of items in it vs. `InitializeOncePerProcess()` and `PlatformInit()` has been random at best. Likewise, some pieces of initialization have been guarded by `NODE_SHARED_MODE`, but also fairly randomly and without any meaningful connection to shared library usage. This leaves embedders in a position to cherry-pick some of the initialization code into their own code to make their application behave like typical Node.js applications to the degree to which they desire it. Electron takes an alternative route and makes direct use of `InitializeOncePerProcess()` already while it is a private API, with a `TODO` to add it to the public API in Node.js. This commit addresses that `TODO`, and `TODO`s around the `NODE_SHARED_MODE` usage. Specifically: - `InitializeOncePerProcess()` and `TearDownOncePerProcess()` are added to the public API. - The `flags` option of these functions are merged with the `flags` option for `InitializeNodeWithArgs()`, since they essentially share the same semantics. - The return value of the function is made an abstract class, rather than a struct, for easier API/ABI stability. - Initialization code from `main()` is brought into these functions (since that makes sense in general). - Add a `TODO` for turning `InitializeNodeWithArgs()` into a small wrapper around `InitializeOncePerProcess()` and eventually removing it (at least one major release cycle each, presumably). - Remove `NODE_SHARED_MODE` guards and replace them with runtime options. PR-URL: #44121 Backport-PR-URL: #44358 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
| Back | FazBrowse Home | New Git URL |
Backport of #44121, only conflict was a minor conflict in node.h with #43629
So far, process initialization has been a bit all over the place
in Node.js. InitializeNodeWithArgs() is our main public API
for this, but inclusion of items in it vs. InitializeOncePerProcess()
and PlatformInit() has been random at best. Likewise,
some pieces of initialization have been guarded by
NODE_SHARED_MODE, but also fairly randomly and without
any meaningful connection to shared library usage.
This leaves embedders in a position to cherry-pick some of
the initialization code into their own code to make their
application behave like typical Node.js applications to the
degree to which they desire it.
Electron takes an alternative route and makes direct use of
InitializeOncePerProcess() already while it is a private
API, with a TODO to add it to the public API in Node.js.
This commit addresses that TODO, and TODOs around the
NODE_SHARED_MODE usage. Specifically:
are added to the public API.
flags option for InitializeNodeWithArgs(), since they
essentially share the same semantics.
rather than a struct, for easier API/ABI stability.
functions (since that makes sense in general).
a small wrapper around InitializeOncePerProcess() and
eventually removing it (at least one major release cycle
each, presumably).
runtime options.
PR-URL: #44121
Reviewed-By: Joyee Cheung joyeec9h3@gmail.com
Reviewed-By: Michael Dawson midawson@redhat.com