Skip to content

chore: bump Node.js to v22.9.0#44281

Merged
jkleinsc merged 88 commits into
mainfrom
nodejs-22
Nov 4, 2024
Merged

chore: bump Node.js to v22.9.0#44281
jkleinsc merged 88 commits into
mainfrom
nodejs-22

Conversation

@codebytere

@codebytere codebytere commented Oct 16, 2024

Copy link
Copy Markdown
Member

Description of Change

Upgrade Node.js to v22.9.0.

Checklist

Release Notes

Notes: Upgrades Node.js to v22.9.0.

@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Oct 16, 2024
@codebytere codebytere changed the title Nodejs 22 chore: bump Node.js to v22.9.0 Oct 16, 2024
@codebytere codebytere force-pushed the nodejs-22 branch 6 times, most recently from 0e5a375 to c3edc06 Compare October 22, 2024 09:22
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label Oct 23, 2024
@codebytere codebytere force-pushed the nodejs-22 branch 4 times, most recently from 0649495 to 837073f Compare October 28, 2024 11:06
@codebytere codebytere force-pushed the nodejs-22 branch 2 times, most recently from 5c4f48f to e7d67d0 Compare October 29, 2024 20:28
@codebytere codebytere added the semver/major incompatible API changes label Oct 30, 2024
@codebytere codebytere requested review from a team as code owners October 31, 2024 16:41

@VerteDinde VerteDinde left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it looks great! Left a few comments on what I thought could be potential breaking or unexpected changes, but am not going to block on them

Comment thread .github/workflows/pipeline-segment-electron-test.yml
Comment on lines +17 to +21
'-fno-rtti',
'-fno-exceptions',
'-fno-strict-aliasing',
- '-std=gnu++17',
+ '-std=gnu++20',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is changing this potentially going to break any native node modules for devs if they don't update their flags to be something similar? Just trying to think if we need to call out anything in the breaking changes doc.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't - this fixes a file split change but won't change anything more than was already changed in #43555!

for await (const args of on(emitter, eventName)) {
if (untilFn(...args)) { return args; }
}
return [];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change a return type here? 👀

Comment thread lib/node/asar-fs-wrapper.ts
Comment thread lib/node/asar-fs-wrapper.ts
Comment thread patches/node/build_enable_perfetto.patch
@@ -174,7 +174,6 @@
'src/timers.cc',
'src/timer_wrap.cc',
'src/tracing/agent.cc',

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#include <openssl/evp.h>
#include <openssl/hmac.h>
#include <openssl/pkcs12.h>
+#include <openssl/rand.h>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upstreamed at nodejs/node#55425

Comment thread lib/node/asar-fs-wrapper.ts
Comment thread patches/node/.patches
esm_drop_support_for_import_assertions.patch
fix_-wextra-semi_errors_in_nghttp2_helper_h.patch
fix_remove_harmony-import-assertions_from_node_cc.patch
win_almost_fix_race_detecting_esrch_in_uv_kill.patch

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work cleaning this up!

Comment thread patches/node/build_enable_perfetto.patch
@@ -1,5 +1,4 @@
#include <node_api.h>
#undef NAPI_VERSION

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be necessary due to a conflict in #include <node_buffer.h> and #include <node_api.h> born of duplicate declarations, and isn't anymore as the duplicates got refactored out.

});

it('does not inherit parent env when custom env is provided', async () => {
// TODO(codebytere): figure out why this is failing in ASAN- builds on Linux.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails in ASAN but importantly does not cause a crash (the test just fails), so I'm disabling it for now to unblock the roll. There are no issues in non-ASAN builds.

@@ -0,0 +1,70 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addresses nodejs/node#51766 - which we were seeing in CI on Windows.

Comment thread .github/workflows/pipeline-segment-electron-test.yml

@jkleinsc jkleinsc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM

@ckerr ckerr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM

@release-clerk

release-clerk Bot commented Nov 4, 2024

Copy link
Copy Markdown

Release Notes Persisted

Upgrades Node.js to v22.9.0.

VerteDinde added a commit that referenced this pull request Nov 8, 2024
VerteDinde added a commit that referenced this pull request Nov 8, 2024
* Revert "chore: bump Node.js to v22.9.0 (#44281)"

This reverts commit c63d0d6.

* chore: update patches
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants