Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

[v10.x backport] deps: V8: cherry-pick eec10a2fd8fa #35393

Closed
wants to merge 1 commit into from
Closed

[v10.x backport] deps: V8: cherry-pick eec10a2fd8fa #35393

wants to merge 1 commit into from

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented Sep 28, 2020

Original commit message:

[promisehook] Add before/after hooks to thenable tasks

This will allow Node.js to properly track async context in thenables.

Change-Id: If441423789a78307a57ad7e645daabf551cddb57
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Sathya Gunasekaran  <gsathya@chromium.org>
Commit-Queue: Gus Caplan <me@gus.host>
Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: #33778
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Michaël Zasso targos@protonmail.com
Reviewed-By: Gus Caplan me@gus.host
Reviewed-By: Gerhard Stöbich deb2001-github@yahoo.de
Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@Qard @Flarna
deps: V8: cherry-pick eec10a2fd8fa
795dba7 
Original commit message:

    [promisehook] Add before/after hooks to thenable tasks

    This will allow Node.js to properly track async context in thenables.

    Change-Id: If441423789a78307a57ad7e645daabf551cddb57
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Sathya Gunasekaran  <gsathya@chromium.org>
    Commit-Queue: Gus Caplan <me@gus.host>
    Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: nodejs#33778
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v10.x v8 engine Issues and PRs related to the V8 dependency. labels Sep 28, 2020
@Flarna Flarna added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2020
@nodejs-github-bot

This comment has been minimized.

# to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 28, 2020 •
edited by Flarna
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/33275/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/3409

@Flarna
Copy link
Member Author

Flarna commented Sep 28, 2020

It seems there is a problem with V8 CI on v10.x-staging. My run failed but I'm quite sure it's unrelated to my changes.

I triggered V8 CI on v10.x-staging branch and it shows the same failures: see https://ci.nodejs.org/job/node-test-commit-v8-linux/3412/

  • Compile error because of a warning in mark-compact.cc for rhel7-s390x,v8test and centos7-ppcle,v8test
  • Test v8-updates/test-linux-perf fails because Couldn't find interpreted functionOne()

@nodejs/v8 Are these known issues on v10?

@gengjiawen
Copy link
Member

It seems there is a problem with V8 CI on v10.x-staging. My run failed but I'm quite sure it's unrelated to my changes.

I triggered V8 CI on v10.x-staging branch and it shows the same failures: see ci.nodejs.org/job/node-test-commit-v8-linux/3412

  • Compile error because of a warning in mark-compact.cc for rhel7-s390x,v8test and centos7-ppcle,v8test
  • Test v8-updates/test-linux-perf fails because Couldn't find interpreted functionOne()

@nodejs/v8 Are these known issues on v10?

cc @mmarchini Any thought on v8-updates/test-linux-perf fails ?

@Flarna
Copy link
Member Author

Flarna commented Oct 1, 2020

@nodejs/build Any recent changes in compiler versions used which could explain the unrelated warning?

@rvagg
Copy link
Member

rvagg commented Oct 2, 2020

No, shouldn't be for 10.x. But it might be a question for the IBM folks since compiler failures are on IBM platforms.

@richardlau richardlau mentioned this pull request Oct 5, 2020
Closed
@richardlau
Copy link
Member

No, shouldn't be for 10.x. But it might be a question for the IBM folks since compiler failures are on IBM platforms.

Raised #35504 to investigate.

@richardlau
Copy link
Member

Cherry-picked a commit over to v10.x-staging that ought to fix the builds for the IBM platforms.

V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/3443/

@Flarna Flarna requested a review from Qard October 5, 2020 19:07
@Flarna Flarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 5, 2020
richardlau pushed a commit that referenced this pull request Oct 7, 2020
@Qard @richardlau
deps: V8: cherry-pick eec10a2fd8fa
3564424 
Original commit message:

    [promisehook] Add before/after hooks to thenable tasks

    This will allow Node.js to properly track async context in thenables.

    Change-Id: If441423789a78307a57ad7e645daabf551cddb57
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Sathya Gunasekaran  <gsathya@chromium.org>
    Commit-Queue: Gus Caplan <me@gus.host>
    Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: #33778
Backport-PR-URL: #35393
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@richardlau
Copy link
Member

Landed in 3564424.

@richardlau richardlau closed this Oct 7, 2020
@Flarna Flarna deleted the v10-backport-thenables branch October 7, 2020 11:00
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Qard Qard Qard approved these changes

@BethGriggs BethGriggs BethGriggs approved these changes

@ryzokuken ryzokuken ryzokuken approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@Flarna @nodejs-github-bot @gengjiawen @rvagg @richardlau @Qard @BethGriggs @ryzokuken