Skip to content

Commit 97fda75

Browse files
authored
Merge pull request #4750 from aramissennyeydd/clustering-fixes
[rush]: fix operation clustering for shards and no-ops
2 parents 8375171 + 55b0f7d commit 97fda75

File tree

5 files changed

+61
-15
lines changed

5 files changed

+61
-15
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": "Fixes build cache no-op and sharded operation clustering.",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}

common/reviews/api/rush-lib.api.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,6 +1398,12 @@ export class RushLifecycleHooks {
13981398
export class RushProjectConfiguration {
13991399
readonly disableBuildCacheForProject: boolean;
14001400
getCacheDisabledReason(trackedFileNames: Iterable<string>, phaseName: string, isNoOp: boolean): string | undefined;
1401+
static getCacheDisabledReasonForProject(options: {
1402+
projectConfiguration: RushProjectConfiguration | undefined;
1403+
trackedFileNames: Iterable<string>;
1404+
phaseName: string;
1405+
isNoOp: boolean;
1406+
}): string | undefined;
14011407
readonly incrementalBuildIgnoredGlobs: ReadonlyArray<string>;
14021408
// (undocumented)
14031409
readonly operationSettingsByOperationName: ReadonlyMap<string, Readonly<IOperationSettings>>;

libraries/rush-lib/src/api/RushProjectConfiguration.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,32 @@ export class RushProjectConfiguration {
391391
}
392392
}
393393

394+
/**
395+
* Source of truth for whether a project is unable to use the build cache for a given phase.
396+
* As some operations may not have a rush-project.json file defined at all, but may be no-op operations
397+
* we'll want to ignore those completely.
398+
*/
399+
public static getCacheDisabledReasonForProject(options: {
400+
projectConfiguration: RushProjectConfiguration | undefined;
401+
trackedFileNames: Iterable<string>;
402+
phaseName: string;
403+
isNoOp: boolean;
404+
}): string | undefined {
405+
const { projectConfiguration, trackedFileNames, phaseName, isNoOp } = options;
406+
if (isNoOp) {
407+
return undefined;
408+
}
409+
410+
if (!projectConfiguration) {
411+
return (
412+
`Project does not have a ${RushConstants.rushProjectConfigFilename} configuration file, ` +
413+
'or one provided by a rig, so it does not support caching.'
414+
);
415+
}
416+
417+
return projectConfiguration.getCacheDisabledReason(trackedFileNames, phaseName, isNoOp);
418+
}
419+
394420
/**
395421
* Loads the rush-project.json data for the specified project.
396422
*/

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ import type { Operation } from './Operation';
1111
import { clusterOperations, type IOperationBuildCacheContext } from './CacheableOperationPlugin';
1212
import { DisjointSet } from '../cobuild/DisjointSet';
1313
import type { IOperationExecutionResult } from './IOperationExecutionResult';
14-
import { RushConstants } from '../RushConstants';
15-
import type { RushProjectConfiguration } from '../../api/RushProjectConfiguration';
14+
import { RushProjectConfiguration } from '../../api/RushProjectConfiguration';
1615

1716
const PLUGIN_NAME: 'BuildPlanPlugin' = 'BuildPlanPlugin';
1817

@@ -64,14 +63,16 @@ export class BuildPlanPlugin implements IPhasedCommandPlugin {
6463
projectConfigurations.get(associatedProject);
6564
const fileHashes: Map<string, string> | undefined =
6665
await projectChangeAnalyzer._tryGetProjectDependenciesAsync(associatedProject, terminal);
67-
const cacheDisabledReason: string | undefined = projectConfiguration
68-
? projectConfiguration.getCacheDisabledReason(
69-
fileHashes!.keys(),
70-
associatedPhase.name,
71-
operation.isNoOp
72-
)
73-
: `Project does not have a ${RushConstants.rushProjectConfigFilename} configuration file, ` +
74-
'or one provided by a rig, so it does not support caching.';
66+
if (!fileHashes) {
67+
continue;
68+
}
69+
const cacheDisabledReason: string | undefined =
70+
RushProjectConfiguration.getCacheDisabledReasonForProject({
71+
projectConfiguration,
72+
trackedFileNames: fileHashes.keys(),
73+
isNoOp: operation.isNoOp,
74+
phaseName: associatedPhase.name
75+
});
7576
buildCacheByOperation.set(operation, { cacheDisabledReason });
7677
}
7778
}

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { OperationStatus } from './OperationStatus';
1212
import { CobuildLock, type ICobuildCompletedState } from '../cobuild/CobuildLock';
1313
import { ProjectBuildCache } from '../buildCache/ProjectBuildCache';
1414
import { RushConstants } from '../RushConstants';
15-
import type { IOperationSettings, RushProjectConfiguration } from '../../api/RushProjectConfiguration';
15+
import { type IOperationSettings, RushProjectConfiguration } from '../../api/RushProjectConfiguration';
1616
import { getHashesForGlobsAsync } from '../buildCache/getHashesForGlobsAsync';
1717
import { ProjectLogWritable } from './ProjectLogWritable';
1818
import type { CobuildConfiguration } from '../../api/CobuildConfiguration';
@@ -123,10 +123,13 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin {
123123
);
124124
}
125125

126-
const cacheDisabledReason: string | undefined = projectConfiguration
127-
? projectConfiguration.getCacheDisabledReason(fileHashes.keys(), phaseName, operation.isNoOp)
128-
: `Project does not have a ${RushConstants.rushProjectConfigFilename} configuration file, ` +
129-
'or one provided by a rig, so it does not support caching.';
126+
const cacheDisabledReason: string | undefined =
127+
RushProjectConfiguration.getCacheDisabledReasonForProject({
128+
projectConfiguration,
129+
phaseName: phaseName,
130+
isNoOp: operation.isNoOp,
131+
trackedFileNames: fileHashes.keys()
132+
});
130133

131134
disjointSet?.add(operation);
132135

0 commit comments

Comments
 (0)