Merge pull request microsoft#3754 from yoyo930021/fix-upstream-phase

[rush-lib] Fix phase command to respect upstream
spring6225 · Mar 27, 2023 · 2f3aca6 · 2f3aca6
2 parents da214d0 + 6324ead
commit 2f3aca6
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 6 deletions.
diff --git a/common/changes/@microsoft/rush/fix-upstream-phase_2022-11-16-08-22.json b/common/changes/@microsoft/rush/fix-upstream-phase_2022-11-16-08-22.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Modify the scheduling behavior of phased commands to schedule only the expressly enumerated phases in all selected projects, adding additional phases only where needed to satisfy dependencies.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}
diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md
@@ -263,6 +263,7 @@ export interface ICreateOperationsContext {
     readonly isIncrementalBuildAllowed: boolean;
     readonly isInitial: boolean;
     readonly isWatch: boolean;
+    readonly phaseOriginal: ReadonlySet<IPhase>;
     readonly phaseSelection: ReadonlySet<IPhase>;
     readonly projectChangeAnalyzer: ProjectChangeAnalyzer;
     readonly projectSelection: ReadonlySet<RushConfigurationProject>;

diff --git a/libraries/rush-lib/src/api/CommandLineConfiguration.ts b/libraries/rush-lib/src/api/CommandLineConfiguration.ts
@@ -96,6 +96,10 @@ export interface IPhasedCommandConfig extends IPhasedCommandWithoutPhasesJson, I
   isSynthetic: boolean;
   disableBuildCache?: boolean;
 
+  originalPhases: Set<IPhase>;
+  /**
+   * Include upstream and self phases.
+   */
   phases: Set<IPhase>;
 
   /**
@@ -297,6 +301,7 @@ export class CommandLineConfiguration {
 
     const commandsJson: ICommandLineJson['commands'] = commandLineJson?.commands;
     let buildCommandPhases: IPhasedCommandConfig['phases'] | undefined;
+    let buildCommandOriginalPhases: IPhasedCommandConfig['phases'] | undefined;
     if (commandsJson) {
       for (const command of commandsJson) {
         if (this.commands.has(command.name)) {
@@ -309,13 +314,15 @@ export class CommandLineConfiguration {
         let normalizedCommand: Command;
         switch (command.commandKind) {
           case RushConstants.phasedCommandKind: {
+            const originalPhases: Set<IPhase> = new Set();
             const commandPhases: Set<IPhase> = new Set();
             const watchPhases: Set<IPhase> = new Set();
 
             normalizedCommand = {
               ...command,
               isSynthetic: false,
               associatedParameters: new Set<IParameterJson>(),
+              originalPhases,
               phases: commandPhases,
               watchPhases,
               alwaysWatch: false,
@@ -331,6 +338,7 @@ export class CommandLineConfiguration {
                 );
               }
 
+              originalPhases.add(phase);
               commandPhases.add(phase);
             }
 
@@ -407,6 +415,7 @@ export class CommandLineConfiguration {
           } else if (normalizedCommand.name === RushConstants.buildCommandName) {
             // Record the build command phases in case we need to construct a synthetic "rebuild" command
             buildCommandPhases = normalizedCommand.phases;
+            buildCommandOriginalPhases = normalizedCommand.originalPhases;
           }
         }
 
@@ -421,12 +430,13 @@ export class CommandLineConfiguration {
         buildCommand = this._translateBulkCommandToPhasedCommand(DEFAULT_BUILD_COMMAND_JSON);
         buildCommand.disableBuildCache = DEFAULT_BUILD_COMMAND_JSON.disableBuildCache;
         buildCommandPhases = buildCommand.phases;
+        buildCommandOriginalPhases = buildCommand.originalPhases;
         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) {
+        if (!buildCommandPhases || !buildCommandOriginalPhases) {
           throw new Error(`Phases for the "${RushConstants.buildCommandName}" were not found.`);
         }
 
@@ -437,6 +447,7 @@ export class CommandLineConfiguration {
           phases: buildCommandPhases,
           disableBuildCache: DEFAULT_REBUILD_COMMAND_JSON.disableBuildCache,
           associatedParameters: buildCommand.associatedParameters, // rebuild should share build's parameters in this case,
+          originalPhases: buildCommandOriginalPhases,
           watchPhases: new Set(),
           alwaysWatch: false,
           alwaysInstall: undefined
@@ -689,6 +700,7 @@ export class CommandLineConfiguration {
       isSynthetic: true,
       associatedParameters: new Set<IParameterJson>(),
       phases,
+      originalPhases: phases,
       // Bulk commands used the same phases for watch as for regular execution. Preserve behavior.
       watchPhases: command.watchForChanges ? phases : new Set(),
       alwaysWatch: !!command.watchForChanges,

diff --git a/libraries/rush-lib/src/cli/RushCommandLineParser.ts b/libraries/rush-lib/src/cli/RushCommandLineParser.ts
@@ -395,6 +395,7 @@ export class RushCommandLineParser extends CommandLineParser {
         disableBuildCache: command.disableBuildCache || false,
 
         initialPhases: command.phases,
+        originalPhases: command.originalPhases,
         watchPhases: command.watchPhases,
         watchDebounceMs: command.watchDebounceMs ?? RushConstants.defaultWatchDebounceMs,
         phases: commandLineConfiguration.phases,

diff --git a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts
@@ -47,6 +47,7 @@ export interface IPhasedScriptActionOptions extends IBaseScriptActionOptions<IPh
   incremental: boolean;
   disableBuildCache: boolean;
 
+  originalPhases: Set<IPhase>;
   initialPhases: Set<IPhase>;
   watchPhases: Set<IPhase>;
   phases: Map<string, IPhase>;
@@ -103,6 +104,7 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {
   private readonly _enableParallelism: boolean;
   private readonly _isIncrementalBuildAllowed: boolean;
   private readonly _disableBuildCache: boolean;
+  private readonly _originalPhases: ReadonlySet<IPhase>;
   private readonly _initialPhases: ReadonlySet<IPhase>;
   private readonly _watchPhases: ReadonlySet<IPhase>;
   private readonly _watchDebounceMs: number;
@@ -125,6 +127,7 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {
     this._enableParallelism = options.enableParallelism;
     this._isIncrementalBuildAllowed = options.incremental;
     this._disableBuildCache = options.disableBuildCache;
+    this._originalPhases = options.originalPhases;
     this._initialPhases = options.initialPhases;
     this._watchPhases = options.watchPhases;
     this._watchDebounceMs = options.watchDebounceMs ?? RushConstants.defaultWatchDebounceMs;
@@ -331,6 +334,7 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {
       isInitial: true,
       isWatch,
       rushConfiguration: this.rushConfiguration,
+      phaseOriginal: new Set(this._originalPhases),
       phaseSelection: new Set(this._initialPhases),
       projectChangeAnalyzer,
       projectSelection,
@@ -407,6 +411,7 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {
   private async _runWatchPhases(options: IRunPhasesOptions): Promise<void> {
     const { initialCreateOperationsContext, executionManagerOptions, stopwatch, terminal } = options;
 
+    const phaseOriginal: Set<IPhase> = new Set(this._watchPhases);
     const phaseSelection: Set<IPhase> = new Set(this._watchPhases);
 
     const { projectChangeAnalyzer: initialState, projectSelection: projectsToWatch } =
@@ -464,6 +469,7 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {
         isInitial: false,
         projectChangeAnalyzer: state,
         projectsInUnknownState: changedProjects,
+        phaseOriginal,
         phaseSelection
       };
 

diff --git a/libraries/rush-lib/src/logic/operations/PhasedOperationPlugin.ts b/libraries/rush-lib/src/logic/operations/PhasedOperationPlugin.ts
@@ -29,13 +29,18 @@ function createOperations(
   existingOperations: Set<Operation>,
   context: ICreateOperationsContext
 ): Set<Operation> {
-  const { projectsInUnknownState: changedProjects, phaseSelection, projectSelection } = context;
+  const {
+    projectsInUnknownState: changedProjects,
+    phaseOriginal,
+    phaseSelection,
+    projectSelection
+  } = context;
   const operationsWithWork: Set<Operation> = new Set();
 
   const operations: Map<string, Operation> = new Map();
 
   // Create tasks for selected phases and projects
-  for (const phase of phaseSelection) {
+  for (const phase of phaseOriginal) {
     for (const project of projectSelection) {
       getOrCreateOperation(phase, project);
     }

diff --git a/libraries/rush-lib/src/logic/operations/test/PhasedOperationPlugin.test.ts b/libraries/rush-lib/src/logic/operations/test/PhasedOperationPlugin.test.ts
@@ -68,8 +68,9 @@ describe(PhasedOperationPlugin.name, () => {
 
     const context: Pick<
       ICreateOperationsContext,
-      'phaseSelection' | 'projectSelection' | 'projectsInUnknownState'
+      'phaseOriginal' | 'phaseSelection' | 'projectSelection' | 'projectsInUnknownState'
     > = {
+      phaseOriginal: phaseSelection,
       phaseSelection,
       projectSelection,
       projectsInUnknownState: changedProjects

diff --git a/libraries/rush-lib/src/logic/operations/test/ShellOperationRunnerPlugin.test.ts b/libraries/rush-lib/src/logic/operations/test/ShellOperationRunnerPlugin.test.ts
@@ -44,8 +44,9 @@ describe(ShellOperationRunnerPlugin.name, () => {
 
     const fakeCreateOperationsContext: Pick<
       ICreateOperationsContext,
-      'phaseSelection' | 'projectSelection' | 'projectsInUnknownState'
+      'phaseOriginal' | 'phaseSelection' | 'projectSelection' | 'projectsInUnknownState'
     > = {
+      phaseOriginal: echoCommand.phases,
       phaseSelection: echoCommand.phases,
       projectSelection: new Set(rushConfiguration.projects),
       projectsInUnknownState: new Set(rushConfiguration.projects)
@@ -86,8 +87,9 @@ describe(ShellOperationRunnerPlugin.name, () => {
 
     const fakeCreateOperationsContext: Pick<
       ICreateOperationsContext,
-      'phaseSelection' | 'projectSelection' | 'projectsInUnknownState'
+      'phaseOriginal' | 'phaseSelection' | 'projectSelection' | 'projectsInUnknownState'
     > = {
+      phaseOriginal: echoCommand.phases,
       phaseSelection: echoCommand.phases,
       projectSelection: new Set(rushConfiguration.projects),
       projectsInUnknownState: new Set(rushConfiguration.projects)

diff --git a/libraries/rush-lib/src/pluginFramework/PhasedCommandHooks.ts b/libraries/rush-lib/src/pluginFramework/PhasedCommandHooks.ts
@@ -52,6 +52,10 @@ export interface ICreateOperationsContext {
    * If true, the command is running in watch mode.
    */
   readonly isWatch: boolean;
+  /**
+   * The set of phases original for the current command execution.
+   */
+  readonly phaseOriginal: ReadonlySet<IPhase>;
   /**
    * The set of phases selected for the current command execution.
    */