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

[v20.x] module: bootstrap module loaders in shadow realm #51239

Closed

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Dec 20, 2023

/cc @UlisesGascon

Setting the PR target as v20.x-staging-backport because v20.x-staging has partial commits from #48655.

legendecas and others added 6 commits December 20, 2023 22:27
Avoid the realm being strongly referenced by the process binding data.

PR-URL: nodejs#48655
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs#48655
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs#48655
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs#48655
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
The flag is always true and can be determined by isLoaderWorker solely.

PR-URL: nodejs#48655
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
This bootstraps ESM loaders in the ShadowRealm with
`ShadowRealm.prototype.importValue` as its entry point and enables
loading ESM and CJS modules in the ShadowRealm. The module is imported
without a parent URL and resolved with the current process's working
directory.

PR-URL: nodejs#48655
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/realm
  • @nodejs/startup
  • @nodejs/vm

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. labels Dec 20, 2023
@UlisesGascon UlisesGascon changed the base branch from v20.x-staging-backport to v20.x-staging December 20, 2023 15:15
@UlisesGascon
Copy link
Member

I updated the base to nodejs:v20.x-staging as I updated the brach matching nodejs:v20.x-staging-backport

Copy link
Member

@UlisesGascon UlisesGascon left a comment

Choose a reason for hiding this comment

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

Thanks @legendecas for the backport! LGTM

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 20, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 20, 2023
@nodejs-github-bot

This comment was marked as outdated.

@UlisesGascon UlisesGascon mentioned this pull request Dec 20, 2023
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@UlisesGascon
Copy link
Member

Not sure why the CI is hanging currently (maybe due osx resources) but between this and the previous executions the CI can be considered green, I was tracking the differences and the test errors. I will land this in the proposal, there I will run the CI again.

UlisesGascon pushed a commit that referenced this pull request Dec 20, 2023
Avoid the realm being strongly referenced by the process binding data.

PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 20, 2023
PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 20, 2023
PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 20, 2023
PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 20, 2023
The flag is always true and can be determined by isLoaderWorker solely.

PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 20, 2023
This bootstraps ESM loaders in the ShadowRealm with
`ShadowRealm.prototype.importValue` as its entry point and enables
loading ESM and CJS modules in the ShadowRealm. The module is imported
without a parent URL and resolved with the current process's working
directory.

PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@UlisesGascon
Copy link
Member

Landed in 7af8e20...b3041a0

@legendecas legendecas deleted the backport-48655-to-20 branch December 21, 2023 13:23
UlisesGascon pushed a commit that referenced this pull request Jan 9, 2024
Avoid the realm being strongly referenced by the process binding data.

PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Jan 9, 2024
PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Jan 9, 2024
PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Jan 9, 2024
PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Jan 9, 2024
The flag is always true and can be determined by isLoaderWorker solely.

PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Jan 9, 2024
This bootstraps ESM loaders in the ShadowRealm with
`ShadowRealm.prototype.importValue` as its entry point and enables
loading ESM and CJS modules in the ShadowRealm. The module is imported
without a parent URL and resolved with the current process's working
directory.

PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
codebytere added a commit to electron/electron that referenced this pull request Jan 16, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 16, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 18, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Jan 18, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 18, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Jan 18, 2024
* chore: bump node in DEPS to v20.11.0

* module: bootstrap module loaders in shadow realm

nodejs/node#48655

* src: add commit hash shorthand in zlib version

nodejs/node#50158

* v8,tools: expose necessary V8 defines

nodejs/node#50820

* esm: do not call getSource when format is commonjs

nodejs/node#50465

* esm: fallback to readFileSync when source is nullish

nodejs/node#50825

* vm: allow dynamic import with a referrer realm

nodejs/node#50360

* test: skip test-diagnostics-channel-memory-leak.js

nodejs/node#50327

* esm: do not call getSource when format is commonjs

nodejs/node#50465

* lib: fix assert throwing different error messages in ESM and CJS

nodejs/node#50634

* src: fix compatility with upcoming V8 12.1 APIs

nodejs/node#50709

* deps: update base64 to 0.5.1

nodejs/node#50629

* src: avoid silent coercion to signed/unsigned int

nodejs/node#50663

* src: fix compatility with upcoming V8 12.1 APIs

nodejs/node#50709

* chore: fix patch indices

* chore: update patches

* test: disable TLS cipher test

This can't be enabled owing to BoringSSL incompatibilities.

nodejs/node#50186

* fix: check for Buffer and global definition in shadow realm

nodejs/node#51239

* test: disable parallel/test-shadow-realm-custom-loader

Incompatible with our asar logic, resulting in the following failure:

> Failed to CompileAndCall electron script: electron/js2c/asar_bundle

* chore: remove deleted parallel/test-crypto-modp1-error test

* test: make test-node-output-v8-warning generic

nodejs/node#50421

* chore: fixup ModuleWrap patch

* test: match wpt/streams/transferable/transform-stream-members.any.js to upstream

* fix: sandbox is not enabled on arm

* chore: disable v8 sandbox on ia32/arm

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
Co-authored-by: Cheng Zhao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants