From 440642d00b8c1c2c6806afdc43294e992b398b2e Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 13 Dec 2019 18:38:02 -0500 Subject: [PATCH] tools: remove lint-js.js lint-js.js was implemented before ESLint had a caching feature. It is now only used in CI. Let's remove it on the following grounds: * It results in occasional (and puzzling) yellow CI runs for node-test-linter because the tap file is corrupted somehow. Interleaved maybe? I don't know, but a simple solution is removing it and running ESLint directly. * On my local laptop, it reduces the linting from about 75 seconds to about 55 seconds. This kind of savings is not worth the added complexity and the instability noted above. PR-URL: https://github.com/nodejs/node/pull/30955 Reviewed-By: Richard Lau Reviewed-By: Jiawen Geng Reviewed-By: Daijiro Wachi Reviewed-By: Anatoli Papirovski Reviewed-By: Michael Dawson Reviewed-By: James M Snell --- Makefile | 5 +- tools/lint-js.js | 272 ----------------------------------------------- vcbuild.bat | 10 +- 3 files changed, 4 insertions(+), 283 deletions(-) delete mode 100644 tools/lint-js.js diff --git a/Makefile b/Makefile index 89cd8f68885865..4391499052a449 100644 --- a/Makefile +++ b/Makefile @@ -1243,8 +1243,9 @@ lint-js: jslint: lint-js @echo "Please use lint-js instead of jslint" -run-lint-js-ci = tools/lint-js.js $(PARALLEL_ARGS) -f tap -o test-eslint.tap \ - $(LINT_JS_TARGETS) +run-lint-js-ci = tools/node_modules/eslint/bin/eslint.js \ + --report-unused-disable-directives --ext=.js,.mjs,.md -f tap \ + -o test-eslint.tap $(LINT_JS_TARGETS) .PHONY: lint-js-ci # On the CI the output is emitted in the TAP format. diff --git a/tools/lint-js.js b/tools/lint-js.js deleted file mode 100644 index 4ddbe228d16d37..00000000000000 --- a/tools/lint-js.js +++ /dev/null @@ -1,272 +0,0 @@ -'use strict'; - -const rulesDirs = ['tools/eslint-rules']; -const extensions = ['.js', '.mjs', '.md']; -// This is the maximum number of files to be linted per worker at any given time -const maxWorkload = 60; - -const cluster = require('cluster'); -const path = require('path'); -const fs = require('fs'); -const totalCPUs = require('os').cpus().length; - -const CLIEngine = require('eslint').CLIEngine; -const glob = require('eslint/node_modules/glob'); - -const cliOptions = { - rulePaths: rulesDirs, - extensions: extensions, -}; - -// Check if we should fix errors that are fixable -if (process.argv.indexOf('-F') !== -1) - cliOptions.fix = true; - -const cli = new CLIEngine(cliOptions); - -if (cluster.isMaster) { - let numCPUs = 1; - const paths = []; - let files = null; - let totalPaths = 0; - let failures = 0; - let successes = 0; - let lastLineLen = 0; - let curPath = 'Starting ...'; - let showProgress = true; - const globOptions = { - nodir: true, - ignore: '**/node_modules/**/*' - }; - const workerConfig = {}; - let startTime; - let formatter; - let outFn; - let fd; - let i; - - // Check if spreading work among all cores/cpus - if (process.argv.indexOf('-J') !== -1) - numCPUs = totalCPUs; - - // Check if spreading work among an explicit number of cores/cpus - i = process.argv.indexOf('-j'); - if (i !== -1) { - if (!process.argv[i + 1]) - throw new Error('Missing parallel job count'); - numCPUs = parseInt(process.argv[i + 1], 10); - if (!isFinite(numCPUs) || numCPUs <= 0) - throw new Error('Bad parallel job count'); - } - - // Check for custom ESLint report formatter - i = process.argv.indexOf('-f'); - if (i !== -1) { - if (!process.argv[i + 1]) - throw new Error('Missing format name'); - const format = process.argv[i + 1]; - formatter = cli.getFormatter(format); - if (!formatter) - throw new Error('Invalid format name'); - // Automatically disable progress display - showProgress = false; - // Tell worker to send all results, not just linter errors - workerConfig.sendAll = true; - } else { - // Use default formatter - formatter = cli.getFormatter(); - } - - // Check if outputting ESLint report to a file instead of stdout - i = process.argv.indexOf('-o'); - if (i !== -1) { - if (!process.argv[i + 1]) - throw new Error('Missing output filename'); - const outPath = path.resolve(process.argv[i + 1]); - fd = fs.openSync(outPath, 'w'); - outFn = function(str) { - fs.writeSync(fd, str, 'utf8'); - }; - process.on('exit', () => { fs.closeSync(fd); }); - } else { - outFn = function(str) { - process.stdout.write(str); - }; - } - - // Process the rest of the arguments as paths to lint, ignoring any unknown - // flags - for (i = 2; i < process.argv.length; ++i) { - if (process.argv[i][0] === '-') { - switch (process.argv[i]) { - case '-f': // Skip format name - case '-o': // Skip filename - case '-j': // Skip parallel job count number - ++i; - break; - } - continue; - } - paths.push(process.argv[i]); - } - - if (paths.length === 0) - return; - totalPaths = paths.length; - - if (showProgress) { - // Start the progress display update timer when the first worker is ready - cluster.once('online', () => { - startTime = process.hrtime(); - setInterval(printProgress, 1000).unref(); - printProgress(); - }); - } - - cluster.on('online', (worker) => { - // Configure worker and give it some initial work to do - worker.send(workerConfig); - sendWork(worker); - }); - - process.on('exit', (code) => { - if (showProgress) { - curPath = 'Done'; - printProgress(); - outFn('\r\n'); - } - if (code === 0) - process.exit(failures ? 1 : 0); - }); - - for (i = 0; i < numCPUs; ++i) - cluster.fork().on('message', onWorkerMessage).on('exit', onWorkerExit); - - function onWorkerMessage(results) { - if (typeof results !== 'number') { - // The worker sent us results that are not all successes - if (workerConfig.sendAll) { - failures += results.errorCount; - results = results.results; - } else { - failures += results.length; - } - outFn(`${formatter(results)}\r\n`); - printProgress(); - } else { - successes += results; - } - // Try to give the worker more work to do - sendWork(this); - } - - function onWorkerExit(code, signal) { - if (code !== 0 || signal) - process.exit(2); - } - - function sendWork(worker) { - if (!files || !files.length) { - // We either just started or we have no more files to lint for the current - // path. Find the next path that has some files to be linted. - while (paths.length) { - let dir = paths.shift(); - curPath = dir; - const patterns = cli.resolveFileGlobPatterns([dir]); - dir = path.resolve(patterns[0]); - files = glob.sync(dir, globOptions); - if (files.length) - break; - } - if ((!files || !files.length) && !paths.length) { - // We exhausted all input paths and thus have nothing left to do, so end - // the worker - return worker.disconnect(); - } - } - // Give the worker an equal portion of the work left for the current path, - // but not exceeding a maximum file count in order to help keep *all* - // workers busy most of the time instead of only a minority doing most of - // the work. - const sliceLen = Math.min(maxWorkload, Math.ceil(files.length / numCPUs)); - let slice; - if (sliceLen === files.length) { - // Micro-optimization to avoid splicing to an empty array - slice = files; - files = null; - } else { - slice = files.splice(0, sliceLen); - } - worker.send(slice); - } - - function printProgress() { - if (!showProgress) - return; - - // Clear line - outFn(`\r ${' '.repeat(lastLineLen)}\r`); - - // Calculate and format the data for displaying - const elapsed = process.hrtime(startTime)[0]; - const mins = `${Math.floor(elapsed / 60)}`.padStart(2, '0'); - const secs = `${elapsed % 60}`.padStart(2, '0'); - const passed = `${successes}`.padStart(6); - const failed = `${failures}`.padStart(6); - let pct = `${Math.ceil(((totalPaths - paths.length) / totalPaths) * 100)}`; - pct = pct.padStart(3); - - let line = `[${mins}:${secs}|%${pct}|+${passed}|-${failed}]: ${curPath}`; - - // Truncate line like cpplint does in case it gets too long - if (line.length > 75) - line = `${line.slice(0, 75)}...`; - - // Store the line length so we know how much to erase the next time around - lastLineLen = line.length; - - outFn(line); - } -} else { - // Worker - - let config = {}; - process.on('message', (files) => { - if (files instanceof Array) { - // Lint some files - const report = cli.executeOnFiles(files); - - // If we were asked to fix the fixable issues, do so. - if (cliOptions.fix) - CLIEngine.outputFixes(report); - - if (config.sendAll) { - // Return both success and error results - - const results = report.results; - // Silence warnings for files with no errors while keeping the "ok" - // status - if (report.warningCount > 0) { - for (let i = 0; i < results.length; ++i) { - const result = results[i]; - if (result.errorCount === 0 && result.warningCount > 0) { - result.warningCount = 0; - result.messages = []; - } - } - } - process.send({ results: results, errorCount: report.errorCount }); - } else if (report.errorCount === 0) { - // No errors, return number of successful lint operations - process.send(files.length); - } else { - // One or more errors, return the error results only - process.send(CLIEngine.getErrorResults(report.results)); - } - } else if (typeof files === 'object') { - // The master process is actually sending us our configuration and not a - // list of files to lint - config = files; - } - }); -} diff --git a/vcbuild.bat b/vcbuild.bat index f1e83ead5d372c..37f23017eeb0cf 100644 --- a/vcbuild.bat +++ b/vcbuild.bat @@ -118,8 +118,6 @@ if /i "%1"=="test-v8-all" set test_v8=1&set test_v8_intl=1&set test_v8_ben if /i "%1"=="lint-cpp" set lint_cpp=1&goto arg-ok if /i "%1"=="lint-js" set lint_js=1&goto arg-ok if /i "%1"=="jslint" set lint_js=1&echo Please use lint-js instead of jslint&goto arg-ok -if /i "%1"=="lint-js-ci" set lint_js_ci=1&goto arg-ok -if /i "%1"=="jslint-ci" set lint_js_ci=1&echo Please use lint-js-ci instead of jslint-ci&goto arg-ok if /i "%1"=="lint-md" set lint_md=1&goto arg-ok if /i "%1"=="lint-md-build" set lint_md_build=1&goto arg-ok if /i "%1"=="lint" set lint_cpp=1&set lint_js=1&set lint_md=1&goto arg-ok @@ -662,18 +660,12 @@ goto lint-js goto lint-js :lint-js -if defined lint_js_ci goto lint-js-ci if not defined lint_js goto lint-md-build if not exist tools\node_modules\eslint goto no-lint echo running lint-js %node_exe% tools\node_modules\eslint\bin\eslint.js --cache --report-unused-disable-directives --rule "linebreak-style: 0" --ext=.js,.mjs,.md .eslintrc.js benchmark doc lib test tools goto lint-md-build -:lint-js-ci -echo running lint-js-ci -%node_exe% tools\lint-js.js -J -f tap -o test-eslint.tap benchmark doc lib test tools -goto lint-md-build - :no-lint echo Linting is not available through the source tarball. echo Use the git repo instead: $ git clone https://github.com/nodejs/node.git @@ -705,7 +697,7 @@ set exit_code=1 goto exit :help -echo vcbuild.bat [debug/release] [msi] [doc] [test/test-all/test-addons/test-js-native-api/test-node-api/test-benchmark/test-internet/test-pummel/test-simple/test-message/test-tick-processor/test-known-issues/test-node-inspect/test-check-deopts/test-npm/test-async-hooks/test-v8/test-v8-intl/test-v8-benchmarks/test-v8-all] [ignore-flaky] [static/dll] [noprojgen] [projgen] [small-icu/full-icu/without-intl] [nobuild] [nosnapshot] [noetw] [ltcg] [licensetf] [sign] [ia32/x86/x64/arm64] [vs2017/vs2019] [download-all] [lint/lint-ci/lint-js/lint-js-ci/lint-md] [lint-md-build] [package] [build-release] [upload] [no-NODE-OPTIONS] [link-module path-to-module] [debug-http2] [debug-nghttp2] [clean] [cctest] [no-cctest] [openssl-no-asm] +echo vcbuild.bat [debug/release] [msi] [doc] [test/test-all/test-addons/test-js-native-api/test-node-api/test-benchmark/test-internet/test-pummel/test-simple/test-message/test-tick-processor/test-known-issues/test-node-inspect/test-check-deopts/test-npm/test-async-hooks/test-v8/test-v8-intl/test-v8-benchmarks/test-v8-all] [ignore-flaky] [static/dll] [noprojgen] [projgen] [small-icu/full-icu/without-intl] [nobuild] [nosnapshot] [noetw] [ltcg] [licensetf] [sign] [ia32/x86/x64/arm64] [vs2017/vs2019] [download-all] [lint/lint-ci/lint-js/lint-md] [lint-md-build] [package] [build-release] [upload] [no-NODE-OPTIONS] [link-module path-to-module] [debug-http2] [debug-nghttp2] [clean] [cctest] [no-cctest] [openssl-no-asm] echo Examples: echo vcbuild.bat : builds release build echo vcbuild.bat debug : builds debug build