Merge pull request #31916 from backstage/rugvip/run

cli-common: add common helpers for running sub processes

.changeset/afraid-items-drum.md

@@ -0,0 +1,5 @@
+---
+'@backstage/cli-node': patch
+---
+
+Updated to use new utilities from `@backstage/cli-common`.

.changeset/chilly-bikes-rule.md

@@ -0,0 +1,5 @@
+---
+'@techdocs/cli': patch
+---
+
+Updated to use new utilities from `@backstage/cli-common`.

.changeset/funny-papayas-rest.md

@@ -0,0 +1,6 @@
+---
+'@backstage/repo-tools': patch
+'@backstage/codemods': patch
+---
+
+Updated to use new utilities from `@backstage/cli-common`.

.changeset/modern-taxes-start.md

@@ -0,0 +1,5 @@
+---
+'@backstage/cli-common': patch
+---
+
+Added new `run`, `runOutput`, and `runCheck` utilities to help run child processes in a safe and portable way.

.changeset/tender-dancers-hunt.md

@@ -0,0 +1,5 @@
+---
+'@backstage/cli': patch
+---
+
+Updated to use new utilities from `@backstage/cli-common`.

packages/cli-common/package.json

@@ -35,11 +35,14 @@
     "test": "backstage-cli package test"
   },
   "dependencies": {
+    "@backstage/errors": "workspace:^",
+    "cross-spawn": "^7.0.3",
     "global-agent": "^3.0.0",
     "undici": "^7.2.3"
   },
   "devDependencies": {
     "@backstage/cli": "workspace:^",
+    "@types/cross-spawn": "^6.0.2",
     "@types/node": "^20.16.0"
   }
 }

packages/cli-common/report.api.md

@@ -3,12 +3,23 @@
 > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).
 
 ```ts
+import { ChildProcess } from 'child_process';
+import { CustomErrorBase } from '@backstage/errors';
+import { SpawnOptions } from 'child_process';
+
 // @public
 export const BACKSTAGE_JSON = 'backstage.json';
 
 // @public
 export function bootstrapEnvProxyAgents(): void;
 
+// @public
+export class ExitCodeError extends CustomErrorBase {
+  constructor(code: number, command?: string);
+  // (undocumented)
+  readonly code: number;
+}
+
 // @public
 export function findPaths(searchDir: string): Paths;
 
@@ -29,4 +40,32 @@ export type Paths = {
 
 // @public
 export type ResolveFunc = (...paths: string[]) => string;
+
+// @public
+export function run(args: string[], options?: RunOptions): RunChildProcess;
+
+// @public
+export function runCheck(args: string[]): Promise<boolean>;
+
+// @public
+export interface RunChildProcess extends ChildProcess {
+  waitForExit(): Promise<void>;
+}
+
+// @public
+export type RunOnOutput = (data: Buffer) => void;
+
+// @public
+export type RunOptions = Omit<SpawnOptions, 'env'> & {
+  env?: Partial<NodeJS.ProcessEnv>;
+  onStdout?: RunOnOutput;
+  onStderr?: RunOnOutput;
+  stdio?: SpawnOptions['stdio'];
+};
+
+// @public
+export function runOutput(
+  args: string[],
+  options?: RunOptions,
+): Promise<string>;
 ```

packages/cli-common/src/errors.ts

@@ -0,0 +1,34 @@
+/*
+ * Copyright 2020 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { CustomErrorBase } from '@backstage/errors';
+
+/**
+ * Error thrown when a child process exits with a non-zero code.
+ * @public
+ */
+export class ExitCodeError extends CustomErrorBase {
+  readonly code: number;
+
+  constructor(code: number, command?: string) {
+    super(
+      command
+        ? `Command '${command}' exited with code ${code}`
+        : `Child exited with code ${code}`,
+    );
+    this.code = code;
+  }
+}

packages/cli-common/src/index.ts

@@ -24,3 +24,12 @@ export { findPaths, BACKSTAGE_JSON } from './paths';
 export { isChildPath } from './isChildPath';
 export type { Paths, ResolveFunc } from './paths';
 export { bootstrapEnvProxyAgents } from './proxyBootstrap';
+export {
+  run,
+  runOutput,
+  runCheck,
+  type RunChildProcess,
+  type RunOptions,
+  type RunOnOutput,
+} from './run';
+export { ExitCodeError } from './errors';

packages/cli-common/src/run.test.ts

@@ -0,0 +1,342 @@
+/*
+ * Copyright 2020 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { run, runOutput, runCheck } from './run';
+import { ExitCodeError } from './errors';
+
+describe('run', () => {
+  const activeChildren: Array<{ kill: () => void }> = [];
+
+  afterEach(() => {
+    // Clean up any active child processes
+    activeChildren.forEach(child => {
+      try {
+        child.kill();
+      } catch {
+        // Ignore errors during cleanup
+      }
+    });
+    activeChildren.length = 0;
+    jest.restoreAllMocks();
+  });
+
+  describe('run', () => {
+    it('should throw error for empty args', () => {
+      expect(() => run([])).toThrow('run requires at least one argument');
+    });
+
+    it('should run a successful command', async () => {
+      const child = run(['node', '--version']);
+      activeChildren.push(child);
+      await expect(child.waitForExit()).resolves.not.toThrow();
+    });
+
+    it('should throw ExitCodeError for non-zero exit code', async () => {
+      const child = run(['node', '--eval', 'process.exit(1)']);
+      activeChildren.push(child);
+      await expect(child.waitForExit()).rejects.toThrow(ExitCodeError);
+      await expect(child.waitForExit()).rejects.toThrow(
+        /Command 'node --eval process\.exit\(1\)' exited with code 1/,
+      );
+    });
+
+    it('should call onStdout callback', async () => {
+      const stdoutChunks: Buffer[] = [];
+      const child = run(['node', '--version'], {
+        onStdout: data => stdoutChunks.push(data),
+      });
+      activeChildren.push(child);
+      await child.waitForExit();
+      expect(stdoutChunks.length).toBeGreaterThan(0);
+      const output = Buffer.concat(stdoutChunks).toString();
+      expect(output).toMatch(/v\d+\.\d+\.\d+/);
+    });
+
+    it('should call onStderr callback', async () => {
+      const stderrChunks: Buffer[] = [];
+      const child = run(['node', '--eval', 'console.error("test error")'], {
+        onStderr: data => stderrChunks.push(data),
+      });
+      activeChildren.push(child);
+      await child.waitForExit();
+      expect(stderrChunks.length).toBeGreaterThan(0);
+      const output = Buffer.concat(stderrChunks).toString();
+      expect(output).toContain('test error');
+    });
+
+    it('should use custom stdio', async () => {
+      const child = run(['node', '--version'], {
+        stdio: 'pipe',
+      });
+      activeChildren.push(child);
+      expect(child.stdout).toBeTruthy();
+      expect(child.stderr).toBeTruthy();
+      await child.waitForExit();
+    });
+
+    it('should use custom env', async () => {
+      const customEnv = { CUSTOM_VAR: 'test-value' };
+      const child = run(
+        ['node', '--eval', 'console.log(process.env.CUSTOM_VAR)'],
+        {
+          env: customEnv,
+          onStdout: data => {
+            const output = data.toString();
+            expect(output).toContain('test-value');
+          },
+        },
+      );
+      activeChildren.push(child);
+      await child.waitForExit();
+    });
+
+    it('should set FORCE_COLOR in env', async () => {
+      const child = run(
+        ['node', '--eval', 'console.log(process.env.FORCE_COLOR)'],
+        {
+          onStdout: data => {
+            const output = data.toString();
+            expect(output.trim()).toBe('true');
+          },
+        },
+      );
+      activeChildren.push(child);
+      await child.waitForExit();
+    });
+
+    it('should handle process already exited', async () => {
+      const child = run(['node', '--version']);
+      activeChildren.push(child);
+      // Wait for it to complete
+      await child.waitForExit();
+      // Call waitForExit again - should return immediately
+      await expect(child.waitForExit()).resolves.not.toThrow();
+    });
+
+    it('should handle multiple simultaneous calls to waitForExit', async () => {
+      const child = run(['node', '--version']);
+      activeChildren.push(child);
+      // Call waitForExit multiple times simultaneously
+      const [result1, result2, result3] = await Promise.all([
+        child.waitForExit(),
+        child.waitForExit(),
+        child.waitForExit(),
+      ]);
+      // All should resolve successfully
+      expect(result1).toBeUndefined();
+      expect(result2).toBeUndefined();
+      expect(result3).toBeUndefined();
+    });
+
+    it('should handle multiple simultaneous calls to waitForExit with error', async () => {
+      const child = run(['node', '--eval', 'process.exit(1)']);
+      activeChildren.push(child);
+      // Call waitForExit multiple times simultaneously
+      const promises = [
+        child.waitForExit(),
+        child.waitForExit(),
+        child.waitForExit(),
+      ];
+      // All should reject with the same error
+      for (const promise of promises) {
+        await expect(promise).rejects.toThrow(ExitCodeError);
+        await expect(promise).rejects.toThrow(
+          /Command 'node --eval process\.exit\(1\)' exited with code 1/,
+        );
+      }
+    });
+
+    it('should handle signal handlers cleanup', async () => {
+      const child = run(['node', '--version']);
+      activeChildren.push(child);
+      const originalListeners = process.listenerCount('SIGINT');
+      await child.waitForExit();
+      // Signal handlers should be cleaned up
+      expect(process.listenerCount('SIGINT')).toBe(originalListeners);
+    });
+
+    it('should kill child process on SIGINT', async () => {
+      const child = run(['node', '--eval', 'setTimeout(() => {}, 10000)']);
+      activeChildren.push(child);
+      const killSpy = jest.spyOn(child, 'kill');
+      // Start waiting (this registers signal handlers)
+      const waitPromise = child.waitForExit();
+      // Simulate SIGINT
+      process.emit('SIGINT' as any, 'SIGINT');
+      // Give it a moment
+      await new Promise(resolve => setTimeout(resolve, 100));
+      expect(killSpy).toHaveBeenCalled();
+      killSpy.mockRestore();
+      // Clean up
+      child.kill();
+      // Wait for cleanup
+      try {
+        await Promise.race([
+          waitPromise,
+          new Promise(resolve => setTimeout(resolve, 100)),
+        ]);
+      } catch {
+        // Expected to fail
+      }
+    });
+
+    it('should kill child process on SIGTERM', async () => {
+      const child = run(['node', '--eval', 'setTimeout(() => {}, 10000)']);
+      activeChildren.push(child);
+      const killSpy = jest.spyOn(child, 'kill');
+      // Start waiting (this registers signal handlers)
+      const waitPromise = child.waitForExit();
+      // Simulate SIGTERM
+      process.emit('SIGTERM' as any, 'SIGTERM');
+      // Give it a moment
+      await new Promise(resolve => setTimeout(resolve, 100));
+      expect(killSpy).toHaveBeenCalled();
+      killSpy.mockRestore();
+      // Clean up
+      child.kill();
+      // Wait for cleanup
+      try {
+        await Promise.race([
+          waitPromise,
+          new Promise(resolve => setTimeout(resolve, 100)),
+        ]);
+      } catch {
+        // Expected to fail
+      }
+    });
+
+    it('should not kill already killed process on signal', async () => {
+      const child = run(['node', '--version']);
+      activeChildren.push(child);
+      await child.waitForExit();
+      const killSpy = jest.spyOn(child, 'kill');
+      // Simulate SIGINT after process has exited
+      process.emit('SIGINT' as any, 'SIGINT');
+      await new Promise(resolve => setTimeout(resolve, 100));
+      // Should not be called since process already exited
+      expect(killSpy).not.toHaveBeenCalled();
+      killSpy.mockRestore();
+    });
+
+    it('should handle process error', async () => {
+      const child = run(['nonexistent-command-12345']);
+      activeChildren.push(child);
+      await expect(child.waitForExit()).rejects.toThrow();
+    });
+  });
+
+  describe('runOutput', () => {
+    it('should throw error for empty args', async () => {
+      await expect(runOutput([])).rejects.toThrow(
+        'runOutput requires at least one argument',
+      );
+    });
+
+    it('should return stdout', async () => {
+      const output = await runOutput([
+        'node',
+        '--eval',
+        'console.log("test output")',
+      ]);
+      expect(output).toBe('test output');
+    });
+
+    it('should trim output', async () => {
+      const output = await runOutput([
+        'node',
+        '--eval',
+        'console.log("  test output  ")',
+      ]);
+      expect(output).toBe('test output');
+    });
+
+    it('should attach stdout to error on failure', async () => {
+      let error: Error | undefined;
+      try {
+        await runOutput([
+          'node',
+          '--eval',
+          'console.log("stdout before error"); process.exit(1)',
+        ]);
+      } catch (err) {
+        error = err;
+      }
+      expect(error).toBeInstanceOf(ExitCodeError);
+      expect((error as Error & { stdout?: string }).stdout).toContain(
+        'stdout before error',
+      );
+    });
+
+    it('should attach stderr to error on failure', async () => {
+      let error: Error | undefined;
+      try {
+        await runOutput([
+          'node',
+          '--eval',
+          'console.error("stderr error"); process.exit(1)',
+        ]);
+      } catch (err) {
+        error = err;
+      }
+      expect(error).toBeInstanceOf(ExitCodeError);
+      expect((error as Error & { stderr?: string }).stderr).toContain(
+        'stderr error',
+      );
+    });
+
+    it('should call custom onStdout callback', async () => {
+      const customChunks: Buffer[] = [];
+      const output = await runOutput(
+        ['node', '--eval', 'console.log("test")'],
+        {
+          onStdout: data => customChunks.push(data),
+        },
+      );
+      expect(output).toBe('test');
+      expect(customChunks.length).toBeGreaterThan(0);
+    });
+
+    it('should call custom onStderr callback', async () => {
+      const customChunks: Buffer[] = [];
+      await runOutput(
+        ['node', '--eval', 'console.error("error"); console.log("ok")'],
+        {
+          onStderr: data => customChunks.push(data),
+        },
+      );
+      expect(customChunks.length).toBeGreaterThan(0);
+      const errorOutput = Buffer.concat(customChunks).toString();
+      expect(errorOutput).toContain('error');
+    });
+  });
+
+  describe('runCheck', () => {
+    it('should return true for successful command', async () => {
+      const result = await runCheck(['node', '--version']);
+      expect(result).toBe(true);
+    });
+
+    it('should return false for failed command', async () => {
+      const result = await runCheck(['node', '--eval', 'process.exit(1)']);
+      expect(result).toBe(false);
+    });
+
+    it('should return false for nonexistent command', async () => {
+      const result = await runCheck(['nonexistent-command-12345']);
+      expect(result).toBe(false);
+    });
+  });
+});

packages/cli-common/src/run.ts

@@ -0,0 +1,219 @@
+/*
+ * Copyright 2020 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { ChildProcess, SpawnOptions } from 'child_process';
+import spawn from 'cross-spawn';
+import { ExitCodeError } from './errors';
+import { assertError } from '@backstage/errors';
+
+/**
+ * Callback function that can be used to receive stdout or stderr data from a child process.
+ *
+ * @public
+ */
+export type RunOnOutput = (data: Buffer) => void;
+
+/**
+ * Options for running a child process with {@link run} or related functions.
+ *
+ * @public
+ */
+export type RunOptions = Omit<SpawnOptions, 'env'> & {
+  env?: Partial<NodeJS.ProcessEnv>;
+  onStdout?: RunOnOutput;
+  onStderr?: RunOnOutput;
+  stdio?: SpawnOptions['stdio'];
+};
+
+/**
+ * Child process handle returned by {@link run}.
+ *
+ * @public
+ */
+export interface RunChildProcess extends ChildProcess {
+  /**
+   * Waits for the child process to exit.
+   *
+   * @remarks
+   *
+   * Resolves when the process exits successfully (exit code 0) or is terminated by a signal.
+   * If the process exits with a non-zero exit code, the promise is rejected with an {@link ExitCodeError}.
+   *
+   * @returns A promise that resolves when the process exits successfully or is terminated by a signal, or rejects on error.
+   */
+  waitForExit(): Promise<void>;
+}
+
+/**
+ * Runs a command and returns a child process handle.
+ *
+ * @public
+ */
+export function run(args: string[], options: RunOptions = {}): RunChildProcess {
+  if (args.length === 0) {
+    throw new Error('run requires at least one argument');
+  }
+
+  const [name, ...cmdArgs] = args;
+
+  const { onStdout, onStderr, stdio: customStdio, ...spawnOptions } = options;
+  const env: NodeJS.ProcessEnv = {
+    ...process.env,
+    FORCE_COLOR: 'true',
+    ...(options.env ?? {}),
+  };
+
+  const stdio =
+    customStdio ??
+    ([
+      'inherit',
+      onStdout ? 'pipe' : 'inherit',
+      onStderr ? 'pipe' : 'inherit',
+    ] as ('inherit' | 'pipe')[]);
+
+  const child = spawn(name, cmdArgs, {
+    ...spawnOptions,
+    stdio,
+    env,
+  }) as RunChildProcess;
+
+  if (onStdout && child.stdout) {
+    child.stdout.on('data', onStdout);
+  }
+  if (onStderr && child.stderr) {
+    child.stderr.on('data', onStderr);
+  }
+
+  const commandName = args.join(' ');
+
+  let waitPromise: Promise<void> | undefined;
+
+  child.waitForExit = async (): Promise<void> => {
+    if (waitPromise) {
+      return waitPromise;
+    }
+
+    waitPromise = new Promise<void>((resolve, reject) => {
+      if (typeof child.exitCode === 'number') {
+        if (child.exitCode) {
+          reject(new ExitCodeError(child.exitCode, commandName));
+        } else {
+          resolve();
+        }
+        return;
+      }
+
+      function onError(error: Error) {
+        cleanup();
+        reject(error);
+      }
+
+      function onExit(code: number | null) {
+        cleanup();
+        if (code) {
+          reject(new ExitCodeError(code, commandName));
+        } else {
+          resolve();
+        }
+      }
+
+      function onSignal() {
+        if (!child.killed && child.exitCode === null) {
+          child.kill();
+        }
+      }
+
+      function cleanup() {
+        for (const signal of ['SIGINT', 'SIGTERM'] as const) {
+          process.removeListener(signal, onSignal);
+        }
+        child.removeListener('error', onError);
+        child.removeListener('exit', onExit);
+      }
+
+      child.once('error', onError);
+      child.once('exit', onExit);
+
+      for (const signal of ['SIGINT', 'SIGTERM'] as const) {
+        process.addListener(signal, onSignal);
+      }
+    });
+
+    return waitPromise;
+  };
+
+  return child;
+}
+
+/**
+ * Runs a command and returns the stdout.
+ *
+ * @remarks
+ *
+ * On error, both stdout and stderr are attached to the error object as properties.
+ *
+ * @public
+ */
+export async function runOutput(
+  args: string[],
+  options?: RunOptions,
+): Promise<string> {
+  const stdoutChunks: Buffer[] = [];
+  const stderrChunks: Buffer[] = [];
+
+  if (args.length === 0) {
+    throw new Error('runOutput requires at least one argument');
+  }
+
+  try {
+    await run(args, {
+      ...options,
+      onStdout: data => {
+        stdoutChunks.push(data);
+        options?.onStdout?.(data);
+      },
+      onStderr: data => {
+        stderrChunks.push(data);
+        options?.onStderr?.(data);
+      },
+    }).waitForExit();
+
+    return Buffer.concat(stdoutChunks).toString().trim();
+  } catch (error) {
+    assertError(error);
+
+    (error as Error & { stdout?: string }).stdout =
+      Buffer.concat(stdoutChunks).toString();
+    (error as Error & { stderr?: string }).stderr =
+      Buffer.concat(stderrChunks).toString();
+
+    throw error;
+  }
+}
+
+/**
+ * Runs a command and returns true if it exits with code 0, false otherwise.
+ *
+ * @public
+ */
+export async function runCheck(args: string[]): Promise<boolean> {
+  try {
+    await run(args).waitForExit();
+    return true;
+  } catch {
+    return false;
+  }
+}

packages/cli-node/src/git/GitUtils.ts

@@ -15,23 +15,27 @@
  */
 
 import { assertError, ForwardedError } from '@backstage/errors';
-import { execFile, paths } from '../util';
+import { paths } from '../paths';
+import { runOutput } from '@backstage/cli-common';
 
 /**
  * Run a git command, trimming the output splitting it into lines.
  */
 export async function runGit(...args: string[]) {
   try {
-    const { stdout } = await execFile('git', args, {
-      shell: true,
+    const stdout = await runOutput(['git', ...args], {
       cwd: paths.targetRoot,
     });
     return stdout.trim().split(/\r\n|\r|\n/);
   } catch (error) {
     assertError(error);
-    if (error.stderr || typeof error.code === 'number') {
-      const stderr = (error.stderr as undefined | Buffer)?.toString('utf8');
-      const msg = stderr?.trim() ?? `with exit code ${error.code}`;
+    if (
+      'code' in error &&
+      typeof (error as { code?: number }).code === 'number'
+    ) {
+      const code = (error as { code?: number }).code;
+      const stderr = (error as { stderr?: string }).stderr;
+      const msg = stderr?.trim() ?? `with exit code ${code}`;
       throw new Error(`git ${args[0]} failed, ${msg}`);
     }
     throw new ForwardedError('Unknown execution error', error);
@@ -83,10 +87,8 @@ export class GitUtils {
       // silently fall back to using the ref directly if merge base is not available
     }
 
-    const { stdout } = await execFile('git', ['show', `${showRef}:${path}`], {
-      shell: true,
+    const stdout = await runOutput(['git', 'show', `${showRef}:${path}`], {
       cwd: paths.targetRoot,
-      maxBuffer: 1024 * 1024 * 50,
     });
     return stdout;
   }

packages/cli-node/src/monorepo/PackageGraph.test.ts

@@ -23,7 +23,7 @@ import { GitUtils } from '../git';
 const mockListChangedFiles = jest.spyOn(GitUtils, 'listChangedFiles');
 const mockReadFileAtRef = jest.spyOn(GitUtils, 'readFileAtRef');
 
-jest.mock('../util', () => ({
+jest.mock('../paths', () => ({
   paths: {
     targetRoot: '/',
     resolveTargetRoot: (...paths: string[]) => resolvePath('/', ...paths),

packages/cli-node/src/monorepo/PackageGraph.ts

@@ -16,7 +16,7 @@
 
 import path from 'path';
 import { getPackages, Package } from '@manypkg/get-packages';
-import { paths } from '../util';
+import { paths } from '../paths';
 import { PackageRole } from '../roles';
 import { GitUtils } from '../git';
 import { Lockfile } from './Lockfile';

packages/cli-node/src/monorepo/isMonoRepo.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { paths } from '../util';
+import { paths } from '../paths';
 import fs from 'fs-extra';
 
 /**

packages/cli-node/src/monorepo/isMonorepo.test.ts

@@ -19,7 +19,7 @@ import { createMockDirectory } from '@backstage/backend-test-utils';
 
 const mockDir = createMockDirectory();
 
-jest.mock('../util', () => ({
+jest.mock('../paths', () => ({
   paths: { resolveTargetRoot: (...args: string[]) => mockDir.resolve(...args) },
 }));
 

packages/cli-node/src/pacman/PackageManager.test.ts

@@ -21,8 +21,8 @@ import { withLogCollector } from '@backstage/test-utils';
 
 const mockDir = createMockDirectory();
 
-jest.mock('../util', () => ({
-  ...jest.requireActual('../util'),
+jest.mock('../paths', () => ({
+  ...jest.requireActual('../paths'),
   paths: { resolveTargetRoot: (...args: string[]) => mockDir.resolve(...args) },
 }));
 

packages/cli-node/src/pacman/PackageManager.ts

@@ -16,7 +16,8 @@
 
 import { Yarn } from './yarn';
 import { Lockfile } from './Lockfile';
-import { SpawnOptionsPartialEnv, paths } from '../util';
+import { paths } from '../paths';
+import { RunOptions } from '@backstage/cli-common';
 import fs from 'fs-extra';
 
 /**
@@ -55,7 +56,7 @@ export interface PackageManager {
   getMonorepoPackages(): Promise<string[]>;
 
   /** Uses the package manager to run a command in the repo. */
-  run(args: string[], options?: SpawnOptionsPartialEnv): Promise<void>;
+  run(args: string[], options?: RunOptions): Promise<void>;
 
   /**
    * Executes the package manager's pack command to bundle the repo into an

packages/cli-node/src/pacman/yarn/Yarn.test.ts

@@ -19,8 +19,8 @@ import { Yarn } from './Yarn';
 
 const mockDir = createMockDirectory();
 
-jest.mock('../../util', () => ({
-  ...jest.requireActual('../../util'),
+jest.mock('../../paths', () => ({
+  ...jest.requireActual('../../paths'),
   paths: { resolveTargetRoot: (...args: string[]) => mockDir.resolve(...args) },
 }));
 

packages/cli-node/src/pacman/yarn/Yarn.ts

@@ -23,7 +23,8 @@ import { PackageInfo, PackageManager } from '../PackageManager';
 import { Lockfile } from '../Lockfile';
 import { YarnVersion } from './types';
 import fs from 'fs-extra';
-import { paths, run, execFile, SpawnOptionsPartialEnv } from '../../util';
+import { paths } from '../../paths';
+import { run, runOutput, RunOptions } from '@backstage/cli-common';
 
 export class Yarn implements PackageManager {
   constructor(private readonly yarnVersion: YarnVersion) {}
@@ -63,8 +64,8 @@ export class Yarn implements PackageManager {
     });
   }
 
-  async run(args: string[], options?: SpawnOptionsPartialEnv) {
-    await run('yarn', args, options);
+  async run(args: string[], options?: RunOptions) {
+    await run(['yarn', ...args], options).waitForExit();
   }
 
   async fetchPackageInfo(): Promise<PackageInfo> {
@@ -98,8 +99,7 @@ function detectYarnVersion(dir?: string): Promise<YarnVersion> {
 
   const promise = Promise.resolve().then(async () => {
     try {
-      const { stdout } = await execFile('yarn', ['--version'], {
-        shell: true,
+      const stdout = await runOutput(['yarn', '--version'], {
         cwd,
       });
       const versionString = stdout.trim();
@@ -109,9 +109,6 @@ function detectYarnVersion(dir?: string): Promise<YarnVersion> {
       return { version: versionString, codename };
     } catch (error) {
       assertError(error);
-      if ('stderr' in error) {
-        process.stderr.write(error.stderr as Buffer);
-      }
       throw new ForwardedError('Failed to determine yarn version', error);
     }
   });

packages/cli-node/src/paths.ts

@@ -0,0 +1,20 @@
+/*
+ * Copyright 2020 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { findPaths } from '@backstage/cli-common';
+
+/* eslint-disable-next-line no-restricted-syntax */
+export const paths = findPaths(__dirname);

packages/cli-node/src/util.ts

@@ -1,109 +0,0 @@
-/*
- * Copyright 2020 The Backstage Authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import {
-  ChildProcess,
-  execFile as execFileCb,
-  spawn,
-  SpawnOptions,
-} from 'child_process';
-import { promisify } from 'util';
-import { findPaths } from '@backstage/cli-common';
-import { ExitCodeError } from './errors';
-
-export const execFile = promisify(execFileCb);
-
-/* eslint-disable-next-line no-restricted-syntax */
-export const paths = findPaths(__dirname);
-
-/**
- * A function that can be used to log data from a child process
- *
- * @public
- */
-export type LogFunc = (data: Buffer) => void;
-
-/**
- * Options for running a child process
- *
- * @public
- */
-export type SpawnOptionsPartialEnv = Omit<SpawnOptions, 'env'> & {
-  env?: Partial<NodeJS.ProcessEnv>;
-  // Pipe stdout to this log function
-  stdoutLogFunc?: LogFunc;
-  // Pipe stderr to this log function
-  stderrLogFunc?: LogFunc;
-};
-
-// Runs a child command, returning a promise that is only resolved if the child exits with code 0.
-export async function run(
-  name: string,
-  args: string[] = [],
-  options: SpawnOptionsPartialEnv = {},
-) {
-  const { stdoutLogFunc, stderrLogFunc } = options;
-  const env: NodeJS.ProcessEnv = {
-    ...process.env,
-    FORCE_COLOR: 'true',
-    ...(options.env ?? {}),
-  };
-
-  const stdio = [
-    'inherit',
-    stdoutLogFunc ? 'pipe' : 'inherit',
-    stderrLogFunc ? 'pipe' : 'inherit',
-  ] as ('inherit' | 'pipe')[];
-
-  const child = spawn(name, args, {
-    stdio,
-    shell: true,
-    ...options,
-    env,
-  });
-
-  if (stdoutLogFunc && child.stdout) {
-    child.stdout.on('data', stdoutLogFunc);
-  }
-  if (stderrLogFunc && child.stderr) {
-    child.stderr.on('data', stderrLogFunc);
-  }
-
-  await waitForExit(child, name);
-}
-
-async function waitForExit(
-  child: ChildProcess & { exitCode: number | null },
-  name?: string,
-): Promise<void> {
-  if (typeof child.exitCode === 'number') {
-    if (child.exitCode) {
-      throw new ExitCodeError(child.exitCode, name);
-    }
-    return;
-  }
-
-  await new Promise<void>((resolve, reject) => {
-    child.once('error', error => reject(error));
-    child.once('exit', code => {
-      if (code) {
-        reject(new ExitCodeError(code, name));
-      } else {
-        resolve();
-      }
-    });
-  });
-}

packages/cli/src/lib/run.ts

@@ -1,121 +0,0 @@
-/*
- * Copyright 2020 The Backstage Authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import {
-  SpawnOptions,
-  spawn,
-  ChildProcess,
-  execFile as execFileCb,
-} from 'child_process';
-import { ExitCodeError } from './errors';
-import { promisify } from 'util';
-import { assertError, ForwardedError } from '@backstage/errors';
-
-export const execFile = promisify(execFileCb);
-
-type LogFunc = (data: Buffer) => void;
-
-type SpawnOptionsPartialEnv = Omit<SpawnOptions, 'env'> & {
-  env?: Partial<NodeJS.ProcessEnv>;
-  // Pipe stdout to this log function
-  stdoutLogFunc?: LogFunc;
-  // Pipe stderr to this log function
-  stderrLogFunc?: LogFunc;
-};
-
-// Runs a child command, returning a promise that is only resolved if the child exits with code 0.
-export async function run(
-  name: string,
-  args: string[] = [],
-  options: SpawnOptionsPartialEnv = {},
-) {
-  const { stdoutLogFunc, stderrLogFunc } = options;
-  const env: NodeJS.ProcessEnv = {
-    ...process.env,
-    FORCE_COLOR: 'true',
-    ...(options.env ?? {}),
-  };
-
-  const stdio = [
-    'inherit',
-    stdoutLogFunc ? 'pipe' : 'inherit',
-    stderrLogFunc ? 'pipe' : 'inherit',
-  ] as ('inherit' | 'pipe')[];
-
-  const child = spawn(name, args, {
-    stdio,
-    shell: true,
-    ...options,
-    env,
-  });
-
-  if (stdoutLogFunc && child.stdout) {
-    child.stdout.on('data', stdoutLogFunc);
-  }
-  if (stderrLogFunc && child.stderr) {
-    child.stderr.on('data', stderrLogFunc);
-  }
-
-  await waitForExit(child, name);
-}
-
-export async function runPlain(cmd: string, ...args: string[]) {
-  try {
-    const { stdout } = await execFile(cmd, args, { shell: true });
-    return stdout.trim();
-  } catch (error) {
-    assertError(error);
-    if ('stderr' in error) {
-      process.stderr.write(error.stderr as Buffer);
-    }
-    if (typeof error.code === 'number') {
-      throw new ExitCodeError(error.code, [cmd, ...args].join(' '));
-    }
-    throw new ForwardedError('Unknown execution error', error);
-  }
-}
-
-export async function runCheck(cmd: string, ...args: string[]) {
-  try {
-    await execFile(cmd, args, { shell: true });
-    return true;
-  } catch (error) {
-    return false;
-  }
-}
-
-export async function waitForExit(
-  child: ChildProcess & { exitCode: number | null },
-  name?: string,
-): Promise<void> {
-  if (typeof child.exitCode === 'number') {
-    if (child.exitCode) {
-      throw new ExitCodeError(child.exitCode, name);
-    }
-    return;
-  }
-
-  await new Promise<void>((resolve, reject) => {
-    child.once('error', error => reject(error));
-    child.once('exit', code => {
-      if (code) {
-        reject(new ExitCodeError(code, name));
-      } else {
-        resolve();
-      }
-    });
-  });
-}

packages/cli/src/lib/versioning/packages.test.ts

@@ -14,16 +14,17 @@
  * limitations under the License.
  */
 
-import * as runObj from '../run';
+import * as runObj from '@backstage/cli-common';
 import * as yarn from './yarn';
 import { fetchPackageInfo, mapDependencies } from './packages';
 import { createMockDirectory } from '@backstage/backend-test-utils';
 import { NotFoundError } from '@backstage/errors';
 
-jest.mock('../run', () => {
+jest.mock('@backstage/cli-common', () => {
+  const actual = jest.requireActual('@backstage/cli-common');
   return {
-    run: jest.fn(),
-    execFile: jest.fn(),
+    ...actual,
+    runOutput: jest.fn(),
   };
 });
 
@@ -39,42 +40,40 @@ describe('fetchPackageInfo', () => {
   });
 
   it('should forward info for yarn classic', async () => {
-    jest.spyOn(runObj, 'execFile').mockResolvedValue({
-      stdout: `{"type":"inspect","data":{"the":"data"}}`,
-      stderr: '',
-    });
+    jest
+      .spyOn(runObj, 'runOutput')
+      .mockResolvedValue(`{"type":"inspect","data":{"the":"data"}}`);
     jest.spyOn(yarn, 'detectYarnVersion').mockResolvedValue('classic');
 
     await expect(fetchPackageInfo('my-package')).resolves.toEqual({
       the: 'data',
     });
-    expect(runObj.execFile).toHaveBeenCalledWith(
+    expect(runObj.runOutput).toHaveBeenCalledWith([
       'yarn',
-      ['info', '--json', 'my-package'],
-      { shell: true },
-    );
+      'info',
+      '--json',
+      'my-package',
+    ]);
   });
 
   it('should forward info for yarn berry', async () => {
-    jest
-      .spyOn(runObj, 'execFile')
-      .mockResolvedValue({ stdout: `{"the":"data"}`, stderr: '' });
+    jest.spyOn(runObj, 'runOutput').mockResolvedValue(`{"the":"data"}`);
     jest.spyOn(yarn, 'detectYarnVersion').mockResolvedValue('berry');
 
     await expect(fetchPackageInfo('my-package')).resolves.toEqual({
       the: 'data',
     });
-    expect(runObj.execFile).toHaveBeenCalledWith(
+    expect(runObj.runOutput).toHaveBeenCalledWith([
       'yarn',
-      ['npm', 'info', '--json', 'my-package'],
-      { shell: true },
-    );
+      'npm',
+      'info',
+      '--json',
+      'my-package',
+    ]);
   });
 
   it('should throw if no info with yarn classic', async () => {
-    jest
-      .spyOn(runObj, 'execFile')
-      .mockResolvedValue({ stdout: '', stderr: '' });
+    jest.spyOn(runObj, 'runOutput').mockResolvedValue('');
     jest.spyOn(yarn, 'detectYarnVersion').mockResolvedValue('classic');
 
     await expect(fetchPackageInfo('my-package')).rejects.toThrow(
@@ -83,9 +82,10 @@ describe('fetchPackageInfo', () => {
   });
 
   it('should throw if no info with yarn berry', async () => {
-    jest
-      .spyOn(runObj, 'execFile')
-      .mockRejectedValue({ stdout: 'bla bla bla Response Code: 404 bla bla' });
+    const error = new Error('Command failed');
+    (error as Error & { stdout?: string }).stdout =
+      'bla bla bla Response Code: 404 bla bla';
+    jest.spyOn(runObj, 'runOutput').mockRejectedValue(error);
     jest.spyOn(yarn, 'detectYarnVersion').mockResolvedValue('berry');
 
     await expect(fetchPackageInfo('my-package')).rejects.toThrow(

packages/cli/src/lib/versioning/packages.ts

@@ -17,7 +17,7 @@
 import { minimatch } from 'minimatch';
 import { getPackages } from '@manypkg/get-packages';
 import { detectYarnVersion } from './yarn';
-import { execFile } from '../run';
+import { runOutput } from '@backstage/cli-common';
 import { NotFoundError } from '@backstage/errors';
 
 const DEP_TYPES = [
@@ -54,11 +54,7 @@ export async function fetchPackageInfo(
 
   const cmd = yarnVersion === 'classic' ? ['info'] : ['npm', 'info'];
   try {
-    const { stdout: output } = await execFile(
-      'yarn',
-      [...cmd, '--json', name],
-      { shell: true },
-    );
+    const output = await runOutput(['yarn', ...cmd, '--json', name]);
 
     if (!output) {
       throw new NotFoundError(
@@ -81,7 +77,12 @@ export async function fetchPackageInfo(
       throw error;
     }
 
-    if (error?.stdout.includes('Response Code: 404')) {
+    if (
+      error instanceof Error &&
+      'stdout' in error &&
+      typeof error.stdout === 'string' &&
+      error.stdout.includes('Response Code: 404')
+    ) {
       throw new NotFoundError(
         `No package information found for package ${name}`,
       );

packages/cli/src/lib/versioning/yarn.ts

@@ -15,10 +15,7 @@
  */
 
 import { assertError, ForwardedError } from '@backstage/errors';
-import { execFile as execFileCb } from 'child_process';
-import { promisify } from 'util';
-
-const execFile = promisify(execFileCb);
+import { runOutput } from '@backstage/cli-common';
 
 const versions = new Map<string, Promise<'classic' | 'berry'>>();
 
@@ -30,16 +27,12 @@ export function detectYarnVersion(dir?: string): Promise<'classic' | 'berry'> {
 
   const promise = Promise.resolve().then(async () => {
     try {
-      const { stdout } = await execFile('yarn', ['--version'], {
-        shell: true,
+      const stdout = await runOutput(['yarn', '--version'], {
         cwd,
       });
       return stdout.trim().startsWith('1.') ? 'classic' : 'berry';
     } catch (error) {
       assertError(error);
-      if ('stderr' in error) {
-        process.stderr.write(error.stderr as Buffer);
-      }
       throw new ForwardedError('Failed to determine yarn version', error);
     }
   });

packages/cli/src/modules/build/lib/bundler/config.ts

@@ -29,7 +29,7 @@ import { paths as cliPaths } from '../../../../lib/paths';
 import fs from 'fs-extra';
 import { optimization as optimizationConfig } from './optimization';
 import pickBy from 'lodash/pickBy';
-import { runPlain } from '../../../../lib/run';
+import { runOutput } from '@backstage/cli-common';
 import { transforms } from './transforms';
 import { version } from '../../../../lib/version';
 import yn from 'yn';
@@ -78,14 +78,14 @@ async function readBuildInfo() {
 
   let commit: string | undefined;
   try {
-    commit = await runPlain('git', 'rev-parse', 'HEAD');
+    commit = await runOutput(['git', 'rev-parse', 'HEAD']);
   } catch (error) {
     // ignore, see below
   }
 
   let gitVersion: string | undefined;
   try {
-    gitVersion = await runPlain('git', 'describe', '--always');
+    gitVersion = await runOutput(['git', 'describe', '--always']);
   } catch (error) {
     // ignore, see below
   }

packages/cli/src/modules/build/lib/packager/createDistWorkspace.ts

@@ -25,7 +25,7 @@ import { tmpdir } from 'os';
 import tar, { CreateOptions, FileOptions } from 'tar';
 import partition from 'lodash/partition';
 import { paths } from '../../../../lib/paths';
-import { run } from '../../../../lib/run';
+import { run } from '@backstage/cli-common';
 import {
   dependencies as cliDependencies,
   devDependencies as cliDevDependencies,
@@ -228,11 +228,11 @@ export async function createDistWorkspace(
       await runParallelWorkers({
         items: customBuild,
         worker: async ({ name, dir, args }) => {
-          await run('yarn', ['run', 'build', ...(args || [])], {
+          await run(['yarn', 'run', 'build', ...(args || [])], {
             cwd: dir,
-            stdoutLogFunc: prefixLogFunc(`${name}: `, 'stdout'),
-            stderrLogFunc: prefixLogFunc(`${name}: `, 'stderr'),
-          });
+            onStdout: prefixLogFunc(`${name}: `, 'stdout'),
+            onStderr: prefixLogFunc(`${name}: `, 'stderr'),
+          }).waitForExit();
         },
       });
     }
@@ -321,9 +321,9 @@ async function moveToDistWorkspace(
     console.log(`Repacking ${target.name} into dist workspace`);
     const archivePath = resolvePath(workspaceDir, archive);
 
-    await run('yarn', ['pack', '--filename', archivePath], {
+    await run(['yarn', 'pack', '--filename', archivePath], {
       cwd: target.dir,
-    });
+    }).waitForExit();
 
     const outputDir = relativePath(paths.targetRoot, target.dir);
     const absoluteOutputPath = resolvePath(workspaceDir, outputDir);

packages/cli/src/modules/info/commands/info.ts

@@ -16,14 +16,14 @@
 
 import { version as cliVersion } from '../../../../package.json';
 import os from 'os';
-import { runPlain } from '../../../lib/run';
+import { runOutput } from '@backstage/cli-common';
 import { paths } from '../../../lib/paths';
 import { Lockfile } from '../../../lib/versioning';
 import fs from 'fs-extra';
 
 export default async () => {
   await new Promise(async () => {
-    const yarnVersion = await runPlain('yarn --version');
+    const yarnVersion = await runOutput(['yarn', '--version']);
     const isLocal = fs.existsSync(paths.resolveOwn('./src'));
 
     const backstageFile = paths.resolveTargetRoot('backstage.json');

packages/cli/src/modules/maintenance/commands/repo/clean.ts

@@ -14,14 +14,11 @@
  * limitations under the License.
  */
 
-import { execFile as execFileCb } from 'child_process';
 import fs from 'fs-extra';
 import { resolve as resolvePath } from 'path';
-import { promisify } from 'util';
 import { PackageGraph } from '@backstage/cli-node';
 import { paths } from '../../../../lib/paths';
-
-const execFile = promisify(execFileCb);
+import { run } from '@backstage/cli-common';
 
 export async function command(): Promise<void> {
   const packages = await PackageGraph.listTargetPackages();
@@ -44,12 +41,9 @@ export async function command(): Promise<void> {
           await fs.remove(resolvePath(pkg.dir, 'dist-types'));
           await fs.remove(resolvePath(pkg.dir, 'coverage'));
         } else if (cleanScript) {
-          const result = await execFile('yarn', ['run', 'clean'], {
+          await run(['yarn', 'run', 'clean'], {
             cwd: pkg.dir,
-            shell: true,
-          });
-          process.stdout.write(result.stdout);
-          process.stderr.write(result.stderr);
+          }).waitForExit();
         }
       }
     }),

packages/cli/src/modules/migrate/commands/packageLintConfigs.ts

@@ -17,7 +17,7 @@
 import fs from 'fs-extra';
 import { resolve as resolvePath } from 'path';
 import { PackageGraph } from '@backstage/cli-node';
-import { runPlain } from '../../../lib/run';
+import { runOutput } from '@backstage/cli-common';
 
 const PREFIX = `module.exports = require('@backstage/cli/config/eslint-factory')`;
 
@@ -84,6 +84,6 @@ export async function command() {
   }
 
   if (hasPrettier) {
-    await runPlain('prettier', '--write', ...configPaths);
+    await runOutput(['prettier', '--write', ...configPaths]);
   }
 }

packages/cli/src/modules/migrate/commands/versions/bump.test.ts

@@ -15,7 +15,7 @@
  */
 import fs from 'fs-extra';
 import { Command } from 'commander';
-import * as runObj from '../../../../lib/run';
+import * as runObj from '@backstage/cli-common';
 import bump, { bumpBackstageJsonVersion, createVersionFinder } from './bump';
 import { registerMswTestHooks, withLogCollector } from '@backstage/test-utils';
 import { YarnInfoInspectData } from '../../../../lib/versioning/packages';
@@ -60,21 +60,22 @@ jest.mock('ora', () => ({
 }));
 
 let mockDir: MockDirectory;
-jest.mock('@backstage/cli-common', () => ({
-  ...jest.requireActual('@backstage/cli-common'),
-  findPaths: () => ({
-    resolveTargetRoot(filename: string) {
-      return mockDir.resolve(filename);
-    },
-    get targetDir() {
-      return mockDir.path;
-    },
-  }),
-}));
-
-jest.mock('../../../../lib/run', () => {
+jest.mock('@backstage/cli-common', () => {
+  const actual = jest.requireActual('@backstage/cli-common');
   return {
-    run: jest.fn(),
+    ...actual,
+    findPaths: () => ({
+      resolveTargetRoot(filename: string) {
+        return mockDir.resolve(filename);
+      },
+      get targetDir() {
+        return mockDir.path;
+      },
+    }),
+    run: jest.fn().mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    }),
   };
 });
 
@@ -184,7 +185,10 @@ describe('bump', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://versions.backstage.io/v1/tags/main/manifest.json',
@@ -222,8 +226,7 @@ describe('bump', () => {
 
     expect(runObj.run).toHaveBeenCalledTimes(1);
     expect(runObj.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+      ['yarn', 'install'],
       expect.any(Object),
     );
 
@@ -277,7 +280,10 @@ describe('bump', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://versions.backstage.io/v1/tags/main/manifest.json',
@@ -318,8 +324,7 @@ describe('bump', () => {
     expect(mockFetchPackageInfo).toHaveBeenCalledWith('@backstage/theme');
 
     expect(runObj.run).not.toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+      ['yarn', 'install'],
       expect.any(Object),
     );
 
@@ -373,7 +378,10 @@ describe('bump', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://versions.backstage.io/v1/tags/main/manifest.json',
@@ -421,8 +429,7 @@ describe('bump', () => {
 
     expect(runObj.run).toHaveBeenCalledTimes(1);
     expect(runObj.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+      ['yarn', 'install'],
       expect.any(Object),
     );
 
@@ -477,7 +484,10 @@ describe('bump', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://versions.backstage.io/v1/tags/main/manifest.json',
@@ -526,14 +536,14 @@ describe('bump', () => {
     expect(mockFetchPackageInfo).toHaveBeenCalledWith('@backstage/core');
 
     expect(runObj.run).toHaveBeenCalledTimes(2);
-    expect(runObj.run).toHaveBeenCalledWith('yarn', [
+    expect(runObj.run).toHaveBeenCalledWith([
+      'yarn',
       'plugin',
       'import',
       'https://versions.backstage.io/v1/releases/0.0.1/yarn-plugin',
     ]);
     expect(runObj.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+      ['yarn', 'install'],
       expect.any(Object),
     );
 
@@ -587,7 +597,10 @@ describe('bump', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://versions.backstage.io/v1/releases/999.0.1/manifest.json',
@@ -656,7 +669,10 @@ describe('bump', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://versions.backstage.io/v1/tags/main/manifest.json',
@@ -763,7 +779,10 @@ describe('bump', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://versions.backstage.io/v1/tags/main/manifest.json',
@@ -813,8 +832,7 @@ describe('bump', () => {
 
     expect(runObj.run).toHaveBeenCalledTimes(1);
     expect(runObj.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+      ['yarn', 'install'],
       expect.any(Object),
     );
 
@@ -873,7 +891,10 @@ describe('bump', () => {
     });
 
     mockFetchPackageInfo.mockRejectedValue(new NotFoundError('Nope'));
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://versions.backstage.io/v1/tags/main/manifest.json',
@@ -1094,7 +1115,10 @@ describe('environment variables', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://custom.example.com/v1/tags/main/manifest.json',
@@ -1131,8 +1155,7 @@ describe('environment variables', () => {
 
     expect(runObj.run).toHaveBeenCalledTimes(1);
     expect(runObj.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+      ['yarn', 'install'],
       expect.any(Object),
     );
 
@@ -1186,7 +1209,10 @@ describe('environment variables', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
 
     const { log: logs } = await withLogCollector(['log', 'warn'], async () => {
       await bump({ pattern: null, release: 'main' } as unknown as Command);
@@ -1215,8 +1241,7 @@ describe('environment variables', () => {
 
     expect(runObj.run).toHaveBeenCalledTimes(1);
     expect(runObj.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+      ['yarn', 'install'],
       expect.any(Object),
     );
 
@@ -1253,7 +1278,10 @@ describe('environment variables', () => {
       },
     });
 
-    jest.spyOn(runObj, 'run').mockResolvedValue(undefined);
+    jest.spyOn(runObj, 'run').mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    } as any);
     worker.use(
       rest.get(
         'https://custom.example.com/v1/tags/main/manifest.json',
@@ -1291,14 +1319,14 @@ describe('environment variables', () => {
     ]);
 
     expect(runObj.run).toHaveBeenCalledTimes(2);
-    expect(runObj.run).toHaveBeenCalledWith('yarn', [
+    expect(runObj.run).toHaveBeenCalledWith([
+      'yarn',
       'plugin',
       'import',
       'https://custom.example.com/v1/releases/1.5.0/yarn-plugin',
     ]);
     expect(runObj.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+      ['yarn', 'install'],
       expect.any(Object),
     );
   });

packages/cli/src/modules/migrate/commands/versions/bump.ts

@@ -41,7 +41,7 @@ import {
 } from '@backstage/release-manifests';
 import { migrateMovedPackages } from './migrate';
 import { runYarnInstall } from '../../lib/utils';
-import { run } from '../../../../lib/run';
+import { run } from '@backstage/cli-common';
 
 const DEP_TYPES = [
   'dependencies',
@@ -135,7 +135,7 @@ export default async (opts: OptionValues) => {
       ? `${env.BACKSTAGE_VERSIONS_BASE_URL}/v1/releases/${releaseManifest.releaseVersion}/yarn-plugin`
       : `https://versions.backstage.io/v1/releases/${releaseManifest.releaseVersion}/yarn-plugin`;
 
-    await run('yarn', ['plugin', 'import', yarnPluginUrl]);
+    await run(['yarn', 'plugin', 'import', yarnPluginUrl]).waitForExit();
     console.log();
   }
 

packages/cli/src/modules/migrate/commands/versions/migrate.test.ts

@@ -17,7 +17,7 @@ import {
   MockDirectory,
   createMockDirectory,
 } from '@backstage/backend-test-utils';
-import * as run from '../../../../lib/run';
+import * as runObj from '@backstage/cli-common';
 import migrate from './migrate';
 import { withLogCollector } from '@backstage/test-utils';
 import fs from 'fs-extra';
@@ -33,21 +33,22 @@ jest.mock('chalk', () => ({
 }));
 
 let mockDir: MockDirectory;
-jest.mock('@backstage/cli-common', () => ({
-  ...jest.requireActual('@backstage/cli-common'),
-  findPaths: () => ({
-    resolveTargetRoot(filename: string) {
-      return mockDir.resolve(filename);
-    },
-    get targetDir() {
-      return mockDir.path;
-    },
-  }),
-}));
-
-jest.mock('../../../../lib/run', () => {
+jest.mock('@backstage/cli-common', () => {
+  const actual = jest.requireActual('@backstage/cli-common');
   return {
-    run: jest.fn(),
+    ...actual,
+    findPaths: () => ({
+      resolveTargetRoot(filename: string) {
+        return mockDir.resolve(filename);
+      },
+      get targetDir() {
+        return mockDir.path;
+      },
+    }),
+    run: jest.fn().mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    }),
   };
 });
 
@@ -58,8 +59,15 @@ function expectLogsToMatch(receivedLogs: String[], expected: String[]): void {
 describe('versions:migrate', () => {
   mockDir = createMockDirectory();
 
+  beforeEach(() => {
+    (runObj.run as jest.Mock).mockReturnValue({
+      exitCode: null,
+      waitForExit: jest.fn().mockResolvedValue(undefined),
+    });
+  });
+
   afterEach(() => {
-    jest.resetAllMocks();
+    (runObj.run as jest.Mock).mockClear();
   });
 
   it('should bump to the moved version when the package is moved', async () => {
@@ -116,8 +124,6 @@ describe('versions:migrate', () => {
       },
     });
 
-    jest.spyOn(run, 'run').mockResolvedValue(undefined);
-
     const { warn, log: logs } = await withLogCollector(async () => {
       await migrate({});
     });
@@ -136,10 +142,9 @@ describe('versions:migrate', () => {
       'Could not find package.json for @backstage/theme@^1.0.0 in b (dependencies)',
     ]);
 
-    expect(run.run).toHaveBeenCalledTimes(1);
-    expect(run.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+    expect(runObj.run).toHaveBeenCalledTimes(1);
+    expect(runObj.run).toHaveBeenCalledWith(
+      ['yarn', 'install'],
       expect.any(Object),
     );
 
@@ -227,16 +232,13 @@ describe('versions:migrate', () => {
       },
     });
 
-    jest.spyOn(run, 'run').mockResolvedValue(undefined);
-
     await withLogCollector(async () => {
       await migrate({});
     });
 
-    expect(run.run).toHaveBeenCalledTimes(1);
-    expect(run.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+    expect(runObj.run).toHaveBeenCalledTimes(1);
+    expect(runObj.run).toHaveBeenCalledWith(
+      ['yarn', 'install'],
       expect.any(Object),
     );
 
@@ -259,7 +261,7 @@ describe('versions:migrate', () => {
     );
   });
 
-  it('should replaces the occurrences of changed packages, and is careful', async () => {
+  it('should replace occurrences of changed packages, and is careful', async () => {
     mockDir.setContent({
       'package.json': JSON.stringify({
         workspaces: {
@@ -314,16 +316,13 @@ describe('versions:migrate', () => {
       },
     });
 
-    jest.spyOn(run, 'run').mockResolvedValue(undefined);
-
     await withLogCollector(async () => {
       await migrate({});
     });
 
-    expect(run.run).toHaveBeenCalledTimes(1);
-    expect(run.run).toHaveBeenCalledWith(
-      'yarn',
-      ['install'],
+    expect(runObj.run).toHaveBeenCalledTimes(1);
+    expect(runObj.run).toHaveBeenCalledWith(
+      ['yarn', 'install'],
       expect.any(Object),
     );
 

packages/cli/src/modules/migrate/lib/utils.ts

@@ -16,7 +16,7 @@
 
 import ora from 'ora';
 import chalk from 'chalk';
-import { run } from '../../../lib/run';
+import { run } from '@backstage/cli-common';
 
 export async function runYarnInstall() {
   const spinner = ora({
@@ -27,7 +27,7 @@ export async function runYarnInstall() {
 
   const installOutput = new Array<Buffer>();
   try {
-    await run('yarn', ['install'], {
+    await run(['yarn', 'install'], {
       env: {
         FORCE_COLOR: 'true',
         // We filter out all of the npm_* environment variables that are added when
@@ -39,9 +39,9 @@ export async function runYarnInstall() {
           ),
         ),
       },
-      stdoutLogFunc: data => installOutput.push(data),
-      stderrLogFunc: data => installOutput.push(data),
-    });
+      onStdout: data => installOutput.push(data),
+      onStderr: data => installOutput.push(data),
+    }).waitForExit();
     spinner.succeed();
   } catch (error) {
     spinner.fail();

packages/cli/src/modules/new/lib/execution/executePortableTemplate.ts

@@ -24,6 +24,7 @@ import {
 } from '../types';
 import { installNewPackage } from './installNewPackage';
 import { writeTemplateContents } from './writeTemplateContents';
+import { run } from '@backstage/cli-common';
 
 type ExecuteNewTemplateOptions = {
   config: PortableTemplateConfig;
@@ -54,14 +55,25 @@ export async function executePortableTemplate(
     }
 
     if (!options.skipInstall) {
-      await Task.forCommand('yarn install', {
-        cwd: targetDir,
-        optional: true,
-      });
-      await Task.forCommand('yarn lint --fix', {
-        cwd: targetDir,
-        optional: true,
-      });
+      for (const command of [
+        ['yarn', 'install'],
+        ['yarn', 'lint', '--fix'],
+      ]) {
+        const commandStr = command.join(' ');
+        try {
+          await Task.forItem('executing', commandStr, async () => {
+            await run(command, {
+              cwd: targetDir,
+              stdio: 'ignore',
+            }).waitForExit();
+          });
+        } catch (error) {
+          assertError(error);
+          Task.error(
+            `Warning: Failed to execute command '${commandStr}', ${error}`,
+          );
+        }
+      }
     }
 
     Task.log();

packages/cli/src/modules/new/lib/tasks.ts

@@ -16,11 +16,6 @@
 
 import chalk from 'chalk';
 import ora from 'ora';
-import { promisify } from 'util';
-import { exec as execCb } from 'child_process';
-import { assertError } from '@backstage/errors';
-
-const exec = promisify(execCb);
 
 const TASK_NAME_MAX_LENGTH = 14;
 
@@ -64,30 +59,4 @@ export class Task {
       throw error;
     }
   }
-
-  static async forCommand(
-    command: string,
-    options?: { cwd?: string; optional?: boolean },
-  ) {
-    try {
-      await Task.forItem('executing', command, async () => {
-        await exec(command, { cwd: options?.cwd });
-      });
-    } catch (error) {
-      assertError(error);
-      if (error.stderr) {
-        process.stderr.write(error.stderr as Buffer);
-      }
-      if (error.stdout) {
-        process.stdout.write(error.stdout as Buffer);
-      }
-      if (options?.optional) {
-        Task.error(`Warning: Failed to execute command ${chalk.cyan(command)}`);
-      } else {
-        throw new Error(
-          `Failed to execute command '${chalk.cyan(command)}', ${error}`,
-        );
-      }
-    }
-  }
 }

packages/cli/src/modules/test/commands/package/test.ts

@@ -16,7 +16,7 @@
 
 import { Command, OptionValues } from 'commander';
 import { paths } from '../../../../lib/paths';
-import { runCheck } from '../../../../lib/run';
+import { runCheck } from '@backstage/cli-common';
 
 function includesAnyOf(hayStack: string[], ...needles: string[]) {
   for (const needle of needles) {
@@ -55,8 +55,8 @@ export default async (_opts: OptionValues, cmd: Command) => {
     !includesAnyOf(args, '--watch', '--watchAll')
   ) {
     const isGitRepo = () =>
-      runCheck('git', 'rev-parse', '--is-inside-work-tree');
-    const isMercurialRepo = () => runCheck('hg', '--cwd', '.', 'root');
+      runCheck(['git', 'rev-parse', '--is-inside-work-tree']);
+    const isMercurialRepo = () => runCheck(['hg', '--cwd', '.', 'root']);
 
     if ((await isGitRepo()) || (await isMercurialRepo())) {
       args.push('--watch');

packages/cli/src/modules/test/commands/repo/test.ts

@@ -22,7 +22,7 @@ import { relative as relativePath } from 'path';
 import { Command, OptionValues } from 'commander';
 import { Lockfile, PackageGraph } from '@backstage/cli-node';
 import { paths } from '../../../../lib/paths';
-import { runCheck, runPlain } from '../../../../lib/run';
+import { runCheck, runOutput } from '@backstage/cli-common';
 import { isChildPath } from '@backstage/cli-common';
 import { SuccessCache } from '../../../../lib/cache/SuccessCache';
 
@@ -63,14 +63,14 @@ async function readPackageTreeHashes(graph: PackageGraph) {
     ...pkg,
     path: relativePath(paths.targetRoot, pkg.dir),
   }));
-  const output = await runPlain(
+  const output = await runOutput([
     'git',
     'ls-tree',
-    '--format="%(objectname)=%(path)"',
+    '--format=%(objectname)=%(path)',
     'HEAD',
     '--',
     ...pkgs.map(pkg => pkg.path),
-  );
+  ]);
 
   const map = new Map(
     output
@@ -175,8 +175,8 @@ export async function command(opts: OptionValues, cmd: Command): Promise<void> {
     !hasFlags('--coverage', '--watch', '--watchAll')
   ) {
     const isGitRepo = () =>
-      runCheck('git', 'rev-parse', '--is-inside-work-tree');
-    const isMercurialRepo = () => runCheck('hg', '--cwd', '.', 'root');
+      runCheck(['git', 'rev-parse', '--is-inside-work-tree']);
+    const isMercurialRepo = () => runCheck(['hg', '--cwd', '.', 'root']);
 
     if ((await isGitRepo()) || (await isMercurialRepo())) {
       isSingleWatchMode = true;

packages/codemods/src/action.ts

@@ -15,11 +15,9 @@
  */
 
 import { relative as relativePath } from 'path';
-import { spawn } from 'child_process';
 import { OptionValues } from 'commander';
-import { findPaths } from '@backstage/cli-common';
+import { findPaths, run } from '@backstage/cli-common';
 import { platform } from 'os';
-import { ExitCodeError } from './errors';
 
 // eslint-disable-next-line no-restricted-syntax
 const paths = findPaths(__dirname);
@@ -51,42 +49,25 @@ export function createCodemodAction(name: string) {
 
     console.log(`Running jscodeshift with these arguments: ${args.join(' ')}`);
 
-    let command;
+    let commandArgs: string[];
     if (platform() === 'win32') {
-      command = 'jscodeshift';
+      commandArgs = ['jscodeshift', ...args];
     } else {
       // jscodeshift ships a slightly broken bin script with windows
       // line endings so we need to execute it using node rather than
       // letting the `#!/usr/bin/env node` take care of it
-      command = process.argv0;
-      args.unshift(require.resolve('.bin/jscodeshift'));
+      commandArgs = [
+        process.argv0,
+        require.resolve('.bin/jscodeshift'),
+        ...args,
+      ];
     }
 
-    const child = spawn(command, args, {
-      stdio: 'inherit',
-      shell: true,
+    await run(commandArgs, {
       env: {
         ...process.env,
         FORCE_COLOR: 'true',
       },
-    });
-
-    if (typeof child.exitCode === 'number') {
-      if (child.exitCode) {
-        throw new ExitCodeError(child.exitCode, name);
-      }
-      return;
-    }
-
-    await new Promise<void>((resolve, reject) => {
-      child.once('error', error => reject(error));
-      child.once('exit', code => {
-        if (code) {
-          reject(new ExitCodeError(code, name));
-        } else {
-          resolve();
-        }
-      });
-    });
+    }).waitForExit();
   };
 }

packages/codemods/src/errors.ts

@@ -15,6 +15,7 @@
  */
 
 import chalk from 'chalk';
+import { ExitCodeError } from '@backstage/cli-common';
 
 export class CustomError extends Error {
   get name(): string {
@@ -22,19 +23,6 @@ export class CustomError extends Error {
   }
 }
 
-export class ExitCodeError extends CustomError {
-  readonly code: number;
-
-  constructor(code: number, command?: string) {
-    if (command) {
-      super(`Command '${command}' exited with code ${code}`);
-    } else {
-      super(`Child exited with code ${code}`);
-    }
-    this.code = code;
-  }
-}
-
 export function exitWithError(error: Error): never {
   if (error instanceof ExitCodeError) {
     process.stderr.write(`\n${chalk.red(error.message)}\n\n`);

packages/e2e-test/src/commands/index.ts

@@ -15,12 +15,12 @@
  */
 
 import { Command } from 'commander';
-import { run } from './run';
+import { runCommand } from './runCommand';
 
 export function registerCommands(program: Command) {
   program
     .command('run')
     .option('--keep', 'Do not remove the temporary dir after tests complete')
     .description('Run e2e tests')
-    .action(run);
+    .action(runCommand);
 }

packages/e2e-test/src/commands/runCommand.ts

@@ -22,18 +22,12 @@ import killTree from 'tree-kill';
 import { resolve as resolvePath, join as joinPath } from 'path';
 import path from 'path';
 
-import {
-  spawnPiped,
-  runPlain,
-  waitFor,
-  waitForExit,
-  print,
-} from '../lib/helpers';
+import { waitFor, print } from '../lib/helpers';
 
 import mysql from 'mysql2/promise';
 import pgtools from 'pgtools';
 
-import { findPaths } from '@backstage/cli-common';
+import { findPaths, runOutput, run } from '@backstage/cli-common';
 import { OptionValues } from 'commander';
 
 // eslint-disable-next-line no-restricted-syntax
@@ -46,7 +40,7 @@ const templatePackagePaths = [
   'packages/create-app/templates/default-app/packages/backend/package.json.hbs',
 ];
 
-export async function run(opts: OptionValues) {
+export async function runCommand(opts: OptionValues) {
   const rootDir = await fs.mkdtemp(resolvePath(os.tmpdir(), 'backstage-e2e-'));
   print(`CLI E2E test root: ${rootDir}\n`);
 
@@ -67,7 +61,7 @@ export async function run(opts: OptionValues) {
   await createPlugin({ appDir, pluginId, select: 'backend-plugin' });
 
   print(`Running 'yarn test:e2e' in newly created app with new plugin`);
-  await runPlain(['yarn', 'test:e2e'], {
+  await runOutput(['yarn', 'test:e2e'], {
     cwd: appDir,
     env: { ...process.env, CI: undefined },
   });
@@ -75,13 +69,13 @@ export async function run(opts: OptionValues) {
   await switchToReact17(appDir);
 
   print(`Running 'yarn install' to install React 17`);
-  await runPlain(['yarn', 'install'], { cwd: appDir });
+  await runOutput(['yarn', 'install'], { cwd: appDir });
 
   print(`Running 'yarn tsc' with React 17`);
-  await runPlain(['yarn', 'tsc'], { cwd: appDir });
+  await runOutput(['yarn', 'tsc'], { cwd: appDir });
 
   print(`Running 'yarn test:e2e' with React 17`);
-  await runPlain(['yarn', 'test:e2e'], {
+  await runOutput(['yarn', 'test:e2e'], {
     cwd: appDir,
     env: { ...process.env, CI: undefined },
   });
@@ -190,7 +184,7 @@ async function buildDistWorkspace(workspaceName: string, rootDir: string) {
   appendDeps(require('@backstage/create-app/package.json'));
 
   print(`Preparing workspace`);
-  await runPlain([
+  await runOutput([
     'yarn',
     'backstage-cli',
     'build-workspace',
@@ -209,7 +203,7 @@ async function buildDistWorkspace(workspaceName: string, rootDir: string) {
   }
 
   print('Installing workspace dependencies');
-  await runPlain(['yarn', 'workspaces', 'focus', '--all', '--production'], {
+  await runOutput(['yarn', 'workspaces', 'focus', '--all', '--production'], {
     cwd: workspaceDir,
   });
 
@@ -257,7 +251,7 @@ async function createApp(
   workspaceDir: string,
   rootDir: string,
 ) {
-  const child = spawnPiped(
+  const child = run(
     [
       'node',
       resolvePath(workspaceDir, 'packages/create-app/bin/backstage-create-app'),
@@ -265,6 +259,7 @@ async function createApp(
     ],
     {
       cwd: rootDir,
+      stdio: ['pipe', 'pipe', 'pipe'],
     },
   );
 
@@ -278,7 +273,7 @@ async function createApp(
     child.stdin?.write(`${appName}\n`);
 
     print('Waiting for app create script to be done');
-    await waitForExit(child);
+    await child.waitForExit();
 
     const appDir = resolvePath(rootDir, appName);
 
@@ -318,7 +313,7 @@ async function createApp(
       'test:all',
     ]) {
       print(`Running 'yarn ${cmd}' in newly created app`);
-      await runPlain(['yarn', cmd], { cwd: appDir });
+      await runOutput(['yarn', cmd], { cwd: appDir });
     }
 
     return appDir;
@@ -378,10 +373,11 @@ async function createPlugin(options: {
   select: string;
 }) {
   const { appDir, pluginId, select } = options;
-  const child = spawnPiped(
+  const child = run(
     ['yarn', 'new', '--select', select, '--option', `pluginId=${pluginId}`],
     {
       cwd: appDir,
+      stdio: ['pipe', 'pipe', 'pipe'],
     },
   );
 
@@ -392,7 +388,7 @@ async function createPlugin(options: {
     });
 
     print('Waiting for plugin create script to be done');
-    await waitForExit(child);
+    await child.waitForExit();
 
     const pluginDir = resolvePath(
       appDir,
@@ -401,11 +397,11 @@ async function createPlugin(options: {
     );
 
     print(`Running 'yarn tsc' in root for newly created plugin`);
-    await runPlain(['yarn', 'tsc'], { cwd: appDir });
+    await runOutput(['yarn', 'tsc'], { cwd: appDir });
 
     for (const cmd of [['lint'], ['test', '--no-watch']]) {
       print(`Running 'yarn ${cmd.join(' ')}' in newly created plugin`);
-      await runPlain(['yarn', ...cmd], { cwd: pluginDir });
+      await runOutput(['yarn', ...cmd], { cwd: pluginDir });
     }
   } finally {
     child.kill();
@@ -494,7 +490,7 @@ async function dropClientDatabases(client: string) {
  * Start serving the newly created backend and make sure that all db migrations works correctly
  */
 async function testBackendStart(appDir: string, ...args: string[]) {
-  const child = spawnPiped(['yarn', 'workspace', 'backend', 'start', ...args], {
+  const child = run(['yarn', 'workspace', 'backend', 'start', ...args], {
     cwd: appDir,
     // Windows does not like piping stdin here, the child process will hang when requiring the 'process' module
     stdio: ['ignore', 'pipe', 'pipe'],
@@ -586,7 +582,7 @@ async function testBackendStart(appDir: string, ...args: string[]) {
   }
 
   try {
-    await waitForExit(child);
+    await child.waitForExit();
   } catch (error) {
     if (!successful) {
       throw new Error(`Backend failed to startup: ${stderr}`);

packages/e2e-test/src/index.ts

@@ -18,7 +18,6 @@ import { program } from 'commander';
 import chalk from 'chalk';
 import { registerCommands } from './commands';
 import { version } from '../package.json';
-import { exitWithError } from './lib/helpers';
 
 async function main(argv: string[]) {
   program.name('e2e-test').version(version);
@@ -36,4 +35,12 @@ async function main(argv: string[]) {
   program.parse(argv);
 }
 
-main(process.argv).catch(exitWithError);
+main(process.argv).catch(err => {
+  process.stdout.write(`${err.name}: ${err.stack || err.message}\n`);
+
+  if (typeof err.code === 'number') {
+    process.exit(err.code);
+  } else {
+    process.exit(1);
+  }
+});

packages/e2e-test/src/lib/helpers.ts

@@ -14,77 +14,6 @@
  * limitations under the License.
  */
 
-import { assertError } from '@backstage/errors';
-import {
-  spawn,
-  execFile as execFileCb,
-  SpawnOptions,
-  ChildProcess,
-} from 'child_process';
-import { promisify } from 'util';
-
-const execFile = promisify(execFileCb);
-
-export function spawnPiped(cmd: string[], options?: SpawnOptions) {
-  function pipeWithPrefix(stream: NodeJS.WriteStream, prefix = '') {
-    return (data: Buffer) => {
-      const prefixedMsg = data
-        .toString('utf8')
-        .trimEnd()
-        .replace(/^/gm, prefix);
-      stream.write(`${prefixedMsg}\n`, 'utf8');
-    };
-  }
-
-  const child = spawn(cmd[0], cmd.slice(1), {
-    stdio: 'pipe',
-    shell: true,
-    ...options,
-  });
-  child.on('error', exitWithError);
-
-  const logPrefix = cmd.map(s => s.replace(/.+\//, '')).join(' ');
-  child.stdout?.on(
-    'data',
-    pipeWithPrefix(process.stdout, `[${logPrefix}].out: `),
-  );
-  child.stderr?.on(
-    'data',
-    pipeWithPrefix(process.stderr, `[${logPrefix}].err: `),
-  );
-
-  return child;
-}
-
-export async function runPlain(cmd: string[], options?: SpawnOptions) {
-  try {
-    const { stdout } = await execFile(cmd[0], cmd.slice(1), {
-      ...options,
-      shell: true,
-    });
-    return stdout.trim();
-  } catch (error) {
-    assertError(error);
-    if (error.stdout) {
-      process.stdout.write(error.stdout as Buffer);
-    }
-    if (error.stderr) {
-      process.stderr.write(error.stderr as Buffer);
-    }
-    throw error;
-  }
-}
-
-export function exitWithError(err: Error & { code?: unknown }) {
-  process.stdout.write(`${err.name}: ${err.stack || err.message}\n`);
-
-  if (typeof err.code === 'number') {
-    process.exit(err.code);
-  } else {
-    process.exit(1);
-  }
-}
-
 /**
  * Waits for fn() to be true
  * Checks every 100ms
@@ -108,22 +37,6 @@ export function waitFor(fn: () => boolean, maxSeconds: number = 120) {
   });
 }
 
-export async function waitForExit(child: ChildProcess) {
-  if (child.exitCode !== null) {
-    throw new Error(`Child already exited with code ${child.exitCode}`);
-  }
-  await new Promise<void>((resolve, reject) =>
-    child.once('exit', code => {
-      if (code) {
-        reject(new Error(`Child exited with code ${code}`));
-      } else {
-        print('Child finished');
-        resolve();
-      }
-    }),
-  );
-}
-
 export function print(msg: string) {
   return process.stdout.write(`${msg}\n`);
 }

packages/repo-tools/src/commands/api-reports/api-reports/generateTypeDeclarations.ts

@@ -15,7 +15,7 @@
  */
 
 import fs from 'fs-extra';
-import { spawnSync } from 'child_process';
+import { run, ExitCodeError } from '@backstage/cli-common';
 import { paths as cliPaths } from '../../../lib/paths';
 
 /**
@@ -30,21 +30,26 @@ import { paths as cliPaths } from '../../../lib/paths';
  */
 export async function generateTypeDeclarations(tsconfigFilePath: string) {
   await fs.remove(cliPaths.resolveTargetRoot('dist-types'));
-  const { status } = spawnSync(
-    'yarn',
-    [
-      'tsc',
-      ['--project', tsconfigFilePath],
-      ['--skipLibCheck', 'false'],
-      ['--incremental', 'false'],
-    ].flat(),
-    {
-      stdio: 'inherit',
-      shell: true,
-      cwd: cliPaths.targetRoot,
-    },
-  );
-  if (status !== 0) {
-    process.exit(status || undefined);
+  try {
+    await run(
+      [
+        'yarn',
+        'tsc',
+        '--project',
+        tsconfigFilePath,
+        '--skipLibCheck',
+        'false',
+        '--incremental',
+        'false',
+      ],
+      {
+        cwd: cliPaths.targetRoot,
+      },
+    ).waitForExit();
+  } catch (error) {
+    if (error instanceof ExitCodeError) {
+      process.exit(error.code);
+    }
+    throw error;
   }
 }

packages/techdocs-cli/src/commands/serve/mkdocs.ts

@@ -18,7 +18,7 @@ import { OptionValues } from 'commander';
 import openBrowser from 'react-dev-utils/openBrowser';
 import { createLogger } from '../../lib/utility';
 import { runMkdocsServer } from '../../lib/mkdocsServer';
-import { LogFunc, waitForSignal } from '../../lib/run';
+import { RunOnOutput } from '@backstage/cli-common';
 import { getMkdocsYml } from '@backstage/plugin-techdocs-node';
 import fs from 'fs-extra';
 import { checkIfDockerIsOperational } from './utils';
@@ -45,7 +45,7 @@ export default async function serveMkdocs(opts: OptionValues) {
   // We want to open browser only once based on a log.
   let boolOpenBrowserTriggered = false;
 
-  const logFunc: LogFunc = data => {
+  const logFunc: RunOnOutput = data => {
     // Sometimes the lines contain an unnecessary extra new line in between
     const logLines = data.toString().split('\n');
     const logPrefix = opts.docker ? '[docker/mkdocs]' : '[mkdocs]';
@@ -74,18 +74,18 @@ export default async function serveMkdocs(opts: OptionValues) {
   // Had me questioning this whole implementation for half an hour.
 
   // Commander stores --no-docker in cmd.docker variable
-  const childProcess = await runMkdocsServer({
+  const childProcess = runMkdocsServer({
     port: opts.port,
     dockerImage: opts.dockerImage,
     dockerEntrypoint: opts.dockerEntrypoint,
     dockerOptions: opts.dockerOption,
     useDocker: opts.docker,
-    stdoutLogFunc: logFunc,
-    stderrLogFunc: logFunc,
+    onStdout: logFunc,
+    onStderr: logFunc,
   });
 
   // Keep waiting for user to cancel the process
-  await waitForSignal([childProcess]);
+  await childProcess.waitForExit();
 
   if (configIsTemporary) {
     process.on('exit', async () => {

packages/techdocs-cli/src/commands/serve/serve.ts

@@ -17,10 +17,9 @@
 import { OptionValues } from 'commander';
 import path from 'path';
 import openBrowser from 'react-dev-utils/openBrowser';
-import { findPaths } from '@backstage/cli-common';
+import { findPaths, RunOnOutput } from '@backstage/cli-common';
 import HTTPServer from '../../lib/httpServer';
 import { runMkdocsServer } from '../../lib/mkdocsServer';
-import { LogFunc, waitForSignal } from '../../lib/run';
 import { createLogger } from '../../lib/utility';
 import { getMkdocsYml } from '@backstage/plugin-techdocs-node';
 import fs from 'fs-extra';
@@ -83,7 +82,7 @@ export default async function serve(opts: OptionValues) {
   }
 
   let mkdocsServerHasStarted = false;
-  const mkdocsLogFunc: LogFunc = data => {
+  const mkdocsLogFunc: RunOnOutput = data => {
     // Sometimes the lines contain an unnecessary extra new line
     const logLines = data.toString().split('\n');
     const logPrefix = opts.docker ? '[docker/mkdocs]' : '[mkdocs]';
@@ -107,14 +106,14 @@ export default async function serve(opts: OptionValues) {
   // https://github.com/mkdocs/mkdocs/issues/879#issuecomment-203536006
   // Had me questioning this whole implementation for half an hour.
   logger.info('Starting mkdocs server.');
-  const mkdocsChildProcess = await runMkdocsServer({
+  const mkdocsChildProcess = runMkdocsServer({
     port: opts.mkdocsPort,
     dockerImage: opts.dockerImage,
     dockerEntrypoint: opts.dockerEntrypoint,
     dockerOptions: opts.dockerOption,
     useDocker: opts.docker,
-    stdoutLogFunc: mkdocsLogFunc,
-    stderrLogFunc: mkdocsLogFunc,
+    onStdout: mkdocsLogFunc,
+    onStderr: mkdocsLogFunc,
     mkdocsConfigFileName: mkdocsYmlPath,
     mkdocsParameterClean: opts.mkdocsParameterClean,
     mkdocsParameterDirtyReload: opts.mkdocsParameterDirtyreload,
@@ -161,7 +160,7 @@ export default async function serve(opts: OptionValues) {
       );
     });
 
-  await waitForSignal([mkdocsChildProcess]);
+  await mkdocsChildProcess.waitForExit();
 
   if (configIsTemporary) {
     process.on('exit', async () => {

packages/techdocs-cli/src/commands/serve/utils.ts

@@ -14,25 +14,22 @@
  * limitations under the License.
  */
 
-import { promisify } from 'util';
 import * as winston from 'winston';
-import { execFile } from 'child_process';
+import { runCheck } from '@backstage/cli-common';
 
 export async function checkIfDockerIsOperational(
   logger: winston.Logger,
 ): Promise<boolean> {
   logger.info('Checking Docker status...');
-  try {
-    const runCheck = promisify(execFile);
-    await runCheck('docker', ['info'], { shell: true });
+  const isOperational = await runCheck(['docker', 'info']);
+  if (isOperational) {
     logger.info(
       'Docker is up and running. Proceed to starting up mkdocs server',
     );
     return true;
-  } catch {
-    logger.error(
-      'Docker is not running. Exiting. Please check status of Docker daemon with `docker info` before re-running',
-    );
-    return false;
   }
+  logger.error(
+    'Docker is not running. Exiting. Please check status of Docker daemon with `docker info` before re-running',
+  );
+  return false;
 }

packages/techdocs-cli/src/lib/mkdocsServer.test.ts

@@ -15,9 +15,9 @@
  */
 
 import { runMkdocsServer } from './mkdocsServer';
-import { run } from './run';
+import { run } from '@backstage/cli-common';
 
-jest.mock('./run', () => {
+jest.mock('@backstage/cli-common', () => {
   return {
     run: jest.fn(),
   };
@@ -29,12 +29,12 @@ describe('runMkdocsServer', () => {
   });
 
   describe('docker', () => {
-    it('should run docker directly by default', async () => {
-      await runMkdocsServer({});
+    it('should run docker directly by default', () => {
+      runMkdocsServer({});
 
       expect(run).toHaveBeenCalledWith(
-        'docker',
         expect.arrayContaining([
+          'docker',
           'run',
           `${process.cwd()}:/content`,
           '8000:8000',
@@ -47,26 +47,24 @@ describe('runMkdocsServer', () => {
       );
     });
 
-    it('should accept port option', async () => {
-      await runMkdocsServer({ port: '5678' });
+    it('should accept port option', () => {
+      runMkdocsServer({ port: '5678' });
       expect(run).toHaveBeenCalledWith(
-        'docker',
-        expect.arrayContaining(['5678:5678', '0.0.0.0:5678']),
+        expect.arrayContaining(['docker', '5678:5678', '0.0.0.0:5678']),
         expect.objectContaining({}),
       );
     });
 
-    it('should accept custom docker image', async () => {
-      await runMkdocsServer({ dockerImage: 'my-org/techdocs' });
+    it('should accept custom docker image', () => {
+      runMkdocsServer({ dockerImage: 'my-org/techdocs' });
       expect(run).toHaveBeenCalledWith(
-        'docker',
-        expect.arrayContaining(['my-org/techdocs']),
+        expect.arrayContaining(['docker', 'my-org/techdocs']),
         expect.objectContaining({}),
       );
     });
 
-    it('should accept custom docker options', async () => {
-      await runMkdocsServer({
+    it('should accept custom docker options', () => {
+      runMkdocsServer({
         dockerOptions: [
           '--add-host=internal.host:192.168.11.12',
           '--name',
@@ -75,8 +73,8 @@ describe('runMkdocsServer', () => {
       });
 
       expect(run).toHaveBeenCalledWith(
-        'docker',
         expect.arrayContaining([
+          'docker',
           'run',
           '--rm',
           '-w',
@@ -98,14 +96,14 @@ describe('runMkdocsServer', () => {
       );
     });
 
-    it('should accept additinoal mkdocs CLI parameters', async () => {
-      await runMkdocsServer({
+    it('should accept additinoal mkdocs CLI parameters', () => {
+      runMkdocsServer({
         mkdocsParameterClean: true,
         mkdocsParameterStrict: true,
       });
       expect(run).toHaveBeenCalledWith(
-        'docker',
         expect.arrayContaining([
+          'docker',
           'serve',
           '--dev-addr',
           '0.0.0.0:8000',
@@ -118,21 +116,24 @@ describe('runMkdocsServer', () => {
   });
 
   describe('mkdocs', () => {
-    it('should run mkdocs if specified', async () => {
-      await runMkdocsServer({ useDocker: false });
+    it('should run mkdocs if specified', () => {
+      runMkdocsServer({ useDocker: false });
 
       expect(run).toHaveBeenCalledWith(
-        'mkdocs',
-        expect.arrayContaining(['serve', '--dev-addr', '127.0.0.1:8000']),
+        expect.arrayContaining([
+          'mkdocs',
+          'serve',
+          '--dev-addr',
+          '127.0.0.1:8000',
+        ]),
         expect.objectContaining({}),
       );
     });
 
-    it('should accept port option', async () => {
-      await runMkdocsServer({ useDocker: false, port: '5678' });
+    it('should accept port option', () => {
+      runMkdocsServer({ useDocker: false, port: '5678' });
       expect(run).toHaveBeenCalledWith(
-        'mkdocs',
-        expect.arrayContaining(['127.0.0.1:5678']),
+        expect.arrayContaining(['mkdocs', '127.0.0.1:5678']),
         expect.objectContaining({}),
       );
     });

packages/techdocs-cli/src/lib/mkdocsServer.ts

@@ -14,30 +14,29 @@
  * limitations under the License.
  */
 
-import { ChildProcess } from 'child_process';
-import { run, LogFunc } from './run';
+import { run, RunChildProcess, RunOnOutput } from '@backstage/cli-common';
 
-export const runMkdocsServer = async (options: {
+export const runMkdocsServer = (options: {
   port?: string;
   useDocker?: boolean;
   dockerImage?: string;
   dockerEntrypoint?: string;
   dockerOptions?: string[];
-  stdoutLogFunc?: LogFunc;
-  stderrLogFunc?: LogFunc;
+  onStdout?: RunOnOutput;
+  onStderr?: RunOnOutput;
   mkdocsConfigFileName?: string;
   mkdocsParameterClean?: boolean;
   mkdocsParameterDirtyReload?: boolean;
   mkdocsParameterStrict?: boolean;
-}): Promise<ChildProcess> => {
+}): RunChildProcess => {
   const port = options.port ?? '8000';
   const useDocker = options.useDocker ?? true;
   const dockerImage = options.dockerImage ?? 'spotify/techdocs';
 
   if (useDocker) {
-    return await run(
-      'docker',
+    return run(
       [
+        'docker',
         'run',
         '--rm',
         '-w',
@@ -63,15 +62,15 @@ export const runMkdocsServer = async (options: {
         ...(options.mkdocsParameterStrict ? ['--strict'] : []),
       ],
       {
-        stdoutLogFunc: options.stdoutLogFunc,
-        stderrLogFunc: options.stderrLogFunc,
+        onStdout: options.onStdout,
+        onStderr: options.onStderr,
       },
     );
   }
 
-  return await run(
-    'mkdocs',
+  return run(
     [
+      'mkdocs',
       'serve',
       '--dev-addr',
       `127.0.0.1:${port}`,
@@ -83,8 +82,8 @@ export const runMkdocsServer = async (options: {
       ...(options.mkdocsParameterStrict ? ['--strict'] : []),
     ],
     {
-      stdoutLogFunc: options.stdoutLogFunc,
-      stderrLogFunc: options.stderrLogFunc,
+      onStdout: options.onStdout,
+      onStderr: options.onStderr,
     },
   );
 };

packages/techdocs-cli/src/lib/run.ts

@@ -1,99 +0,0 @@
-/*
- * Copyright 2020 The Backstage Authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import { spawn, SpawnOptions, ChildProcess } from 'child_process';
-
-export type LogFunc = (data: Buffer | string) => void;
-type SpawnOptionsPartialEnv = Omit<SpawnOptions, 'env'> & {
-  env?: Partial<NodeJS.ProcessEnv>;
-  // Pipe stdout to this log function
-  stdoutLogFunc?: LogFunc;
-  // Pipe stderr to this log function
-  stderrLogFunc?: LogFunc;
-};
-
-// TODO: Accept log functions to pipe logs with.
-// Runs a child command, returning the child process instance.
-// Use it along with waitForSignal to run a long running process e.g. mkdocs serve
-export const run = async (
-  name: string,
-  args: string[] = [],
-  options: SpawnOptionsPartialEnv = {},
-): Promise<ChildProcess> => {
-  const { stdoutLogFunc, stderrLogFunc } = options;
-
-  const env: NodeJS.ProcessEnv = {
-    ...process.env,
-    FORCE_COLOR: 'true',
-    ...(options.env ?? {}),
-  };
-
-  // Refer: https://nodejs.org/api/child_process.html#child_process_subprocess_stdio
-  const stdio = [
-    'inherit',
-    stdoutLogFunc ? 'pipe' : 'inherit',
-    stderrLogFunc ? 'pipe' : 'inherit',
-  ] as ('inherit' | 'pipe')[];
-
-  const childProcess = spawn(name, args, {
-    stdio: stdio,
-    ...options,
-    env,
-  });
-
-  if (stdoutLogFunc && childProcess.stdout) {
-    childProcess.stdout.on('data', stdoutLogFunc);
-  }
-  if (stderrLogFunc && childProcess.stderr) {
-    childProcess.stderr.on('data', stderrLogFunc);
-  }
-
-  return childProcess;
-};
-
-// Block indefinitely and wait for a signal to stop the child process(es)
-// Throw error if any child process errors
-// Resolves only when all processes exit with status code 0
-export async function waitForSignal(
-  childProcesses: Array<ChildProcess>,
-): Promise<void> {
-  const promises: Array<Promise<void>> = [];
-
-  for (const signal of ['SIGINT', 'SIGTERM'] as const) {
-    process.on(signal, () => {
-      childProcesses.forEach(childProcess => {
-        childProcess.kill();
-      });
-    });
-  }
-
-  childProcesses.forEach(childProcess => {
-    if (typeof childProcess.exitCode === 'number') {
-      if (childProcess.exitCode) {
-        throw new Error(`Non zero exit code from child process`);
-      }
-      return;
-    }
-
-    promises.push(
-      new Promise<void>((resolve, reject) => {
-        childProcess.once('error', reject);
-        childProcess.once('exit', resolve);
-      }),
-    );
-  });
-
-  await Promise.all(promises);
-}

yarn.lock

@@ -3140,7 +3140,10 @@ __metadata:
   resolution: "@backstage/cli-common@workspace:packages/cli-common"
   dependencies:
     "@backstage/cli": "workspace:^"
+    "@backstage/errors": "workspace:^"
+    "@types/cross-spawn": "npm:^6.0.2"
     "@types/node": "npm:^20.16.0"
+    cross-spawn: "npm:^7.0.3"
     global-agent: "npm:^3.0.0"
     undici: "npm:^7.2.3"
   languageName: unknown