eslint broken when node built without openssl

When building Node.js with configure --without-ssl, make lint and make test error out with:

bash-3.2$ make lint
./node tools/eslint/bin/eslint.js benchmark lib src test tools/doc \
          tools/eslint-rules --rulesdir tools/eslint-rules
crypto.js:17
  throw new Error('Node.js is not compiled with openssl crypto support');
  ^

Error: Node.js is not compiled with openssl crypto support
    at crypto.js:17:9
    at NativeModule.compile (node.js:956:5)
    at Function.NativeModule.require (node.js:900:18)
    at Function.Module._load (module.js:299:25)
    at Module.require (module.js:367:17)
    at require (internal/module.js:16:19)
    at Object.<anonymous> (/Users/james/Node/main/node/tools/eslint/lib/cli-engine.js:40:14)
    at Module._compile (module.js:417:34)
    at Object.Module._extensions..js (module.js:426:10)
    at Module.load (module.js:357:32)
make: *** [jslint] Error 1

Why do we use ./node to run it? It can be broken at any point, doesn't it make sense to use global node which is known to be stable?

That would assume that the machine has a global node installed, which may not be the case.

I don't think we can make an assumption about a global node.

They seem to use md5 from crypto:

/**
 * create a md5Hash of a given string
 * @param  {string} str the string to calculate the hash for
 * @returns {string}    the calculated hash
 */
function md5Hash(str) {
    return crypto
        .createHash("md5")
        .update(str, "utf8")
        .digest("hex");
}

Perhaps just file a PR upstream with a javascript version of md5?

@jasnell

@jasnell it should be a requirement, then.

@vkurchatkin

@vkurchatkin I disagree, everything else works without a global node and iirc that is how the CI runs. We should attempt to make it run without that.

@Fishrock123

@Fishrock123 the idea is that node compiled without OpenSSL is kind of broken: a lot of modules won't work. We can make eslint work by sticking js implementation of md5, but it is not a good long term solution:

updates (including dependencies) can break it again
./node can be broken in many other ways that could prevent eslint and other tools from running correctly

@vkurchatkin

@vkurchatkin if its broken we shouldn't support it.

The test suite currently passes (albeit with skipped crypto tests) and I don't think we should remove that build option.

@jbergstroem

@jbergstroem it's broken in a sense that it's not "node" platform, it's "node-without-ssl" platform, and eslint doesn't support running on "node-without-ssl". Using js implementation of md5 is not going to change that.

@vkurchatkin

@vkurchatkin ok; lib/cli-engine was the only occurrence I could find. Got more info about eslint not supporting being run on "node-without-ssl"?

@jbergstroem

@jbergstroem by supporting "node-without-ssl" I mean running tests on it as a part of CI. It's very unlikely that they would do this (nobody does)

The other option here is to simply disable make lint and linting during make test when openssl is not available.

FWIW, in CI we run nodejs from FreeBSD's pkg. I'd still argue that the tradeoff for eslint to support non-ssl nodejs is so small they (I) should patch (PR) it.

Fixed with #6132.

Navigation Menu

Repository navigation

eslint broken when node built without openssl #5610

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

eslint broken when node built without openssl #5610

Description

Activity

vkurchatkin commented on Mar 8, 2016

jasnell commented on Mar 8, 2016

jbergstroem commented on Mar 9, 2016

vkurchatkin commented on Mar 9, 2016

Fishrock123 commented on Mar 9, 2016

vkurchatkin commented on Mar 9, 2016

jbergstroem commented on Mar 9, 2016

vkurchatkin commented on Mar 9, 2016

jbergstroem commented on Mar 9, 2016

vkurchatkin commented on Mar 9, 2016

jasnell commented on Mar 9, 2016

jbergstroem commented on Mar 9, 2016

jbergstroem commented on Apr 12, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions