fix: don't set stdioString for any spawn/run-script calls

lukekarrys · wraithgar · commit 1f5382dada18 · 2022-11-02T10:08:27.000-07:00
These libraries now return strings by default which is what we always want in the CLI for error reporting. Fixes #5766
diff --git a/lib/commands/ci.js b/lib/commands/ci.js
@@ -94,7 +94,6 @@ class CI extends ArboristWorkspaceCmd {
           args: [],
           scriptShell,
           stdio: 'inherit',
-          stdioString: true,
           banner: !this.npm.silent,
           event,
         })
diff --git a/lib/commands/explore.js b/lib/commands/explore.js
@@ -59,7 +59,6 @@ class Explore extends BaseCommand {
         pkg,
         banner: false,
         path,
-        stdioString: true,
         event: '_explore',
         stdio: 'inherit',
       }).catch(er => {
diff --git a/lib/commands/install.js b/lib/commands/install.js
@@ -161,7 +161,6 @@ class Install extends ArboristWorkspaceCmd {
           args: [],
           scriptShell,
           stdio: 'inherit',
-          stdioString: true,
           banner: !this.npm.silent,
           event,
         })
diff --git a/lib/commands/run-script.js b/lib/commands/run-script.js
@@ -117,7 +117,6 @@ class RunScript extends BaseCommand {
       args,
       scriptShell,
       stdio: 'inherit',
-      stdioString: true,
       pkg,
       banner: !this.npm.silent,
     }
diff --git a/scripts/util.js b/scripts/util.js
@@ -55,7 +55,6 @@ const spawn = async (cmd, ...allArgs) => {
   let res = null
   try {
     const spawnOpts = {
-      stdioString: true,
       stdio: quiet || out || lines ? 'pipe' : 'inherit',
       cwd: CWD,
       ...opts,
diff --git a/smoke-tests/test/index.js b/smoke-tests/test/index.js
@@ -95,7 +95,6 @@ const exec = async (...args) => {
       PATH: `${PATH}:${binLocation}`,
       COMSPEC: process.env.COMSPEC,
     },
-    stdioString: true,
     encoding: 'utf-8',
   })
 
diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js
@@ -100,7 +100,6 @@ for (const [name, bash] of bashes) {
       const result = await spawn(bash, args, {
         env: { PATH: path, npm_config_update_notifier: 'false' },
         cwd: path,
-        stdioString: true,
       })
       t.match(result, {
         cmd: bash,
@@ -119,7 +118,6 @@ for (const [name, bash] of bashes) {
       const result = await spawn(bash, args, {
         env: { PATH: path, npm_config_update_notifier: 'false' },
         cwd: path,
-        stdioString: true,
       })
       t.match(result, {
         cmd: bash,
diff --git a/test/lib/commands/run-script.js b/test/lib/commands/run-script.js
@@ -125,7 +125,6 @@ t.test('default env, start, and restart scripts', t => {
         args: [],
         scriptShell: undefined,
         stdio: 'inherit',
-        stdioString: true,
         pkg: { name: 'x', version: '1.2.3', _id: 'x@1.2.3', scripts: {} },
         event: 'start',
       },
@@ -140,7 +139,6 @@ t.test('default env, start, and restart scripts', t => {
         args: [],
         scriptShell: undefined,
         stdio: 'inherit',
-        stdioString: true,
         pkg: {
           name: 'x',
           version: '1.2.3',
@@ -162,7 +160,6 @@ t.test('default env, start, and restart scripts', t => {
         args: [],
         scriptShell: undefined,
         stdio: 'inherit',
-        stdioString: true,
         pkg: {
           name: 'x',
           version: '1.2.3',
@@ -185,7 +182,6 @@ t.test('default env, start, and restart scripts', t => {
         args: [],
         scriptShell: undefined,
         stdio: 'inherit',
-        stdioString: true,
         pkg: {
           name: 'x',
           version: '1.2.3',
@@ -220,7 +216,6 @@ t.test('non-default env script', t => {
         args: [],
         scriptShell: undefined,
         stdio: 'inherit',
-        stdioString: true,
         pkg: {
           name: 'x',
           version: '1.2.3',
@@ -242,7 +237,6 @@ t.test('non-default env script', t => {
         args: [],
         scriptShell: undefined,
         stdio: 'inherit',
-        stdioString: true,
         pkg: {
           name: 'x',
           version: '1.2.3',
@@ -305,7 +299,6 @@ t.test('run pre/post hooks', async t => {
       args: [],
       scriptShell: undefined,
       stdio: 'inherit',
-      stdioString: true,
       pkg: {
         name: 'x',
         version: '1.2.3',
@@ -342,7 +335,6 @@ t.test('skip pre/post hooks when using ignoreScripts', async t => {
       args: [],
       scriptShell: undefined,
       stdio: 'inherit',
-      stdioString: true,
       pkg: {
         name: 'x',
         version: '1.2.3',
@@ -385,7 +377,6 @@ t.test('run silent', async t => {
       args: [],
       scriptShell: undefined,
       stdio: 'inherit',
-      stdioString: true,
       pkg: {
         name: 'x',
         version: '1.2.3',
diff --git a/workspaces/arborist/lib/arborist/rebuild.js b/workspaces/arborist/lib/arborist/rebuild.js
@@ -345,7 +345,6 @@ module.exports = cls => class Builder extends cls {
         event,
         path,
         pkg,
-        stdioString: true,
         stdio,
         env,
         scriptShell: this[_scriptShell],
diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js
@@ -1558,7 +1558,6 @@ module.exports = cls => class Reifier extends cls {
               event,
               path,
               pkg,
-              stdioString: true,
               stdio,
               scriptShell: this.options.scriptShell,
             })
diff --git a/workspaces/arborist/test/arborist/rebuild.js b/workspaces/arborist/test/arborist/rebuild.js
@@ -421,7 +421,6 @@ t.test('rebuild node-gyp dependencies lacking both preinstall and install script
       event: 'install',
       path: resolve(path, 'node_modules/dep'),
       pkg: { scripts: { install: 'node-gyp rebuild' } },
-      stdioString: true,
       env: {
         npm_package_resolved: null,
         npm_package_integrity: null,
diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js
@@ -1770,7 +1770,6 @@ console.log('ok 1 - this is fine')
     event: 'test',
     path,
     pkg,
-    stdioString: true,
     stdio: 'pipe',
   }), 'test result')
 })
diff --git a/workspaces/libnpmexec/lib/run-script.js b/workspaces/libnpmexec/lib/run-script.js
@@ -68,7 +68,6 @@ const run = async ({
       banner: false,
       // we always run in cwd, not --prefix
       path: runPath,
-      stdioString: true,
       binPaths,
       event: 'npx',
       args,
diff --git a/workspaces/libnpmpack/test/index.js b/workspaces/libnpmpack/test/index.js
@@ -16,11 +16,6 @@ const OPTS = {
 
 const REG = OPTS.registry
 
-// TODO this ... smells.  npm "script-shell" config mentions defaults but those
-// are handled by run-script, not npm.  So for now we have to tie tests to some
-// pretty specific internals of runScript
-const makeSpawnArgs = require('@npmcli/run-script/lib/make-spawn-args.js')
-
 t.test('packs from local directory', async t => {
   const testDir = t.testdir({
     'package.json': JSON.stringify({
@@ -152,13 +147,15 @@ t.test('runs scripts in foreground when foregroundScripts === true', async t =>
   const cwd = process.cwd()
   process.chdir(testDir)
 
-  const [scriptShell, scriptArgs] = makeSpawnArgs({
-    event: 'prepack',
-    path: testDir,
-    cmd: 'touch prepack',
-  })
+  const shell = process.platform === 'win32'
+    ? process.env.COMSPEC
+    : 'sh'
+
+  const args = process.platform === 'win32'
+    ? ['/d', '/s', '/c', 'touch prepack']
+    : ['-c', 'touch prepack']
 
-  const prepack = spawk.spawn(scriptShell, scriptArgs)
+  const prepack = spawk.spawn(shell, args)
 
   await pack('file:.', {
     packDestination: testDir,
@@ -186,13 +183,7 @@ t.test('doesn\'t run scripts when ignoreScripts === true', async t => {
   const cwd = process.cwd()
   process.chdir(testDir)
 
-  const [scriptShell, scriptArgs] = makeSpawnArgs({
-    event: 'prepack',
-    path: testDir,
-    cmd: 'touch prepack',
-  })
-
-  const prepack = spawk.spawn(scriptShell, scriptArgs)
+  const prepack = spawk.spawn('sh', ['-c', 'touch prepack'])
 
   await pack('file:.', {
     packDestination: testDir,

Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,6 @@ class RunScript extends BaseCommand {`
`117`	`117`	`args,`
`118`	`118`	`scriptShell,`
`119`	`119`	`stdio: 'inherit',`
`120`		`- stdioString: true,`
`121`	`120`	`pkg,`
`122`	`121`	`banner: !this.npm.silent,`
`123`	`122`	`}`