From f950fd76e9ad27325febef10d702a75e92a15625 Mon Sep 17 00:00:00 2001 From: David Michon Date: Thu, 3 Oct 2024 01:47:35 +0000 Subject: [PATCH 1/3] [rush] Support `:incremental` scripts --- .../script-priority_2024-10-03-01-47.json | 10 ++++ .../operations/IPCOperationRunnerPlugin.ts | 20 +++++-- .../operations/ShardedPhaseOperationPlugin.ts | 23 ++++---- .../logic/operations/ShellOperationRunner.ts | 5 +- .../operations/ShellOperationRunnerPlugin.ts | 54 ++++++++++--------- .../src/schemas/command-line.schema.json | 2 +- 6 files changed, 68 insertions(+), 46 deletions(-) create mode 100644 common/changes/@microsoft/rush/script-priority_2024-10-03-01-47.json diff --git a/common/changes/@microsoft/rush/script-priority_2024-10-03-01-47.json b/common/changes/@microsoft/rush/script-priority_2024-10-03-01-47.json new file mode 100644 index 00000000000..2839e79a2ab --- /dev/null +++ b/common/changes/@microsoft/rush/script-priority_2024-10-03-01-47.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Changes the behavior of phased commands in watch mode to, when running a phase `_phase:` in all iterations after the first, prefer a script entry named `_phase::incremental` if such a script exists. The build cache will expect the outputs from the corresponding `_phase:` script (with otherwise the same inputs) to be equivalent when looking for a cache hit.", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts index 6db9afaecfd..71593606695 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts @@ -38,7 +38,11 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin { before: ShellOperationPluginName }, async (operations: Set, context: ICreateOperationsContext) => { - const { isWatch } = context; + const { isWatch, isInitial } = context; + if (!isWatch) { + return operations; + } + currentContext = context; const getCustomParameterValuesForPhase: (phase: IPhase) => ReadonlyArray = @@ -51,12 +55,20 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin { continue; } - const rawScript: string | undefined = project.packageJson.scripts?.[`${phase.name}:ipc`]; + const { name: phaseName } = phase; + + const { scripts } = project.packageJson; + const rawScript: string | undefined = [ + !isInitial && scripts?.[`${phaseName}:incremental:ipc`], + scripts?.[`${phaseName}:ipc`] + ].find((x): x is string => typeof x === 'string'); + if (!rawScript) { continue; } - const commandToRun: string = formatCommand(rawScript, getCustomParameterValuesForPhase(phase)); + const customParameterValues: ReadonlyArray = getCustomParameterValuesForPhase(phase); + const commandToRun: string = formatCommand(rawScript, customParameterValues); const operationName: string = getDisplayName(phase, project); let maybeIpcOperationRunner: IPCOperationRunner | undefined = runnerCache.get(operationName); @@ -66,7 +78,7 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin { project, name: operationName, shellCommand: commandToRun, - persist: isWatch, + persist: true, requestRun: (requestor?: string) => { const operationState: IOperationExecutionResult | undefined = operationStatesByRunner.get(ipcOperationRunner); diff --git a/libraries/rush-lib/src/logic/operations/ShardedPhaseOperationPlugin.ts b/libraries/rush-lib/src/logic/operations/ShardedPhaseOperationPlugin.ts index 81ff503b294..0d88685473e 100644 --- a/libraries/rush-lib/src/logic/operations/ShardedPhaseOperationPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/ShardedPhaseOperationPlugin.ts @@ -13,10 +13,8 @@ import { NullOperationRunner } from './NullOperationRunner'; import { Operation } from './Operation'; import { OperationStatus } from './OperationStatus'; import { - formatCommand, getCustomParameterValuesByPhase, getDisplayName, - getScriptToRun, initializeShellOperationRunner } from './ShellOperationRunnerPlugin'; @@ -129,11 +127,10 @@ function spliceShards(existingOperations: Set, context: ICreateOperat `--shard-count="${shards}"` ]; - const rawCommandToRun: string | undefined = getScriptToRun(project, phase.name, phase.shellCommand); - - const commandToRun: string | undefined = rawCommandToRun - ? formatCommand(rawCommandToRun, collatorParameters) - : undefined; + const { scripts } = project.packageJson; + const commandToRun: string | undefined = [phase.shellCommand, scripts?.[phase.name]].find( + (x) => typeof x === 'string' + ); operation.logFilenameIdentifier = `${baseLogFilenameIdentifier}_collate`; operation.runner = initializeShellOperationRunner({ @@ -141,11 +138,12 @@ function spliceShards(existingOperations: Set, context: ICreateOperat project, displayName: collatorDisplayName, rushConfiguration, - commandToRun: commandToRun + commandToRun, + customParameterValues: collatorParameters }); const shardOperationName: string = `${phase.name}:shard`; - const baseCommand: string | undefined = getScriptToRun(project, shardOperationName, undefined); + const baseCommand: string | undefined = scripts?.[shardOperationName]; if (baseCommand === undefined) { throw new Error( `The project '${project.packageName}' does not define a '${phase.name}:shard' command in the 'scripts' section of its package.json` @@ -205,14 +203,11 @@ function spliceShards(existingOperations: Set, context: ICreateOperat const shardDisplayName: string = `${getDisplayName(phase, project)} - shard ${shard}/${shards}`; - const shardedCommandToRun: string | undefined = baseCommand - ? formatCommand(baseCommand, shardedParameters) - : undefined; - shardOperation.runner = initializeShellOperationRunner({ phase, project, - commandToRun: shardedCommandToRun, + commandToRun: baseCommand, + customParameterValues: shardedParameters, displayName: shardDisplayName, rushConfiguration }); diff --git a/libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts b/libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts index 4585a47382e..e2576b17def 100644 --- a/libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts @@ -19,6 +19,7 @@ export interface IOperationRunnerOptions { rushProject: RushConfigurationProject; rushConfiguration: RushConfiguration; commandToRun: string; + commandForHash: string; displayName: string; phase: IPhase; environment?: IEnvironment; @@ -38,6 +39,7 @@ export class ShellOperationRunner implements IOperationRunner { public readonly warningsAreAllowed: boolean; private readonly _commandToRun: string; + private readonly _commandForHash: string; private readonly _rushProject: RushConfigurationProject; private readonly _rushConfiguration: RushConfiguration; @@ -53,6 +55,7 @@ export class ShellOperationRunner implements IOperationRunner { this._rushProject = options.rushProject; this._rushConfiguration = options.rushConfiguration; this._commandToRun = options.commandToRun; + this._commandForHash = options.commandForHash; this._environment = options.environment; } @@ -65,7 +68,7 @@ export class ShellOperationRunner implements IOperationRunner { } public getConfigHash(): string { - return this._commandToRun; + return this._commandForHash; } private async _executeAsync(context: IOperationRunnerContext): Promise { diff --git a/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts b/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts index a7f4979c4fa..8bee33cda26 100644 --- a/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts @@ -31,7 +31,7 @@ function createShellOperations( operations: Set, context: ICreateOperationsContext ): Set { - const { rushConfiguration } = context; + const { rushConfiguration, isInitial } = context; const getCustomParameterValuesForPhase: (phase: IPhase) => ReadonlyArray = getCustomParameterValuesByPhase(); @@ -44,17 +44,25 @@ function createShellOperations( const customParameterValues: ReadonlyArray = getCustomParameterValuesForPhase(phase); const displayName: string = getDisplayName(phase, project); - - const rawCommandToRun: string | undefined = getScriptToRun(project, phase.name, phase.shellCommand); - - const commandToRun: string | undefined = - rawCommandToRun !== undefined ? formatCommand(rawCommandToRun, customParameterValues) : undefined; + const { name: phaseName, shellCommand } = phase; + + const { scripts } = project.packageJson; + const commandForHash: string | undefined = [shellCommand, scripts?.[phaseName]].find( + (x): x is string => typeof x === 'string' + ); + const commandToRun: string | undefined = [ + shellCommand, + !isInitial && scripts?.[`${phaseName}:incremental`], + scripts?.[phaseName] + ].find((x): x is string => typeof x === 'string'); operation.runner = initializeShellOperationRunner({ phase, project, displayName, + commandForHash, commandToRun, + customParameterValues, rushConfiguration }); } @@ -69,18 +77,28 @@ export function initializeShellOperationRunner(options: { displayName: string; rushConfiguration: RushConfiguration; commandToRun: string | undefined; + commandForHash?: string; + customParameterValues: ReadonlyArray; }): IOperationRunner { - const { phase, project, rushConfiguration, commandToRun, displayName } = options; + const { phase, project, commandToRun: rawCommandToRun, displayName } = options; - if (commandToRun === undefined && phase.missingScriptBehavior === 'error') { + if (rawCommandToRun === undefined && phase.missingScriptBehavior === 'error') { throw new Error( `The project '${project.packageName}' does not define a '${phase.name}' command in the 'scripts' section of its package.json` ); } - if (commandToRun) { + if (rawCommandToRun) { + const { rushConfiguration, commandForHash: rawCommandForHash } = options; + + const commandToRun: string = formatCommand(rawCommandToRun, options.customParameterValues); + const commandForHash: string = rawCommandForHash + ? formatCommand(rawCommandForHash, options.customParameterValues) + : commandToRun; + return new ShellOperationRunner({ - commandToRun: commandToRun || '', + commandToRun, + commandForHash, displayName, phase, rushConfiguration, @@ -96,22 +114,6 @@ export function initializeShellOperationRunner(options: { } } -export function getScriptToRun( - rushProject: RushConfigurationProject, - commandToRun: string, - shellCommand: string | undefined -): string | undefined { - const { scripts } = rushProject.packageJson; - - const rawCommand: string | undefined | null = shellCommand ?? scripts?.[commandToRun]; - - if (rawCommand === undefined || rawCommand === null) { - return undefined; - } - - return rawCommand; -} - /** * Memoizer for custom parameter values by phase * @returns A function that returns the custom parameter values for a given phase diff --git a/libraries/rush-lib/src/schemas/command-line.schema.json b/libraries/rush-lib/src/schemas/command-line.schema.json index 2e3ef224402..dd2655f30b2 100644 --- a/libraries/rush-lib/src/schemas/command-line.schema.json +++ b/libraries/rush-lib/src/schemas/command-line.schema.json @@ -218,7 +218,7 @@ }, "watchPhases": { "title": "Watch Phases", - "description": "List *exactly* the phases that should be run in watch mode for this command. If this property is specified and non-empty, after the phases defined in the \"phases\" property run, a file watcher will be started to watch projects for changes, and will run the phases listed in this property on changed projects.", + "description": "List *exactly* the phases that should be run in watch mode for this command. If this property is specified and non-empty, after the phases defined in the \"phases\" property run, a file watcher will be started to watch projects for changes, and will run the phases listed in this property on changed projects. Rush will prefer scripts named \"${phaseName}:incremental\" over \"${phaseName}\" for every iteration after the first, so you can reuse the same phase name but define different scripts, e.g. to not clean on incremental runs.", "type": "array", "items": { "type": "string" From db08b24735ec8715dfa499e82e7abf149e1712a5 Mon Sep 17 00:00:00 2001 From: David Michon Date: Thu, 3 Oct 2024 21:34:31 +0000 Subject: [PATCH 2/3] PR feedback --- .../operations/IPCOperationRunnerPlugin.ts | 12 +++++++----- .../operations/ShardedPhaseOperationPlugin.ts | 4 +--- .../operations/ShellOperationRunnerPlugin.ts | 18 ++++++++++-------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts index 71593606695..bcbc6f9a281 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts @@ -55,13 +55,15 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin { continue; } + const { scripts } = project.packageJson; + if (!scripts) { + continue; + } + const { name: phaseName } = phase; - const { scripts } = project.packageJson; - const rawScript: string | undefined = [ - !isInitial && scripts?.[`${phaseName}:incremental:ipc`], - scripts?.[`${phaseName}:ipc`] - ].find((x): x is string => typeof x === 'string'); + const rawScript: string | undefined = + (!isInitial ? scripts[`${phaseName}:incremental:ipc`] : undefined) ?? scripts[`${phaseName}:ipc`]; if (!rawScript) { continue; diff --git a/libraries/rush-lib/src/logic/operations/ShardedPhaseOperationPlugin.ts b/libraries/rush-lib/src/logic/operations/ShardedPhaseOperationPlugin.ts index 0d88685473e..7b8e6242487 100644 --- a/libraries/rush-lib/src/logic/operations/ShardedPhaseOperationPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/ShardedPhaseOperationPlugin.ts @@ -128,9 +128,7 @@ function spliceShards(existingOperations: Set, context: ICreateOperat ]; const { scripts } = project.packageJson; - const commandToRun: string | undefined = [phase.shellCommand, scripts?.[phase.name]].find( - (x) => typeof x === 'string' - ); + const commandToRun: string | undefined = phase.shellCommand ?? scripts?.[phase.name]; operation.logFilenameIdentifier = `${baseLogFilenameIdentifier}_collate`; operation.runner = initializeShellOperationRunner({ diff --git a/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts b/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts index 8bee33cda26..e2a9cda2edb 100644 --- a/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts @@ -47,14 +47,16 @@ function createShellOperations( const { name: phaseName, shellCommand } = phase; const { scripts } = project.packageJson; - const commandForHash: string | undefined = [shellCommand, scripts?.[phaseName]].find( - (x): x is string => typeof x === 'string' - ); - const commandToRun: string | undefined = [ - shellCommand, - !isInitial && scripts?.[`${phaseName}:incremental`], - scripts?.[phaseName] - ].find((x): x is string => typeof x === 'string'); + + // This is the command that will be used to identify the cache entry for this operation + const commandForHash: string | undefined = shellCommand ?? scripts?.[phaseName]; + + // For execution of non-initial runs, prefer the `:incremental` script if it exists. + // However, the `shellCommand` value still takes precedence per the spec for that feature. + const commandToRun: string | undefined = + shellCommand ?? + (!isInitial ? scripts?.[`${phaseName}:incremental`] : undefined) ?? + scripts?.[phaseName]; operation.runner = initializeShellOperationRunner({ phase, From 2597a3913dfab298fee702bb6e933280053c2e73 Mon Sep 17 00:00:00 2001 From: David Michon Date: Thu, 3 Oct 2024 21:35:38 +0000 Subject: [PATCH 3/3] Use explicit typeof check --- .../rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts b/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts index e2a9cda2edb..3baad699275 100644 --- a/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/ShellOperationRunnerPlugin.ts @@ -84,7 +84,7 @@ export function initializeShellOperationRunner(options: { }): IOperationRunner { const { phase, project, commandToRun: rawCommandToRun, displayName } = options; - if (rawCommandToRun === undefined && phase.missingScriptBehavior === 'error') { + if (typeof rawCommandToRun !== 'string' && phase.missingScriptBehavior === 'error') { throw new Error( `The project '${project.packageName}' does not define a '${phase.name}' command in the 'scripts' section of its package.json` );