Skip to content

Commit

Permalink
Merge pull request #3103 from iclanton/pre-phased-refactors
Browse files Browse the repository at this point in the history 
[rush] Refactor the task runner code to be more naturally adapted to phased builds.
  • Loading branch information
@iclanton
iclanton authored Dec 21, 2021
2 parents 43d251a + 2780867 commit 919fa13
Show file tree
Hide file tree
Showing 26 changed files with 592 additions and 517 deletions.
69 changes: 33 additions & 36 deletions apps/rush-lib/src/cli/RushCommandLineParser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import { SetupAction } from './actions/SetupAction';
import { EnvironmentConfiguration } from '../api/EnvironmentConfiguration';
import { ICustomCommandLineConfigurationInfo, PluginManager } from '../pluginFramework/PluginManager';
import { RushSession } from '../pluginFramework/RushSession';
import { ProjectLogWritable } from '../logic/taskExecution/ProjectLogWritable';

/**
* Options for `RushCommandLineParser`.
Expand All @@ -67,9 +68,9 @@ export class RushCommandLineParser extends CommandLineParser {
public readonly pluginManager: PluginManager;

private _debugParameter!: CommandLineFlagParameter;
private _rushOptions: IRushCommandLineParserOptions;
private _terminalProvider: ConsoleTerminalProvider;
private _terminal: Terminal;
private readonly _rushOptions: IRushCommandLineParserOptions;
private readonly _terminalProvider: ConsoleTerminalProvider;
private readonly _terminal: Terminal;

public constructor(options?: Partial<IRushCommandLineParserOptions>) {
super({
Expand All @@ -88,7 +89,6 @@ export class RushCommandLineParser extends CommandLineParser {

this._terminalProvider = new ConsoleTerminalProvider();
this._terminal = new Terminal(this._terminalProvider);

this._rushOptions = this._normalizeOptions(options || {});

try {
Expand Down Expand Up @@ -261,6 +261,8 @@ export class RushCommandLineParser extends CommandLineParser {
}

if (!this.tryGetAction(RushConstants.rebuildCommandName)) {
// By default, the "rebuild" action runs the "build" script. However, if the command-line.json file
// overrides "rebuild," the "rebuild" script should be run.
this._addCommandLineConfigAction(
commandLineConfiguration,
CommandLineConfiguration.defaultRebuildCommandJson,
Expand All @@ -283,7 +285,7 @@ export class RushCommandLineParser extends CommandLineParser {
private _addCommandLineConfigAction(
commandLineConfiguration: CommandLineConfiguration | undefined,
command: CommandJson,
commandToRun?: string
commandToRun: string = command.name
): void {
if (this.tryGetAction(command.name)) {
throw new Error(
Expand All @@ -292,18 +294,29 @@ export class RushCommandLineParser extends CommandLineParser {
);
}

this._validateCommandLineConfigCommand(command);
if (
(command.name === RushConstants.buildCommandName ||
command.name === RushConstants.rebuildCommandName) &&
command.safeForSimultaneousRushProcesses
) {
// Build and rebuild can't be designated "safeForSimultaneousRushProcesses"
throw new Error(
`${RushConstants.commandLineFilename} defines a command "${command.name}" using ` +
`"safeForSimultaneousRushProcesses=true". This configuration is not supported for "${command.name}".`
);
}

const overrideAllowWarnings: boolean = EnvironmentConfiguration.allowWarningsInSuccessfulBuild;

switch (command.commandKind) {
case RushConstants.bulkCommandKind: {
const logFilenameIdentifier: string =
ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(commandToRun);

this.addAction(
new BulkScriptAction({
actionName: command.name,

// By default, the "rebuild" action runs the "build" script. However, if the command-line.json file
// overrides "rebuild," the "rebuild" script should be run.
logFilenameIdentifier: logFilenameIdentifier,
commandToRun: commandToRun,

summary: command.summary,
Expand All @@ -327,6 +340,17 @@ export class RushCommandLineParser extends CommandLineParser {
}

case RushConstants.globalCommandKind: {
if (
command.name === RushConstants.buildCommandName ||
command.name === RushConstants.rebuildCommandName
) {
throw new Error(
`${RushConstants.commandLineFilename} defines a command "${command.name}" using ` +
`the command kind "${RushConstants.globalCommandKind}". This command can only be designated as a command ` +
`kind "${RushConstants.bulkCommandKind}" or "${RushConstants.phasedCommandKind}".`
);
}

this.addAction(
new GlobalScriptAction({
actionName: command.name,
Expand Down Expand Up @@ -367,33 +391,6 @@ export class RushCommandLineParser extends CommandLineParser {
}
}

private _validateCommandLineConfigCommand(command: CommandJson): void {
// There are some restrictions on the 'build' and 'rebuild' commands.
if (
command.name !== RushConstants.buildCommandName &&
command.name !== RushConstants.rebuildCommandName
) {
return;
}

if (
command.commandKind !== RushConstants.bulkCommandKind &&
command.commandKind !== RushConstants.phasedCommandKind
) {
throw new Error(
`${RushConstants.commandLineFilename} defines a command "${command.name}" using ` +
`the command kind "${RushConstants.globalCommandKind}". This command can only be designated as a command ` +
`kind "${RushConstants.bulkCommandKind}" or "${RushConstants.phasedCommandKind}".`
);
}
if (command.safeForSimultaneousRushProcesses) {
throw new Error(
`${RushConstants.commandLineFilename} defines a command "${command.name}" using ` +
`"safeForSimultaneousRushProcesses=true". This configuration is not supported for "${command.name}".`
);
}
}

private _reportErrorAndSetExitCode(error: Error): void {
if (!(error instanceof AlreadyReportedError)) {
const prefix: string = 'ERROR: ';
Expand Down
19 changes: 14 additions & 5 deletions apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import { BaseRushAction } from './BaseRushAction';
import { RushCommandLineParser } from '../RushCommandLineParser';

import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration';
import { ProjectBuilder } from '../../logic/taskRunner/ProjectBuilder';
import { ProjectTaskRunner } from '../../logic/taskExecution/ProjectTaskRunner';
import { ProjectChangeAnalyzer } from '../../logic/ProjectChangeAnalyzer';
import { Utilities } from '../../utilities/Utilities';
import { TaskSelector } from '../../logic/TaskSelector';
import { NonPhasedProjectTaskSelector } from '../../logic/NonPhasedProjectTaskSelector';
import { RushConstants } from '../../logic/RushConstants';
import { CommandLineConfiguration } from '../../api/CommandLineConfiguration';
import { ProjectLogWritable } from '../../logic/taskExecution/ProjectLogWritable';

export class WriteBuildCacheAction extends BaseRushAction {
private _command!: CommandLineStringParameter;
Expand Down Expand Up @@ -74,18 +75,26 @@ export class WriteBuildCacheAction extends BaseRushAction {
);

const command: string = this._command.value!;
const commandToRun: string | undefined = TaskSelector.getScriptToRun(project, command, []);
const commandToRun: string | undefined = NonPhasedProjectTaskSelector.getScriptToRun(
project,
command,
[]
);

// TODO: With phased commands, this will need to be updated
const taskName: string = NonPhasedProjectTaskSelector.getTaskNameForProject(project);
const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(this.rushConfiguration);
const projectBuilder: ProjectBuilder = new ProjectBuilder({
const projectBuilder: ProjectTaskRunner = new ProjectTaskRunner({
rushProject: project,
taskName,
rushConfiguration: this.rushConfiguration,
buildCacheConfiguration,
commandName: command,
commandToRun: commandToRun || '',
isIncrementalBuildAllowed: false,
projectChangeAnalyzer,
packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(command)
packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(command),
logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(command)
});

const trackedFiles: string[] = Array.from(
Expand Down
48 changes: 28 additions & 20 deletions apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,16 @@ import { CommandLineFlagParameter, CommandLineStringParameter } from '@rushstack

import { Event } from '../../index';
import { SetupChecks } from '../../logic/SetupChecks';
import { ITaskSelectorOptions, TaskSelector } from '../../logic/TaskSelector';
import {
INonPhasedProjectTaskSelectorOptions,
NonPhasedProjectTaskSelector
} from '../../logic/NonPhasedProjectTaskSelector';
import { Stopwatch, StopwatchState } from '../../utilities/Stopwatch';
import { BaseScriptAction, IBaseScriptActionOptions } from './BaseScriptAction';
import { ITaskRunnerOptions, TaskRunner } from '../../logic/taskRunner/TaskRunner';
import {
ITaskExecutionManagerOptions,
TaskExecutionManager
} from '../../logic/taskExecution/TaskExecutionManager';
import { Utilities } from '../../utilities/Utilities';
import { RushConstants } from '../../logic/RushConstants';
import { EnvironmentVariableNames } from '../../api/EnvironmentConfiguration';
Expand All @@ -34,16 +40,13 @@ export interface IBulkScriptActionOptions extends IBaseScriptActionOptions {
allowWarningsInSuccessfulBuild: boolean;
watchForChanges: boolean;
disableBuildCache: boolean;

/**
* Optional command to run. Otherwise, use the `actionName` as the command to run.
*/
commandToRun?: string;
logFilenameIdentifier: string;
commandToRun: string;
}

