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

repl: do not define wasi on global with no flag #45595

Merged
merged 1 commit into from
Nov 26, 2022

Conversation

cola119
Copy link
Member

@cola119 cola119 commented Nov 23, 2022

Fixes #45560

In the REPL, prepareMainThreadExecution that updates the canBeRequiredByUsers flag of wasi module according to the flag is called after the internal/process/esm_loader.js load that creates builtinModules depending on the module.canBeRequiredByUsers flag. In this case, wasi.canBeRequiredByUsers is always true so wasi module always can be required by users even if the flag is not set.

const builtinModules = [];
for (const { 0: id, 1: mod } of BuiltinModule.map) {
if (mod.canBeRequiredByUsers &&
BuiltinModule.canBeRequiredWithoutScheme(id)) {
ArrayPrototypePush(builtinModules, id);
}
}

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Nov 23, 2022
@cola119 cola119 added the repl Issues and PRs related to the REPL subsystem. label Nov 23, 2022
@cola119 cola119 changed the title repl: do not defined wasi on global with no flag repl: do not define wasi on global with no flag Nov 23, 2022
lib/internal/main/repl.js Show resolved Hide resolved
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. commit-queue Add this label to land a pull request using GitHub Actions. labels Nov 23, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 23, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 25, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/45595
✔  Done loading data for nodejs/node/pull/45595
----------------------------------- PR info ------------------------------------
Title      repl: do not define `wasi` on global with no flag (#45595)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     cola119:fix-45560 -> nodejs:main
Labels     repl, author ready, needs-ci
Commits    1
 - repl: do not define `wasi` on global with no flag
Committers 1
 - cola119 
PR-URL: https://github.com/nodejs/node/pull/45595
Reviewed-By: Ruben Bridgewater 
Reviewed-By: Colin Ihrig 
Reviewed-By: Antoine du Hamel 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/45595
Reviewed-By: Ruben Bridgewater 
Reviewed-By: Colin Ihrig 
Reviewed-By: Antoine du Hamel 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 23 Nov 2022 08:48:35 GMT
   ✔  Approvals: 4
   ✔  - Ruben Bridgewater (@BridgeAR) (TSC): https://github.com/nodejs/node/pull/45595#pullrequestreview-1191699283
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/45595#pullrequestreview-1191760663
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/45595#pullrequestreview-1192375682
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/45595#pullrequestreview-1192246419
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-11-23T22:44:37Z: https://ci.nodejs.org/job/node-test-pull-request/48141/
   ⚠  Commits were pushed after the last Full PR CI run:
   ⚠  - repl: do not define `wasi` on global with no flag
- Querying data for job/node-test-pull-request/48141/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3546553264

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Nov 25, 2022
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 25, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 25, 2022
@nodejs-github-bot
Copy link
Collaborator

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

@cola119 cola119 removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Nov 26, 2022
@cola119 cola119 added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 26, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 26, 2022
@nodejs-github-bot nodejs-github-bot merged commit 5a8895c into nodejs:main Nov 26, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 5a8895c

@cola119 cola119 deleted the fix-45560 branch November 26, 2022 11:16
ErickWendel pushed a commit to ErickWendel/node that referenced this pull request Nov 30, 2022
PR-URL: nodejs#45595
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Dec 12, 2022
PR-URL: #45595
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45595
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45595
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
PR-URL: #45595
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
PR-URL: #45595
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 5, 2023
PR-URL: #45595
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wasi incorrectly defined on global
6 participants