From 4e8c7261e92879429c733442e86cd9009c9d105d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 29 Nov 2025 14:48:22 +0100 Subject: [PATCH] cli-node: update to use new run utils from cli-common Signed-off-by: Patrik Oldsberg --- .changeset/afraid-items-drum.md | 5 + packages/cli-node/src/git/GitUtils.ts | 20 ++-- .../src/monorepo/PackageGraph.test.ts | 2 +- .../cli-node/src/monorepo/PackageGraph.ts | 2 +- packages/cli-node/src/monorepo/isMonoRepo.ts | 2 +- .../cli-node/src/monorepo/isMonorepo.test.ts | 2 +- .../src/pacman/PackageManager.test.ts | 4 +- .../cli-node/src/pacman/PackageManager.ts | 5 +- .../cli-node/src/pacman/yarn/Yarn.test.ts | 4 +- packages/cli-node/src/pacman/yarn/Yarn.ts | 13 +-- packages/cli-node/src/paths.ts | 20 ++++ packages/cli-node/src/util.ts | 109 ------------------ 12 files changed, 52 insertions(+), 136 deletions(-) create mode 100644 .changeset/afraid-items-drum.md create mode 100644 packages/cli-node/src/paths.ts delete mode 100644 packages/cli-node/src/util.ts diff --git a/.changeset/afraid-items-drum.md b/.changeset/afraid-items-drum.md new file mode 100644 index 0000000000..94ddd608cd --- /dev/null +++ b/.changeset/afraid-items-drum.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli-node': patch +--- + +Updated to use new utilities from `@backstage/cli-common`. diff --git a/packages/cli-node/src/git/GitUtils.ts b/packages/cli-node/src/git/GitUtils.ts index 7b87487c56..7edb3581d6 100644 --- a/packages/cli-node/src/git/GitUtils.ts +++ b/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; } diff --git a/packages/cli-node/src/monorepo/PackageGraph.test.ts b/packages/cli-node/src/monorepo/PackageGraph.test.ts index e2378b91c0..9bc9cbf72b 100644 --- a/packages/cli-node/src/monorepo/PackageGraph.test.ts +++ b/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), diff --git a/packages/cli-node/src/monorepo/PackageGraph.ts b/packages/cli-node/src/monorepo/PackageGraph.ts index 2f82af0808..76fbe9ba4f 100644 --- a/packages/cli-node/src/monorepo/PackageGraph.ts +++ b/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'; diff --git a/packages/cli-node/src/monorepo/isMonoRepo.ts b/packages/cli-node/src/monorepo/isMonoRepo.ts index f47ac4b9ad..da78c8dd53 100644 --- a/packages/cli-node/src/monorepo/isMonoRepo.ts +++ b/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'; /** diff --git a/packages/cli-node/src/monorepo/isMonorepo.test.ts b/packages/cli-node/src/monorepo/isMonorepo.test.ts index 30c2d3ccbc..de0dae0893 100644 --- a/packages/cli-node/src/monorepo/isMonorepo.test.ts +++ b/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) }, })); diff --git a/packages/cli-node/src/pacman/PackageManager.test.ts b/packages/cli-node/src/pacman/PackageManager.test.ts index 3997da554c..dd35e74902 100644 --- a/packages/cli-node/src/pacman/PackageManager.test.ts +++ b/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) }, })); diff --git a/packages/cli-node/src/pacman/PackageManager.ts b/packages/cli-node/src/pacman/PackageManager.ts index fc0aaa2c97..44c0849705 100644 --- a/packages/cli-node/src/pacman/PackageManager.ts +++ b/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; /** Uses the package manager to run a command in the repo. */ - run(args: string[], options?: SpawnOptionsPartialEnv): Promise; + run(args: string[], options?: RunOptions): Promise; /** * Executes the package manager's pack command to bundle the repo into an diff --git a/packages/cli-node/src/pacman/yarn/Yarn.test.ts b/packages/cli-node/src/pacman/yarn/Yarn.test.ts index 9584ef54b2..9bf866b821 100644 --- a/packages/cli-node/src/pacman/yarn/Yarn.test.ts +++ b/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) }, })); diff --git a/packages/cli-node/src/pacman/yarn/Yarn.ts b/packages/cli-node/src/pacman/yarn/Yarn.ts index 0a2b8d55ea..a6544344ca 100644 --- a/packages/cli-node/src/pacman/yarn/Yarn.ts +++ b/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 { @@ -98,8 +99,7 @@ function detectYarnVersion(dir?: string): Promise { 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 { 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); } }); diff --git a/packages/cli-node/src/paths.ts b/packages/cli-node/src/paths.ts new file mode 100644 index 0000000000..2c658c27b3 --- /dev/null +++ b/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); diff --git a/packages/cli-node/src/util.ts b/packages/cli-node/src/util.ts deleted file mode 100644 index eaf05855b1..0000000000 --- a/packages/cli-node/src/util.ts +++ /dev/null @@ -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 & { - env?: Partial; - // 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 { - if (typeof child.exitCode === 'number') { - if (child.exitCode) { - throw new ExitCodeError(child.exitCode, name); - } - return; - } - - await new Promise((resolve, reject) => { - child.once('error', error => reject(error)); - child.once('exit', code => { - if (code) { - reject(new ExitCodeError(code, name)); - } else { - resolve(); - } - }); - }); -}