Skip to content

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Oct 28, 2023

  • 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.

Refs: #42566

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/single-executable
  • @nodejs/startup

@nodejs-github-bot 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
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Looks great so far!

@addaleax
Copy link
Member

addaleax commented Nov 16, 2023

@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 🙂

@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 17, 2023

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

@addaleax
Copy link
Member

@joyeecheung Yup, sorry :/

joyeecheung and others added 2 commits November 27, 2023 13:21
- 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.
@joyeecheung joyeecheung marked this pull request as ready for review November 27, 2023 12:28
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 27, 2023
@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 27, 2023

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

@joyeecheung
Copy link
Member Author

cc @nodejs/startup @nodejs/cpp-reviewers

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 27, 2023
@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

@addaleax Do you have time to take another look?

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Sorry, just got back from PTO – looks good!

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 7, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 7, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

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

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

@joyeecheung 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 added a commit that referenced this pull request Dec 15, 2023
- 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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
joyeecheung pushed a commit that referenced this pull request Dec 15, 2023
PR-URL: #50453
Refs: #42566
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
@joyeecheung
Copy link
Member Author

joyeecheung commented Dec 15, 2023

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

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

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.

RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
- 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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
PR-URL: #50453
Refs: #42566
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 2, 2024
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: TODO
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2024
RafaelGSS added a commit that referenced this pull request Jan 2, 2024
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 <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 2, 2024
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 <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 2, 2024
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 <[email protected]>
RafaelGSS added a commit to RafaelGSS/node that referenced this pull request Jan 3, 2024
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 <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 5, 2024
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 <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 11, 2024
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 <[email protected]>
RafaelGSS added a commit that referenced this pull request Jan 15, 2024
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 <[email protected]>
Medhansh404 pushed a commit to Medhansh404/node that referenced this pull request Jan 19, 2024
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 <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
- 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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
PR-URL: #50453
Refs: #42566
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
codebytere added a commit to electron/electron that referenced this pull request Apr 14, 2024
codebytere added a commit to electron/electron that referenced this pull request Apr 16, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Apr 17, 2024
* 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 <[email protected]>
Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
codebytere added a commit to electron/electron that referenced this pull request Jun 1, 2024
* 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 <[email protected]>
Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants