cli-node: update to use new run utils from cli-common

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/afraid-items-drum.md

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

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();
-      }
-    });
-  });
-}