FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

build,test: guard eslint with crypto check by danbev · Pull Request #26182 · nodejs/node · GitHub

/ node Public

build,test: guard eslint with crypto check - #26182

Closed
danbev wants to merge 1 commit into
nodejs:masterfrom
danbev:eslint-crypto-checks
Closed

build,test: guard eslint with crypto check#26182
danbev wants to merge 1 commit into
nodejs:masterfrom
danbev:eslint-crypto-checks

Conversation

danbev commented Feb 18, 2019

Copy link
Copy Markdown
Contributor

Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:

$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:
$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/
cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.

Copy link
Copy Markdown
Collaborator

nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Feb 18, 2019

danbev commented Feb 18, 2019

Copy link
Copy Markdown
Contributor Author

Copy link
Copy Markdown
Member

Do you know if the test failures have always been like that without intl or is this a recent(ish) regression?

danbev commented Feb 18, 2019

Copy link
Copy Markdown
Contributor Author

Do you know if the test failures have always been like that without intl or is this a recent(ish) regression?

I'm not sure about failures --without-intl but these ones are fairly recent, like in the last week I'd say. Perhaps after the ESLint upgrade (77b39c2).

cjihrig commented Feb 18, 2019

Copy link
Copy Markdown
Contributor

It looks like tools/node_modules/eslint/node_modules/file-entry-cache/cache.js indeed just began require()ing crypto (https://github.com/nodejs/node/blame/fd0a861cdb3088f60ee56a8adef05fd50b71f817/tools/node_modules/eslint/node_modules/file-entry-cache/cache.js#L2).

That line seems to have landed upstream two years ago and just made it's way into eslint (https://github.com/royriojas/file-entry-cache/blame/03f700c99b76133dc14648b465a1550ec2930e5c/cache.js#L2).

Trott added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 18, 2019

danbev commented Feb 21, 2019

Copy link
Copy Markdown
Contributor Author

Landed in f17f467.

danbev closed this Feb 21, 2019
danbev deleted the eslint-crypto-checks branch February 21, 2019 04:42
danbev added a commit that referenced this pull request Feb 21, 2019
Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:
$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/
cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.

PR-URL: #26182
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit that referenced this pull request Feb 21, 2019
Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:
$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/
cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.

PR-URL: #26182
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR mentioned this pull request Feb 26, 2019
rvagg pushed a commit that referenced this pull request Feb 28, 2019
Currently, configuring --without-ssl will cause the lint-js target to
fail with the following error:
$ make lint-js
Running JS linter...
internal/util.js:101
    throw new ERR_NO_CRYPTO();
    ^

Error [ERR_NO_CRYPTO]:
Node.js is not compiled with OpenSSL crypto support
at assertCrypto (internal/util.js:101:11)
at crypto.js:31:1
...
(/node/tools/node_modules/eslint/node_modules/file-entry-cache/
cache.js:2:14)
at Module._compile (internal/modules/cjs/loader.js:746:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:757:10)
make: *** [lint-js] Error 1

There are also a number of tests that are affected in a similar way.

This commit adds crypto checks to allow for lint-js and the affected
tests to be skipped when configured --without-ssl.

PR-URL: #26182
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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 outstanding review comments, and a CI started. build Issues and PRs related to build files or the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL