diff --git a/apps/rush/src/RushVersionSelector.ts b/apps/rush/src/RushVersionSelector.ts index 6b789dec4c0..b4ba82fb444 100644 --- a/apps/rush/src/RushVersionSelector.ts +++ b/apps/rush/src/RushVersionSelector.ts @@ -48,7 +48,7 @@ export class RushVersionSelector { if (installIsValid) { console.log('Another process performed the installation.'); } else { - Utilities.installPackageInDirectory({ + await Utilities.installPackageInDirectoryAsync({ directory: expectedRushPath, packageName: isLegacyRushVersion ? '@microsoft/rush' : '@microsoft/rush-lib', version: version, diff --git a/common/changes/@microsoft/rush/main_2023-09-24-06-11.json b/common/changes/@microsoft/rush/main_2023-09-24-06-11.json new file mode 100644 index 00000000000..d7bd565c75c --- /dev/null +++ b/common/changes/@microsoft/rush/main_2023-09-24-06-11.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Update the functionality that runs external lifecycle processes to be async.", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file diff --git a/libraries/rush-lib/config/jest.config.json b/libraries/rush-lib/config/jest.config.json index d82520f508c..ad4d21b0971 100644 --- a/libraries/rush-lib/config/jest.config.json +++ b/libraries/rush-lib/config/jest.config.json @@ -13,5 +13,7 @@ "!lib-commonjs/**/__tests__/**", "!lib-commonjs/**/__fixtures__/**", "!lib-commonjs/**/__mocks__/**" - ] + ], + + "globalTeardown": "/lib-commonjs/utilities/test/global-teardown.js" } diff --git a/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts b/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts index 30e5f35e753..6d9dea0ff66 100644 --- a/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts +++ b/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts @@ -412,24 +412,14 @@ export class RushPnpmCommandLineParser { } try { - await Utilities.executeCommandAndInspectOutputAsync( - { - command: rushConfiguration.packageManagerToolFilename, - args: this._pnpmArgs, - workingDirectory: process.cwd(), - environment: pnpmEnvironmentMap.toObject(), - keepEnvironment: true - }, - onStdoutStreamChunk, - (exitCode: number | null, signal: NodeJS.Signals | null) => { - if (typeof exitCode === 'number') { - process.exitCode = exitCode; - } else { - // Terminated by a signal - process.exitCode = 1; - } - } - ); + await Utilities.executeCommandAsync({ + command: rushConfiguration.packageManagerToolFilename, + args: this._pnpmArgs, + workingDirectory: process.cwd(), + environment: pnpmEnvironmentMap.toObject(), + keepEnvironment: true, + onStdoutStreamChunk + }); } catch (e) { this._terminal.writeDebugLine(`Error: ${e}`); } diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index 3a7c7b95c58..6455cab3989 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -164,8 +164,9 @@ export class ChangeAction extends BaseRushAction { } public async runAsync(): Promise { + const targetBranch: string = await this._getTargetBranchAsync(); // eslint-disable-next-line no-console - console.log(`The target branch is ${this._targetBranch}`); + console.log(`The target branch is ${targetBranch}`); if (this._verifyParameter.value) { const errors: string[] = [ @@ -197,11 +198,11 @@ export class ChangeAction extends BaseRushAction { const sortedProjectList: string[] = (await this._getChangedProjectNamesAsync()).sort(); if (sortedProjectList.length === 0) { this._logNoChangeFileRequired(); - this._warnUnstagedChanges(); + await this._warnUnstagedChangesAsync(); return; } - this._warnUnstagedChanges(); + await this._warnUnstagedChangesAsync(); const inquirer: typeof InquirerType = await import('inquirer'); const promptModule: InquirerType.PromptModule = inquirer.createPromptModule(); @@ -277,7 +278,7 @@ export class ChangeAction extends BaseRushAction { interactiveMode = true; const existingChangeComments: Map = ChangeFiles.getChangeComments( - this._getChangeFiles() + await this._getChangeFilesAsync() ); changeFileData = await this._promptForChangeFileDataAsync( promptModule, @@ -310,7 +311,7 @@ export class ChangeAction extends BaseRushAction { } if (this._commitChangesFlagParameter.value || this._commitChangesMessageStringParameter.value) { if (changefiles && changefiles.length !== 0) { - this._stageAndCommitGitChanges( + await this._stageAndCommitGitChangesAsync( changefiles, this._commitChangesMessageStringParameter.value || this.rushConfiguration.gitChangefilesCommitMessage || @@ -340,15 +341,16 @@ export class ChangeAction extends BaseRushAction { private async _verifyAsync(): Promise { const changedPackages: string[] = await this._getChangedProjectNamesAsync(); if (changedPackages.length > 0) { - this._validateChangeFile(changedPackages); + await this._validateChangeFileAsync(changedPackages); } else { this._logNoChangeFileRequired(); } } - private get _targetBranch(): string { + private async _getTargetBranchAsync(): Promise { if (!this._targetBranchName) { - this._targetBranchName = this._targetBranchParameter.value || this._git.getRemoteDefaultBranch(); + this._targetBranchName = + this._targetBranchParameter.value || (await this._git.getRemoteDefaultBranchAsync()); } return this._targetBranchName; @@ -358,7 +360,7 @@ export class ChangeAction extends BaseRushAction { const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(this.rushConfiguration); const changedProjects: Set = await projectChangeAnalyzer.getChangedProjectsAsync({ - targetBranchName: this._targetBranch, + targetBranchName: await this._getTargetBranchAsync(), terminal: this._terminal, shouldFetch: !this._noFetchParameter.value, // Lockfile evaluation will expand the set of projects that request change files @@ -382,19 +384,28 @@ export class ChangeAction extends BaseRushAction { return Array.from(changedProjectNames); } - private _validateChangeFile(changedPackages: string[]): void { - const files: string[] = this._getChangeFiles(); + private async _validateChangeFileAsync(changedPackages: string[]): Promise { + const files: string[] = await this._getChangeFilesAsync(); ChangeFiles.validate(files, changedPackages, this.rushConfiguration); } - private _getChangeFiles(): string[] { + private async _getChangeFilesAsync(): Promise { const repoRoot: string = getRepoRoot(this.rushConfiguration.rushJsonFolder); const relativeChangesFolder: string = path.relative(repoRoot, this.rushConfiguration.changesFolder); - return this._git - .getChangedFiles(this._targetBranch, this._terminal, true, relativeChangesFolder) - .map((relativePath) => { - return path.join(repoRoot, relativePath); - }); + const targetBranch: string = await this._getTargetBranchAsync(); + const changedFiles: string[] = await this._git.getChangedFilesAsync( + targetBranch, + this._terminal, + true, + relativeChangesFolder + ); + + const result: string[] = []; + for (const changedFile of changedFiles) { + result.push(path.join(repoRoot, changedFile)); + } + + return result; } /** @@ -630,9 +641,10 @@ export class ChangeAction extends BaseRushAction { return email; } - private _warnUnstagedChanges(): void { + private async _warnUnstagedChangesAsync(): Promise { try { - if (this._git.hasUnstagedChanges()) { + const hasUnstagedChanges: boolean = await this._git.hasUnstagedChangesAsync(); + if (hasUnstagedChanges) { // eslint-disable-next-line no-console console.log( '\n' + @@ -738,14 +750,14 @@ export class ChangeAction extends BaseRushAction { console.log('No changes were detected to relevant packages on this branch. Nothing to do.'); } - private _stageAndCommitGitChanges(pattern: string[], message: string): void { + private async _stageAndCommitGitChangesAsync(pattern: string[], message: string): Promise { try { - Utilities.executeCommand({ + await Utilities.executeCommandAsync({ command: 'git', args: ['add', ...pattern], workingDirectory: this.rushConfiguration.changesFolder }); - Utilities.executeCommand({ + await Utilities.executeCommandAsync({ command: 'git', args: ['commit', ...pattern, '-m', message], workingDirectory: this.rushConfiguration.changesFolder diff --git a/libraries/rush-lib/src/cli/actions/PublishAction.ts b/libraries/rush-lib/src/cli/actions/PublishAction.ts index b72f95412d8..90d6266534b 100644 --- a/libraries/rush-lib/src/cli/actions/PublishAction.ts +++ b/libraries/rush-lib/src/cli/actions/PublishAction.ts @@ -241,7 +241,7 @@ export class PublishAction extends BaseRushAction { const git: Git = new Git(this.rushConfiguration); const publishGit: PublishGit = new PublishGit(git, this._targetBranch.value); if (this._includeAll.value) { - this._publishAll(publishGit, allPackages); + await this._publishAllAsync(publishGit, allPackages); } else { this._prereleaseToken = new PrereleaseToken( this._prereleaseName.value, @@ -287,7 +287,7 @@ export class PublishAction extends BaseRushAction { const tempBranchName: string = `publish-${Date.now()}`; // Make changes in temp branch. - publishGit.checkout(tempBranchName, true); + await publishGit.checkoutAsync(tempBranchName, true); this._setDependenciesBeforePublish(); @@ -297,14 +297,14 @@ export class PublishAction extends BaseRushAction { this._setDependenciesBeforeCommit(); - if (git.hasUncommittedChanges()) { + if (await git.hasUncommittedChangesAsync()) { // Stage, commit, and push the changes to remote temp branch. - publishGit.addChanges(':/*'); - publishGit.commit( + await publishGit.addChangesAsync(':/*'); + await publishGit.commitAsync( this.rushConfiguration.gitVersionBumpCommitMessage || DEFAULT_PACKAGE_UPDATE_MESSAGE, !this._ignoreGitHooksParameter.value ); - publishGit.push(tempBranchName, !this._ignoreGitHooksParameter.value); + await publishGit.pushAsync(tempBranchName, !this._ignoreGitHooksParameter.value); this._setDependenciesBeforePublish(); @@ -321,8 +321,8 @@ export class PublishAction extends BaseRushAction { if (change.changeType && change.changeType > ChangeType.dependency) { const project: RushConfigurationProject | undefined = allPackages.get(change.packageName); if (project) { - if (!this._packageExists(project)) { - this._npmPublish(change.packageName, project.publishFolder); + if (!(await this._packageExistsAsync(project))) { + await this._npmPublishAsync(change.packageName, project.publishFolder); } else { // eslint-disable-next-line no-console console.log(`Skip ${change.packageName}. Package exists.`); @@ -337,34 +337,37 @@ export class PublishAction extends BaseRushAction { this._setDependenciesBeforeCommit(); // Create and push appropriate Git tags. - this._gitAddTags(publishGit, orderedChanges); - publishGit.push(tempBranchName, !this._ignoreGitHooksParameter.value); + await this._gitAddTagsAsync(publishGit, orderedChanges); + await publishGit.pushAsync(tempBranchName, !this._ignoreGitHooksParameter.value); // Now merge to target branch. - publishGit.checkout(this._targetBranch.value!); - publishGit.pull(!this._ignoreGitHooksParameter.value); - publishGit.merge(tempBranchName, !this._ignoreGitHooksParameter.value); - publishGit.push(this._targetBranch.value!, !this._ignoreGitHooksParameter.value); - publishGit.deleteBranch(tempBranchName, true, !this._ignoreGitHooksParameter.value); + await publishGit.checkoutAsync(this._targetBranch.value!); + await publishGit.pullAsync(!this._ignoreGitHooksParameter.value); + await publishGit.mergeAsync(tempBranchName, !this._ignoreGitHooksParameter.value); + await publishGit.pushAsync(this._targetBranch.value!, !this._ignoreGitHooksParameter.value); + await publishGit.deleteBranchAsync(tempBranchName, true, !this._ignoreGitHooksParameter.value); } else { - publishGit.checkout(this._targetBranch.value!); - publishGit.deleteBranch(tempBranchName, false, !this._ignoreGitHooksParameter.value); + await publishGit.checkoutAsync(this._targetBranch.value!); + await publishGit.deleteBranchAsync(tempBranchName, false, !this._ignoreGitHooksParameter.value); } } } - private _publishAll(git: PublishGit, allPackages: ReadonlyMap): void { + private async _publishAllAsync( + git: PublishGit, + allPackages: ReadonlyMap + ): Promise { // eslint-disable-next-line no-console console.log(`Rush publish starts with includeAll and version policy ${this._versionPolicy.value}`); let updated: boolean = false; - allPackages.forEach((packageConfig, packageName) => { + for (const [packageName, packageConfig] of allPackages) { if ( packageConfig.shouldPublish && (!this._versionPolicy.value || this._versionPolicy.value === packageConfig.versionPolicyName) ) { - const applyTag: (apply: boolean) => void = (apply: boolean): void => { + const applyTagAsync: (apply: boolean) => Promise = async (apply: boolean): Promise => { if (!apply) { return; } @@ -372,7 +375,7 @@ export class PublishAction extends BaseRushAction { const packageVersion: string = packageConfig.packageJson.version; // Do not create a new tag if one already exists, this will result in a fatal error - if (git.hasTag(packageConfig)) { + if (await git.hasTagAsync(packageConfig)) { // eslint-disable-next-line no-console console.log( `Not tagging ${packageName}@${packageVersion}. A tag already exists for this version.` @@ -380,7 +383,7 @@ export class PublishAction extends BaseRushAction { return; } - git.addTag( + await git.addTagAsync( !!this._publish.value, packageName, packageVersion, @@ -392,32 +395,32 @@ export class PublishAction extends BaseRushAction { if (this._pack.value) { // packs to tarball instead of publishing to NPM repository - this._npmPack(packageName, packageConfig); - applyTag(this._applyGitTagsOnPack.value); - } else if (this._force.value || !this._packageExists(packageConfig)) { + await this._npmPackAsync(packageName, packageConfig); + await applyTagAsync(this._applyGitTagsOnPack.value); + } else if (this._force.value || !(await this._packageExistsAsync(packageConfig))) { // Publish to npm repository - this._npmPublish(packageName, packageConfig.publishFolder); - applyTag(true); + await this._npmPublishAsync(packageName, packageConfig.publishFolder); + await applyTagAsync(true); } else { // eslint-disable-next-line no-console console.log(`Skip ${packageName}. Not updated.`); } } - }); + } if (updated) { - git.push(this._targetBranch.value!, !this._ignoreGitHooksParameter.value); + await git.pushAsync(this._targetBranch.value!, !this._ignoreGitHooksParameter.value); } } - private _gitAddTags(git: PublishGit, orderedChanges: IChangeInfo[]): void { + private async _gitAddTagsAsync(git: PublishGit, orderedChanges: IChangeInfo[]): Promise { for (const change of orderedChanges) { if ( change.changeType && change.changeType > ChangeType.dependency && this.rushConfiguration.projectsByName.get(change.packageName)!.shouldPublish ) { - git.addTag( + await git.addTagAsync( !!this._publish.value && !this._registryUrl.value, change.packageName, change.newVersion!, @@ -428,7 +431,7 @@ export class PublishAction extends BaseRushAction { } } - private _npmPublish(packageName: string, packagePath: string): void { + private async _npmPublishAsync(packageName: string, packagePath: string): Promise { const env: { [key: string]: string | undefined } = PublishUtilities.getEnvArgs(); const args: string[] = ['publish']; @@ -466,7 +469,7 @@ export class PublishAction extends BaseRushAction { // If the auth token was specified via the command line, avoid printing it on the console const secretSubstring: string | undefined = this._npmAuthToken.value; - PublishUtilities.execCommand( + await PublishUtilities.execCommandAsync( !!this._publish.value, packageManagerToolFilename, args, @@ -477,12 +480,12 @@ export class PublishAction extends BaseRushAction { } } - private _packageExists(packageConfig: RushConfigurationProject): boolean { + private async _packageExistsAsync(packageConfig: RushConfigurationProject): Promise { const env: { [key: string]: string | undefined } = PublishUtilities.getEnvArgs(); const args: string[] = []; this._addSharedNpmConfig(env, args); - const publishedVersions: string[] = Npm.publishedVersions( + const publishedVersions: string[] = await Npm.getPublishedVersionsAsync( packageConfig.packageName, packageConfig.publishFolder, env, @@ -511,11 +514,11 @@ export class PublishAction extends BaseRushAction { return publishedVersions.indexOf(normalizedVersion) >= 0; } - private _npmPack(packageName: string, project: RushConfigurationProject): void { + private async _npmPackAsync(packageName: string, project: RushConfigurationProject): Promise { const args: string[] = ['pack']; const env: { [key: string]: string | undefined } = PublishUtilities.getEnvArgs(); - PublishUtilities.execCommand( + await PublishUtilities.execCommandAsync( !!this._publish.value, this.rushConfiguration.packageManagerToolFilename, args, diff --git a/libraries/rush-lib/src/cli/actions/VersionAction.ts b/libraries/rush-lib/src/cli/actions/VersionAction.ts index 542930e25a8..e82ae194813 100644 --- a/libraries/rush-lib/src/cli/actions/VersionAction.ts +++ b/libraries/rush-lib/src/cli/actions/VersionAction.ts @@ -104,7 +104,7 @@ export class VersionAction extends BaseRushAction { } ); const git: Git = new Git(this.rushConfiguration); - const userEmail: string = git.getGitEmail(); + const userEmail: string = await git.getGitEmailAsync(); this._validateInput(); const versionManagerModule: typeof VersionManagerType = await import( @@ -130,7 +130,7 @@ export class VersionAction extends BaseRushAction { if (updatedPackages.size > 0) { // eslint-disable-next-line no-console console.log(`${updatedPackages.size} packages are getting updated.`); - this._gitProcess(tempBranch, this._targetBranch.value); + await this._gitProcessAsync(tempBranch, this._targetBranch.value); } } else if (this._bumpVersion.value) { const tempBranch: string = 'version/bump-' + new Date().getTime(); @@ -140,7 +140,7 @@ export class VersionAction extends BaseRushAction { this._prereleaseIdentifier.value, true ); - this._gitProcess(tempBranch, this._targetBranch.value); + await this._gitProcessAsync(tempBranch, this._targetBranch.value); } } @@ -227,7 +227,7 @@ export class VersionAction extends BaseRushAction { } } - private _gitProcess(tempBranch: string, targetBranch: string | undefined): void { + private async _gitProcessAsync(tempBranch: string, targetBranch: string | undefined): Promise { // Validate the result before commit. this._validateResult(); @@ -235,9 +235,9 @@ export class VersionAction extends BaseRushAction { const publishGit: PublishGit = new PublishGit(git, targetBranch); // Make changes in temp branch. - publishGit.checkout(tempBranch, true); + await publishGit.checkoutAsync(tempBranch, true); - const uncommittedChanges: ReadonlyArray = git.getUncommittedChanges(); + const uncommittedChanges: ReadonlyArray = await git.getUncommittedChangesAsync(); // Stage, commit, and push the changes to remote temp branch. // Need to commit the change log updates in its own commit @@ -246,10 +246,10 @@ export class VersionAction extends BaseRushAction { }); if (changeLogUpdated) { - publishGit.addChanges('.', this.rushConfiguration.changesFolder); - publishGit.addChanges(':/**/CHANGELOG.json'); - publishGit.addChanges(':/**/CHANGELOG.md'); - publishGit.commit( + await publishGit.addChangesAsync('.', this.rushConfiguration.changesFolder); + await publishGit.addChangesAsync(':/**/CHANGELOG.json'); + await publishGit.addChangesAsync(':/**/CHANGELOG.md'); + await publishGit.commitAsync( this.rushConfiguration.gitChangeLogUpdateCommitMessage || DEFAULT_CHANGELOG_UPDATE_MESSAGE, !this._ignoreGitHooksParameter.value ); @@ -261,29 +261,29 @@ export class VersionAction extends BaseRushAction { }); if (packageJsonUpdated) { - publishGit.addChanges(this.rushConfiguration.versionPolicyConfigurationFilePath); - publishGit.addChanges(':/**/package.json'); - publishGit.commit( + await publishGit.addChangesAsync(this.rushConfiguration.versionPolicyConfigurationFilePath); + await publishGit.addChangesAsync(':/**/package.json'); + await publishGit.commitAsync( this.rushConfiguration.gitVersionBumpCommitMessage || DEFAULT_PACKAGE_UPDATE_MESSAGE, !this._ignoreGitHooksParameter.value ); } if (changeLogUpdated || packageJsonUpdated) { - publishGit.push(tempBranch, !this._ignoreGitHooksParameter.value); + await publishGit.pushAsync(tempBranch, !this._ignoreGitHooksParameter.value); // Now merge to target branch. - publishGit.fetch(); - publishGit.checkout(targetBranch); - publishGit.pull(!this._ignoreGitHooksParameter.value); - publishGit.merge(tempBranch, !this._ignoreGitHooksParameter.value); - publishGit.push(targetBranch, !this._ignoreGitHooksParameter.value); - publishGit.deleteBranch(tempBranch, true, !this._ignoreGitHooksParameter.value); + await publishGit.fetchAsync(); + await publishGit.checkoutAsync(targetBranch); + await publishGit.pullAsync(!this._ignoreGitHooksParameter.value); + await publishGit.mergeAsync(tempBranch, !this._ignoreGitHooksParameter.value); + await publishGit.pushAsync(targetBranch, !this._ignoreGitHooksParameter.value); + await publishGit.deleteBranchAsync(tempBranch, true, !this._ignoreGitHooksParameter.value); } else { // skip commits - publishGit.fetch(); - publishGit.checkout(targetBranch); - publishGit.deleteBranch(tempBranch, false, !this._ignoreGitHooksParameter.value); + await publishGit.fetchAsync(); + await publishGit.checkoutAsync(targetBranch); + await publishGit.deleteBranchAsync(tempBranch, false, !this._ignoreGitHooksParameter.value); } } } diff --git a/libraries/rush-lib/src/cli/test/Cli.test.ts b/libraries/rush-lib/src/cli/test/Cli.test.ts index cbd3185a77c..2da2e3a55ff 100644 --- a/libraries/rush-lib/src/cli/test/Cli.test.ts +++ b/libraries/rush-lib/src/cli/test/Cli.test.ts @@ -6,12 +6,12 @@ import * as path from 'path'; import { Utilities } from '../../utilities/Utilities'; describe('CLI', () => { - it('should not fail when there is no rush.json', () => { + it('should not fail when there is no rush.json', async () => { const workingDir: string = '/'; const startPath: string = path.resolve(__dirname, '../../../lib-commonjs/start.js'); - expect(() => { - Utilities.executeCommand({ + await expect(async () => { + await Utilities.executeCommandAsync({ command: 'node', args: [startPath], workingDirectory: workingDir, @@ -20,12 +20,12 @@ describe('CLI', () => { }).not.toThrow(); }); - it('rushx should pass args to scripts', () => { + it('rushx should pass args to scripts', async () => { // Invoke "rushx" const startPath: string = path.resolve(__dirname, '../../../lib-commonjs/startx.js'); // Run "rushx show-args 1 2 -x" in the "repo/rushx-project" folder - const output: string = Utilities.executeCommandAndCaptureOutput( + const output: string = await Utilities.executeCommandAndCaptureOutputAsync( 'node', [startPath, 'show-args', '1', '2', '-x'], `${__dirname}/repo/rushx-project` @@ -38,11 +38,11 @@ describe('CLI', () => { expect(lastLine).toEqual('build.js: ARGS=["1","2","-x"]'); }); - it('rushx should fail in un-rush project', () => { + it('rushx should fail in un-rush project', async () => { // Invoke "rushx" const startPath: string = path.resolve(__dirname, '../../../lib-commonjs/startx.js'); - const output = Utilities.executeCommandAndCaptureOutput( + const output: string = await Utilities.executeCommandAndCaptureOutputAsync( 'node', [startPath, 'show-args', '1', '2', '-x'], `${__dirname}/repo/rushx-not-in-rush-project` diff --git a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts index 207b855ad38..b14510c8dee 100644 --- a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts +++ b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts @@ -20,11 +20,7 @@ import './mockRushCommandLineParser'; import { FileSystem, JsonFile, Path } from '@rushstack/node-core-library'; import { Autoinstaller } from '../../logic/Autoinstaller'; import type { ITelemetryData } from '../../logic/Telemetry'; -import type { IParserTestInstance } from './TestUtils'; -import { getDirnameInLib, getCommandLineParserInstanceAsync } from './TestUtils'; - -// eslint-disable-next-line @typescript-eslint/naming-convention -const __dirnameInLib: string = getDirnameInLib(); +import { getCommandLineParserInstanceAsync } from './TestUtils'; function pathEquals(actual: string, expected: string): void { expect(Path.convertToSlashes(actual)).toEqual(Path.convertToSlashes(expected)); @@ -44,64 +40,67 @@ describe('RushCommandLineParser', () => { describe("'build' action", () => { it(`executes the package's 'build' script`, async () => { const repoName: string = 'basicAndRunBuildActionRepo'; - const instance: IParserTestInstance = await getCommandLineParserInstanceAsync(repoName, 'build'); + const { parser, spawnMock, repoPath } = await getCommandLineParserInstanceAsync(repoName, 'build'); - await expect(instance.parser.executeAsync()).resolves.toEqual(true); + await expect(parser.executeAsync()).resolves.toEqual(true); // There should be 1 build per package - const packageCount: number = instance.spawnMock.mock.calls.length; + const packageCount: number = spawnMock.mock.calls.length; expect(packageCount).toEqual(2); // Use regex for task name in case spaces were prepended or appended to spawned command const expectedBuildTaskRegexp: RegExp = /fake_build_task_but_works_with_mock/; // eslint-disable-next-line @typescript-eslint/no-explicit-any - const firstSpawn: any[] = instance.spawnMock.mock.calls[0]; + const firstSpawn: any[] = spawnMock.mock.calls[0]; expect(firstSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(firstSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/a`); + pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/a`); // eslint-disable-next-line @typescript-eslint/no-explicit-any - const secondSpawn: any[] = instance.spawnMock.mock.calls[1]; + const secondSpawn: any[] = spawnMock.mock.calls[1]; expect(secondSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(secondSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/b`); + pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/b`); }); }); describe("'rebuild' action", () => { it(`executes the package's 'build' script`, async () => { const repoName: string = 'basicAndRunRebuildActionRepo'; - const instance: IParserTestInstance = await getCommandLineParserInstanceAsync(repoName, 'rebuild'); + const { parser, spawnMock, repoPath } = await getCommandLineParserInstanceAsync( + repoName, + 'rebuild' + ); - await expect(instance.parser.executeAsync()).resolves.toEqual(true); + await expect(parser.executeAsync()).resolves.toEqual(true); // There should be 1 build per package - const packageCount: number = instance.spawnMock.mock.calls.length; + const packageCount: number = spawnMock.mock.calls.length; expect(packageCount).toEqual(2); // Use regex for task name in case spaces were prepended or appended to spawned command const expectedBuildTaskRegexp: RegExp = /fake_build_task_but_works_with_mock/; // eslint-disable-next-line @typescript-eslint/no-explicit-any - const firstSpawn: any[] = instance.spawnMock.mock.calls[0]; + const firstSpawn: any[] = spawnMock.mock.calls[0]; expect(firstSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(firstSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/a`); + pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/a`); // eslint-disable-next-line @typescript-eslint/no-explicit-any - const secondSpawn: any[] = instance.spawnMock.mock.calls[1]; + const secondSpawn: any[] = spawnMock.mock.calls[1]; expect(secondSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(secondSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/b`); + pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/b`); }); }); }); @@ -110,64 +109,67 @@ describe('RushCommandLineParser', () => { describe("'build' action", () => { it(`executes the package's 'build' script`, async () => { const repoName: string = 'overrideRebuildAndRunBuildActionRepo'; - const instance: IParserTestInstance = await getCommandLineParserInstanceAsync(repoName, 'build'); + const { parser, spawnMock, repoPath } = await getCommandLineParserInstanceAsync(repoName, 'build'); - await expect(instance.parser.executeAsync()).resolves.toEqual(true); + await expect(parser.executeAsync()).resolves.toEqual(true); // There should be 1 build per package - const packageCount: number = instance.spawnMock.mock.calls.length; + const packageCount: number = spawnMock.mock.calls.length; expect(packageCount).toEqual(2); // Use regex for task name in case spaces were prepended or appended to spawned command const expectedBuildTaskRegexp: RegExp = /fake_build_task_but_works_with_mock/; // eslint-disable-next-line @typescript-eslint/no-explicit-any - const firstSpawn: any[] = instance.spawnMock.mock.calls[0]; + const firstSpawn: any[] = spawnMock.mock.calls[0]; expect(firstSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(firstSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/a`); + pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/a`); // eslint-disable-next-line @typescript-eslint/no-explicit-any - const secondSpawn: any[] = instance.spawnMock.mock.calls[1]; + const secondSpawn: any[] = spawnMock.mock.calls[1]; expect(secondSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(secondSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/b`); + pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/b`); }); }); describe("'rebuild' action", () => { it(`executes the package's 'rebuild' script`, async () => { const repoName: string = 'overrideRebuildAndRunRebuildActionRepo'; - const instance: IParserTestInstance = await getCommandLineParserInstanceAsync(repoName, 'rebuild'); + const { parser, spawnMock, repoPath } = await getCommandLineParserInstanceAsync( + repoName, + 'rebuild' + ); - await expect(instance.parser.executeAsync()).resolves.toEqual(true); + await expect(parser.executeAsync()).resolves.toEqual(true); // There should be 1 build per package - const packageCount: number = instance.spawnMock.mock.calls.length; + const packageCount: number = spawnMock.mock.calls.length; expect(packageCount).toEqual(2); // Use regex for task name in case spaces were prepended or appended to spawned command const expectedBuildTaskRegexp: RegExp = /fake_REbuild_task_but_works_with_mock/; // eslint-disable-next-line @typescript-eslint/no-explicit-any - const firstSpawn: any[] = instance.spawnMock.mock.calls[0]; + const firstSpawn: any[] = spawnMock.mock.calls[0]; expect(firstSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(firstSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/a`); + pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/a`); // eslint-disable-next-line @typescript-eslint/no-explicit-any - const secondSpawn: any[] = instance.spawnMock.mock.calls[1]; + const secondSpawn: any[] = spawnMock.mock.calls[1]; expect(secondSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(secondSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/b`); + pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/b`); }); }); }); @@ -176,31 +178,31 @@ describe('RushCommandLineParser', () => { describe("'build' action", () => { it(`executes the package's 'build' script`, async () => { const repoName: string = 'overrideAndDefaultBuildActionRepo'; - const instance: IParserTestInstance = await getCommandLineParserInstanceAsync(repoName, 'build'); - await expect(instance.parser.executeAsync()).resolves.toEqual(true); + const { parser, spawnMock, repoPath } = await getCommandLineParserInstanceAsync(repoName, 'build'); + await expect(parser.executeAsync()).resolves.toEqual(true); // There should be 1 build per package - const packageCount: number = instance.spawnMock.mock.calls.length; + const packageCount: number = spawnMock.mock.calls.length; expect(packageCount).toEqual(2); // Use regex for task name in case spaces were prepended or appended to spawned command const expectedBuildTaskRegexp: RegExp = /fake_build_task_but_works_with_mock/; // eslint-disable-next-line @typescript-eslint/no-explicit-any - const firstSpawn: any[] = instance.spawnMock.mock.calls[0]; + const firstSpawn: any[] = spawnMock.mock.calls[0]; expect(firstSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(firstSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/a`); + pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/a`); // eslint-disable-next-line @typescript-eslint/no-explicit-any - const secondSpawn: any[] = instance.spawnMock.mock.calls[1]; + const secondSpawn: any[] = spawnMock.mock.calls[1]; expect(secondSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(secondSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/b`); + pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/b`); }); }); @@ -208,31 +210,34 @@ describe('RushCommandLineParser', () => { it(`executes the package's 'build' script`, async () => { // broken const repoName: string = 'overrideAndDefaultRebuildActionRepo'; - const instance: IParserTestInstance = await getCommandLineParserInstanceAsync(repoName, 'rebuild'); - await expect(instance.parser.executeAsync()).resolves.toEqual(true); + const { parser, spawnMock, repoPath } = await getCommandLineParserInstanceAsync( + repoName, + 'rebuild' + ); + await expect(parser.executeAsync()).resolves.toEqual(true); // There should be 1 build per package - const packageCount: number = instance.spawnMock.mock.calls.length; + const packageCount: number = spawnMock.mock.calls.length; expect(packageCount).toEqual(2); // Use regex for task name in case spaces were prepended or appended to spawned command const expectedBuildTaskRegexp: RegExp = /fake_build_task_but_works_with_mock/; // eslint-disable-next-line @typescript-eslint/no-explicit-any - const firstSpawn: any[] = instance.spawnMock.mock.calls[0]; + const firstSpawn: any[] = spawnMock.mock.calls[0]; expect(firstSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(firstSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/a`); + pathEquals(firstSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/a`); // eslint-disable-next-line @typescript-eslint/no-explicit-any - const secondSpawn: any[] = instance.spawnMock.mock.calls[1]; + const secondSpawn: any[] = spawnMock.mock.calls[1]; expect(secondSpawn[SPAWN_ARG_ARGS]).toEqual( expect.arrayContaining([expect.stringMatching(expectedBuildTaskRegexp)]) ); expect(secondSpawn[SPAWN_ARG_OPTIONS]).toEqual(expect.any(Object)); - pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${__dirnameInLib}/${repoName}/b`); + pathEquals(secondSpawn[SPAWN_ARG_OPTIONS].cwd, `${repoPath}/b`); }); }); }); @@ -288,8 +293,8 @@ describe('RushCommandLineParser', () => { describe('in repo plugin custom flushTelemetry', () => { it('creates a custom telemetry file', async () => { const repoName: string = 'tapFlushTelemetryAndRunBuildActionRepo'; - const instance: IParserTestInstance = await getCommandLineParserInstanceAsync(repoName, 'build'); - const telemetryFilePath: string = `${instance.parser.rushConfiguration.commonTempFolder}/test-telemetry.json`; + const { parser } = await getCommandLineParserInstanceAsync(repoName, 'build'); + const telemetryFilePath: string = `${parser.rushConfiguration.commonTempFolder}/test-telemetry.json`; FileSystem.deleteFile(telemetryFilePath); /** @@ -297,14 +302,11 @@ describe('RushCommandLineParser', () => { */ jest.spyOn(Autoinstaller.prototype, 'prepareAsync').mockImplementation(async function () {}); - await expect(instance.parser.executeAsync()).resolves.toEqual(true); + await expect(parser.executeAsync()).resolves.toEqual(true); expect(FileSystem.exists(telemetryFilePath)).toEqual(true); - let telemetryStore: ITelemetryData[] = []; - expect(() => { - telemetryStore = JsonFile.load(telemetryFilePath); - }).not.toThrowError(); + const telemetryStore: ITelemetryData[] = await JsonFile.loadAsync(telemetryFilePath); expect(telemetryStore?.[0].name).toEqual('build'); expect(telemetryStore?.[0].result).toEqual('Succeeded'); }); diff --git a/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts b/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts index 4d586133f5b..387951cd5d9 100644 --- a/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts +++ b/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts @@ -21,7 +21,6 @@ jest.mock(`@rushstack/package-deps-hash`, () => { import { FileSystem, JsonFile } from '@rushstack/node-core-library'; import { Autoinstaller } from '../../logic/Autoinstaller'; import type { ITelemetryData } from '../../logic/Telemetry'; -import type { IParserTestInstance } from './TestUtils'; import { getCommandLineParserInstanceAsync, setSpawnMock } from './TestUtils'; describe('RushCommandLineParserFailureCases', () => { @@ -43,15 +42,15 @@ describe('RushCommandLineParserFailureCases', () => { }) as any); }); - const instance: IParserTestInstance = await getCommandLineParserInstanceAsync(repoName, 'build'); + const { parser } = await getCommandLineParserInstanceAsync(repoName, 'build'); - const telemetryFilePath: string = `${instance.parser.rushConfiguration.commonTempFolder}/test-telemetry.json`; + const telemetryFilePath: string = `${parser.rushConfiguration.commonTempFolder}/test-telemetry.json`; FileSystem.deleteFile(telemetryFilePath); jest.spyOn(Autoinstaller.prototype, 'prepareAsync').mockImplementation(async function () {}); setSpawnMock({ emitError: false, returnCode: 1 }); - await instance.parser.executeAsync(); + await parser.executeAsync(); await procProm; expect(process.exit).toHaveBeenCalledWith(1); diff --git a/libraries/rush-lib/src/cli/test/TestUtils.ts b/libraries/rush-lib/src/cli/test/TestUtils.ts index 323f3fff90c..69d95224657 100644 --- a/libraries/rush-lib/src/cli/test/TestUtils.ts +++ b/libraries/rush-lib/src/cli/test/TestUtils.ts @@ -1,8 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import * as path from 'path'; -import { FileSystem, PackageJsonLookup } from '@rushstack/node-core-library'; +import { AlreadyExistsBehavior, FileSystem, PackageJsonLookup } from '@rushstack/node-core-library'; import type { RushCommandLineParser as RushCommandLineParserType } from '../RushCommandLineParser'; import { FlagFile } from '../../api/FlagFile'; import { RushConstants } from '../../logic/RushConstants'; @@ -13,6 +12,7 @@ import { RushConstants } from '../../logic/RushConstants'; export interface IParserTestInstance { parser: RushCommandLineParserType; spawnMock: jest.Mock; + repoPath: string; } /** @@ -45,20 +45,8 @@ export function setSpawnMock(options?: ISpawnMockConfig): jest.Mock { return spawnMock; } -export function getDirnameInLib(): string { - // Run these tests in the /lib folder because some of them require compiled output - const projectRootFolder: string = PackageJsonLookup.instance.tryGetPackageFolderFor(__dirname)!; - const projectRootRelativeDirnamePath: string = path.relative(projectRootFolder, __dirname); - const projectRootRelativeLibDirnamePath: string = projectRootRelativeDirnamePath.replace( - /^src/, - 'lib-commonjs' - ); - const dirnameInLIb: string = `${projectRootFolder}/${projectRootRelativeLibDirnamePath}`; - return dirnameInLIb; -} - -// eslint-disable-next-line @typescript-eslint/naming-convention -const __dirnameInLib: string = getDirnameInLib(); +const PROJECT_ROOT: string = PackageJsonLookup.instance.tryGetPackageFolderFor(__dirname)!; +export const TEST_REPO_FOLDER_PATH: string = `${PROJECT_ROOT}/temp/test/unit-test-repos`; /** * Helper to set up a test instance for RushCommandLineParser. @@ -67,14 +55,21 @@ export async function getCommandLineParserInstanceAsync( repoName: string, taskName: string ): Promise { - // Run these tests in the /lib folder because some of them require compiled output - // Point to the test repo folder - const startPath: string = `${__dirnameInLib}/${repoName}`; + // Copy the test repo to a sandbox folder + const repoPath: string = `${TEST_REPO_FOLDER_PATH}/${repoName}-${performance.now()}`; + + await FileSystem.copyFilesAsync({ + sourcePath: `${__dirname}/${repoName}`, + destinationPath: repoPath, + alreadyExistsBehavior: AlreadyExistsBehavior.Error + }); // The `build` task is hard-coded to be incremental. So delete the package-deps file folder in // the test repo to guarantee the test actually runs. - FileSystem.deleteFolder(`${startPath}/a/.rush/temp`); - FileSystem.deleteFolder(`${startPath}/b/.rush/temp`); + await Promise.all([ + FileSystem.deleteFolderAsync(`${repoPath}/a/.rush/temp`), + FileSystem.deleteFolderAsync(`${repoPath}/b/.rush/temp`) + ]); const { RushCommandLineParser } = await import('../RushCommandLineParser'); @@ -82,7 +77,7 @@ export async function getCommandLineParserInstanceAsync( // to exit and clear the Rush file lock. So running multiple `it` or `describe` test blocks over the same test // repo will fail due to contention over the same lock which is kept until the test runner process // ends. - const parser: RushCommandLineParserType = new RushCommandLineParser({ cwd: startPath }); + const parser: RushCommandLineParserType = new RushCommandLineParser({ cwd: repoPath }); // Bulk tasks are hard-coded to expect install to have been completed. So, ensure the last-link.flag // file exists and is valid @@ -98,6 +93,7 @@ export async function getCommandLineParserInstanceAsync( return { parser, - spawnMock + spawnMock, + repoPath }; } diff --git a/libraries/rush-lib/src/logic/Autoinstaller.ts b/libraries/rush-lib/src/logic/Autoinstaller.ts index a48bd369cea..91d7218f33c 100644 --- a/libraries/rush-lib/src/logic/Autoinstaller.ts +++ b/libraries/rush-lib/src/logic/Autoinstaller.ts @@ -144,7 +144,7 @@ export class Autoinstaller { `Installing dependencies under ${autoinstallerFullPath}...\n` ); - Utilities.executeCommand({ + await Utilities.executeCommandAsync({ command: this._rushConfiguration.packageManagerToolFilename, args: ['install', '--frozen-lockfile'], workingDirectory: autoinstallerFullPath, @@ -225,7 +225,7 @@ export class Autoinstaller { targetNpmrcFolder: this.folderFullPath }); - Utilities.executeCommand({ + await Utilities.executeCommandAsync({ command: this._rushConfiguration.packageManagerToolFilename, args: ['install'], workingDirectory: this.folderFullPath, @@ -236,7 +236,7 @@ export class Autoinstaller { if (this._rushConfiguration.packageManager === 'npm') { this._logIfConsoleOutputIsNotRestricted(Colorize.bold('Running "npm shrinkwrap"...')); - Utilities.executeCommand({ + await Utilities.executeCommandAsync({ command: this._rushConfiguration.packageManagerToolFilename, args: ['shrinkwrap'], workingDirectory: this.folderFullPath, diff --git a/libraries/rush-lib/src/logic/Git.ts b/libraries/rush-lib/src/logic/Git.ts index e831932301e..dd1a5ef10d5 100644 --- a/libraries/rush-lib/src/logic/Git.ts +++ b/libraries/rush-lib/src/logic/Git.ts @@ -7,7 +7,7 @@ import * as path from 'path'; import * as url from 'url'; import { trueCasePathSync } from 'true-case-path'; -import { Executable, AlreadyReportedError, Path } from '@rushstack/node-core-library'; +import { Executable, AlreadyReportedError, Path, Async } from '@rushstack/node-core-library'; import { Colorize, type ITerminal } from '@rushstack/terminal'; import { ensureGitMinimumVersion } from '@rushstack/package-deps-hash'; @@ -89,42 +89,24 @@ export class Git { } } - /** - * If a Git email address is configured and is nonempty, this returns it. - * Otherwise, undefined is returned. - */ - public tryGetGitEmail(): string | undefined { - const emailResult: IResultOrError = this._tryGetGitEmail(); - if (emailResult.result !== undefined && emailResult.result.length > 0) { - return emailResult.result; - } - return undefined; - } - /** * If a Git email address is configured and is nonempty, this returns it. * Otherwise, configuration instructions are printed to the console, * and AlreadyReportedError is thrown. */ - public getGitEmail(): string { + public async getGitEmailAsync(): Promise { // Determine the user's account // Ex: "bob@example.com" - const emailResult: IResultOrError = this._tryGetGitEmail(); - if (emailResult.error) { - // eslint-disable-next-line no-console - console.log( - [ - `Error: ${emailResult.error.message}`, - 'Unable to determine your Git configuration using this command:', - '', - ' git config user.email', - '' - ].join('\n') - ); - throw new AlreadyReportedError(); - } + const emailResult: string | undefined = await this.tryGetGitEmailAsync(); + return this.validateGitEmail(emailResult); + } - if (emailResult.result === undefined || emailResult.result.length === 0) { + /** + * If the Git email address is configured and non-empty, this returns it. Otherwise + * it prints an error message and throws. + */ + public validateGitEmail(userEmail: string | undefined): string { + if (userEmail === undefined || userEmail.length === 0) { // eslint-disable-next-line no-console console.log( [ @@ -139,7 +121,7 @@ export class Git { throw new AlreadyReportedError(); } - return emailResult.result; + return userEmail; } /** @@ -154,7 +136,7 @@ export class Git { return undefined; } - public isHooksPathDefault(): boolean { + public async getIsHooksPathDefaultAsync(): Promise { const repoInfo: gitInfo.GitRepoInfo | undefined = this.getGitInfo(); if (!repoInfo?.commonGitDir) { // This should have never been called in a non-Git environment @@ -167,7 +149,7 @@ export class Git { /* ignore errors from true-case-path */ } const defaultHooksPath: string = path.resolve(commonGitDir, 'hooks'); - const hooksResult: IResultOrError = this._tryGetGitHooksPath(); + const hooksResult: IResultOrError = await this._tryGetGitHooksPathAsync(); if (hooksResult.error) { // eslint-disable-next-line no-console console.log( @@ -195,11 +177,13 @@ export class Git { return true; } - public getConfigHooksPath(): string { + public async getConfigHooksPathAsync(): Promise { let configHooksPath: string = ''; const gitPath: string = this.getGitPathOrThrow(); try { - configHooksPath = this._executeGitCommandAndCaptureOutput(gitPath, ['config', 'core.hooksPath']).trim(); + configHooksPath = ( + await this._executeGitCommandAndCaptureOutputAsync(gitPath, ['config', 'core.hooksPath']) + ).trim(); } catch (e) { // git config returns error code 1 if core.hooksPath is not set. } @@ -228,14 +212,18 @@ export class Git { return this._gitInfo; } - public getMergeBase(targetBranch: string, terminal: ITerminal, shouldFetch: boolean = false): string { + public async getMergeBaseAsync( + targetBranch: string, + terminal: ITerminal, + shouldFetch: boolean = false + ): Promise { if (shouldFetch) { this._fetchRemoteBranch(targetBranch, terminal); } const gitPath: string = this.getGitPathOrThrow(); try { - const output: string = this._executeGitCommandAndCaptureOutput(gitPath, [ + const output: string = await this._executeGitCommandAndCaptureOutputAsync(gitPath, [ '--no-optional-locks', 'merge-base', '--', @@ -256,9 +244,9 @@ export class Git { } } - public getBlobContent({ blobSpec, repositoryRoot }: IGetBlobOptions): string { + public async getBlobContentAsync({ blobSpec, repositoryRoot }: IGetBlobOptions): Promise { const gitPath: string = this.getGitPathOrThrow(); - const output: string = this._executeGitCommandAndCaptureOutput( + const output: string = await this._executeGitCommandAndCaptureOutputAsync( gitPath, ['cat-file', 'blob', blobSpec, '--'], repositoryRoot @@ -274,18 +262,18 @@ export class Git { * those in the provided {@param targetBranch}. If a {@param pathPrefix} is provided, * this function only returns results under the that path. */ - public getChangedFiles( + public async getChangedFilesAsync( targetBranch: string, terminal: ITerminal, skipFetch: boolean = false, pathPrefix?: string - ): string[] { + ): Promise { if (!skipFetch) { this._fetchRemoteBranch(targetBranch, terminal); } const gitPath: string = this.getGitPathOrThrow(); - const output: string = this._executeGitCommandAndCaptureOutput(gitPath, [ + const output: string = await this._executeGitCommandAndCaptureOutputAsync(gitPath, [ 'diff', `${targetBranch}...`, '--name-only', @@ -318,11 +306,11 @@ export class Git { * * @param rushConfiguration - rush configuration */ - public getRemoteDefaultBranch(): string { + public async getRemoteDefaultBranchAsync(): Promise { const repositoryUrls: string[] = this._rushConfiguration.repositoryUrls; if (repositoryUrls.length > 0) { const gitPath: string = this.getGitPathOrThrow(); - const output: string = this._executeGitCommandAndCaptureOutput(gitPath, ['remote']).trim(); + const output: string = (await this._executeGitCommandAndCaptureOutputAsync(gitPath, ['remote'])).trim(); const normalizedRepositoryUrls: Set = new Set(); for (const repositoryUrl of repositoryUrls) { @@ -330,28 +318,31 @@ export class Git { normalizedRepositoryUrls.add(Git.normalizeGitUrlForComparison(repositoryUrl).toUpperCase()); } - const matchingRemotes: string[] = output.split('\n').filter((remoteName) => { - if (remoteName) { - const remoteUrl: string = this._executeGitCommandAndCaptureOutput(gitPath, [ - 'remote', - 'get-url', - '--', - remoteName - ]).trim(); - - if (!remoteUrl) { - return false; + const matchingRemotes: string[] = []; + await Async.forEachAsync( + output.split('\n'), + async (remoteName) => { + if (remoteName) { + const remoteUrl: string = ( + await this._executeGitCommandAndCaptureOutputAsync(gitPath, [ + 'remote', + 'get-url', + '--', + remoteName + ]) + ).trim(); + + if (remoteUrl) { + // Also apply toUpperCase() for a case-insensitive comparison + const normalizedRemoteUrl: string = Git.normalizeGitUrlForComparison(remoteUrl).toUpperCase(); + if (normalizedRepositoryUrls.has(normalizedRemoteUrl)) { + matchingRemotes.push(remoteName); + } + } } - - // Also apply toUpperCase() for a case-insensitive comparison - const normalizedRemoteUrl: string = Git.normalizeGitUrlForComparison(remoteUrl).toUpperCase(); - if (normalizedRepositoryUrls.has(normalizedRemoteUrl)) { - return true; - } - } - - return false; - }); + }, + { concurrency: 10 } + ); if (matchingRemotes.length > 0) { if (matchingRemotes.length > 1) { @@ -385,8 +376,8 @@ export class Git { } } - public hasUncommittedChanges(): boolean { - const gitStatusEntries: Iterable = this.getGitStatus(); + public async hasUncommittedChangesAsync(): Promise { + const gitStatusEntries: Iterable = await this.getGitStatusAsync(); // eslint-disable-next-line @typescript-eslint/no-unused-vars for (const gitStatusEntry of gitStatusEntries) { // If there are any changes, return true. We only need to evaluate the first iterator entry @@ -396,8 +387,8 @@ export class Git { return false; } - public hasUnstagedChanges(): boolean { - const gitStatusEntries: Iterable = this.getGitStatus(); + public async hasUnstagedChangesAsync(): Promise { + const gitStatusEntries: Iterable = await this.getGitStatusAsync(); for (const gitStatusEntry of gitStatusEntries) { if ( gitStatusEntry.kind === 'untracked' || @@ -413,9 +404,9 @@ export class Git { /** * The list of files changed but not committed */ - public getUncommittedChanges(): ReadonlyArray { + public async getUncommittedChangesAsync(): Promise> { const result: string[] = []; - const gitStatusEntries: Iterable = this.getGitStatus(); + const gitStatusEntries: Iterable = await this.getGitStatusAsync(); for (const gitStatusEntry of gitStatusEntries) { result.push(gitStatusEntry.path); } @@ -427,10 +418,10 @@ export class Git { return this._rushConfiguration.gitTagSeparator || DEFAULT_GIT_TAG_SEPARATOR; } - public getGitStatus(): Iterable { + public async getGitStatusAsync(): Promise> { const gitPath: string = this.getGitPathOrThrow(); // See Git.test.ts for example output - const output: string = this._executeGitCommandAndCaptureOutput(gitPath, [ + const output: string = await this._executeGitCommandAndCaptureOutputAsync(gitPath, [ 'status', '--porcelain=2', '--null', @@ -509,12 +500,18 @@ export class Git { return result; } - private _tryGetGitEmail(): IResultOrError { + /** + * Returns an object containing either the result of the `git config user.email` + * command or an error. + */ + public async tryGetGitEmailAsync(): Promise { if (this._gitEmailResult === undefined) { const gitPath: string = this.getGitPathOrThrow(); try { this._gitEmailResult = { - result: this._executeGitCommandAndCaptureOutput(gitPath, ['config', 'user.email']).trim() + result: ( + await this._executeGitCommandAndCaptureOutputAsync(gitPath, ['config', 'user.email']) + ).trim() }; } catch (e) { this._gitEmailResult = { @@ -523,19 +520,32 @@ export class Git { } } - return this._gitEmailResult; + const { error, result } = this._gitEmailResult; + if (error) { + // eslint-disable-next-line no-console + console.log( + [ + `Error: ${error.message}`, + 'Unable to determine your Git configuration using this command:', + '', + ' git config user.email', + '' + ].join('\n') + ); + throw new AlreadyReportedError(); + } + + return result; } - private _tryGetGitHooksPath(): IResultOrError { + private async _tryGetGitHooksPathAsync(): Promise> { if (this._gitHooksPath === undefined) { const gitPath: string = this.getGitPathOrThrow(); try { this._gitHooksPath = { - result: this._executeGitCommandAndCaptureOutput(gitPath, [ - 'rev-parse', - '--git-path', - 'hooks' - ]).trim() + result: ( + await this._executeGitCommandAndCaptureOutputAsync(gitPath, ['rev-parse', '--git-path', 'hooks']) + ).trim() }; } catch (e) { this._gitHooksPath = { @@ -583,13 +593,13 @@ export class Git { /** * @internal */ - public _executeGitCommandAndCaptureOutput( + public async _executeGitCommandAndCaptureOutputAsync( gitPath: string, args: string[], repositoryRoot: string = this._rushConfiguration.rushJsonFolder - ): string { + ): Promise { try { - return Utilities.executeCommandAndCaptureOutput(gitPath, args, repositoryRoot); + return await Utilities.executeCommandAndCaptureOutputAsync(gitPath, args, repositoryRoot); } catch (e) { ensureGitMinimumVersion(gitPath); throw e; @@ -599,19 +609,20 @@ export class Git { * * @param ref Given a ref which can be branch name, commit hash, tag name, etc, check if it is a commit hash */ - public isRefACommit(ref: string): boolean { + public async determineIfRefIsACommitAsync(ref: string): Promise { const gitPath: string = this.getGitPathOrThrow(); try { - const output: string = this._executeGitCommandAndCaptureOutput(gitPath, ['rev-parse', '--verify', ref]); + const output: string = await this._executeGitCommandAndCaptureOutputAsync(gitPath, [ + 'rev-parse', + '--verify', + ref + ]); const result: string = output.trim(); - if (result === ref) { - return true; - } + return result === ref; } catch (e) { // assume not a commit return false; } - return false; } } diff --git a/libraries/rush-lib/src/logic/PackageJsonUpdater.ts b/libraries/rush-lib/src/logic/PackageJsonUpdater.ts index f9ad3706b80..64bad74cf6b 100644 --- a/libraries/rush-lib/src/logic/PackageJsonUpdater.ts +++ b/libraries/rush-lib/src/logic/PackageJsonUpdater.ts @@ -666,7 +666,7 @@ export class PackageJsonUpdater { commandArgs = ['view', packageName, 'versions', '--json']; } - const allVersions: string = Utilities.executeCommandAndCaptureOutput( + const allVersions: string = await Utilities.executeCommandAndCaptureOutputAsync( this._rushConfiguration.packageManagerToolFilename, commandArgs, this._rushConfiguration.commonTempFolder @@ -727,10 +727,12 @@ export class PackageJsonUpdater { commandArgs = ['view', `${packageName}@latest`, 'version']; } - selectedVersion = Utilities.executeCommandAndCaptureOutput( - this._rushConfiguration.packageManagerToolFilename, - commandArgs, - this._rushConfiguration.commonTempFolder + selectedVersion = ( + await Utilities.executeCommandAndCaptureOutputAsync( + this._rushConfiguration.packageManagerToolFilename, + commandArgs, + this._rushConfiguration.commonTempFolder + ) ).trim(); } diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index 19d48f6a5b0..bd128fe2657 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -223,9 +223,10 @@ export class ProjectChangeAnalyzer { const repoRoot: string = getRepoRoot(rushConfiguration.rushJsonFolder); // if the given targetBranchName is a commit, we assume it is the merge base - const mergeCommit: string = this._git.isRefACommit(targetBranchName) + const IsTargetBranchACommit: boolean = await this._git.determineIfRefIsACommitAsync(targetBranchName); + const mergeCommit: string = IsTargetBranchACommit ? targetBranchName - : this._git.getMergeBase(targetBranchName, terminal, shouldFetch); + : await this._git.getMergeBaseAsync(targetBranchName, terminal, shouldFetch); const repoChanges: Map = getRepoChanges(repoRoot, mergeCommit, gitPath); @@ -256,7 +257,7 @@ export class ProjectChangeAnalyzer { throw new Error(`Unable to obtain current shrinkwrap file.`); } - const oldShrinkwrapText: string = this._git.getBlobContent({ + const oldShrinkwrapText: string = await this._git.getBlobContentAsync({ // : syntax: https://git-scm.com/docs/gitrevisions blobSpec: `${mergeCommit}:${shrinkwrapFile}`, repositoryRoot: repoRoot diff --git a/libraries/rush-lib/src/logic/PublishGit.ts b/libraries/rush-lib/src/logic/PublishGit.ts index 53e9c033993..ba70185efa1 100644 --- a/libraries/rush-lib/src/logic/PublishGit.ts +++ b/libraries/rush-lib/src/logic/PublishGit.ts @@ -19,7 +19,7 @@ export class PublishGit { this._gitTagSeparator = git.getTagSeparator(); } - public checkout(branchName: string | undefined, createBranch: boolean = false): void { + public async checkoutAsync(branchName: string | undefined, createBranch: boolean = false): Promise { const params: string[] = ['checkout']; if (createBranch) { params.push('-b'); @@ -27,11 +27,11 @@ export class PublishGit { params.push(branchName || DUMMY_BRANCH_NAME); - PublishUtilities.execCommand(!!this._targetBranch, this._gitPath, params); + await PublishUtilities.execCommandAsync(!!this._targetBranch, this._gitPath, params); } - public merge(branchName: string, verify: boolean = false): void { - PublishUtilities.execCommand(!!this._targetBranch, this._gitPath, [ + public async mergeAsync(branchName: string, verify: boolean = false): Promise { + await PublishUtilities.execCommandAsync(!!this._targetBranch, this._gitPath, [ 'merge', branchName, '--no-edit', @@ -39,18 +39,22 @@ export class PublishGit { ]); } - public deleteBranch( + public async deleteBranchAsync( branchName: string | undefined, hasRemote: boolean = true, verify: boolean = false - ): void { + ): Promise { if (!branchName) { branchName = DUMMY_BRANCH_NAME; } - PublishUtilities.execCommand(!!this._targetBranch, this._gitPath, ['branch', '-d', branchName]); + await PublishUtilities.execCommandAsync(!!this._targetBranch, this._gitPath, [ + 'branch', + '-d', + branchName + ]); if (hasRemote) { - PublishUtilities.execCommand(!!this._targetBranch, this._gitPath, [ + await PublishUtilities.execCommandAsync(!!this._targetBranch, this._gitPath, [ 'push', 'origin', '--delete', @@ -60,7 +64,7 @@ export class PublishGit { } } - public pull(verify: boolean = false): void { + public async pullAsync(verify: boolean = false): Promise { const params: string[] = ['pull', 'origin']; if (this._targetBranch) { params.push(this._targetBranch); @@ -69,16 +73,16 @@ export class PublishGit { params.push('--no-verify'); } - PublishUtilities.execCommand(!!this._targetBranch, this._gitPath, params); + await PublishUtilities.execCommandAsync(!!this._targetBranch, this._gitPath, params); } - public fetch(): void { - PublishUtilities.execCommand(!!this._targetBranch, this._gitPath, ['fetch', 'origin']); + public async fetchAsync(): Promise { + await PublishUtilities.execCommandAsync(!!this._targetBranch, this._gitPath, ['fetch', 'origin']); } - public addChanges(pathspec?: string, workingDirectory?: string): void { + public async addChangesAsync(pathspec?: string, workingDirectory?: string): Promise { const files: string = pathspec ? pathspec : '.'; - PublishUtilities.execCommand( + await PublishUtilities.execCommandAsync( !!this._targetBranch, this._gitPath, ['add', files], @@ -86,20 +90,20 @@ export class PublishGit { ); } - public addTag( + public async addTagAsync( shouldExecute: boolean, packageName: string, packageVersion: string, commitId: string | undefined, preReleaseName?: string - ): void { + ): Promise { // Tagging only happens if we're publishing to real NPM and committing to git. const tagName: string = PublishUtilities.createTagname( packageName, packageVersion, this._gitTagSeparator ); - PublishUtilities.execCommand(!!this._targetBranch && shouldExecute, this._gitPath, [ + await PublishUtilities.execCommandAsync(!!this._targetBranch && shouldExecute, this._gitPath, [ 'tag', '-a', preReleaseName ? `${tagName}-${preReleaseName}` : tagName, @@ -111,25 +115,27 @@ export class PublishGit { ]); } - public hasTag(packageConfig: RushConfigurationProject): boolean { + public async hasTagAsync(packageConfig: RushConfigurationProject): Promise { const tagName: string = PublishUtilities.createTagname( packageConfig.packageName, packageConfig.packageJson.version, this._gitTagSeparator ); - const tagOutput: string = Utilities.executeCommandAndCaptureOutput( - this._gitPath, - ['tag', '-l', tagName], - packageConfig.projectFolder, - PublishUtilities.getEnvArgs(), - true + const tagOutput: string = ( + await Utilities.executeCommandAndCaptureOutputAsync( + this._gitPath, + ['tag', '-l', tagName], + packageConfig.projectFolder, + PublishUtilities.getEnvArgs(), + true + ) ).replace(/(\r\n|\n|\r)/gm, ''); return tagOutput === tagName; } - public commit(commitMessage: string, verify: boolean = false): void { - PublishUtilities.execCommand(!!this._targetBranch, this._gitPath, [ + public async commitAsync(commitMessage: string, verify: boolean = false): Promise { + await PublishUtilities.execCommandAsync(!!this._targetBranch, this._gitPath, [ 'commit', '-m', commitMessage, @@ -137,8 +143,8 @@ export class PublishGit { ]); } - public push(branchName: string | undefined, verify: boolean = false): void { - PublishUtilities.execCommand( + public async pushAsync(branchName: string | undefined, verify: boolean = false): Promise { + await PublishUtilities.execCommandAsync( !!this._targetBranch, this._gitPath, // We append "--no-verify" to prevent Git hooks from running. For example, people may diff --git a/libraries/rush-lib/src/logic/PublishUtilities.ts b/libraries/rush-lib/src/logic/PublishUtilities.ts index d85031cf119..6feb9070111 100644 --- a/libraries/rush-lib/src/logic/PublishUtilities.ts +++ b/libraries/rush-lib/src/logic/PublishUtilities.ts @@ -253,14 +253,14 @@ export class PublishUtilities { * @param secretSubstring -- if specified, a substring to be replaced by `<>` to avoid printing secrets * on the console */ - public static execCommand( + public static async execCommandAsync( shouldExecute: boolean, command: string, args: string[] = [], workingDirectory: string = process.cwd(), environment?: IEnvironment, secretSubstring?: string - ): void { + ): Promise { let relativeDirectory: string = path.relative(process.cwd(), workingDirectory); if (relativeDirectory) { @@ -280,7 +280,7 @@ export class PublishUtilities { ); if (shouldExecute) { - Utilities.executeCommand({ + await Utilities.executeCommandAsync({ command, args, workingDirectory, diff --git a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts index ed2875f72df..4b5eccf2c3a 100644 --- a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts +++ b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts @@ -323,7 +323,7 @@ export abstract class BaseInstallManager { // Check the policies await PolicyValidator.validatePolicyAsync(this.rushConfiguration, subspace, this.options); - this._installGitHooks(); + await this._installGitHooksAsync(); const approvedPackagesChecker: ApprovedPackagesChecker = new ApprovedPackagesChecker( this.rushConfiguration @@ -541,7 +541,7 @@ export abstract class BaseInstallManager { /** * Git hooks are only installed if the repo opts in by including files in /common/git-hooks */ - private _installGitHooks(): void { + private async _installGitHooksAsync(): Promise { const hookSource: string = path.join(this.rushConfiguration.commonFolder, 'git-hooks'); const git: Git = new Git(this.rushConfiguration); const hookDestination: string | undefined = git.getHooksFolder(); @@ -554,7 +554,8 @@ export abstract class BaseInstallManager { // eslint-disable-next-line no-console console.log('\n' + Colorize.bold('Found files in the "common/git-hooks" folder.')); - if (!git.isHooksPathDefault()) { + if (!(await git.getIsHooksPathDefaultAsync())) { + const hooksPath: string = await git.getConfigHooksPathAsync(); const color: (str: string) => string = this.options.bypassPolicy ? Colorize.yellow : Colorize.red; // eslint-disable-next-line no-console console.error( @@ -562,7 +563,7 @@ export abstract class BaseInstallManager { [ ' ', `Rush cannot install the "common/git-hooks" scripts because your Git configuration `, - `specifies "core.hooksPath=${git.getConfigHooksPath()}". You can remove the setting by running:`, + `specifies "core.hooksPath=${hooksPath}". You can remove the setting by running:`, ' ', ' git config --unset core.hooksPath', ' ' @@ -957,10 +958,10 @@ ${gitLfsHookHandling} } private _syncTempShrinkwrap(subspace: Subspace, shrinkwrapFile: BaseShrinkwrapFile | undefined): void { - const commitedShrinkwrapFileName: string = subspace.getCommittedShrinkwrapFilename(); + const committedShrinkwrapFileName: string = subspace.getCommittedShrinkwrapFilename(); if (shrinkwrapFile) { - Utilities.syncFile(commitedShrinkwrapFileName, subspace.getTempShrinkwrapFilename()); - Utilities.syncFile(commitedShrinkwrapFileName, subspace.getTempShrinkwrapPreinstallFilename()); + Utilities.syncFile(committedShrinkwrapFileName, subspace.getTempShrinkwrapFilename()); + Utilities.syncFile(committedShrinkwrapFileName, subspace.getTempShrinkwrapPreinstallFilename()); } else { // Otherwise delete the temporary file FileSystem.deleteFile(subspace.getTempShrinkwrapFilename()); diff --git a/libraries/rush-lib/src/logic/installManager/InstallHelpers.ts b/libraries/rush-lib/src/logic/installManager/InstallHelpers.ts index 388c5c85c2a..121a47d1d70 100644 --- a/libraries/rush-lib/src/logic/installManager/InstallHelpers.ts +++ b/libraries/rush-lib/src/logic/installManager/InstallHelpers.ts @@ -177,7 +177,7 @@ export class InstallHelpers { ); // note that this will remove the last-install flag from the directory - Utilities.installPackageInDirectory({ + await Utilities.installPackageInDirectoryAsync({ directory: packageManagerToolFolder, packageName: packageManager, version: rushConfiguration.packageManagerToolVersion, diff --git a/libraries/rush-lib/src/logic/installManager/RushInstallManager.ts b/libraries/rush-lib/src/logic/installManager/RushInstallManager.ts index 76588b61992..87b3bd79646 100644 --- a/libraries/rush-lib/src/logic/installManager/RushInstallManager.ts +++ b/libraries/rush-lib/src/logic/installManager/RushInstallManager.ts @@ -524,7 +524,7 @@ export class RushInstallManager extends BaseInstallManager { const args: string[] = ['prune']; this.pushConfigurationArgs(args, this.options, subspace); - Utilities.executeCommandWithRetry( + await Utilities.executeCommandWithRetryAsync( { command: packageManagerFilename, args: args, @@ -605,7 +605,7 @@ export class RushInstallManager extends BaseInstallManager { ); } - Utilities.executeCommandWithRetry( + await Utilities.executeCommandWithRetryAsync( { command: packageManagerFilename, args: installArgs, @@ -634,7 +634,7 @@ export class RushInstallManager extends BaseInstallManager { console.log('\n' + Colorize.bold('Running "npm shrinkwrap"...')); const npmArgs: string[] = ['shrinkwrap']; this.pushConfigurationArgs(npmArgs, this.options, subspace); - Utilities.executeCommand({ + await Utilities.executeCommandAsync({ command: this.rushConfiguration.packageManagerToolFilename, args: npmArgs, workingDirectory: this.rushConfiguration.commonTempFolder diff --git a/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts b/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts index b4d78794c58..1a59282671c 100644 --- a/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts +++ b/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts @@ -463,16 +463,16 @@ export class WorkspaceInstallManager extends BaseInstallManager { } : undefined; try { - await Utilities.executeCommandAndProcessOutputWithRetryAsync( + await Utilities.executeCommandWithRetryAsync( { command: packageManagerFilename, args: installArgs, workingDirectory: subspace.getSubspaceTempFolder(), environment: packageManagerEnv, - suppressOutput: false + suppressOutput: false, + onStdoutStreamChunk: onPnpmStdoutChunk }, this.options.maxInstallAttempts, - onPnpmStdoutChunk, () => { if (this.rushConfiguration.packageManager === 'pnpm') { this._terminal.writeWarningLine(`Deleting the "node_modules" folder`); diff --git a/libraries/rush-lib/src/logic/policy/GitEmailPolicy.ts b/libraries/rush-lib/src/logic/policy/GitEmailPolicy.ts index 5a995b3ce03..2a58ce2328d 100644 --- a/libraries/rush-lib/src/logic/policy/GitEmailPolicy.ts +++ b/libraries/rush-lib/src/logic/policy/GitEmailPolicy.ts @@ -10,7 +10,10 @@ import { Git } from '../Git'; import { RushConstants } from '../RushConstants'; import type { IPolicyValidatorOptions } from './PolicyValidator'; -export function validate(rushConfiguration: RushConfiguration, options: IPolicyValidatorOptions): void { +export async function validateAsync( + rushConfiguration: RushConfiguration, + options: IPolicyValidatorOptions +): Promise { const git: Git = new Git(rushConfiguration); if (!git.isGitPresent()) { @@ -31,10 +34,11 @@ export function validate(rushConfiguration: RushConfiguration, options: IPolicyV return; } + let userEmail: string | undefined = await git.tryGetGitEmailAsync(); // If there isn't a Git policy, then we don't care whether the person configured - // a Git email address at all. This helps people who don't + // a Git email address at all. if (rushConfiguration.gitAllowedEmailRegExps.length === 0) { - if (git.tryGetGitEmail() === undefined) { + if (userEmail === undefined) { return; } @@ -42,9 +46,8 @@ export function validate(rushConfiguration: RushConfiguration, options: IPolicyV // sanity checks (e.g. no spaces in the address). } - let userEmail: string; try { - userEmail = git.getGitEmail(); + userEmail = git.validateGitEmail(userEmail); // sanity check; a valid email should not contain any whitespace // if this fails, then we have another issue to report @@ -97,10 +100,8 @@ export function validate(rushConfiguration: RushConfiguration, options: IPolicyV // Ex. "Example Name " let fancyEmail: string = Colorize.cyan(userEmail); try { - const userName: string = Utilities.executeCommandAndCaptureOutput( - git.gitPath!, - ['config', 'user.name'], - '.' + const userName: string = ( + await Utilities.executeCommandAndCaptureOutputAsync(git.gitPath!, ['config', 'user.name'], '.') ).trim(); if (userName) { fancyEmail = `${userName} <${fancyEmail}>`; diff --git a/libraries/rush-lib/src/logic/policy/PolicyValidator.ts b/libraries/rush-lib/src/logic/policy/PolicyValidator.ts index 21aebbc3c90..a035a2ac802 100644 --- a/libraries/rush-lib/src/logic/policy/PolicyValidator.ts +++ b/libraries/rush-lib/src/logic/policy/PolicyValidator.ts @@ -19,7 +19,7 @@ export async function validatePolicyAsync( options: IPolicyValidatorOptions ): Promise { if (!options.bypassPolicy) { - GitEmailPolicy.validate(rushConfiguration, options); + await GitEmailPolicy.validateAsync(rushConfiguration, options); await EnvironmentPolicy.validateAsync(rushConfiguration, options); if (!options.allowShrinkwrapUpdates) { // Don't validate the shrinkwrap if updates are allowed, as it's likely to change diff --git a/libraries/rush-lib/src/logic/test/Git.test.ts b/libraries/rush-lib/src/logic/test/Git.test.ts index 166ceee6068..0e8c7fec837 100644 --- a/libraries/rush-lib/src/logic/test/Git.test.ts +++ b/libraries/rush-lib/src/logic/test/Git.test.ts @@ -35,24 +35,26 @@ describe(Git.name, () => { }); }); - describe(Git.prototype.getGitStatus.name, () => { - function getGitStatusEntriesForCommandOutput(outputSections: string[]): IGitStatusEntry[] { + describe(Git.prototype.getGitStatusAsync.name, () => { + async function getGitStatusEntriesForCommandOutputAsync( + outputSections: string[] + ): Promise { const gitInstance: Git = new Git({ rushJsonFolder: '/repo/root' } as RushConfiguration); jest.spyOn(gitInstance, 'getGitPathOrThrow').mockReturnValue('/git/bin/path'); jest - .spyOn(gitInstance, '_executeGitCommandAndCaptureOutput') - .mockImplementation((gitPath: string, args: string[]) => { + .spyOn(gitInstance, '_executeGitCommandAndCaptureOutputAsync') + .mockImplementation(async (gitPath: string, args: string[]) => { expect(gitPath).toEqual('/git/bin/path'); expect(args).toEqual(['status', '--porcelain=2', '--null', '--ignored=no']); return outputSections.join('\0'); }); - return Array.from(gitInstance.getGitStatus()); + return Array.from(await gitInstance.getGitStatusAsync()); } - it('parses a git status', () => { - expect( - getGitStatusEntriesForCommandOutput([ + it('parses a git status', async () => { + await expect( + getGitStatusEntriesForCommandOutputAsync([ // Staged add '1 A. N... 000000 100644 100644 0000000000000000000000000000000000000000 a171a25d2c978ba071959f39dbeaa339fe84f768 path/a.ts', // Modifications, some staged and some unstaged @@ -73,7 +75,7 @@ describe(Git.name, () => { '1 AM N... 000000 100644 100644 0000000000000000000000000000000000000000 9d9ab4adc79c591c0aa72f7fd29a008c80893e3e path/h.ts', '' ]) - ).toMatchInlineSnapshot(` + ).resolves.toMatchInlineSnapshot(` Array [ Object { "headFileMode": "000000", @@ -183,39 +185,42 @@ describe(Git.name, () => { `); }); - it('throws with invalid git output', () => { - expect(() => - getGitStatusEntriesForCommandOutput(['1 A. N... 000000 100644 100644 000000000000000000']) - ).toThrowErrorMatchingInlineSnapshot(`"Unexpected end of git status output after position 31"`); + it('throws with invalid git output', async () => { + await expect(() => + getGitStatusEntriesForCommandOutputAsync(['1 A. N... 000000 100644 100644 000000000000000000']) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Unexpected end of git status output after position 31"`); }); }); - describe(Git.prototype.isRefACommit.name, () => { + describe(Git.prototype.determineIfRefIsACommitAsync.name, () => { const commit = `d9bc1881959b9e44d846655521cd055fcf713f4d`; - function getMockedGitIsRefACommit(ref: string): boolean { + async function getMockedGitIsRefACommitAsync(ref: string): Promise { const gitInstance: Git = new Git({ rushJsonFolder: '/repo/root' } as RushConfiguration); jest.spyOn(gitInstance, 'getGitPathOrThrow').mockReturnValue('/git/bin/path'); jest - .spyOn(gitInstance, '_executeGitCommandAndCaptureOutput') - .mockImplementation((gitPath: string, args: string[]) => { + .spyOn(gitInstance, '_executeGitCommandAndCaptureOutputAsync') + .mockImplementation(async (gitPath: string, args: string[]) => { expect(gitPath).toEqual('/git/bin/path'); expect(args).toEqual(['rev-parse', '--verify', ref]); return commit; }); - return gitInstance.isRefACommit(ref); + return await gitInstance.determineIfRefIsACommitAsync(ref); } - it('Returns true for commit ref', () => { - expect(getMockedGitIsRefACommit(commit)).toBe(true); + it('Returns true for commit ref', async () => { + await expect(getMockedGitIsRefACommitAsync(commit)).resolves.toBe(true); }); - it('Returns false for branch ref', () => { - expect(getMockedGitIsRefACommit('kenrick/skip-merge-base')).toBe(false); + + it('Returns false for branch ref', async () => { + await expect(getMockedGitIsRefACommitAsync('kenrick/skip-merge-base')).resolves.toBe(false); }); - it('Returns false for ref that is a tag', () => { - expect(getMockedGitIsRefACommit('testing-tag-v1.2.3')).toBe(false); + + it('Returns false for ref that is a tag', async () => { + await expect(getMockedGitIsRefACommitAsync('testing-tag-v1.2.3')).resolves.toBe(false); }); - it('Returns false for ref that is other string', () => { - expect(getMockedGitIsRefACommit('HEAD')).toBe(false); + + it('Returns false for ref that is other string', async () => { + await expect(getMockedGitIsRefACommitAsync('HEAD')).resolves.toBe(false); }); }); }); diff --git a/libraries/rush-lib/src/logic/test/PublishGit.test.ts b/libraries/rush-lib/src/logic/test/PublishGit.test.ts index af257b24bc8..75db6cc8405 100644 --- a/libraries/rush-lib/src/logic/test/PublishGit.test.ts +++ b/libraries/rush-lib/src/logic/test/PublishGit.test.ts @@ -19,7 +19,7 @@ describe('PublishGit Test', () => { }); beforeEach(() => { - execCommand = jest.spyOn(PublishUtilities, 'execCommand').mockImplementation(() => { + execCommand = jest.spyOn(PublishUtilities, 'execCommandAsync').mockImplementation(async () => { /* no-op */ }); @@ -33,8 +33,8 @@ describe('PublishGit Test', () => { execCommand.mockClear(); }); - it('Test git with no command line arg tag', () => { - publishGit.addTag( + it('Test git with no command line arg tag', async () => { + await publishGit.addTagAsync( false, 'project1', '2', @@ -45,8 +45,8 @@ describe('PublishGit Test', () => { expect(execCommand).toBeCalledWith(false, gitPath, ['tag', '-a', `project1_v2`, '-m', 'project1 v2']); }); - it('Test git with command line arg tag', () => { - publishGit.addTag( + it('Test git with command line arg tag', async () => { + await publishGit.addTagAsync( false, 'project1', '2', diff --git a/libraries/rush-lib/src/utilities/Npm.ts b/libraries/rush-lib/src/utilities/Npm.ts index 905260ebaa9..3e368cf19f0 100644 --- a/libraries/rush-lib/src/utilities/Npm.ts +++ b/libraries/rush-lib/src/utilities/Npm.ts @@ -5,15 +5,15 @@ import { Utilities } from './Utilities'; import * as semver from 'semver'; export class Npm { - public static publishedVersions( + public static async getPublishedVersionsAsync( packageName: string, cwd: string, env: { [key: string]: string | undefined }, extraArgs: string[] = [] - ): string[] { + ): Promise { const versions: string[] = []; try { - const packageTime: string = Utilities.executeCommandAndCaptureOutput( + const packageTime: string = await Utilities.executeCommandAndCaptureOutputAsync( 'npm', ['view', packageName, 'time', '--json', ...extraArgs], cwd, @@ -30,7 +30,7 @@ export class Npm { // eslint-disable-next-line no-console console.log(`Package ${packageName} time value does not exist. Fall back to versions.`); // time property does not exist. It happens sometimes. Fall back to versions. - const packageVersions: string = Utilities.executeCommandAndCaptureOutput( + const packageVersions: string = await Utilities.executeCommandAndCaptureOutputAsync( 'npm', ['view', packageName, 'versions', '--json', ...extraArgs], cwd, diff --git a/libraries/rush-lib/src/utilities/Utilities.ts b/libraries/rush-lib/src/utilities/Utilities.ts index 9e6a12ab949..9e7005b67af 100644 --- a/libraries/rush-lib/src/utilities/Utilities.ts +++ b/libraries/rush-lib/src/utilities/Utilities.ts @@ -12,7 +12,8 @@ import { FileSystem, FileConstants, type FileSystemStats, - SubprocessTerminator + SubprocessTerminator, + Executable } from '@rushstack/node-core-library'; import type { RushConfiguration } from '../api/RushConfiguration'; @@ -33,7 +34,7 @@ export interface IEnvironment { } /** - * Options for Utilities.executeCommand(). + * Options for {@link Utilities.executeCommandAsync}. */ export interface IExecuteCommandOptions { command: string; @@ -42,10 +43,14 @@ export interface IExecuteCommandOptions { environment?: IEnvironment; suppressOutput?: boolean; keepEnvironment?: boolean; + /** + * Note that this takes precedence over {@link IExecuteCommandOptions.suppressOutput} + */ + onStdoutStreamChunk?: (chunk: string) => string | void; } /** - * Options for Utilities.installPackageInDirectory(). + * Options for {@link Utilities.installPackageInDirectoryAsync}. */ export interface IInstallPackageInDirectoryOptions { directory: string; @@ -144,6 +149,11 @@ interface ICreateEnvironmentForRushCommandOptions { pathOptions?: ICreateEnvironmentForRushCommandPathOptions; } +interface IExecuteCommandResult { + stdout: string; + stderr: string; +} + export class Utilities { public static syncNpmrc: typeof syncNpmrc = syncNpmrc; @@ -302,119 +312,68 @@ export class Utilities { * Executes the command with the specified command-line parameters, and waits for it to complete. * The current directory will be set to the specified workingDirectory. */ - public static executeCommand(options: IExecuteCommandOptions): void { - Utilities._executeCommandInternal( - options.command, - options.args, - options.workingDirectory, - options.suppressOutput ? undefined : [0, 1, 2], - options.environment, - options.keepEnvironment - ); - } - - /** - * Executes the command with the specified command-line parameters, and waits for it to complete. - * The current directory will be set to the specified workingDirectory. - * - * It's basically the same as executeCommand() except that it returns a Promise. - */ - public static async executeCommandAndInspectOutputAsync( - options: IExecuteCommandOptions, - onStdoutStreamChunk?: (chunk: string) => string | void, - // eslint-disable-next-line @rushstack/no-new-null - onExit?: (exitCode: number | null, signal: NodeJS.Signals | null) => void - ): Promise { - await Utilities._executeCommandAndInspectOutputInternalAsync( - options.command, - options.args, - options.workingDirectory, - options.suppressOutput ? undefined : onStdoutStreamChunk ? ['inherit', 'pipe', 'inherit'] : [0, 1, 2], - options.environment, - options.keepEnvironment, + public static async executeCommandAsync({ + command, + args, + workingDirectory, + suppressOutput, + onStdoutStreamChunk, + environment, + keepEnvironment + }: IExecuteCommandOptions): Promise { + await Utilities._executeCommandInternalAsync({ + command, + args, + workingDirectory, + stdio: onStdoutStreamChunk + ? // Inherit the stdin and stderr streams, but pipe the stdout stream, which will then be piped + // to the process's stdout after being intercepted by the onStdoutStreamChunk callback. + ['inherit', 'pipe', 'inherit'] + : suppressOutput + ? // If the output is being suppressed, create pipes for all streams to prevent the child process + // from printing to the parent process's (this process's) stdout/stderr, but allow the stdout and + // stderr to be inspected if an error occurs. + // TODO: Consider ignoring stdout and stdin and only piping stderr for inspection on error. + ['pipe', 'pipe', 'pipe'] + : // If the output is not being suppressed or intercepted, inherit all streams from the parent process. + ['inherit', 'inherit', 'inherit'], + environment, + keepEnvironment, onStdoutStreamChunk, - onExit - ); + captureOutput: false + }); } /** * Executes the command with the specified command-line parameters, and waits for it to complete. * The current directory will be set to the specified workingDirectory. */ - public static executeCommandAndCaptureOutput( + public static async executeCommandAndCaptureOutputAsync( command: string, args: string[], workingDirectory: string, environment?: IEnvironment, keepEnvironment: boolean = false - ): string { - const result: child_process.SpawnSyncReturns = Utilities._executeCommandInternal( + ): Promise { + const { stdout } = await Utilities._executeCommandInternalAsync({ command, args, workingDirectory, - ['pipe', 'pipe', 'pipe'], + stdio: ['pipe', 'pipe', 'pipe'], environment, - keepEnvironment - ); - - return result.stdout.toString(); - } - - /** - * Attempts to run Utilities.executeCommand() up to maxAttempts times before giving up. - */ - public static executeCommandWithRetry( - options: IExecuteCommandOptions, - maxAttempts: number, - retryCallback?: () => void - ): void { - if (maxAttempts < 1) { - throw new Error('The maxAttempts parameter cannot be less than 1'); - } - - let attemptNumber: number = 1; - - for (;;) { - try { - Utilities.executeCommand(options); - } catch (error) { - // eslint-disable-next-line no-console - console.log('\nThe command failed:'); - // eslint-disable-next-line no-console - console.log(` ${options.command} ` + options.args.join(' ')); - // eslint-disable-next-line no-console - console.log(`ERROR: ${(error as Error).toString()}`); - - if (attemptNumber < maxAttempts) { - ++attemptNumber; - // eslint-disable-next-line no-console - console.log(`Trying again (attempt #${attemptNumber})...\n`); - if (retryCallback) { - retryCallback(); - } - - continue; - } else { - // eslint-disable-next-line no-console - console.error(`Giving up after ${attemptNumber} attempts\n`); - throw error; - } - } + keepEnvironment, + captureOutput: true + }); - break; - } + return stdout; } /** * Attempts to run Utilities.executeCommand() up to maxAttempts times before giving up. - * Using `onStdoutStreamChunk` to process the output of the command. - * - * Note: This is similar to {@link executeCommandWithRetry} except that it returns a Promise and provides a callback to process the output. */ - public static async executeCommandAndProcessOutputWithRetryAsync( + public static async executeCommandWithRetryAsync( options: IExecuteCommandOptions, maxAttempts: number, - onStdoutStreamChunk?: (chunk: string) => string | void, retryCallback?: () => void ): Promise { if (maxAttempts < 1) { @@ -425,12 +384,13 @@ export class Utilities { for (;;) { try { - await Utilities.executeCommandAndInspectOutputAsync(options, onStdoutStreamChunk); + await Utilities.executeCommandAsync(options); } catch (error) { // eslint-disable-next-line no-console console.log('\nThe command failed:'); + const { command, args } = options; // eslint-disable-next-line no-console - console.log(` ${options.command} ` + options.args.join(' ')); + console.log(` ${command} ` + args.join(' ')); // eslint-disable-next-line no-console console.log(`ERROR: ${(error as Error).toString()}`); @@ -464,7 +424,11 @@ export class Utilities { Utilities._executeLifecycleCommandInternal(command, child_process.spawnSync, options); if (options.handleOutput) { - Utilities._processResult(result); + Utilities._processResult({ + error: result.error, + status: result.status, + stderr: result.stderr.toString() + }); } if (result.status !== null) { @@ -509,29 +473,38 @@ export class Utilities { /** * Installs a package by name and version in the specified directory. */ - public static installPackageInDirectory(options: IInstallPackageInDirectoryOptions): void { - const directory: string = path.resolve(options.directory); - if (FileSystem.exists(directory)) { + public static async installPackageInDirectoryAsync({ + packageName, + version, + tempPackageTitle, + commonRushConfigFolder, + maxInstallAttempts, + suppressOutput, + directory + }: IInstallPackageInDirectoryOptions): Promise { + directory = path.resolve(directory); + const directoryExists: boolean = await FileSystem.existsAsync(directory); + if (directoryExists) { // eslint-disable-next-line no-console console.log('Deleting old files from ' + directory); } - FileSystem.ensureEmptyFolder(directory); + await FileSystem.ensureEmptyFolderAsync(directory); const npmPackageJson: IPackageJson = { dependencies: { - [options.packageName]: options.version + [packageName]: version }, description: 'Temporary file generated by the Rush tool', - name: options.tempPackageTitle, + name: tempPackageTitle, private: true, version: '0.0.0' }; - JsonFile.save(npmPackageJson, path.join(directory, FileConstants.PackageJson)); + await JsonFile.saveAsync(npmPackageJson, path.join(directory, FileConstants.PackageJson)); - if (options.commonRushConfigFolder) { + if (commonRushConfigFolder) { Utilities.syncNpmrc({ - sourceNpmrcFolder: options.commonRushConfigFolder, + sourceNpmrcFolder: commonRushConfigFolder, targetNpmrcFolder: directory }); } @@ -540,15 +513,15 @@ export class Utilities { console.log('\nRunning "npm install" in ' + directory); // NOTE: Here we use whatever version of NPM we happen to find in the PATH - Utilities.executeCommandWithRetry( + await Utilities.executeCommandWithRetryAsync( { command: 'npm', args: ['install'], workingDirectory: directory, environment: Utilities._createEnvironmentForRushCommand({}), - suppressOutput: options.suppressOutput + suppressOutput }, - options.maxInstallAttempts + maxInstallAttempts ); } @@ -763,85 +736,20 @@ export class Utilities { /** * Executes the command with the specified command-line parameters, and waits for it to complete. * The current directory will be set to the specified workingDirectory. - * - * It's the same as _executeCommandInternal except that it returns a promise. */ - private static async _executeCommandAndInspectOutputInternalAsync( - command: string, - args: string[], - workingDirectory: string, - stdio: child_process.SpawnSyncOptions['stdio'], - environment?: IEnvironment, - keepEnvironment: boolean = false, - onStdoutStreamChunk?: (chunk: string) => string | void, - onExit?: (exitCode: number | null, signal: NodeJS.Signals | null) => void - ): Promise { - return new Promise((resolve, reject) => { - const options: child_process.SpawnOptions = { - cwd: workingDirectory, - shell: true, - stdio: stdio, - env: keepEnvironment - ? environment - : Utilities._createEnvironmentForRushCommand({ initialEnvironment: environment }) - }; - - // Only escape the command if it actually contains spaces: - const escapedCommand: string = - command.indexOf(' ') < 0 ? command : Utilities.escapeShellParameter(command); - - const escapedArgs: string[] = args.map((x) => Utilities.escapeShellParameter(x)); - - const childProcess: child_process.ChildProcess = child_process.spawn( - escapedCommand, - escapedArgs, - options - ); - - const inspectStream: Transform = new Transform({ - transform: onStdoutStreamChunk - ? ( - chunk: string | Buffer, - encoding: BufferEncoding, - callback: (error?: Error, data?: string | Buffer) => void - ) => { - const chunkString: string = chunk.toString(); - const updatedChunk: string | void = onStdoutStreamChunk(chunkString); - callback(undefined, updatedChunk ?? chunk); - } - : undefined - }); - - childProcess.on('close', (exitCode: number | null, signal: NodeJS.Signals | null) => { - onExit?.(exitCode, signal); - - // TODO: Is it possible that the childProcess is closed before the receiving the last chunks? - if (exitCode === 0) { - resolve(); - } else if (signal) { - reject(new Error(`The command was terminated by signal ${signal}`)); - } else { - // mimic the current sync version "_executeCommandInternal" behavior. - reject(new Error(`The command failed with exit code ${exitCode}`)); - } - }); - - childProcess.stdout?.pipe(inspectStream).pipe(process.stdout); - }); - } - - /** - * Executes the command with the specified command-line parameters, and waits for it to complete. - * The current directory will be set to the specified workingDirectory. - */ - private static _executeCommandInternal( - command: string, - args: string[], - workingDirectory: string, - stdio: child_process.SpawnSyncOptions['stdio'], - environment?: IEnvironment, - keepEnvironment: boolean = false - ): child_process.SpawnSyncReturns { + private static async _executeCommandInternalAsync({ + command, + args, + workingDirectory, + stdio, + environment, + keepEnvironment, + onStdoutStreamChunk, + captureOutput + }: Omit & { + stdio: child_process.SpawnSyncOptions['stdio']; + captureOutput: boolean; + }): Promise { const options: child_process.SpawnSyncOptions = { cwd: workingDirectory, shell: true, @@ -870,39 +778,62 @@ export class Utilities { const escapedArgs: string[] = args.map((x) => Utilities.escapeShellParameter(x)); - let result: child_process.SpawnSyncReturns = child_process.spawnSync( + const childProcess: child_process.ChildProcess = child_process.spawn( escapedCommand, escapedArgs, options ); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - if (result.error && (result.error as any).errno === 'ENOENT') { - // This is a workaround for GitHub issue #25330 - // https://github.com/nodejs/node-v0.x-archive/issues/25330 - // - // TODO: The fully worked out solution for this problem is now provided by the "Executable" API - // from @rushstack/node-core-library - result = child_process.spawnSync(command + '.cmd', args, options); + if (onStdoutStreamChunk) { + const inspectStream: Transform = new Transform({ + transform: onStdoutStreamChunk + ? ( + chunk: string | Buffer, + encoding: BufferEncoding, + callback: (error?: Error, data?: string | Buffer) => void + ) => { + const chunkString: string = chunk.toString(); + const updatedChunk: string | void = onStdoutStreamChunk(chunkString); + callback(undefined, updatedChunk ?? chunk); + } + : undefined + }); + + childProcess.stdout?.pipe(inspectStream).pipe(process.stdout); } - Utilities._processResult(result); - return result; + const { stdout, stderr } = await Executable.waitForExitAsync(childProcess, { + encoding: captureOutput ? 'utf8' : undefined, + throwOnNonZeroExitCode: true, + throwOnSignal: true + }); + + return { + stdout, + stderr + }; } - private static _processResult(result: child_process.SpawnSyncReturns): void { - if (result.error) { - result.error.message += '\n' + (result.stderr ? result.stderr.toString() + '\n' : ''); - throw result.error; + private static _processResult({ + error, + stderr, + status + }: { + error: Error | undefined; + stderr: string; + status: number | null; + }): void { + if (error) { + error.message += `\n${stderr}`; + if (status) { + error.message += `\nExited with status ${status}`; + } + + throw error; } - if (result.status) { - throw new Error( - 'The command failed with exit code ' + - result.status + - '\n' + - (result.stderr ? result.stderr.toString() : '') - ); + if (status) { + throw new Error(`The command failed with exit code ${status}\n${stderr}`); } } } diff --git a/libraries/rush-lib/src/utilities/test/Npm.test.ts b/libraries/rush-lib/src/utilities/test/Npm.test.ts index 83b82bfb661..6e0d7cc4552 100644 --- a/libraries/rush-lib/src/utilities/test/Npm.test.ts +++ b/libraries/rush-lib/src/utilities/test/Npm.test.ts @@ -11,7 +11,7 @@ describe(Npm.name, () => { let stub: jest.SpyInstance; beforeEach(() => { - stub = jest.spyOn(Utilities, 'executeCommandAndCaptureOutput'); + stub = jest.spyOn(Utilities, 'executeCommandAndCaptureOutputAsync'); }); afterEach(() => { @@ -19,7 +19,7 @@ describe(Npm.name, () => { stub.mockRestore(); }); - it('publishedVersions gets versions when package time is available.', () => { + it('publishedVersions gets versions when package time is available.', async () => { const json: string = `{ "modified": "2017-03-30T18:37:27.757Z", "created": "2017-01-03T20:28:10.342Z", @@ -28,9 +28,9 @@ describe(Npm.name, () => { "1.4.1": "2017-01-09T19:22:00.488Z", "2.4.0-alpha.1": "2017-03-30T18:37:27.757Z" }`; - stub.mockImplementationOnce(() => json); + stub.mockImplementationOnce(() => Promise.resolve(json)); - const versions: string[] = Npm.publishedVersions(packageName, __dirname, process.env); + const versions: string[] = await Npm.getPublishedVersionsAsync(packageName, __dirname, process.env); expect(stub).toHaveBeenCalledWith( 'npm', @@ -44,17 +44,17 @@ describe(Npm.name, () => { expect(versions).toMatchObject(['0.0.0', '1.4.0', '1.4.1', '2.4.0-alpha.1']); }); - it('publishedVersions gets versions when package time is not available', () => { + it('publishedVersions gets versions when package time is not available', async () => { const json: string = `[ "0.0.0", "1.4.0", "1.4.1", "2.4.0-alpha.1" ]`; - stub.mockImplementationOnce(() => ''); - stub.mockImplementationOnce(() => json); + stub.mockImplementationOnce(() => Promise.resolve('')); + stub.mockImplementationOnce(() => Promise.resolve(json)); - const versions: string[] = Npm.publishedVersions(packageName, __dirname, process.env); + const versions: string[] = await Npm.getPublishedVersionsAsync(packageName, __dirname, process.env); expect(stub).toHaveBeenCalledWith( 'npm', diff --git a/libraries/rush-lib/src/utilities/test/global-teardown.ts b/libraries/rush-lib/src/utilities/test/global-teardown.ts new file mode 100644 index 00000000000..98b5d0b77f9 --- /dev/null +++ b/libraries/rush-lib/src/utilities/test/global-teardown.ts @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { FileSystem } from '@rushstack/node-core-library'; +import { TEST_REPO_FOLDER_PATH } from '../../cli/test/TestUtils'; + +export default async function globalTeardown(): Promise { + await FileSystem.deleteFolderAsync(TEST_REPO_FOLDER_PATH); +}