interface IExecuteInternalOptions {
taskSelectorOptions: ITaskSelectorOptions;
taskRunnerOptions: ITaskRunnerOptions;
taskSelectorOptions: INonPhasedProjectTaskSelectorOptions;
taskExecutionManagerOptions: ITaskExecutionManagerOptions;
stopwatch: Stopwatch;
ignoreHooks?: boolean;
terminal: Terminal;
Expand All @@ -68,6 +71,7 @@ export class BulkScriptAction extends BaseScriptAction {
private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined;
private readonly _ignoreDependencyOrder: boolean;
private readonly _allowWarningsInSuccessfulBuild: boolean;
private readonly _logFilenameIdentifier: string;

private _changedProjectsOnly!: CommandLineFlagParameter;
private _selectionParameters!: SelectionParameterSet;
Expand All @@ -80,12 +84,13 @@ export class BulkScriptAction extends BaseScriptAction {
this._enableParallelism = options.enableParallelism;
this._ignoreMissingScript = options.ignoreMissingScript;
this._isIncrementalBuildAllowed = options.incremental;
this._commandToRun = options.commandToRun || options.actionName;
this._commandToRun = options.commandToRun;
this._ignoreDependencyOrder = options.ignoreDependencyOrder;
this._allowWarningsInSuccessfulBuild = options.allowWarningsInSuccessfulBuild;
this._watchForChanges = options.watchForChanges;
this._disableBuildCache = options.disableBuildCache;
this._repoCommandLineConfiguration = options.commandLineConfiguration;
this._logFilenameIdentifier = options.logFilenameIdentifier;
}

public async runAsync(): Promise<void> {
Expand Down Expand Up @@ -139,7 +144,7 @@ export class BulkScriptAction extends BaseScriptAction {
return;
}

const taskSelectorOptions: ITaskSelectorOptions = {
const taskSelectorOptions: INonPhasedProjectTaskSelectorOptions = {
rushConfiguration: this.rushConfiguration,
buildCacheConfiguration,
selection,
Expand All @@ -152,21 +157,22 @@ export class BulkScriptAction extends BaseScriptAction {
ignoreMissingScript: this._ignoreMissingScript,
ignoreDependencyOrder: this._ignoreDependencyOrder,
allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild,
packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(this._commandToRun)
packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(this._commandToRun),
logFilenameIdentifier: this._logFilenameIdentifier
};

const taskRunnerOptions: ITaskRunnerOptions = {
const taskExecutionManagerOptions: ITaskExecutionManagerOptions = {
quietMode: isQuietMode,
debugMode: this.parser.isDebug,
parallelism: parallelism,
changedProjectsOnly: changedProjectsOnly,
allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild,
allowWarningsInSuccessfulExecution: this._allowWarningsInSuccessfulBuild,
repoCommandLineConfiguration: this._repoCommandLineConfiguration
};

const executeOptions: IExecuteInternalOptions = {
taskSelectorOptions,
taskRunnerOptions,
taskExecutionManagerOptions: taskExecutionManagerOptions,
stopwatch,
terminal
};
Expand Down Expand Up @@ -245,7 +251,7 @@ export class BulkScriptAction extends BaseScriptAction {
// Pass the ProjectChangeAnalyzer from the state differ to save a bit of overhead
projectChangeAnalyzer: state
},
taskRunnerOptions: options.taskRunnerOptions,
taskExecutionManagerOptions: options.taskExecutionManagerOptions,
stopwatch,
// For now, don't run pre-build or post-build in watch mode
ignoreHooks: true,
Expand Down Expand Up @@ -318,19 +324,21 @@ export class BulkScriptAction extends BaseScriptAction {
* Runs a single invocation of the command
*/
private async _runOnce(options: IExecuteInternalOptions): Promise<void> {
const taskSelector: TaskSelector = new TaskSelector(options.taskSelectorOptions);
const taskSelector: NonPhasedProjectTaskSelector = new NonPhasedProjectTaskSelector(
options.taskSelectorOptions
);

// Register all tasks with the task collection

const taskRunner: TaskRunner = new TaskRunner(
const taskExecutionManager: TaskExecutionManager = new TaskExecutionManager(
taskSelector.registerTasks().getOrderedTasks(),
options.taskRunnerOptions
options.taskExecutionManagerOptions
);

const { ignoreHooks, stopwatch } = options;

try {
await taskRunner.executeAsync();
await taskExecutionManager.executeAsync();

stopwatch.stop();
console.log(colors.green(`rush ${this.actionName} (${stopwatch.toString()})`));
Expand Down
16 changes: 12 additions & 4 deletions apps/rush-lib/src/cli/test/RushCommandLineParser.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,9 @@ describe('RushCommandLineParser', () => {

await expect(() => {
getCommandLineParserInstance(repoName, 'doesnt-matter');
}).toThrowError('This command can only be designated as a command kind "bulk"');
}).toThrowErrorMatchingInlineSnapshot(
`"command-line.json defines a command \\"build\\" using the command kind \\"global\\". This command can only be designated as a command kind \\"bulk\\" or \\"phased\\"."`
);
});
});

Expand All @@ -301,7 +303,9 @@ describe('RushCommandLineParser', () => {

await expect(() => {
getCommandLineParserInstance(repoName, 'doesnt-matter');
}).toThrowError('This command can only be designated as a command kind "bulk"');
}).toThrowErrorMatchingInlineSnapshot(
`"command-line.json defines a command \\"rebuild\\" using the command kind \\"global\\". This command can only be designated as a command kind \\"bulk\\" or \\"phased\\"."`
);
});
});

Expand All @@ -311,7 +315,9 @@ describe('RushCommandLineParser', () => {

await expect(() => {
getCommandLineParserInstance(repoName, 'doesnt-matter');
}).toThrowError('"safeForSimultaneousRushProcesses=true". This configuration is not supported');
}).toThrowErrorMatchingInlineSnapshot(
`"command-line.json defines a command \\"build\\" using \\"safeForSimultaneousRushProcesses=true\\". This configuration is not supported for \\"build\\"."`
);
});
});

Expand All @@ -321,7 +327,9 @@ describe('RushCommandLineParser', () => {

await expect(() => {
getCommandLineParserInstance(repoName, 'doesnt-matter');
}).toThrowError('"safeForSimultaneousRushProcesses=true". This configuration is not supported');
}).toThrowErrorMatchingInlineSnapshot(
`"command-line.json defines a command \\"rebuild\\" using \\"safeForSimultaneousRushProcesses=true\\". This configuration is not supported for \\"rebuild\\"."`
);
});
});
});
Expand Down
Loading

0 comments on commit 919fa13

Please sign in to comment.