★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/27365Nouvelle recherche | Portail wanayoo
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

Jump to bottom

build: disable custom v8 snapshot by default #27365

Closed
wants to merge 1 commit into from
Closed

build: disable custom v8 snapshot by default #27365

wants to merge 1 commit into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Apr 23, 2019 •
edited

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

@joyeecheung
build: disable custom v8 snapshot by default
4c3c877 
This currently breaks `test/pummel/test-hash-seed.js`
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 23, 2019

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

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Apr 23, 2019

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/

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 23, 2019

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

refack
refack approved these changes Apr 23, 2019
View changes
configure.py Show resolved Hide resolved
@refack

This comment has been hidden.

Sign in to view
@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Apr 23, 2019

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 joyeecheung mentioned this pull request Apr 23, 2019
Closed
ryzokuken
ryzokuken approved these changes Apr 23, 2019
View changes
Copy link
Member

@ryzokuken ryzokuken left a comment

LGTM.

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Apr 23, 2019

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

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Apr 23, 2019

Landed in a41a4ee

joyeecheung added a commit that referenced this issue Apr 23, 2019
@joyeecheung
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 added a commit that referenced this issue Apr 27, 2019
@joyeecheung @targos
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 targos mentioned this pull request Apr 27, 2019
Merged
joyeecheung added a commit to joyeecheung/v8 that referenced this issue May 9, 2019
Commit Bot
[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 issue Jun 6, 2019
@joyeecheung
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@{#61175}

Refs: v8/v8@e0a109c
joyeecheung added a commit that referenced this issue Jun 11, 2019
@joyeecheung
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 joyeecheung mentioned this pull request Jun 11, 2019
Closed
2 tasks
joyeecheung added a commit that referenced this issue Jun 17, 2019
@joyeecheung
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 issue Jun 17, 2019
@joyeecheung
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 added a commit that referenced this issue Jun 17, 2019
@joyeecheung @BridgeAR
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 added a commit that referenced this issue Jun 17, 2019
@joyeecheung @BridgeAR
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 added a commit that referenced this issue Jun 17, 2019
@joyeecheung @BridgeAR
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 added a commit that referenced this issue Jun 18, 2019
@joyeecheung @targos
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 added a commit that referenced this issue Jun 18, 2019
@joyeecheung @targos
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack

@devsnek devsnek

@ryzokuken ryzokuken

Assignees
No one assigned
Labels
build fast-track
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
5 participants
@joyeecheung @nodejs-github-bot @refack @devsnek @ryzokuken