[v22.x backport] Backport 57519 and 57479 #58217
Open
+58,088
−38,510
Conversation
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
|
Review requested:
|
nodejs-github-bot
added
c++
jazelly
added
the
commit-queue-rebase
|
@jazelly Any reason why this was closed? Would you like to pick it up again? |
|
I got a couple of JS crash/OOM in GHA like https://github.com/nodejs/node/actions/runs/14944261967/job/41985588150?pr=58217 I was thinking maybe this cannot be backported without backporting #55453? Was closing this to give me more time to debug this. |
|
I think these crashes are irrelevant to this change. |
|
For this GHA failed build on windows I got it reproduced in my windows machine, and reverting #57578 fixed it. It failed due to a major change that lives in v23 and after but not before, specifically this one. The LocalVector was introduced before/in v22, but it was then stablized in v23 with that change. I don't think #57578 should be back ported to v22. Not sure if we have v22.x-staging build periodically. cause I believe the v22.x staging won't build on windows successfully with that LocalVector change. |
We have daily builds of v22.x-staging https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-v22.x-staging/ It looks like Windows builds have been failing since 7 May with e0a025a -- they were passing the day before with 3f5899f. |
Sorry I put a wrong link before. I was talking about this back port commit from #57733
#57733 was back ported to v22.x-staging 5 hours before that, so it could be the one that causing the failed build. If the failure log on windows about the the static assertion, then I think we need to revert that back port. I can raise a separate revert PR to revert backport c408a7f. But I am not sure about the OOM on linux, which I guess are also failing the v22.x-staging build. |
Refs: nodejs#57578 PR-URL: nodejs#57733 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>
Co-authored-by: Antoine du Hamel <[email protected]> PR-URL: nodejs#57508 Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Xuguang Mei <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Zijian Liu <[email protected]>
PR-URL: nodejs#57754 Reviewed-By: Tim Perry <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Clarify that the lexical scope of `const` is invalidated when using top-level `await` in REPL. As part of this change also add tests for the documented behavior Fixes: nodejs#45918 Co-authored-by: Antoine du Hamel <[email protected]> Co-authored-by: Dario Piotrowicz <[email protected]> PR-URL: nodejs#57653 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#57676 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Previously, parseEnv would create multiple environment
variables if a single line contained multiple ‘=‘ characters
(e.g. A=B=C would become { A: ‘B=C’, B: ‘C’ }).
This commit ensures that only the first ‘=‘ is used as
the key-value delimiter, and the rest of the line is treated
as the value.
Fixes: nodejs#57411
PR-URL: nodejs#57421
Reviewed-By: Daniel Lemire <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#57596 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#57599 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ulises Gascón <[email protected]>
Co-authored-by: Jake Yuesong Li <[email protected]> PR-URL: nodejs#57425 Fixes: nodejs#57422 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
PR-URL: nodejs#57742 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
The recent changes made to better protect CI against malicious PRs have changed how the `Retry` button can be used, it will no longer be useful when follow up commits were pushed due to the non-matching commit SHA. PR-URL: nodejs#57743 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#57745 Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#57747 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: James M Snell <[email protected]>
Avoiding use of v8 `Checked()` APIs where appropriate. Few other style cleanups PR-URL: nodejs#57758 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Avoiding use of ToLocalChecked PR-URL: nodejs#57757 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
PR-URL: nodejs#57760 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
When passing the return value of `BackingStore::Data()` as the first or second argument to `memcpy()`, it is unnecessary to cast the returned pointer to `char*`. Refs: nodejs#52292 PR-URL: nodejs#57791 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Allow comparing a `BaseObjectPtr` or implicitly construct a `BaseObjectPtr` with `nullptr`. PR-URL: nodejs#56585 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: nodejs#57865 Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#57868 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Qingyu Deng <[email protected]> Reviewed-By: Zeyu "Alex" Yang <[email protected]>
The latest performance optimization did not take into account that an object may have a property called constructor. This is addressed in this PR by adding a new fast path and using fallbacks. PR-URL: nodejs#57876 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#57852 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#57910 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs#57951 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Pietro Marchini <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Stefan Stojanovic <[email protected]>
PR-URL: nodejs#57943 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#57969 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#57286 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Edy Silva <[email protected]>
PR-URL: nodejs#58047 Refs: nodejs#47452 Refs: nodejs#54918 Reviewed-By: Stephen Belanger <[email protected]>
PR-URL: nodejs#57016 Backport-PR-URL: nodejs#57958 Refs: nodejs#53787 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#57016 Backport-PR-URL: nodejs#57958 Refs: nodejs#53787 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#57170 Backport-PR-URL: nodejs#57958 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#57170 Backport-PR-URL: nodejs#57958 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#57171 Backport-PR-URL: nodejs#57958 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
`parallel/test-config-json-schema` compares to a generated fixture that assumes that Node.js was built with default configuration settings. Skip the test if non-default quic is enabled (`configure --with-quic`). Refs: nodejs#57016 Refs: nodejs#57142 PR-URL: nodejs#57225 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
PR-URL: nodejs#57237 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#57210 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
The documentation states people can use the `$schema` property, but the JSON schema forbids this. This change allows it. PR-URL: nodejs#57560 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Pietro Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message:
Enable --perf-prof flag on MacOS
MacOS actually can share the implementation of {PerfJitLogger} with
Linux. From the issue 40112126, only Fuzzer tests on Windows ran
into UNREACHABLE/FATAL because of the unimplemented {PerfJitLogger}.
This CL enables the flag --perf-prof on MacOS.
Bug: 403765219
Change-Id: I97871fbcc0cb9890c51ca14fd7a6e65bd0e3c0d2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6385655
Reviewed-by: Clemens Backes <[email protected]>
Reviewed-by: Matthias Liedtke <[email protected]>
Auto-Submit: 杨文明 <[email protected]>
Commit-Queue: Clemens Backes <[email protected]>
Cr-Commit-Position: refs/heads/main@{#99885}
Refs: v8/v8@e5dbbba
Co-authored-by: Darshan Sen <[email protected]>
PR-URL: nodejs#58120
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#57519 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#57479 Reviewed-By: Chengzhong Wu <[email protected]>
|
@jazelly thanks for investigating that, I'm able to confirm that c408a7f is the culprit: I've removed it from the staging branch |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backport 2 commits where the second one was rebased on the first one.
src: ensure primordials are initialized exactly once
src: initialize privateSymbols for per_context
The manual backport bypassed the usage of Get/SetPrototypeV2 that does not exist on v22.x.
Refs: #57519
Refs: #57479