diff --git a/lib/exec.js b/lib/exec.js index 793abb75a0d01..28fbe24bea7b3 100644 --- a/lib/exec.js +++ b/lib/exec.js @@ -126,19 +126,25 @@ const exec = async args => { // no need to install if already present if (add.length) { + const isTTY = process.stdin.isTTY && process.stdout.isTTY if (!npm.flatOptions.yes) { // set -n to always say no if (npm.flatOptions.yes === false) { throw 'canceled' } - const addList = add.map(a => ` ${a.replace(/@$/, '')}`) - .join('\n') + '\n' - const prompt = `Need to install the following packages:\n${ - addList - }Ok to proceed? ` - const confirm = await read({ prompt, default: 'y' }) - if (confirm.trim().toLowerCase().charAt(0) !== 'y') { - throw 'canceled' + + if (!isTTY) { + npm.log.warn('exec', `The following package${add.length === 1 ? ' was' : 's were'} not found and will be installed: ${add.map((pkg) => pkg.replace(/@$/, '')).join(', ')}`) + } else { + const addList = add.map(a => ` ${a.replace(/@$/, '')}`) + .join('\n') + '\n' + const prompt = `Need to install the following packages:\n${ + addList + }Ok to proceed? ` + const confirm = await read({ prompt, default: 'y' }) + if (confirm.trim().toLowerCase().charAt(0) !== 'y') { + throw 'canceled' + } } } await arb.reify({ ...npm.flatOptions, add }) diff --git a/test/lib/exec.js b/test/lib/exec.js index c93517315ce89..6e5c1b37231c8 100644 --- a/test/lib/exec.js +++ b/test/lib/exec.js @@ -19,6 +19,7 @@ class Arborist { } let PROGRESS_ENABLED = true +const LOG_WARN = [] const npm = { flatOptions: { yes: true, @@ -41,6 +42,9 @@ const npm = { }, enableProgress: () => { PROGRESS_ENABLED = true + }, + warn: (...args) => { + LOG_WARN.push(args) } } } @@ -88,6 +92,7 @@ t.afterEach(cb => { READ.length = 0 READ_RESULT = '' READ_ERROR = null + LOG_WARN.length = 0 npm.flatOptions.legacyPeerDeps = false npm.flatOptions.package = [] npm.flatOptions.call = '' @@ -464,7 +469,16 @@ t.test('positional args and --call together is an error', t => { return exec(['foo'], er => t.equal(er, exec.usage)) }) -t.test('prompt when installs are needed if not already present', async t => { +t.test('prompt when installs are needed if not already present and shell is a TTY', async t => { + const stdoutTTY = process.stdout.isTTY + const stdinTTY = process.stdin.isTTY + t.teardown(() => { + process.stdout.isTTY = stdoutTTY + process.stdin.isTTY = stdinTTY + }) + process.stdout.isTTY = true + process.stdin.isTTY = true + const packages = ['foo', 'bar'] READ_RESULT = 'yolo' @@ -522,7 +536,138 @@ t.test('prompt when installs are needed if not already present', async t => { }]) }) +t.test('skip prompt when installs are needed if not already present and shell is not a tty (multiple packages)', async t => { + const stdoutTTY = process.stdout.isTTY + const stdinTTY = process.stdin.isTTY + t.teardown(() => { + process.stdout.isTTY = stdoutTTY + process.stdin.isTTY = stdinTTY + }) + process.stdout.isTTY = false + process.stdin.isTTY = false + + const packages = ['foo', 'bar'] + READ_RESULT = 'yolo' + + npm.flatOptions.package = packages + npm.flatOptions.yes = undefined + + const add = packages.map(p => `${p}@`).sort((a, b) => a.localeCompare(b)) + const path = t.testdir() + const installDir = resolve('cache-dir/_npx/07de77790e5f40f2') + npm.localPrefix = path + ARB_ACTUAL_TREE[path] = { + children: new Map() + } + ARB_ACTUAL_TREE[installDir] = { + children: new Map() + } + MANIFESTS.foo = { + name: 'foo', + version: '1.2.3', + bin: { + foo: 'foo' + }, + _from: 'foo@' + } + MANIFESTS.bar = { + name: 'bar', + version: '1.2.3', + bin: { + bar: 'bar' + }, + _from: 'bar@' + } + await exec(['foobar'], er => { + if (er) { + throw er + } + }) + t.strictSame(MKDIRPS, [installDir], 'need to make install dir') + t.match(ARB_CTOR, [ { package: packages, path } ]) + t.match(ARB_REIFY, [{add, legacyPeerDeps: false}], 'need to install both packages') + t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') + const PATH = `${resolve(installDir, 'node_modules', '.bin')}${delimiter}${process.env.PATH}` + t.match(RUN_SCRIPTS, [{ + pkg: { scripts: { npx: 'foobar' } }, + banner: false, + path: process.cwd(), + stdioString: true, + event: 'npx', + env: { PATH }, + stdio: 'inherit' + }]) + t.strictSame(READ, [], 'should not have prompted') + t.strictSame(LOG_WARN, [['exec', 'The following packages were not found and will be installed: bar, foo']], 'should have printed a warning') +}) + +t.test('skip prompt when installs are needed if not already present and shell is not a tty (single package)', async t => { + const stdoutTTY = process.stdout.isTTY + const stdinTTY = process.stdin.isTTY + t.teardown(() => { + process.stdout.isTTY = stdoutTTY + process.stdin.isTTY = stdinTTY + }) + process.stdout.isTTY = false + process.stdin.isTTY = false + + const packages = ['foo'] + READ_RESULT = 'yolo' + + npm.flatOptions.package = packages + npm.flatOptions.yes = undefined + + const add = packages.map(p => `${p}@`).sort((a, b) => a.localeCompare(b)) + const path = t.testdir() + const installDir = resolve('cache-dir/_npx/f7fbba6e0636f890') + npm.localPrefix = path + ARB_ACTUAL_TREE[path] = { + children: new Map() + } + ARB_ACTUAL_TREE[installDir] = { + children: new Map() + } + MANIFESTS.foo = { + name: 'foo', + version: '1.2.3', + bin: { + foo: 'foo' + }, + _from: 'foo@' + } + await exec(['foobar'], er => { + if (er) { + throw er + } + }) + t.strictSame(MKDIRPS, [installDir], 'need to make install dir') + t.match(ARB_CTOR, [ { package: packages, path } ]) + t.match(ARB_REIFY, [{add, legacyPeerDeps: false}], 'need to install the package') + t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') + const PATH = `${resolve(installDir, 'node_modules', '.bin')}${delimiter}${process.env.PATH}` + t.match(RUN_SCRIPTS, [{ + pkg: { scripts: { npx: 'foobar' } }, + banner: false, + path: process.cwd(), + stdioString: true, + event: 'npx', + env: { PATH }, + stdio: 'inherit' + }]) + t.strictSame(READ, [], 'should not have prompted') + t.strictSame(LOG_WARN, [['exec', 'The following package was not found and will be installed: foo']], 'should have printed a warning') +}) + t.test('abort if prompt rejected', async t => { + const stdoutTTY = process.stdout.isTTY + const stdinTTY = process.stdin.isTTY + t.teardown(() => { + process.stdout.isTTY = stdoutTTY + process.stdin.isTTY = stdinTTY + }) + process.stdout.isTTY = true + process.stdin.isTTY = true + const packages = ['foo', 'bar'] READ_RESULT = 'no, why would I want such a thing??' @@ -570,6 +715,15 @@ t.test('abort if prompt rejected', async t => { }) t.test('abort if prompt false', async t => { + const stdoutTTY = process.stdout.isTTY + const stdinTTY = process.stdin.isTTY + t.teardown(() => { + process.stdout.isTTY = stdoutTTY + process.stdin.isTTY = stdinTTY + }) + process.stdout.isTTY = true + process.stdin.isTTY = true + const packages = ['foo', 'bar'] READ_ERROR = 'canceled' @@ -617,6 +771,15 @@ t.test('abort if prompt false', async t => { }) t.test('abort if -n provided', async t => { + const stdoutTTY = process.stdout.isTTY + const stdinTTY = process.stdin.isTTY + t.teardown(() => { + process.stdout.isTTY = stdoutTTY + process.stdin.isTTY = stdinTTY + }) + process.stdout.isTTY = true + process.stdin.isTTY = true + const packages = ['foo', 'bar'] npm.flatOptions.package = packages