From 45dd8b8615bb1d7a93e1733746581049a1f399e6 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 23 Mar 2022 15:16:49 -0700 Subject: [PATCH] fix: move shellout logic into commands Each command now signals whether or not it is a shellout --- lib/commands/birthday.js | 1 + lib/commands/exec.js | 1 + lib/commands/run-script.js | 1 + lib/lifecycle-cmd.js | 1 + lib/npm.js | 6 +----- lib/utils/cmd-list.js | 14 -------------- lib/utils/exit-handler.js | 2 +- tap-snapshots/test/lib/utils/cmd-list.js.test.cjs | 9 --------- test/lib/npm.js | 1 - test/lib/utils/exit-handler.js | 5 ++++- 10 files changed, 10 insertions(+), 31 deletions(-) diff --git a/lib/commands/birthday.js b/lib/commands/birthday.js index e889b39f25377..d8305cf8d582b 100644 --- a/lib/commands/birthday.js +++ b/lib/commands/birthday.js @@ -3,6 +3,7 @@ const BaseCommand = require('../base-command.js') class Birthday extends BaseCommand { static name = 'birthday' static ignoreImplicitWorkspace = true + static isShellout = true async exec () { this.npm.config.set('yes', true) diff --git a/lib/commands/exec.js b/lib/commands/exec.js index 6b402c856ab1e..5e6a94296d287 100644 --- a/lib/commands/exec.js +++ b/lib/commands/exec.js @@ -46,6 +46,7 @@ class Exec extends BaseCommand { ] static ignoreImplicitWorkspace = false + static isShellout = true async exec (_args, { locationMsg, path, runPath } = {}) { if (!path) { diff --git a/lib/commands/run-script.js b/lib/commands/run-script.js index 74757e984aeed..e31172cc8fa61 100644 --- a/lib/commands/run-script.js +++ b/lib/commands/run-script.js @@ -41,6 +41,7 @@ class RunScript extends BaseCommand { static name = 'run-script' static usage = [' [-- ]'] static ignoreImplicitWorkspace = false + static isShellout = true async completion (opts) { const argv = opts.conf.argv.remain diff --git a/lib/lifecycle-cmd.js b/lib/lifecycle-cmd.js index e2190c2de1c0f..41633a4ba389c 100644 --- a/lib/lifecycle-cmd.js +++ b/lib/lifecycle-cmd.js @@ -4,6 +4,7 @@ const BaseCommand = require('./base-command.js') class LifecycleCmd extends BaseCommand { static usage = ['[-- ]'] + static isShellout = true async exec (args, cb) { return this.npm.exec('run-script', [this.constructor.name, ...args]) diff --git a/lib/npm.js b/lib/npm.js index 4cd1d05b373ec..fb4fb8d13e362 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -6,7 +6,6 @@ const Config = require('@npmcli/config') require('graceful-fs').gracefulify(require('fs')) const { definitions, flatten, shorthands } = require('./utils/config/index.js') -const { shellouts } = require('./utils/cmd-list.js') const usage = require('./utils/npm-usage.js') const which = require('which') @@ -62,10 +61,6 @@ class Npm extends EventEmitter { return this.constructor.version } - get shelloutCommands () { - return shellouts - } - // Get an instantiated npm command // npm.command is already taken as the currently running command, a refactor // would be needed to change this @@ -92,6 +87,7 @@ class Npm extends EventEmitter { if (!this.command) { process.env.npm_command = command.name this.command = command.name + this.commandInstance = command } // this is async but we dont await it, since its ok if it doesnt diff --git a/lib/utils/cmd-list.js b/lib/utils/cmd-list.js index b3089a62c60d2..1469ebdd6f901 100644 --- a/lib/utils/cmd-list.js +++ b/lib/utils/cmd-list.js @@ -139,24 +139,10 @@ const cmdList = [ const plumbing = ['birthday', 'help-search'] -// these commands just shell out to something else or handle the -// error themselves, so it's confusing and weird to write out -// our full error log banner when they exit non-zero -const shellouts = [ - 'exec', - 'run-script', - 'test', - 'start', - 'stop', - 'restart', - 'birthday', -] - module.exports = { aliases: Object.assign({}, shorthands, affordances), shorthands, affordances, cmdList, plumbing, - shellouts, } diff --git a/lib/utils/exit-handler.js b/lib/utils/exit-handler.js index f96d162ce9753..d8ae9994dfecc 100644 --- a/lib/utils/exit-handler.js +++ b/lib/utils/exit-handler.js @@ -144,7 +144,7 @@ const exitHandler = err => { // will presumably print its own errors and exit with a proper status // code if there's a problem. If we got an error with a code=0, then... // something else went wrong along the way, so maybe an npm problem? - const isShellout = npm.shelloutCommands.includes(npm.command) + const isShellout = npm.commandInstance && npm.commandInstance.constructor.isShellout const quietShellout = isShellout && typeof err.code === 'number' && err.code if (quietShellout) { exitCode = err.code diff --git a/tap-snapshots/test/lib/utils/cmd-list.js.test.cjs b/tap-snapshots/test/lib/utils/cmd-list.js.test.cjs index f842e689f1631..e5935846e7497 100644 --- a/tap-snapshots/test/lib/utils/cmd-list.js.test.cjs +++ b/tap-snapshots/test/lib/utils/cmd-list.js.test.cjs @@ -176,15 +176,6 @@ Object { "birthday", "help-search", ], - "shellouts": Array [ - "exec", - "run-script", - "test", - "start", - "stop", - "restart", - "birthday", - ], "shorthands": Object { "c": "config", "cit": "install-ci-test", diff --git a/test/lib/npm.js b/test/lib/npm.js index 4302437a6f018..541c1258d3ba1 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -43,7 +43,6 @@ t.test('not yet loaded', async t => { set: Function, }, version: String, - shelloutCommands: Array, }) t.throws(() => npm.config.set('foo', 'bar')) t.throws(() => npm.config.get('foo')) diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index 73bbf06fe85e7..23942cca1c078 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -32,7 +32,7 @@ t.cleanSnapshot = (path) => cleanDate(cleanCwd(path)) // nerf itself, thinking global.process is broken or gone. mockGlobals(t, { process: Object.assign(new EventEmitter(), { - ...pick(process, 'execPath', 'stdout', 'stderr', 'cwd', 'env'), + ...pick(process, 'execPath', 'stdout', 'stderr', 'cwd', 'env', 'umask'), argv: ['/node', ...process.argv.slice(1)], version: 'v1.0.0', kill: () => {}, @@ -450,7 +450,10 @@ t.test('exits uncleanly when only emitting exit event', async (t) => { t.test('do no fancy handling for shellouts', async t => { const { exitHandler, npm, logs } = await mockExitHandler(t) + const exec = await npm.cmd('exec') + npm.command = 'exec' + npm.commandInstance = exec const loudNoises = () => logs.filter(([level]) => ['warn', 'error'].includes(level))