Sitelet https://github.com/nodejs/node/pull/47125
Skip to content

sea: use JSON configuration and blob content for SEA - #47125

Merged
nodejs-github-bot merged 5 commits into
nodejs:mainfrom
joyeecheung:sea-blob
Apr 9, 2023
Merged

sea: use JSON configuration and blob content for SEA#47125
nodejs-github-bot merged 5 commits into
nodejs:mainfrom
joyeecheung:sea-blob

Conversation

@joyeecheung

@joyeecheung joyeecheung commented Mar 16, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

This implements the ideas discussed in nodejs/single-executable#58. This changes the SEA contract in that:

  1. The data injected is now a blob whose layout is controlled by us (generated with --experimental-sea-config), this makes it possible to include other resources into the SEA other than just the JS code
  2. The configuration is now JSON-based (accepted by --experimental-sea-config) which allows for more flexibility.

Refs: nodejs/single-executable#58

@nodejs-github-bot

nodejs-github-bot commented Mar 16, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

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

@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 involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Mar 16, 2023
anonrig
anonrig reviewed Mar 16, 2023
View reviewed changes
Comment thread src/node_sea.cc Outdated
RaisinTen
RaisinTen reviewed Mar 17, 2023
View reviewed changes
Comment thread src/node_sea.cc Outdated
addaleax
addaleax reviewed Mar 17, 2023
View reviewed changes
Comment thread src/node_sea.cc Outdated
@joyeecheung
joyeecheung force-pushed the sea-blob branch 2 times, most recently from 0f67d94 to c43ae7c Compare March 28, 2023 19:58
@joyeecheung

joyeecheung commented Mar 28, 2023

Copy link
Copy Markdown
Member Author

Updated the documentation a bit, still needs to write a bunch of tests...

@joyeecheung
joyeecheung force-pushed the sea-blob branch 4 times, most recently from 8654fbd to 807b866 Compare April 4, 2023 17:09
@joyeecheung joyeecheung changed the title POC: use JSON configuration and blob content for SEA src: use JSON configuration and blob content for SEA Apr 4, 2023
@joyeecheung joyeecheung changed the title src: use JSON configuration and blob content for SEA sea: use JSON configuration and blob content for SEA Apr 4, 2023
@joyeecheung
joyeecheung marked this pull request as ready for review April 4, 2023 17:12
@nodejs-github-bot

nodejs-github-bot commented Apr 4, 2023

Copy link
Copy Markdown
Collaborator

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

@joyeecheung

joyeecheung commented Apr 4, 2023

Copy link
Copy Markdown
Member Author

Added tests and documentation. @nodejs/single-executable PTAL thanks!

@github-actions github-actions Bot mentioned this pull request Apr 5, 2023
Open
26 tasks
tony-go
tony-go reviewed Apr 5, 2023
View reviewed changes

ghost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👏🏼 Awesome

Comment thread doc/api/single-executable-applications.md
Comment thread src/node_sea.cc Outdated
Comment thread test/parallel/test-single-executable-blob-config-errors.js
Comment thread test/parallel/test-single-executable-blob-config-errors.js
Comment thread test/parallel/test-single-executable-blob-config.js
@nodejs-github-bot

ghost commented Apr 5, 2023

Copy link
Copy Markdown
Collaborator

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

RaisinTen
RaisinTen reviewed Apr 5, 2023
View reviewed changes
Comment thread doc/api/cli.md
Comment thread doc/api/cli.md Outdated
Comment thread doc/api/single-executable-applications.md Outdated
Comment thread doc/api/single-executable-applications.md Outdated
Comment thread doc/api/single-executable-applications.md Outdated
Comment thread src/node_sea.cc
Comment thread src/node_sea.cc
Comment thread src/node_sea.cc Outdated
Comment thread src/node_sea.cc
RaisinTen
RaisinTen reviewed Apr 6, 2023
View reviewed changes
Comment thread test/parallel/test-single-executable-blob-config-errors.js Outdated
Comment thread test/parallel/test-single-executable-blob-config-errors.js Outdated
Comment thread test/parallel/test-single-executable-blob-config-errors.js Outdated
Comment thread src/node_sea.cc
@joyeecheung joyeecheung added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Apr 9, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Apr 9, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 491a5c9 into nodejs:main Apr 9, 2023
@nodejs-github-bot

ghost commented Apr 9, 2023

Copy link
Copy Markdown
Collaborator

Landed in 491a5c9

@RafaelGSS RafaelGSS mentioned this pull request Apr 11, 2023
Merged
@github-actions github-actions Bot mentioned this pull request Apr 12, 2023
Open
31 tasks
@RaisinTen

ghost commented Apr 12, 2023

Copy link
Copy Markdown
Member

@joyeecheung should this be labelled as a notable-change PRs with changes that should be highlighted in changelogs. because this changes how this feature is supposed to be used?

@joyeecheung joyeecheung added the notable-change PRs with changes that should be highlighted in changelogs. label Apr 12, 2023
@github-actions

ghost commented Apr 12, 2023

Copy link
Copy Markdown
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.

@joyeecheung

ghost commented Apr 12, 2023

Copy link
Copy Markdown
Member Author

Good idea. Done.

@RaisinTen

ghost commented Apr 13, 2023

Copy link
Copy Markdown
Member

FYI @joyeecheung:

Originally posted by @RafaelGSS in #47381 (comment)

I'll include both in the "Other notable changes" section. In case you feel it should be placed in the same section as V8/Permission Model changes, update the PR (#47125 and #47233) description according to the bot request.

Should we add some release notes for this change?

@RaisinTen RaisinTen added the single-executable Issues and PRs related to single-executable applications. label May 5, 2023
This was referenced Jul 3, 2023
Merged
Closed
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax left review comments
@anonrig anonrig anonrig left review comments
@RaisinTen RaisinTen RaisinTen approved these changes
+1 more reviewer
@tony-go tony-go tony-go approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash PRs the Commit Queue should land as one squashed commit. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. single-executable Issues and PRs related to single-executable applications.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@joyeecheung @nodejs-github-bot @RaisinTen @addaleax @anonrig @tony-go @danielleadams