Skip to content

Conversation

@targos
Copy link
Member

@targos targos commented Jun 30, 2024

@targos targos added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jun 30, 2024
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Jun 30, 2024
@targos
Copy link
Member Author

targos commented Jun 30, 2024

ASan build error:

../deps/v8/src/common/code-memory-access.h:548:1: error: declaration of anonymous class must be a definition class V8_NODISCARD V8_ALLOW_UNUSED NopRwxMemoryWriteScope final { ^ ../deps/v8/src/common/code-memory-access.h:548:1: warning: declaration does not declare anything [-Wmissing-declarations] 1 warning and 1 error generated. 
@anonrig
Copy link
Member

anonrig commented Jun 30, 2024

@targos where can i find the changelog for this release?

@gengjiawen
Copy link
Member

@targos where can i find the changelog for this release?

V8 no longer give release date anymore https://v8.dev/blog/discontinuing-release-posts

@gengjiawen
Copy link
Member

ASan build error:

../deps/v8/src/common/code-memory-access.h:548:1: error: declaration of anonymous class must be a definition class V8_NODISCARD V8_ALLOW_UNUSED NopRwxMemoryWriteScope final { ^ ../deps/v8/src/common/code-memory-access.h:548:1: warning: declaration does not declare anything [-Wmissing-declarations] 1 warning and 1 error generated. 

Maybe give ubuntu 24 a try ? Clang 11 (latest is 18) maybe have compatible issues for new grammer.

@targos
Copy link
Member Author

targos commented Jul 1, 2024

@gengjiawen See #52374. TBH I will probably open a PR to disable test-asan as I'm not getting any help.

targos added a commit to targos/node that referenced this pull request Jul 14, 2024
It is running on ubuntu-20.04, which will inevitably be removed from GitHub actions at some point. Attempts to upgrade it to ubuntu-22.04 and ubuntu-24.04 have failed. It is now blocking V8 updates because of errors that happen only with the `test-asan` job. Refs: nodejs#52374 Refs: nodejs#53651 (comment)
nodejs-github-bot pushed a commit that referenced this pull request Jul 16, 2024
It is running on ubuntu-20.04, which will inevitably be removed from GitHub actions at some point. Attempts to upgrade it to ubuntu-22.04 and ubuntu-24.04 have failed. It is now blocking V8 updates because of errors that happen only with the `test-asan` job. Refs: #52374 Refs: #53651 (comment) PR-URL: #53844 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 17, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 17, 2024
@targos
Copy link
Member Author

targos commented Jul 17, 2024

Unfortunately this seems to be blocked on macOS system update:

10:00:55 In file included from Release/obj/gen/torque-generated/src/builtins/js-to-wasm-tq-csa.cc:1: 10:00:55 In file included from ../deps/v8/src/ast/ast.h:10: 10:00:55 In file included from ../deps/v8/src/ast/ast-value-factory.h:38: 10:00:55 In file included from ../deps/v8/src/objects/name.h:12: 10:00:55 In file included from ../deps/v8/src/objects/primitive-heap-object.h:8: 10:00:55 In file included from ../deps/v8/src/objects/heap-object.h:11: 10:00:55 In file included from ../deps/v8/src/objects/slots.h:11: 10:00:55 In file included from ../deps/v8/src/sandbox/external-pointer-table.h:13: 10:00:55 In file included from ../deps/v8/src/sandbox/compactible-external-entity-table.h:10: 10:00:55 In file included from ../deps/v8/src/sandbox/external-entity-table.h:15: 10:00:55 ../deps/v8/src/common/code-memory-access.h:548:1: error: declaration of anonymous class must be a definition 10:00:55 class V8_NODISCARD V8_ALLOW_UNUSED NopRwxMemoryWriteScope final { 10:00:55 ^ 10:00:55 ../deps/v8/src/common/code-memory-access.h:548:1: warning: declaration does not declare anything [-Wmissing-declarations] 10:00:55 1 warning and 1 error generated. 10:00:55 make[2]: *** [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx11-x64/out/Release/obj.target/v8_initializers_slow/gen/torque-generated/src/builtins/js-to-wasm-tq-csa.o] Error 1 
@targos
Copy link
Member Author

targos commented Jul 17, 2024

We have a lot of debug build crashes related to heap dump and heap snapshot: https://ci.nodejs.org/job/node-test-commit-arm-debug/13753/#showFailuresLink

@legendecas @joyeecheung

@targos
Copy link
Member Author

targos commented Jul 17, 2024

targos added 6 commits July 17, 2024 12:21
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 12.7. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
dllexport introduces issues when compiling with MSVC. PR-URL: nodejs#47251 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
It introduces process hangs on some platforms because Node.js doesn't tear down V8 correctly. Disable it while we work on a solution. Refs: nodejs#47297 Refs: https://bugs.chromium.org/p/v8/issues/detail?id=13902 PR-URL: nodejs#47450 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=14221 PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos and others added 7 commits July 17, 2024 12:21
Original commit message: [heap] Fix allocation tracker line end retrieval Tests in NodeJS with DCHECK enabled were failing because of two different problems: - One was that we were also disallowing heap allocations in the serialization stage. But NodeJS tests process the heap snapshot result in JS, so all those were broken. - But, also, the code that would retrieve from line ends would assume all the scripts populated line ends in the snapshot. But this was wrong. To fix it, this patch adds another storage of the line ends in the allocation tracker. This storage needs to keep weak references to the scripts so we do not leak the line ends data when scripts are disposed. This change keeps a weak reference to the scripts so, when they are freed, the line ends cache is also freed as it is not useful anymore. Node issue: nodejs/node-v8#282 Change-Id: I4314d707903175f0005893e9aa06d3ae52fc57f8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5531355 Commit-Queue: José Dapena Paz <jdapena@igalia.com> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Simon Zünd <szuend@chromium.org> Cr-Commit-Position: refs/heads/main@{#94418} Refs: v8/v8@058870c
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5447267 Co-authored-by: Michaël Zasso <targos@protonmail.com>
As V8 has moved away from wrapper-descriptor-based CppHeap, this patch: 1. Create the CppHeap without using wrapper descriptors. 2. Deprecates node::SetCppgcReference() in favor of v8::Object::Wrap() since the wrapper descriptor is no longer relevant. It is still kept as a compatibility layer for addons that need to also work on Node.js versions without v8::Object::Wrap().
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 17, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 17, 2024
ehsankhfr pushed a commit to ehsankhfr/node that referenced this pull request Jul 18, 2024
It is running on ubuntu-20.04, which will inevitably be removed from GitHub actions at some point. Attempts to upgrade it to ubuntu-22.04 and ubuntu-24.04 have failed. It is now blocking V8 updates because of errors that happen only with the `test-asan` job. Refs: nodejs#52374 Refs: nodejs#53651 (comment) PR-URL: nodejs#53844 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Add/remove abseil files introduced by V8 12.7 update found by: ``` git diff-tree --no-commit-id --name-status 0ec8f7e -r | grep '^[AD].*abseil.*' ```
@richardlau
Copy link
Member

richardlau commented Jul 18, 2024

I pushed a8deb42 which looks like it will fix the AIX build (missing abseil files from our gyp file). Feel free to squash/reword as necessary.

(I only looked at abseil as that was what was broken on AIX -- I haven't validated whether anything else is missing from the gypfile from the V8 update.)

@richardlau
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/60432/

https://ci.nodejs.org/job/node-test-commit-aix/52565/nodes=aix72-ppc64/ failed with lots of EMLINK errors -- this was because the temp dir used by tests had too many entries in it (I'm wondering if there's some failure path that doesn't clear the temp dir and we've just accumulated too much). I cleared out the directory and reran https://ci.nodejs.org/job/node-test-commit-aix/52567/ which now passes.

@targos
Copy link
Member Author

targos commented Jul 19, 2024

My last commit should fix the linker errors related to zlib. I expect the next CI to only fail because of macOS.

targos added a commit that referenced this pull request Jul 28, 2024
It is running on ubuntu-20.04, which will inevitably be removed from GitHub actions at some point. Attempts to upgrade it to ubuntu-22.04 and ubuntu-24.04 have failed. It is now blocking V8 updates because of errors that happen only with the `test-asan` job. Refs: #52374 Refs: #53651 (comment) PR-URL: #53844 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
@targos
Copy link
Member Author

targos commented Jul 28, 2024

@targos targos closed this Jul 28, 2024
marco-ippolito pushed a commit that referenced this pull request Aug 19, 2024
It is running on ubuntu-20.04, which will inevitably be removed from GitHub actions at some point. Attempts to upgrade it to ubuntu-22.04 and ubuntu-24.04 have failed. It is now blocking V8 updates because of errors that happen only with the `test-asan` job. Refs: #52374 Refs: #53651 (comment) PR-URL: #53844 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Aug 19, 2024
It is running on ubuntu-20.04, which will inevitably be removed from GitHub actions at some point. Attempts to upgrade it to ubuntu-22.04 and ubuntu-24.04 have failed. It is now blocking V8 updates because of errors that happen only with the `test-asan` job. Refs: #52374 Refs: #53651 (comment) PR-URL: #53844 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Aug 19, 2024
It is running on ubuntu-20.04, which will inevitably be removed from GitHub actions at some point. Attempts to upgrade it to ubuntu-22.04 and ubuntu-24.04 have failed. It is now blocking V8 updates because of errors that happen only with the `test-asan` job. Refs: #52374 Refs: #53651 (comment) PR-URL: #53844 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos deleted the v8-127 branch May 2, 2025 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version. v8 engine Issues and PRs related to the V8 dependency.

10 participants