FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

build: disable custom v8 snapshot by default by joyeecheung · Pull Request #27365 · nodejs/node · GitHub

/ node Public

build: disable custom v8 snapshot by default - #27365

Closed
joyeecheung wants to merge 1 commit into
nodejs:masterfrom
joyeecheung:configure-snapshot
ConversationCommits (1)ChecksFiles changed
Closed

build: disable custom v8 snapshot by default#27365
joyeecheung wants to merge 1 commit into
nodejs:masterfrom
joyeecheung:configure-snapshot

Conversation

joyeecheung commented Apr 23, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

This currently breaks test/pummel/test-hash-seed.js

Refs: #27321

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

build: disable custom v8 snapshot by default
4c3c877 
This currently breaks `test/pummel/test-hash-seed.js`

nodejs-github-bot commented Apr 23, 2019

Copy link
Copy Markdown
Collaborator

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3339

nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Apr 23, 2019

joyeecheung commented Apr 23, 2019

Copy link
Copy Markdown
Member Author

pummel/test-hash-seed on master https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/6416/
pummel/test-hash-seed for this PR https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/6417/

joyeecheung added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 23, 2019

nodejs-github-bot commented Apr 23, 2019

Copy link
Copy Markdown
Collaborator

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

refack approved these changes Apr 23, 2019
View reviewed changes
Comment thread configure.py

This comment has been minimized.

Sign in to view

joyeecheung commented Apr 23, 2019

Copy link
Copy Markdown
Member Author

Please thumb this up if you are +1 to fast track. This breaks pummel/test-hash-seed on master (which breaks the daily build).

joyeecheung mentioned this pull request Apr 23, 2019
Closed
ryzokuken approved these changes Apr 23, 2019
View reviewed changes

ryzokuken left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

joyeecheung commented Apr 23, 2019

Copy link
Copy Markdown
Member Author

This fixed the regression on master as shown in #27365 (comment) . Landing..

joyeecheung commented Apr 23, 2019

Copy link
Copy Markdown
Member Author

Landed in a41a4ee

joyeecheung added a commit that referenced this pull request Apr 23, 2019
build: disable custom v8 snapshot by default
a41a4ee 
This currently breaks `test/pummel/test-hash-seed.js`

PR-URL: #27365
Refs: #27321
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
targos pushed a commit that referenced this pull request Apr 27, 2019
build: disable custom v8 snapshot by default
5023446 
This currently breaks `test/pummel/test-hash-seed.js`

PR-URL: #27365
Refs: #27321
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
targos mentioned this pull request Apr 27, 2019
Merged
joyeecheung added a commit to joyeecheung/v8 that referenced this pull request May 9, 2019
[api] Implement StartupData::CanBeRehashed() for the snapshot blob
e0a109c 
This enables the embedder to check if the snapshot generated
from SnapshotCreator::CreateBlob() can be rehashed and the seed
can be recomputed during deserialization.

The lack of this functionality resulted in a temporary vunerability
in Node.js: nodejs/node#27365

Change-Id: I88d52337217c40f79c26438be3c87d2db874d980
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578661
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61175}
joyeecheung added a commit to joyeecheung/node that referenced this pull request Jun 6, 2019
deps: V8: cherry-pick e0a109c
2702abb 
Original commit message:

    [api] Implement StartupData::CanBeRehashed() for the snapshot blob

    This enables the embedder to check if the snapshot generated
    from SnapshotCreator::CreateBlob() can be rehashed and the seed
    can be recomputed during deserialization.

    The lack of this functionality resulted in a temporary vunerability
    in Node.js: nodejs#27365

    Change-Id: I88d52337217c40f79c26438be3c87d2db874d980
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578661
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#61175}

Refs: v8/v8@e0a109c
joyeecheung added a commit that referenced this pull request Jun 11, 2019
deps: V8: cherry-pick e0a109c
d2634be 
Original commit message:

    [api] Implement StartupData::CanBeRehashed() for the snapshot blob

    This enables the embedder to check if the snapshot generated
    from SnapshotCreator::CreateBlob() can be rehashed and the seed
    can be recomputed during deserialization.

    The lack of this functionality resulted in a temporary vunerability
    in Node.js: #27365

    Change-Id: I88d52337217c40f79c26438be3c87d2db874d980
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578661
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#61175}

Refs: v8/v8@e0a109c

PR-URL: #27533
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
joyeecheung mentioned this pull request Jun 11, 2019
Closed
2 tasks
joyeecheung added a commit that referenced this pull request Jun 17, 2019
tools: assert that the snapshot can be rehashed in node_mksnapshot
b6e41f0 
PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
joyeecheung added a commit that referenced this pull request Jun 17, 2019
build: turn on custom V8 snapshot by default
28d3f19 
This patch re-enables custom V8 snapshot integration. Also renames
the experimental configure switch from `--with-node-snapshot`
to `--without-node-snapshot` to be consistent with`--without-snapshot`
which is used for the default V8 snapshot.

PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
deps: V8: cherry-pick e0a109c
9382b3b 
Original commit message:

    [api] Implement StartupData::CanBeRehashed() for the snapshot blob

    This enables the embedder to check if the snapshot generated
    from SnapshotCreator::CreateBlob() can be rehashed and the seed
    can be recomputed during deserialization.

    The lack of this functionality resulted in a temporary vunerability
    in Node.js: #27365

    Change-Id: I88d52337217c40f79c26438be3c87d2db874d980
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578661
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#61175}

Refs: v8/v8@e0a109c

PR-URL: #27533
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
tools: assert that the snapshot can be rehashed in node_mksnapshot
9085412 
PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
build: turn on custom V8 snapshot by default
2201484 
This patch re-enables custom V8 snapshot integration. Also renames
the experimental configure switch from `--with-node-snapshot`
to `--without-node-snapshot` to be consistent with`--without-snapshot`
which is used for the default V8 snapshot.

PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Jun 18, 2019
tools: assert that the snapshot can be rehashed in node_mksnapshot
aeda0c3 
PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Jun 18, 2019
build: turn on custom V8 snapshot by default
32b0803 
This patch re-enables custom V8 snapshot integration. Also renames
the experimental configure switch from `--with-node-snapshot`
to `--without-node-snapshot` to be consistent with`--without-snapshot`
which is used for the default V8 snapshot.

PR-URL: #28181
Refs: #27365
Refs: #17058
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
refack refack approved these changes
devsnek devsnek approved these changes
ryzokuken ryzokuken approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. fast-track PRs that do not need to wait for 48 hours to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL