Skip to content

Commit 33db957

Browse files
[rush] Fix bug including unnecessary operations (#5141)
* [rush] Fix bug including unnecessary operations * Reword message. Co-authored-by: Pete Gonzalez <[email protected]> --------- Co-authored-by: David Michon <[email protected]> Co-authored-by: Pete Gonzalez <[email protected]>
1 parent d6d0d8b commit 33db957

File tree

3 files changed

+50
-19
lines changed

3 files changed

+50
-19
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "Fix an issue where `--include-phase-deps` and watch mode sometimes included operations that were not required",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}

libraries/rush-lib/src/logic/operations/PhasedOperationPlugin.ts

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,29 +37,61 @@ function createOperations(
3737
includePhaseDeps,
3838
isInitial
3939
} = context;
40-
const operationsWithWork: Set<Operation> = new Set();
4140

4241
const operations: Map<string, Operation> = new Map();
4342

4443
// Create tasks for selected phases and projects
44+
// This also creates the minimal set of dependencies needed
4545
for (const phase of phaseOriginal) {
4646
for (const project of projectSelection) {
4747
getOrCreateOperation(phase, project);
4848
}
4949
}
5050

51-
// Recursively expand all consumers in the `operationsWithWork` set.
51+
// Grab all operations that were explicitly requested.
52+
const operationsWithWork: Set<Operation> = new Set();
53+
for (const operation of existingOperations) {
54+
const { associatedPhase, associatedProject } = operation;
55+
if (!associatedPhase || !associatedProject) {
56+
// Fix this when these are required properties.
57+
continue;
58+
}
59+
60+
if (phaseSelection.has(associatedPhase) && changedProjects.has(associatedProject)) {
61+
operationsWithWork.add(operation);
62+
}
63+
}
64+
65+
// Add all operations that are selected that depend on the explicitly requested operations.
66+
// This will mostly be relevant during watch; in initial runs it should not add any new operations.
5267
for (const operation of operationsWithWork) {
5368
for (const consumer of operation.consumers) {
5469
operationsWithWork.add(consumer);
5570
}
5671
}
5772

58-
for (const operation of operations.values()) {
59-
if (!operationsWithWork.has(operation)) {
60-
// This operation is in scope, but did not change since it was last executed by the current command.
61-
// However, we have no state tracking across executions, so treat as unknown.
62-
operation.enabled = false;
73+
if (includePhaseDeps && isInitial) {
74+
// Add all operations that are dependencies of the operations already scheduled.
75+
for (const operation of operationsWithWork) {
76+
for (const dependency of operation.dependencies) {
77+
operationsWithWork.add(dependency);
78+
}
79+
}
80+
}
81+
82+
for (const operation of existingOperations) {
83+
// Enable exactly the set of operations that are requested.
84+
operation.enabled &&= operationsWithWork.has(operation);
85+
86+
if (!includePhaseDeps || !isInitial) {
87+
const { associatedPhase, associatedProject } = operation;
88+
if (!associatedPhase || !associatedProject) {
89+
// Fix this when these are required properties.
90+
continue;
91+
}
92+
93+
// This filter makes the "unsafe" selections happen.
94+
operation.enabled &&= phaseSelection.has(associatedPhase) && projectSelection.has(associatedProject);
6395
}
6496
}
6597

@@ -86,17 +118,6 @@ function createOperations(
86118
logFilenameIdentifier: logFilenameIdentifier
87119
});
88120

89-
if (!phaseSelection.has(phase) || !projectSelection.has(project)) {
90-
if (includePhaseDeps && isInitial) {
91-
operationsWithWork.add(operation);
92-
} else {
93-
// Not in scope. Mark disabled, which will report as OperationStatus.Skipped.
94-
operation.enabled = false;
95-
}
96-
} else if (changedProjects.has(project)) {
97-
operationsWithWork.add(operation);
98-
}
99-
100121
operations.set(key, operation);
101122
existingOperations.add(operation);
102123

libraries/rush-lib/src/logic/operations/test/PhasedOperationPlugin.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import path from 'path';
55
import { JsonFile } from '@rushstack/node-core-library';
66

77
import { RushConfiguration } from '../../../api/RushConfiguration';
8+
import type { RushConfigurationProject } from '../../../api/RushConfigurationProject';
89
import {
910
CommandLineConfiguration,
1011
type IPhase,
@@ -19,7 +20,6 @@ import {
1920
type ICreateOperationsContext,
2021
PhasedCommandHooks
2122
} from '../../../pluginFramework/PhasedCommandHooks';
22-
import type { RushConfigurationProject } from '../../..';
2323

2424
interface ISerializedOperation {
2525
name: string;

0 commit comments

Comments
 (0)