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.

Sign up for GitHub

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

Jump to bottom

deps: backport 5c8cb16 from upstream V8 #9422

Closed
wants to merge 1 commit into from
Closed

deps: backport 5c8cb16 from upstream V8 #9422

wants to merge 1 commit into from

Conversation

cristiancavalli
Copy link

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

deps

Description of change

Backport of bugfix from upstream V8

cc @nodejs/v8
V8 commit: v8/v8@5c8cb16

@mscdex mscdex added the v8 engine Issues and PRs related to the V8 dependency. label Nov 2, 2016
@mscdex mscdex mentioned this pull request Nov 2, 2016
Closed
bnoordhuis
bnoordhuis approved these changes Nov 3, 2016
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM. This is a back-port and not a clean cherry-pick, right?

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

Someone should start the V8 CI once it's working again.

@ofrobots
Copy link
Contributor

ofrobots commented Nov 3, 2016

This is indeed a port of the bugfix (i.e. cherry pick doesn't land cleanly on v6.x, and needs fixup). @cristiancavalli Can you s/cherry-pick/backport/ in the commit message?

@ofrobots
Copy link
Contributor

ofrobots commented Nov 3, 2016

/cc @nodejs/v8 as the original mention didn't work.

@cristiancavalli cristiancavalli changed the title deps: cherry-pick 5c8cb16 from upstream V8 deps: backport 5c8cb16 from upstream V8 Nov 3, 2016
@cristiancavalli
Copy link
Author

@ofrobots updated

@ofrobots
Copy link
Contributor

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

@ofrobots
Copy link
Contributor

@cristiancavalli I suspect that this PR needs a rebase before the V8 CI can run. Can you rebase?

@cristiancavalli
Copy link
Author

@ofrobots Rebased

@bnoordhuis
Copy link
Member

bnoordhuis commented Nov 10, 2016 •
edited
Loading

https://ci.nodejs.org/job/node-test-pull-request/4821/
https://ci.nodejs.org/job/node-test-commit-v8-linux/408/

EDIT1: https://ci.nodejs.org/job/node-test-commit-v8-linux/409/
EDIT2: Sigh, the V8 job still seems broken... "fatal: unable to access 'https://github.com/cristiancavalli/node.git/branches/HEAD/deps/v8/': The requested URL returned error: 410"

@ofrobots
Copy link
Contributor

/cc @mhdawson @jbajwa regarding the V8 CI issue above.

@jbajwa
Copy link
Contributor

jbajwa commented Nov 10, 2016 •
edited
Loading

I see the branch is missing the fix for V8 job. 4aca347

@ofrobots
Copy link
Contributor

Ah! That's because this fix is targeting the v6.x-staging branch. @jbajwa could we get 4aca347 backported to the LTS branches?

@MylesBorins
Copy link
Contributor

MylesBorins commented Nov 10, 2016 •
edited
Loading

@ofrobots I'm backporting to v6.x and v4.x right now... just running some local tests first

edit: done 🎉

@jbajwa
Copy link
Contributor

jbajwa commented Nov 10, 2016

@thealphanerd thanks.

@cristiancavalli
deps: backport 5c8cb16 from upstream V8
5c21909 
Original Commit Message:
  [ic] Don't call LookupIterator::GetStoreTarget() when receiver is not a JSReceiver.

  BUG=chromium:619166,chromium:625155

  Review-Url: https://codereview.chromium.org/2175273002
  Cr-Commit-Position: refs/heads/master@{nodejs#38018}
@ofrobots
Copy link
Contributor

ofrobots commented Nov 14, 2016 •
edited
Loading

New V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/415/
CI: https://ci.nodejs.org/job/node-test-pull-request/4842/

ofrobots pushed a commit that referenced this pull request Nov 15, 2016
@cristiancavalli @ofrobots
deps: backport 5c8cb16 from upstream V8
bda45b5 
Original Commit Message:
  [ic] Don't call LookupIterator::GetStoreTarget() when receiver is not a JSReceiver.

  BUG=chromium:619166,chromium:625155

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

PR-URL: #9422
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
@ofrobots
Copy link
Contributor

ofrobots commented Nov 15, 2016 •
edited
Loading

Landed on v6.x-staging as bda45b5.

@ofrobots ofrobots closed this Nov 15, 2016
@MylesBorins MylesBorins mentioned this pull request Nov 22, 2016
Merged
@ofrobots ofrobots mentioned this pull request Dec 29, 2016
Closed
ofrobots pushed a commit to matthewaveryusa/node that referenced this pull request Dec 29, 2016
@matthewaveryusa @ofrobots
deps: fix compile bug in v8/lookup.h
516b627 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport nodejs#9422.

Ref: nodejs#9422
PR-URL: nodejs#10525
ofrobots pushed a commit that referenced this pull request Jan 4, 2017
@matthewaveryusa @ofrobots
deps: fix compile bug in v8/lookup.h
1859558 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport #9422.

Ref: #9422
PR-URL: #10525
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jan 5, 2017
@matthewaveryusa @MylesBorins
deps: fix compile bug in v8/lookup.h
463addc 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport #9422.

Ref: #9422
PR-URL: #10525
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@matthewaveryusa @MylesBorins
deps: fix compile bug in v8/lookup.h
182e042 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport #9422.

Ref: #9422
PR-URL: #10525
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
@matthewaveryusa @MylesBorins
deps: fix compile bug in v8/lookup.h
fdc373d 
Fixed a small error that manifests when --debug is specified. This
seems to have been introduced during the backport #9422.

Ref: #9422
PR-URL: #10525
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
@aliscco aliscco mentioned this pull request Jun 21, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos approved these changes

@jasnell jasnell jasnell approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

Assignees
No one assigned
Labels
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.
9 participants
@cristiancavalli @ofrobots @bnoordhuis @jbajwa @MylesBorins @jasnell @targos @mscdex @nodejs-github-bot