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

wasi: require CLI flag to require() wasi module #30963

Closed
wants to merge 2 commits into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Dec 14, 2019

This commit ensures that the WASI module cannot be require()'ed without a CLI flag while the module is still experimental.

This fixes a regression from #30778.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

cjihrig added a commit to cjihrig/node that referenced this pull request Dec 15, 2019
test-wasi-start-validation.js should require the
--experimental-wasi-unstable-preview0 flag in order to run.
However, due to a recent regression, that hasn't been enforced.
nodejs#30963 fixes the regression
and will cause this test to start (correctly) failing. This
commit adds the missing flag.
@cjihrig cjihrig mentioned this pull request Dec 15, 2019
3 tasks
@richardlau
Copy link
Member

Test failure looks relevant:
https://travis-ci.com/nodejs/node/jobs/267277987#L273-L296

=== release test-code-cache ===
Path: parallel/test-code-cache
--- stderr ---
internal/modules/cjs/loader.js:1026
    throw err;
    ^

Error: Cannot find module 'wasi'
Require stack:
- /home/travis/build/nodejs/node/test/parallel/test-code-cache.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:1023:17)
    at Function.Module._load (internal/modules/cjs/loader.js:910:27)
    at Module.require (internal/modules/cjs/loader.js:1085:19)
    at require (internal/modules/cjs/helpers.js:72:18)
    at Object.<anonymous> (/home/travis/build/nodejs/node/test/parallel/test-code-cache.js:19:3)
    at Module._compile (internal/modules/cjs/loader.js:1196:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1216:10)
    at Module.load (internal/modules/cjs/loader.js:1045:32)
    at Function.Module._load (internal/modules/cjs/loader.js:949:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/home/travis/build/nodejs/node/test/parallel/test-code-cache.js' ]
}
Command: out/Release/node --expose-internals /home/travis/build/nodejs/node/test/parallel/test-code-cache.js

Trott pushed a commit that referenced this pull request Dec 15, 2019
test-wasi-start-validation.js should require the
--experimental-wasi-unstable-preview0 flag in order to run.
However, due to a recent regression, that hasn't been enforced.
#30963 fixes the regression
and will cause this test to start (correctly) failing. This
commit adds the missing flag.

PR-URL: #30971
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
This commit ensures that the WASI module cannot be require()'ed
without a CLI flag while the module is still experimental.

This fixes a regression from
nodejs#30778.
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 15, 2019

@cjihrig cjihrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 15, 2019
@danbev
Copy link
Contributor

danbev commented Dec 17, 2019

Landed in efb9084.

@danbev danbev closed this Dec 17, 2019
danbev pushed a commit that referenced this pull request Dec 17, 2019
This commit ensures that the WASI module cannot be require()'ed
without a CLI flag while the module is still experimental.

This fixes a regression from
#30778.

PR-URL: #30963
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@cjihrig cjihrig deleted the require-wasi branch December 17, 2019 14:44
@ljharb
Copy link
Member

ljharb commented Dec 17, 2019

v13.4 seems to have been released without this fix.

@ljharb
Copy link
Member

ljharb commented Dec 17, 2019

For future reference, if CIGTM includes resolve's tests, then this sort of thing can be caught prior to a release :-)

MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
test-wasi-start-validation.js should require the
--experimental-wasi-unstable-preview0 flag in order to run.
However, due to a recent regression, that hasn't been enforced.
#30963 fixes the regression
and will cause this test to start (correctly) failing. This
commit adds the missing flag.

PR-URL: #30971
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
This commit ensures that the WASI module cannot be require()'ed
without a CLI flag while the module is still experimental.

This fixes a regression from
#30778.

PR-URL: #30963
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 17, 2019
@ljharb ljharb mentioned this pull request Dec 17, 2019
15 tasks
MylesBorins added a commit that referenced this pull request Dec 18, 2019
Notable Changes:

* cli:
  * add --trace-exit cli option (legendecas)
    #30516
* http,https:
  * increase server headers timeout (Tim Costa)
    #30071
* readline:
  * update ansi-regex (Ruben Bridgewater)
    #30907
  * promote \_getCursorPos to public api (Jeremy Albright)
    #30687
* repl:
  * add completion preview (Ruben Bridgewater)
    #30907
* util:
  * add Set and map size to inspect output (Ruben Bridgewater)
    #30225
* wasi:
  * require CLI flag to require() wasi module (Colin Ihrig)
    #30963

PR-URL: #31010
MylesBorins added a commit that referenced this pull request Dec 18, 2019
Notable Changes:

* cli:
  * add --trace-exit cli option (legendecas)
    #30516
* http,https:
  * increase server headers timeout (Tim Costa)
    #30071
* readline:
  * update ansi-regex (Ruben Bridgewater)
    #30907
  * promote \_getCursorPos to public api (Jeremy Albright)
    #30687
* repl:
  * add completion preview (Ruben Bridgewater)
    #30907
* util:
  * add Set and map size to inspect output (Ruben Bridgewater)
    #30225
* wasi:
  * require CLI flag to require() wasi module (Colin Ihrig)
    #30963

PR-URL: #31010
richardlau pushed a commit to nodejs/citgm that referenced this pull request Jan 7, 2020
Running `resolve`'s tests will help avoid problems like nodejs/node#30963 from occurring silently.
targos pushed a commit that referenced this pull request Jan 14, 2020
test-wasi-start-validation.js should require the
--experimental-wasi-unstable-preview0 flag in order to run.
However, due to a recent regression, that hasn't been enforced.
#30963 fixes the regression
and will cause this test to start (correctly) failing. This
commit adds the missing flag.

PR-URL: #30971
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this pull request Jan 14, 2020
This commit ensures that the WASI module cannot be require()'ed
without a CLI flag while the module is still experimental.

This fixes a regression from
#30778.

PR-URL: #30963
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: David Carlier <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
test-wasi-start-validation.js should require the
--experimental-wasi-unstable-preview0 flag in order to run.
However, due to a recent regression, that hasn't been enforced.
#30963 fixes the regression
and will cause this test to start (correctly) failing. This
commit adds the missing flag.

PR-URL: #30971
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This commit ensures that the WASI module cannot be require()'ed
without a CLI flag while the module is still experimental.

This fixes a regression from
#30778.

PR-URL: #30963
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants