deps: backport 22c5e46 from V8 #7584
Merged
Conversation
This file contains 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
|
/cc @nodejs/lts @nodejs/v8 @misterdjules |
|
You need to bump the patch level in v8-version.h. |
|
It's still not clear to me why the diagnostic code was left, and why it still makes some node programs crash when the bug fix for the associated issue is present in the code base. I wouldn't want to merge this change before these two questions are answered. |
The reason why the diagnostic code crashes on some OSes (like SmartOS) while the fix for the original code is in the code base is now clear per #7574 (comment), so I'm OK with merging this, as long as @bnoordhuis' comments are addressed. |
Wouldn't we end up with a V8 patch level that doesn't exist in any V8 release line? |
|
Yes, but that's alright for 4.5 because we're the de facto upstream. |
|
Any ideas when this would be merged and released for 4.5? |
|
@thealphanerd Do you mind updating the patch level? Otherwise I'd need to create another PR from another branch. |
MylesBorins
force-pushed
the
backport/22c5e46
branch
from
6679e61
to
7fb0bc5
Compare
|
@thealphanerd Thanks for bumping the patch level! |
|
bumped patch level. Completely at a loss as to why I couldn't find the file at first. Will make sure to bump that in the future I'm going to mention in nodejs/Release#111 that we should perhaps look at having an official LTS 4.5 release. /cc @natorion |
|
can I get some LGTM's on this please 😄 |
|
LGTM |
1 similar comment
|
LGTM |
|
LGTM. |
This removes the diagnostic code for the issue described in https://bugs.chromium.org/p/chromium/issues/detail?id=454297. That issue is private, probably due to the fact that it contains information about a security vulnerability. The original issue was fixed in V8 by https://codereview.chromium.org/1286343004, which was integrated into node v4.x with c431725, so there's no need for the corresponding diagnostic code anymore. Original commit message: [heap] Remove debugging code of crbug/454297. BUG= Review URL: https://codereview.chromium.org/1420253002 Cr-Commit-Position: refs/heads/master@{nodejs#31523} PR-URL: nodejs#7584 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
MylesBorins
force-pushed
the
backport/22c5e46
branch
from
7fb0bc5
to
4184cb0
Compare
|
thank everyone landed in 4184cb0 |
MylesBorins
pushed a commit
that referenced
this pull request
Jul 11, 2016
This removes the diagnostic code for the issue described in https://bugs.chromium.org/p/chromium/issues/detail?id=454297. That issue is private, probably due to the fact that it contains information about a security vulnerability. The original issue was fixed in V8 by https://codereview.chromium.org/1286343004, which was integrated into node v4.x with c431725, so there's no need for the corresponding diagnostic code anymore. Original commit message: [heap] Remove debugging code of crbug/454297. BUG= Review URL: https://codereview.chromium.org/1420253002 Cr-Commit-Position: refs/heads/master@{#31523} PR-URL: #7584 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
MylesBorins
pushed a commit
that referenced
this pull request
Jul 12, 2016
This removes the diagnostic code for the issue described in https://bugs.chromium.org/p/chromium/issues/detail?id=454297. That issue is private, probably due to the fact that it contains information about a security vulnerability. The original issue was fixed in V8 by https://codereview.chromium.org/1286343004, which was integrated into node v4.x with c431725, so there's no need for the corresponding diagnostic code anymore. Original commit message: [heap] Remove debugging code of crbug/454297. BUG= Review URL: https://codereview.chromium.org/1420253002 Cr-Commit-Position: refs/heads/master@{#31523} PR-URL: #7584 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Merged
richardlau
added a commit
to ibmruntimes/node
that referenced
this pull request
Jul 14, 2016
Original commit message: deps backport 22c5e46 from V8 This removes the diagnostic code for the issue described in https://bugs.chromium.org/p/chromium/issues/detail?id=454297. That issue is private, probably due to the fact that it contains information about a security vulnerability. The original issue was fixed in V8 by https://codereview.chromium.org/1286343004, which was integrated into node v4.x with c431725, so there's no need for the corresponding diagnostic code anymore. Original commit message: [heap] Remove debugging code of crbug/454297. BUG= Review URL: https://codereview.chromium.org/1420253002 Cr-Commit-Position: refs/heads/master@{#31523} PR-URL: nodejs/node#7584 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
MylesBorins
pushed a commit
that referenced
this pull request
Jul 14, 2016
This removes the diagnostic code for the issue described in https://bugs.chromium.org/p/chromium/issues/detail?id=454297. That issue is private, probably due to the fact that it contains information about a security vulnerability. The original issue was fixed in V8 by https://codereview.chromium.org/1286343004, which was integrated into node v4.x with c431725, so there's no need for the corresponding diagnostic code anymore. Original commit message: [heap] Remove debugging code of crbug/454297. BUG= Review URL: https://codereview.chromium.org/1420253002 Cr-Commit-Position: refs/heads/master@{#31523} PR-URL: #7584 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
MylesBorins
pushed a commit
that referenced
this pull request
Jul 14, 2016
This removes the diagnostic code for the issue described in https://bugs.chromium.org/p/chromium/issues/detail?id=454297. That issue is private, probably due to the fact that it contains information about a security vulnerability. The original issue was fixed in V8 by https://codereview.chromium.org/1286343004, which was integrated into node v4.x with c431725, so there's no need for the corresponding diagnostic code anymore. Original commit message: [heap] Remove debugging code of crbug/454297. BUG= Review URL: https://codereview.chromium.org/1420253002 Cr-Commit-Position: refs/heads/master@{#31523} PR-URL: #7584 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
richardlau
added a commit
to ibmruntimes/node
that referenced
this pull request
Jul 18, 2016
Original commit message: deps backport 22c5e46 from V8 This removes the diagnostic code for the issue described in https://bugs.chromium.org/p/chromium/issues/detail?id=454297. That issue is private, probably due to the fact that it contains information about a security vulnerability. The original issue was fixed in V8 by https://codereview.chromium.org/1286343004, which was integrated into node v4.x with c431725, so there's no need for the corresponding diagnostic code anymore. Original commit message: [heap] Remove debugging code of crbug/454297. BUG= Review URL: https://codereview.chromium.org/1420253002 Cr-Commit-Position: refs/heads/master@{#31523} PR-URL: nodejs/node#7584 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
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.
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
deps
Description of change
This removes the diagnostic code for the issue described in
https://bugs.chromium.org/p/chromium/issues/detail?id=454297. That issue
is private, probably due to the fact that it contains information about
a security vulnerability.
The original issue was fixed in V8 by
https://codereview.chromium.org/1286343004, which was integrated into
node v4.x with c431725, so there's no
need for the corresponding diagnostic code anymore.
Original commit message:
[heap] Remove debugging code of crbug/454297.
BUG=
Review URL: https://codereview.chromium.org/1420253002
Cr-Commit-Position: refs/heads/master@{#31523}
Fixes #7574.