Skip to content

Commit 817e924

Browse files
Copilotdmichon-msfticlanton
authored
[rush] Add project-level parameter ignoring to prevent unnecessary cache invalidation (#5433)
* Initial plan * Add parameter ignoring feature to rush-lib Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Add tests for parameter ignoring feature Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Refactor to eliminate code duplication in parameter collection Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Improve documentation and add clarifying comments Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Address PR feedback: use ignoredParameterValues and improve filtering Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Address final PR feedback: improve logging and test coverage Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Centralize parameter creation helper and share with BaseScriptAction Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Ensure all parameter types are tested for filtering Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Refactor parameter helpers into cli/parsing directory Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Use CommandLineParser to parse parameter values in test Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Use executeWithoutErrorHandlingAsync instead of executeAsync Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> * Add rush change file for project-level parameter ignoring feature Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> * Adjust formatting of change file --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> Co-authored-by: David Michon <dmichon@microsoft.com>
1 parent ef8e24d commit 817e924

21 files changed

Lines changed: 564 additions & 120 deletions
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"comment": "Add project-level parameter ignoring to prevent unnecessary cache invalidation. Projects can now use \"parameterNamesToIgnore\" in \"rush-project.json\" to exclude custom command-line parameters that don't affect their operations.",
5+
"type": "minor",
6+
"packageName": "@microsoft/rush"
7+
}
8+
],
9+
"packageName": "@microsoft/rush",
10+
"email": "198982749+Copilot@users.noreply.github.com"
11+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,7 @@ export interface IOperationSettings {
670670
ignoreChangedProjectsOnlyFlag?: boolean;
671671
operationName: string;
672672
outputFolderNames?: string[];
673+
parameterNamesToIgnore?: string[];
673674
sharding?: IRushPhaseSharding;
674675
weight?: number;
675676
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,14 @@ export interface IOperationSettings {
146146
* If true, this operation will never be skipped by the `--changed-projects-only` flag.
147147
*/
148148
ignoreChangedProjectsOnlyFlag?: boolean;
149+
150+
/**
151+
* An optional list of custom command-line parameter names (their `parameterLongName` values from
152+
* command-line.json) that should be ignored when invoking the command for this operation.
153+
* This allows a project to opt out of parameters that don't affect its operation, preventing
154+
* unnecessary cache invalidation for this operation and its consumers.
155+
*/
156+
parameterNamesToIgnore?: string[];
149157
}
150158

151159
interface IOldRushProjectJson {
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import { InternalError } from '@rushstack/node-core-library';
5+
import type { CommandLineParameter } from '@rushstack/ts-command-line';
6+
7+
import type { IParameterJson, IPhase } from '../../api/CommandLineConfiguration';
8+
9+
/**
10+
* Associates command line parameters with their associated phases.
11+
* This helper is used to populate the `associatedParameters` set on each phase
12+
* based on the `associatedPhases` property of each parameter.
13+
*
14+
* @param customParameters - Map of parameter definitions to their CommandLineParameter instances
15+
* @param knownPhases - Map of phase names to IPhase objects
16+
*/
17+
export function associateParametersByPhase(
18+
customParameters: ReadonlyMap<IParameterJson, CommandLineParameter>,
19+
knownPhases: ReadonlyMap<string, IPhase>
20+
): void {
21+
for (const [parameterJson, tsCommandLineParameter] of customParameters) {
22+
if (parameterJson.associatedPhases) {
23+
for (const phaseName of parameterJson.associatedPhases) {
24+
const phase: IPhase | undefined = knownPhases.get(phaseName);
25+
if (!phase) {
26+
throw new InternalError(`Could not find a phase matching ${phaseName}.`);
27+
}
28+
phase.associatedParameters.add(tsCommandLineParameter);
29+
}
30+
}
31+
}
32+
}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import type { CommandLineAction, CommandLineParameter } from '@rushstack/ts-command-line';
5+
6+
import type { IParameterJson } from '../../api/CommandLineConfiguration';
7+
import { RushConstants } from '../../logic/RushConstants';
8+
import type { ParameterJson } from '../../api/CommandLineJson';
9+
10+
/**
11+
* Helper function to create CommandLineParameter instances from parameter definitions.
12+
* This centralizes the logic for defining parameters based on their kind.
13+
*
14+
* @param action - The CommandLineAction to define the parameters on
15+
* @param associatedParameters - The set of parameter definitions
16+
* @param targetMap - The map to populate with parameter definitions to CommandLineParameter instances
17+
*/
18+
export function defineCustomParameters(
19+
action: CommandLineAction,
20+
associatedParameters: Iterable<IParameterJson>,
21+
targetMap: Map<IParameterJson, CommandLineParameter>
22+
): void {
23+
for (const parameter of associatedParameters) {
24+
let tsCommandLineParameter: CommandLineParameter | undefined;
25+
26+
switch (parameter.parameterKind) {
27+
case 'flag':
28+
tsCommandLineParameter = action.defineFlagParameter({
29+
parameterShortName: parameter.shortName,
30+
parameterLongName: parameter.longName,
31+
description: parameter.description,
32+
required: parameter.required
33+
});
34+
break;
35+
case 'choice':
36+
tsCommandLineParameter = action.defineChoiceParameter({
37+
parameterShortName: parameter.shortName,
38+
parameterLongName: parameter.longName,
39+
description: parameter.description,
40+
required: parameter.required,
41+
alternatives: parameter.alternatives.map((x) => x.name),
42+
defaultValue: parameter.defaultValue
43+
});
44+
break;
45+
case 'string':
46+
tsCommandLineParameter = action.defineStringParameter({
47+
parameterLongName: parameter.longName,
48+
parameterShortName: parameter.shortName,
49+
description: parameter.description,
50+
required: parameter.required,
51+
argumentName: parameter.argumentName
52+
});
53+
break;
54+
case 'integer':
55+
tsCommandLineParameter = action.defineIntegerParameter({
56+
parameterLongName: parameter.longName,
57+
parameterShortName: parameter.shortName,
58+
description: parameter.description,
59+
required: parameter.required,
60+
argumentName: parameter.argumentName
61+
});
62+
break;
63+
case 'stringList':
64+
tsCommandLineParameter = action.defineStringListParameter({
65+
parameterLongName: parameter.longName,
66+
parameterShortName: parameter.shortName,
67+
description: parameter.description,
68+
required: parameter.required,
69+
argumentName: parameter.argumentName
70+
});
71+
break;
72+
case 'integerList':
73+
tsCommandLineParameter = action.defineIntegerListParameter({
74+
parameterLongName: parameter.longName,
75+
parameterShortName: parameter.shortName,
76+
description: parameter.description,
77+
required: parameter.required,
78+
argumentName: parameter.argumentName
79+
});
80+
break;
81+
case 'choiceList':
82+
tsCommandLineParameter = action.defineChoiceListParameter({
83+
parameterShortName: parameter.shortName,
84+
parameterLongName: parameter.longName,
85+
description: parameter.description,
86+
required: parameter.required,
87+
alternatives: parameter.alternatives.map((x) => x.name)
88+
});
89+
break;
90+
default:
91+
throw new Error(
92+
`${RushConstants.commandLineFilename} defines a parameter "${
93+
(parameter as ParameterJson).longName
94+
}" using an unsupported parameter kind "${(parameter as ParameterJson).parameterKind}"`
95+
);
96+
}
97+
98+
targetMap.set(parameter, tsCommandLineParameter);
99+
}
100+
}

libraries/rush-lib/src/cli/scriptActions/BaseScriptAction.ts

Lines changed: 3 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ import type { CommandLineParameter } from '@rushstack/ts-command-line';
55

66
import { BaseRushAction, type IBaseRushActionOptions } from '../actions/BaseRushAction';
77
import type { Command, CommandLineConfiguration, IParameterJson } from '../../api/CommandLineConfiguration';
8-
import { RushConstants } from '../../logic/RushConstants';
9-
import type { ParameterJson } from '../../api/CommandLineJson';
8+
import { defineCustomParameters } from '../parsing/defineCustomParameters';
109

1110
/**
1211
* Constructor parameters for BaseScriptAction
@@ -42,83 +41,7 @@ export abstract class BaseScriptAction<TCommand extends Command> extends BaseRus
4241
return;
4342
}
4443

45-
// Find any parameters that are associated with this command
46-
for (const parameter of this.command.associatedParameters) {
47-
let tsCommandLineParameter: CommandLineParameter | undefined;
48-
49-
switch (parameter.parameterKind) {
50-
case 'flag':
51-
tsCommandLineParameter = this.defineFlagParameter({
52-
parameterShortName: parameter.shortName,
53-
parameterLongName: parameter.longName,
54-
description: parameter.description,
55-
required: parameter.required
56-
});
57-
break;
58-
case 'choice':
59-
tsCommandLineParameter = this.defineChoiceParameter({
60-
parameterShortName: parameter.shortName,
61-
parameterLongName: parameter.longName,
62-
description: parameter.description,
63-
required: parameter.required,
64-
alternatives: parameter.alternatives.map((x) => x.name),
65-
defaultValue: parameter.defaultValue
66-
});
67-
break;
68-
case 'string':
69-
tsCommandLineParameter = this.defineStringParameter({
70-
parameterLongName: parameter.longName,
71-
parameterShortName: parameter.shortName,
72-
description: parameter.description,
73-
required: parameter.required,
74-
argumentName: parameter.argumentName
75-
});
76-
break;
77-
case 'integer':
78-
tsCommandLineParameter = this.defineIntegerParameter({
79-
parameterLongName: parameter.longName,
80-
parameterShortName: parameter.shortName,
81-
description: parameter.description,
82-
required: parameter.required,
83-
argumentName: parameter.argumentName
84-
});
85-
break;
86-
case 'stringList':
87-
tsCommandLineParameter = this.defineStringListParameter({
88-
parameterLongName: parameter.longName,
89-
parameterShortName: parameter.shortName,
90-
description: parameter.description,
91-
required: parameter.required,
92-
argumentName: parameter.argumentName
93-
});
94-
break;
95-
case 'integerList':
96-
tsCommandLineParameter = this.defineIntegerListParameter({
97-
parameterLongName: parameter.longName,
98-
parameterShortName: parameter.shortName,
99-
description: parameter.description,
100-
required: parameter.required,
101-
argumentName: parameter.argumentName
102-
});
103-
break;
104-
case 'choiceList':
105-
tsCommandLineParameter = this.defineChoiceListParameter({
106-
parameterShortName: parameter.shortName,
107-
parameterLongName: parameter.longName,
108-
description: parameter.description,
109-
required: parameter.required,
110-
alternatives: parameter.alternatives.map((x) => x.name)
111-
});
112-
break;
113-
default:
114-
throw new Error(
115-
`${RushConstants.commandLineFilename} defines a parameter "${
116-
(parameter as ParameterJson).longName
117-
}" using an unsupported parameter kind "${(parameter as ParameterJson).parameterKind}"`
118-
);
119-
}
120-
121-
this.customParameters.set(parameter, tsCommandLineParameter);
122-
}
44+
// Use the centralized helper to create CommandLineParameter instances
45+
defineCustomParameters(this, this.command.associatedParameters, this.customParameters);
12346
}
12447
}

libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import type { AsyncSeriesHook } from 'tapable';
55

6-
import { AlreadyReportedError, InternalError } from '@rushstack/node-core-library';
6+
import { AlreadyReportedError } from '@rushstack/node-core-library';
77
import { type ITerminal, Terminal, Colorize } from '@rushstack/terminal';
88
import type {
99
CommandLineFlagParameter,
@@ -33,6 +33,7 @@ import { SelectionParameterSet } from '../parsing/SelectionParameterSet';
3333
import type { IPhase, IPhasedCommandConfig } from '../../api/CommandLineConfiguration';
3434
import type { Operation } from '../../logic/operations/Operation';
3535
import type { OperationExecutionRecord } from '../../logic/operations/OperationExecutionRecord';
36+
import { associateParametersByPhase } from '../parsing/associateParametersByPhase';
3637
import { PhasedOperationPlugin } from '../../logic/operations/PhasedOperationPlugin';
3738
import { ShellOperationRunnerPlugin } from '../../logic/operations/ShellOperationRunnerPlugin';
3839
import { Event } from '../../api/EventHooks';
@@ -327,17 +328,8 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> i
327328

328329
this.defineScriptParameters();
329330

330-
for (const [{ associatedPhases }, tsCommandLineParameter] of this.customParameters) {
331-
if (associatedPhases) {
332-
for (const phaseName of associatedPhases) {
333-
const phase: IPhase | undefined = this._knownPhases.get(phaseName);
334-
if (!phase) {
335-
throw new InternalError(`Could not find a phase matching ${phaseName}.`);
336-
}
337-
phase.associatedParameters.add(tsCommandLineParameter);
338-
}
339-
}
340-
}
331+
// Associate parameters with their respective phases
332+
associateParametersByPhase(this.customParameters, this._knownPhases);
341333
}
342334

343335
public async runAsync(): Promise<void> {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export interface IIPCOperationRunnerOptions {
3030
commandForHash: string;
3131
persist: boolean;
3232
requestRun: OperationRequestRunCallback;
33+
ignoredParameterValues: ReadonlyArray<string>;
3334
}
3435

3536
function isAfterExecuteEventMessage(message: unknown): message is IAfterExecuteEventMessage {
@@ -59,6 +60,7 @@ export class IPCOperationRunner implements IOperationRunner {
5960
private readonly _commandForHash: string;
6061
private readonly _persist: boolean;
6162
private readonly _requestRun: OperationRequestRunCallback;
63+
private readonly _ignoredParameterValues: ReadonlyArray<string>;
6264

6365
private _ipcProcess: ChildProcess | undefined;
6466
private _processReadyPromise: Promise<void> | undefined;
@@ -75,13 +77,21 @@ export class IPCOperationRunner implements IOperationRunner {
7577

7678
this._persist = options.persist;
7779
this._requestRun = options.requestRun;
80+
this._ignoredParameterValues = options.ignoredParameterValues;
7881
}
7982

8083
public async executeAsync(context: IOperationRunnerContext): Promise<OperationStatus> {
8184
return await context.runWithTerminalAsync(
8285
async (terminal: ITerminal, terminalProvider: ITerminalProvider): Promise<OperationStatus> => {
8386
let isConnected: boolean = false;
8487
if (!this._ipcProcess || typeof this._ipcProcess.exitCode === 'number') {
88+
// Log any ignored parameters
89+
if (this._ignoredParameterValues.length > 0) {
90+
terminal.writeLine(
91+
`These parameters were ignored for this operation by project-level configuration: ${this._ignoredParameterValues.join(' ')}`
92+
);
93+
}
94+
8595
// Run the operation
8696
terminal.writeLine('Invoking: ' + this._commandToRun);
8797

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
22
// See LICENSE in the project root for license information.
33

4-
import type { IPhase } from '../../api/CommandLineConfiguration';
54
import type {
65
ICreateOperationsContext,
76
IPhasedCommandPlugin,
@@ -14,7 +13,8 @@ import { OperationStatus } from './OperationStatus';
1413
import {
1514
PLUGIN_NAME as ShellOperationPluginName,
1615
formatCommand,
17-
getCustomParameterValuesByPhase,
16+
getCustomParameterValuesByOperation,
17+
type ICustomParameterValuesForOperation,
1818
getDisplayName
1919
} from './ShellOperationRunnerPlugin';
2020

@@ -45,8 +45,8 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin {
4545

4646
currentContext = context;
4747

48-
const getCustomParameterValuesForPhase: (phase: IPhase) => ReadonlyArray<string> =
49-
getCustomParameterValuesByPhase();
48+
const getCustomParameterValues: (operation: Operation) => ICustomParameterValuesForOperation =
49+
getCustomParameterValuesByOperation();
5050

5151
for (const operation of operations) {
5252
const { associatedPhase: phase, associatedProject: project, runner } = operation;
@@ -73,7 +73,8 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin {
7373
// for this operation (or downstream operations) to be restored from the build cache.
7474
const commandForHash: string | undefined = phase.shellCommand ?? scripts?.[phaseName];
7575

76-
const customParameterValues: ReadonlyArray<string> = getCustomParameterValuesForPhase(phase);
76+
const { parameterValues: customParameterValues, ignoredParameterValues } =
77+
getCustomParameterValues(operation);
7778
const commandToRun: string = formatCommand(rawScript, customParameterValues);
7879

7980
const operationName: string = getDisplayName(phase, project);
@@ -86,6 +87,7 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin {
8687
commandToRun,
8788
commandForHash,
8889
persist: true,
90+
ignoredParameterValues,
8991
requestRun: (requestor: string, detail?: string) => {
9092
const operationState: IOperationExecutionResult | undefined =
9193
operationStatesByRunner.get(ipcOperationRunner);

0 commit comments

Comments
 (0)