diff --git a/dev-infra/build-worker.js b/dev-infra/build-worker.js index aad3843510..eb05917793 100644 --- a/dev-infra/build-worker.js +++ b/dev-infra/build-worker.js @@ -145,6 +145,7 @@ var GitCommandError = /** @class */ (function (_super) { // we sanitize the command that will be part of the error message. _super.call(this, "Command failed: git " + client.sanitizeConsoleOutput(args.join(' '))) || this; _this.args = args; + Object.setPrototypeOf(_this, GitCommandError.prototype); return _this; } return GitCommandError; diff --git a/dev-infra/ng-dev.js b/dev-infra/ng-dev.js index 7b7c2fb3d8..ee84e30658 100755 --- a/dev-infra/ng-dev.js +++ b/dev-infra/ng-dev.js @@ -317,6 +317,7 @@ var GitCommandError = /** @class */ (function (_super) { // we sanitize the command that will be part of the error message. _super.call(this, "Command failed: git " + client.sanitizeConsoleOutput(args.join(' '))) || this; _this.args = args; + Object.setPrototypeOf(_this, GitCommandError.prototype); return _this; } return GitCommandError; @@ -3463,30 +3464,40 @@ function discoverNewConflictsForPr(newPrNumber, updatedAfter) { info(`Retrieved ${allPendingPRs.length} total pending PRs`); info(`Checking ${pendingPrs.length} PRs for conflicts after a merge of #${newPrNumber}`); // Fetch and checkout the PR being checked. - exec(`git fetch ${requestedPr.headRef.repository.url} ${requestedPr.headRef.name}`); - exec(`git checkout -B ${tempWorkingBranch} FETCH_HEAD`); + git.run(['fetch', '-q', requestedPr.headRef.repository.url, requestedPr.headRef.name]); + git.run(['checkout', '-q', '-B', tempWorkingBranch, 'FETCH_HEAD']); // Rebase the PR against the PRs target branch. - exec(`git fetch ${requestedPr.baseRef.repository.url} ${requestedPr.baseRef.name}`); - const result = exec(`git rebase FETCH_HEAD`); - if (result.code) { - error('The requested PR currently has conflicts'); - cleanUpGitState(previousBranchOrRevision); - process.exit(1); + git.run(['fetch', '-q', requestedPr.baseRef.repository.url, requestedPr.baseRef.name]); + try { + git.run(['rebase', 'FETCH_HEAD'], { stdio: 'ignore' }); + } + catch (err) { + if (err instanceof GitCommandError) { + error('The requested PR currently has conflicts'); + git.checkout(previousBranchOrRevision, true); + process.exit(1); + } + throw err; } // Start the progress bar progressBar.start(pendingPrs.length, 0); // Check each PR to determine if it can merge cleanly into the repo after the target PR. for (const pr of pendingPrs) { // Fetch and checkout the next PR - exec(`git fetch ${pr.headRef.repository.url} ${pr.headRef.name}`); - exec(`git checkout --detach FETCH_HEAD`); + git.run(['fetch', '-q', pr.headRef.repository.url, pr.headRef.name]); + git.run(['checkout', '-q', '--detach', 'FETCH_HEAD']); // Check if the PR cleanly rebases into the repo after the target PR. - const result = exec(`git rebase ${tempWorkingBranch}`); - if (result.code !== 0) { - conflicts.push(pr); + try { + git.run(['rebase', tempWorkingBranch], { stdio: 'ignore' }); + } + catch (err) { + if (err instanceof GitCommandError) { + conflicts.push(pr); + } + throw err; } // Abort any outstanding rebase attempt. - exec(`git rebase --abort`); + git.runGraceful(['rebase', '--abort'], { stdio: 'ignore' }); progressBar.increment(1); } // End the progress bar as all PRs have been processed. @@ -5978,7 +5989,7 @@ const ReleaseNotesCommandModule = { * * @returns a Promise resolving on success, and rejecting on command failure with the status code. */ -function spawnInteractiveCommand(command, args, options) { +function spawnInteractive(command, args, options) { if (options === void 0) { options = {}; } return new Promise(function (resolve, reject) { var commandText = command + " " + args.join(' '); @@ -5993,9 +6004,9 @@ function spawnInteractiveCommand(command, args, options) { * output mode, stdout/stderr output is also printed to the console, or only on error. * * @returns a Promise resolving with captured stdout and stderr on success. The promise - * rejects on command failure. + * rejects on command failure */ -function spawnWithDebugOutput(command, args, options) { +function spawn(command, args, options) { if (options === void 0) { options = {}; } return new Promise(function (resolve, reject) { var commandText = command + " " + args.join(' '); @@ -6025,15 +6036,16 @@ function spawnWithDebugOutput(command, args, options) { process.stderr.write(message); } }); - childProcess.on('exit', function (status, signal) { - var exitDescription = status !== null ? "exit code \"" + status + "\"" : "signal \"" + signal + "\""; + childProcess.on('exit', function (exitCode, signal) { + var exitDescription = exitCode !== null ? "exit code \"" + exitCode + "\"" : "signal \"" + signal + "\""; var printFn = outputMode === 'on-error' ? error : debug; + var status = statusFromExitCodeAndSignal(exitCode, signal); printFn("Command \"" + commandText + "\" completed with " + exitDescription + "."); printFn("Process output: \n" + logOutput); // On success, resolve the promise. Otherwise reject with the captured stderr // and stdout log output if the output mode was set to `silent`. - if (status === 0) { - resolve({ stdout: stdout, stderr: stderr }); + if (status === 0 || options.suppressErrorOnFailingExitCode) { + resolve({ stdout: stdout, stderr: stderr, status: status }); } else { reject(outputMode === 'silent' ? logOutput : undefined); @@ -6041,6 +6053,10 @@ function spawnWithDebugOutput(command, args, options) { }); }); } +/** Convert the provided exitCode and signal to a single status code. */ +function statusFromExitCodeAndSignal(exitCode, signal) { + return exitCode !== null ? exitCode : signal !== null ? signal : -1; +} /** * @license @@ -6060,7 +6076,7 @@ function runNpmPublish(packagePath, distTag, registryUrl) { if (registryUrl !== undefined) { args.push('--registry', registryUrl); } - yield spawnWithDebugOutput('npm', args, { cwd: packagePath, mode: 'silent' }); + yield spawn('npm', args, { cwd: packagePath, mode: 'silent' }); }); } /** @@ -6074,7 +6090,7 @@ function setNpmTagForPackage(packageName, distTag, version, registryUrl) { if (registryUrl !== undefined) { args.push('--registry', registryUrl); } - yield spawnWithDebugOutput('npm', args, { mode: 'silent' }); + yield spawn('npm', args, { mode: 'silent' }); }); } /** @@ -6089,7 +6105,7 @@ function npmIsLoggedIn(registryUrl) { args.push('--registry', registryUrl); } try { - yield spawnWithDebugOutput('npm', args, { mode: 'silent' }); + yield spawn('npm', args, { mode: 'silent' }); } catch (e) { return false; @@ -6112,7 +6128,7 @@ function npmLogin(registryUrl) { } // The login command prompts for username, password and other profile information. Hence // the process needs to be interactive (i.e. respecting current TTYs stdin). - yield spawnInteractiveCommand('npm', args); + yield spawnInteractive('npm', args); }); } /** @@ -6129,7 +6145,7 @@ function npmLogout(registryUrl) { args.splice(1, 0, '--registry', registryUrl); } try { - yield spawnWithDebugOutput('npm', args, { mode: 'silent' }); + yield spawn('npm', args, { mode: 'silent' }); } finally { return npmIsLoggedIn(registryUrl); @@ -6251,7 +6267,7 @@ function invokeSetNpmDistCommand(npmDistTag, version) { return tslib.__awaiter(this, void 0, void 0, function* () { try { // Note: No progress indicator needed as that is the responsibility of the command. - yield spawnWithDebugOutput('yarn', ['--silent', 'ng-dev', 'release', 'set-dist-tag', npmDistTag, version.format()]); + yield spawn('yarn', ['--silent', 'ng-dev', 'release', 'set-dist-tag', npmDistTag, version.format()]); info(green(` ✓ Set "${npmDistTag}" NPM dist tag for all packages to v${version}.`)); } catch (e) { @@ -6271,7 +6287,7 @@ function invokeReleaseBuildCommand() { try { // Since we expect JSON to be printed from the `ng-dev release build` command, // we spawn the process in silent mode. We have set up an Ora progress spinner. - const { stdout } = yield spawnWithDebugOutput('yarn', ['--silent', 'ng-dev', 'release', 'build', '--json'], { mode: 'silent' }); + const { stdout } = yield spawn('yarn', ['--silent', 'ng-dev', 'release', 'build', '--json'], { mode: 'silent' }); spinner.stop(); info(green(' ✓ Built release output for all packages.')); // The `ng-dev release build` command prints a JSON array to stdout @@ -6295,7 +6311,7 @@ function invokeYarnInstallCommand(projectDir) { try { // Note: No progress indicator needed as that is the responsibility of the command. // TODO: Consider using an Ora spinner instead to ensure minimal console output. - yield spawnWithDebugOutput('yarn', ['install', '--frozen-lockfile', '--non-interactive'], { cwd: projectDir }); + yield spawn('yarn', ['install', '--frozen-lockfile', '--non-interactive'], { cwd: projectDir }); info(green(' ✓ Installed project dependencies.')); } catch (e) { @@ -7450,7 +7466,7 @@ class ReleaseTool { try { // Note: We do not rely on `/usr/bin/env` but rather access the `env` binary directly as it // should be part of the shell's `$PATH`. This is necessary for compatibility with Windows. - const pyVersion = yield spawnWithDebugOutput('env', ['python', '--version'], { mode: 'silent' }); + const pyVersion = yield spawn('env', ['python', '--version'], { mode: 'silent' }); const version = pyVersion.stdout.trim() || pyVersion.stderr.trim(); if (version.startsWith('Python 3.')) { debug(`Local python version: ${version}`); diff --git a/dev-infra/pr/discover-new-conflicts/index.ts b/dev-infra/pr/discover-new-conflicts/index.ts index b164155566..32138da310 100644 --- a/dev-infra/pr/discover-new-conflicts/index.ts +++ b/dev-infra/pr/discover-new-conflicts/index.ts @@ -11,7 +11,7 @@ import {types as graphqlTypes} from 'typed-graphqlify'; import {error, info} from '../../utils/console'; import {AuthenticatedGitClient} from '../../utils/git/authenticated-git-client'; -import {GitClient} from '../../utils/git/git-client'; +import {GitCommandError} from '../../utils/git/git-client'; import {getPendingPrs} from '../../utils/github'; import {exec} from '../../utils/shelljs'; @@ -95,16 +95,20 @@ export async function discoverNewConflictsForPr(newPrNumber: number, updatedAfte info(`Checking ${pendingPrs.length} PRs for conflicts after a merge of #${newPrNumber}`); // Fetch and checkout the PR being checked. - exec(`git fetch ${requestedPr.headRef.repository.url} ${requestedPr.headRef.name}`); - exec(`git checkout -B ${tempWorkingBranch} FETCH_HEAD`); + git.run(['fetch', '-q', requestedPr.headRef.repository.url, requestedPr.headRef.name]); + git.run(['checkout', '-q', '-B', tempWorkingBranch, 'FETCH_HEAD']); // Rebase the PR against the PRs target branch. - exec(`git fetch ${requestedPr.baseRef.repository.url} ${requestedPr.baseRef.name}`); - const result = exec(`git rebase FETCH_HEAD`); - if (result.code) { - error('The requested PR currently has conflicts'); - cleanUpGitState(previousBranchOrRevision); - process.exit(1); + git.run(['fetch', '-q', requestedPr.baseRef.repository.url, requestedPr.baseRef.name]); + try { + git.run(['rebase', 'FETCH_HEAD'], {stdio: 'ignore'}); + } catch (err) { + if (err instanceof GitCommandError) { + error('The requested PR currently has conflicts'); + git.checkout(previousBranchOrRevision, true); + process.exit(1); + } + throw err; } // Start the progress bar @@ -113,15 +117,19 @@ export async function discoverNewConflictsForPr(newPrNumber: number, updatedAfte // Check each PR to determine if it can merge cleanly into the repo after the target PR. for (const pr of pendingPrs) { // Fetch and checkout the next PR - exec(`git fetch ${pr.headRef.repository.url} ${pr.headRef.name}`); - exec(`git checkout --detach FETCH_HEAD`); + git.run(['fetch', '-q', pr.headRef.repository.url, pr.headRef.name]); + git.run(['checkout', '-q', '--detach', 'FETCH_HEAD']); // Check if the PR cleanly rebases into the repo after the target PR. - const result = exec(`git rebase ${tempWorkingBranch}`); - if (result.code !== 0) { - conflicts.push(pr); + try { + git.run(['rebase', tempWorkingBranch], {stdio: 'ignore'}); + } catch (err) { + if (err instanceof GitCommandError) { + conflicts.push(pr); + } + throw err; } // Abort any outstanding rebase attempt. - exec(`git rebase --abort`); + git.runGraceful(['rebase', '--abort'], {stdio: 'ignore'}); progressBar.increment(1); } diff --git a/dev-infra/release/publish/external-commands.ts b/dev-infra/release/publish/external-commands.ts index 2683e3ce46..f0b9edf366 100644 --- a/dev-infra/release/publish/external-commands.ts +++ b/dev-infra/release/publish/external-commands.ts @@ -9,7 +9,7 @@ import * as ora from 'ora'; import * as semver from 'semver'; -import {spawnWithDebugOutput} from '../../utils/child-process'; +import {spawn} from '../../utils/child-process'; import {error, green, info, red} from '../../utils/console'; import {BuiltPackage} from '../config/index'; import {NpmDistTag} from '../versioning'; @@ -40,7 +40,7 @@ import {FatalReleaseActionError} from './actions-error'; export async function invokeSetNpmDistCommand(npmDistTag: NpmDistTag, version: semver.SemVer) { try { // Note: No progress indicator needed as that is the responsibility of the command. - await spawnWithDebugOutput( + await spawn( 'yarn', ['--silent', 'ng-dev', 'release', 'set-dist-tag', npmDistTag, version.format()]); info(green(` ✓ Set "${npmDistTag}" NPM dist tag for all packages to v${version}.`)); } catch (e) { @@ -59,8 +59,8 @@ export async function invokeReleaseBuildCommand(): Promise { try { // Since we expect JSON to be printed from the `ng-dev release build` command, // we spawn the process in silent mode. We have set up an Ora progress spinner. - const {stdout} = await spawnWithDebugOutput( - 'yarn', ['--silent', 'ng-dev', 'release', 'build', '--json'], {mode: 'silent'}); + const {stdout} = + await spawn('yarn', ['--silent', 'ng-dev', 'release', 'build', '--json'], {mode: 'silent'}); spinner.stop(); info(green(' ✓ Built release output for all packages.')); // The `ng-dev release build` command prints a JSON array to stdout @@ -82,8 +82,7 @@ export async function invokeYarnInstallCommand(projectDir: string): Promise args.splice(1, 0, '--registry', registryUrl); } try { - await spawnWithDebugOutput('npm', args, {mode: 'silent'}); + await spawn('npm', args, {mode: 'silent'}); } finally { return npmIsLoggedIn(registryUrl); } diff --git a/dev-infra/utils/child-process.ts b/dev-infra/utils/child-process.ts index 1a1950f3d5..64fcdcb634 100644 --- a/dev-infra/utils/child-process.ts +++ b/dev-infra/utils/child-process.ts @@ -6,21 +6,34 @@ * found in the LICENSE file at https://angular.io/license */ -import {spawn, SpawnOptions} from 'child_process'; +import {spawn as _spawn, SpawnOptions as _SpawnOptions, spawnSync as _spawnSync, SpawnSyncOptions as _SpawnSyncOptions} from 'child_process'; import {debug, error} from './console'; -/** Interface describing the options for spawning a process. */ -export interface SpawnedProcessOptions extends Omit { - /** Console output mode. Defaults to "enabled". */ - mode?: 'enabled'|'silent'|'on-error'; + +export interface SpawnSyncOptions extends Omit<_SpawnSyncOptions, 'shell'|'stdio'> { + /** Whether to prevent exit codes being treated as failures. */ + suppressErrorOnFailingExitCode?: boolean; } +/** Interface describing the options for spawning a process. */ +export interface SpawnOptions extends Omit<_SpawnOptions, 'shell'|'stdio'> { + /** Console output mode. Defaults to "enabled". */ + mode?: 'enabled'|'silent'|'on-error'; + /** Whether to prevent exit codes being treated as failures. */ + suppressErrorOnFailingExitCode?: boolean; +} + +/** Interface describing the options for spawning an interactive process. */ +export type SpawnInteractiveCommandOptions = Omit<_SpawnOptions, 'shell'|'stdio'>; + /** Interface describing the result of a spawned process. */ -export interface SpawnedProcessResult { +export interface SpawnResult { /** Captured stdout in string format. */ stdout: string; /** Captured stderr in string format. */ stderr: string; + /** The exit code or signal of the process. */ + status: number|NodeJS.Signals; } /** @@ -29,12 +42,12 @@ export interface SpawnedProcessResult { * * @returns a Promise resolving on success, and rejecting on command failure with the status code. */ -export function spawnInteractiveCommand( - command: string, args: string[], options: Omit = {}) { +export function spawnInteractive( + command: string, args: string[], options: SpawnInteractiveCommandOptions = {}) { return new Promise((resolve, reject) => { const commandText = `${command} ${args.join(' ')}`; debug(`Executing command: ${commandText}`); - const childProcess = spawn(command, args, {...options, shell: true, stdio: 'inherit'}); + const childProcess = _spawn(command, args, {...options, shell: true, stdio: 'inherit'}); childProcess.on('exit', status => status === 0 ? resolve() : reject(status)); }); } @@ -45,18 +58,17 @@ export function spawnInteractiveCommand( * output mode, stdout/stderr output is also printed to the console, or only on error. * * @returns a Promise resolving with captured stdout and stderr on success. The promise - * rejects on command failure. + * rejects on command failure */ -export function spawnWithDebugOutput( - command: string, args: string[], - options: SpawnedProcessOptions = {}): Promise { +export function spawn( + command: string, args: string[], options: SpawnOptions = {}): Promise { return new Promise((resolve, reject) => { const commandText = `${command} ${args.join(' ')}`; const outputMode = options.mode; debug(`Executing command: ${commandText}`); - const childProcess = spawn(command, args, {...options, shell: true, stdio: 'pipe'}); + const childProcess = _spawn(command, args, {...options, shell: true, stdio: 'pipe'}); let logOutput = ''; let stdout = ''; let stderr = ''; @@ -83,20 +95,51 @@ export function spawnWithDebugOutput( } }); - childProcess.on('exit', (status, signal) => { - const exitDescription = status !== null ? `exit code "${status}"` : `signal "${signal}"`; + childProcess.on('exit', (exitCode, signal) => { + const exitDescription = exitCode !== null ? `exit code "${exitCode}"` : `signal "${signal}"`; const printFn = outputMode === 'on-error' ? error : debug; + const status = statusFromExitCodeAndSignal(exitCode, signal); printFn(`Command "${commandText}" completed with ${exitDescription}.`); printFn(`Process output: \n${logOutput}`); // On success, resolve the promise. Otherwise reject with the captured stderr // and stdout log output if the output mode was set to `silent`. - if (status === 0) { - resolve({stdout, stderr}); + if (status === 0 || options.suppressErrorOnFailingExitCode) { + resolve({stdout, stderr, status}); } else { reject(outputMode === 'silent' ? logOutput : undefined); } }); }); } + +/** + * Spawns a given command with the specified arguments inside a shell syncronously. + * + * @returns The command's stdout and stderr. + */ +export function spawnSync( + command: string, args: string[], options: SpawnOptions = {}): SpawnResult { + const commandText = `${command} ${args.join(' ')}`; + debug(`Executing command: ${commandText}`); + + const {status: exitCode, signal, stdout, stderr} = + _spawnSync(command, args, {...options, encoding: 'utf8', shell: true, stdio: 'pipe'}); + + /** The status of the spawn result. */ + const status = statusFromExitCodeAndSignal(exitCode, signal); + + if (status === 0 || options.suppressErrorOnFailingExitCode) { + return {status, stdout, stderr}; + } + + throw new Error(stderr); +} + + + +/** Convert the provided exitCode and signal to a single status code. */ +function statusFromExitCodeAndSignal(exitCode: number|null, signal: NodeJS.Signals|null) { + return exitCode !== null ? exitCode : signal !== null ? signal : -1; +} diff --git a/dev-infra/utils/git/git-client.ts b/dev-infra/utils/git/git-client.ts index eab11db987..52e803b9a5 100644 --- a/dev-infra/utils/git/git-client.ts +++ b/dev-infra/utils/git/git-client.ts @@ -9,7 +9,6 @@ import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process'; import {Options as SemVerOptions, parse, SemVer} from 'semver'; -import {spawnWithDebugOutput} from '../child-process'; import {getConfig, GithubConfig, NgDevConfig} from '../config'; import {debug, info} from '../console'; import {DryRunError, isDryRun} from '../dry-run'; @@ -24,6 +23,7 @@ export class GitCommandError extends Error { // accidentally leak the Github token that might be used in a command, // we sanitize the command that will be part of the error message. super(`Command failed: git ${client.sanitizeConsoleOutput(args.join(' '))}`); + Object.setPrototypeOf(this, GitCommandError.prototype); } }