From 3e3f9cc2061a2931312f9e956ad83785302efbec Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 21 Dec 2021 18:21:56 -0800 Subject: [PATCH 01/19] Refactors for phased commands. --- .../src/api/CommandLineConfiguration.ts | 7 +- .../rush-lib/src/cli/RushCommandLineParser.ts | 167 ++++++++++-------- .../src/cli/actions/WriteBuildCacheAction.ts | 2 - ...criptAction.ts => BaseBulkScriptAction.ts} | 40 ++--- .../NonPhasedBulkScriptAction.ts | 36 ++++ .../src/logic/NonPhasedProjectTaskSelector.ts | 19 +- ...ctorBase.ts => ProjectTaskSelectorBase.ts} | 14 +- .../logic/taskExecution/ProjectTaskRunner.ts | 3 +- apps/rush-lib/src/utilities/Utilities.ts | 4 - 9 files changed, 164 insertions(+), 128 deletions(-) rename apps/rush-lib/src/cli/scriptActions/{BulkScriptAction.ts => BaseBulkScriptAction.ts} (92%) create mode 100644 apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts rename apps/rush-lib/src/logic/{TaskSelectorBase.ts => ProjectTaskSelectorBase.ts} (87%) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index 0f2e8c5fc6a..d67c9f5131b 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -12,7 +12,8 @@ import { ICommandLineJson, IPhaseJson, ParameterJson, - IPhasedCommandJson + IPhasedCommandJson, + IBulkCommandJson } from './CommandLineJson'; const EXPECTED_PHASE_NAME_PREFIX: '_phase:' = '_phase:'; @@ -47,7 +48,7 @@ export class CommandLineConfiguration { */ private _shellCommandTokenContext: IShellCommandTokenContext | undefined; - public static readonly defaultBuildCommandJson: CommandJson = { + public static readonly defaultBuildCommandJson: IBulkCommandJson = { commandKind: RushConstants.bulkCommandKind, name: RushConstants.buildCommandName, summary: "Build all projects that haven't been built, or have changed since they were last built.", @@ -68,7 +69,7 @@ export class CommandLineConfiguration { safeForSimultaneousRushProcesses: false }; - public static readonly defaultRebuildCommandJson: CommandJson = { + public static readonly defaultRebuildCommandJson: IBulkCommandJson = { commandKind: RushConstants.bulkCommandKind, name: RushConstants.rebuildCommandName, summary: 'Clean and rebuild the entire set of projects', diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts index ead36b52fd3..5bd44f04704 100644 --- a/apps/rush-lib/src/cli/RushCommandLineParser.ts +++ b/apps/rush-lib/src/cli/RushCommandLineParser.ts @@ -17,7 +17,7 @@ import { PrintUtilities } from '@rushstack/terminal'; import { RushConfiguration } from '../api/RushConfiguration'; import { RushConstants } from '../logic/RushConstants'; import { CommandLineConfiguration } from '../api/CommandLineConfiguration'; -import { CommandJson } from '../api/CommandLineJson'; +import { CommandJson, IBulkCommandJson, IGlobalCommandJson } from '../api/CommandLineJson'; import { Utilities } from '../utilities/Utilities'; import { AddAction } from './actions/AddAction'; @@ -40,8 +40,9 @@ import { VersionAction } from './actions/VersionAction'; import { UpdateCloudCredentialsAction } from './actions/UpdateCloudCredentialsAction'; import { WriteBuildCacheAction } from './actions/WriteBuildCacheAction'; -import { BulkScriptAction } from './scriptActions/BulkScriptAction'; +import { NonPhasedBulkScriptAction } from './scriptActions/NonPhasedBulkScriptAction'; import { GlobalScriptAction } from './scriptActions/GlobalScriptAction'; +import { IBaseScriptActionOptions } from './scriptActions/BaseScriptAction'; import { Telemetry } from '../logic/Telemetry'; import { RushGlobalFolder } from '../api/RushGlobalFolder'; @@ -254,7 +255,7 @@ export class RushCommandLineParser extends CommandLineParser { private _addDefaultBuildActions(commandLineConfiguration?: CommandLineConfiguration): void { if (!this.tryGetAction(RushConstants.buildCommandName)) { - this._addCommandLineConfigAction( + this._addBulkCommandLineConfigAction( commandLineConfiguration, CommandLineConfiguration.defaultBuildCommandJson ); @@ -263,7 +264,7 @@ export class RushCommandLineParser extends CommandLineParser { if (!this.tryGetAction(RushConstants.rebuildCommandName)) { // By default, the "rebuild" action runs the "build" script. However, if the command-line.json file // overrides "rebuild," the "rebuild" script should be run. - this._addCommandLineConfigAction( + this._addBulkCommandLineConfigAction( commandLineConfiguration, CommandLineConfiguration.defaultRebuildCommandJson, RushConstants.buildCommandName @@ -284,8 +285,7 @@ export class RushCommandLineParser extends CommandLineParser { private _addCommandLineConfigAction( commandLineConfiguration: CommandLineConfiguration | undefined, - command: CommandJson, - commandToRun: string = command.name + command: CommandJson ): void { if (this.tryGetAction(command.name)) { throw new Error( @@ -294,78 +294,14 @@ export class RushCommandLineParser extends CommandLineParser { ); } - if ( - (command.name === RushConstants.buildCommandName || - command.name === RushConstants.rebuildCommandName) && - command.safeForSimultaneousRushProcesses - ) { - // Build and rebuild can't be designated "safeForSimultaneousRushProcesses" - throw new Error( - `${RushConstants.commandLineFilename} defines a command "${command.name}" using ` + - `"safeForSimultaneousRushProcesses=true". This configuration is not supported for "${command.name}".` - ); - } - - const overrideAllowWarnings: boolean = EnvironmentConfiguration.allowWarningsInSuccessfulBuild; - switch (command.commandKind) { case RushConstants.bulkCommandKind: { - const logFilenameIdentifier: string = - ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(commandToRun); - - this.addAction( - new BulkScriptAction({ - actionName: command.name, - logFilenameIdentifier: logFilenameIdentifier, - commandToRun: commandToRun, - - summary: command.summary, - documentation: command.description || command.summary, - safeForSimultaneousRushProcesses: command.safeForSimultaneousRushProcesses, - - parser: this, - commandLineConfiguration: commandLineConfiguration, - - enableParallelism: command.enableParallelism, - ignoreMissingScript: command.ignoreMissingScript || false, - ignoreDependencyOrder: command.ignoreDependencyOrder || false, - incremental: command.incremental || false, - allowWarningsInSuccessfulBuild: overrideAllowWarnings || !!command.allowWarningsInSuccessfulBuild, - - watchForChanges: command.watchForChanges || false, - disableBuildCache: command.disableBuildCache || false - }) - ); + this._addBulkCommandLineConfigAction(commandLineConfiguration, command); break; } case RushConstants.globalCommandKind: { - if ( - command.name === RushConstants.buildCommandName || - command.name === RushConstants.rebuildCommandName - ) { - throw new Error( - `${RushConstants.commandLineFilename} defines a command "${command.name}" using ` + - `the command kind "${RushConstants.globalCommandKind}". This command can only be designated as a command ` + - `kind "${RushConstants.bulkCommandKind}" or "${RushConstants.phasedCommandKind}".` - ); - } - - this.addAction( - new GlobalScriptAction({ - actionName: command.name, - summary: command.summary, - documentation: command.description || command.summary, - safeForSimultaneousRushProcesses: command.safeForSimultaneousRushProcesses, - - parser: this, - commandLineConfiguration: commandLineConfiguration, - - shellCommand: command.shellCommand, - - autoinstallerName: command.autoinstallerName - }) - ); + this._addGlobalScriptAction(commandLineConfiguration, command); break; } @@ -391,6 +327,93 @@ export class RushCommandLineParser extends CommandLineParser { } } + private _getSharedCommandActionOptions( + commandLineConfiguration: CommandLineConfiguration | undefined, + command: CommandJson + ): IBaseScriptActionOptions { + return { + actionName: command.name, + summary: command.summary, + documentation: command.description || command.summary, + safeForSimultaneousRushProcesses: command.safeForSimultaneousRushProcesses, + + parser: this, + commandLineConfiguration: commandLineConfiguration + }; + } + + private _addBulkCommandLineConfigAction( + commandLineConfiguration: CommandLineConfiguration | undefined, + command: IBulkCommandJson, + commandToRun: string = command.name + ): void { + if ( + (command.name === RushConstants.buildCommandName || + command.name === RushConstants.rebuildCommandName) && + command.safeForSimultaneousRushProcesses + ) { + // Build and rebuild can't be designated "safeForSimultaneousRushProcesses" + throw new Error( + `${RushConstants.commandLineFilename} defines a command "${command.name}" using ` + + `"safeForSimultaneousRushProcesses=true". This configuration is not supported for "${command.name}".` + ); + } + const logFilenameIdentifier: string = + ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(commandToRun); + const sharedCommandOptions: IBaseScriptActionOptions = this._getSharedCommandActionOptions( + commandLineConfiguration, + command + ); + + this.addAction( + new NonPhasedBulkScriptAction({ + ...sharedCommandOptions, + + enableParallelism: command.enableParallelism, + ignoreMissingScript: command.ignoreMissingScript || false, + ignoreDependencyOrder: command.ignoreDependencyOrder || false, + incremental: command.incremental || false, + allowWarningsInSuccessfulBuild: + EnvironmentConfiguration.allowWarningsInSuccessfulBuild || !!command.allowWarningsInSuccessfulBuild, + logFilenameIdentifier: logFilenameIdentifier, + commandToRun: commandToRun, + + watchForChanges: command.watchForChanges || false, + disableBuildCache: command.disableBuildCache || false + }) + ); + } + + private _addGlobalScriptAction( + commandLineConfiguration: CommandLineConfiguration | undefined, + command: IGlobalCommandJson + ): void { + if ( + command.name === RushConstants.buildCommandName || + command.name === RushConstants.rebuildCommandName + ) { + throw new Error( + `${RushConstants.commandLineFilename} defines a command "${command.name}" using ` + + `the command kind "${RushConstants.globalCommandKind}". This command can only be designated as a command ` + + `kind "${RushConstants.bulkCommandKind}" or "${RushConstants.phasedCommandKind}".` + ); + } + + const sharedCommandOptions: IBaseScriptActionOptions = this._getSharedCommandActionOptions( + commandLineConfiguration, + command + ); + + this.addAction( + new GlobalScriptAction({ + ...sharedCommandOptions, + + shellCommand: command.shellCommand, + autoinstallerName: command.autoinstallerName + }) + ); + } + private _reportErrorAndSetExitCode(error: Error): void { if (!(error instanceof AlreadyReportedError)) { const prefix: string = 'ERROR: '; diff --git a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts index b220e956fce..6e581452336 100644 --- a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts +++ b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts @@ -12,7 +12,6 @@ import { RushCommandLineParser } from '../RushCommandLineParser'; import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; import { ProjectTaskRunner } from '../../logic/taskExecution/ProjectTaskRunner'; import { ProjectChangeAnalyzer } from '../../logic/ProjectChangeAnalyzer'; -import { Utilities } from '../../utilities/Utilities'; import { NonPhasedProjectTaskSelector } from '../../logic/NonPhasedProjectTaskSelector'; import { RushConstants } from '../../logic/RushConstants'; import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; @@ -93,7 +92,6 @@ export class WriteBuildCacheAction extends BaseRushAction { commandToRun: commandToRun || '', isIncrementalBuildAllowed: false, projectChangeAnalyzer, - packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(command), logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(command) }); diff --git a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BaseBulkScriptAction.ts similarity index 92% rename from apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts rename to apps/rush-lib/src/cli/scriptActions/BaseBulkScriptAction.ts index 463f244d3e6..b8b717157ab 100644 --- a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BaseBulkScriptAction.ts @@ -9,17 +9,12 @@ import { CommandLineFlagParameter, CommandLineStringParameter } from '@rushstack import { Event } from '../../index'; import { SetupChecks } from '../../logic/SetupChecks'; -import { - INonPhasedProjectTaskSelectorOptions, - NonPhasedProjectTaskSelector -} from '../../logic/NonPhasedProjectTaskSelector'; import { Stopwatch, StopwatchState } from '../../utilities/Stopwatch'; import { BaseScriptAction, IBaseScriptActionOptions } from './BaseScriptAction'; import { ITaskExecutionManagerOptions, TaskExecutionManager } from '../../logic/taskExecution/TaskExecutionManager'; -import { Utilities } from '../../utilities/Utilities'; import { RushConstants } from '../../logic/RushConstants'; import { EnvironmentVariableNames } from '../../api/EnvironmentConfiguration'; import { LastLinkFlag, LastLinkFlagFactory } from '../../api/LastLinkFlag'; @@ -28,11 +23,12 @@ import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; import { Selection } from '../../logic/Selection'; import { SelectionParameterSet } from '../SelectionParameterSet'; import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; +import { ITaskSelectorBaseOptions, ProjectTaskSelectorBase } from '../../logic/ProjectTaskSelectorBase'; /** * Constructor parameters for BulkScriptAction. */ -export interface IBulkScriptActionOptions extends IBaseScriptActionOptions { +export interface IBaseBulkScriptActionOptions extends IBaseScriptActionOptions { enableParallelism: boolean; ignoreMissingScript: boolean; ignoreDependencyOrder: boolean; @@ -40,12 +36,10 @@ export interface IBulkScriptActionOptions extends IBaseScriptActionOptions { allowWarningsInSuccessfulBuild: boolean; watchForChanges: boolean; disableBuildCache: boolean; - logFilenameIdentifier: string; - commandToRun: string; } interface IExecuteInternalOptions { - taskSelectorOptions: INonPhasedProjectTaskSelectorOptions; + taskSelectorOptions: ITaskSelectorBaseOptions; taskExecutionManagerOptions: ITaskExecutionManagerOptions; stopwatch: Stopwatch; ignoreHooks?: boolean; @@ -53,25 +47,24 @@ interface IExecuteInternalOptions { } /** - * This class implements bulk commands which are run individually for each project in the repo, - * possibly in parallel. The action executes a script found in the project's package.json file. + * This base class implements bulk commands which are run individually for each project in the repo, + * possibly in parallel. The task selector is abstract and is implemented for phased or non-phased + * commands. * * @remarks * Bulk commands can be defined via common/config/command-line.json. Rush's predefined "build" * and "rebuild" commands are also modeled as bulk commands, because they essentially just * execute scripts from package.json in the same as any custom command. */ -export class BulkScriptAction extends BaseScriptAction { +export abstract class BaseBulkScriptAction extends BaseScriptAction { private readonly _enableParallelism: boolean; private readonly _ignoreMissingScript: boolean; private readonly _isIncrementalBuildAllowed: boolean; - private readonly _commandToRun: string; private readonly _watchForChanges: boolean; private readonly _disableBuildCache: boolean; private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined; private readonly _ignoreDependencyOrder: boolean; private readonly _allowWarningsInSuccessfulBuild: boolean; - private readonly _logFilenameIdentifier: string; private _changedProjectsOnly!: CommandLineFlagParameter; private _selectionParameters!: SelectionParameterSet; @@ -79,18 +72,16 @@ export class BulkScriptAction extends BaseScriptAction { private _parallelismParameter: CommandLineStringParameter | undefined; private _ignoreHooksParameter!: CommandLineFlagParameter; - public constructor(options: IBulkScriptActionOptions) { + public constructor(options: IBaseBulkScriptActionOptions) { super(options); this._enableParallelism = options.enableParallelism; this._ignoreMissingScript = options.ignoreMissingScript; this._isIncrementalBuildAllowed = options.incremental; - this._commandToRun = options.commandToRun; this._ignoreDependencyOrder = options.ignoreDependencyOrder; this._allowWarningsInSuccessfulBuild = options.allowWarningsInSuccessfulBuild; this._watchForChanges = options.watchForChanges; this._disableBuildCache = options.disableBuildCache; this._repoCommandLineConfiguration = options.commandLineConfiguration; - this._logFilenameIdentifier = options.logFilenameIdentifier; } public async runAsync(): Promise { @@ -144,21 +135,18 @@ export class BulkScriptAction extends BaseScriptAction { return; } - const taskSelectorOptions: INonPhasedProjectTaskSelectorOptions = { + const taskSelectorOptions: ITaskSelectorBaseOptions = { rushConfiguration: this.rushConfiguration, buildCacheConfiguration, selection, commandName: this.actionName, - commandToRun: this._commandToRun, customParameterValues, isQuietMode: isQuietMode, isDebugMode: isDebugMode, isIncrementalBuildAllowed: this._isIncrementalBuildAllowed, ignoreMissingScript: this._ignoreMissingScript, ignoreDependencyOrder: this._ignoreDependencyOrder, - allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild, - packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(this._commandToRun), - logFilenameIdentifier: this._logFilenameIdentifier + allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild }; const taskExecutionManagerOptions: ITaskExecutionManagerOptions = { @@ -320,13 +308,15 @@ export class BulkScriptAction extends BaseScriptAction { this.defineScriptParameters(); } + protected abstract _getTaskSelector( + baseTaskSelectorOptions: ITaskSelectorBaseOptions + ): ProjectTaskSelectorBase; + /** * Runs a single invocation of the command */ private async _runOnce(options: IExecuteInternalOptions): Promise { - const taskSelector: NonPhasedProjectTaskSelector = new NonPhasedProjectTaskSelector( - options.taskSelectorOptions - ); + const taskSelector: ProjectTaskSelectorBase = this._getTaskSelector(options.taskSelectorOptions); // Register all tasks with the task collection diff --git a/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts new file mode 100644 index 00000000000..7c0b3f7f900 --- /dev/null +++ b/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts @@ -0,0 +1,36 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { NonPhasedProjectTaskSelector } from '../../logic/NonPhasedProjectTaskSelector'; +import { ITaskSelectorBaseOptions } from '../../logic/ProjectTaskSelectorBase'; +import { BaseBulkScriptAction, IBaseBulkScriptActionOptions } from './BaseBulkScriptAction'; + +export interface INonPhasedBulkScriptAction extends IBaseBulkScriptActionOptions { + logFilenameIdentifier: string; + commandToRun: string; +} + +/** + * This class implements bulk commands which are run individually for each project in the repo, + * possibly in parallel, with a single command per project. + */ +export class NonPhasedBulkScriptAction extends BaseBulkScriptAction { + private readonly _logFilenameIdentifier: string; + private readonly _commandToRun: string; + + public constructor(options: INonPhasedBulkScriptAction) { + super(options); + this._logFilenameIdentifier = options.logFilenameIdentifier; + this._commandToRun = options.commandToRun; + } + + protected _getTaskSelector( + baseTaskSelectorOptions: ITaskSelectorBaseOptions + ): NonPhasedProjectTaskSelector { + return new NonPhasedProjectTaskSelector({ + ...baseTaskSelectorOptions, + logFilenameIdentifier: this._logFilenameIdentifier, + commandToRun: this._commandToRun + }); + } +} diff --git a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts index 743e059109c..cf35798d23f 100644 --- a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts +++ b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts @@ -1,23 +1,17 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { ITaskSelectorOptions, TaskSelectorBase } from './TaskSelectorBase'; +import { ITaskSelectorBaseOptions, ProjectTaskSelectorBase } from './ProjectTaskSelectorBase'; import { RushConfigurationProject } from '../api/RushConfigurationProject'; import { TaskCollection } from './taskExecution/TaskCollection'; import { ProjectTaskRunner } from './taskExecution/ProjectTaskRunner'; -export interface INonPhasedProjectTaskSelectorOptions extends ITaskSelectorOptions { +export interface INonPhasedProjectTaskSelectorOptions extends ITaskSelectorBaseOptions { logFilenameIdentifier: string; + commandToRun: string; } -export class NonPhasedProjectTaskSelector extends TaskSelectorBase { - private readonly _logFilenameIdentifier: string; - - public constructor(options: INonPhasedProjectTaskSelectorOptions) { - super(options); - this._logFilenameIdentifier = options.logFilenameIdentifier; - } - +export class NonPhasedProjectTaskSelector extends ProjectTaskSelectorBase { public registerTasks(): TaskCollection { const projects: ReadonlySet = this._options.selection; const taskCollection: TaskCollection = new TaskCollection(); @@ -73,7 +67,7 @@ export class NonPhasedProjectTaskSelector extends TaskSelectorBase { return; } - const commandToRun: string | undefined = TaskSelectorBase.getScriptToRun( + const commandToRun: string | undefined = ProjectTaskSelectorBase.getScriptToRun( project, this._options.commandToRun, this._options.customParameterValues @@ -94,9 +88,8 @@ export class NonPhasedProjectTaskSelector extends TaskSelectorBase { commandName: this._options.commandName, isIncrementalBuildAllowed: this._options.isIncrementalBuildAllowed, projectChangeAnalyzer: this._projectChangeAnalyzer, - packageDepsFilename: this._options.packageDepsFilename, allowWarningsInSuccessfulBuild: this._options.allowWarningsInSuccessfulBuild, - logFilenameIdentifier: this._logFilenameIdentifier + logFilenameIdentifier: this._options.logFilenameIdentifier }) ); } diff --git a/apps/rush-lib/src/logic/TaskSelectorBase.ts b/apps/rush-lib/src/logic/ProjectTaskSelectorBase.ts similarity index 87% rename from apps/rush-lib/src/logic/TaskSelectorBase.ts rename to apps/rush-lib/src/logic/ProjectTaskSelectorBase.ts index 707533c8dc9..5871f3cbe45 100644 --- a/apps/rush-lib/src/logic/TaskSelectorBase.ts +++ b/apps/rush-lib/src/logic/ProjectTaskSelectorBase.ts @@ -8,19 +8,17 @@ import { convertSlashesForWindows } from './taskExecution/ProjectTaskRunner'; import { ProjectChangeAnalyzer } from './ProjectChangeAnalyzer'; import { TaskCollection } from './taskExecution/TaskCollection'; -export interface ITaskSelectorOptions { +export interface ITaskSelectorBaseOptions { rushConfiguration: RushConfiguration; buildCacheConfiguration: BuildCacheConfiguration | undefined; selection: ReadonlySet; commandName: string; - commandToRun: string; customParameterValues: string[]; isQuietMode: boolean; isDebugMode: boolean; isIncrementalBuildAllowed: boolean; ignoreMissingScript: boolean; ignoreDependencyOrder: boolean; - packageDepsFilename: string; projectChangeAnalyzer?: ProjectChangeAnalyzer; allowWarningsInSuccessfulBuild?: boolean; } @@ -31,11 +29,13 @@ export interface ITaskSelectorOptions { * - creating a ProjectBuilder for each project that needs to be built * - registering the necessary ProjectBuilders with the TaskExecutionManager, which actually orchestrates execution */ -export abstract class TaskSelectorBase { - protected _options: ITaskSelectorOptions; +export abstract class ProjectTaskSelectorBase< + TOptions extends ITaskSelectorBaseOptions = ITaskSelectorBaseOptions +> { + protected _options: TOptions; protected _projectChangeAnalyzer: ProjectChangeAnalyzer; - public constructor(options: ITaskSelectorOptions) { + public constructor(options: TOptions) { this._options = options; const { projectChangeAnalyzer = new ProjectChangeAnalyzer(options.rushConfiguration) } = options; @@ -48,7 +48,7 @@ export abstract class TaskSelectorBase { commandToRun: string, customParameterValues: string[] ): string | undefined { - const script: string | undefined = TaskSelectorBase._getScriptCommand(rushProject, commandToRun); + const script: string | undefined = ProjectTaskSelectorBase._getScriptCommand(rushProject, commandToRun); if (script === undefined) { return undefined; diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts index 32ffe88c847..ac3fd8cc4f2 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts @@ -52,7 +52,6 @@ export interface IProjectTaskRunnerOptions { commandName: string; isIncrementalBuildAllowed: boolean; projectChangeAnalyzer: ProjectChangeAnalyzer; - packageDepsFilename: string; allowWarningsInSuccessfulBuild?: boolean; taskName: string; logFilenameIdentifier: string; @@ -113,7 +112,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { this._isCacheReadAllowed = options.isIncrementalBuildAllowed; this.isSkipAllowed = options.isIncrementalBuildAllowed; this._projectChangeAnalyzer = options.projectChangeAnalyzer; - this._packageDepsFilename = options.packageDepsFilename; + this._packageDepsFilename = `package-deps_${this._commandToRun}.json`; this._allowWarningsInSuccessfulBuild = options.allowWarningsInSuccessfulBuild || false; this._logFilenameIdentifier = options.logFilenameIdentifier; } diff --git a/apps/rush-lib/src/utilities/Utilities.ts b/apps/rush-lib/src/utilities/Utilities.ts index a2e5ebc29de..f91b60e570a 100644 --- a/apps/rush-lib/src/utilities/Utilities.ts +++ b/apps/rush-lib/src/utilities/Utilities.ts @@ -594,10 +594,6 @@ export class Utilities { return new Error('Unable to find rush.json configuration file'); } - public static getPackageDepsFilenameForCommand(command: string): string { - return `package-deps_${command}.json`; - } - public static async usingAsync( getDisposableAsync: () => Promise | IDisposable, doActionAsync: (disposable: TDisposable) => Promise | void From 5d61c402b13e7d08129bd6b29296cb9ef96f3d79 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 22 Dec 2021 18:08:50 -0800 Subject: [PATCH 02/19] Refactor CommandLineConfiguration to translate bulk commands to phased commands. --- .../src/api/CommandLineConfiguration.ts | 563 ++++++++++++------ .../src/api/RushProjectConfiguration.ts | 7 +- .../api/test/CommandLineConfiguration.test.ts | 133 ++++- .../rush-lib/src/cli/RushCommandLineParser.ts | 130 ++-- .../src/cli/scriptActions/BaseScriptAction.ts | 26 +- ...ulkScriptAction.ts => BulkScriptAction.ts} | 30 +- .../NonPhasedBulkScriptAction.ts | 37 +- .../scriptActions/PhasedBulkScriptAction.ts | 35 ++ .../src/logic/NonPhasedProjectTaskSelector.ts | 11 +- .../src/logic/ProjectTaskSelectorBase.ts | 20 +- .../src/logic/taskExecution/BaseTaskRunner.ts | 6 + .../logic/taskExecution/ProjectTaskRunner.ts | 11 +- .../taskExecution/TaskExecutionManager.ts | 20 +- .../taskExecution/test/MockTaskRunner.ts | 8 +- .../test/TaskExecutionManager.test.ts | 18 +- 15 files changed, 693 insertions(+), 362 deletions(-) rename apps/rush-lib/src/cli/scriptActions/{BaseBulkScriptAction.ts => BulkScriptAction.ts} (95%) create mode 100644 apps/rush-lib/src/cli/scriptActions/PhasedBulkScriptAction.ts diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index d67c9f5131b..5f511c90a24 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -11,10 +11,14 @@ import { CommandJson, ICommandLineJson, IPhaseJson, - ParameterJson, IPhasedCommandJson, - IBulkCommandJson + IBulkCommandJson, + IGlobalCommandJson, + IFlagParameterJson, + IChoiceParameterJson, + IStringParameterJson } from './CommandLineJson'; +import { ProjectLogWritable } from '../logic/taskExecution/ProjectLogWritable'; const EXPECTED_PHASE_NAME_PREFIX: '_phase:' = '_phase:'; @@ -22,6 +26,66 @@ export interface IShellCommandTokenContext { packageFolder: string; } +export interface IPhase extends IPhaseJson { + logFilenameIdentifier: string; +} + +export interface ICommandWithParameters { + associatedParameters: Set; +} +export interface IPhasedCommand extends IPhasedCommandJson, ICommandWithParameters { + isSynthetic?: boolean; + watchForChanges?: boolean; + disableBuildCache?: boolean; +} + +export interface IGlobalCommand extends IGlobalCommandJson, ICommandWithParameters {} + +export type Command = IGlobalCommand | IPhasedCommand; + +export type Parameter = IFlagParameterJson | IChoiceParameterJson | IStringParameterJson; + +const DEFAULT_BUILD_COMMAND_JSON: IBulkCommandJson = { + commandKind: RushConstants.bulkCommandKind, + name: RushConstants.buildCommandName, + summary: "Build all projects that haven't been built, or have changed since they were last built.", + description: + 'This command is similar to "rush rebuild", except that "rush build" performs' + + ' an incremental build. In other words, it only builds projects whose source files have changed' + + ' since the last successful build. The analysis requires a Git working tree, and only considers' + + ' source files that are tracked by Git and whose path is under the project folder. (For more details' + + ' about this algorithm, see the documentation for the "package-deps-hash" NPM package.) The incremental' + + ' build state is tracked in a per-project folder called ".rush/temp" which should NOT be added to Git. The' + + ' build command is tracked by the "arguments" field in the "package-deps_build.json" file contained' + + ' therein; a full rebuild is forced whenever the command has changed (e.g. "--production" or not).', + enableParallelism: true, + ignoreMissingScript: false, + ignoreDependencyOrder: false, + incremental: true, + allowWarningsInSuccessfulBuild: false, + safeForSimultaneousRushProcesses: false +}; + +const DEFAULT_REBUILD_COMMAND_JSON: IBulkCommandJson = { + commandKind: RushConstants.bulkCommandKind, + name: RushConstants.rebuildCommandName, + summary: 'Clean and rebuild the entire set of projects.', + description: + 'This command assumes that the package.json file for each project contains' + + ' a "scripts" entry for "npm run build" that performs a full clean build.' + + ' Rush invokes this script to build each project that is registered in rush.json.' + + ' Projects are built in parallel where possible, but always respecting the dependency' + + ' graph for locally linked projects. The number of simultaneous processes will be' + + ' based on the number of machine cores unless overridden by the --parallelism flag.' + + ' (For an incremental build, see "rush build" instead of "rush rebuild".)', + enableParallelism: true, + ignoreMissingScript: false, + ignoreDependencyOrder: false, + incremental: false, + allowWarningsInSuccessfulBuild: false, + safeForSimultaneousRushProcesses: false +}; + /** * Custom Commands and Options for the Rush Command Line */ @@ -30,13 +94,9 @@ export class CommandLineConfiguration { path.join(__dirname, '../schemas/command-line.schema.json') ); - public readonly commands: Map = new Map(); - public readonly phases: Map = new Map(); - public readonly parameters: ParameterJson[] = []; - private readonly _commandNames: Set = new Set([ - RushConstants.buildCommandName, - RushConstants.rebuildCommandName - ]); + public readonly commands: Map = new Map(); + public readonly phases: Map = new Map(); + public readonly parameters: Parameter[] = []; /** * These path will be prepended to the PATH environment variable @@ -48,252 +108,373 @@ export class CommandLineConfiguration { */ private _shellCommandTokenContext: IShellCommandTokenContext | undefined; - public static readonly defaultBuildCommandJson: IBulkCommandJson = { - commandKind: RushConstants.bulkCommandKind, - name: RushConstants.buildCommandName, - summary: "Build all projects that haven't been built, or have changed since they were last built.", - description: - 'This command is similar to "rush rebuild", except that "rush build" performs' + - ' an incremental build. In other words, it only builds projects whose source files have changed' + - ' since the last successful build. The analysis requires a Git working tree, and only considers' + - ' source files that are tracked by Git and whose path is under the project folder. (For more details' + - ' about this algorithm, see the documentation for the "package-deps-hash" NPM package.) The incremental' + - ' build state is tracked in a per-project folder called ".rush/temp" which should NOT be added to Git. The' + - ' build command is tracked by the "arguments" field in the "package-deps_build.json" file contained' + - ' therein; a full rebuild is forced whenever the command has changed (e.g. "--production" or not).', - enableParallelism: true, - ignoreMissingScript: false, - ignoreDependencyOrder: false, - incremental: true, - allowWarningsInSuccessfulBuild: false, - safeForSimultaneousRushProcesses: false - }; - - public static readonly defaultRebuildCommandJson: IBulkCommandJson = { - commandKind: RushConstants.bulkCommandKind, - name: RushConstants.rebuildCommandName, - summary: 'Clean and rebuild the entire set of projects', - description: - 'This command assumes that the package.json file for each project contains' + - ' a "scripts" entry for "npm run build" that performs a full clean build.' + - ' Rush invokes this script to build each project that is registered in rush.json.' + - ' Projects are built in parallel where possible, but always respecting the dependency' + - ' graph for locally linked projects. The number of simultaneous processes will be' + - ' based on the number of machine cores unless overridden by the --parallelism flag.' + - ' (For an incremental build, see "rush build" instead of "rush rebuild".)', - enableParallelism: true, - ignoreMissingScript: false, - ignoreDependencyOrder: false, - incremental: false, - allowWarningsInSuccessfulBuild: false, - safeForSimultaneousRushProcesses: false - }; - /** * Use CommandLineConfiguration.loadFromFile() * * @internal */ public constructor(commandLineJson: ICommandLineJson | undefined) { - if (commandLineJson) { - if (commandLineJson.phases) { - for (const phase of commandLineJson.phases) { - if (this.phases.has(phase.name)) { - throw new Error( - `In ${RushConstants.commandLineFilename}, the phase "${phase.name}" is specified ` + - 'more than once.' - ); - } + const commandsForPhase: Map> = new Map(); + const phaseCliques: Map> = new Map(); + if (commandLineJson?.phases) { + for (const phase of commandLineJson.phases) { + if (this.phases.has(phase.name)) { + throw new Error( + `In ${RushConstants.commandLineFilename}, the phase "${phase.name}" is specified ` + + 'more than once.' + ); + } - if (phase.name.substring(0, EXPECTED_PHASE_NAME_PREFIX.length) !== EXPECTED_PHASE_NAME_PREFIX) { - throw new Error( - `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + - `does not begin with the required prefix "${EXPECTED_PHASE_NAME_PREFIX}".` - ); - } + if (phase.name.substring(0, EXPECTED_PHASE_NAME_PREFIX.length) !== EXPECTED_PHASE_NAME_PREFIX) { + throw new Error( + `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + + `does not begin with the required prefix "${EXPECTED_PHASE_NAME_PREFIX}".` + ); + } + + if (phase.name.length <= EXPECTED_PHASE_NAME_PREFIX.length) { + throw new Error( + `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + + `must have characters after "${EXPECTED_PHASE_NAME_PREFIX}"` + ); + } - if (phase.name.length <= EXPECTED_PHASE_NAME_PREFIX.length) { + this.phases.set(phase.name, { + ...phase, + logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(phase.name) + }); + commandsForPhase.set(phase.name, new Set()); + } + } + + for (const phase of this.phases.values()) { + if (phase.dependencies?.self) { + for (const dependencyName of phase.dependencies.self) { + const dependency: IPhase | undefined = this.phases.get(dependencyName); + if (!dependency) { throw new Error( - `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + - `must have characters after "${EXPECTED_PHASE_NAME_PREFIX}"` + `In ${RushConstants.commandLineFilename}, in the phase "${phase.name as string}", the self ` + + `dependency phase "${dependencyName as string}" does not exist.` ); } - - this.phases.set(phase.name, phase); } } - for (const phase of this.phases.values()) { - if (phase.dependencies?.self) { - for (const dependencyName of phase.dependencies.self) { - const dependency: IPhaseJson | undefined = this.phases.get(dependencyName); - if (!dependency) { - throw new Error( - `In ${RushConstants.commandLineFilename}, in the phase "${phase.name}", the self ` + - `dependency phase "${dependencyName}" does not exist.` - ); - } + if (phase.dependencies?.upstream) { + for (const dependency of phase.dependencies.upstream) { + if (!this.phases.has(dependency)) { + throw new Error( + `In ${RushConstants.commandLineFilename}, in the phase "${phase.name as string}", ` + + `the upstream dependency phase "${dependency as string}" does not exist.` + ); } } + } - if (phase.dependencies?.upstream) { - for (const dependency of phase.dependencies.upstream) { - if (!this.phases.has(dependency)) { - throw new Error( - `In ${RushConstants.commandLineFilename}, in the phase "${phase.name}", the upstream ` + - `dependency phase "${dependency}" does not exist.` - ); - } - } - } + this._checkForPhaseSelfCycles(phase); + const phaseClique: Set = new Set(); + this._populatePhaseClique(phase.name, phaseClique); + phaseCliques.set(phase.name, phaseClique); + } - this._checkForPhaseSelfCycles(phase); + function populateCommandsForPhase(phaseName: string, command: IPhasedCommand): void { + const phaseClique: Set = phaseCliques.get(phaseName)!; + for (const phaseCliqueIdentifier of phaseClique) { + commandsForPhase.get(phaseCliqueIdentifier)!.add(command); } + } - if (commandLineJson.commands) { - for (const command of commandLineJson.commands) { - if (this.commands.has(command.name)) { - throw new Error( - `In ${RushConstants.commandLineFilename}, the command "${command.name}" is specified ` + - 'more than once.' - ); - } + // A map of bulk command names to their corresponding synthetic phase identifiers + const syntheticPhasesForTranslatedBulkCommands: Map = new Map(); + const translateBulkCommandToPhasedCommand: (command: IBulkCommandJson) => IPhasedCommand = ( + command: IBulkCommandJson + ) => { + const phaseName: string = command.name; + const phaseForBulkCommand: IPhase = { + name: phaseName, + dependencies: { + upstream: command.ignoreDependencyOrder ? undefined : [phaseName] + }, + ignoreMissingScript: command.ignoreMissingScript, + allowWarningsOnSuccess: command.allowWarningsInSuccessfulBuild, + logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(command.name) + }; + this.phases.set(phaseName, phaseForBulkCommand); + syntheticPhasesForTranslatedBulkCommands.set(command.name, phaseName); + const phaseClique: Set = new Set(); + this._populatePhaseClique(phaseName, phaseClique); + phaseCliques.set(phaseName, phaseClique); + + const translatedCommand: IPhasedCommand = { + ...command, + commandKind: 'phased', + disableBuildCache: true, + isSynthetic: true, + associatedParameters: new Set(), + phases: [phaseName] + }; + commandsForPhase.set(phaseName, new Set()); + populateCommandsForPhase(phaseName, translatedCommand); + return translatedCommand; + }; + + let buildCommandPhases: string[] | undefined; + if (commandLineJson?.commands) { + for (const command of commandLineJson.commands) { + if (this.commands.has(command.name)) { + throw new Error( + `In ${RushConstants.commandLineFilename}, the command "${command.name}" is specified ` + + 'more than once.' + ); + } + + let normalizedCommand: Command; + switch (command.commandKind) { + case RushConstants.phasedCommandKind: { + normalizedCommand = { + ...command, + associatedParameters: new Set() + }; - if (command.commandKind === 'phased') { - const phasedCommand: IPhasedCommandJson = command as IPhasedCommandJson; - for (const phase of phasedCommand.phases) { - if (!this.phases.has(phase)) { + for (const phaseName of normalizedCommand.phases) { + if (!this.phases.has(phaseName)) { throw new Error( `In ${RushConstants.commandLineFilename}, in the "phases" property of the ` + - `"${command.name}" command, the phase "${phase}" does not exist.` + `"${normalizedCommand.name}" command, the phase "${phaseName as string}" does not exist.` ); } + + populateCommandsForPhase(phaseName, normalizedCommand); } - if (phasedCommand.skipPhasesForCommand) { - for (const phase of phasedCommand.skipPhasesForCommand) { - if (!this.phases.has(phase)) { + if (normalizedCommand.skipPhasesForCommand) { + for (const phaseName of normalizedCommand.skipPhasesForCommand) { + if (!this.phases.has(phaseName)) { throw new Error( `In ${RushConstants.commandLineFilename}, in the "skipPhasesForCommand" property of the ` + - `"${command.name}" command, the phase "${phase}" does not exist.` + `"${normalizedCommand.name}" command, the phase ` + + `"${phaseName as string}" does not exist.` ); } + + populateCommandsForPhase(phaseName, normalizedCommand); } } + + break; + } + + case RushConstants.globalCommandKind: { + normalizedCommand = { + ...command, + associatedParameters: new Set() + }; + break; + } + + case RushConstants.bulkCommandKind: { + // Translate the bulk command into a phased command + normalizedCommand = translateBulkCommandToPhasedCommand(command); + break; + } + } + + if ( + normalizedCommand.name === RushConstants.buildCommandName || + normalizedCommand.name === RushConstants.rebuildCommandName + ) { + if (normalizedCommand.commandKind === RushConstants.globalCommandKind) { + throw new Error( + `${RushConstants.commandLineFilename} defines a command "${normalizedCommand.name}" using ` + + `the command kind "${RushConstants.globalCommandKind}". This command can only be designated as a command ` + + `kind "${RushConstants.bulkCommandKind}" or "${RushConstants.phasedCommandKind}".` + ); + } else if (command.safeForSimultaneousRushProcesses) { + throw new Error( + `${RushConstants.commandLineFilename} defines a command "${normalizedCommand.name}" using ` + + `"safeForSimultaneousRushProcesses=true". This configuration is not supported for "${normalizedCommand.name}".` + ); + } else if (normalizedCommand.name === RushConstants.buildCommandName) { + // Record the build command phases in case we need to construct a synthetic "rebuild" command + buildCommandPhases = normalizedCommand.phases; } + } + + this.commands.set(normalizedCommand.name, normalizedCommand); + } + } - this.commands.set(command.name, command); - this._commandNames.add(command.name); + let buildCommand: Command | undefined = this.commands.get(RushConstants.buildCommandName); + if (!buildCommand) { + // If the build command was not specified in the config file, add the default build command + buildCommand = translateBulkCommandToPhasedCommand(DEFAULT_BUILD_COMMAND_JSON); + buildCommand.disableBuildCache = DEFAULT_BUILD_COMMAND_JSON.disableBuildCache; + buildCommandPhases = buildCommand.phases; + this.commands.set(buildCommand.name, buildCommand); + } + + if (!this.commands.has(RushConstants.rebuildCommandName)) { + // If a rebuild command was not specified in the config file, add the default rebuild command + if (!buildCommandPhases) { + throw new Error(`Phases for the "${RushConstants.buildCommandName}" were not found.`); + } + + const rebuildCommand: IPhasedCommand = { + ...DEFAULT_REBUILD_COMMAND_JSON, + commandKind: RushConstants.phasedCommandKind, + isSynthetic: true, + phases: buildCommandPhases, + disableBuildCache: DEFAULT_REBUILD_COMMAND_JSON.disableBuildCache, + associatedParameters: buildCommand.associatedParameters // rebuild should share build's parameters in this case + }; + this.commands.set(rebuildCommand.name, rebuildCommand); + } + + if (commandLineJson?.parameters) { + function populateCommandAssociatedParametersForPhase(phaseName: string, parameter: Parameter): void { + const commands: Set = commandsForPhase.get(phaseName)!; + for (const command of commands) { + command.associatedParameters.add(parameter); } } - if (commandLineJson.parameters) { - for (const parameter of commandLineJson.parameters) { - this.parameters.push(parameter); - - let parameterHasAssociations: boolean = false; - - // Do some basic validation - switch (parameter.parameterKind) { - case 'flag': { - const addPhasesToCommandSet: Set = new Set(); - - if (parameter.addPhasesToCommand) { - for (const phase of parameter.addPhasesToCommand) { - addPhasesToCommandSet.add(phase); - if (!this.phases.has(phase)) { - throw new Error( - `${RushConstants.commandLineFilename} defines a parameter "${parameter.longName}" ` + - `that lists phase "${phase}" in its "addPhasesToCommand" property that does not exist.` - ); - } else { - parameterHasAssociations = true; - } - } - } + for (const parameter of commandLineJson.parameters) { + const normalizedParameter: Parameter = { + ...parameter, + associatedPhases: parameter.associatedPhases ? [...parameter.associatedPhases] : [], + associatedCommands: parameter.associatedCommands ? [...parameter.associatedCommands] : [] + }; - if (parameter.skipPhasesForCommand) { - for (const phase of parameter.skipPhasesForCommand) { - if (!this.phases.has(phase)) { - throw new Error( - `${RushConstants.commandLineFilename} defines a parameter "${parameter.longName}" ` + - `that lists phase "${phase}" in its skipPhasesForCommand" property that does not exist.` - ); - } else if (addPhasesToCommandSet.has(phase)) { - throw new Error( - `${RushConstants.commandLineFilename} defines a parameter "${parameter.longName}" ` + - `that lists phase "${phase}" in both its "addPhasesToCommand" and "skipPhasesForCommand" properties.` - ); - } else { - parameterHasAssociations = true; - } - } - } + this.parameters.push(normalizedParameter); - break; - } + let parameterHasAssociations: boolean = false; - case 'choice': { - const alternativeNames: string[] = parameter.alternatives.map((x) => x.name); + // Do some basic validation + switch (normalizedParameter.parameterKind) { + case 'flag': { + const addPhasesToCommandSet: Set = new Set(); - if (parameter.defaultValue && alternativeNames.indexOf(parameter.defaultValue) < 0) { - throw new Error( - `In ${RushConstants.commandLineFilename}, the parameter "${parameter.longName}",` + - ` specifies a default value "${parameter.defaultValue}"` + - ` which is not one of the defined alternatives: "${alternativeNames.toString()}"` - ); + if (normalizedParameter.addPhasesToCommand) { + for (const phase of normalizedParameter.addPhasesToCommand) { + addPhasesToCommandSet.add(phase); + if (!this.phases.has(phase)) { + throw new Error( + `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + + `that lists phase "${phase as string}" in its "addPhasesToCommand" ` + + 'property that does not exist.' + ); + } else { + populateCommandAssociatedParametersForPhase(phase, normalizedParameter); + parameterHasAssociations = true; + } } + } - break; + if (normalizedParameter.skipPhasesForCommand) { + for (const phase of normalizedParameter.skipPhasesForCommand) { + if (!this.phases.has(phase)) { + throw new Error( + `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + + `that lists phase "${phase as string}" in its skipPhasesForCommand" ` + + 'property that does not exist.' + ); + } else if (addPhasesToCommandSet.has(phase)) { + throw new Error( + `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + + `that lists phase "${phase as string}" in both its "addPhasesToCommand" ` + + 'and "skipPhasesForCommand" properties.' + ); + } else { + populateCommandAssociatedParametersForPhase(phase, normalizedParameter); + parameterHasAssociations = true; + } + } } + + break; } - for (const associatedCommand of parameter.associatedCommands || []) { - if (!this._commandNames.has(associatedCommand)) { + case 'choice': { + const alternativeNames: string[] = normalizedParameter.alternatives.map((x) => x.name); + + if ( + normalizedParameter.defaultValue && + alternativeNames.indexOf(normalizedParameter.defaultValue) < 0 + ) { throw new Error( - `${RushConstants.commandLineFilename} defines a parameter "${parameter.longName}" ` + - `that is associated with a command "${associatedCommand}" that does not exist or does ` + - 'not support custom parameters.' + `In ${RushConstants.commandLineFilename}, the parameter "${normalizedParameter.longName}",` + + ` specifies a default value "${normalizedParameter.defaultValue}"` + + ` which is not one of the defined alternatives: "${alternativeNames.toString()}"` ); - } else { - parameterHasAssociations = true; } + + break; } + } - for (const associatedPhase of parameter.associatedPhases || []) { - if (!this.phases.has(associatedPhase)) { + if (normalizedParameter.associatedCommands) { + for (let i: number = 0; i < normalizedParameter.associatedCommands.length; i++) { + const associatedCommandName: string = normalizedParameter.associatedCommands[i]; + const syntheticPhaseName: string | undefined = + syntheticPhasesForTranslatedBulkCommands.get(associatedCommandName); + if (syntheticPhaseName) { + // If this parameter was associated with a bulk command, change the association to + // the command's synthetic phase + normalizedParameter.associatedPhases!.push(syntheticPhaseName); + normalizedParameter.associatedCommands.splice(i, 1); + i--; + populateCommandAssociatedParametersForPhase(syntheticPhaseName, normalizedParameter); + parameterHasAssociations = true; + } else if (!this.commands.has(associatedCommandName)) { throw new Error( - `${RushConstants.commandLineFilename} defines a parameter "${parameter.longName}" ` + - `that is associated with a phase "${associatedPhase}" that does not exist.` + `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + + `that is associated with a command "${associatedCommandName}" that does not exist or does ` + + 'not support custom parameters.' ); } else { + const associatedCommand: Command = this.commands.get(associatedCommandName)!; + associatedCommand.associatedParameters.add(normalizedParameter); parameterHasAssociations = true; } } + } - if (!parameterHasAssociations) { + for (const associatedPhase of normalizedParameter.associatedPhases || []) { + if (!this.phases.has(associatedPhase)) { throw new Error( - `${RushConstants.commandLineFilename} defines a parameter "${parameter.longName}"` + - ` that lists no associated commands or phases.` + `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + + `that is associated with a phase "${associatedPhase as string}" that does not exist.` ); + } else { + populateCommandAssociatedParametersForPhase(associatedPhase, normalizedParameter); + parameterHasAssociations = true; } } + + if (!parameterHasAssociations) { + throw new Error( + `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}"` + + ` that lists no associated commands or phases.` + ); + } } } } - private _checkForPhaseSelfCycles(phase: IPhaseJson, checkedPhases: Set = new Set()): void { + private _checkForPhaseSelfCycles(phase: IPhase, checkedPhases: Set = new Set()): void { const dependencies: string[] | undefined = phase.dependencies?.self; if (dependencies) { for (const dependencyName of dependencies) { if (checkedPhases.has(dependencyName)) { + const dependencyNameForError: string = + typeof dependencyName === 'string' ? dependencyName : ''; throw new Error( `In ${RushConstants.commandLineFilename}, there exists a cycle within the ` + - `set of ${dependencyName} dependencies: ${Array.from(checkedPhases).join(', ')}` + `set of ${dependencyNameForError} dependencies: ${Array.from(checkedPhases).join(', ')}` ); } else { checkedPhases.add(dependencyName); - const dependency: IPhaseJson | undefined = this.phases.get(dependencyName); + const dependency: IPhase | undefined = this.phases.get(dependencyName); if (!dependency) { return; // Ignore, we check for this separately } else { @@ -312,6 +493,26 @@ export class CommandLineConfiguration { } } + private _populatePhaseClique(phaseName: string, clique: Set): void { + if (!clique.has(phaseName)) { + clique.add(phaseName); + const phase: IPhase = this.phases.get(phaseName)!; + if (phase.dependencies) { + if (phase.dependencies.self) { + for (const dependencyName of phase.dependencies.self) { + this._populatePhaseClique(dependencyName, clique); + } + } + + if (phase.dependencies.upstream) { + for (const dependencyName of phase.dependencies.upstream) { + this._populatePhaseClique(dependencyName, clique); + } + } + } + } + } + /** * Loads the configuration from the specified file and applies any omitted default build * settings. If the file does not exist, then an empty default instance is returned. @@ -334,12 +535,12 @@ export class CommandLineConfiguration { case RushConstants.bulkCommandKind: { switch (command.name) { case RushConstants.buildCommandName: { - commandDefaultDefinition = CommandLineConfiguration.defaultBuildCommandJson; + commandDefaultDefinition = DEFAULT_BUILD_COMMAND_JSON; break; } case RushConstants.rebuildCommandName: { - commandDefaultDefinition = CommandLineConfiguration.defaultRebuildCommandJson; + commandDefaultDefinition = DEFAULT_REBUILD_COMMAND_JSON; break; } } diff --git a/apps/rush-lib/src/api/RushProjectConfiguration.ts b/apps/rush-lib/src/api/RushProjectConfiguration.ts index 762a471f5a2..305bdcde23e 100644 --- a/apps/rush-lib/src/api/RushProjectConfiguration.ts +++ b/apps/rush-lib/src/api/RushProjectConfiguration.ts @@ -293,13 +293,10 @@ export class RushProjectConfiguration { const duplicateCommandNames: Set = new Set(); const invalidCommandNames: string[] = []; if (rushProjectJson.buildCacheOptions?.optionsForCommands) { - const commandNames: Set = new Set([ - RushConstants.buildCommandName, - RushConstants.rebuildCommandName - ]); + const commandNames: Set = new Set(); if (repoCommandLineConfiguration) { for (const [commandName, command] of repoCommandLineConfiguration.commands) { - if (command.commandKind === RushConstants.bulkCommandKind) { + if (command.commandKind === RushConstants.phasedCommandKind) { commandNames.add(commandName); } } diff --git a/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts b/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts index 2f60c11dcd5..4cc7626cf07 100644 --- a/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts +++ b/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts @@ -1,9 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { CommandLineConfiguration } from '../CommandLineConfiguration'; +import { RushConstants } from '../../logic/RushConstants'; +import { Command, CommandLineConfiguration, Parameter } from '../CommandLineConfiguration'; -describe('CommandLineConfiguration', () => { +describe(CommandLineConfiguration.name, () => { it('Forbids a misnamed phase', () => { expect( () => @@ -104,4 +105,132 @@ describe('CommandLineConfiguration', () => { }) ).toThrowErrorMatchingSnapshot(); }); + + describe('associatedParameters', () => { + it('correctly populates the associatedParameters object for a parameter associated with the "build" command', () => { + const commandLineConfiguration: CommandLineConfiguration = new CommandLineConfiguration({ + parameters: [ + { + parameterKind: 'flag', + longName: '--flag', + associatedCommands: ['build'], + description: 'flag' + } + ] + }); + + function validateCommandByName(commandName: string): void { + const command: Command | undefined = commandLineConfiguration.commands.get(commandName); + expect(command).toBeDefined(); + const associatedParametersArray: Parameter[] = Array.from(command!.associatedParameters); + expect(associatedParametersArray).toHaveLength(1); + expect(associatedParametersArray[0].longName).toEqual('--flag'); + } + + validateCommandByName(RushConstants.buildCommandName); + validateCommandByName(RushConstants.rebuildCommandName); + }); + + it('correctly populates the associatedParameters object for a parameter associated with a custom bulk command', () => { + const commandLineConfiguration: CommandLineConfiguration = new CommandLineConfiguration({ + commands: [ + { + commandKind: 'bulk', + name: 'custom-bulk', + summary: 'custom-bulk', + enableParallelism: true, + safeForSimultaneousRushProcesses: false + } + ], + parameters: [ + { + parameterKind: 'flag', + longName: '--flag', + associatedCommands: ['custom-bulk'], + description: 'flag' + } + ] + }); + + const command: Command | undefined = commandLineConfiguration.commands.get('custom-bulk'); + expect(command).toBeDefined(); + const associatedParametersArray: Parameter[] = Array.from(command!.associatedParameters); + expect(associatedParametersArray).toHaveLength(1); + expect(associatedParametersArray[0].longName).toEqual('--flag'); + }); + + it("correctly populates the associatedParameters object for a parameter associated with a custom phased command's phase", () => { + const commandLineConfiguration: CommandLineConfiguration = new CommandLineConfiguration({ + commands: [ + { + commandKind: 'phased', + name: 'custom-phased', + summary: 'custom-phased', + enableParallelism: true, + safeForSimultaneousRushProcesses: false, + phases: ['_phase:A'] + } + ], + phases: [ + { + name: '_phase:A' + } + ], + parameters: [ + { + parameterKind: 'flag', + longName: '--flag', + associatedPhases: ['_phase:A'], + description: 'flag' + } + ] + }); + + const command: Command | undefined = commandLineConfiguration.commands.get('custom-phased'); + expect(command).toBeDefined(); + const associatedParametersArray: Parameter[] = Array.from(command!.associatedParameters); + expect(associatedParametersArray).toHaveLength(1); + expect(associatedParametersArray[0].longName).toEqual('--flag'); + }); + + it("correctly populates the associatedParameters object for a parameter associated with a custom phased command's transitive phase", () => { + const commandLineConfiguration: CommandLineConfiguration = new CommandLineConfiguration({ + commands: [ + { + commandKind: 'phased', + name: 'custom-phased', + summary: 'custom-phased', + enableParallelism: true, + safeForSimultaneousRushProcesses: false, + phases: ['_phase:A'] + } + ], + phases: [ + { + name: '_phase:A', + dependencies: { + upstream: ['_phase:B'] + } + }, + { + name: '_phase:B' + } + ], + parameters: [ + { + parameterKind: 'flag', + longName: '--flag', + associatedPhases: ['_phase:B'], + description: 'flag' + } + ] + }); + + const command: Command | undefined = commandLineConfiguration.commands.get('custom-phased'); + expect(command).toBeDefined(); + const associatedParametersArray: Parameter[] = Array.from(command!.associatedParameters); + expect(associatedParametersArray).toHaveLength(1); + expect(associatedParametersArray[0].longName).toEqual('--flag'); + }); + }); }); diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts index 5bd44f04704..143a4512c61 100644 --- a/apps/rush-lib/src/cli/RushCommandLineParser.ts +++ b/apps/rush-lib/src/cli/RushCommandLineParser.ts @@ -16,8 +16,12 @@ import { PrintUtilities } from '@rushstack/terminal'; import { RushConfiguration } from '../api/RushConfiguration'; import { RushConstants } from '../logic/RushConstants'; -import { CommandLineConfiguration } from '../api/CommandLineConfiguration'; -import { CommandJson, IBulkCommandJson, IGlobalCommandJson } from '../api/CommandLineJson'; +import { + Command, + CommandLineConfiguration, + IGlobalCommand, + IPhasedCommand +} from '../api/CommandLineConfiguration'; import { Utilities } from '../utilities/Utilities'; import { AddAction } from './actions/AddAction'; @@ -40,7 +44,6 @@ import { VersionAction } from './actions/VersionAction'; import { UpdateCloudCredentialsAction } from './actions/UpdateCloudCredentialsAction'; import { WriteBuildCacheAction } from './actions/WriteBuildCacheAction'; -import { NonPhasedBulkScriptAction } from './scriptActions/NonPhasedBulkScriptAction'; import { GlobalScriptAction } from './scriptActions/GlobalScriptAction'; import { IBaseScriptActionOptions } from './scriptActions/BaseScriptAction'; @@ -48,10 +51,9 @@ import { Telemetry } from '../logic/Telemetry'; import { RushGlobalFolder } from '../api/RushGlobalFolder'; import { NodeJsCompatibility } from '../logic/NodeJsCompatibility'; import { SetupAction } from './actions/SetupAction'; -import { EnvironmentConfiguration } from '../api/EnvironmentConfiguration'; import { ICustomCommandLineConfigurationInfo, PluginManager } from '../pluginFramework/PluginManager'; import { RushSession } from '../pluginFramework/RushSession'; -import { ProjectLogWritable } from '../logic/taskExecution/ProjectLogWritable'; +import { PhasedScriptAction } from './scriptActions/PhasedScriptAction'; /** * Options for `RushCommandLineParser`. @@ -248,28 +250,7 @@ export class RushCommandLineParser extends CommandLineParser { commandLineConfiguration = CommandLineConfiguration.loadFromFileOrDefault(commandLineConfigFilePath); } - // Build actions from the command line configuration supersede default build actions. this._addCommandLineConfigActions(commandLineConfiguration); - this._addDefaultBuildActions(commandLineConfiguration); - } - - private _addDefaultBuildActions(commandLineConfiguration?: CommandLineConfiguration): void { - if (!this.tryGetAction(RushConstants.buildCommandName)) { - this._addBulkCommandLineConfigAction( - commandLineConfiguration, - CommandLineConfiguration.defaultBuildCommandJson - ); - } - - if (!this.tryGetAction(RushConstants.rebuildCommandName)) { - // By default, the "rebuild" action runs the "build" script. However, if the command-line.json file - // overrides "rebuild," the "rebuild" script should be run. - this._addBulkCommandLineConfigAction( - commandLineConfiguration, - CommandLineConfiguration.defaultRebuildCommandJson, - RushConstants.buildCommandName - ); - } } private _addCommandLineConfigActions(commandLineConfiguration?: CommandLineConfiguration): void { @@ -284,8 +265,8 @@ export class RushCommandLineParser extends CommandLineParser { } private _addCommandLineConfigAction( - commandLineConfiguration: CommandLineConfiguration | undefined, - command: CommandJson + commandLineConfiguration: CommandLineConfiguration, + command: Command ): void { if (this.tryGetAction(command.name)) { throw new Error( @@ -295,18 +276,16 @@ export class RushCommandLineParser extends CommandLineParser { } switch (command.commandKind) { - case RushConstants.bulkCommandKind: { - this._addBulkCommandLineConfigAction(commandLineConfiguration, command); - break; - } - case RushConstants.globalCommandKind: { this._addGlobalScriptAction(commandLineConfiguration, command); break; } case RushConstants.phasedCommandKind: { - if (!this.rushConfiguration.experimentsConfiguration.configuration._multiPhaseCommands) { + if ( + !command.isSynthetic && // synthetic commands come from bulk commands + !this.rushConfiguration.experimentsConfiguration.configuration._multiPhaseCommands + ) { throw new Error( `${RushConstants.commandLineFilename} defines a command "${command.name}" ` + `that uses the "${RushConstants.phasedCommandKind}" command kind. To use this command kind, ` + @@ -315,101 +294,82 @@ export class RushCommandLineParser extends CommandLineParser { ); } - // TODO + this._addPhasedCommandLineConfigAction(commandLineConfiguration, command); break; } default: throw new Error( - `${RushConstants.commandLineFilename} defines a command "${(command as CommandJson).name}"` + - ` using an unsupported command kind "${(command as CommandJson).commandKind}"` + `${RushConstants.commandLineFilename} defines a command "${(command as Command).name}"` + + ` using an unsupported command kind "${(command as Command).commandKind}"` ); } } - private _getSharedCommandActionOptions( + private _getSharedCommandActionOptions( commandLineConfiguration: CommandLineConfiguration | undefined, - command: CommandJson - ): IBaseScriptActionOptions { + command: TCommand + ): IBaseScriptActionOptions { return { actionName: command.name, summary: command.summary, documentation: command.description || command.summary, safeForSimultaneousRushProcesses: command.safeForSimultaneousRushProcesses, + command, parser: this, commandLineConfiguration: commandLineConfiguration }; } - private _addBulkCommandLineConfigAction( + private _addGlobalScriptAction( commandLineConfiguration: CommandLineConfiguration | undefined, - command: IBulkCommandJson, - commandToRun: string = command.name + command: IGlobalCommand ): void { if ( - (command.name === RushConstants.buildCommandName || - command.name === RushConstants.rebuildCommandName) && - command.safeForSimultaneousRushProcesses + command.name === RushConstants.buildCommandName || + command.name === RushConstants.rebuildCommandName ) { - // Build and rebuild can't be designated "safeForSimultaneousRushProcesses" throw new Error( `${RushConstants.commandLineFilename} defines a command "${command.name}" using ` + - `"safeForSimultaneousRushProcesses=true". This configuration is not supported for "${command.name}".` + `the command kind "${RushConstants.globalCommandKind}". This command can only be designated as a command ` + + `kind "${RushConstants.bulkCommandKind}" or "${RushConstants.phasedCommandKind}".` ); } - const logFilenameIdentifier: string = - ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(commandToRun); - const sharedCommandOptions: IBaseScriptActionOptions = this._getSharedCommandActionOptions( - commandLineConfiguration, - command - ); + + const sharedCommandOptions: IBaseScriptActionOptions = + this._getSharedCommandActionOptions(commandLineConfiguration, command); this.addAction( - new NonPhasedBulkScriptAction({ + new GlobalScriptAction({ ...sharedCommandOptions, - enableParallelism: command.enableParallelism, - ignoreMissingScript: command.ignoreMissingScript || false, - ignoreDependencyOrder: command.ignoreDependencyOrder || false, - incremental: command.incremental || false, - allowWarningsInSuccessfulBuild: - EnvironmentConfiguration.allowWarningsInSuccessfulBuild || !!command.allowWarningsInSuccessfulBuild, - logFilenameIdentifier: logFilenameIdentifier, - commandToRun: commandToRun, - - watchForChanges: command.watchForChanges || false, - disableBuildCache: command.disableBuildCache || false + shellCommand: command.shellCommand, + autoinstallerName: command.autoinstallerName }) ); } - private _addGlobalScriptAction( - commandLineConfiguration: CommandLineConfiguration | undefined, - command: IGlobalCommandJson + private _addPhasedCommandLineConfigAction( + commandLineConfiguration: CommandLineConfiguration, + command: IPhasedCommand ): void { - if ( - command.name === RushConstants.buildCommandName || - command.name === RushConstants.rebuildCommandName - ) { - throw new Error( - `${RushConstants.commandLineFilename} defines a command "${command.name}" using ` + - `the command kind "${RushConstants.globalCommandKind}". This command can only be designated as a command ` + - `kind "${RushConstants.bulkCommandKind}" or "${RushConstants.phasedCommandKind}".` - ); - } - - const sharedCommandOptions: IBaseScriptActionOptions = this._getSharedCommandActionOptions( + const baseCommandOptions: IBaseScriptActionOptions = this._getSharedCommandActionOptions( commandLineConfiguration, command ); this.addAction( - new GlobalScriptAction({ - ...sharedCommandOptions, + new PhasedScriptAction({ + ...baseCommandOptions, - shellCommand: command.shellCommand, - autoinstallerName: command.autoinstallerName + enableParallelism: command.enableParallelism, + incremental: command.incremental || false, + watchForChanges: command.watchForChanges || false, + disableBuildCache: command.disableBuildCache || false, + + actionPhases: command.phases, + phases: commandLineConfiguration.phases }) ); } diff --git a/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts index b6f1c81c3f1..3644c0100fa 100644 --- a/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts @@ -33,6 +33,19 @@ export abstract class BaseScriptAction extends BaseRushAction { this._commandLineConfiguration = options.commandLineConfiguration; } + /** + * @virtual + */ + protected isParameterAssociatedWithThisCommand(parameterJson: ParameterJson): boolean { + for (const associatedCommand of parameterJson.associatedCommands || []) { + if (associatedCommand === this.actionName) { + return true; + } + } + + return false; + } + protected defineScriptParameters(): void { if (!this._commandLineConfiguration) { return; @@ -40,14 +53,7 @@ export abstract class BaseScriptAction extends BaseRushAction { // Find any parameters that are associated with this command for (const parameterJson of this._commandLineConfiguration.parameters) { - let associated: boolean = false; - for (const associatedCommand of parameterJson.associatedCommands || []) { - if (associatedCommand === this.actionName) { - associated = true; - } - } - - if (associated) { + if (this.isParameterAssociatedWithThisCommand(parameterJson)) { let customParameter: CommandLineParameter | undefined; switch (parameterJson.parameterKind) { @@ -86,9 +92,7 @@ export abstract class BaseScriptAction extends BaseRushAction { ); } - if (customParameter) { - this.customParameters.push(customParameter); - } + this.customParameters.push(customParameter); } } } diff --git a/apps/rush-lib/src/cli/scriptActions/BaseBulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts similarity index 95% rename from apps/rush-lib/src/cli/scriptActions/BaseBulkScriptAction.ts rename to apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts index b8b717157ab..26089a1467e 100644 --- a/apps/rush-lib/src/cli/scriptActions/BaseBulkScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts @@ -23,23 +23,28 @@ import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; import { Selection } from '../../logic/Selection'; import { SelectionParameterSet } from '../SelectionParameterSet'; import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; -import { ITaskSelectorBaseOptions, ProjectTaskSelectorBase } from '../../logic/ProjectTaskSelectorBase'; +import { ITaskSelectorOptions, ProjectTaskSelector } from '../../logic/ProjectTaskSelectorBase'; +import { IPhaseJson } from '../../api/CommandLineJson'; + +export interface IPhase extends IPhaseJson { + logFilenameIdentifier: string; +} /** * Constructor parameters for BulkScriptAction. */ export interface IBaseBulkScriptActionOptions extends IBaseScriptActionOptions { enableParallelism: boolean; - ignoreMissingScript: boolean; - ignoreDependencyOrder: boolean; incremental: boolean; - allowWarningsInSuccessfulBuild: boolean; watchForChanges: boolean; disableBuildCache: boolean; + + actionPhases: (string | symbol)[]; + phases: Map; } interface IExecuteInternalOptions { - taskSelectorOptions: ITaskSelectorBaseOptions; + taskSelectorOptions: ITaskSelectorOptions; taskExecutionManagerOptions: ITaskExecutionManagerOptions; stopwatch: Stopwatch; ignoreHooks?: boolean; @@ -56,13 +61,15 @@ interface IExecuteInternalOptions { * and "rebuild" commands are also modeled as bulk commands, because they essentially just * execute scripts from package.json in the same as any custom command. */ -export abstract class BaseBulkScriptAction extends BaseScriptAction { +export class BulkScriptAction extends BaseScriptAction { private readonly _enableParallelism: boolean; private readonly _ignoreMissingScript: boolean; private readonly _isIncrementalBuildAllowed: boolean; private readonly _watchForChanges: boolean; private readonly _disableBuildCache: boolean; private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined; + private readonly _logFilenameIdentifier: string; + private readonly _commandToRun: string; private readonly _ignoreDependencyOrder: boolean; private readonly _allowWarningsInSuccessfulBuild: boolean; @@ -75,6 +82,8 @@ export abstract class BaseBulkScriptAction extends BaseScriptAction { public constructor(options: IBaseBulkScriptActionOptions) { super(options); this._enableParallelism = options.enableParallelism; + this._logFilenameIdentifier = options.logFilenameIdentifier; + this._commandToRun = options.commandToRun; this._ignoreMissingScript = options.ignoreMissingScript; this._isIncrementalBuildAllowed = options.incremental; this._ignoreDependencyOrder = options.ignoreDependencyOrder; @@ -135,7 +144,7 @@ export abstract class BaseBulkScriptAction extends BaseScriptAction { return; } - const taskSelectorOptions: ITaskSelectorBaseOptions = { + const taskSelectorOptions: ITaskSelectorOptions = { rushConfiguration: this.rushConfiguration, buildCacheConfiguration, selection, @@ -154,7 +163,6 @@ export abstract class BaseBulkScriptAction extends BaseScriptAction { debugMode: this.parser.isDebug, parallelism: parallelism, changedProjectsOnly: changedProjectsOnly, - allowWarningsInSuccessfulExecution: this._allowWarningsInSuccessfulBuild, repoCommandLineConfiguration: this._repoCommandLineConfiguration }; @@ -308,15 +316,11 @@ export abstract class BaseBulkScriptAction extends BaseScriptAction { this.defineScriptParameters(); } - protected abstract _getTaskSelector( - baseTaskSelectorOptions: ITaskSelectorBaseOptions - ): ProjectTaskSelectorBase; - /** * Runs a single invocation of the command */ private async _runOnce(options: IExecuteInternalOptions): Promise { - const taskSelector: ProjectTaskSelectorBase = this._getTaskSelector(options.taskSelectorOptions); + const taskSelector: ProjectTaskSelector = this._getTaskSelector(options.taskSelectorOptions); // Register all tasks with the task collection diff --git a/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts index 7c0b3f7f900..d3c7f2af8b6 100644 --- a/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts @@ -1,36 +1,41 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import { RushConfigurationProject } from '../..'; import { NonPhasedProjectTaskSelector } from '../../logic/NonPhasedProjectTaskSelector'; -import { ITaskSelectorBaseOptions } from '../../logic/ProjectTaskSelectorBase'; -import { BaseBulkScriptAction, IBaseBulkScriptActionOptions } from './BaseBulkScriptAction'; +import { ITaskSelectorOptions } from '../../logic/ProjectTaskSelectorBase'; +import { Selection } from '../../logic/Selection'; +import { BulkScriptAction, IBaseBulkScriptActionOptions } from './BulkScriptAction'; -export interface INonPhasedBulkScriptAction extends IBaseBulkScriptActionOptions { - logFilenameIdentifier: string; - commandToRun: string; -} +export interface INonPhasedBulkScriptAction extends IBaseBulkScriptActionOptions {} /** * This class implements bulk commands which are run individually for each project in the repo, * possibly in parallel, with a single command per project. */ -export class NonPhasedBulkScriptAction extends BaseBulkScriptAction { - private readonly _logFilenameIdentifier: string; - private readonly _commandToRun: string; - +export class NonPhasedBulkScriptAction extends BulkScriptAction { public constructor(options: INonPhasedBulkScriptAction) { super(options); - this._logFilenameIdentifier = options.logFilenameIdentifier; - this._commandToRun = options.commandToRun; } - protected _getTaskSelector( - baseTaskSelectorOptions: ITaskSelectorBaseOptions - ): NonPhasedProjectTaskSelector { + protected _getTaskSelector(baseTaskSelectorOptions: ITaskSelectorOptions): NonPhasedProjectTaskSelector { + const selection: ReadonlySet = this._ignoreDependencyOrder + ? baseTaskSelectorOptions.selection + : // If the command ignores dependency order, that means that only the changed projects should be affected + // That said, running watch for commands that ignore dependency order may have unexpected results + Selection.intersection( + Selection.expandAllConsumers(baseTaskSelectorOptions.selection), + projectsToWatch + ); + return new NonPhasedProjectTaskSelector({ ...baseTaskSelectorOptions, + selection, logFilenameIdentifier: this._logFilenameIdentifier, - commandToRun: this._commandToRun + commandToRun: this._commandToRun, + ignoreMissingScript: this._ignoreMissingScript, + ignoreDependencyOrder: this._ignoreDependencyOrder, + allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild }); } } diff --git a/apps/rush-lib/src/cli/scriptActions/PhasedBulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/PhasedBulkScriptAction.ts new file mode 100644 index 00000000000..7aed53b3354 --- /dev/null +++ b/apps/rush-lib/src/cli/scriptActions/PhasedBulkScriptAction.ts @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { IPhasedCommandJson } from '../../api/CommandLineJson'; +import { PhasedProjectTaskSelector } from '../../logic/PhasedProjectTaskSelector'; +import { ITaskSelectorOptions } from '../../logic/ProjectTaskSelectorBase'; +import { BulkScriptAction, IBaseBulkScriptActionOptions } from './BulkScriptAction'; + +export interface IPhasedBulkScriptAction extends IBaseBulkScriptActionOptions { + command: IPhasedCommandJson; +} + +/** + * This class implements bulk commands which are run individually for each project in the repo, + * possibly in parallel, with a single command per project. + */ +export class PhasedBulkScriptAction extends BulkScriptAction { + private readonly _logFilenameIdentifier: string; + private readonly _commandToRun: string; + + public constructor(options: IPhasedBulkScriptAction) { + super(options); + options; + this._logFilenameIdentifier = options.logFilenameIdentifier; + this._commandToRun = options.commandToRun; + } + + protected _getTaskSelector(baseTaskSelectorOptions: ITaskSelectorOptions): PhasedProjectTaskSelector { + return new PhasedProjectTaskSelector({ + ...baseTaskSelectorOptions, + logFilenameIdentifier: this._logFilenameIdentifier, + commandToRun: this._commandToRun + }); + } +} diff --git a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts index cf35798d23f..2c0bc4698f0 100644 --- a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts +++ b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts @@ -1,17 +1,14 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { ITaskSelectorBaseOptions, ProjectTaskSelectorBase } from './ProjectTaskSelectorBase'; +import { ITaskSelectorOptions, ProjectTaskSelector } from './ProjectTaskSelectorBase'; import { RushConfigurationProject } from '../api/RushConfigurationProject'; import { TaskCollection } from './taskExecution/TaskCollection'; import { ProjectTaskRunner } from './taskExecution/ProjectTaskRunner'; -export interface INonPhasedProjectTaskSelectorOptions extends ITaskSelectorBaseOptions { - logFilenameIdentifier: string; - commandToRun: string; -} +export interface INonPhasedProjectTaskSelectorOptions extends ITaskSelectorOptions {} -export class NonPhasedProjectTaskSelector extends ProjectTaskSelectorBase { +export class NonPhasedProjectTaskSelector extends ProjectTaskSelector { public registerTasks(): TaskCollection { const projects: ReadonlySet = this._options.selection; const taskCollection: TaskCollection = new TaskCollection(); @@ -67,7 +64,7 @@ export class NonPhasedProjectTaskSelector extends ProjectTaskSelectorBase; @@ -17,10 +22,7 @@ export interface ITaskSelectorBaseOptions { isQuietMode: boolean; isDebugMode: boolean; isIncrementalBuildAllowed: boolean; - ignoreMissingScript: boolean; - ignoreDependencyOrder: boolean; projectChangeAnalyzer?: ProjectChangeAnalyzer; - allowWarningsInSuccessfulBuild?: boolean; } /** @@ -29,13 +31,11 @@ export interface ITaskSelectorBaseOptions { * - creating a ProjectBuilder for each project that needs to be built * - registering the necessary ProjectBuilders with the TaskExecutionManager, which actually orchestrates execution */ -export abstract class ProjectTaskSelectorBase< - TOptions extends ITaskSelectorBaseOptions = ITaskSelectorBaseOptions -> { - protected _options: TOptions; +export class ProjectTaskSelector { + protected _options: ITaskSelectorOptions; protected _projectChangeAnalyzer: ProjectChangeAnalyzer; - public constructor(options: TOptions) { + public constructor(options: ITaskSelectorOptions) { this._options = options; const { projectChangeAnalyzer = new ProjectChangeAnalyzer(options.rushConfiguration) } = options; @@ -48,7 +48,7 @@ export abstract class ProjectTaskSelectorBase< commandToRun: string, customParameterValues: string[] ): string | undefined { - const script: string | undefined = ProjectTaskSelectorBase._getScriptCommand(rushProject, commandToRun); + const script: string | undefined = ProjectTaskSelector._getScriptCommand(rushProject, commandToRun); if (script === undefined) { return undefined; diff --git a/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts index 99b44ffc760..4f52a52b00c 100644 --- a/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts @@ -37,6 +37,12 @@ export abstract class BaseTaskRunner { */ public abstract hadEmptyScript: boolean; + /** + * If set to true, a warning result should not make Rush exit with a nonzero + * exit code + */ + public abstract warningsAreAllowed: boolean; + /** * Method to be executed for the task. */ diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts index ac3fd8cc4f2..f6d9b841493 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts @@ -78,11 +78,9 @@ function _areShallowEqual(object1: JsonObject, object2: JsonObject): boolean { export class ProjectTaskRunner extends BaseTaskRunner { public readonly name: string; - /** - * This property is mutated by TaskExecutionManager, so is not readonly - */ - public isSkipAllowed: boolean; + public readonly isSkipAllowed: boolean; public hadEmptyScript: boolean = false; + public readonly warningsAreAllowed: boolean; private readonly _rushProject: RushConfigurationProject; private readonly _rushConfiguration: RushConfiguration; @@ -92,7 +90,6 @@ export class ProjectTaskRunner extends BaseTaskRunner { private readonly _isCacheReadAllowed: boolean; private readonly _projectChangeAnalyzer: ProjectChangeAnalyzer; private readonly _packageDepsFilename: string; - private readonly _allowWarningsInSuccessfulBuild: boolean; private readonly _logFilenameIdentifier: string; /** @@ -113,7 +110,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { this.isSkipAllowed = options.isIncrementalBuildAllowed; this._projectChangeAnalyzer = options.projectChangeAnalyzer; this._packageDepsFilename = `package-deps_${this._commandToRun}.json`; - this._allowWarningsInSuccessfulBuild = options.allowWarningsInSuccessfulBuild || false; + this.warningsAreAllowed = options.allowWarningsInSuccessfulBuild || false; this._logFilenameIdentifier = options.logFilenameIdentifier; } @@ -364,7 +361,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { const taskIsSuccessful: boolean = status === TaskStatus.Success || (status === TaskStatus.SuccessWithWarning && - this._allowWarningsInSuccessfulBuild && + this.warningsAreAllowed && !!this._rushConfiguration.experimentsConfiguration.configuration .buildCacheWithAllowWarningsInSuccessfulBuild); diff --git a/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts b/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts index a52112c2d3f..083a4676641 100644 --- a/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts +++ b/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts @@ -25,7 +25,6 @@ export interface ITaskExecutionManagerOptions { debugMode: boolean; parallelism: string | undefined; changedProjectsOnly: boolean; - allowWarningsInSuccessfulExecution: boolean; repoCommandLineConfiguration: CommandLineConfiguration | undefined; destination?: TerminalWritable; } @@ -44,14 +43,13 @@ const ASCII_HEADER_WIDTH: number = 79; export class TaskExecutionManager { private readonly _tasks: Task[]; private readonly _changedProjectsOnly: boolean; - private readonly _allowWarningsInSuccessfulExecution: boolean; private readonly _taskQueue: Task[]; private readonly _quietMode: boolean; private readonly _debugMode: boolean; private readonly _parallelism: number; private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined; private _hasAnyFailures: boolean; - private _hasAnyWarnings: boolean; + private _hasAnyNonAllowedWarnings: boolean; private _currentActiveTasks!: number; private _totalTasks!: number; private _completedTasks!: number; @@ -63,22 +61,14 @@ export class TaskExecutionManager { private _terminal: CollatedTerminal; public constructor(orderedTasks: Task[], options: ITaskExecutionManagerOptions) { - const { - quietMode, - debugMode, - parallelism, - changedProjectsOnly, - allowWarningsInSuccessfulExecution, - repoCommandLineConfiguration - } = options; + const { quietMode, debugMode, parallelism, changedProjectsOnly, repoCommandLineConfiguration } = options; this._tasks = orderedTasks; this._taskQueue = [...orderedTasks]; // Clone the task array this._quietMode = quietMode; this._debugMode = debugMode; this._hasAnyFailures = false; - this._hasAnyWarnings = false; + this._hasAnyNonAllowedWarnings = false; this._changedProjectsOnly = changedProjectsOnly; - this._allowWarningsInSuccessfulExecution = allowWarningsInSuccessfulExecution; this._repoCommandLineConfiguration = repoCommandLineConfiguration; // TERMINAL PIPELINE: @@ -191,7 +181,7 @@ export class TaskExecutionManager { if (this._hasAnyFailures) { this._terminal.writeStderrLine(colors.red('Projects failed to build.') + '\n'); throw new AlreadyReportedError(); - } else if (this._hasAnyWarnings && !this._allowWarningsInSuccessfulExecution) { + } else if (this._hasAnyNonAllowedWarnings) { this._terminal.writeStderrLine(colors.yellow('Projects succeeded with warnings.') + '\n'); throw new AlreadyReportedError(); } @@ -262,7 +252,7 @@ export class TaskExecutionManager { this._markTaskAsSuccess(task); break; case TaskStatus.SuccessWithWarning: - this._hasAnyWarnings = true; + this._hasAnyNonAllowedWarnings = this._hasAnyNonAllowedWarnings || !task.runner.warningsAreAllowed; this._markTaskAsSuccessWithWarning(task); break; case TaskStatus.FromCache: diff --git a/apps/rush-lib/src/logic/taskExecution/test/MockTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/test/MockTaskRunner.ts index 0d6348166e4..b5cd1678b4f 100644 --- a/apps/rush-lib/src/logic/taskExecution/test/MockTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/test/MockTaskRunner.ts @@ -11,12 +11,18 @@ export class MockTaskRunner extends BaseTaskRunner { public readonly name: string; public readonly hadEmptyScript: boolean = false; public readonly isSkipAllowed: boolean = false; + public readonly warningsAreAllowed: boolean; - public constructor(name: string, action?: (terminal: CollatedTerminal) => Promise) { + public constructor( + name: string, + action?: (terminal: CollatedTerminal) => Promise, + warningsAreAllowed: boolean = false + ) { super(); this.name = name; this._action = action; + this.warningsAreAllowed = warningsAreAllowed; } public async executeAsync(context: ITaskRunnerContext): Promise { diff --git a/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts b/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts index f444f062c08..0b026d72857 100644 --- a/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts +++ b/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts @@ -70,7 +70,6 @@ describe(TaskExecutionManager.name, () => { parallelism: 'tequila', changedProjectsOnly: false, destination: mockWritable, - allowWarningsInSuccessfulExecution: false, repoCommandLineConfiguration: undefined }) ).toThrowErrorMatchingSnapshot(); @@ -85,7 +84,6 @@ describe(TaskExecutionManager.name, () => { parallelism: '1', changedProjectsOnly: false, destination: mockWritable, - allowWarningsInSuccessfulExecution: false, repoCommandLineConfiguration: undefined }; }); @@ -143,7 +141,6 @@ describe(TaskExecutionManager.name, () => { parallelism: '1', changedProjectsOnly: false, destination: mockWritable, - allowWarningsInSuccessfulExecution: false, repoCommandLineConfiguration: undefined }; }); @@ -179,7 +176,6 @@ describe(TaskExecutionManager.name, () => { parallelism: '1', changedProjectsOnly: false, destination: mockWritable, - allowWarningsInSuccessfulExecution: true, repoCommandLineConfiguration: undefined }; }); @@ -187,11 +183,15 @@ describe(TaskExecutionManager.name, () => { it('Logs warnings correctly', async () => { taskExecutionManager = createTaskExecutionManager( taskExecutionManagerOptions, - new MockTaskRunner('success with warnings (success)', async (terminal: CollatedTerminal) => { - terminal.writeStdoutLine('Build step 1' + EOL); - terminal.writeStdoutLine('Warning: step 1 succeeded with warnings' + EOL); - return TaskStatus.SuccessWithWarning; - }) + new MockTaskRunner( + 'success with warnings (success)', + async (terminal: CollatedTerminal) => { + terminal.writeStdoutLine('Build step 1' + EOL); + terminal.writeStdoutLine('Warning: step 1 succeeded with warnings' + EOL); + return TaskStatus.SuccessWithWarning; + }, + /* warningsAreAllowed */ true + ) ); await taskExecutionManager.executeAsync(); From bf046589a80b922ecdad8bcfc890ddfc0ad6ceaa Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Thu, 23 Dec 2021 15:11:50 -0800 Subject: [PATCH 03/19] Plumb command-line configuration to actions. --- .../src/cli/actions/WriteBuildCacheAction.ts | 10 +- .../src/cli/scriptActions/BaseScriptAction.ts | 70 +++---- .../cli/scriptActions/GlobalScriptAction.ts | 10 +- .../NonPhasedBulkScriptAction.ts | 41 ---- .../scriptActions/PhasedBulkScriptAction.ts | 35 ---- ...kScriptAction.ts => PhasedScriptAction.ts} | 77 +++----- .../src/logic/NonPhasedProjectTaskSelector.ts | 101 ---------- .../rush-lib/src/logic/ProjectTaskSelector.ts | 186 ++++++++++++++++++ .../src/logic/ProjectTaskSelectorBase.ts | 83 -------- 9 files changed, 252 insertions(+), 361 deletions(-) delete mode 100644 apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts delete mode 100644 apps/rush-lib/src/cli/scriptActions/PhasedBulkScriptAction.ts rename apps/rush-lib/src/cli/scriptActions/{BulkScriptAction.ts => PhasedScriptAction.ts} (83%) delete mode 100644 apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts create mode 100644 apps/rush-lib/src/logic/ProjectTaskSelector.ts delete mode 100644 apps/rush-lib/src/logic/ProjectTaskSelectorBase.ts diff --git a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts index 6e581452336..3de57603763 100644 --- a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts +++ b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts @@ -12,7 +12,7 @@ import { RushCommandLineParser } from '../RushCommandLineParser'; import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; import { ProjectTaskRunner } from '../../logic/taskExecution/ProjectTaskRunner'; import { ProjectChangeAnalyzer } from '../../logic/ProjectChangeAnalyzer'; -import { NonPhasedProjectTaskSelector } from '../../logic/NonPhasedProjectTaskSelector'; +import { ProjectTaskSelector } from '../../logic/ProjectTaskSelector'; import { RushConstants } from '../../logic/RushConstants'; import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; import { ProjectLogWritable } from '../../logic/taskExecution/ProjectLogWritable'; @@ -74,14 +74,10 @@ export class WriteBuildCacheAction extends BaseRushAction { ); const command: string = this._command.value!; - const commandToRun: string | undefined = NonPhasedProjectTaskSelector.getScriptToRun( - project, - command, - [] - ); + const commandToRun: string | undefined = ProjectTaskSelector.getScriptToRun(project, command, []); // TODO: With phased commands, this will need to be updated - const taskName: string = NonPhasedProjectTaskSelector.getTaskNameForProject(project); + const taskName: string = ProjectTaskSelector.getPhaseTaskNameForProject(project); const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(this.rushConfiguration); const projectBuilder: ProjectTaskRunner = new ProjectTaskRunner({ rushProject: project, diff --git a/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts index 3644c0100fa..1f716d44503 100644 --- a/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts @@ -3,15 +3,16 @@ import { CommandLineParameter } from '@rushstack/ts-command-line'; import { BaseRushAction, IBaseRushActionOptions } from '../actions/BaseRushAction'; -import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; +import { Command, CommandLineConfiguration } from '../../api/CommandLineConfiguration'; import { RushConstants } from '../../logic/RushConstants'; import type { ParameterJson } from '../../api/CommandLineJson'; /** * Constructor parameters for BaseScriptAction */ -export interface IBaseScriptActionOptions extends IBaseRushActionOptions { +export interface IBaseScriptActionOptions extends IBaseRushActionOptions { commandLineConfiguration: CommandLineConfiguration | undefined; + command: TCommand; } /** @@ -24,71 +25,60 @@ export interface IBaseScriptActionOptions extends IBaseRushActionOptions { * * The two subclasses are BulkScriptAction and GlobalScriptAction. */ -export abstract class BaseScriptAction extends BaseRushAction { - protected readonly _commandLineConfiguration: CommandLineConfiguration | undefined; +export abstract class BaseScriptAction extends BaseRushAction { + protected readonly commandLineConfiguration: CommandLineConfiguration | undefined; protected readonly customParameters: CommandLineParameter[] = []; + protected readonly command: TCommand; - public constructor(options: IBaseScriptActionOptions) { + public constructor(options: IBaseScriptActionOptions) { super(options); - this._commandLineConfiguration = options.commandLineConfiguration; - } - - /** - * @virtual - */ - protected isParameterAssociatedWithThisCommand(parameterJson: ParameterJson): boolean { - for (const associatedCommand of parameterJson.associatedCommands || []) { - if (associatedCommand === this.actionName) { - return true; - } - } - - return false; + this.commandLineConfiguration = options.commandLineConfiguration; + this.command = options.command; } protected defineScriptParameters(): void { - if (!this._commandLineConfiguration) { + if (!this.commandLineConfiguration) { return; } // Find any parameters that are associated with this command - for (const parameterJson of this._commandLineConfiguration.parameters) { - if (this.isParameterAssociatedWithThisCommand(parameterJson)) { + for (const parameter of this.commandLineConfiguration.parameters) { + if (this.command.associatedParameters.has(parameter)) { let customParameter: CommandLineParameter | undefined; - switch (parameterJson.parameterKind) { + switch (parameter.parameterKind) { case 'flag': customParameter = this.defineFlagParameter({ - parameterShortName: parameterJson.shortName, - parameterLongName: parameterJson.longName, - description: parameterJson.description, - required: parameterJson.required + parameterShortName: parameter.shortName, + parameterLongName: parameter.longName, + description: parameter.description, + required: parameter.required }); break; case 'choice': customParameter = this.defineChoiceParameter({ - parameterShortName: parameterJson.shortName, - parameterLongName: parameterJson.longName, - description: parameterJson.description, - required: parameterJson.required, - alternatives: parameterJson.alternatives.map((x) => x.name), - defaultValue: parameterJson.defaultValue + parameterShortName: parameter.shortName, + parameterLongName: parameter.longName, + description: parameter.description, + required: parameter.required, + alternatives: parameter.alternatives.map((x) => x.name), + defaultValue: parameter.defaultValue }); break; case 'string': customParameter = this.defineStringParameter({ - parameterLongName: parameterJson.longName, - parameterShortName: parameterJson.shortName, - description: parameterJson.description, - required: parameterJson.required, - argumentName: parameterJson.argumentName + parameterLongName: parameter.longName, + parameterShortName: parameter.shortName, + description: parameter.description, + required: parameter.required, + argumentName: parameter.argumentName }); break; default: throw new Error( `${RushConstants.commandLineFilename} defines a parameter "${ - (parameterJson as ParameterJson).longName - }" using an unsupported parameter kind "${(parameterJson as ParameterJson).parameterKind}"` + (parameter as ParameterJson).longName + }" using an unsupported parameter kind "${(parameter as ParameterJson).parameterKind}"` ); } diff --git a/apps/rush-lib/src/cli/scriptActions/GlobalScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/GlobalScriptAction.ts index d7fc51fde29..2c49b9e1878 100644 --- a/apps/rush-lib/src/cli/scriptActions/GlobalScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/GlobalScriptAction.ts @@ -9,12 +9,12 @@ import { FileSystem, IPackageJson, JsonFile, AlreadyReportedError, Text } from ' import { BaseScriptAction, IBaseScriptActionOptions } from './BaseScriptAction'; import { Utilities } from '../../utilities/Utilities'; import { Autoinstaller } from '../../logic/Autoinstaller'; -import { IShellCommandTokenContext } from '../../api/CommandLineConfiguration'; +import { IGlobalCommand, IShellCommandTokenContext } from '../../api/CommandLineConfiguration'; /** * Constructor parameters for GlobalScriptAction. */ -export interface IGlobalScriptActionOptions extends IBaseScriptActionOptions { +export interface IGlobalScriptActionOptions extends IBaseScriptActionOptions { shellCommand: string; autoinstallerName: string | undefined; } @@ -29,7 +29,7 @@ export interface IGlobalScriptActionOptions extends IBaseScriptActionOptions { * and "rebuild" commands are also modeled as bulk commands, because they essentially just * invoke scripts from package.json in the same way as a custom command. */ -export class GlobalScriptAction extends BaseScriptAction { +export class GlobalScriptAction extends BaseScriptAction { private readonly _shellCommand: string; private readonly _autoinstallerName: string; private readonly _autoinstallerFullPath: string; @@ -88,7 +88,7 @@ export class GlobalScriptAction extends BaseScriptAction { public async runAsync(): Promise { const additionalPathFolders: string[] = - this._commandLineConfiguration?.additionalPathFolders.slice() || []; + this.commandLineConfiguration?.additionalPathFolders.slice() || []; if (this._autoinstallerName) { await this._prepareAutoinstallerName(); @@ -121,7 +121,7 @@ export class GlobalScriptAction extends BaseScriptAction { } const shellCommandTokenContext: IShellCommandTokenContext | undefined = - this._commandLineConfiguration?.shellCommandTokenContext; + this.commandLineConfiguration?.shellCommandTokenContext; if (shellCommandTokenContext) { shellCommand = this._expandShellCommandWithTokens(shellCommand, shellCommandTokenContext); } diff --git a/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts deleted file mode 100644 index d3c7f2af8b6..00000000000 --- a/apps/rush-lib/src/cli/scriptActions/NonPhasedBulkScriptAction.ts +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -import { RushConfigurationProject } from '../..'; -import { NonPhasedProjectTaskSelector } from '../../logic/NonPhasedProjectTaskSelector'; -import { ITaskSelectorOptions } from '../../logic/ProjectTaskSelectorBase'; -import { Selection } from '../../logic/Selection'; -import { BulkScriptAction, IBaseBulkScriptActionOptions } from './BulkScriptAction'; - -export interface INonPhasedBulkScriptAction extends IBaseBulkScriptActionOptions {} - -/** - * This class implements bulk commands which are run individually for each project in the repo, - * possibly in parallel, with a single command per project. - */ -export class NonPhasedBulkScriptAction extends BulkScriptAction { - public constructor(options: INonPhasedBulkScriptAction) { - super(options); - } - - protected _getTaskSelector(baseTaskSelectorOptions: ITaskSelectorOptions): NonPhasedProjectTaskSelector { - const selection: ReadonlySet = this._ignoreDependencyOrder - ? baseTaskSelectorOptions.selection - : // If the command ignores dependency order, that means that only the changed projects should be affected - // That said, running watch for commands that ignore dependency order may have unexpected results - Selection.intersection( - Selection.expandAllConsumers(baseTaskSelectorOptions.selection), - projectsToWatch - ); - - return new NonPhasedProjectTaskSelector({ - ...baseTaskSelectorOptions, - selection, - logFilenameIdentifier: this._logFilenameIdentifier, - commandToRun: this._commandToRun, - ignoreMissingScript: this._ignoreMissingScript, - ignoreDependencyOrder: this._ignoreDependencyOrder, - allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild - }); - } -} diff --git a/apps/rush-lib/src/cli/scriptActions/PhasedBulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/PhasedBulkScriptAction.ts deleted file mode 100644 index 7aed53b3354..00000000000 --- a/apps/rush-lib/src/cli/scriptActions/PhasedBulkScriptAction.ts +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -import { IPhasedCommandJson } from '../../api/CommandLineJson'; -import { PhasedProjectTaskSelector } from '../../logic/PhasedProjectTaskSelector'; -import { ITaskSelectorOptions } from '../../logic/ProjectTaskSelectorBase'; -import { BulkScriptAction, IBaseBulkScriptActionOptions } from './BulkScriptAction'; - -export interface IPhasedBulkScriptAction extends IBaseBulkScriptActionOptions { - command: IPhasedCommandJson; -} - -/** - * This class implements bulk commands which are run individually for each project in the repo, - * possibly in parallel, with a single command per project. - */ -export class PhasedBulkScriptAction extends BulkScriptAction { - private readonly _logFilenameIdentifier: string; - private readonly _commandToRun: string; - - public constructor(options: IPhasedBulkScriptAction) { - super(options); - options; - this._logFilenameIdentifier = options.logFilenameIdentifier; - this._commandToRun = options.commandToRun; - } - - protected _getTaskSelector(baseTaskSelectorOptions: ITaskSelectorOptions): PhasedProjectTaskSelector { - return new PhasedProjectTaskSelector({ - ...baseTaskSelectorOptions, - logFilenameIdentifier: this._logFilenameIdentifier, - commandToRun: this._commandToRun - }); - } -} diff --git a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts similarity index 83% rename from apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts rename to apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index 26089a1467e..3939bd70d96 100644 --- a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -20,31 +20,25 @@ import { EnvironmentVariableNames } from '../../api/EnvironmentConfiguration'; import { LastLinkFlag, LastLinkFlagFactory } from '../../api/LastLinkFlag'; import { RushConfigurationProject } from '../../api/RushConfigurationProject'; import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; -import { Selection } from '../../logic/Selection'; import { SelectionParameterSet } from '../SelectionParameterSet'; -import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; -import { ITaskSelectorOptions, ProjectTaskSelector } from '../../logic/ProjectTaskSelectorBase'; -import { IPhaseJson } from '../../api/CommandLineJson'; - -export interface IPhase extends IPhaseJson { - logFilenameIdentifier: string; -} +import { CommandLineConfiguration, IPhase, IPhasedCommand } from '../../api/CommandLineConfiguration'; +import { IProjectTaskSelectorOptions, ProjectTaskSelector } from '../../logic/ProjectTaskSelector'; /** * Constructor parameters for BulkScriptAction. */ -export interface IBaseBulkScriptActionOptions extends IBaseScriptActionOptions { +export interface IPhasedScriptActionOptions extends IBaseScriptActionOptions { enableParallelism: boolean; incremental: boolean; watchForChanges: boolean; disableBuildCache: boolean; - actionPhases: (string | symbol)[]; - phases: Map; + actionPhases: string[]; + phases: Map; } interface IExecuteInternalOptions { - taskSelectorOptions: ITaskSelectorOptions; + taskSelectorOptions: IProjectTaskSelectorOptions; taskExecutionManagerOptions: ITaskExecutionManagerOptions; stopwatch: Stopwatch; ignoreHooks?: boolean; @@ -52,7 +46,7 @@ interface IExecuteInternalOptions { } /** - * This base class implements bulk commands which are run individually for each project in the repo, + * This class implements bulk commands which are run individually for each project in the repo, * possibly in parallel. The task selector is abstract and is implemented for phased or non-phased * commands. * @@ -61,17 +55,14 @@ interface IExecuteInternalOptions { * and "rebuild" commands are also modeled as bulk commands, because they essentially just * execute scripts from package.json in the same as any custom command. */ -export class BulkScriptAction extends BaseScriptAction { +export class PhasedScriptAction extends BaseScriptAction { private readonly _enableParallelism: boolean; - private readonly _ignoreMissingScript: boolean; private readonly _isIncrementalBuildAllowed: boolean; private readonly _watchForChanges: boolean; private readonly _disableBuildCache: boolean; private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined; - private readonly _logFilenameIdentifier: string; - private readonly _commandToRun: string; - private readonly _ignoreDependencyOrder: boolean; - private readonly _allowWarningsInSuccessfulBuild: boolean; + private readonly _actionPhases: string[]; + private readonly _phases: Map; private _changedProjectsOnly!: CommandLineFlagParameter; private _selectionParameters!: SelectionParameterSet; @@ -79,18 +70,15 @@ export class BulkScriptAction extends BaseScriptAction { private _parallelismParameter: CommandLineStringParameter | undefined; private _ignoreHooksParameter!: CommandLineFlagParameter; - public constructor(options: IBaseBulkScriptActionOptions) { + public constructor(options: IPhasedScriptActionOptions) { super(options); this._enableParallelism = options.enableParallelism; - this._logFilenameIdentifier = options.logFilenameIdentifier; - this._commandToRun = options.commandToRun; - this._ignoreMissingScript = options.ignoreMissingScript; this._isIncrementalBuildAllowed = options.incremental; - this._ignoreDependencyOrder = options.ignoreDependencyOrder; - this._allowWarningsInSuccessfulBuild = options.allowWarningsInSuccessfulBuild; this._watchForChanges = options.watchForChanges; this._disableBuildCache = options.disableBuildCache; this._repoCommandLineConfiguration = options.commandLineConfiguration; + this._actionPhases = options.actionPhases; + this._phases = options.phases; } public async runAsync(): Promise { @@ -127,7 +115,7 @@ export class BulkScriptAction extends BaseScriptAction { const terminal: Terminal = new Terminal(this.rushSession.terminalProvider); let buildCacheConfiguration: BuildCacheConfiguration | undefined; - if (!this._disableBuildCache && ['build', 'rebuild'].includes(this.actionName)) { + if (!this._disableBuildCache) { buildCacheConfiguration = await BuildCacheConfiguration.tryLoadAsync( terminal, this.rushConfiguration, @@ -135,27 +123,24 @@ export class BulkScriptAction extends BaseScriptAction { ); } - const selection: Set = await this._selectionParameters.getSelectedProjectsAsync( - terminal - ); + const projectSelection: Set = + await this._selectionParameters.getSelectedProjectsAsync(terminal); - if (!selection.size) { + if (!projectSelection.size) { terminal.writeLine(colors.yellow(`The command line selection parameters did not match any projects.`)); return; } - const taskSelectorOptions: ITaskSelectorOptions = { + const taskSelectorOptions: IProjectTaskSelectorOptions = { rushConfiguration: this.rushConfiguration, buildCacheConfiguration, - selection, - commandName: this.actionName, + projectSelection, customParameterValues, isQuietMode: isQuietMode, isDebugMode: isDebugMode, isIncrementalBuildAllowed: this._isIncrementalBuildAllowed, - ignoreMissingScript: this._ignoreMissingScript, - ignoreDependencyOrder: this._ignoreDependencyOrder, - allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild + phasesToRun: this._actionPhases, + phases: this._phases }; const taskExecutionManagerOptions: ITaskExecutionManagerOptions = { @@ -189,7 +174,7 @@ export class BulkScriptAction extends BaseScriptAction { */ private async _runWatch(options: IExecuteInternalOptions): Promise { const { - taskSelectorOptions: { selection: projectsToWatch }, + taskSelectorOptions: { projectSelection: projectsToWatch }, stopwatch, terminal } = options; @@ -219,31 +204,25 @@ export class BulkScriptAction extends BaseScriptAction { // On the initial invocation, this promise will return immediately with the full set of projects const { changedProjects, state } = await projectWatcher.waitForChange(onWatchingFiles); - let selection: ReadonlySet = changedProjects; - if (stopwatch.state === StopwatchState.Stopped) { // Clear and reset the stopwatch so that we only report time from a single execution at a time stopwatch.reset(); stopwatch.start(); } - terminal.writeLine(`Detected changes in ${selection.size} project${selection.size === 1 ? '' : 's'}:`); - const names: string[] = [...selection].map((x) => x.packageName).sort(); + terminal.writeLine( + `Detected changes in ${changedProjects.size} project${changedProjects.size === 1 ? '' : 's'}:` + ); + const names: string[] = [...changedProjects].map((x) => x.packageName).sort(); for (const name of names) { terminal.writeLine(` ${colors.cyan(name)}`); } - // If the command ignores dependency order, that means that only the changed projects should be affected - // That said, running watch for commands that ignore dependency order may have unexpected results - if (!this._ignoreDependencyOrder) { - selection = Selection.intersection(Selection.expandAllConsumers(selection), projectsToWatch); - } - const executeOptions: IExecuteInternalOptions = { taskSelectorOptions: { ...options.taskSelectorOptions, // Revise down the set of projects to execute the command on - selection, + projectSelection: changedProjects, // Pass the ProjectChangeAnalyzer from the state differ to save a bit of overhead projectChangeAnalyzer: state }, @@ -320,7 +299,7 @@ export class BulkScriptAction extends BaseScriptAction { * Runs a single invocation of the command */ private async _runOnce(options: IExecuteInternalOptions): Promise { - const taskSelector: ProjectTaskSelector = this._getTaskSelector(options.taskSelectorOptions); + const taskSelector: ProjectTaskSelector = new ProjectTaskSelector(options.taskSelectorOptions); // Register all tasks with the task collection diff --git a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts deleted file mode 100644 index 2c0bc4698f0..00000000000 --- a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts +++ /dev/null @@ -1,101 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -import { ITaskSelectorOptions, ProjectTaskSelector } from './ProjectTaskSelectorBase'; -import { RushConfigurationProject } from '../api/RushConfigurationProject'; -import { TaskCollection } from './taskExecution/TaskCollection'; -import { ProjectTaskRunner } from './taskExecution/ProjectTaskRunner'; - -export interface INonPhasedProjectTaskSelectorOptions extends ITaskSelectorOptions {} - -export class NonPhasedProjectTaskSelector extends ProjectTaskSelector { - public registerTasks(): TaskCollection { - const projects: ReadonlySet = this._options.selection; - const taskCollection: TaskCollection = new TaskCollection(); - - // Register all tasks - for (const rushProject of projects) { - this._registerProjectTask(rushProject, taskCollection); - } - - if (!this._options.ignoreDependencyOrder) { - const dependencyMap: Map> = new Map(); - - // Generate the filtered dependency graph for selected projects - function getDependencyTaskNames(project: RushConfigurationProject): Set { - const cached: Set | undefined = dependencyMap.get(project); - if (cached) { - return cached; - } - - const dependencyTaskNames: Set = new Set(); - dependencyMap.set(project, dependencyTaskNames); - - for (const dep of project.dependencyProjects) { - if (projects.has(dep)) { - // Add direct relationships for projects in the set - dependencyTaskNames.add(NonPhasedProjectTaskSelector.getTaskNameForProject(dep)); - } else { - // Add indirect relationships for projects not in the set - for (const indirectDep of getDependencyTaskNames(dep)) { - dependencyTaskNames.add(indirectDep); - } - } - } - - return dependencyTaskNames; - } - - // Add ordering relationships for each dependency - for (const project of projects) { - taskCollection.addDependencies( - NonPhasedProjectTaskSelector.getTaskNameForProject(project), - getDependencyTaskNames(project) - ); - } - } - - return taskCollection; - } - - private _registerProjectTask(project: RushConfigurationProject, taskCollection: TaskCollection): void { - const taskName: string = NonPhasedProjectTaskSelector.getTaskNameForProject(project); - if (taskCollection.hasTask(taskName)) { - return; - } - - const commandToRun: string | undefined = ProjectTaskSelector.getScriptToRun( - project, - this._options.commandToRun, - this._options.customParameterValues - ); - if (commandToRun === undefined && !this._options.ignoreMissingScript) { - throw new Error( - `The project [${project.packageName}] does not define a '${this._options.commandToRun}' command in the 'scripts' section of its package.json` - ); - } - - taskCollection.addTask( - new ProjectTaskRunner({ - rushProject: project, - taskName, - rushConfiguration: this._options.rushConfiguration, - buildCacheConfiguration: this._options.buildCacheConfiguration, - commandToRun: commandToRun || '', - commandName: this._options.commandName, - isIncrementalBuildAllowed: this._options.isIncrementalBuildAllowed, - projectChangeAnalyzer: this._projectChangeAnalyzer, - allowWarningsInSuccessfulBuild: this._options.allowWarningsInSuccessfulBuild, - logFilenameIdentifier: this._options.logFilenameIdentifier - }) - ); - } - - /** - * A helper method to determine the task name of a ProjectBuilder. Used when the task - * name is required before a task is created. - */ - public static getTaskNameForProject(rushProject: RushConfigurationProject): string { - return rushProject.packageName; - } -} diff --git a/apps/rush-lib/src/logic/ProjectTaskSelector.ts b/apps/rush-lib/src/logic/ProjectTaskSelector.ts new file mode 100644 index 00000000000..64bb6804868 --- /dev/null +++ b/apps/rush-lib/src/logic/ProjectTaskSelector.ts @@ -0,0 +1,186 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { BuildCacheConfiguration } from '../api/BuildCacheConfiguration'; +import { RushConfiguration } from '../api/RushConfiguration'; +import { RushConfigurationProject } from '../api/RushConfigurationProject'; +import { convertSlashesForWindows, ProjectTaskRunner } from './taskExecution/ProjectTaskRunner'; +import { ProjectChangeAnalyzer } from './ProjectChangeAnalyzer'; +import { TaskCollection } from './taskExecution/TaskCollection'; +import { IPhase } from '../api/CommandLineConfiguration'; +import { EnvironmentConfiguration } from '..'; + +export interface IProjectTaskSelectorOptions { + rushConfiguration: RushConfiguration; + buildCacheConfiguration: BuildCacheConfiguration | undefined; + projectSelection: ReadonlySet; + customParameterValues: string[]; + isQuietMode: boolean; + isDebugMode: boolean; + isIncrementalBuildAllowed: boolean; + projectChangeAnalyzer?: ProjectChangeAnalyzer; + + phasesToRun: ReadonlyArray; + phases: Map; +} + +/** + * This class is responsible for: + * - based on to/from flags, solving the dependency graph and figuring out which projects need to be run + * - creating a ProjectBuilder for each project that needs to be built + * - registering the necessary ProjectBuilders with the TaskExecutionManager, which actually orchestrates execution + */ +export class ProjectTaskSelector { + private readonly _options: IProjectTaskSelectorOptions; + private readonly _projectChangeAnalyzer: ProjectChangeAnalyzer; + private readonly _phasesToRun: ReadonlyArray; + private readonly _phases: Map; + private readonly _overrideAllowWarnings: boolean = EnvironmentConfiguration.allowWarningsInSuccessfulBuild; + + public constructor(options: IProjectTaskSelectorOptions) { + this._options = options; + this._projectChangeAnalyzer = + options.projectChangeAnalyzer || new ProjectChangeAnalyzer(options.rushConfiguration); + this._phasesToRun = options.phasesToRun; + this._phases = options.phases; + } + + public static getScriptToRun( + rushProject: RushConfigurationProject, + commandToRun: string, + customParameterValues: string[] + ): string | undefined { + const script: string | undefined = ProjectTaskSelector._getScriptCommand(rushProject, commandToRun); + + if (script === undefined) { + return undefined; + } + + if (!script) { + return ''; + } else { + const taskCommand: string = `${script} ${customParameterValues.join(' ')}`; + return process.platform === 'win32' ? convertSlashesForWindows(taskCommand) : taskCommand; + } + } + + public registerTasks(): TaskCollection { + const projects: ReadonlySet = this._options.projectSelection; + const taskCollection: TaskCollection = new TaskCollection(); + + // Register all tasks + for (const phaseName of this._phasesToRun) { + const phase: IPhase | undefined = this._phases.get(phaseName); + if (!phase) { + throw new Error(`Phase ${phaseName} not found`); + } + + for (const rushProject of projects) { + this._registerProjectPhaseTask(rushProject, phase, taskCollection); + } + } + + // TODO: Expand phase dependencies + + // if (!this._options.ignoreDependencyOrder) { + // const dependencyMap: Map> = new Map(); + + // // Generate the filtered dependency graph for selected projects + // function getDependencyTaskNames(project: RushConfigurationProject): Set { + // const cached: Set | undefined = dependencyMap.get(project); + // if (cached) { + // return cached; + // } + + // const dependencyTaskNames: Set = new Set(); + // dependencyMap.set(project, dependencyTaskNames); + + // for (const dep of project.dependencyProjects) { + // if (projects.has(dep)) { + // // Add direct relationships for projects in the set + // dependencyTaskNames.add(ProjectTaskSelector.getPhaseTaskNameForProject(dep)); + // } else { + // // Add indirect relationships for projects not in the set + // for (const indirectDep of getDependencyTaskNames(dep)) { + // dependencyTaskNames.add(indirectDep); + // } + // } + // } + + // return dependencyTaskNames; + // } + + // // Add ordering relationships for each dependency + // for (const project of projects) { + // taskCollection.addDependencies( + // ProjectTaskSelector.getPhaseTaskNameForProject(project), + // getDependencyTaskNames(project) + // ); + // } + // } + + return taskCollection; + } + + private _registerProjectPhaseTask( + project: RushConfigurationProject, + phase: IPhase, + taskCollection: TaskCollection + ): void { + const taskName: string = ProjectTaskSelector.getPhaseTaskNameForProject(project, phase); + if (taskCollection.hasTask(taskName)) { + return; + } + + const commandToRun: string | undefined = ProjectTaskSelector.getScriptToRun( + project, + phase.name, + this._options.customParameterValues + ); + if (commandToRun === undefined && !phase.ignoreMissingScript) { + throw new Error( + `The project [${project.packageName}] does not define a '${phase.name}' command in the 'scripts' section of its package.json` + ); + } + + taskCollection.addTask( + new ProjectTaskRunner({ + rushProject: project, + taskName, + rushConfiguration: this._options.rushConfiguration, + buildCacheConfiguration: this._options.buildCacheConfiguration, + commandToRun: commandToRun || '', + commandName: phase.name, + isIncrementalBuildAllowed: this._options.isIncrementalBuildAllowed, + projectChangeAnalyzer: this._projectChangeAnalyzer, + allowWarningsInSuccessfulBuild: this._overrideAllowWarnings || phase.allowWarningsOnSuccess, + logFilenameIdentifier: phase.logFilenameIdentifier + }) + ); + } + + /** + * A helper method to determine the task name of a ProjectBuilder. Used when the task + * name is required before a task is created. + */ + public static getPhaseTaskNameForProject(rushProject: RushConfigurationProject, phase: IPhase): string { + return `${rushProject.packageName} (${phase.name})`; + } + + private static _getScriptCommand( + rushProject: RushConfigurationProject, + script: string + ): string | undefined { + if (!rushProject.packageJson.scripts) { + return undefined; + } + + const rawCommand: string = rushProject.packageJson.scripts[script]; + + if (rawCommand === undefined || rawCommand === null) { + return undefined; + } + + return rawCommand; + } +} diff --git a/apps/rush-lib/src/logic/ProjectTaskSelectorBase.ts b/apps/rush-lib/src/logic/ProjectTaskSelectorBase.ts deleted file mode 100644 index 2b5c3e3c0d2..00000000000 --- a/apps/rush-lib/src/logic/ProjectTaskSelectorBase.ts +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -import { BuildCacheConfiguration } from '../api/BuildCacheConfiguration'; -import { RushConfiguration } from '../api/RushConfiguration'; -import { RushConfigurationProject } from '../api/RushConfigurationProject'; -import { convertSlashesForWindows } from './taskExecution/ProjectTaskRunner'; -import { ProjectChangeAnalyzer } from './ProjectChangeAnalyzer'; -import { TaskCollection } from './taskExecution/TaskCollection'; - -export interface ITaskSelectorOptions { - logFilenameIdentifier: string; - commandToRun: string; - ignoreMissingScript: boolean; - ignoreDependencyOrder: boolean; - allowWarningsInSuccessfulBuild?: boolean; - rushConfiguration: RushConfiguration; - buildCacheConfiguration: BuildCacheConfiguration | undefined; - selection: ReadonlySet; - commandName: string; - customParameterValues: string[]; - isQuietMode: boolean; - isDebugMode: boolean; - isIncrementalBuildAllowed: boolean; - projectChangeAnalyzer?: ProjectChangeAnalyzer; -} - -/** - * This class is responsible for: - * - based on to/from flags, solving the dependency graph and figuring out which projects need to be run - * - creating a ProjectBuilder for each project that needs to be built - * - registering the necessary ProjectBuilders with the TaskExecutionManager, which actually orchestrates execution - */ -export class ProjectTaskSelector { - protected _options: ITaskSelectorOptions; - protected _projectChangeAnalyzer: ProjectChangeAnalyzer; - - public constructor(options: ITaskSelectorOptions) { - this._options = options; - - const { projectChangeAnalyzer = new ProjectChangeAnalyzer(options.rushConfiguration) } = options; - - this._projectChangeAnalyzer = projectChangeAnalyzer; - } - - public static getScriptToRun( - rushProject: RushConfigurationProject, - commandToRun: string, - customParameterValues: string[] - ): string | undefined { - const script: string | undefined = ProjectTaskSelector._getScriptCommand(rushProject, commandToRun); - - if (script === undefined) { - return undefined; - } - - if (!script) { - return ''; - } else { - const taskCommand: string = `${script} ${customParameterValues.join(' ')}`; - return process.platform === 'win32' ? convertSlashesForWindows(taskCommand) : taskCommand; - } - } - - public abstract registerTasks(): TaskCollection; - - private static _getScriptCommand( - rushProject: RushConfigurationProject, - script: string - ): string | undefined { - if (!rushProject.packageJson.scripts) { - return undefined; - } - - const rawCommand: string = rushProject.packageJson.scripts[script]; - - if (rawCommand === undefined || rawCommand === null) { - return undefined; - } - - return rawCommand; - } -} From 8a2f012cdb3580fd4a1e3ad3982667328a8ce6fc Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Fri, 24 Dec 2021 19:58:35 -0800 Subject: [PATCH 04/19] Remove the write-build-cache action. --- .../rush-lib/src/cli/RushCommandLineParser.ts | 2 - .../src/cli/actions/WriteBuildCacheAction.ts | 116 ------------------ .../CommandLineHelp.test.ts.snap | 17 --- .../logic/taskExecution/ProjectTaskRunner.ts | 21 +--- 4 files changed, 5 insertions(+), 151 deletions(-) delete mode 100644 apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts index 143a4512c61..348a59b6dc6 100644 --- a/apps/rush-lib/src/cli/RushCommandLineParser.ts +++ b/apps/rush-lib/src/cli/RushCommandLineParser.ts @@ -42,7 +42,6 @@ import { UpdateAction } from './actions/UpdateAction'; import { UpdateAutoinstallerAction } from './actions/UpdateAutoinstallerAction'; import { VersionAction } from './actions/VersionAction'; import { UpdateCloudCredentialsAction } from './actions/UpdateCloudCredentialsAction'; -import { WriteBuildCacheAction } from './actions/WriteBuildCacheAction'; import { GlobalScriptAction } from './scriptActions/GlobalScriptAction'; import { IBaseScriptActionOptions } from './scriptActions/BaseScriptAction'; @@ -228,7 +227,6 @@ export class RushCommandLineParser extends CommandLineParser { this.addAction(new UpdateAutoinstallerAction(this)); this.addAction(new UpdateCloudCredentialsAction(this)); this.addAction(new VersionAction(this)); - this.addAction(new WriteBuildCacheAction(this)); this._populateScriptActions(); } catch (error) { diff --git a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts deleted file mode 100644 index 3de57603763..00000000000 --- a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts +++ /dev/null @@ -1,116 +0,0 @@ -// 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 { AlreadyReportedError, ConsoleTerminalProvider, Terminal } from '@rushstack/node-core-library'; -import { CommandLineFlagParameter, CommandLineStringParameter } from '@rushstack/ts-command-line'; - -import { RushConfigurationProject } from '../../api/RushConfigurationProject'; -import { BaseRushAction } from './BaseRushAction'; -import { RushCommandLineParser } from '../RushCommandLineParser'; - -import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; -import { ProjectTaskRunner } from '../../logic/taskExecution/ProjectTaskRunner'; -import { ProjectChangeAnalyzer } from '../../logic/ProjectChangeAnalyzer'; -import { ProjectTaskSelector } from '../../logic/ProjectTaskSelector'; -import { RushConstants } from '../../logic/RushConstants'; -import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; -import { ProjectLogWritable } from '../../logic/taskExecution/ProjectLogWritable'; - -export class WriteBuildCacheAction extends BaseRushAction { - private _command!: CommandLineStringParameter; - private _verboseFlag!: CommandLineFlagParameter; - - public constructor(parser: RushCommandLineParser) { - super({ - actionName: 'write-build-cache', - summary: 'Writes the current state of the current project to the cache.', - documentation: - '(EXPERIMENTAL) If the build cache is configured, when this command is run in the folder of ' + - 'a project, write the current state of the project to the cache.', - safeForSimultaneousRushProcesses: true, - parser - }); - } - - public onDefineParameters(): void { - this._command = this.defineStringParameter({ - parameterLongName: '--command', - parameterShortName: '-c', - required: true, - argumentName: 'COMMAND', - description: - '(Required) The command run in the current project that produced the current project state.' - }); - - this._verboseFlag = this.defineFlagParameter({ - parameterLongName: '--verbose', - parameterShortName: '-v', - description: 'Display verbose log information.' - }); - } - - public async runAsync(): Promise { - const project: RushConfigurationProject | undefined = this.rushConfiguration.tryGetProjectForPath( - process.cwd() - ); - - if (!project) { - throw new Error( - `The "rush ${this.actionName}" command must be invoked under a project` + - ` folder that is registered in rush.json.` - ); - } - - const terminal: Terminal = new Terminal( - new ConsoleTerminalProvider({ verboseEnabled: this._verboseFlag.value }) - ); - - const buildCacheConfiguration: BuildCacheConfiguration = - await BuildCacheConfiguration.loadAndRequireEnabledAsync( - terminal, - this.rushConfiguration, - this.rushSession - ); - - const command: string = this._command.value!; - const commandToRun: string | undefined = ProjectTaskSelector.getScriptToRun(project, command, []); - - // TODO: With phased commands, this will need to be updated - const taskName: string = ProjectTaskSelector.getPhaseTaskNameForProject(project); - const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(this.rushConfiguration); - const projectBuilder: ProjectTaskRunner = new ProjectTaskRunner({ - rushProject: project, - taskName, - rushConfiguration: this.rushConfiguration, - buildCacheConfiguration, - commandName: command, - commandToRun: commandToRun || '', - isIncrementalBuildAllowed: false, - projectChangeAnalyzer, - logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(command) - }); - - const trackedFiles: string[] = Array.from( - (await projectChangeAnalyzer._tryGetProjectDependenciesAsync(project, terminal))!.keys() - ); - const commandLineConfigFilePath: string = path.join( - this.rushConfiguration.commonRushConfigFolder, - RushConstants.commandLineFilename - ); - const repoCommandLineConfiguration: CommandLineConfiguration | undefined = - CommandLineConfiguration.loadFromFileOrDefault(commandLineConfigFilePath); - - const cacheWriteSuccess: boolean | undefined = await projectBuilder.tryWriteCacheEntryAsync( - terminal, - trackedFiles, - repoCommandLineConfiguration - ); - if (cacheWriteSuccess === undefined) { - terminal.writeErrorLine('This project does not support caching or Git is not present.'); - throw new AlreadyReportedError(); - } else if (cacheWriteSuccess === false) { - terminal.writeErrorLine('Writing cache entry failed.'); - } - } -} diff --git a/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap b/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap index 1fb2d972550..777b7ba729f 100644 --- a/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap +++ b/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap @@ -54,8 +54,6 @@ Positional arguments: (EXPERIMENTAL) Update the credentials used by the build cache provider. version Manage package versions in the repo. - write-build-cache Writes the current state of the current project to - the cache. import-strings Imports translated strings into each project. upload Uploads the built files to the server build Build all projects that haven't been built, or have @@ -1235,18 +1233,3 @@ Optional arguments: what you are skipping. " `; - -exports[`CommandLineHelp prints the help for each action: write-build-cache 1`] = ` -"usage: rush write-build-cache [-h] -c COMMAND [-v] - -(EXPERIMENTAL) If the build cache is configured, when this command is run in -the folder of a project, write the current state of the project to the cache. - -Optional arguments: - -h, --help Show this help message and exit. - -c COMMAND, --command COMMAND - (Required) The command run in the current project - that produced the current project state. - -v, --verbose Display verbose log information. -" -`; diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts index f6d9b841493..c16fa697b9c 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts @@ -125,19 +125,6 @@ export class ProjectTaskRunner extends BaseTaskRunner { } } - public async tryWriteCacheEntryAsync( - terminal: ITerminal, - trackedFilePaths: string[] | undefined, - repoCommandLineConfiguration: CommandLineConfiguration | undefined - ): Promise { - const projectBuildCache: ProjectBuildCache | undefined = await this._getProjectBuildCacheAsync( - terminal, - trackedFilePaths, - repoCommandLineConfiguration - ); - return projectBuildCache?.trySetCacheEntryAsync(terminal); - } - private async _executeTaskAsync(context: ITaskRunnerContext): Promise { // TERMINAL PIPELINE: // @@ -266,7 +253,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { // let buildCacheReadAttempted: boolean = false; if (this._isCacheReadAllowed) { - const projectBuildCache: ProjectBuildCache | undefined = await this._getProjectBuildCacheAsync( + const projectBuildCache: ProjectBuildCache | undefined = await this._tryGetProjectBuildCacheAsync( terminal, trackedFiles, context.repoCommandLineConfiguration @@ -373,11 +360,13 @@ export class ProjectTaskRunner extends BaseTaskRunner { // If the command is successful and we can calculate project hash, we will write a // new cache entry even if incremental execution is not allowed. - const setCacheEntryPromise: Promise = this.tryWriteCacheEntryAsync( + const projectBuildCache: ProjectBuildCache | undefined = await this._tryGetProjectBuildCacheAsync( terminal, trackedFiles, context.repoCommandLineConfiguration ); + const setCacheEntryPromise: Promise | undefined = + projectBuildCache?.trySetCacheEntryAsync(terminal); const [, cacheWriteSuccess] = await Promise.all([writeProjectStatePromise, setCacheEntryPromise]); @@ -402,7 +391,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { } } - private async _getProjectBuildCacheAsync( + private async _tryGetProjectBuildCacheAsync( terminal: ITerminal, trackedProjectFiles: string[] | undefined, commandLineConfiguration: CommandLineConfiguration | undefined From e2518183e61fd9cd0b3004446dce7d6d9f4c5c28 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Fri, 24 Dec 2021 20:04:07 -0800 Subject: [PATCH 05/19] Improve logged task names and fix caching for phased commands. --- .../src/api/CommandLineConfiguration.ts | 31 ++++-- .../src/api/RushProjectConfiguration.ts | 105 +++++++++++------- .../rush-lib/src/cli/RushCommandLineParser.ts | 19 ++-- .../src/cli/scriptActions/BaseScriptAction.ts | 2 +- .../cli/scriptActions/PhasedScriptAction.ts | 2 +- .../CommandLineHelp.test.ts.snap | 2 +- .../src/logic/ProjectChangeAnalyzer.ts | 21 ++-- .../rush-lib/src/logic/ProjectTaskSelector.ts | 103 ++++++++--------- .../src/logic/buildCache/ProjectBuildCache.ts | 23 ++-- .../buildCache/test/ProjectBuildCache.test.ts | 2 +- .../src/logic/taskExecution/BaseTaskRunner.ts | 10 +- .../logic/taskExecution/ProjectTaskRunner.ts | 33 ++++-- .../taskExecution/TaskExecutionManager.ts | 4 +- .../test/TaskExecutionManager.test.ts | 8 +- .../logic/test/ProjectChangeAnalyzer.test.ts | 26 ++--- 15 files changed, 215 insertions(+), 176 deletions(-) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index 5f511c90a24..5a4f4255598 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -7,7 +7,7 @@ import { JsonFile, JsonSchema, FileSystem } from '@rushstack/node-core-library'; import { RushConstants } from '../logic/RushConstants'; -import { +import type { CommandJson, ICommandLineJson, IPhaseJson, @@ -19,6 +19,7 @@ import { IStringParameterJson } from './CommandLineJson'; import { ProjectLogWritable } from '../logic/taskExecution/ProjectLogWritable'; +import type { RushConfigurationProject } from './RushConfigurationProject'; const EXPECTED_PHASE_NAME_PREFIX: '_phase:' = '_phase:'; @@ -28,6 +29,7 @@ export interface IShellCommandTokenContext { export interface IPhase extends IPhaseJson { logFilenameIdentifier: string; + getDisplayNameForProject(rushProject: RushConfigurationProject): string; } export interface ICommandWithParameters { @@ -139,9 +141,12 @@ export class CommandLineConfiguration { ); } + const phaseNameWithoutPrefix: string = phase.name.substring(EXPECTED_PHASE_NAME_PREFIX.length); this.phases.set(phase.name, { ...phase, - logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(phase.name) + logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(phase.name), + getDisplayNameForProject: (rushProject: RushConfigurationProject) => + `${rushProject.packageName} (${phaseNameWithoutPrefix})` }); commandsForPhase.set(phase.name, new Set()); } @@ -186,9 +191,10 @@ export class CommandLineConfiguration { // A map of bulk command names to their corresponding synthetic phase identifiers const syntheticPhasesForTranslatedBulkCommands: Map = new Map(); - const translateBulkCommandToPhasedCommand: (command: IBulkCommandJson) => IPhasedCommand = ( - command: IBulkCommandJson - ) => { + const translateBulkCommandToPhasedCommand: ( + command: IBulkCommandJson, + isBuildCommand: boolean + ) => IPhasedCommand = (command: IBulkCommandJson, isBuildCommand: boolean) => { const phaseName: string = command.name; const phaseForBulkCommand: IPhase = { name: phaseName, @@ -197,7 +203,9 @@ export class CommandLineConfiguration { }, ignoreMissingScript: command.ignoreMissingScript, allowWarningsOnSuccess: command.allowWarningsInSuccessfulBuild, - logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(command.name) + logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(command.name), + // Because this is a synthetic phase, just use the project name because there aren't any other phases + getDisplayNameForProject: (rushProject: RushConfigurationProject) => rushProject.packageName }; this.phases.set(phaseName, phaseForBulkCommand); syntheticPhasesForTranslatedBulkCommands.set(command.name, phaseName); @@ -274,7 +282,7 @@ export class CommandLineConfiguration { case RushConstants.bulkCommandKind: { // Translate the bulk command into a phased command - normalizedCommand = translateBulkCommandToPhasedCommand(command); + normalizedCommand = translateBulkCommandToPhasedCommand(command, /* isBuildCommand */ false); break; } } @@ -307,7 +315,10 @@ export class CommandLineConfiguration { let buildCommand: Command | undefined = this.commands.get(RushConstants.buildCommandName); if (!buildCommand) { // If the build command was not specified in the config file, add the default build command - buildCommand = translateBulkCommandToPhasedCommand(DEFAULT_BUILD_COMMAND_JSON); + buildCommand = translateBulkCommandToPhasedCommand( + DEFAULT_BUILD_COMMAND_JSON, + /* isBuildCommand */ true + ); buildCommand.disableBuildCache = DEFAULT_BUILD_COMMAND_JSON.disableBuildCache; buildCommandPhases = buildCommand.phases; this.commands.set(buildCommand.name, buildCommand); @@ -518,9 +529,9 @@ export class CommandLineConfiguration { * settings. If the file does not exist, then an empty default instance is returned. * If the file contains errors, then an exception is thrown. */ - public static loadFromFileOrDefault(jsonFilename: string): CommandLineConfiguration { + public static loadFromFileOrDefault(jsonFilename?: string): CommandLineConfiguration { let commandLineJson: ICommandLineJson | undefined = undefined; - if (FileSystem.exists(jsonFilename)) { + if (jsonFilename && FileSystem.exists(jsonFilename)) { commandLineJson = JsonFile.load(jsonFilename); // merge commands specified in command-line.json and default (re)build settings diff --git a/apps/rush-lib/src/api/RushProjectConfiguration.ts b/apps/rush-lib/src/api/RushProjectConfiguration.ts index 305bdcde23e..ea192088f1f 100644 --- a/apps/rush-lib/src/api/RushProjectConfiguration.ts +++ b/apps/rush-lib/src/api/RushProjectConfiguration.ts @@ -154,19 +154,15 @@ export class RushProjectConfiguration { public readonly project: RushConfigurationProject; - /** - * A list of folder names under the project root that should be cached. - * - * These folders should not be tracked by git. - */ - public readonly projectOutputFolderNames?: ReadonlyArray; - /** * A list of folder names under the project root that should be cached for each phase. * * These folders should not be tracked by git. + * + * @remarks + * The `projectOutputFolderNames` property is populated in a "build" entry */ - public readonly projectOutputFolderNamesForPhases?: ReadonlyMap>; + public readonly projectOutputFolderNamesForPhases: ReadonlyMap>; /** * The incremental analyzer can skip Rush commands for projects whose input files have @@ -185,12 +181,10 @@ export class RushProjectConfiguration { private constructor( project: RushConfigurationProject, rushProjectJson: IRushProjectJson, - projectOutputFolderNamesForPhases: ReadonlyMap> | undefined + projectOutputFolderNamesForPhases: ReadonlyMap> ) { this.project = project; - this.projectOutputFolderNames = rushProjectJson.projectOutputFolderNames; - this.projectOutputFolderNamesForPhases = projectOutputFolderNamesForPhases; this.incrementalBuildIgnoredGlobs = rushProjectJson.incrementalBuildIgnoredGlobs; @@ -215,28 +209,20 @@ export class RushProjectConfiguration { */ public static async tryLoadForProjectAsync( project: RushConfigurationProject, - repoCommandLineConfiguration: CommandLineConfiguration | undefined, - terminal: ITerminal, - skipCache?: boolean + repoCommandLineConfiguration: CommandLineConfiguration, + terminal: ITerminal ): Promise { // false is a signal that the project config does not exist - const cacheEntry: RushProjectConfiguration | false | undefined = skipCache - ? undefined - : RushProjectConfiguration._configCache.get(project); + const cacheEntry: RushProjectConfiguration | false | undefined = + RushProjectConfiguration._configCache.get(project); if (cacheEntry !== undefined) { return cacheEntry || undefined; } - const rigConfig: RigConfig = await RigConfig.loadForProjectFolderAsync({ - projectFolderPath: project.projectFolder - }); - - const rushProjectJson: IRushProjectJson | undefined = - await this._projectBuildCacheConfigurationFile.tryLoadConfigurationFileForProjectAsync( - terminal, - project.projectFolder, - rigConfig - ); + const rushProjectJson: IRushProjectJson | undefined = await this._tryLoadJsonForProjectAsync( + project, + terminal + ); if (rushProjectJson) { const result: RushProjectConfiguration = RushProjectConfiguration._getRushProjectConfiguration( @@ -253,10 +239,48 @@ export class RushProjectConfiguration { } } + /** + * Load only the `incrementalBuildIgnoredGlobs` property from the rush-project.json file, skipping + * validation of other parts of the config file. + * + * @remarks + * This function exists to allow the ProjectChangeAnalyzer to load just the ignore globs without + * having to validate the rest of the `rush-project.json` file against the repo's command-line configuration. + */ + public static async tryLoadIgnoreGlobsForProjectAsync( + project: RushConfigurationProject, + terminal: ITerminal + ): Promise | undefined> { + const rushProjectJson: IRushProjectJson | undefined = await this._tryLoadJsonForProjectAsync( + project, + terminal + ); + + return rushProjectJson?.incrementalBuildIgnoredGlobs; + } + + private static async _tryLoadJsonForProjectAsync( + project: RushConfigurationProject, + terminal: ITerminal + ): Promise { + const rigConfig: RigConfig = await RigConfig.loadForProjectFolderAsync({ + projectFolderPath: project.projectFolder + }); + + const rushProjectJson: IRushProjectJson | undefined = + await this._projectBuildCacheConfigurationFile.tryLoadConfigurationFileForProjectAsync( + terminal, + project.projectFolder, + rigConfig + ); + + return rushProjectJson; + } + private static _getRushProjectConfiguration( project: RushConfigurationProject, rushProjectJson: IRushProjectJson, - repoCommandLineConfiguration: CommandLineConfiguration | undefined, + repoCommandLineConfiguration: CommandLineConfiguration, terminal: ITerminal ): RushProjectConfiguration { if (rushProjectJson.projectOutputFolderNames) { @@ -294,11 +318,9 @@ export class RushProjectConfiguration { const invalidCommandNames: string[] = []; if (rushProjectJson.buildCacheOptions?.optionsForCommands) { const commandNames: Set = new Set(); - if (repoCommandLineConfiguration) { - for (const [commandName, command] of repoCommandLineConfiguration.commands) { - if (command.commandKind === RushConstants.phasedCommandKind) { - commandNames.add(commandName); - } + for (const [commandName, command] of repoCommandLineConfiguration.commands) { + if (command.commandKind === RushConstants.phasedCommandKind) { + commandNames.add(commandName); } } @@ -331,11 +353,16 @@ export class RushProjectConfiguration { ); } - let projectOutputFolderNamesForPhases: Map | undefined; + const projectOutputFolderNamesForPhases: Map = new Map(); + if (rushProjectJson.projectOutputFolderNames) { + projectOutputFolderNamesForPhases.set( + RushConstants.buildCommandName, + rushProjectJson.projectOutputFolderNames + ); + } + if (rushProjectJson.phaseOptions) { const overlappingPathAnalyzer: OverlappingPathAnalyzer = new OverlappingPathAnalyzer(); - - projectOutputFolderNamesForPhases = new Map(); const phaseOptionsByPhase: Map = new Map< string, IRushProjectJsonPhaseOptionsJson @@ -372,7 +399,7 @@ export class RushProjectConfiguration { } terminal.writeErrorLine(errorMessage); - } else if (!repoCommandLineConfiguration?.phases.has(phaseName)) { + } else if (!repoCommandLineConfiguration.phases.has(phaseName)) { terminal.writeErrorLine( `Invalid "${RushProjectConfiguration._projectBuildCacheConfigurationFile.projectRelativeFilePath}"` + ` for project "${project.packageName}". Phase "${phaseName}" is not defined in the repo's ${RushConstants.commandLineFilename}.` @@ -398,7 +425,7 @@ export class RushProjectConfiguration { terminal.writeErrorLine( `Invalid "${RushProjectConfiguration._projectBuildCacheConfigurationFile.projectRelativeFilePath}" ` + `for project "${project.packageName}". The project output folder name "${projectOutputFolderName}" in ` + - `phase ${phaseName} overlaps with other another folder name in the same phase.` + `phase ${phaseName} overlaps with another folder name in the same phase.` ); } else { const otherPhaseNames: string[] = overlappingPhaseNames.filter( @@ -407,7 +434,7 @@ export class RushProjectConfiguration { terminal.writeErrorLine( `Invalid "${RushProjectConfiguration._projectBuildCacheConfigurationFile.projectRelativeFilePath}" ` + `for project "${project.packageName}". The project output folder name "${projectOutputFolderName}" in ` + - `phase ${phaseName} overlaps with other another folder names in the same phase and with ` + + `phase ${phaseName} overlaps with other folder names in the same phase and with ` + `folder names in the following other phases: ${otherPhaseNames.join(', ')}.` ); } diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts index 348a59b6dc6..018326818ce 100644 --- a/apps/rush-lib/src/cli/RushCommandLineParser.ts +++ b/apps/rush-lib/src/cli/RushCommandLineParser.ts @@ -235,27 +235,22 @@ export class RushCommandLineParser extends CommandLineParser { } private _populateScriptActions(): void { - let commandLineConfiguration: CommandLineConfiguration | undefined = undefined; - // If there is not a rush.json file, we still want "build" and "rebuild" to appear in the // command-line help + let commandLineConfigFilePath: string | undefined; if (this.rushConfiguration) { - const commandLineConfigFilePath: string = path.join( + commandLineConfigFilePath = path.join( this.rushConfiguration.commonRushConfigFolder, RushConstants.commandLineFilename ); - - commandLineConfiguration = CommandLineConfiguration.loadFromFileOrDefault(commandLineConfigFilePath); } + const commandLineConfiguration: CommandLineConfiguration = + CommandLineConfiguration.loadFromFileOrDefault(commandLineConfigFilePath); this._addCommandLineConfigActions(commandLineConfiguration); } - private _addCommandLineConfigActions(commandLineConfiguration?: CommandLineConfiguration): void { - if (!commandLineConfiguration) { - return; - } - + private _addCommandLineConfigActions(commandLineConfiguration: CommandLineConfiguration): void { // Register each custom command for (const command of commandLineConfiguration.commands.values()) { this._addCommandLineConfigAction(commandLineConfiguration, command); @@ -305,7 +300,7 @@ export class RushCommandLineParser extends CommandLineParser { } private _getSharedCommandActionOptions( - commandLineConfiguration: CommandLineConfiguration | undefined, + commandLineConfiguration: CommandLineConfiguration, command: TCommand ): IBaseScriptActionOptions { return { @@ -321,7 +316,7 @@ export class RushCommandLineParser extends CommandLineParser { } private _addGlobalScriptAction( - commandLineConfiguration: CommandLineConfiguration | undefined, + commandLineConfiguration: CommandLineConfiguration, command: IGlobalCommand ): void { if ( diff --git a/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts index 1f716d44503..b1b57406b0b 100644 --- a/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts @@ -11,7 +11,7 @@ import type { ParameterJson } from '../../api/CommandLineJson'; * Constructor parameters for BaseScriptAction */ export interface IBaseScriptActionOptions extends IBaseRushActionOptions { - commandLineConfiguration: CommandLineConfiguration | undefined; + commandLineConfiguration: CommandLineConfiguration; command: TCommand; } diff --git a/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index 3939bd70d96..8f66f23bc81 100644 --- a/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -60,7 +60,7 @@ export class PhasedScriptAction extends BaseScriptAction { private readonly _isIncrementalBuildAllowed: boolean; private readonly _watchForChanges: boolean; private readonly _disableBuildCache: boolean; - private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined; + private readonly _repoCommandLineConfiguration: CommandLineConfiguration; private readonly _actionPhases: string[]; private readonly _phases: Map; diff --git a/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap b/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap index 777b7ba729f..552a1297f07 100644 --- a/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap +++ b/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap @@ -58,7 +58,7 @@ Positional arguments: upload Uploads the built files to the server build Build all projects that haven't been built, or have changed since they were last built. - rebuild Clean and rebuild the entire set of projects + rebuild Clean and rebuild the entire set of projects. tab-complete Provides tab completion. Optional arguments: diff --git a/apps/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/apps/rush-lib/src/logic/ProjectChangeAnalyzer.ts index 6f76365bc70..08e623a7015 100644 --- a/apps/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/apps/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -22,6 +22,7 @@ import { RushConfigurationProject } from '../api/RushConfigurationProject'; import { RushConstants } from './RushConstants'; import { LookupByPath } from './LookupByPath'; import { PnpmShrinkwrapFile } from './pnpm/PnpmShrinkwrapFile'; +import { UNINITIALIZED } from '../utilities/Utilities'; /** * @beta @@ -63,10 +64,10 @@ export class ProjectChangeAnalyzer { * UNINITIALIZED === we haven't looked * undefined === data isn't available (i.e. - git isn't present) */ - private _data: IRawRepoState | undefined = undefined; - private _filteredData: Map> = new Map(); - private _projectStateCache: Map = new Map(); - private _rushConfiguration: RushConfiguration; + private _data: IRawRepoState | UNINITIALIZED | undefined = UNINITIALIZED; + private readonly _filteredData: Map> = new Map(); + private readonly _projectStateCache: Map = new Map(); + private readonly _rushConfiguration: RushConfiguration; private readonly _git: Git; public constructor(rushConfiguration: RushConfiguration) { @@ -92,10 +93,14 @@ export class ProjectChangeAnalyzer { return filteredProjectData; } - if (this._data === undefined) { + if (this._data === UNINITIALIZED) { this._data = this._getData(terminal); } + if (!this._data) { + return undefined; + } + const { projectState, rootDir } = this._data; if (projectState === undefined) { @@ -410,10 +415,8 @@ export class ProjectChangeAnalyzer { project: RushConfigurationProject, terminal: ITerminal ): Promise { - const projectConfiguration: RushProjectConfiguration | undefined = - await RushProjectConfiguration.tryLoadForProjectAsync(project, undefined, terminal); - - const { incrementalBuildIgnoredGlobs } = projectConfiguration || {}; + const incrementalBuildIgnoredGlobs: ReadonlyArray | undefined = + await RushProjectConfiguration.tryLoadIgnoreGlobsForProjectAsync(project, terminal); if (incrementalBuildIgnoredGlobs && incrementalBuildIgnoredGlobs.length) { const ignoreMatcher: Ignore = ignore(); diff --git a/apps/rush-lib/src/logic/ProjectTaskSelector.ts b/apps/rush-lib/src/logic/ProjectTaskSelector.ts index 64bb6804868..37070380076 100644 --- a/apps/rush-lib/src/logic/ProjectTaskSelector.ts +++ b/apps/rush-lib/src/logic/ProjectTaskSelector.ts @@ -8,7 +8,6 @@ import { convertSlashesForWindows, ProjectTaskRunner } from './taskExecution/Pro import { ProjectChangeAnalyzer } from './ProjectChangeAnalyzer'; import { TaskCollection } from './taskExecution/TaskCollection'; import { IPhase } from '../api/CommandLineConfiguration'; -import { EnvironmentConfiguration } from '..'; export interface IProjectTaskSelectorOptions { rushConfiguration: RushConfiguration; @@ -35,7 +34,6 @@ export class ProjectTaskSelector { private readonly _projectChangeAnalyzer: ProjectChangeAnalyzer; private readonly _phasesToRun: ReadonlyArray; private readonly _phases: Map; - private readonly _overrideAllowWarnings: boolean = EnvironmentConfiguration.allowWarningsInSuccessfulBuild; public constructor(options: IProjectTaskSelectorOptions) { this._options = options; @@ -70,55 +68,12 @@ export class ProjectTaskSelector { // Register all tasks for (const phaseName of this._phasesToRun) { - const phase: IPhase | undefined = this._phases.get(phaseName); - if (!phase) { - throw new Error(`Phase ${phaseName} not found`); - } - + const phase: IPhase = this._getPhaseByName(phaseName); for (const rushProject of projects) { this._registerProjectPhaseTask(rushProject, phase, taskCollection); } } - // TODO: Expand phase dependencies - - // if (!this._options.ignoreDependencyOrder) { - // const dependencyMap: Map> = new Map(); - - // // Generate the filtered dependency graph for selected projects - // function getDependencyTaskNames(project: RushConfigurationProject): Set { - // const cached: Set | undefined = dependencyMap.get(project); - // if (cached) { - // return cached; - // } - - // const dependencyTaskNames: Set = new Set(); - // dependencyMap.set(project, dependencyTaskNames); - - // for (const dep of project.dependencyProjects) { - // if (projects.has(dep)) { - // // Add direct relationships for projects in the set - // dependencyTaskNames.add(ProjectTaskSelector.getPhaseTaskNameForProject(dep)); - // } else { - // // Add indirect relationships for projects not in the set - // for (const indirectDep of getDependencyTaskNames(dep)) { - // dependencyTaskNames.add(indirectDep); - // } - // } - // } - - // return dependencyTaskNames; - // } - - // // Add ordering relationships for each dependency - // for (const project of projects) { - // taskCollection.addDependencies( - // ProjectTaskSelector.getPhaseTaskNameForProject(project), - // getDependencyTaskNames(project) - // ); - // } - // } - return taskCollection; } @@ -126,10 +81,10 @@ export class ProjectTaskSelector { project: RushConfigurationProject, phase: IPhase, taskCollection: TaskCollection - ): void { - const taskName: string = ProjectTaskSelector.getPhaseTaskNameForProject(project, phase); + ): string { + const taskName: string = phase.getDisplayNameForProject(project); if (taskCollection.hasTask(taskName)) { - return; + return taskName; } const commandToRun: string | undefined = ProjectTaskSelector.getScriptToRun( @@ -150,21 +105,53 @@ export class ProjectTaskSelector { rushConfiguration: this._options.rushConfiguration, buildCacheConfiguration: this._options.buildCacheConfiguration, commandToRun: commandToRun || '', - commandName: phase.name, isIncrementalBuildAllowed: this._options.isIncrementalBuildAllowed, projectChangeAnalyzer: this._projectChangeAnalyzer, - allowWarningsInSuccessfulBuild: this._overrideAllowWarnings || phase.allowWarningsOnSuccess, - logFilenameIdentifier: phase.logFilenameIdentifier + phase }) ); + + const dependencyTasks: Set = new Set(); + if (phase.dependencies?.self) { + for (const dependencyPhaseName of phase.dependencies.self) { + const dependencyPhase: IPhase = this._getPhaseByName(dependencyPhaseName); + const dependencyTaskName: string = this._registerProjectPhaseTask( + project, + dependencyPhase, + taskCollection + ); + + dependencyTasks.add(dependencyTaskName); + } + } + + if (phase.dependencies?.upstream) { + for (const dependencyPhaseName of phase.dependencies.upstream) { + const dependencyPhase: IPhase = this._getPhaseByName(dependencyPhaseName); + for (const dependencyProject of project.dependencyProjects) { + const dependencyTaskName: string = this._registerProjectPhaseTask( + dependencyProject, + dependencyPhase, + taskCollection + ); + + dependencyTasks.add(dependencyTaskName); + } + } + } + + taskCollection.addDependencies(taskName, dependencyTasks); + + return taskName; } - /** - * A helper method to determine the task name of a ProjectBuilder. Used when the task - * name is required before a task is created. - */ - public static getPhaseTaskNameForProject(rushProject: RushConfigurationProject, phase: IPhase): string { - return `${rushProject.packageName} (${phase.name})`; + private _getPhaseByName(phaseName: string): IPhase { + const phase: IPhase | undefined = this._phases.get(phaseName); + if (!phase) { + throw new Error(`Phase ${phaseName} not found`); + } + + return phase; } private static _getScriptCommand( diff --git a/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts b/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts index 45503d32140..b9fa6dced7b 100644 --- a/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts +++ b/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts @@ -22,6 +22,7 @@ import { Utilities } from '../../utilities/Utilities'; export interface IProjectBuildCacheOptions { buildCacheConfiguration: BuildCacheConfiguration; projectConfiguration: RushProjectConfiguration; + projectOutputFolderNames: ReadonlyArray; command: string; trackedProjectFiles: string[] | undefined; projectChangeAnalyzer: ProjectChangeAnalyzer; @@ -52,7 +53,7 @@ export class ProjectBuildCache { this._localBuildCacheProvider = options.buildCacheConfiguration.localCacheProvider; this._cloudBuildCacheProvider = options.buildCacheConfiguration.cloudCacheProvider; this._buildCacheEnabled = options.buildCacheConfiguration.buildCacheEnabled; - this._projectOutputFolderNames = options.projectConfiguration.projectOutputFolderNames || []; + this._projectOutputFolderNames = options.projectOutputFolderNames || []; this._cacheId = cacheId; } @@ -67,12 +68,19 @@ export class ProjectBuildCache { public static async tryGetProjectBuildCache( options: IProjectBuildCacheOptions ): Promise { - const { terminal, projectConfiguration, trackedProjectFiles } = options; + const { terminal, projectConfiguration, projectOutputFolderNames, trackedProjectFiles } = options; if (!trackedProjectFiles) { return undefined; } - if (!ProjectBuildCache._validateProject(terminal, projectConfiguration, trackedProjectFiles)) { + if ( + !ProjectBuildCache._validateProject( + terminal, + projectConfiguration, + projectOutputFolderNames, + trackedProjectFiles + ) + ) { return undefined; } @@ -83,14 +91,15 @@ export class ProjectBuildCache { private static _validateProject( terminal: ITerminal, projectConfiguration: RushProjectConfiguration, + projectOutputFolderNames: ReadonlyArray, trackedProjectFiles: string[] ): boolean { const normalizedProjectRelativeFolder: string = Path.convertToSlashes( projectConfiguration.project.projectRelativeFolder ); const outputFolders: string[] = []; - if (projectConfiguration.projectOutputFolderNames) { - for (const outputFolderName of projectConfiguration.projectOutputFolderNames) { + if (projectOutputFolderNames) { + for (const outputFolderName of projectOutputFolderNames) { outputFolders.push(`${normalizedProjectRelativeFolder}/${outputFolderName}/`); } } @@ -470,9 +479,7 @@ export class ProjectBuildCache { const sortedProjectStates: string[] = projectStates.sort(); const hash: crypto.Hash = crypto.createHash('sha1'); - const serializedOutputFolders: string = JSON.stringify( - options.projectConfiguration.projectOutputFolderNames - ); + const serializedOutputFolders: string = JSON.stringify(options.projectOutputFolderNames); hash.update(serializedOutputFolders); hash.update(RushConstants.hashDelimiter); hash.update(options.command); diff --git a/apps/rush-lib/src/logic/buildCache/test/ProjectBuildCache.test.ts b/apps/rush-lib/src/logic/buildCache/test/ProjectBuildCache.test.ts index 28897961b0b..786c838eb39 100644 --- a/apps/rush-lib/src/logic/buildCache/test/ProjectBuildCache.test.ts +++ b/apps/rush-lib/src/logic/buildCache/test/ProjectBuildCache.test.ts @@ -35,8 +35,8 @@ describe('ProjectBuildCache', () => { isCacheWriteAllowed: options.hasOwnProperty('writeAllowed') ? options.writeAllowed : false } } as unknown as BuildCacheConfiguration, + projectOutputFolderNames: ['dist'], projectConfiguration: { - projectOutputFolderNames: ['dist'], project: { packageName: 'acme-wizard', projectRelativeFolder: 'apps/acme-wizard', diff --git a/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts index 4f52a52b00c..e89ec822230 100644 --- a/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts @@ -1,14 +1,14 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { StdioSummarizer } from '@rushstack/terminal'; -import { CollatedWriter } from '@rushstack/stream-collator'; +import type { StdioSummarizer } from '@rushstack/terminal'; +import type { CollatedWriter } from '@rushstack/stream-collator'; -import { TaskStatus } from './TaskStatus'; -import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; +import type { TaskStatus } from './TaskStatus'; +import type { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; export interface ITaskRunnerContext { - repoCommandLineConfiguration: CommandLineConfiguration | undefined; + repoCommandLineConfiguration: CommandLineConfiguration; collatedWriter: CollatedWriter; stdioSummarizer: StdioSummarizer; quietMode: boolean; diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts index c16fa697b9c..eef50d26984 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts @@ -24,20 +24,21 @@ import { } from '@rushstack/terminal'; import { CollatedTerminal } from '@rushstack/stream-collator'; -import { RushConfiguration } from '../../api/RushConfiguration'; -import { RushConfigurationProject } from '../../api/RushConfigurationProject'; +import type { RushConfiguration } from '../../api/RushConfiguration'; +import type { RushConfigurationProject } from '../../api/RushConfigurationProject'; import { Utilities, UNINITIALIZED } from '../../utilities/Utilities'; import { TaskStatus } from './TaskStatus'; import { TaskError } from './TaskError'; -import { ProjectChangeAnalyzer } from '../ProjectChangeAnalyzer'; +import type { ProjectChangeAnalyzer } from '../ProjectChangeAnalyzer'; import { BaseTaskRunner, ITaskRunnerContext } from './BaseTaskRunner'; import { ProjectLogWritable } from './ProjectLogWritable'; import { ProjectBuildCache } from '../buildCache/ProjectBuildCache'; -import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; +import type { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; import { ICacheOptionsForCommand, RushProjectConfiguration } from '../../api/RushProjectConfiguration'; import { CollatedTerminalProvider } from '../../utilities/CollatedTerminalProvider'; -import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; +import type { CommandLineConfiguration, IPhase } from '../../api/CommandLineConfiguration'; import { RushConstants } from '../RushConstants'; +import { EnvironmentConfiguration } from '../../api/EnvironmentConfiguration'; export interface IProjectDeps { files: { [filePath: string]: string }; @@ -49,12 +50,11 @@ export interface IProjectTaskRunnerOptions { rushConfiguration: RushConfiguration; buildCacheConfiguration: BuildCacheConfiguration | undefined; commandToRun: string; - commandName: string; isIncrementalBuildAllowed: boolean; projectChangeAnalyzer: ProjectChangeAnalyzer; allowWarningsInSuccessfulBuild?: boolean; taskName: string; - logFilenameIdentifier: string; + phase: IPhase; } function _areShallowEqual(object1: JsonObject, object2: JsonObject): boolean { @@ -83,6 +83,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { public readonly warningsAreAllowed: boolean; private readonly _rushProject: RushConfigurationProject; + private readonly _phase: IPhase; private readonly _rushConfiguration: RushConfiguration; private readonly _buildCacheConfiguration: BuildCacheConfiguration | undefined; private readonly _commandName: string; @@ -100,18 +101,23 @@ export class ProjectTaskRunner extends BaseTaskRunner { public constructor(options: IProjectTaskRunnerOptions) { super(); + const phase: IPhase = options.phase; this.name = options.taskName; this._rushProject = options.rushProject; + this._phase = phase; this._rushConfiguration = options.rushConfiguration; this._buildCacheConfiguration = options.buildCacheConfiguration; - this._commandName = options.commandName; + this._commandName = phase.name; this._commandToRun = options.commandToRun; this._isCacheReadAllowed = options.isIncrementalBuildAllowed; this.isSkipAllowed = options.isIncrementalBuildAllowed; this._projectChangeAnalyzer = options.projectChangeAnalyzer; - this._packageDepsFilename = `package-deps_${this._commandToRun}.json`; - this.warningsAreAllowed = options.allowWarningsInSuccessfulBuild || false; - this._logFilenameIdentifier = options.logFilenameIdentifier; + this._packageDepsFilename = `package-deps_${phase.logFilenameIdentifier}.json`; + this.warningsAreAllowed = + EnvironmentConfiguration.allowWarningsInSuccessfulBuild || + options.allowWarningsInSuccessfulBuild || + false; + this._logFilenameIdentifier = phase.logFilenameIdentifier; } public async executeAsync(context: ITaskRunnerContext): Promise { @@ -394,7 +400,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { private async _tryGetProjectBuildCacheAsync( terminal: ITerminal, trackedProjectFiles: string[] | undefined, - commandLineConfiguration: CommandLineConfiguration | undefined + commandLineConfiguration: CommandLineConfiguration ): Promise { if (this._projectBuildCache === UNINITIALIZED) { this._projectBuildCache = undefined; @@ -417,8 +423,11 @@ export class ProjectTaskRunner extends BaseTaskRunner { `Caching has been disabled for this project's "${this._commandName}" command.` ); } else { + const projectOutputFolderNames: ReadonlyArray = + projectConfiguration.projectOutputFolderNamesForPhases.get(this._phase.name) || []; this._projectBuildCache = await ProjectBuildCache.tryGetProjectBuildCache({ projectConfiguration, + projectOutputFolderNames, buildCacheConfiguration: this._buildCacheConfiguration, terminal, command: this._commandToRun, diff --git a/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts b/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts index 083a4676641..0c7cf39977d 100644 --- a/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts +++ b/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts @@ -25,7 +25,7 @@ export interface ITaskExecutionManagerOptions { debugMode: boolean; parallelism: string | undefined; changedProjectsOnly: boolean; - repoCommandLineConfiguration: CommandLineConfiguration | undefined; + repoCommandLineConfiguration: CommandLineConfiguration; destination?: TerminalWritable; } @@ -47,7 +47,7 @@ export class TaskExecutionManager { private readonly _quietMode: boolean; private readonly _debugMode: boolean; private readonly _parallelism: number; - private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined; + private readonly _repoCommandLineConfiguration: CommandLineConfiguration; private _hasAnyFailures: boolean; private _hasAnyNonAllowedWarnings: boolean; private _currentActiveTasks!: number; diff --git a/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts b/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts index 0b026d72857..f4d26734898 100644 --- a/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts +++ b/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts @@ -70,7 +70,7 @@ describe(TaskExecutionManager.name, () => { parallelism: 'tequila', changedProjectsOnly: false, destination: mockWritable, - repoCommandLineConfiguration: undefined + repoCommandLineConfiguration: undefined! }) ).toThrowErrorMatchingSnapshot(); }); @@ -84,7 +84,7 @@ describe(TaskExecutionManager.name, () => { parallelism: '1', changedProjectsOnly: false, destination: mockWritable, - repoCommandLineConfiguration: undefined + repoCommandLineConfiguration: undefined! }; }); @@ -141,7 +141,7 @@ describe(TaskExecutionManager.name, () => { parallelism: '1', changedProjectsOnly: false, destination: mockWritable, - repoCommandLineConfiguration: undefined + repoCommandLineConfiguration: undefined! }; }); @@ -176,7 +176,7 @@ describe(TaskExecutionManager.name, () => { parallelism: '1', changedProjectsOnly: false, destination: mockWritable, - repoCommandLineConfiguration: undefined + repoCommandLineConfiguration: undefined! }; }); diff --git a/apps/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/apps/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index f22cd454699..47a5818cd52 100644 --- a/apps/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/apps/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -9,11 +9,12 @@ import { EnvironmentConfiguration } from '../../api/EnvironmentConfiguration'; import { RushConfigurationProject } from '../../api/RushConfigurationProject'; import { RushProjectConfiguration } from '../../api/RushProjectConfiguration'; import { LookupByPath } from '../LookupByPath'; +import { UNINITIALIZED } from '../../utilities/Utilities'; describe(ProjectChangeAnalyzer.name, () => { beforeEach(() => { jest.spyOn(EnvironmentConfiguration, 'gitBinaryPath', 'get').mockReturnValue(undefined); - jest.spyOn(RushProjectConfiguration, 'tryLoadForProjectAsync').mockResolvedValue(undefined); + jest.spyOn(RushProjectConfiguration, 'tryLoadIgnoreGlobsForProjectAsync').mockResolvedValue(undefined); }); afterEach(() => { @@ -87,11 +88,13 @@ describe(ProjectChangeAnalyzer.name, () => { it('ignores files specified by project configuration files, relative to project folder', async () => { // rush-project.json configuration for 'apple' - jest.spyOn(RushProjectConfiguration, 'tryLoadForProjectAsync').mockResolvedValueOnce({ - incrementalBuildIgnoredGlobs: ['assets/*.png', '*.js.map'] as ReadonlyArray - } as RushProjectConfiguration); + jest + .spyOn(RushProjectConfiguration, 'tryLoadIgnoreGlobsForProjectAsync') + .mockResolvedValueOnce(['assets/*.png', '*.js.map']); // rush-project.json configuration for 'banana' does not exist - jest.spyOn(RushProjectConfiguration, 'tryLoadForProjectAsync').mockResolvedValueOnce(undefined); + jest + .spyOn(RushProjectConfiguration, 'tryLoadIgnoreGlobsForProjectAsync') + .mockResolvedValueOnce(undefined); const projects: RushConfigurationProject[] = [ { @@ -132,13 +135,9 @@ describe(ProjectChangeAnalyzer.name, () => { it('interprets ignored globs as a dot-ignore file (not as individually handled globs)', async () => { // rush-project.json configuration for 'apple' - jest.spyOn(RushProjectConfiguration, 'tryLoadForProjectAsync').mockResolvedValue({ - incrementalBuildIgnoredGlobs: [ - '*.png', - 'assets/*.psd', - '!assets/important/**' - ] as ReadonlyArray - } as RushProjectConfiguration); + jest + .spyOn(RushProjectConfiguration, 'tryLoadIgnoreGlobsForProjectAsync') + .mockResolvedValue(['*.png', 'assets/*.psd', '!assets/important/**']); const projects: RushConfigurationProject[] = [ { @@ -247,11 +246,12 @@ describe(ProjectChangeAnalyzer.name, () => { // ProjectChangeAnalyzer is inert until someone actually requests project data, // this test makes that expectation explicit. - expect(subject['_data']).toEqual(undefined); + expect(subject['_data']).toEqual(UNINITIALIZED); expect(await subject._tryGetProjectDependenciesAsync(projects[0], terminal)).toEqual( new Map([['apps/apple/core.js', 'a101']]) ); expect(subject['_data']).toBeDefined(); + expect(subject['_data']).not.toEqual(UNINITIALIZED); expect(await subject._tryGetProjectDependenciesAsync(projects[0], terminal)).toEqual( new Map([['apps/apple/core.js', 'a101']]) ); From 8075eae74effb3c496b8e2ad5bd33fa805661867 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Fri, 24 Dec 2021 20:44:43 -0800 Subject: [PATCH 06/19] Add support for sticking the phase name in the build cache entry name. --- .../common/config/rush/build-cache.json | 4 +- .../src/api/CommandLineConfiguration.ts | 21 +++--- .../src/logic/buildCache/CacheEntryId.ts | 27 +++++++ .../src/logic/buildCache/ProjectBuildCache.ts | 4 +- .../buildCache/test/CacheEntryId.test.ts | 37 ++++++++-- .../buildCache/test/ProjectBuildCache.test.ts | 3 +- .../__snapshots__/CacheEntryId.test.ts.snap | 70 +++++++++++++++---- .../logic/taskExecution/ProjectLogWritable.ts | 4 -- .../logic/taskExecution/ProjectTaskRunner.ts | 3 +- 9 files changed, 133 insertions(+), 40 deletions(-) diff --git a/apps/rush-lib/assets/rush-init/common/config/rush/build-cache.json b/apps/rush-lib/assets/rush-init/common/config/rush/build-cache.json index 04cdd9cc7a9..2a967fae966 100644 --- a/apps/rush-lib/assets/rush-init/common/config/rush/build-cache.json +++ b/apps/rush-lib/assets/rush-init/common/config/rush/build-cache.json @@ -2,7 +2,7 @@ * This configuration file manages Rush's build cache feature. * More documentation is available on the Rush website: https://rushjs.io */ - { +{ "$schema": "https://developer.microsoft.com/json-schemas/rush/v5/build-cache.schema.json", /** @@ -23,7 +23,7 @@ * Setting this property overrides the cache entry ID. If this property is set, it must contain * a [hash] token. It may also contain a [projectName] or a [projectName:normalize] token. */ - // "cacheEntryNamePattern": "[projectName:normalize]-[hash]" + // "cacheEntryNamePattern": "[projectName:normalize]-[phaseName:normalize]-[hash]" /** * Use this configuration with "cacheProvider"="azure-blob-storage" diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index 5a4f4255598..7517ed548bd 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -18,7 +18,6 @@ import type { IChoiceParameterJson, IStringParameterJson } from './CommandLineJson'; -import { ProjectLogWritable } from '../logic/taskExecution/ProjectLogWritable'; import type { RushConfigurationProject } from './RushConfigurationProject'; const EXPECTED_PHASE_NAME_PREFIX: '_phase:' = '_phase:'; @@ -101,14 +100,14 @@ export class CommandLineConfiguration { public readonly parameters: Parameter[] = []; /** - * These path will be prepended to the PATH environment variable + * shellCommand from plugin custom command line configuration needs to be expanded with tokens */ - private _additionalPathFolders: string[] = []; + public shellCommandTokenContext: IShellCommandTokenContext | undefined; /** - * shellCommand from plugin custom command line configuration needs to be expanded with tokens + * These path will be prepended to the PATH environment variable */ - private _shellCommandTokenContext: IShellCommandTokenContext | undefined; + private _additionalPathFolders: string[] = []; /** * Use CommandLineConfiguration.loadFromFile() @@ -144,7 +143,7 @@ export class CommandLineConfiguration { const phaseNameWithoutPrefix: string = phase.name.substring(EXPECTED_PHASE_NAME_PREFIX.length); this.phases.set(phase.name, { ...phase, - logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(phase.name), + logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(phase.name), getDisplayNameForProject: (rushProject: RushConfigurationProject) => `${rushProject.packageName} (${phaseNameWithoutPrefix})` }); @@ -203,7 +202,7 @@ export class CommandLineConfiguration { }, ignoreMissingScript: command.ignoreMissingScript, allowWarningsOnSuccess: command.allowWarningsInSuccessfulBuild, - logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(command.name), + logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(command.name), // Because this is a synthetic phase, just use the project name because there aren't any other phases getDisplayNameForProject: (rushProject: RushConfigurationProject) => rushProject.packageName }; @@ -581,11 +580,7 @@ export class CommandLineConfiguration { this._additionalPathFolders.unshift(pathFolder); } - public get shellCommandTokenContext(): Readonly | undefined { - return this._shellCommandTokenContext; - } - - public set shellCommandTokenContext(tokenContext: IShellCommandTokenContext | undefined) { - this._shellCommandTokenContext = tokenContext; + private _normalizeNameForLogFilenameIdentifiers(name: string): string { + return name.replace(/:/g, '_'); // Replace colons with underscores to be filesystem-safe } } diff --git a/apps/rush-lib/src/logic/buildCache/CacheEntryId.ts b/apps/rush-lib/src/logic/buildCache/CacheEntryId.ts index 04b994cf9a9..130ee095b77 100644 --- a/apps/rush-lib/src/logic/buildCache/CacheEntryId.ts +++ b/apps/rush-lib/src/logic/buildCache/CacheEntryId.ts @@ -5,6 +5,7 @@ const OPTIONS_ARGUMENT_NAME: string = 'options'; export interface IGenerateCacheEntryIdOptions { projectName: string; + phaseName: string; projectStateHash: string; } @@ -12,6 +13,7 @@ export type GetCacheEntryIdFunction = (options: IGenerateCacheEntryIdOptions) => const HASH_TOKEN_NAME: string = 'hash'; const PROJECT_NAME_TOKEN_NAME: string = 'projectName'; +const PHASE_NAME_TOKEN_NAME: string = 'phaseName'; // This regex matches substrings that look like [token] const TOKEN_REGEX: RegExp = /\[[^\]]*\]/g; @@ -84,6 +86,31 @@ export class CacheEntryId { } } + case PHASE_NAME_TOKEN_NAME: { + switch (tokenAttribute) { + case undefined: { + throw new Error( + 'Either the "normalize" or the "trimPrefix" attribute is required ' + + `for the "${tokenName}" token.` + ); + } + + case 'normalize': { + // Replace colons with underscores. + return `\${${OPTIONS_ARGUMENT_NAME}.phaseName.replace(/:/g, '_')}`; + } + + case 'trimPrefix': { + // Trim the "_phase:" prefix from the phase name. + return `\${${OPTIONS_ARGUMENT_NAME}.phaseName.replace(/^_phase:/, '')}`; + } + + default: { + throw new Error(`Unexpected attribute "${tokenAttribute}" for the "${tokenName}" token.`); + } + } + } + default: { throw new Error(`Unexpected token name "${tokenName}".`); } diff --git a/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts b/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts index b9fa6dced7b..d132be7a5b4 100644 --- a/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts +++ b/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts @@ -27,6 +27,7 @@ export interface IProjectBuildCacheOptions { trackedProjectFiles: string[] | undefined; projectChangeAnalyzer: ProjectChangeAnalyzer; terminal: ITerminal; + phaseName: string; } interface IPathsToCache { @@ -493,7 +494,8 @@ export class ProjectBuildCache { return options.buildCacheConfiguration.getCacheEntryId({ projectName: options.projectConfiguration.project.packageName, - projectStateHash + projectStateHash, + phaseName: options.phaseName }); } } diff --git a/apps/rush-lib/src/logic/buildCache/test/CacheEntryId.test.ts b/apps/rush-lib/src/logic/buildCache/test/CacheEntryId.test.ts index 05ebd5c7044..c05247f5c56 100644 --- a/apps/rush-lib/src/logic/buildCache/test/CacheEntryId.test.ts +++ b/apps/rush-lib/src/logic/buildCache/test/CacheEntryId.test.ts @@ -1,36 +1,62 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { CacheEntryId, GetCacheEntryIdFunction } from '../CacheEntryId'; +import { CacheEntryId, GetCacheEntryIdFunction, IGenerateCacheEntryIdOptions } from '../CacheEntryId'; describe(CacheEntryId.name, () => { describe('Valid pattern names', () => { - function validatePatternMatchesSnapshot(projectName: string, pattern?: string): void { + function validatePatternMatchesSnapshot( + projectName: string, + pattern?: string, + generateCacheEntryIdOptions?: Partial + ): void { const getCacheEntryId: GetCacheEntryIdFunction = CacheEntryId.parsePattern(pattern); expect( getCacheEntryId({ projectName, - projectStateHash: '09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3' + projectStateHash: '09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3', + phaseName: '_phase:compile', + ...generateCacheEntryIdOptions }) - ).toMatchSnapshot(); + ).toMatchSnapshot(pattern || 'no pattern'); } + // prettier-ignore it('Handles a cache entry name for a project name without a scope', () => { const projectName: string = 'project+name'; validatePatternMatchesSnapshot(projectName); validatePatternMatchesSnapshot(projectName, '[hash]'); validatePatternMatchesSnapshot(projectName, '[projectName]_[hash]'); + validatePatternMatchesSnapshot(projectName, '[phaseName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, '[phaseName:trimPrefix]_[hash]'); validatePatternMatchesSnapshot(projectName, '[projectName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, '[projectName:normalize]_[phaseName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, '[projectName:normalize]_[phaseName:trimPrefix]_[hash]'); validatePatternMatchesSnapshot(projectName, 'prefix/[projectName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName:normalize]_[phaseName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName:normalize]_[phaseName:trimPrefix]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName]_[phaseName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName]_[phaseName:trimPrefix]_[hash]'); }); + // prettier-ignore it('Handles a cache entry name for a project name with a scope', () => { const projectName: string = '@scope/project+name'; validatePatternMatchesSnapshot(projectName); validatePatternMatchesSnapshot(projectName, '[hash]'); validatePatternMatchesSnapshot(projectName, '[projectName]_[hash]'); + validatePatternMatchesSnapshot(projectName, '[phaseName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, '[phaseName:trimPrefix]_[hash]'); validatePatternMatchesSnapshot(projectName, '[projectName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, '[projectName:normalize]_[phaseName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, '[projectName:normalize]_[phaseName:trimPrefix]_[hash]'); validatePatternMatchesSnapshot(projectName, 'prefix/[projectName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName:normalize]_[phaseName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName:normalize]_[phaseName:trimPrefix]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName]_[phaseName:normalize]_[hash]'); + validatePatternMatchesSnapshot(projectName, 'prefix/[projectName]_[phaseName:trimPrefix]_[hash]'); }); }); @@ -48,6 +74,9 @@ describe(CacheEntryId.name, () => { await validateInvalidPatternErrorMatchesSnapshotAsync('[hash:badAttribute:attr2]'); await validateInvalidPatternErrorMatchesSnapshotAsync('[projectName:badAttribute]'); await validateInvalidPatternErrorMatchesSnapshotAsync('[projectName:]'); + await validateInvalidPatternErrorMatchesSnapshotAsync('[phaseName]'); + await validateInvalidPatternErrorMatchesSnapshotAsync('[phaseName:]'); + await validateInvalidPatternErrorMatchesSnapshotAsync('[phaseName:badAttribute]'); await validateInvalidPatternErrorMatchesSnapshotAsync('[:attr1]'); await validateInvalidPatternErrorMatchesSnapshotAsync('[projectName:attr1:attr2]'); await validateInvalidPatternErrorMatchesSnapshotAsync('/[hash]'); diff --git a/apps/rush-lib/src/logic/buildCache/test/ProjectBuildCache.test.ts b/apps/rush-lib/src/logic/buildCache/test/ProjectBuildCache.test.ts index 786c838eb39..c1d79d46507 100644 --- a/apps/rush-lib/src/logic/buildCache/test/ProjectBuildCache.test.ts +++ b/apps/rush-lib/src/logic/buildCache/test/ProjectBuildCache.test.ts @@ -46,7 +46,8 @@ describe('ProjectBuildCache', () => { command: 'build', trackedProjectFiles: options.hasOwnProperty('trackedProjectFiles') ? options.trackedProjectFiles : [], projectChangeAnalyzer, - terminal + terminal, + phaseName: 'build' }); return subject; diff --git a/apps/rush-lib/src/logic/buildCache/test/__snapshots__/CacheEntryId.test.ts.snap b/apps/rush-lib/src/logic/buildCache/test/__snapshots__/CacheEntryId.test.ts.snap index bd4c527b1eb..899f088afe0 100644 --- a/apps/rush-lib/src/logic/buildCache/test/__snapshots__/CacheEntryId.test.ts.snap +++ b/apps/rush-lib/src/logic/buildCache/test/__snapshots__/CacheEntryId.test.ts.snap @@ -16,30 +16,72 @@ exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid p exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 8`] = `"Unexpected attribute \\"\\" for the \\"projectName\\" token."`; -exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 9`] = `"Unexpected token name \\"\\"."`; +exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 9`] = `"Either the \\"normalize\\" or the \\"trimPrefix\\" attribute is required for the \\"phaseName\\" token."`; -exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 10`] = `"Unexpected attribute \\"attr1:attr2\\" for the \\"projectName\\" token."`; +exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 10`] = `"Unexpected attribute \\"\\" for the \\"phaseName\\" token."`; -exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 11`] = `"Cache entry name patterns may not start with a slash."`; +exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 11`] = `"Unexpected attribute \\"badAttribute\\" for the \\"phaseName\\" token."`; -exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 12`] = `"Cache entry name pattern contains an invalid character. Only alphanumeric characters, slashes, underscores, and hyphens are allowed."`; +exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 12`] = `"Unexpected token name \\"\\"."`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope 1`] = `"09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 13`] = `"Unexpected attribute \\"attr1:attr2\\" for the \\"projectName\\" token."`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope 2`] = `"09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 14`] = `"Cache entry name patterns may not start with a slash."`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope 3`] = `"@scope/project+name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Invalid pattern names Throws an exception for an invalid pattern 15`] = `"Cache entry name pattern contains an invalid character. Only alphanumeric characters, slashes, underscores, and hyphens are allowed."`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope 4`] = `"@scope+project++name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: [hash] 1`] = `"09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope 5`] = `"prefix/@scope+project++name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: [phaseName:normalize]_[hash] 1`] = `"_phase_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope 1`] = `"09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: [phaseName:trimPrefix]_[hash] 1`] = `"compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope 2`] = `"09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: [projectName:normalize]_[hash] 1`] = `"@scope+project++name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope 3`] = `"project+name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: [projectName:normalize]_[phaseName:normalize]_[hash] 1`] = `"@scope+project++name__phase_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope 4`] = `"project++name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: [projectName:normalize]_[phaseName:trimPrefix]_[hash] 1`] = `"@scope+project++name_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; -exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope 5`] = `"prefix/project++name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: [projectName]_[hash] 1`] = `"@scope/project+name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: no pattern 1`] = `"09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: prefix/[projectName:normalize]_[hash] 1`] = `"prefix/@scope+project++name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: prefix/[projectName:normalize]_[phaseName:normalize]_[hash] 1`] = `"prefix/@scope+project++name__phase_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: prefix/[projectName:normalize]_[phaseName:trimPrefix]_[hash] 1`] = `"prefix/@scope+project++name_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: prefix/[projectName]_[hash] 1`] = `"prefix/@scope/project+name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: prefix/[projectName]_[phaseName:normalize]_[hash] 1`] = `"prefix/@scope/project+name__phase_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name with a scope: prefix/[projectName]_[phaseName:trimPrefix]_[hash] 1`] = `"prefix/@scope/project+name_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: [hash] 1`] = `"09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: [phaseName:normalize]_[hash] 1`] = `"_phase_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: [phaseName:trimPrefix]_[hash] 1`] = `"compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: [projectName:normalize]_[hash] 1`] = `"project++name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: [projectName:normalize]_[phaseName:normalize]_[hash] 1`] = `"project++name__phase_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: [projectName:normalize]_[phaseName:trimPrefix]_[hash] 1`] = `"project++name_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: [projectName]_[hash] 1`] = `"project+name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: no pattern 1`] = `"09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: prefix/[projectName:normalize]_[hash] 1`] = `"prefix/project++name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: prefix/[projectName:normalize]_[phaseName:normalize]_[hash] 1`] = `"prefix/project++name__phase_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: prefix/[projectName:normalize]_[phaseName:trimPrefix]_[hash] 1`] = `"prefix/project++name_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: prefix/[projectName]_[hash] 1`] = `"prefix/project+name_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: prefix/[projectName]_[phaseName:normalize]_[hash] 1`] = `"prefix/project+name__phase_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; + +exports[`CacheEntryId Valid pattern names Handles a cache entry name for a project name without a scope: prefix/[projectName]_[phaseName:trimPrefix]_[hash] 1`] = `"prefix/project+name_compile_09d1ecee6d5f888fa6c35ca804b5dac7c3735ce3"`; diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts index 31cb4004a08..06dec7feee6 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts @@ -68,10 +68,6 @@ export class ProjectLogWritable extends TerminalWritable { this._logWriter = FileWriter.open(this._logPath); } - public static normalizeNameForLogFilenameIdentifiers(name: string): string { - return name.replace(/:/g, '_'); // Replace colons with underscores to be filesystem-safe - } - protected onWriteChunk(chunk: ITerminalChunk): void { if (!this._logWriter) { throw new InternalError('Output file was closed'); diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts index eef50d26984..3a5e71bb357 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts @@ -432,7 +432,8 @@ export class ProjectTaskRunner extends BaseTaskRunner { terminal, command: this._commandToRun, trackedProjectFiles: trackedProjectFiles, - projectChangeAnalyzer: this._projectChangeAnalyzer + projectChangeAnalyzer: this._projectChangeAnalyzer, + phaseName: this._phase.name }); } } From 2a9c358dacbff4a2b0e28ba59be4983c4b82cf71 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Fri, 24 Dec 2021 20:53:19 -0800 Subject: [PATCH 07/19] Rename the _multiPhaseCommands to multiPhaseCommands. --- apps/rush-lib/src/api/ExperimentsConfiguration.ts | 4 +--- apps/rush-lib/src/cli/RushCommandLineParser.ts | 4 ++-- apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts | 2 +- apps/rush-lib/src/schemas/experiments.schema.json | 4 ++-- common/reviews/api/rush-lib.api.md | 2 +- 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/apps/rush-lib/src/api/ExperimentsConfiguration.ts b/apps/rush-lib/src/api/ExperimentsConfiguration.ts index 45488a196a3..986eead3b14 100644 --- a/apps/rush-lib/src/api/ExperimentsConfiguration.ts +++ b/apps/rush-lib/src/api/ExperimentsConfiguration.ts @@ -44,10 +44,8 @@ export interface IExperimentsJson { /** * If true, the multi-phase commands feature is enabled. To use this feature, create a "phased" command * in common/config/rush/command-line.json. - * - * THIS FEATURE IS NOT READY FOR USAGE YET. SEE GITHUB #2300 FOR STATUS. */ - _multiPhaseCommands?: boolean; + multiPhaseCommands?: boolean; } /** diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts index 018326818ce..e955514aaa3 100644 --- a/apps/rush-lib/src/cli/RushCommandLineParser.ts +++ b/apps/rush-lib/src/cli/RushCommandLineParser.ts @@ -277,12 +277,12 @@ export class RushCommandLineParser extends CommandLineParser { case RushConstants.phasedCommandKind: { if ( !command.isSynthetic && // synthetic commands come from bulk commands - !this.rushConfiguration.experimentsConfiguration.configuration._multiPhaseCommands + !this.rushConfiguration.experimentsConfiguration.configuration.multiPhaseCommands ) { throw new Error( `${RushConstants.commandLineFilename} defines a command "${command.name}" ` + `that uses the "${RushConstants.phasedCommandKind}" command kind. To use this command kind, ` + - 'the "_multiPhaseCommands" experiment must be enabled. Note that this feature is not complete ' + + 'the "multiPhaseCommands" experiment must be enabled. Note that this feature is not complete ' + 'and will not work as expected.' ); } diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts index 06dec7feee6..501b362dffa 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts @@ -46,7 +46,7 @@ export class ProjectLogWritable extends TerminalWritable { 'build' ); // If the multi-phase commands experiment is enabled, put logs under `rush-logs` - if (project.rushConfiguration.experimentsConfiguration.configuration._multiPhaseCommands) { + if (project.rushConfiguration.experimentsConfiguration.configuration.multiPhaseCommands) { // Delete the legacy logs FileSystem.deleteFile(legacyLogPath); FileSystem.deleteFile(legacyErrorLogPath); diff --git a/apps/rush-lib/src/schemas/experiments.schema.json b/apps/rush-lib/src/schemas/experiments.schema.json index 96feb9d7c32..1c7afff172c 100644 --- a/apps/rush-lib/src/schemas/experiments.schema.json +++ b/apps/rush-lib/src/schemas/experiments.schema.json @@ -30,8 +30,8 @@ "description": "If true, build caching will respect the allowWarningsInSuccessfulBuild flag and cache builds with warnings. This will not replay warnings from the cached build.", "type": "boolean" }, - "_multiPhaseCommands": { - "description": "(NOT YET FEATURE COMPLETE) If true, the multi-phase commands feature is enabled. To use this feature, create a \"phased\" command in common/config/rush/command-line.json.", + "multiPhaseCommands": { + "description": "If true, the multi-phase commands feature is enabled. To use this feature, create a \"phased\" command in common/config/rush/command-line.json.", "type": "boolean" } }, diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 86fc8f7184b..ceb7f7596be 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -241,7 +241,7 @@ export interface IEnvironmentConfigurationInitializeOptions { // @beta export interface IExperimentsJson { buildCacheWithAllowWarningsInSuccessfulBuild?: boolean; - _multiPhaseCommands?: boolean; + multiPhaseCommands?: boolean; noChmodFieldInTarHeaderNormalization?: boolean; omitImportersFromPreventManualShrinkwrapChanges?: boolean; usePnpmFrozenLockfileForRushInstall?: boolean; From dc43e774a8cbcfa5b969b4a4abaed54b91719853 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Fri, 24 Dec 2021 20:53:30 -0800 Subject: [PATCH 08/19] Rush change --- .../rush/phased-commands_2021-12-25-04-47.json | 10 ++++++++++ .../rush/phased-commands_2021-12-25-04-52.json | 10 ++++++++++ 2 files changed, 20 insertions(+) create mode 100644 common/changes/@microsoft/rush/phased-commands_2021-12-25-04-47.json create mode 100644 common/changes/@microsoft/rush/phased-commands_2021-12-25-04-52.json diff --git a/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-47.json b/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-47.json new file mode 100644 index 00000000000..4a4732a3622 --- /dev/null +++ b/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-47.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "(BREAKING CHANGE) Remove the \"write-build-cache\" action.", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file diff --git a/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-52.json b/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-52.json new file mode 100644 index 00000000000..9a344497c71 --- /dev/null +++ b/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-52.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Add support for phased commands behind the multiPhaseCommands experiment.", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file From 1227542de65867cf7721c8e87980eda555f24462 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sat, 25 Dec 2021 13:52:05 -0800 Subject: [PATCH 09/19] Add support for the "addPhasesToCommand" and "skipPhasesForCommand" properties of Flag parameters. --- .../src/api/CommandLineConfiguration.ts | 40 ++++++++++--------- .../src/cli/scriptActions/BaseScriptAction.ts | 2 +- .../cli/scriptActions/PhasedScriptAction.ts | 23 ++++++++++- .../rush-lib/src/logic/ProjectTaskSelector.ts | 4 +- 4 files changed, 47 insertions(+), 22 deletions(-) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index 7517ed548bd..1b6d9b27d27 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -27,6 +27,7 @@ export interface IShellCommandTokenContext { } export interface IPhase extends IPhaseJson { + isSynthetic?: boolean; logFilenameIdentifier: string; getDisplayNameForProject(rushProject: RushConfigurationProject): string; } @@ -157,8 +158,8 @@ export class CommandLineConfiguration { const dependency: IPhase | undefined = this.phases.get(dependencyName); if (!dependency) { throw new Error( - `In ${RushConstants.commandLineFilename}, in the phase "${phase.name as string}", the self ` + - `dependency phase "${dependencyName as string}" does not exist.` + `In ${RushConstants.commandLineFilename}, in the phase "${phase.name}", the self ` + + `dependency phase "${dependencyName}" does not exist.` ); } } @@ -168,8 +169,8 @@ export class CommandLineConfiguration { for (const dependency of phase.dependencies.upstream) { if (!this.phases.has(dependency)) { throw new Error( - `In ${RushConstants.commandLineFilename}, in the phase "${phase.name as string}", ` + - `the upstream dependency phase "${dependency as string}" does not exist.` + `In ${RushConstants.commandLineFilename}, in the phase "${phase.name}", ` + + `the upstream dependency phase "${dependency}" does not exist.` ); } } @@ -197,6 +198,7 @@ export class CommandLineConfiguration { const phaseName: string = command.name; const phaseForBulkCommand: IPhase = { name: phaseName, + isSynthetic: true, dependencies: { upstream: command.ignoreDependencyOrder ? undefined : [phaseName] }, @@ -247,7 +249,7 @@ export class CommandLineConfiguration { if (!this.phases.has(phaseName)) { throw new Error( `In ${RushConstants.commandLineFilename}, in the "phases" property of the ` + - `"${normalizedCommand.name}" command, the phase "${phaseName as string}" does not exist.` + `"${normalizedCommand.name}" command, the phase "${phaseName}" does not exist.` ); } @@ -260,7 +262,7 @@ export class CommandLineConfiguration { throw new Error( `In ${RushConstants.commandLineFilename}, in the "skipPhasesForCommand" property of the ` + `"${normalizedCommand.name}" command, the phase ` + - `"${phaseName as string}" does not exist.` + `"${phaseName}" does not exist.` ); } @@ -365,37 +367,39 @@ export class CommandLineConfiguration { const addPhasesToCommandSet: Set = new Set(); if (normalizedParameter.addPhasesToCommand) { - for (const phase of normalizedParameter.addPhasesToCommand) { - addPhasesToCommandSet.add(phase); - if (!this.phases.has(phase)) { + for (const phaseName of normalizedParameter.addPhasesToCommand) { + addPhasesToCommandSet.add(phaseName); + const phase: IPhase | undefined = this.phases.get(phaseName); + if (!phase || phase.isSynthetic) { throw new Error( `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + - `that lists phase "${phase as string}" in its "addPhasesToCommand" ` + + `that lists phase "${phaseName}" in its "addPhasesToCommand" ` + 'property that does not exist.' ); } else { - populateCommandAssociatedParametersForPhase(phase, normalizedParameter); + populateCommandAssociatedParametersForPhase(phaseName, normalizedParameter); parameterHasAssociations = true; } } } if (normalizedParameter.skipPhasesForCommand) { - for (const phase of normalizedParameter.skipPhasesForCommand) { - if (!this.phases.has(phase)) { + for (const phaseName of normalizedParameter.skipPhasesForCommand) { + const phase: IPhase | undefined = this.phases.get(phaseName); + if (!phase || phase.isSynthetic) { throw new Error( `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + - `that lists phase "${phase as string}" in its skipPhasesForCommand" ` + + `that lists phase "${phaseName}" in its skipPhasesForCommand" ` + 'property that does not exist.' ); - } else if (addPhasesToCommandSet.has(phase)) { + } else if (addPhasesToCommandSet.has(phaseName)) { throw new Error( `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + - `that lists phase "${phase as string}" in both its "addPhasesToCommand" ` + + `that lists phase "${phaseName}" in both its "addPhasesToCommand" ` + 'and "skipPhasesForCommand" properties.' ); } else { - populateCommandAssociatedParametersForPhase(phase, normalizedParameter); + populateCommandAssociatedParametersForPhase(phaseName, normalizedParameter); parameterHasAssociations = true; } } @@ -453,7 +457,7 @@ export class CommandLineConfiguration { if (!this.phases.has(associatedPhase)) { throw new Error( `${RushConstants.commandLineFilename} defines a parameter "${normalizedParameter.longName}" ` + - `that is associated with a phase "${associatedPhase as string}" that does not exist.` + `that is associated with a phase "${associatedPhase}" that does not exist.` ); } else { populateCommandAssociatedParametersForPhase(associatedPhase, normalizedParameter); diff --git a/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts index b1b57406b0b..af0eb470ed4 100644 --- a/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BaseScriptAction.ts @@ -26,7 +26,7 @@ export interface IBaseScriptActionOptions extends IBas * The two subclasses are BulkScriptAction and GlobalScriptAction. */ export abstract class BaseScriptAction extends BaseRushAction { - protected readonly commandLineConfiguration: CommandLineConfiguration | undefined; + protected readonly commandLineConfiguration: CommandLineConfiguration; protected readonly customParameters: CommandLineParameter[] = []; protected readonly command: TCommand; diff --git a/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index 8f66f23bc81..d1a9125008b 100644 --- a/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -23,6 +23,7 @@ import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; import { SelectionParameterSet } from '../SelectionParameterSet'; import { CommandLineConfiguration, IPhase, IPhasedCommand } from '../../api/CommandLineConfiguration'; import { IProjectTaskSelectorOptions, ProjectTaskSelector } from '../../logic/ProjectTaskSelector'; +import { IFlagParameterJson } from '../../api/CommandLineJson'; /** * Constructor parameters for BulkScriptAction. @@ -131,6 +132,26 @@ export class PhasedScriptAction extends BaseScriptAction { return; } + const phasesToRun: Set = new Set(this._actionPhases); + for (const parameter of this.commandLineConfiguration.parameters) { + if (parameter.parameterKind === 'flag') { + if (this.getFlagParameter(parameter.longName)!.value) { + const flagParameter: IFlagParameterJson = parameter as IFlagParameterJson; + if (flagParameter.addPhasesToCommand) { + for (const phase of flagParameter.addPhasesToCommand) { + phasesToRun.add(phase); + } + } + + if (flagParameter.skipPhasesForCommand) { + for (const phase of flagParameter.skipPhasesForCommand) { + phasesToRun.delete(phase); + } + } + } + } + } + const taskSelectorOptions: IProjectTaskSelectorOptions = { rushConfiguration: this.rushConfiguration, buildCacheConfiguration, @@ -139,7 +160,7 @@ export class PhasedScriptAction extends BaseScriptAction { isQuietMode: isQuietMode, isDebugMode: isDebugMode, isIncrementalBuildAllowed: this._isIncrementalBuildAllowed, - phasesToRun: this._actionPhases, + phasesToRun: phasesToRun, phases: this._phases }; diff --git a/apps/rush-lib/src/logic/ProjectTaskSelector.ts b/apps/rush-lib/src/logic/ProjectTaskSelector.ts index 37070380076..712b0debccb 100644 --- a/apps/rush-lib/src/logic/ProjectTaskSelector.ts +++ b/apps/rush-lib/src/logic/ProjectTaskSelector.ts @@ -19,7 +19,7 @@ export interface IProjectTaskSelectorOptions { isIncrementalBuildAllowed: boolean; projectChangeAnalyzer?: ProjectChangeAnalyzer; - phasesToRun: ReadonlyArray; + phasesToRun: Iterable; phases: Map; } @@ -32,7 +32,7 @@ export interface IProjectTaskSelectorOptions { export class ProjectTaskSelector { private readonly _options: IProjectTaskSelectorOptions; private readonly _projectChangeAnalyzer: ProjectChangeAnalyzer; - private readonly _phasesToRun: ReadonlyArray; + private readonly _phasesToRun: Iterable; private readonly _phases: Map; public constructor(options: IProjectTaskSelectorOptions) { From 7d9d96a4a3b2112935901b37b1de66a7eda50c01 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 15:31:21 -0800 Subject: [PATCH 10/19] Rename phaseCliques to relatedPhaseSets and add a clarifying comment. --- .../src/api/CommandLineConfiguration.ts | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index 1b6d9b27d27..35d48be1963 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -117,7 +117,10 @@ export class CommandLineConfiguration { */ public constructor(commandLineJson: ICommandLineJson | undefined) { const commandsForPhase: Map> = new Map(); - const phaseCliques: Map> = new Map(); + // This maps phase names to the names of all other phases that depend on it or are + // dependent on it. This is used to determine which commands a phase affects, even + // if that phase isn't explicitly listed for that command. + const relatedPhaseSets: Map> = new Map(); if (commandLineJson?.phases) { for (const phase of commandLineJson.phases) { if (this.phases.has(phase.name)) { @@ -177,15 +180,15 @@ export class CommandLineConfiguration { } this._checkForPhaseSelfCycles(phase); - const phaseClique: Set = new Set(); - this._populatePhaseClique(phase.name, phaseClique); - phaseCliques.set(phase.name, phaseClique); + const relatedPhaseSet: Set = new Set(); + this._populateRelatedPhaseSets(phase.name, relatedPhaseSet); + relatedPhaseSets.set(phase.name, relatedPhaseSet); } function populateCommandsForPhase(phaseName: string, command: IPhasedCommand): void { - const phaseClique: Set = phaseCliques.get(phaseName)!; - for (const phaseCliqueIdentifier of phaseClique) { - commandsForPhase.get(phaseCliqueIdentifier)!.add(command); + const populateRelatedPhaseSet: Set = relatedPhaseSets.get(phaseName)!; + for (const relatedPhaseSetIdentifier of populateRelatedPhaseSet) { + commandsForPhase.get(relatedPhaseSetIdentifier)!.add(command); } } @@ -210,9 +213,9 @@ export class CommandLineConfiguration { }; this.phases.set(phaseName, phaseForBulkCommand); syntheticPhasesForTranslatedBulkCommands.set(command.name, phaseName); - const phaseClique: Set = new Set(); - this._populatePhaseClique(phaseName, phaseClique); - phaseCliques.set(phaseName, phaseClique); + const relatedPhaseSet: Set = new Set(); + this._populateRelatedPhaseSets(phaseName, relatedPhaseSet); + relatedPhaseSets.set(phaseName, relatedPhaseSet); const translatedCommand: IPhasedCommand = { ...command, @@ -507,20 +510,20 @@ export class CommandLineConfiguration { } } - private _populatePhaseClique(phaseName: string, clique: Set): void { - if (!clique.has(phaseName)) { - clique.add(phaseName); + private _populateRelatedPhaseSets(phaseName: string, relatedPhaseSet: Set): void { + if (!relatedPhaseSet.has(phaseName)) { + relatedPhaseSet.add(phaseName); const phase: IPhase = this.phases.get(phaseName)!; if (phase.dependencies) { if (phase.dependencies.self) { for (const dependencyName of phase.dependencies.self) { - this._populatePhaseClique(dependencyName, clique); + this._populateRelatedPhaseSets(dependencyName, relatedPhaseSet); } } if (phase.dependencies.upstream) { for (const dependencyName of phase.dependencies.upstream) { - this._populatePhaseClique(dependencyName, clique); + this._populateRelatedPhaseSets(dependencyName, relatedPhaseSet); } } } From 116b6ff2ad74476f0959abd595bf8cc9f2fcdfe5 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 15:32:49 -0800 Subject: [PATCH 11/19] Rename commandsForPhase to commandsByPhaseName --- apps/rush-lib/src/api/CommandLineConfiguration.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index 35d48be1963..4109fd7fb03 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -116,7 +116,7 @@ export class CommandLineConfiguration { * @internal */ public constructor(commandLineJson: ICommandLineJson | undefined) { - const commandsForPhase: Map> = new Map(); + const commandsByPhaseName: Map> = new Map(); // This maps phase names to the names of all other phases that depend on it or are // dependent on it. This is used to determine which commands a phase affects, even // if that phase isn't explicitly listed for that command. @@ -151,7 +151,7 @@ export class CommandLineConfiguration { getDisplayNameForProject: (rushProject: RushConfigurationProject) => `${rushProject.packageName} (${phaseNameWithoutPrefix})` }); - commandsForPhase.set(phase.name, new Set()); + commandsByPhaseName.set(phase.name, new Set()); } } @@ -188,7 +188,7 @@ export class CommandLineConfiguration { function populateCommandsForPhase(phaseName: string, command: IPhasedCommand): void { const populateRelatedPhaseSet: Set = relatedPhaseSets.get(phaseName)!; for (const relatedPhaseSetIdentifier of populateRelatedPhaseSet) { - commandsForPhase.get(relatedPhaseSetIdentifier)!.add(command); + commandsByPhaseName.get(relatedPhaseSetIdentifier)!.add(command); } } @@ -225,7 +225,7 @@ export class CommandLineConfiguration { associatedParameters: new Set(), phases: [phaseName] }; - commandsForPhase.set(phaseName, new Set()); + commandsByPhaseName.set(phaseName, new Set()); populateCommandsForPhase(phaseName, translatedCommand); return translatedCommand; }; @@ -347,7 +347,7 @@ export class CommandLineConfiguration { if (commandLineJson?.parameters) { function populateCommandAssociatedParametersForPhase(phaseName: string, parameter: Parameter): void { - const commands: Set = commandsForPhase.get(phaseName)!; + const commands: Set = commandsByPhaseName.get(phaseName)!; for (const command of commands) { command.associatedParameters.add(parameter); } From b87fbf16ba1b4637a169c998c1e08c5d54a13aa4 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 15:36:40 -0800 Subject: [PATCH 12/19] Rename the multiPhaseCommands experiment to phasedCommands. --- apps/rush-lib/src/api/ExperimentsConfiguration.ts | 4 ++-- apps/rush-lib/src/cli/RushCommandLineParser.ts | 4 ++-- apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts | 4 ++-- apps/rush-lib/src/schemas/experiments.schema.json | 4 ++-- common/reviews/api/rush-lib.api.md | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/apps/rush-lib/src/api/ExperimentsConfiguration.ts b/apps/rush-lib/src/api/ExperimentsConfiguration.ts index 986eead3b14..1a85315775e 100644 --- a/apps/rush-lib/src/api/ExperimentsConfiguration.ts +++ b/apps/rush-lib/src/api/ExperimentsConfiguration.ts @@ -42,10 +42,10 @@ export interface IExperimentsJson { buildCacheWithAllowWarningsInSuccessfulBuild?: boolean; /** - * If true, the multi-phase commands feature is enabled. To use this feature, create a "phased" command + * If true, the phased commands feature is enabled. To use this feature, create a "phased" command * in common/config/rush/command-line.json. */ - multiPhaseCommands?: boolean; + phasedCommands?: boolean; } /** diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts index e955514aaa3..396ab5dd80a 100644 --- a/apps/rush-lib/src/cli/RushCommandLineParser.ts +++ b/apps/rush-lib/src/cli/RushCommandLineParser.ts @@ -277,12 +277,12 @@ export class RushCommandLineParser extends CommandLineParser { case RushConstants.phasedCommandKind: { if ( !command.isSynthetic && // synthetic commands come from bulk commands - !this.rushConfiguration.experimentsConfiguration.configuration.multiPhaseCommands + !this.rushConfiguration.experimentsConfiguration.configuration.phasedCommands ) { throw new Error( `${RushConstants.commandLineFilename} defines a command "${command.name}" ` + `that uses the "${RushConstants.phasedCommandKind}" command kind. To use this command kind, ` + - 'the "multiPhaseCommands" experiment must be enabled. Note that this feature is not complete ' + + 'the "phasedCommands" experiment must be enabled. Note that this feature is not complete ' + 'and will not work as expected.' ); } diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts index 501b362dffa..0310f0c6269 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts @@ -45,8 +45,8 @@ export class ProjectLogWritable extends TerminalWritable { projectFolder, 'build' ); - // If the multi-phase commands experiment is enabled, put logs under `rush-logs` - if (project.rushConfiguration.experimentsConfiguration.configuration.multiPhaseCommands) { + // If the phased commands experiment is enabled, put logs under `rush-logs` + if (project.rushConfiguration.experimentsConfiguration.configuration.phasedCommands) { // Delete the legacy logs FileSystem.deleteFile(legacyLogPath); FileSystem.deleteFile(legacyErrorLogPath); diff --git a/apps/rush-lib/src/schemas/experiments.schema.json b/apps/rush-lib/src/schemas/experiments.schema.json index 1c7afff172c..8b8f776e00e 100644 --- a/apps/rush-lib/src/schemas/experiments.schema.json +++ b/apps/rush-lib/src/schemas/experiments.schema.json @@ -30,8 +30,8 @@ "description": "If true, build caching will respect the allowWarningsInSuccessfulBuild flag and cache builds with warnings. This will not replay warnings from the cached build.", "type": "boolean" }, - "multiPhaseCommands": { - "description": "If true, the multi-phase commands feature is enabled. To use this feature, create a \"phased\" command in common/config/rush/command-line.json.", + "phasedCommands": { + "description": "If true, the phased commands feature is enabled. To use this feature, create a \"phased\" command in common/config/rush/command-line.json.", "type": "boolean" } }, diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index ceb7f7596be..68284e86014 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -241,9 +241,9 @@ export interface IEnvironmentConfigurationInitializeOptions { // @beta export interface IExperimentsJson { buildCacheWithAllowWarningsInSuccessfulBuild?: boolean; - multiPhaseCommands?: boolean; noChmodFieldInTarHeaderNormalization?: boolean; omitImportersFromPreventManualShrinkwrapChanges?: boolean; + phasedCommands?: boolean; usePnpmFrozenLockfileForRushInstall?: boolean; usePnpmPreferFrozenLockfileForRushUpdate?: boolean; } From 5002793835517c7615312f371c682ca36d56d5c5 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 15:38:52 -0800 Subject: [PATCH 13/19] Rush change. Co-authored-by: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> --- .../@microsoft/rush/phased-commands_2021-12-25-04-47.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-47.json b/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-47.json index 4a4732a3622..5e6c1b7be32 100644 --- a/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-47.json +++ b/common/changes/@microsoft/rush/phased-commands_2021-12-25-04-47.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@microsoft/rush", - "comment": "(BREAKING CHANGE) Remove the \"write-build-cache\" action.", + "comment": "(BREAKING CHANGE) Remove the experimental command \"rush write-build-cache\", since it is no longer needed and would be incompatible with phased builds. If you need this command for some reason, please create a GitHub issue.", "type": "none" } ], From a86f92a41a848779bfec0881971b74afaf07c669 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 17:02:22 -0800 Subject: [PATCH 14/19] Add docs for IPhase and IPhasedCommand properties. --- apps/rush-lib/src/api/CommandLineConfiguration.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index 4109fd7fb03..d67fbd144c4 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -27,8 +27,19 @@ export interface IShellCommandTokenContext { } export interface IPhase extends IPhaseJson { + /** + * If set to "true," this this phase was generated from a bulk command, and + * was not explicitly defined in the command-line.json file. + */ isSynthetic?: boolean; + + /** + * This property is used in the name of the filename for the logs generated by this + * phase. This is a filesystem-safe version of the phase name. For example, + * a phase with name "_phase:compile" has a `logFilenameIdentifier` of "_phase_compile". + */ logFilenameIdentifier: string; + getDisplayNameForProject(rushProject: RushConfigurationProject): string; } @@ -36,6 +47,10 @@ export interface ICommandWithParameters { associatedParameters: Set; } export interface IPhasedCommand extends IPhasedCommandJson, ICommandWithParameters { + /** + * If set to "true," this this phased command was generated from a bulk command, and + * was not explicitly defined in the command-line.json file. + */ isSynthetic?: boolean; watchForChanges?: boolean; disableBuildCache?: boolean; From 73ace451b812d71c46590484154eb8385b08a0ae Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 17:10:56 -0800 Subject: [PATCH 15/19] Move phase name generation code to the ProjectTaskSelector. --- .../src/api/CommandLineConfiguration.ts | 25 ++++++------------- .../rush-lib/src/logic/ProjectTaskSelector.ts | 13 +++++++++- apps/rush-lib/src/logic/RushConstants.ts | 5 ++++ common/reviews/api/rush-lib.api.md | 1 + 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index d67fbd144c4..4be1966b766 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -2,11 +2,9 @@ // See LICENSE in the project root for license information. import * as path from 'path'; - import { JsonFile, JsonSchema, FileSystem } from '@rushstack/node-core-library'; import { RushConstants } from '../logic/RushConstants'; - import type { CommandJson, ICommandLineJson, @@ -18,9 +16,6 @@ import type { IChoiceParameterJson, IStringParameterJson } from './CommandLineJson'; -import type { RushConfigurationProject } from './RushConfigurationProject'; - -const EXPECTED_PHASE_NAME_PREFIX: '_phase:' = '_phase:'; export interface IShellCommandTokenContext { packageFolder: string; @@ -39,8 +34,6 @@ export interface IPhase extends IPhaseJson { * a phase with name "_phase:compile" has a `logFilenameIdentifier` of "_phase_compile". */ logFilenameIdentifier: string; - - getDisplayNameForProject(rushProject: RushConfigurationProject): string; } export interface ICommandWithParameters { @@ -137,6 +130,7 @@ export class CommandLineConfiguration { // if that phase isn't explicitly listed for that command. const relatedPhaseSets: Map> = new Map(); if (commandLineJson?.phases) { + const phaseNamePrefix: string = RushConstants.phaseNamePrefix; for (const phase of commandLineJson.phases) { if (this.phases.has(phase.name)) { throw new Error( @@ -145,26 +139,23 @@ export class CommandLineConfiguration { ); } - if (phase.name.substring(0, EXPECTED_PHASE_NAME_PREFIX.length) !== EXPECTED_PHASE_NAME_PREFIX) { + if (phase.name.substring(0, phaseNamePrefix.length) !== phaseNamePrefix) { throw new Error( `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + - `does not begin with the required prefix "${EXPECTED_PHASE_NAME_PREFIX}".` + `does not begin with the required prefix "${phaseNamePrefix}".` ); } - if (phase.name.length <= EXPECTED_PHASE_NAME_PREFIX.length) { + if (phase.name.length <= phaseNamePrefix.length) { throw new Error( `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + - `must have characters after "${EXPECTED_PHASE_NAME_PREFIX}"` + `must have characters after "${phaseNamePrefix}"` ); } - const phaseNameWithoutPrefix: string = phase.name.substring(EXPECTED_PHASE_NAME_PREFIX.length); this.phases.set(phase.name, { ...phase, - logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(phase.name), - getDisplayNameForProject: (rushProject: RushConfigurationProject) => - `${rushProject.packageName} (${phaseNameWithoutPrefix})` + logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(phase.name) }); commandsByPhaseName.set(phase.name, new Set()); } @@ -222,9 +213,7 @@ export class CommandLineConfiguration { }, ignoreMissingScript: command.ignoreMissingScript, allowWarningsOnSuccess: command.allowWarningsInSuccessfulBuild, - logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(command.name), - // Because this is a synthetic phase, just use the project name because there aren't any other phases - getDisplayNameForProject: (rushProject: RushConfigurationProject) => rushProject.packageName + logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(command.name) }; this.phases.set(phaseName, phaseForBulkCommand); syntheticPhasesForTranslatedBulkCommands.set(command.name, phaseName); diff --git a/apps/rush-lib/src/logic/ProjectTaskSelector.ts b/apps/rush-lib/src/logic/ProjectTaskSelector.ts index 712b0debccb..32cccdf9af9 100644 --- a/apps/rush-lib/src/logic/ProjectTaskSelector.ts +++ b/apps/rush-lib/src/logic/ProjectTaskSelector.ts @@ -8,6 +8,7 @@ import { convertSlashesForWindows, ProjectTaskRunner } from './taskExecution/Pro import { ProjectChangeAnalyzer } from './ProjectChangeAnalyzer'; import { TaskCollection } from './taskExecution/TaskCollection'; import { IPhase } from '../api/CommandLineConfiguration'; +import { RushConstants } from './RushConstants'; export interface IProjectTaskSelectorOptions { rushConfiguration: RushConfiguration; @@ -82,7 +83,7 @@ export class ProjectTaskSelector { phase: IPhase, taskCollection: TaskCollection ): string { - const taskName: string = phase.getDisplayNameForProject(project); + const taskName: string = this._getPhaseDisplayNameForProject(phase, project); if (taskCollection.hasTask(taskName)) { return taskName; } @@ -154,6 +155,16 @@ export class ProjectTaskSelector { return phase; } + private _getPhaseDisplayNameForProject(phase: IPhase, project: RushConfigurationProject): string { + if (phase.isSynthetic) { + // Because this is a synthetic phase, just use the project name because there aren't any other phases + return project.packageName; + } else { + const phaseNameWithoutPrefix: string = phase.name.substring(RushConstants.phaseNamePrefix.length); + return `${project.packageName} (${phaseNameWithoutPrefix})`; + } + } + private static _getScriptCommand( rushProject: RushConfigurationProject, script: string diff --git a/apps/rush-lib/src/logic/RushConstants.ts b/apps/rush-lib/src/logic/RushConstants.ts index 8d7c8269459..3ea528951c2 100644 --- a/apps/rush-lib/src/logic/RushConstants.ts +++ b/apps/rush-lib/src/logic/RushConstants.ts @@ -234,4 +234,9 @@ export class RushConstants { * The name of the project `rush-logs` folder. */ public static readonly rushLogsFolderName: string = 'rush-logs'; + + /** + * The expected prefix for phase names in "common/config/rush/command-line.json" + */ + public static readonly phaseNamePrefix: '_phase:' = '_phase:'; } diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 68284e86014..cebfb4b4029 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -627,6 +627,7 @@ export class RushConstants { static readonly nonbrowserApprovedPackagesFilename: string; static readonly npmShrinkwrapFilename: string; static readonly phasedCommandKind: 'phased'; + static readonly phaseNamePrefix: '_phase:'; // @deprecated static readonly pinnedVersionsFilename: string; static readonly pnpmfileV1Filename: string; From f354e114212090be81aeef081ac67096281f7e91 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 17:19:38 -0800 Subject: [PATCH 16/19] Make isSynthetic a required property. --- apps/rush-lib/src/api/CommandLineConfiguration.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index 4be1966b766..7567ddcbeb2 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -26,7 +26,7 @@ export interface IPhase extends IPhaseJson { * If set to "true," this this phase was generated from a bulk command, and * was not explicitly defined in the command-line.json file. */ - isSynthetic?: boolean; + isSynthetic: boolean; /** * This property is used in the name of the filename for the logs generated by this @@ -44,7 +44,7 @@ export interface IPhasedCommand extends IPhasedCommandJson, ICommandWithParamete * If set to "true," this this phased command was generated from a bulk command, and * was not explicitly defined in the command-line.json file. */ - isSynthetic?: boolean; + isSynthetic: boolean; watchForChanges?: boolean; disableBuildCache?: boolean; } @@ -155,6 +155,7 @@ export class CommandLineConfiguration { this.phases.set(phase.name, { ...phase, + isSynthetic: false, logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(phase.name) }); commandsByPhaseName.set(phase.name, new Set()); @@ -249,6 +250,7 @@ export class CommandLineConfiguration { case RushConstants.phasedCommandKind: { normalizedCommand = { ...command, + isSynthetic: false, associatedParameters: new Set() }; From 00664c9a3f4d3f442667552b00c28637ea01ee90 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 17:29:24 -0800 Subject: [PATCH 17/19] Extract some inlined functions in the CommandLineConfiguration constuctor to private member functions. --- .../src/api/CommandLineConfiguration.ts | 143 +++++++++--------- 1 file changed, 72 insertions(+), 71 deletions(-) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index 7567ddcbeb2..de1eca71dc9 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -104,8 +104,8 @@ export class CommandLineConfiguration { path.join(__dirname, '../schemas/command-line.schema.json') ); - public readonly commands: Map = new Map(); - public readonly phases: Map = new Map(); + public readonly commands: Map = new Map(); + public readonly phases: Map = new Map(); public readonly parameters: Parameter[] = []; /** @@ -118,17 +118,26 @@ export class CommandLineConfiguration { */ private _additionalPathFolders: string[] = []; + private readonly _commandsByPhaseName: Map> = new Map(); + + /** + * This maps phase names to the names of all other phases that depend on it or are + * dependent on it. This is used to determine which commands a phase affects, even + * if that phase isn't explicitly listed for that command. + */ + private readonly _relatedPhaseSets: Map> = new Map(); + + /** + * A map of bulk command names to their corresponding synthetic phase identifiers + */ + private readonly _syntheticPhasesForTranslatedBulkCommands: Map = new Map(); + /** * Use CommandLineConfiguration.loadFromFile() * * @internal */ public constructor(commandLineJson: ICommandLineJson | undefined) { - const commandsByPhaseName: Map> = new Map(); - // This maps phase names to the names of all other phases that depend on it or are - // dependent on it. This is used to determine which commands a phase affects, even - // if that phase isn't explicitly listed for that command. - const relatedPhaseSets: Map> = new Map(); if (commandLineJson?.phases) { const phaseNamePrefix: string = RushConstants.phaseNamePrefix; for (const phase of commandLineJson.phases) { @@ -158,7 +167,7 @@ export class CommandLineConfiguration { isSynthetic: false, logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(phase.name) }); - commandsByPhaseName.set(phase.name, new Set()); + this._commandsByPhaseName.set(phase.name, new Set()); } } @@ -189,52 +198,9 @@ export class CommandLineConfiguration { this._checkForPhaseSelfCycles(phase); const relatedPhaseSet: Set = new Set(); this._populateRelatedPhaseSets(phase.name, relatedPhaseSet); - relatedPhaseSets.set(phase.name, relatedPhaseSet); - } - - function populateCommandsForPhase(phaseName: string, command: IPhasedCommand): void { - const populateRelatedPhaseSet: Set = relatedPhaseSets.get(phaseName)!; - for (const relatedPhaseSetIdentifier of populateRelatedPhaseSet) { - commandsByPhaseName.get(relatedPhaseSetIdentifier)!.add(command); - } + this._relatedPhaseSets.set(phase.name, relatedPhaseSet); } - // A map of bulk command names to their corresponding synthetic phase identifiers - const syntheticPhasesForTranslatedBulkCommands: Map = new Map(); - const translateBulkCommandToPhasedCommand: ( - command: IBulkCommandJson, - isBuildCommand: boolean - ) => IPhasedCommand = (command: IBulkCommandJson, isBuildCommand: boolean) => { - const phaseName: string = command.name; - const phaseForBulkCommand: IPhase = { - name: phaseName, - isSynthetic: true, - dependencies: { - upstream: command.ignoreDependencyOrder ? undefined : [phaseName] - }, - ignoreMissingScript: command.ignoreMissingScript, - allowWarningsOnSuccess: command.allowWarningsInSuccessfulBuild, - logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(command.name) - }; - this.phases.set(phaseName, phaseForBulkCommand); - syntheticPhasesForTranslatedBulkCommands.set(command.name, phaseName); - const relatedPhaseSet: Set = new Set(); - this._populateRelatedPhaseSets(phaseName, relatedPhaseSet); - relatedPhaseSets.set(phaseName, relatedPhaseSet); - - const translatedCommand: IPhasedCommand = { - ...command, - commandKind: 'phased', - disableBuildCache: true, - isSynthetic: true, - associatedParameters: new Set(), - phases: [phaseName] - }; - commandsByPhaseName.set(phaseName, new Set()); - populateCommandsForPhase(phaseName, translatedCommand); - return translatedCommand; - }; - let buildCommandPhases: string[] | undefined; if (commandLineJson?.commands) { for (const command of commandLineJson.commands) { @@ -262,7 +228,7 @@ export class CommandLineConfiguration { ); } - populateCommandsForPhase(phaseName, normalizedCommand); + this._populateCommandsForPhase(phaseName, normalizedCommand); } if (normalizedCommand.skipPhasesForCommand) { @@ -275,7 +241,7 @@ export class CommandLineConfiguration { ); } - populateCommandsForPhase(phaseName, normalizedCommand); + this._populateCommandsForPhase(phaseName, normalizedCommand); } } @@ -292,7 +258,7 @@ export class CommandLineConfiguration { case RushConstants.bulkCommandKind: { // Translate the bulk command into a phased command - normalizedCommand = translateBulkCommandToPhasedCommand(command, /* isBuildCommand */ false); + normalizedCommand = this._translateBulkCommandToPhasedCommand(command); break; } } @@ -325,10 +291,7 @@ export class CommandLineConfiguration { let buildCommand: Command | undefined = this.commands.get(RushConstants.buildCommandName); if (!buildCommand) { // If the build command was not specified in the config file, add the default build command - buildCommand = translateBulkCommandToPhasedCommand( - DEFAULT_BUILD_COMMAND_JSON, - /* isBuildCommand */ true - ); + buildCommand = this._translateBulkCommandToPhasedCommand(DEFAULT_BUILD_COMMAND_JSON); buildCommand.disableBuildCache = DEFAULT_BUILD_COMMAND_JSON.disableBuildCache; buildCommandPhases = buildCommand.phases; this.commands.set(buildCommand.name, buildCommand); @@ -352,13 +315,6 @@ export class CommandLineConfiguration { } if (commandLineJson?.parameters) { - function populateCommandAssociatedParametersForPhase(phaseName: string, parameter: Parameter): void { - const commands: Set = commandsByPhaseName.get(phaseName)!; - for (const command of commands) { - command.associatedParameters.add(parameter); - } - } - for (const parameter of commandLineJson.parameters) { const normalizedParameter: Parameter = { ...parameter, @@ -386,7 +342,7 @@ export class CommandLineConfiguration { 'property that does not exist.' ); } else { - populateCommandAssociatedParametersForPhase(phaseName, normalizedParameter); + this._populateCommandAssociatedParametersForPhase(phaseName, normalizedParameter); parameterHasAssociations = true; } } @@ -408,7 +364,7 @@ export class CommandLineConfiguration { 'and "skipPhasesForCommand" properties.' ); } else { - populateCommandAssociatedParametersForPhase(phaseName, normalizedParameter); + this._populateCommandAssociatedParametersForPhase(phaseName, normalizedParameter); parameterHasAssociations = true; } } @@ -439,14 +395,14 @@ export class CommandLineConfiguration { for (let i: number = 0; i < normalizedParameter.associatedCommands.length; i++) { const associatedCommandName: string = normalizedParameter.associatedCommands[i]; const syntheticPhaseName: string | undefined = - syntheticPhasesForTranslatedBulkCommands.get(associatedCommandName); + this._syntheticPhasesForTranslatedBulkCommands.get(associatedCommandName); if (syntheticPhaseName) { // If this parameter was associated with a bulk command, change the association to // the command's synthetic phase normalizedParameter.associatedPhases!.push(syntheticPhaseName); normalizedParameter.associatedCommands.splice(i, 1); i--; - populateCommandAssociatedParametersForPhase(syntheticPhaseName, normalizedParameter); + this._populateCommandAssociatedParametersForPhase(syntheticPhaseName, normalizedParameter); parameterHasAssociations = true; } else if (!this.commands.has(associatedCommandName)) { throw new Error( @@ -469,7 +425,7 @@ export class CommandLineConfiguration { `that is associated with a phase "${associatedPhase}" that does not exist.` ); } else { - populateCommandAssociatedParametersForPhase(associatedPhase, normalizedParameter); + this._populateCommandAssociatedParametersForPhase(associatedPhase, normalizedParameter); parameterHasAssociations = true; } } @@ -596,4 +552,49 @@ export class CommandLineConfiguration { private _normalizeNameForLogFilenameIdentifiers(name: string): string { return name.replace(/:/g, '_'); // Replace colons with underscores to be filesystem-safe } + + private _populateCommandsForPhase(phaseName: string, command: IPhasedCommand): void { + const populateRelatedPhaseSet: Set = this._relatedPhaseSets.get(phaseName)!; + for (const relatedPhaseSetIdentifier of populateRelatedPhaseSet) { + this._commandsByPhaseName.get(relatedPhaseSetIdentifier)!.add(command); + } + } + + private _populateCommandAssociatedParametersForPhase(phaseName: string, parameter: Parameter): void { + const commands: Set = this._commandsByPhaseName.get(phaseName)!; + for (const command of commands) { + command.associatedParameters.add(parameter); + } + } + + private _translateBulkCommandToPhasedCommand(command: IBulkCommandJson): IPhasedCommand { + const phaseName: string = command.name; + const phaseForBulkCommand: IPhase = { + name: phaseName, + isSynthetic: true, + dependencies: { + upstream: command.ignoreDependencyOrder ? undefined : [phaseName] + }, + ignoreMissingScript: command.ignoreMissingScript, + allowWarningsOnSuccess: command.allowWarningsInSuccessfulBuild, + logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(command.name) + }; + this.phases.set(phaseName, phaseForBulkCommand); + this._syntheticPhasesForTranslatedBulkCommands.set(command.name, phaseName); + const relatedPhaseSet: Set = new Set(); + this._populateRelatedPhaseSets(phaseName, relatedPhaseSet); + this._relatedPhaseSets.set(phaseName, relatedPhaseSet); + + const translatedCommand: IPhasedCommand = { + ...command, + commandKind: 'phased', + disableBuildCache: true, + isSynthetic: true, + associatedParameters: new Set(), + phases: [phaseName] + }; + this._commandsByPhaseName.set(phaseName, new Set()); + this._populateCommandsForPhase(phaseName, translatedCommand); + return translatedCommand; + } } From c630e6e63578656d61b7e5669411cac8391ac1ae Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 18:03:05 -0800 Subject: [PATCH 18/19] More tightly restrict allowed phase names. --- .../src/api/CommandLineConfiguration.ts | 32 +++++---- .../api/test/CommandLineConfiguration.test.ts | 68 +++++++++++++------ .../CommandLineConfiguration.test.ts.snap | 18 +++-- 3 files changed, 79 insertions(+), 39 deletions(-) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index de1eca71dc9..cfbede85e96 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -139,7 +139,9 @@ export class CommandLineConfiguration { */ public constructor(commandLineJson: ICommandLineJson | undefined) { if (commandLineJson?.phases) { - const phaseNamePrefix: string = RushConstants.phaseNamePrefix; + const phaseNameRegexp: RegExp = new RegExp( + `^${RushConstants.phaseNamePrefix}[a-z][a-z0-9]*([-][a-z0-9]+)*$` + ); for (const phase of commandLineJson.phases) { if (this.phases.has(phase.name)) { throw new Error( @@ -148,17 +150,13 @@ export class CommandLineConfiguration { ); } - if (phase.name.substring(0, phaseNamePrefix.length) !== phaseNamePrefix) { + if (!phase.name.match(phaseNameRegexp)) { throw new Error( `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + - `does not begin with the required prefix "${phaseNamePrefix}".` - ); - } - - if (phase.name.length <= phaseNamePrefix.length) { - throw new Error( - `In ${RushConstants.commandLineFilename}, the phase "${phase.name}"'s name ` + - `must have characters after "${phaseNamePrefix}"` + 'is not a valid phase name. Phase names must begin with the ' + + `required prefix "${RushConstants.phaseNamePrefix}" followed by a name containing ` + + 'lowercase letters, numbers, or hyphens. The name must start with a letter and ' + + 'must not end with a hyphen.' ); } @@ -441,9 +439,9 @@ export class CommandLineConfiguration { } private _checkForPhaseSelfCycles(phase: IPhase, checkedPhases: Set = new Set()): void { - const dependencies: string[] | undefined = phase.dependencies?.self; - if (dependencies) { - for (const dependencyName of dependencies) { + const phaseSelfDependencies: string[] | undefined = phase.dependencies?.self; + if (phaseSelfDependencies) { + for (const dependencyName of phaseSelfDependencies) { if (checkedPhases.has(dependencyName)) { const dependencyNameForError: string = typeof dependencyName === 'string' ? dependencyName : ''; @@ -457,7 +455,7 @@ export class CommandLineConfiguration { if (!dependency) { return; // Ignore, we check for this separately } else { - if (dependencies.length > 1) { + if (phaseSelfDependencies.length > 1) { this._checkForPhaseSelfCycles( dependency, // Clone the set of checked phases if there are multiple branches we need to check @@ -549,6 +547,12 @@ export class CommandLineConfiguration { this._additionalPathFolders.unshift(pathFolder); } + /** + * This function replaces colons (":") with underscores ("_"). + * + * ts-command-line restricts command names to lowercase letters, numbers, underscores, and colons. + * Replacing colons with underscores produces a filesystem-safe name. + */ private _normalizeNameForLogFilenameIdentifiers(name: string): string { return name.replace(/:/g, '_'); // Replace colons with underscores to be filesystem-safe } diff --git a/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts b/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts index 4cc7626cf07..d3c0082b927 100644 --- a/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts +++ b/apps/rush-lib/src/api/test/CommandLineConfiguration.test.ts @@ -26,6 +26,36 @@ describe(CommandLineConfiguration.name, () => { ] }) ).toThrowErrorMatchingSnapshot(); + expect( + () => + new CommandLineConfiguration({ + phases: [ + { + name: '_phase:0' + } + ] + }) + ).toThrowErrorMatchingSnapshot(); + expect( + () => + new CommandLineConfiguration({ + phases: [ + { + name: '_phase:A' + } + ] + }) + ).toThrowErrorMatchingSnapshot(); + expect( + () => + new CommandLineConfiguration({ + phases: [ + { + name: '_phase:A-' + } + ] + }) + ).toThrowErrorMatchingSnapshot(); }); it('Detects a missing phase', () => { @@ -41,7 +71,7 @@ describe(CommandLineConfiguration.name, () => { safeForSimultaneousRushProcesses: false, enableParallelism: true, - phases: ['_phase:A'] + phases: ['_phase:a'] } ] }) @@ -54,9 +84,9 @@ describe(CommandLineConfiguration.name, () => { new CommandLineConfiguration({ phases: [ { - name: '_phase:A', + name: '_phase:a', dependencies: { - upstream: ['_phase:B'] + upstream: ['_phase:b'] } } ] @@ -68,9 +98,9 @@ describe(CommandLineConfiguration.name, () => { new CommandLineConfiguration({ phases: [ { - name: '_phase:A', + name: '_phase:a', dependencies: { - self: ['_phase:B'] + self: ['_phase:b'] } } ] @@ -84,21 +114,21 @@ describe(CommandLineConfiguration.name, () => { new CommandLineConfiguration({ phases: [ { - name: '_phase:A', + name: '_phase:a', dependencies: { - self: ['_phase:B'] + self: ['_phase:b'] } }, { - name: '_phase:B', + name: '_phase:b', dependencies: { - self: ['_phase:C'] + self: ['_phase:c'] } }, { - name: '_phase:C', + name: '_phase:c', dependencies: { - self: ['_phase:A'] + self: ['_phase:a'] } } ] @@ -168,19 +198,19 @@ describe(CommandLineConfiguration.name, () => { summary: 'custom-phased', enableParallelism: true, safeForSimultaneousRushProcesses: false, - phases: ['_phase:A'] + phases: ['_phase:a'] } ], phases: [ { - name: '_phase:A' + name: '_phase:a' } ], parameters: [ { parameterKind: 'flag', longName: '--flag', - associatedPhases: ['_phase:A'], + associatedPhases: ['_phase:a'], description: 'flag' } ] @@ -202,25 +232,25 @@ describe(CommandLineConfiguration.name, () => { summary: 'custom-phased', enableParallelism: true, safeForSimultaneousRushProcesses: false, - phases: ['_phase:A'] + phases: ['_phase:a'] } ], phases: [ { - name: '_phase:A', + name: '_phase:a', dependencies: { - upstream: ['_phase:B'] + upstream: ['_phase:b'] } }, { - name: '_phase:B' + name: '_phase:b' } ], parameters: [ { parameterKind: 'flag', longName: '--flag', - associatedPhases: ['_phase:B'], + associatedPhases: ['_phase:b'], description: 'flag' } ] diff --git a/apps/rush-lib/src/api/test/__snapshots__/CommandLineConfiguration.test.ts.snap b/apps/rush-lib/src/api/test/__snapshots__/CommandLineConfiguration.test.ts.snap index 83e3d560365..55015a7af18 100644 --- a/apps/rush-lib/src/api/test/__snapshots__/CommandLineConfiguration.test.ts.snap +++ b/apps/rush-lib/src/api/test/__snapshots__/CommandLineConfiguration.test.ts.snap @@ -1,13 +1,19 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`CommandLineConfiguration Detects a cycle among phases 1`] = `"In command-line.json, there exists a cycle within the set of _phase:B dependencies: _phase:B, _phase:C, _phase:A"`; +exports[`CommandLineConfiguration Detects a cycle among phases 1`] = `"In command-line.json, there exists a cycle within the set of _phase:b dependencies: _phase:b, _phase:c, _phase:a"`; -exports[`CommandLineConfiguration Detects a missing phase 1`] = `"In command-line.json, in the \\"phases\\" property of the \\"example\\" command, the phase \\"_phase:A\\" does not exist."`; +exports[`CommandLineConfiguration Detects a missing phase 1`] = `"In command-line.json, in the \\"phases\\" property of the \\"example\\" command, the phase \\"_phase:a\\" does not exist."`; -exports[`CommandLineConfiguration Detects a missing phase dependency 1`] = `"In command-line.json, in the phase \\"_phase:A\\", the upstream dependency phase \\"_phase:B\\" does not exist."`; +exports[`CommandLineConfiguration Detects a missing phase dependency 1`] = `"In command-line.json, in the phase \\"_phase:a\\", the upstream dependency phase \\"_phase:b\\" does not exist."`; -exports[`CommandLineConfiguration Detects a missing phase dependency 2`] = `"In command-line.json, in the phase \\"_phase:A\\", the self dependency phase \\"_phase:B\\" does not exist."`; +exports[`CommandLineConfiguration Detects a missing phase dependency 2`] = `"In command-line.json, in the phase \\"_phase:a\\", the self dependency phase \\"_phase:b\\" does not exist."`; -exports[`CommandLineConfiguration Forbids a misnamed phase 1`] = `"In command-line.json, the phase \\"_faze:A\\"'s name does not begin with the required prefix \\"_phase:\\"."`; +exports[`CommandLineConfiguration Forbids a misnamed phase 1`] = `"In command-line.json, the phase \\"_faze:A\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`; -exports[`CommandLineConfiguration Forbids a misnamed phase 2`] = `"In command-line.json, the phase \\"_phase:\\"'s name must have characters after \\"_phase:\\""`; +exports[`CommandLineConfiguration Forbids a misnamed phase 2`] = `"In command-line.json, the phase \\"_phase:\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`; + +exports[`CommandLineConfiguration Forbids a misnamed phase 3`] = `"In command-line.json, the phase \\"_phase:0\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`; + +exports[`CommandLineConfiguration Forbids a misnamed phase 4`] = `"In command-line.json, the phase \\"_phase:A\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`; + +exports[`CommandLineConfiguration Forbids a misnamed phase 5`] = `"In command-line.json, the phase \\"_phase:A-\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`; From 9cf3437e4dd232fdd4e1f39865d9fdcb892d2261 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 29 Dec 2021 18:18:05 -0800 Subject: [PATCH 19/19] Rename a few maps to be more consistent. --- apps/rush-lib/src/api/CommandLineConfiguration.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/rush-lib/src/api/CommandLineConfiguration.ts b/apps/rush-lib/src/api/CommandLineConfiguration.ts index cfbede85e96..c258df0dd4e 100644 --- a/apps/rush-lib/src/api/CommandLineConfiguration.ts +++ b/apps/rush-lib/src/api/CommandLineConfiguration.ts @@ -125,12 +125,12 @@ export class CommandLineConfiguration { * dependent on it. This is used to determine which commands a phase affects, even * if that phase isn't explicitly listed for that command. */ - private readonly _relatedPhaseSets: Map> = new Map(); + private readonly _relatedPhaseSetsByPhaseName: Map> = new Map(); /** * A map of bulk command names to their corresponding synthetic phase identifiers */ - private readonly _syntheticPhasesForTranslatedBulkCommands: Map = new Map(); + private readonly _syntheticPhasesNamesByTranslatedBulkCommandName: Map = new Map(); /** * Use CommandLineConfiguration.loadFromFile() @@ -196,7 +196,7 @@ export class CommandLineConfiguration { this._checkForPhaseSelfCycles(phase); const relatedPhaseSet: Set = new Set(); this._populateRelatedPhaseSets(phase.name, relatedPhaseSet); - this._relatedPhaseSets.set(phase.name, relatedPhaseSet); + this._relatedPhaseSetsByPhaseName.set(phase.name, relatedPhaseSet); } let buildCommandPhases: string[] | undefined; @@ -393,7 +393,7 @@ export class CommandLineConfiguration { for (let i: number = 0; i < normalizedParameter.associatedCommands.length; i++) { const associatedCommandName: string = normalizedParameter.associatedCommands[i]; const syntheticPhaseName: string | undefined = - this._syntheticPhasesForTranslatedBulkCommands.get(associatedCommandName); + this._syntheticPhasesNamesByTranslatedBulkCommandName.get(associatedCommandName); if (syntheticPhaseName) { // If this parameter was associated with a bulk command, change the association to // the command's synthetic phase @@ -558,7 +558,7 @@ export class CommandLineConfiguration { } private _populateCommandsForPhase(phaseName: string, command: IPhasedCommand): void { - const populateRelatedPhaseSet: Set = this._relatedPhaseSets.get(phaseName)!; + const populateRelatedPhaseSet: Set = this._relatedPhaseSetsByPhaseName.get(phaseName)!; for (const relatedPhaseSetIdentifier of populateRelatedPhaseSet) { this._commandsByPhaseName.get(relatedPhaseSetIdentifier)!.add(command); } @@ -584,10 +584,10 @@ export class CommandLineConfiguration { logFilenameIdentifier: this._normalizeNameForLogFilenameIdentifiers(command.name) }; this.phases.set(phaseName, phaseForBulkCommand); - this._syntheticPhasesForTranslatedBulkCommands.set(command.name, phaseName); + this._syntheticPhasesNamesByTranslatedBulkCommandName.set(command.name, phaseName); const relatedPhaseSet: Set = new Set(); this._populateRelatedPhaseSets(phaseName, relatedPhaseSet); - this._relatedPhaseSets.set(phaseName, relatedPhaseSet); + this._relatedPhaseSetsByPhaseName.set(phaseName, relatedPhaseSet); const translatedCommand: IPhasedCommand = { ...command,