Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

deps: cherry-pick e560815 from upstream v8 #12779

Merged
merged 1 commit into from
May 12, 2017

Conversation

ofrobots
Copy link
Contributor

@ofrobots ofrobots commented May 1, 2017

Original commit message:

[runtime] Fix Array.prototype.concat with complex @@ species
Array.prototype.concat does not properly handle JSProxy species that will
modify the currently visited array.

BUG=682194

Review-Url: https://codereview.chromium.org/2655623004
Cr-Commit-Position: refs/heads/master@{#42640}

This is already fixed on V8 5.7 and 5.8. Needs to be fixed on V8 5.5 (Node 7) and V8 5.1 (Node 6).

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

deps:v8

CI: https://ci.nodejs.org/job/node-test-pull-request/7781/
V8 CI: https://ci.nodejs.org/view/All/job/node-test-commit-v8-linux/675/

@nodejs-github-bot nodejs-github-bot added v7.x v8 engine Issues and PRs related to the V8 dependency. labels May 1, 2017
@ofrobots ofrobots force-pushed the backport-682194 branch 2 times, most recently from 618fea0 to 86951a7 Compare May 1, 2017 21:10
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

Original commit message:
  [runtime] Fix Array.prototype.concat with complex @@species
  Array.prototype.concat does not properly handle JSProxy species that will
  modify the currently visited array.

  BUG=682194

  Review-Url: https://codereview.chromium.org/2655623004
  Cr-Commit-Position: refs/heads/master@{nodejs#42640}

PR-URL: nodejs#12779
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: targos - Michaël Zasso <targos@protonmail.com>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
@ofrobots ofrobots merged commit 57c0e71 into nodejs:v7.x-staging May 12, 2017
@ofrobots
Copy link
Contributor Author

Thanks. Landed on v7.x-staging as 57c0e71.

@ofrobots ofrobots deleted the backport-682194 branch May 12, 2017 02:03
ofrobots added a commit to ofrobots/node that referenced this pull request Oct 10, 2017
Original commit message:
  [runtime] Fix Array.prototype.concat with complex @@species
  Array.prototype.concat does not properly handle JSProxy species that will
  modify the currently visited array.

  BUG=682194

  Review-Url: https://codereview.chromium.org/2655623004
  Cr-Commit-Position: refs/heads/master@{nodejs#42640}

PR-URL:
Refs: nodejs#12779
@ofrobots
Copy link
Contributor Author

Removing label lts-watch-v6.x as I have opened #16133 for v6.x

@nodejs/lts note that the process for auditing potential backports for 6.x failed for this one. This was originally fixed in May 11 but never ported back to v6.x. I suspect this is because this bugfix never landed on master because it was not needed there.

How do prevent issues like this from leaking? Do we need an audit of all issues still tagged with lts-watch-v6.x?

@gibfahn
Copy link
Member

gibfahn commented Oct 12, 2017

How do prevent issues like this from leaking? Do we need an audit of all issues still tagged with lts-watch-v6.x?

I think branch-diff didn't catch this because it never went into a 7.x release. I guess it might make sense to check v7.x-staging after v7.x goes EoL.

Here's the command we currently use:

branch-diff v6.x-staging upstream/v7.x --exclude-label semver-major,semver-minor,dont-land-on-v6.x,backport-requested-v6.x,backported-to-v6.x,baking-for-lts --reverse --filter-release Raw

MylesBorins pushed a commit that referenced this pull request Oct 13, 2017
Original commit message:
  [runtime] Fix Array.prototype.concat with complex @@species
  Array.prototype.concat does not properly handle JSProxy species that will
  modify the currently visited array.

  BUG=682194

  Review-Url: https://codereview.chromium.org/2655623004
  Cr-Commit-Position: refs/heads/master@{#42640}

Refs: #12779

PR-URL: #16133
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
Original commit message:
  [runtime] Fix Array.prototype.concat with complex @@species
  Array.prototype.concat does not properly handle JSProxy species that will
  modify the currently visited array.

  BUG=682194

  Review-Url: https://codereview.chromium.org/2655623004
  Cr-Commit-Position: refs/heads/master@{#42640}

Refs: #12779

PR-URL: #16133
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants