src: support configurable snapshot by joyeecheung · Pull Request #50453 · nodejs/node · GitHub

Add support for --build-snapshot-config which allows passing snapshot configurations via a JSON configuration file.
Add support for node::SnapshotConfig in the embedder API

The initial configurable options are:

"builder" (SnapshotConfig::builder_script_path): path to the builder script.
"withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable code cache generation.

Review requested:

@nodejs/single-executable
@nodejs/startup

Looks great so far!

@joyeecheung Since the embedder API has documentation here already, we only need to document --build-snapshot-config as well, right? In that case, addaleax/node@ba0b08d is a short addition to the docs that describes it 🙂

@addaleax Thanks, although it seems that commit link is not reachable..paste error?

@joyeecheung Yup, sorry :/

- Add support for --build-snapshot-config which allows passing snapshot configurations via a JSON configuration file. - Add support for node::SnapshotConfig in the embedder API The initial configurable options are: - "builder" (SnapshotConfig::builder_script_path): path to the builder script. - "withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable code cache generation.

Thanks. Applied documentation and I think this is now ready for review

cc @nodejs/startup @nodejs/cpp-reviewers

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

@addaleax Do you have time to take another look?

Sorry, just got back from PTO – looks good!

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

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

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

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

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

Commit Queue failed

- Loading data for nodejs/node/pull/50453
✔  Done loading data for nodejs/node/pull/50453
----------------------------------- PR info ------------------------------------
Title      src: support configurable snapshot (#50453)
Author     Joyee Cheung  (@joyeecheung)
Branch     joyeecheung:snapshot-config -> nodejs:main
Labels     c++, lib / src, needs-ci, commit-queue-rebase
Commits    2
 - src: support configurable snapshot
 - doc: add documentation for --build-snapshot-config
Committers 1
 - Joyee Cheung 
PR-URL: https://github.com/nodejs/node/pull/50453
Refs: https://github.com/nodejs/node/issues/42566
Reviewed-By: Anna Henningsen 
Reviewed-By: James M Snell 
Reviewed-By: Stephen Belanger 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50453
Refs: https://github.com/nodejs/node/issues/42566
Reviewed-By: Anna Henningsen 
Reviewed-By: James M Snell 
Reviewed-By: Stephen Belanger 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 28 Oct 2023 20:10:36 GMT
   ✔  Approvals: 3
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/50453#pullrequestreview-1770444150
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/50453#pullrequestreview-1773719524
   ✔  - Stephen Belanger (@Qard): https://github.com/nodejs/node/pull/50453#pullrequestreview-1774806892
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2023-12-14T23:35:32Z: https://ci.nodejs.org/job/node-test-pull-request/56296/
- Querying data for job/node-test-pull-request/56296/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/7223088600

- Add support for --build-snapshot-config which allows passing snapshot configurations via a JSON configuration file. - Add support for node::SnapshotConfig in the embedder API The initial configurable options are: - "builder" (SnapshotConfig::builder_script_path): path to the builder script. - "withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable code cache generation. PR-URL: #50453 Refs: #42566 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

PR-URL: #50453 Refs: #42566 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

CI was green though git node was not able to understand the GitHub CI results again..Landed in 215f4d0...62ca050

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @joyeecheung.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

- Add support for --build-snapshot-config which allows passing snapshot configurations via a JSON configuration file. - Add support for node::SnapshotConfig in the embedder API The initial configurable options are: - "builder" (SnapshotConfig::builder_script_path): path to the builder script. - "withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable code cache generation. PR-URL: #50453 Refs: #42566 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

PR-URL: #50453 Refs: #42566 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

Notable changes: doc: * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) #50453 lib,src,permission: * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) #50758 net: * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) #51045 src: * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) #50453 src,permission: * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) #51183 timers: * (SEMVER-MINOR) export timers.promises (Marco Ippolito) #51246 PR-URL: #51342 Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>

Notable changes: doc: * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) nodejs#50453 lib,src,permission: * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) nodejs#50758 net: * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) nodejs#51045 src: * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) nodejs#50453 src,permission: * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) nodejs#51183 timers: * (SEMVER-MINOR) export timers.promises (Marco Ippolito) nodejs#51246 PR-URL: nodejs#51342 Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>

Notable changes: doc: * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) #50453 lib,src,permission: * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) #50758 net: * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) #51045 src: * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) #50453 src,permission: * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) #51183 timers: * (SEMVER-MINOR) export timers.promises (Marco Ippolito) #51246 PR-URL: #51342 Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>

Notable changes: doc: * (SEMVER-MINOR) add documentation for --build-snapshot-config (Anna Henningsen) nodejs#50453 lib,src,permission: * (SEMVER-MINOR) port path.resolve to C++ (Rafael Gonzaga) nodejs#50758 net: * (SEMVER-MINOR) add connection attempt events (Paolo Insogna) nodejs#51045 src: * (SEMVER-MINOR) support configurable snapshot (Joyee Cheung) nodejs#50453 src,permission: * (SEMVER-MINOR) add --allow-addon flag (Rafael Gonzaga) nodejs#51183 timers: * (SEMVER-MINOR) export timers.promises (Marco Ippolito) nodejs#51246 PR-URL: nodejs#51342 Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>

- Add support for --build-snapshot-config which allows passing snapshot configurations via a JSON configuration file. - Add support for node::SnapshotConfig in the embedder API The initial configurable options are: - "builder" (SnapshotConfig::builder_script_path): path to the builder script. - "withoutCodeCache" (SnapshotFlags::kWithoutCodeCache): disable code cache generation. PR-URL: #50453 Refs: #42566 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

PR-URL: #50453 Refs: #42566 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

nodejs/node#50453

* chore: bump node in DEPS to v20.12.0 * chore: update build_add_gn_build_files.patch * chore: update patches * chore: bump node in DEPS to v20.12.1 * chore: update patches * build: encode non-ASCII Latin1 characters as one byte in JS2C nodejs/node#51605 * crypto: use EVP_MD_fetch and cache EVP_MD for hashes nodejs/node#51034 * chore: update filenames.json * chore: bump node in DEPS to v20.12.2 * chore: update patches * src: support configurable snapshot nodejs/node#50453 * test: remove test-domain-error-types flaky designation nodejs/node#51717 * src: avoid draining platform tasks at FreeEnvironment nodejs/node#51290 * chore: fix accidentally deleted v8 dep * lib: define FormData and fetch etc. in the built-in snapshot nodejs/node#51598 * chore: rebase on main * chore: remove stray log --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Cheng <zcbenz@gmail.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

nodejs/node#50453

* chore: bump node in DEPS to v20.13.1 * chore: bump node in DEPS to v20.14.0 * chore: update build_add_gn_build_files.patch * chore: update patches * chore: update patches * build: encode non-ASCII Latin1 characters as one byte in JS2C nodejs/node#51605 * crypto: use EVP_MD_fetch and cache EVP_MD for hashes nodejs/node#51034 * chore: update filenames.json * chore: update patches * src: support configurable snapshot nodejs/node#50453 * test: remove test-domain-error-types flaky designation nodejs/node#51717 * src: avoid draining platform tasks at FreeEnvironment nodejs/node#51290 * chore: fix accidentally deleted v8 dep * lib: define FormData and fetch etc. in the built-in snapshot nodejs/node#51598 * chore: remove stray log * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: fixup * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: fixup indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Cheng <zcbenz@gmail.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Oct 28, 2023

joyeecheung force-pushed the snapshot-config branch from 0b30ea3 to c146c18 Compare October 28, 2023 20:13

addaleax reviewed Nov 7, 2023

View reviewed changes

Comment thread src/node.h Outdated

Comment thread src/node.h Outdated

Comment thread src/node.h

Comment thread test/embedding/embedtest.cc Outdated

joyeecheung and others added 2 commits November 27, 2023 13:21

doc: add documentation for --build-snapshot-config

0b6ab4c

joyeecheung force-pushed the snapshot-config branch from feed446 to 0b6ab4c Compare November 27, 2023 12:28

joyeecheung marked this pull request as ready for review November 27, 2023 12:28

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 27, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 27, 2023

addaleax approved these changes Dec 7, 2023

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 7, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 7, 2023

jasnell approved these changes Dec 9, 2023

View reviewed changes

aymen94 approved these changes Dec 11, 2023

View reviewed changes

Qard approved these changes Dec 11, 2023

View reviewed changes

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Dec 15, 2023

joyeecheung added semver-minor PRs that contain new features and should be released in the next minor version. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 15, 2023

joyeecheung closed this Dec 15, 2023

joyeecheung added the notable-change PRs with changes that should be highlighted in changelogs. label Dec 15, 2023

RafaelGSS mentioned this pull request Jan 2, 2024

v21.6.0 proposal #51342

Merged

richardlau mentioned this pull request Mar 25, 2024

v20.12.0 proposal #52212

Merged

codebytere added a commit to electron/electron that referenced this pull request Apr 14, 2024

src: support configurable snapshot

6e1da42

nodejs/node#50453

codebytere added a commit to electron/electron that referenced this pull request Apr 16, 2024

src: support configurable snapshot

dfb1cf4

nodejs/node#50453

codebytere added a commit to electron/electron that referenced this pull request May 31, 2024

src: support configurable snapshot

e95e702

nodejs/node#50453

src: support configurable snapshot by joyeecheung · Pull Request #50453 · nodejs/node · GitHub

Uh oh!

Uh oh!

src: support configurable snapshot - #50453

src: support configurable snapshot#50453joyeecheung wants to merge 2 commits intonodejs:mainnodejs/node:mainfrom joyeecheung:snapshot-configjoyeecheung/node:snapshot-configCopy head branch name to clipboard

Conversation

joyeecheung commented Oct 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 28, 2023

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

addaleax commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented Nov 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

addaleax commented Nov 21, 2023

Uh oh!

joyeecheung commented Nov 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented Nov 27, 2023

Uh oh!

nodejs-github-bot commented Nov 27, 2023

Uh oh!

joyeecheung commented Nov 29, 2023

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Dec 7, 2023

Uh oh!

nodejs-github-bot commented Dec 10, 2023

Uh oh!

nodejs-github-bot commented Dec 11, 2023

Uh oh!

nodejs-github-bot commented Dec 12, 2023

Uh oh!

nodejs-github-bot commented Dec 14, 2023

Uh oh!

nodejs-github-bot commented Dec 15, 2023

Uh oh!

joyeecheung commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Dec 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

src: support configurable snapshot#50453
joyeecheung wants to merge 2 commits into
nodejs:mainfrom
joyeecheung:snapshot-config

joyeecheung commented Oct 28, 2023 •
edited
Loading

addaleax commented Nov 16, 2023 •
edited
Loading

joyeecheung commented Nov 17, 2023 •
edited
Loading

joyeecheung commented Nov 27, 2023 •
edited
Loading

joyeecheung commented Dec 15, 2023 •
edited
Loading