From f4a067e14b5300ab9d8b43fdaa4704518c1e7ff6 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Mon, 20 Dec 2021 13:22:32 -0800 Subject: [PATCH 1/5] Refactor the task runner code to be more naturally adapted to phased builds. --- .../src/cli/actions/WriteBuildCacheAction.ts | 4 +- .../src/cli/scriptActions/BulkScriptAction.ts | 21 +++-- apps/rush-lib/src/logic/TaskSelector.ts | 17 ++-- .../BaseTaskRunner.ts} | 16 ++-- .../ProjectLogWritable.ts | 31 ++++--- .../ProjectTaskRunner.ts} | 83 +++++++++---------- .../{taskRunner => taskExecution}/Task.ts | 22 ++--- .../TaskCollection.ts | 19 ++--- .../src/logic/taskExecution/TaskError.ts | 23 +++++ .../TaskExecutionManager.ts} | 71 ++++++++-------- .../TaskStatus.ts | 0 .../test/MockTaskRunner.ts} | 6 +- .../test/ProjectTaskRunner.test.ts} | 2 +- .../test/TaskCollection.test.ts | 12 +-- .../test/TaskExecutionManager.test.ts} | 73 ++++++++-------- .../__snapshots__/TaskCollection.test.ts.snap | 0 .../TaskExecutionManager.test.ts.snap} | 16 ++-- .../src/logic/taskRunner/TaskError.ts | 48 ----------- 18 files changed, 221 insertions(+), 243 deletions(-) rename apps/rush-lib/src/logic/{taskRunner/BaseBuilder.ts => taskExecution/BaseTaskRunner.ts} (67%) rename apps/rush-lib/src/logic/{taskRunner => taskExecution}/ProjectLogWritable.ts (69%) rename apps/rush-lib/src/logic/{taskRunner/ProjectBuilder.ts => taskExecution/ProjectTaskRunner.ts} (87%) rename apps/rush-lib/src/logic/{taskRunner => taskExecution}/Task.ts (83%) rename apps/rush-lib/src/logic/{taskRunner => taskExecution}/TaskCollection.ts (90%) create mode 100644 apps/rush-lib/src/logic/taskExecution/TaskError.ts rename apps/rush-lib/src/logic/{taskRunner/TaskRunner.ts => taskExecution/TaskExecutionManager.ts} (90%) rename apps/rush-lib/src/logic/{taskRunner => taskExecution}/TaskStatus.ts (100%) rename apps/rush-lib/src/logic/{taskRunner/test/MockBuilder.ts => taskExecution/test/MockTaskRunner.ts} (80%) rename apps/rush-lib/src/logic/{taskRunner/test/ProjectTask.test.ts => taskExecution/test/ProjectTaskRunner.test.ts} (95%) rename apps/rush-lib/src/logic/{taskRunner => taskExecution}/test/TaskCollection.test.ts (87%) rename apps/rush-lib/src/logic/{taskRunner/test/TaskRunner.test.ts => taskExecution/test/TaskExecutionManager.test.ts} (68%) rename apps/rush-lib/src/logic/{taskRunner => taskExecution}/test/__snapshots__/TaskCollection.test.ts.snap (100%) rename apps/rush-lib/src/logic/{taskRunner/test/__snapshots__/TaskRunner.test.ts.snap => taskExecution/test/__snapshots__/TaskExecutionManager.test.ts.snap} (88%) delete mode 100644 apps/rush-lib/src/logic/taskRunner/TaskError.ts diff --git a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts index 66486da192a..cd62431f139 100644 --- a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts +++ b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts @@ -10,7 +10,7 @@ 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'; @@ -77,7 +77,7 @@ export class WriteBuildCacheAction extends BaseRushAction { const commandToRun: string | undefined = TaskSelector.getScriptToRun(project, command, []); const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(this.rushConfiguration); - const projectBuilder: ProjectBuilder = new ProjectBuilder({ + const projectBuilder: ProjectTaskRunner = new ProjectTaskRunner({ rushProject: project, rushConfiguration: this.rushConfiguration, buildCacheConfiguration, diff --git a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts index 332740e8596..c5804964ee9 100644 --- a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts @@ -12,7 +12,10 @@ import { SetupChecks } from '../../logic/SetupChecks'; import { ITaskSelectorOptions, TaskSelector } from '../../logic/TaskSelector'; 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'; @@ -43,7 +46,7 @@ export interface IBulkScriptActionOptions extends IBaseScriptActionOptions { interface IExecuteInternalOptions { taskSelectorOptions: ITaskSelectorOptions; - taskRunnerOptions: ITaskRunnerOptions; + taskExecutionManagerOptions: ITaskExecutionManagerOptions; stopwatch: Stopwatch; ignoreHooks?: boolean; terminal: Terminal; @@ -155,18 +158,18 @@ export class BulkScriptAction extends BaseScriptAction { packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(this._commandToRun) }; - 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 }; @@ -245,7 +248,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, @@ -322,15 +325,15 @@ export class BulkScriptAction extends BaseScriptAction { // 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()})`)); diff --git a/apps/rush-lib/src/logic/TaskSelector.ts b/apps/rush-lib/src/logic/TaskSelector.ts index 578a30a264a..7f047ea2b7b 100644 --- a/apps/rush-lib/src/logic/TaskSelector.ts +++ b/apps/rush-lib/src/logic/TaskSelector.ts @@ -4,9 +4,9 @@ import { BuildCacheConfiguration } from '../api/BuildCacheConfiguration'; import { RushConfiguration } from '../api/RushConfiguration'; import { RushConfigurationProject } from '../api/RushConfigurationProject'; -import { ProjectBuilder, convertSlashesForWindows } from '../logic/taskRunner/ProjectBuilder'; +import { ProjectTaskRunner, convertSlashesForWindows } from './taskExecution/ProjectTaskRunner'; import { ProjectChangeAnalyzer } from './ProjectChangeAnalyzer'; -import { TaskCollection } from './taskRunner/TaskCollection'; +import { TaskCollection } from './taskExecution/TaskCollection'; export interface ITaskSelectorOptions { rushConfiguration: RushConfiguration; @@ -29,7 +29,7 @@ export interface ITaskSelectorOptions { * This class is responsible for: * - based on to/from flags, solving the dependency graph and figuring out which projects need to be run * - creating a ProjectBuilder for each project that needs to be built - * - registering the necessary ProjectBuilders with the TaskRunner, which actually orchestrates execution + * - registering the necessary ProjectBuilders with the TaskExecutionManager, which actually orchestrates execution */ export class TaskSelector { private _options: ITaskSelectorOptions; @@ -87,7 +87,7 @@ export class TaskSelector { for (const dep of project.dependencyProjects) { if (projects.has(dep)) { // Add direct relationships for projects in the set - dependencyTaskNames.add(ProjectBuilder.getTaskName(dep)); + dependencyTaskNames.add(ProjectTaskRunner.getTaskName(dep)); } else { // Add indirect relationships for projects not in the set for (const indirectDep of getDependencyTaskNames(dep)) { @@ -101,7 +101,10 @@ export class TaskSelector { // Add ordering relationships for each dependency for (const project of projects) { - taskCollection.addDependencies(ProjectBuilder.getTaskName(project), getDependencyTaskNames(project)); + taskCollection.addDependencies( + ProjectTaskRunner.getTaskName(project), + getDependencyTaskNames(project) + ); } } @@ -109,7 +112,7 @@ export class TaskSelector { } private _registerTask(project: RushConfigurationProject | undefined, taskCollection: TaskCollection): void { - if (!project || taskCollection.hasTask(ProjectBuilder.getTaskName(project))) { + if (!project || taskCollection.hasTask(ProjectTaskRunner.getTaskName(project))) { return; } @@ -125,7 +128,7 @@ export class TaskSelector { } taskCollection.addTask( - new ProjectBuilder({ + new ProjectTaskRunner({ rushProject: project, rushConfiguration: this._options.rushConfiguration, buildCacheConfiguration: this._options.buildCacheConfiguration, diff --git a/apps/rush-lib/src/logic/taskRunner/BaseBuilder.ts b/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts similarity index 67% rename from apps/rush-lib/src/logic/taskRunner/BaseBuilder.ts rename to apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts index e5f09a88a42..99b44ffc760 100644 --- a/apps/rush-lib/src/logic/taskRunner/BaseBuilder.ts +++ b/apps/rush-lib/src/logic/taskExecution/BaseTaskRunner.ts @@ -7,7 +7,7 @@ import { CollatedWriter } from '@rushstack/stream-collator'; import { TaskStatus } from './TaskStatus'; import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; -export interface IBuilderContext { +export interface ITaskRunnerContext { repoCommandLineConfiguration: CommandLineConfiguration | undefined; collatedWriter: CollatedWriter; stdioSummarizer: StdioSummarizer; @@ -16,11 +16,11 @@ export interface IBuilderContext { } /** - * The `Task` class is a node in the dependency graph of work that needs to be scheduled by the `TaskRunner`. - * Each `Task` has a `BaseBuilder` member, whose subclass manages the actual operations for building a single - * project. + * The `Task` class is a node in the dependency graph of work that needs to be scheduled by the + * `TaskExecutionManager`. Each `Task` has a `runner` member of type `BaseTaskRunner`, whose subclass + * manages the actual operations for running a single task. */ -export abstract class BaseBuilder { +export abstract class BaseTaskRunner { /** * Name of the task definition. */ @@ -32,13 +32,13 @@ export abstract class BaseBuilder { public abstract isSkipAllowed: boolean; /** - * Assigned by execute(). True if the build script was an empty string. Operationally an empty string is - * like a shell command that succeeds instantly, but e.g. it would be odd to report build time statistics for it. + * Assigned by execute(). True if the script was an empty string. Operationally an empty string is + * like a shell command that succeeds instantly, but e.g. it would be odd to report time statistics for it. */ public abstract hadEmptyScript: boolean; /** * Method to be executed for the task. */ - public abstract executeAsync(context: IBuilderContext): Promise; + public abstract executeAsync(context: ITaskRunnerContext): Promise; } diff --git a/apps/rush-lib/src/logic/taskRunner/ProjectLogWritable.ts b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts similarity index 69% rename from apps/rush-lib/src/logic/taskRunner/ProjectLogWritable.ts rename to apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts index 08654633b08..0716f145e33 100644 --- a/apps/rush-lib/src/logic/taskRunner/ProjectLogWritable.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import * as path from 'path'; import { FileSystem, FileWriter, InternalError } from '@rushstack/node-core-library'; import { TerminalChunkKind, TerminalWritable, ITerminalChunk } from '@rushstack/terminal'; import { CollatedTerminal } from '@rushstack/stream-collator'; @@ -13,13 +12,13 @@ export class ProjectLogWritable extends TerminalWritable { private readonly _project: RushConfigurationProject; private readonly _terminal: CollatedTerminal; - private _buildLogPath: string; + private _logPath: string; private _errorLogPath: string; - private _buildLogWriter: FileWriter | undefined = undefined; + private _logWriter: FileWriter | undefined = undefined; private _errorLogWriter: FileWriter | undefined = undefined; - public constructor(project: RushConfigurationProject, terminal: CollatedTerminal) { + public constructor(project: RushConfigurationProject, terminal: CollatedTerminal, commandName: string) { super(); this._project = project; this._terminal = terminal; @@ -28,24 +27,24 @@ export class ProjectLogWritable extends TerminalWritable { this._project.packageName ); - this._buildLogPath = path.join(this._project.projectFolder, `${unscopedProjectName}.build.log`); - this._errorLogPath = path.join(this._project.projectFolder, `${unscopedProjectName}.build.error.log`); + this._logPath = `${this._project.projectFolder}/${unscopedProjectName}.${commandName}.log`; + this._errorLogPath = `${this._project.projectFolder}/${unscopedProjectName}.${commandName}.error.log`; - FileSystem.deleteFile(this._buildLogPath); + FileSystem.deleteFile(this._logPath); FileSystem.deleteFile(this._errorLogPath); - this._buildLogWriter = FileWriter.open(this._buildLogPath); + this._logWriter = FileWriter.open(this._logPath); } protected onWriteChunk(chunk: ITerminalChunk): void { - if (!this._buildLogWriter) { + if (!this._logWriter) { throw new InternalError('Output file was closed'); } - // Both stderr and stdout get written to *.build.log - this._buildLogWriter.write(chunk.text); + // Both stderr and stdout get written to *..log + this._logWriter.write(chunk.text); if (chunk.kind === TerminalChunkKind.Stderr) { - // Only stderr gets written to *.build.error.log + // Only stderr gets written to *..error.log if (!this._errorLogWriter) { this._errorLogWriter = FileWriter.open(this._errorLogPath); } @@ -54,13 +53,13 @@ export class ProjectLogWritable extends TerminalWritable { } protected onClose(): void { - if (this._buildLogWriter) { + if (this._logWriter) { try { - this._buildLogWriter.close(); + this._logWriter.close(); } catch (error) { - this._terminal.writeStderrLine('Failed to close file handle for ' + this._buildLogWriter.filePath); + this._terminal.writeStderrLine('Failed to close file handle for ' + this._logWriter.filePath); } - this._buildLogWriter = undefined; + this._logWriter = undefined; } if (this._errorLogWriter) { diff --git a/apps/rush-lib/src/logic/taskRunner/ProjectBuilder.ts b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts similarity index 87% rename from apps/rush-lib/src/logic/taskRunner/ProjectBuilder.ts rename to apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts index 5926afe95ac..4413b8dbb4a 100644 --- a/apps/rush-lib/src/logic/taskRunner/ProjectBuilder.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts @@ -30,7 +30,7 @@ import { Utilities, UNINITIALIZED } from '../../utilities/Utilities'; import { TaskStatus } from './TaskStatus'; import { TaskError } from './TaskError'; import { ProjectChangeAnalyzer } from '../ProjectChangeAnalyzer'; -import { BaseBuilder, IBuilderContext } from './BaseBuilder'; +import { BaseTaskRunner, ITaskRunnerContext } from './BaseTaskRunner'; import { ProjectLogWritable } from './ProjectLogWritable'; import { ProjectBuildCache } from '../buildCache/ProjectBuildCache'; import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; @@ -39,12 +39,12 @@ import { CollatedTerminalProvider } from '../../utilities/CollatedTerminalProvid import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; import { RushConstants } from '../RushConstants'; -export interface IProjectBuildDeps { +export interface IProjectDeps { files: { [filePath: string]: string }; arguments: string; } -export interface IProjectBuilderOptions { +export interface IProjectTaskRunnerOptions { rushProject: RushConfigurationProject; rushConfiguration: RushConfiguration; buildCacheConfiguration: BuildCacheConfiguration | undefined; @@ -71,16 +71,16 @@ function _areShallowEqual(object1: JsonObject, object2: JsonObject): boolean { } /** - * A `BaseBuilder` subclass that builds a Rush project and updates its package-deps-hash + * A `BaseTaskRunner` subclass that executes a task for a Rush project and updates its package-deps-hash * incremental state. */ -export class ProjectBuilder extends BaseBuilder { +export class ProjectTaskRunner extends BaseTaskRunner { public get name(): string { - return ProjectBuilder.getTaskName(this._rushProject); + return ProjectTaskRunner.getTaskName(this._rushProject); } /** - * This property is mutated by TaskRunner, so is not readonly + * This property is mutated by TaskExecutionManager, so is not readonly */ public isSkipAllowed: boolean; public hadEmptyScript: boolean = false; @@ -94,6 +94,7 @@ export class ProjectBuilder extends BaseBuilder { private readonly _projectChangeAnalyzer: ProjectChangeAnalyzer; private readonly _packageDepsFilename: string; private readonly _allowWarningsInSuccessfulBuild: boolean; + private readonly _commandNameForLogFilenames: string; /** * UNINITIALIZED === we haven't tried to initialize yet @@ -101,7 +102,7 @@ export class ProjectBuilder extends BaseBuilder { */ private _projectBuildCache: ProjectBuildCache | undefined | UNINITIALIZED = UNINITIALIZED; - public constructor(options: IProjectBuilderOptions) { + public constructor(options: IProjectTaskRunnerOptions) { super(); this._rushProject = options.rushProject; this._rushConfiguration = options.rushConfiguration; @@ -113,6 +114,7 @@ export class ProjectBuilder extends BaseBuilder { this._projectChangeAnalyzer = options.projectChangeAnalyzer; this._packageDepsFilename = options.packageDepsFilename; this._allowWarningsInSuccessfulBuild = options.allowWarningsInSuccessfulBuild || false; + this._commandNameForLogFilenames = options.commandName; } /** @@ -123,7 +125,7 @@ export class ProjectBuilder extends BaseBuilder { return rushProject.packageName; } - public async executeAsync(context: IBuilderContext): Promise { + public async executeAsync(context: ITaskRunnerContext): Promise { try { if (!this._commandToRun) { this.hadEmptyScript = true; @@ -147,7 +149,7 @@ export class ProjectBuilder extends BaseBuilder { return projectBuildCache?.trySetCacheEntryAsync(terminal); } - private async _executeTaskAsync(context: IBuilderContext): Promise { + private async _executeTaskAsync(context: ITaskRunnerContext): Promise { // TERMINAL PIPELINE: // // +--> quietModeTransform? --> collatedWriter @@ -157,7 +159,8 @@ export class ProjectBuilder extends BaseBuilder { // +--> stdioSummarizer const projectLogWritable: ProjectLogWritable = new ProjectLogWritable( this._rushProject, - context.collatedWriter.terminal + context.collatedWriter.terminal, + this._commandNameForLogFilenames ); try { @@ -198,7 +201,7 @@ export class ProjectBuilder extends BaseBuilder { let hasWarningOrError: boolean = false; const projectFolder: string = this._rushProject.projectFolder; - let lastProjectBuildDeps: IProjectBuildDeps | undefined = undefined; + let lastProjectDeps: IProjectDeps | undefined = undefined; const currentDepsPath: string = path.join( this._rushProject.projectRushTempFolder, @@ -207,7 +210,7 @@ export class ProjectBuilder extends BaseBuilder { if (FileSystem.exists(currentDepsPath)) { try { - lastProjectBuildDeps = JsonFile.load(currentDepsPath); + lastProjectDeps = JsonFile.load(currentDepsPath); } catch (e) { // Warn and ignore - treat failing to load the file as the project being not built. terminal.writeWarningLine( @@ -217,7 +220,7 @@ export class ProjectBuilder extends BaseBuilder { } } - let projectBuildDeps: IProjectBuildDeps | undefined; + let projectDeps: IProjectDeps | undefined; let trackedFiles: string[] | undefined; try { const fileHashes: Map | undefined = @@ -231,7 +234,7 @@ export class ProjectBuilder extends BaseBuilder { trackedFiles.push(filePath); } - projectBuildDeps = { + projectDeps = { files, arguments: this._commandToRun }; @@ -241,7 +244,7 @@ export class ProjectBuilder extends BaseBuilder { terminal.writeLine({ text: PrintUtilities.wrapWords( 'This workspace does not appear to be tracked by Git. ' + - 'Rush will proceed without incremental build, caching, and change detection.' + 'Rush will proceed without incremental execution, caching, and change detection.' ), foregroundColor: ColorValue.Cyan }); @@ -249,27 +252,27 @@ export class ProjectBuilder extends BaseBuilder { } catch (error) { // To test this code path: // Delete a project's ".rush/temp/shrinkwrap-deps.json" then run "rush build --verbose" - terminal.writeLine('Unable to calculate incremental build state: ' + (error as Error).toString()); + terminal.writeLine('Unable to calculate incremental state: ' + (error as Error).toString()); terminal.writeLine({ - text: 'Rush will proceed without incremental build, caching, and change detection.', + text: 'Rush will proceed without incremental execution, caching, and change detection.', foregroundColor: ColorValue.Cyan }); } - // If possible, we want to skip this build -- either by restoring it from the - // build cache, if build caching is enabled, or determining that the project - // is unchanged (using the older incremental build logic). These two approaches, + // If possible, we want to skip this task -- either by restoring it from the + // cache, if caching is enabled, or determining that the project + // is unchanged (using the older incremental execution logic). These two approaches, // "caching" and "skipping", are incompatible, so only one applies. // - // Note that "build caching" and "build skipping" take two different approaches + // Note that "caching" and "skipping" take two different approaches // to tracking dependents: // - // - For build caching, "isCacheReadAllowed" is set if a project supports + // - For caching, "isCacheReadAllowed" is set if a project supports // incremental builds, and determining whether this project or a dependent // has changed happens inside the hashing logic. // - // - For build skipping, "isSkipAllowed" is set to true initially, and during - // the process of building dependents, it will be changed by TaskRunner to + // - For skipping, "isSkipAllowed" is set to true initially, and during + // the process of running dependents, it will be changed by TaskExecutionManager to // false if a dependency wasn't able to be skipped. // let buildCacheReadAttempted: boolean = false; @@ -290,10 +293,10 @@ export class ProjectBuilder extends BaseBuilder { } if (this.isSkipAllowed && !buildCacheReadAttempted) { const isPackageUnchanged: boolean = !!( - lastProjectBuildDeps && - projectBuildDeps && - projectBuildDeps.arguments === lastProjectBuildDeps.arguments && - _areShallowEqual(projectBuildDeps.files, lastProjectBuildDeps.files) + lastProjectDeps && + projectDeps && + projectDeps.arguments === lastProjectDeps.arguments && + _areShallowEqual(projectDeps.files, lastProjectDeps.files) ); if (isPackageUnchanged) { @@ -301,7 +304,7 @@ export class ProjectBuilder extends BaseBuilder { } } - // If the deps file exists, remove it before starting a build. + // If the deps file exists, remove it before starting execution. FileSystem.deleteFile(currentDepsPath); // TODO: Remove legacyDepsPath with the next major release of Rush @@ -311,8 +314,8 @@ export class ProjectBuilder extends BaseBuilder { if (!this._commandToRun) { // Write deps on success. - if (projectBuildDeps) { - JsonFile.save(projectBuildDeps, currentDepsPath, { + if (projectDeps) { + JsonFile.save(projectDeps, currentDepsPath, { ensureFolderExists: true }); } @@ -333,7 +336,7 @@ export class ProjectBuilder extends BaseBuilder { } }); - // Hook into events, in order to get live streaming of build log + // Hook into events, in order to get live streaming of the log if (task.stdout !== null) { task.stdout.on('data', (data: Buffer) => { const text: string = data.toString(); @@ -373,18 +376,14 @@ export class ProjectBuilder extends BaseBuilder { !!this._rushConfiguration.experimentsConfiguration.configuration .buildCacheWithAllowWarningsInSuccessfulBuild); - if (taskIsSuccessful && projectBuildDeps) { + if (taskIsSuccessful && projectDeps) { // Write deps on success. - const writeProjectStatePromise: Promise = JsonFile.saveAsync( - projectBuildDeps, - currentDepsPath, - { - ensureFolderExists: true - } - ); + const writeProjectStatePromise: Promise = JsonFile.saveAsync(projectDeps, currentDepsPath, { + ensureFolderExists: true + }); // If the command is successful and we can calculate project hash, we will write a - // new cache entry even if incremental builds are not allowed. + // new cache entry even if incremental execution is not allowed. const setCacheEntryPromise: Promise = this.tryWriteCacheEntryAsync( terminal, trackedFiles, diff --git a/apps/rush-lib/src/logic/taskRunner/Task.ts b/apps/rush-lib/src/logic/taskExecution/Task.ts similarity index 83% rename from apps/rush-lib/src/logic/taskRunner/Task.ts rename to apps/rush-lib/src/logic/taskExecution/Task.ts index ddb17bbeb63..4f641d8d8c3 100644 --- a/apps/rush-lib/src/logic/taskRunner/Task.ts +++ b/apps/rush-lib/src/logic/taskExecution/Task.ts @@ -7,19 +7,19 @@ import { CollatedWriter } from '@rushstack/stream-collator'; import { Stopwatch } from '../../utilities/Stopwatch'; import { TaskStatus } from './TaskStatus'; import { TaskError } from './TaskError'; -import { BaseBuilder } from './BaseBuilder'; +import { BaseTaskRunner } from './BaseTaskRunner'; /** - * The `Task` class is a node in the dependency graph of work that needs to be scheduled by the `TaskRunner`. - * Each `Task` has a `BaseBuilder` member, whose subclass manages the actual operations for building a single - * project. + * The `Task` class is a node in the dependency graph of work that needs to be scheduled by the + * `TaskExecutionManager`. Each `Task` has a `runner` member of type `BaseTaskRunner`, whose subclass + * manages the actual operations for running a single task. */ export class Task { /** - * When the scheduler is ready to process this `Task`, the `builder` implements the actual work of - * building the project. + * When the scheduler is ready to process this `Task`, the `runner` implements the actual work of + * running the task. */ - public builder: BaseBuilder; + public runner: BaseTaskRunner; /** * The current execution status of a task. Tasks start in the 'ready' state, @@ -67,7 +67,7 @@ export class Task { * Z has a score of 2, since only X depends on it, and X has a score of 1 * Y has a score of 2, since the chain Y->X->C is longer than Y->C * - * The algorithm is implemented in TaskRunner as _calculateCriticalPaths() + * The algorithm is implemented in TaskExecutionManager as _calculateCriticalPaths() */ public criticalPathLength: number | undefined; @@ -89,12 +89,12 @@ export class Task { */ public stopwatch!: Stopwatch; - public constructor(builder: BaseBuilder, initialStatus: TaskStatus) { - this.builder = builder; + public constructor(runner: BaseTaskRunner, initialStatus: TaskStatus) { + this.runner = runner; this.status = initialStatus; } public get name(): string { - return this.builder.name; + return this.runner.name; } } diff --git a/apps/rush-lib/src/logic/taskRunner/TaskCollection.ts b/apps/rush-lib/src/logic/taskExecution/TaskCollection.ts similarity index 90% rename from apps/rush-lib/src/logic/taskRunner/TaskCollection.ts rename to apps/rush-lib/src/logic/taskExecution/TaskCollection.ts index 659ce50543b..3edc2311bb1 100644 --- a/apps/rush-lib/src/logic/taskRunner/TaskCollection.ts +++ b/apps/rush-lib/src/logic/taskExecution/TaskCollection.ts @@ -5,7 +5,7 @@ import { Sort } from '@rushstack/node-core-library'; import { Task } from './Task'; import { TaskStatus } from './TaskStatus'; -import { BaseBuilder } from './BaseBuilder'; +import { BaseTaskRunner } from './BaseTaskRunner'; /** * This class represents a set of tasks with interdependencies. Any class of task definition @@ -22,12 +22,12 @@ export class TaskCollection { /** * Registers a task definition to the map of defined tasks */ - public addTask(builder: BaseBuilder): void { - if (this._tasks.has(builder.name)) { + public addTask(taskRunner: BaseTaskRunner): void { + if (this._tasks.has(taskRunner.name)) { throw new Error('A task with that name has already been registered.'); } - const task: Task = new Task(builder, TaskStatus.Ready); + const task: Task = new Task(taskRunner, TaskStatus.Ready); task.criticalPathLength = undefined; this._tasks.set(task.name, task); } @@ -76,16 +76,13 @@ export class TaskCollection { this._calculateCriticalPaths(task); }); - const buildQueue: Task[] = []; - // Add everything to the buildQueue - this._tasks.forEach((task: Task) => { - buildQueue.push(task); - }); + // Add everything to the queue + const queue: Task[] = [...this._tasks.values()]; // Sort the queue in descending order, nothing will mess with the order - Sort.sortBy(buildQueue, (task: Task): number => -task.criticalPathLength!); + Sort.sortBy(queue, (task: Task): number => -task.criticalPathLength!); - return buildQueue; + return queue; } /** diff --git a/apps/rush-lib/src/logic/taskExecution/TaskError.ts b/apps/rush-lib/src/logic/taskExecution/TaskError.ts new file mode 100644 index 00000000000..de08b87c0cd --- /dev/null +++ b/apps/rush-lib/src/logic/taskExecution/TaskError.ts @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +/** + * Encapsulates information about an error + */ +export class TaskError extends Error { + protected _type: string; + + public constructor(type: string, message: string) { + super(message); + + this._type = type; + } + + public get message(): string { + return `[${this._type}] '${super.message}'`; + } + + public toString(): string { + return this.message; + } +} diff --git a/apps/rush-lib/src/logic/taskRunner/TaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts similarity index 90% rename from apps/rush-lib/src/logic/taskRunner/TaskRunner.ts rename to apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts index ac66037ff65..a52112c2d3f 100644 --- a/apps/rush-lib/src/logic/taskRunner/TaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/TaskExecutionManager.ts @@ -16,34 +16,36 @@ import { AlreadyReportedError, NewlineKind, InternalError, Sort } from '@rushsta import { Stopwatch } from '../../utilities/Stopwatch'; import { Task } from './Task'; import { TaskStatus } from './TaskStatus'; -import { IBuilderContext } from './BaseBuilder'; +import { ITaskRunnerContext } from './BaseTaskRunner'; import { CommandLineConfiguration } from '../../api/CommandLineConfiguration'; import { TaskError } from './TaskError'; -export interface ITaskRunnerOptions { +export interface ITaskExecutionManagerOptions { quietMode: boolean; debugMode: boolean; parallelism: string | undefined; changedProjectsOnly: boolean; - allowWarningsInSuccessfulBuild: boolean; + allowWarningsInSuccessfulExecution: boolean; repoCommandLineConfiguration: CommandLineConfiguration | undefined; destination?: TerminalWritable; } +/** + * Format "======" lines for a shell window with classic 80 columns + */ +const ASCII_HEADER_WIDTH: number = 79; + /** * A class which manages the execution of a set of tasks with interdependencies. * Initially, and at the end of each task execution, all unblocked tasks * are added to a ready queue which is then executed. This is done continually until all * tasks are complete, or prematurely fails if any of the tasks fail. */ -export class TaskRunner { - // Format "======" lines for a shell window with classic 80 columns - private static readonly _ASCII_HEADER_WIDTH: number = 79; - +export class TaskExecutionManager { private readonly _tasks: Task[]; private readonly _changedProjectsOnly: boolean; - private readonly _allowWarningsInSuccessfulBuild: boolean; - private readonly _buildQueue: Task[]; + private readonly _allowWarningsInSuccessfulExecution: boolean; + private readonly _taskQueue: Task[]; private readonly _quietMode: boolean; private readonly _debugMode: boolean; private readonly _parallelism: number; @@ -60,23 +62,23 @@ export class TaskRunner { private _terminal: CollatedTerminal; - public constructor(orderedTasks: Task[], options: ITaskRunnerOptions) { + public constructor(orderedTasks: Task[], options: ITaskExecutionManagerOptions) { const { quietMode, debugMode, parallelism, changedProjectsOnly, - allowWarningsInSuccessfulBuild, + allowWarningsInSuccessfulExecution, repoCommandLineConfiguration } = options; this._tasks = orderedTasks; - this._buildQueue = orderedTasks.slice(0); + this._taskQueue = [...orderedTasks]; // Clone the task array this._quietMode = quietMode; this._debugMode = debugMode; this._hasAnyFailures = false; this._hasAnyWarnings = false; this._changedProjectsOnly = changedProjectsOnly; - this._allowWarningsInSuccessfulBuild = allowWarningsInSuccessfulBuild; + this._allowWarningsInSuccessfulExecution = allowWarningsInSuccessfulExecution; this._repoCommandLineConfiguration = repoCommandLineConfiguration; // TERMINAL PIPELINE: @@ -145,7 +147,7 @@ export class TaskRunner { // middlePart: "]=================[" const twoBracketsLength: number = 2; const middlePartLengthMinusTwoBrackets: number = Math.max( - TaskRunner._ASCII_HEADER_WIDTH - (leftPartLength + rightPartLength + twoBracketsLength), + ASCII_HEADER_WIDTH - (leftPartLength + rightPartLength + twoBracketsLength), 0 ); @@ -189,28 +191,28 @@ export class TaskRunner { if (this._hasAnyFailures) { this._terminal.writeStderrLine(colors.red('Projects failed to build.') + '\n'); throw new AlreadyReportedError(); - } else if (this._hasAnyWarnings && !this._allowWarningsInSuccessfulBuild) { + } else if (this._hasAnyWarnings && !this._allowWarningsInSuccessfulExecution) { this._terminal.writeStderrLine(colors.yellow('Projects succeeded with warnings.') + '\n'); throw new AlreadyReportedError(); } } /** - * Pulls the next task with no dependencies off the build queue - * Removes any non-ready tasks from the build queue (this should only be blocked tasks) + * Pulls the next task with no dependencies off the task queue + * Removes any non-ready tasks from the task queue (this should only be blocked tasks) */ private _getNextTask(): Task | undefined { - for (let i: number = 0; i < this._buildQueue.length; i++) { - const task: Task = this._buildQueue[i]; + for (let i: number = 0; i < this._taskQueue.length; i++) { + const task: Task = this._taskQueue[i]; if (task.status !== TaskStatus.Ready) { // It shouldn't be on the queue, remove it - this._buildQueue.splice(i, 1); + this._taskQueue.splice(i, 1); // Decrement since we modified the array i--; } else if (task.dependencies.size === 0 && task.status === TaskStatus.Ready) { // this is a task which is ready to go. remove it and return it - return this._buildQueue.splice(i, 1)[0]; + return this._taskQueue.splice(i, 1)[0]; } // Otherwise task is still waiting } @@ -223,10 +225,10 @@ export class TaskRunner { */ private async _startAvailableTasksAsync(): Promise { const taskPromises: Promise[] = []; - let ctask: Task | undefined; - while (this._currentActiveTasks < this._parallelism && (ctask = this._getNextTask())) { + let currentTask: Task | undefined; + while (this._currentActiveTasks < this._parallelism && (currentTask = this._getNextTask())) { this._currentActiveTasks++; - const task: Task = ctask; + const task: Task = currentTask; task.status = TaskStatus.Executing; task.stopwatch = Stopwatch.start(); @@ -240,7 +242,7 @@ export class TaskRunner { } private async _executeTaskAndChainAsync(task: Task): Promise { - const context: IBuilderContext = { + const context: ITaskRunnerContext = { repoCommandLineConfiguration: this._repoCommandLineConfiguration, stdioSummarizer: task.stdioSummarizer, collatedWriter: task.collatedWriter, @@ -249,7 +251,7 @@ export class TaskRunner { }; try { - const result: TaskStatus = await task.builder.executeAsync(context); + const result: TaskStatus = await task.runner.executeAsync(context); task.stopwatch.stop(); task.stdioSummarizer.close(); @@ -328,7 +330,7 @@ export class TaskRunner { * Marks a task as being completed, and removes it from the dependencies list of all its dependents */ private _markTaskAsSuccess(task: Task): void { - if (task.builder.hadEmptyScript) { + if (task.runner.hadEmptyScript) { task.collatedWriter.terminal.writeStdoutLine(colors.green(`"${task.name}" had an empty script.`)); } else { task.collatedWriter.terminal.writeStdoutLine( @@ -339,7 +341,7 @@ export class TaskRunner { task.dependents.forEach((dependent: Task) => { if (!this._changedProjectsOnly) { - dependent.builder.isSkipAllowed = false; + dependent.runner.isSkipAllowed = false; } dependent.dependencies.delete(task); }); @@ -356,7 +358,7 @@ export class TaskRunner { task.status = TaskStatus.SuccessWithWarning; task.dependents.forEach((dependent: Task) => { if (!this._changedProjectsOnly) { - dependent.builder.isSkipAllowed = false; + dependent.runner.isSkipAllowed = false; } dependent.dependencies.delete(task); }); @@ -481,7 +483,7 @@ export class TaskRunner { const longestTaskName: number = Math.max(...tasks.map((x) => x.name.length)); for (const task of tasks) { - if (task.stopwatch && !task.builder.hadEmptyScript && task.status !== TaskStatus.Skipped) { + if (task.stopwatch && !task.runner.hadEmptyScript && task.status !== TaskStatus.Skipped) { const time: string = task.stopwatch.toString(); const padding: string = ' '.repeat(longestTaskName - task.name.length); this._terminal.writeStdoutLine(` ${task.name}${padding} ${time}`); @@ -504,7 +506,7 @@ export class TaskRunner { // // --[ WARNINGS: f ]------------------------------------[ 5.07 seconds ]-- // - // [eslint] Warning: src/logic/taskRunner/TaskRunner.ts:393:3 ... + // [eslint] Warning: src/logic/taskExecution/TaskExecutionManager.ts:393:3 ... const tasks: Task[] | undefined = tasksByStatus[status]; if (!tasks || tasks.length === 0) { @@ -536,7 +538,7 @@ export class TaskRunner { // middlePart: "]----------------------[" const twoBracketsLength: number = 2; const middlePartLengthMinusTwoBrackets: number = Math.max( - TaskRunner._ASCII_HEADER_WIDTH - (leftPartLength + rightPartLength + twoBracketsLength), + ASCII_HEADER_WIDTH - (leftPartLength + rightPartLength + twoBracketsLength), 0 ); @@ -571,10 +573,7 @@ export class TaskRunner { const leftPart: string = colors.gray('==[') + ' ' + headingColor(headingText) + ' '; const leftPartLength: number = 3 + 1 + headingText.length + 1; - const rightPartLengthMinusBracket: number = Math.max( - TaskRunner._ASCII_HEADER_WIDTH - (leftPartLength + 1), - 0 - ); + const rightPartLengthMinusBracket: number = Math.max(ASCII_HEADER_WIDTH - (leftPartLength + 1), 0); // rightPart: "]======================" const rightPart: string = colors.gray(']' + '='.repeat(rightPartLengthMinusBracket)); diff --git a/apps/rush-lib/src/logic/taskRunner/TaskStatus.ts b/apps/rush-lib/src/logic/taskExecution/TaskStatus.ts similarity index 100% rename from apps/rush-lib/src/logic/taskRunner/TaskStatus.ts rename to apps/rush-lib/src/logic/taskExecution/TaskStatus.ts diff --git a/apps/rush-lib/src/logic/taskRunner/test/MockBuilder.ts b/apps/rush-lib/src/logic/taskExecution/test/MockTaskRunner.ts similarity index 80% rename from apps/rush-lib/src/logic/taskRunner/test/MockBuilder.ts rename to apps/rush-lib/src/logic/taskExecution/test/MockTaskRunner.ts index 7857aae696e..0d6348166e4 100644 --- a/apps/rush-lib/src/logic/taskRunner/test/MockBuilder.ts +++ b/apps/rush-lib/src/logic/taskExecution/test/MockTaskRunner.ts @@ -4,9 +4,9 @@ import { CollatedTerminal } from '@rushstack/stream-collator'; import { TaskStatus } from '../TaskStatus'; -import { BaseBuilder, IBuilderContext } from '../BaseBuilder'; +import { BaseTaskRunner, ITaskRunnerContext } from '../BaseTaskRunner'; -export class MockBuilder extends BaseBuilder { +export class MockTaskRunner extends BaseTaskRunner { private readonly _action: ((terminal: CollatedTerminal) => Promise) | undefined; public readonly name: string; public readonly hadEmptyScript: boolean = false; @@ -19,7 +19,7 @@ export class MockBuilder extends BaseBuilder { this._action = action; } - public async executeAsync(context: IBuilderContext): Promise { + public async executeAsync(context: ITaskRunnerContext): Promise { let result: TaskStatus | void; if (this._action) { result = await this._action(context.collatedWriter.terminal); diff --git a/apps/rush-lib/src/logic/taskRunner/test/ProjectTask.test.ts b/apps/rush-lib/src/logic/taskExecution/test/ProjectTaskRunner.test.ts similarity index 95% rename from apps/rush-lib/src/logic/taskRunner/test/ProjectTask.test.ts rename to apps/rush-lib/src/logic/taskExecution/test/ProjectTaskRunner.test.ts index c3481dac14a..1a0cb3d7f08 100644 --- a/apps/rush-lib/src/logic/taskRunner/test/ProjectTask.test.ts +++ b/apps/rush-lib/src/logic/taskExecution/test/ProjectTaskRunner.test.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { convertSlashesForWindows } from '../ProjectBuilder'; +import { convertSlashesForWindows } from '../ProjectTaskRunner'; describe('convertSlashesForWindows()', () => { it('converted inputs', () => { diff --git a/apps/rush-lib/src/logic/taskRunner/test/TaskCollection.test.ts b/apps/rush-lib/src/logic/taskExecution/test/TaskCollection.test.ts similarity index 87% rename from apps/rush-lib/src/logic/taskRunner/test/TaskCollection.test.ts rename to apps/rush-lib/src/logic/taskExecution/test/TaskCollection.test.ts index 8aed7901b64..9bcb66bfac6 100644 --- a/apps/rush-lib/src/logic/taskRunner/test/TaskCollection.test.ts +++ b/apps/rush-lib/src/logic/taskExecution/test/TaskCollection.test.ts @@ -4,7 +4,7 @@ import { TaskCollection } from '../TaskCollection'; import { Task } from '../Task'; import { StringBufferTerminalProvider } from '@rushstack/node-core-library'; -import { MockBuilder } from './MockBuilder'; +import { MockTaskRunner } from './MockTaskRunner'; import { TaskStatus } from '../TaskStatus'; function checkConsoleOutput(terminalProvider: StringBufferTerminalProvider): void { @@ -32,13 +32,13 @@ describe('TaskCollection', () => { }); it('throwsErrorOnNonExistentDependency', () => { - taskCollection.addTask(new MockBuilder('foo')); + taskCollection.addTask(new MockTaskRunner('foo')); expect(() => taskCollection.addDependencies('foo', ['bar'])).toThrowErrorMatchingSnapshot(); }); it('detectsDependencyCycle', () => { - taskCollection.addTask(new MockBuilder('foo')); - taskCollection.addTask(new MockBuilder('bar')); + taskCollection.addTask(new MockTaskRunner('foo')); + taskCollection.addTask(new MockTaskRunner('bar')); taskCollection.addDependencies('foo', ['bar']); taskCollection.addDependencies('bar', ['foo']); expect(() => taskCollection.getOrderedTasks()).toThrowErrorMatchingSnapshot(); @@ -47,13 +47,13 @@ describe('TaskCollection', () => { it('respectsDependencyOrder', () => { const result: string[] = []; taskCollection.addTask( - new MockBuilder('two', async () => { + new MockTaskRunner('two', async () => { result.push('2'); return TaskStatus.Success; }) ); taskCollection.addTask( - new MockBuilder('one', async () => { + new MockTaskRunner('one', async () => { result.push('1'); return TaskStatus.Success; }) diff --git a/apps/rush-lib/src/logic/taskRunner/test/TaskRunner.test.ts b/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts similarity index 68% rename from apps/rush-lib/src/logic/taskRunner/test/TaskRunner.test.ts rename to apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts index bf8e836c734..f444f062c08 100644 --- a/apps/rush-lib/src/logic/taskRunner/test/TaskRunner.test.ts +++ b/apps/rush-lib/src/logic/taskExecution/test/TaskExecutionManager.test.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -// The TaskRunner prints "x.xx seconds" in TestRunner.test.ts.snap; ensure that the Stopwatch timing is deterministic +// The TaskExecutionManager prints "x.xx seconds" in TestRunner.test.ts.snap; ensure that the Stopwatch timing is deterministic jest.mock('../../../utilities/Utilities'); import colors from 'colors/safe'; @@ -9,12 +9,12 @@ import { EOL } from 'os'; import { CollatedTerminal } from '@rushstack/stream-collator'; import { MockWritable } from '@rushstack/terminal'; -import { TaskRunner, ITaskRunnerOptions } from '../TaskRunner'; +import { TaskExecutionManager, ITaskExecutionManagerOptions } from '../TaskExecutionManager'; import { TaskStatus } from '../TaskStatus'; import { Task } from '../Task'; import { Utilities } from '../../../utilities/Utilities'; -import { BaseBuilder } from '../BaseBuilder'; -import { MockBuilder } from './MockBuilder'; +import { BaseTaskRunner } from '../BaseTaskRunner'; +import { MockTaskRunner } from './MockTaskRunner'; const mockGetTimeInMs: jest.Mock = jest.fn(); Utilities.getTimeInMs = mockGetTimeInMs; @@ -28,17 +28,20 @@ mockGetTimeInMs.mockImplementation(() => { const mockWritable: MockWritable = new MockWritable(); -function createTaskRunner(taskRunnerOptions: ITaskRunnerOptions, builder: BaseBuilder): TaskRunner { - const task: Task = new Task(builder, TaskStatus.Ready); +function createTaskExecutionManager( + taskExecutionManagerOptions: ITaskExecutionManagerOptions, + taskRunner: BaseTaskRunner +): TaskExecutionManager { + const task: Task = new Task(taskRunner, TaskStatus.Ready); - return new TaskRunner([task], taskRunnerOptions); + return new TaskExecutionManager([task], taskExecutionManagerOptions); } -const EXPECTED_FAIL: string = `Promise returned by ${TaskRunner.prototype.executeAsync.name}() resolved but was expected to fail`; +const EXPECTED_FAIL: string = `Promise returned by ${TaskExecutionManager.prototype.executeAsync.name}() resolved but was expected to fail`; -describe('TaskRunner', () => { - let taskRunner: TaskRunner; - let taskRunnerOptions: ITaskRunnerOptions; +describe(TaskExecutionManager.name, () => { + let taskExecutionManager: TaskExecutionManager; + let taskExecutionManagerOptions: ITaskExecutionManagerOptions; let initialColorsEnabled: boolean; @@ -61,13 +64,13 @@ describe('TaskRunner', () => { it('throwsErrorOnInvalidParallelism', () => { expect( () => - new TaskRunner([], { + new TaskExecutionManager([], { quietMode: false, debugMode: false, parallelism: 'tequila', changedProjectsOnly: false, destination: mockWritable, - allowWarningsInSuccessfulBuild: false, + allowWarningsInSuccessfulExecution: false, repoCommandLineConfiguration: undefined }) ).toThrowErrorMatchingSnapshot(); @@ -76,21 +79,21 @@ describe('TaskRunner', () => { describe('Error logging', () => { beforeEach(() => { - taskRunnerOptions = { + taskExecutionManagerOptions = { quietMode: false, debugMode: false, parallelism: '1', changedProjectsOnly: false, destination: mockWritable, - allowWarningsInSuccessfulBuild: false, + allowWarningsInSuccessfulExecution: false, repoCommandLineConfiguration: undefined }; }); it('printedStderrAfterError', async () => { - taskRunner = createTaskRunner( - taskRunnerOptions, - new MockBuilder('stdout+stderr', async (terminal: CollatedTerminal) => { + taskExecutionManager = createTaskExecutionManager( + taskExecutionManagerOptions, + new MockTaskRunner('stdout+stderr', async (terminal: CollatedTerminal) => { terminal.writeStdoutLine('Build step 1' + EOL); terminal.writeStderrLine('Error: step 1 failed' + EOL); return TaskStatus.Failure; @@ -98,7 +101,7 @@ describe('TaskRunner', () => { ); try { - await taskRunner.executeAsync(); + await taskExecutionManager.executeAsync(); fail(EXPECTED_FAIL); } catch (err) { expect((err as Error).message).toMatchSnapshot(); @@ -109,9 +112,9 @@ describe('TaskRunner', () => { }); it('printedStdoutAfterErrorWithEmptyStderr', async () => { - taskRunner = createTaskRunner( - taskRunnerOptions, - new MockBuilder('stdout only', async (terminal: CollatedTerminal) => { + taskExecutionManager = createTaskExecutionManager( + taskExecutionManagerOptions, + new MockTaskRunner('stdout only', async (terminal: CollatedTerminal) => { terminal.writeStdoutLine('Build step 1' + EOL); terminal.writeStdoutLine('Error: step 1 failed' + EOL); return TaskStatus.Failure; @@ -119,7 +122,7 @@ describe('TaskRunner', () => { ); try { - await taskRunner.executeAsync(); + await taskExecutionManager.executeAsync(); fail(EXPECTED_FAIL); } catch (err) { expect((err as Error).message).toMatchSnapshot(); @@ -134,21 +137,21 @@ describe('TaskRunner', () => { describe('Warning logging', () => { describe('Fail on warning', () => { beforeEach(() => { - taskRunnerOptions = { + taskExecutionManagerOptions = { quietMode: false, debugMode: false, parallelism: '1', changedProjectsOnly: false, destination: mockWritable, - allowWarningsInSuccessfulBuild: false, + allowWarningsInSuccessfulExecution: false, repoCommandLineConfiguration: undefined }; }); it('Logs warnings correctly', async () => { - taskRunner = createTaskRunner( - taskRunnerOptions, - new MockBuilder('success with warnings (failure)', async (terminal: CollatedTerminal) => { + taskExecutionManager = createTaskExecutionManager( + taskExecutionManagerOptions, + new MockTaskRunner('success with warnings (failure)', async (terminal: CollatedTerminal) => { terminal.writeStdoutLine('Build step 1' + EOL); terminal.writeStdoutLine('Warning: step 1 succeeded with warnings' + EOL); return TaskStatus.SuccessWithWarning; @@ -156,7 +159,7 @@ describe('TaskRunner', () => { ); try { - await taskRunner.executeAsync(); + await taskExecutionManager.executeAsync(); fail(EXPECTED_FAIL); } catch (err) { expect((err as Error).message).toMatchSnapshot(); @@ -170,28 +173,28 @@ describe('TaskRunner', () => { describe('Success on warning', () => { beforeEach(() => { - taskRunnerOptions = { + taskExecutionManagerOptions = { quietMode: false, debugMode: false, parallelism: '1', changedProjectsOnly: false, destination: mockWritable, - allowWarningsInSuccessfulBuild: true, + allowWarningsInSuccessfulExecution: true, repoCommandLineConfiguration: undefined }; }); it('Logs warnings correctly', async () => { - taskRunner = createTaskRunner( - taskRunnerOptions, - new MockBuilder('success with warnings (success)', async (terminal: CollatedTerminal) => { + taskExecutionManager = createTaskExecutionManager( + taskExecutionManagerOptions, + new MockTaskRunner('success with warnings (success)', async (terminal: CollatedTerminal) => { terminal.writeStdoutLine('Build step 1' + EOL); terminal.writeStdoutLine('Warning: step 1 succeeded with warnings' + EOL); return TaskStatus.SuccessWithWarning; }) ); - await taskRunner.executeAsync(); + await taskExecutionManager.executeAsync(); const allMessages: string = mockWritable.getAllOutput(); expect(allMessages).toContain('Build step 1'); expect(allMessages).toContain('Warning: step 1 succeeded with warnings'); diff --git a/apps/rush-lib/src/logic/taskRunner/test/__snapshots__/TaskCollection.test.ts.snap b/apps/rush-lib/src/logic/taskExecution/test/__snapshots__/TaskCollection.test.ts.snap similarity index 100% rename from apps/rush-lib/src/logic/taskRunner/test/__snapshots__/TaskCollection.test.ts.snap rename to apps/rush-lib/src/logic/taskExecution/test/__snapshots__/TaskCollection.test.ts.snap diff --git a/apps/rush-lib/src/logic/taskRunner/test/__snapshots__/TaskRunner.test.ts.snap b/apps/rush-lib/src/logic/taskExecution/test/__snapshots__/TaskExecutionManager.test.ts.snap similarity index 88% rename from apps/rush-lib/src/logic/taskRunner/test/__snapshots__/TaskRunner.test.ts.snap rename to apps/rush-lib/src/logic/taskExecution/test/__snapshots__/TaskExecutionManager.test.ts.snap index 1411923b6eb..72b96e57e72 100644 --- a/apps/rush-lib/src/logic/taskRunner/test/__snapshots__/TaskRunner.test.ts.snap +++ b/apps/rush-lib/src/logic/taskExecution/test/__snapshots__/TaskExecutionManager.test.ts.snap @@ -1,10 +1,10 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`TaskRunner Constructor throwsErrorOnInvalidParallelism 1`] = `"Invalid parallelism value of 'tequila', expected a number or 'max'"`; +exports[`TaskExecutionManager Constructor throwsErrorOnInvalidParallelism 1`] = `"Invalid parallelism value of 'tequila', expected a number or 'max'"`; -exports[`TaskRunner Error logging printedStderrAfterError 1`] = `"An error occurred."`; +exports[`TaskExecutionManager Error logging printedStderrAfterError 1`] = `"An error occurred."`; -exports[`TaskRunner Error logging printedStderrAfterError 2`] = ` +exports[`TaskExecutionManager Error logging printedStderrAfterError 2`] = ` Array [ Object { "kind": "O", @@ -104,9 +104,9 @@ Array [ ] `; -exports[`TaskRunner Error logging printedStdoutAfterErrorWithEmptyStderr 1`] = `"An error occurred."`; +exports[`TaskExecutionManager Error logging printedStdoutAfterErrorWithEmptyStderr 1`] = `"An error occurred."`; -exports[`TaskRunner Error logging printedStdoutAfterErrorWithEmptyStderr 2`] = ` +exports[`TaskExecutionManager Error logging printedStdoutAfterErrorWithEmptyStderr 2`] = ` Array [ Object { "kind": "O", @@ -206,9 +206,9 @@ Array [ ] `; -exports[`TaskRunner Warning logging Fail on warning Logs warnings correctly 1`] = `"An error occurred."`; +exports[`TaskExecutionManager Warning logging Fail on warning Logs warnings correctly 1`] = `"An error occurred."`; -exports[`TaskRunner Warning logging Fail on warning Logs warnings correctly 2`] = ` +exports[`TaskExecutionManager Warning logging Fail on warning Logs warnings correctly 2`] = ` Array [ Object { "kind": "O", @@ -308,7 +308,7 @@ Array [ ] `; -exports[`TaskRunner Warning logging Success on warning Logs warnings correctly 1`] = ` +exports[`TaskExecutionManager Warning logging Success on warning Logs warnings correctly 1`] = ` Array [ Object { "kind": "O", diff --git a/apps/rush-lib/src/logic/taskRunner/TaskError.ts b/apps/rush-lib/src/logic/taskRunner/TaskError.ts deleted file mode 100644 index 3496edabab5..00000000000 --- a/apps/rush-lib/src/logic/taskRunner/TaskError.ts +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -/** - * Encapsulates information about an error - */ -export class TaskError extends Error { - protected _type: string; - - public constructor(type: string, message: string) { - super(message); - - this._type = type; - } - - public get message(): string { - return `[${this._type}] '${super.message}'`; - } - - public toString(): string { - return this.message; - } -} - -/** - * TestTaskError extends TaskError - */ -export class BuildTaskError extends TaskError { - protected _file: string; - protected _line: number; - protected _offset: number; - - public constructor(type: string, message: string, file: string, line: number, offset: number) { - super(type, message); - this._file = file; - this._line = line; - this._offset = offset; - } - - public get message(): string { - // Example: "C:\Project\Blah.ts(123,1): [tslint] error no-any: 'any' is not allowed" - return `${this._file}(${this._line},${this._offset}): ${super.message}`; - } - - public toString(): string { - return this.message; - } -} From e99fc22372a09eddf7166cb1e4805861d5c35ba6 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Mon, 20 Dec 2021 18:03:17 -0800 Subject: [PATCH 2/5] Refactor the TaskSelector to be adapted to multi-phase builds, and plumb the log filename through from the command line parser. --- .../rush-lib/src/cli/RushCommandLineParser.ts | 89 +++++++--- .../src/cli/actions/WriteBuildCacheAction.ts | 15 +- .../src/cli/scriptActions/BulkScriptAction.ts | 19 ++- .../cli/test/RushCommandLineParser.test.ts | 29 +++- .../a/package.json | 9 + .../b/package.json | 12 ++ .../common/config/rush/command-line.json | 16 ++ .../rush.json | 17 ++ .../src/logic/NonPhasedProjectTaskSelector.ts | 111 ++++++++++++ apps/rush-lib/src/logic/TaskSelector.ts | 161 ------------------ apps/rush-lib/src/logic/TaskSelectorBase.ts | 83 +++++++++ .../logic/taskExecution/ProjectLogWritable.ts | 4 + .../logic/taskExecution/ProjectTaskRunner.ts | 17 +- 13 files changed, 374 insertions(+), 208 deletions(-) create mode 100644 apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/a/package.json create mode 100644 apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/b/package.json create mode 100644 apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/common/config/rush/command-line.json create mode 100644 apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/rush.json create mode 100644 apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts delete mode 100644 apps/rush-lib/src/logic/TaskSelector.ts create mode 100644 apps/rush-lib/src/logic/TaskSelectorBase.ts diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts index f29eead71e5..c47bd222af4 100644 --- a/apps/rush-lib/src/cli/RushCommandLineParser.ts +++ b/apps/rush-lib/src/cli/RushCommandLineParser.ts @@ -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`. @@ -70,6 +71,8 @@ export class RushCommandLineParser extends CommandLineParser { private _rushOptions: IRushCommandLineParserOptions; private _terminalProvider: ConsoleTerminalProvider; private _terminal: Terminal; + private readonly _commandNameForLogFilenamesToCommandNameMap: Map> = new Map(); + private readonly _commandNamesWithLogFilenameCollisions: Set = new Set(); public constructor(options?: Partial) { super({ @@ -250,6 +253,24 @@ export class RushCommandLineParser extends CommandLineParser { // Build actions from the command line configuration supersede default build actions. this._addCommandLineConfigActions(commandLineConfiguration); this._addDefaultBuildActions(commandLineConfiguration); + + if (this._commandNamesWithLogFilenameCollisions.size > 0) { + const errorMessageParts: string[] = []; + for (const commandNameWithLogFilenameCollisions of this._commandNamesWithLogFilenameCollisions) { + const collidingCommands: string[] = Array.from( + this._commandNameForLogFilenamesToCommandNameMap.get(commandNameWithLogFilenameCollisions)! + ); + errorMessageParts.push( + `- [${collidingCommands.join(', ')}] will all write to ` + + `".${commandNameWithLogFilenameCollisions}.log" files` + ); + } + + throw new Error( + `The following command names will produce log files with names that will ` + + `collide:\n${errorMessageParts.join('\n')}` + ); + } } private _addDefaultBuildActions(commandLineConfiguration?: CommandLineConfiguration): void { @@ -292,15 +313,28 @@ 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 commandNameForLogFilenames: string = this._getCommandNameForLogFilenames(command, commandToRun); + this.addAction( new BulkScriptAction({ actionName: command.name, + commandNameForLogFilenames, // By default, the "rebuild" action runs the "build" script. However, if the command-line.json file // overrides "rebuild," the "rebuild" script should be run. @@ -327,6 +361,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, @@ -367,31 +412,29 @@ 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}".` + private _getCommandNameForLogFilenames(command: CommandJson, commandToRun: string = command.name): string { + const commandNameForLogFilenames: string = ProjectLogWritable.normalizeCommandNameForLogFilenames( + // "rebuild" and "build" should write to the same log file ("build.log") + commandToRun + ); + + // Ensure two log files won't collide + let commandNamesForCommandNameForLogFilenames: Set | undefined = + this._commandNameForLogFilenamesToCommandNameMap.get(commandNameForLogFilenames); + if (!commandNamesForCommandNameForLogFilenames) { + commandNamesForCommandNameForLogFilenames = new Set(); + this._commandNameForLogFilenamesToCommandNameMap.set( + commandNameForLogFilenames, + commandNamesForCommandNameForLogFilenames ); } - if (command.safeForSimultaneousRushProcesses) { - throw new Error( - `${RushConstants.commandLineFilename} defines a command "${command.name}" using ` + - `"safeForSimultaneousRushProcesses=true". This configuration is not supported for "${command.name}".` - ); + + commandNamesForCommandNameForLogFilenames.add(commandToRun); + if (commandNamesForCommandNameForLogFilenames.size > 1) { + this._commandNamesWithLogFilenameCollisions.add(commandNameForLogFilenames); } + + return commandNameForLogFilenames; } private _reportErrorAndSetExitCode(error: Error): void { diff --git a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts index cd62431f139..445b1410b01 100644 --- a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts +++ b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts @@ -13,9 +13,10 @@ import { BuildCacheConfiguration } from '../../api/BuildCacheConfiguration'; 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; @@ -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: 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), + commandNameForLogFilenames: ProjectLogWritable.normalizeCommandNameForLogFilenames(command) }); const trackedFiles: string[] = Array.from( diff --git a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts index c5804964ee9..979462b846a 100644 --- a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts @@ -9,7 +9,10 @@ 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 { @@ -37,6 +40,7 @@ export interface IBulkScriptActionOptions extends IBaseScriptActionOptions { allowWarningsInSuccessfulBuild: boolean; watchForChanges: boolean; disableBuildCache: boolean; + commandNameForLogFilenames: string; /** * Optional command to run. Otherwise, use the `actionName` as the command to run. @@ -45,7 +49,7 @@ export interface IBulkScriptActionOptions extends IBaseScriptActionOptions { } interface IExecuteInternalOptions { - taskSelectorOptions: ITaskSelectorOptions; + taskSelectorOptions: INonPhasedProjectTaskSelectorOptions; taskExecutionManagerOptions: ITaskExecutionManagerOptions; stopwatch: Stopwatch; ignoreHooks?: boolean; @@ -71,6 +75,7 @@ export class BulkScriptAction extends BaseScriptAction { private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined; private readonly _ignoreDependencyOrder: boolean; private readonly _allowWarningsInSuccessfulBuild: boolean; + private readonly _commandNameForLogFilenames: string; private _changedProjectsOnly!: CommandLineFlagParameter; private _selectionParameters!: SelectionParameterSet; @@ -89,6 +94,7 @@ export class BulkScriptAction extends BaseScriptAction { this._watchForChanges = options.watchForChanges; this._disableBuildCache = options.disableBuildCache; this._repoCommandLineConfiguration = options.commandLineConfiguration; + this._commandNameForLogFilenames = options.commandNameForLogFilenames; } public async runAsync(): Promise { @@ -142,7 +148,7 @@ export class BulkScriptAction extends BaseScriptAction { return; } - const taskSelectorOptions: ITaskSelectorOptions = { + const taskSelectorOptions: INonPhasedProjectTaskSelectorOptions = { rushConfiguration: this.rushConfiguration, buildCacheConfiguration, selection, @@ -155,7 +161,8 @@ export class BulkScriptAction extends BaseScriptAction { ignoreMissingScript: this._ignoreMissingScript, ignoreDependencyOrder: this._ignoreDependencyOrder, allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild, - packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(this._commandToRun) + packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(this._commandToRun), + commandNameForLogFilenames: this._commandNameForLogFilenames }; const taskExecutionManagerOptions: ITaskExecutionManagerOptions = { @@ -321,7 +328,9 @@ export class BulkScriptAction extends BaseScriptAction { * Runs a single invocation of the command */ private async _runOnce(options: IExecuteInternalOptions): Promise { - const taskSelector: TaskSelector = new TaskSelector(options.taskSelectorOptions); + const taskSelector: NonPhasedProjectTaskSelector = new NonPhasedProjectTaskSelector( + options.taskSelectorOptions + ); // Register all tasks with the task collection diff --git a/apps/rush-lib/src/cli/test/RushCommandLineParser.test.ts b/apps/rush-lib/src/cli/test/RushCommandLineParser.test.ts index d2d973bc048..59e942d4a0e 100644 --- a/apps/rush-lib/src/cli/test/RushCommandLineParser.test.ts +++ b/apps/rush-lib/src/cli/test/RushCommandLineParser.test.ts @@ -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\\"."` + ); }); }); @@ -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\\"."` + ); }); }); @@ -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\\"."` + ); }); }); @@ -321,7 +327,22 @@ 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\\"."` + ); + }); + }); + + describe(`in repo with two commands that will produce colliding log filenames`, () => { + it('throws an error when starting Rush', async () => { + const repoName: string = 'commandsWithCollidingLogNamesRepo'; + + await expect(() => { + getCommandLineParserInstance(repoName, 'doesnt-matter'); + }).toThrowErrorMatchingInlineSnapshot(` + "The following command names will produce log files with names that will collide: + - [command:a, command-a] will all write to \\".command_a.log\\" files" + `); }); }); }); diff --git a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/a/package.json b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/a/package.json new file mode 100644 index 00000000000..84b3f0dd05c --- /dev/null +++ b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/a/package.json @@ -0,0 +1,9 @@ +{ + "name": "a", + "version": "1.0.0", + "description": "Test package a", + "scripts": { + "command:a": "fake_build_task_but_works_with_mock", + "command-a": "fake_build_task_but_works_with_mock" + } +} diff --git a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/b/package.json b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/b/package.json new file mode 100644 index 00000000000..859d606b22f --- /dev/null +++ b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/b/package.json @@ -0,0 +1,12 @@ +{ + "name": "b", + "version": "1.0.0", + "description": "Test package b", + "dependencies": { + "a": "1.0.0" + }, + "scripts": { + "command:a": "fake_build_task_but_works_with_mock", + "command-a": "fake_build_task_but_works_with_mock" + } +} diff --git a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/common/config/rush/command-line.json b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/common/config/rush/command-line.json new file mode 100644 index 00000000000..43f9ad1bb71 --- /dev/null +++ b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/common/config/rush/command-line.json @@ -0,0 +1,16 @@ +{ + "commands": [ + { + "name": "command:a", + "commandKind": "bulk", + "summary": "Test command", + "enableParallelism": true + }, + { + "name": "command-a", + "commandKind": "bulk", + "summary": "Test command", + "enableParallelism": true + } + ] +} diff --git a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/rush.json b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/rush.json new file mode 100644 index 00000000000..f39da1606c4 --- /dev/null +++ b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/rush.json @@ -0,0 +1,17 @@ +{ + "npmVersion": "6.4.1", + "rushVersion": "5.5.2", + "projectFolderMinDepth": 1, + "projectFolderMaxDepth": 99, + + "projects": [ + { + "packageName": "a", + "projectFolder": "a" + }, + { + "packageName": "b", + "projectFolder": "b" + } + ] +} diff --git a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts new file mode 100644 index 00000000000..45d53f18219 --- /dev/null +++ b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts @@ -0,0 +1,111 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { ITaskSelectorOptions, TaskSelectorBase } from './TaskSelectorBase'; +import { RushConfigurationProject } from '../api/RushConfigurationProject'; +import { TaskCollection } from './taskExecution/TaskCollection'; +import { ProjectTaskRunner } from './taskExecution/ProjectTaskRunner'; + +export interface INonPhasedProjectTaskSelectorOptions extends ITaskSelectorOptions { + commandNameForLogFilenames: string; +} + +export class NonPhasedProjectTaskSelector extends TaskSelectorBase { + private readonly _commandNameForLogFilenames: string; + + public constructor(options: INonPhasedProjectTaskSelectorOptions) { + super(options); + this._commandNameForLogFilenames = options.commandNameForLogFilenames; + } + + public registerTasks(): TaskCollection { + const projects: ReadonlySet = this._options.selection; + const taskCollection: TaskCollection = new TaskCollection(); + + // Register all tasks + for (const rushProject of projects) { + this._registerProjectTask(rushProject, taskCollection); + } + + if (!this._options.ignoreDependencyOrder) { + const dependencyMap: Map> = new Map(); + + // Generate the filtered dependency graph for selected projects + function getDependencyTaskNames(project: RushConfigurationProject): Set { + const cached: Set | undefined = dependencyMap.get(project); + if (cached) { + return cached; + } + + const dependencyTaskNames: Set = new Set(); + dependencyMap.set(project, dependencyTaskNames); + + for (const dep of project.dependencyProjects) { + if (projects.has(dep)) { + // Add direct relationships for projects in the set + dependencyTaskNames.add(NonPhasedProjectTaskSelector.getTaskNameForProject(dep)); + } else { + // Add indirect relationships for projects not in the set + for (const indirectDep of getDependencyTaskNames(dep)) { + dependencyTaskNames.add(indirectDep); + } + } + } + + return dependencyTaskNames; + } + + // Add ordering relationships for each dependency + for (const project of projects) { + taskCollection.addDependencies( + NonPhasedProjectTaskSelector.getTaskNameForProject(project), + getDependencyTaskNames(project) + ); + } + } + + return taskCollection; + } + + private _registerProjectTask(project: RushConfigurationProject, taskCollection: TaskCollection): void { + const taskName: string = NonPhasedProjectTaskSelector.getTaskNameForProject(project); + if (taskCollection.hasTask(taskName)) { + return; + } + + const commandToRun: string | undefined = TaskSelectorBase.getScriptToRun( + project, + this._options.commandToRun, + this._options.customParameterValues + ); + if (commandToRun === undefined && !this._options.ignoreMissingScript) { + throw new Error( + `The project [${project.packageName}] does not define a '${this._options.commandToRun}' command in the 'scripts' section of its package.json` + ); + } + + taskCollection.addTask( + new ProjectTaskRunner({ + rushProject: project, + taskName, + rushConfiguration: this._options.rushConfiguration, + buildCacheConfiguration: this._options.buildCacheConfiguration, + commandToRun: commandToRun || '', + commandName: this._options.commandName, + isIncrementalBuildAllowed: this._options.isIncrementalBuildAllowed, + projectChangeAnalyzer: this._projectChangeAnalyzer, + packageDepsFilename: this._options.packageDepsFilename, + allowWarningsInSuccessfulBuild: this._options.allowWarningsInSuccessfulBuild, + commandNameForLogFilenames: this._commandNameForLogFilenames + }) + ); + } + + /** + * A helper method to determine the task name of a ProjectBuilder. Used when the task + * name is required before a task is created. + */ + public static getTaskNameForProject(rushProject: RushConfigurationProject): string { + return rushProject.packageName; + } +} diff --git a/apps/rush-lib/src/logic/TaskSelector.ts b/apps/rush-lib/src/logic/TaskSelector.ts deleted file mode 100644 index 7f047ea2b7b..00000000000 --- a/apps/rush-lib/src/logic/TaskSelector.ts +++ /dev/null @@ -1,161 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -import { BuildCacheConfiguration } from '../api/BuildCacheConfiguration'; -import { RushConfiguration } from '../api/RushConfiguration'; -import { RushConfigurationProject } from '../api/RushConfigurationProject'; -import { ProjectTaskRunner, convertSlashesForWindows } from './taskExecution/ProjectTaskRunner'; -import { ProjectChangeAnalyzer } from './ProjectChangeAnalyzer'; -import { TaskCollection } from './taskExecution/TaskCollection'; - -export interface ITaskSelectorOptions { - rushConfiguration: RushConfiguration; - buildCacheConfiguration: BuildCacheConfiguration | undefined; - selection: ReadonlySet; - commandName: string; - commandToRun: string; - customParameterValues: string[]; - isQuietMode: boolean; - isDebugMode: boolean; - isIncrementalBuildAllowed: boolean; - ignoreMissingScript: boolean; - ignoreDependencyOrder: boolean; - packageDepsFilename: string; - projectChangeAnalyzer?: ProjectChangeAnalyzer; - allowWarningsInSuccessfulBuild?: boolean; -} - -/** - * This class is responsible for: - * - based on to/from flags, solving the dependency graph and figuring out which projects need to be run - * - creating a ProjectBuilder for each project that needs to be built - * - registering the necessary ProjectBuilders with the TaskExecutionManager, which actually orchestrates execution - */ -export class TaskSelector { - private _options: ITaskSelectorOptions; - private _projectChangeAnalyzer: ProjectChangeAnalyzer; - - public constructor(options: ITaskSelectorOptions) { - this._options = options; - - const { projectChangeAnalyzer = new ProjectChangeAnalyzer(options.rushConfiguration) } = options; - - this._projectChangeAnalyzer = projectChangeAnalyzer; - } - - public static getScriptToRun( - rushProject: RushConfigurationProject, - commandToRun: string, - customParameterValues: string[] - ): string | undefined { - const script: string | undefined = TaskSelector._getScriptCommand(rushProject, commandToRun); - - if (script === undefined) { - return undefined; - } - - if (!script) { - return ''; - } else { - const taskCommand: string = `${script} ${customParameterValues.join(' ')}`; - return process.platform === 'win32' ? convertSlashesForWindows(taskCommand) : taskCommand; - } - } - - public registerTasks(): TaskCollection { - const projects: ReadonlySet = this._options.selection; - const taskCollection: TaskCollection = new TaskCollection(); - - // Register all tasks - for (const rushProject of projects) { - this._registerTask(rushProject, taskCollection); - } - - if (!this._options.ignoreDependencyOrder) { - const dependencyMap: Map> = new Map(); - - // Generate the filtered dependency graph for selected projects - function getDependencyTaskNames(project: RushConfigurationProject): Set { - const cached: Set | undefined = dependencyMap.get(project); - if (cached) { - return cached; - } - - const dependencyTaskNames: Set = new Set(); - dependencyMap.set(project, dependencyTaskNames); - - for (const dep of project.dependencyProjects) { - if (projects.has(dep)) { - // Add direct relationships for projects in the set - dependencyTaskNames.add(ProjectTaskRunner.getTaskName(dep)); - } else { - // Add indirect relationships for projects not in the set - for (const indirectDep of getDependencyTaskNames(dep)) { - dependencyTaskNames.add(indirectDep); - } - } - } - - return dependencyTaskNames; - } - - // Add ordering relationships for each dependency - for (const project of projects) { - taskCollection.addDependencies( - ProjectTaskRunner.getTaskName(project), - getDependencyTaskNames(project) - ); - } - } - - return taskCollection; - } - - private _registerTask(project: RushConfigurationProject | undefined, taskCollection: TaskCollection): void { - if (!project || taskCollection.hasTask(ProjectTaskRunner.getTaskName(project))) { - return; - } - - const commandToRun: string | undefined = TaskSelector.getScriptToRun( - project, - this._options.commandToRun, - this._options.customParameterValues - ); - if (commandToRun === undefined && !this._options.ignoreMissingScript) { - throw new Error( - `The project [${project.packageName}] does not define a '${this._options.commandToRun}' command in the 'scripts' section of its package.json` - ); - } - - taskCollection.addTask( - new ProjectTaskRunner({ - rushProject: project, - rushConfiguration: this._options.rushConfiguration, - buildCacheConfiguration: this._options.buildCacheConfiguration, - commandToRun: commandToRun || '', - commandName: this._options.commandName, - isIncrementalBuildAllowed: this._options.isIncrementalBuildAllowed, - projectChangeAnalyzer: this._projectChangeAnalyzer, - packageDepsFilename: this._options.packageDepsFilename, - allowWarningsInSuccessfulBuild: this._options.allowWarningsInSuccessfulBuild - }) - ); - } - - private static _getScriptCommand( - rushProject: RushConfigurationProject, - script: string - ): string | undefined { - if (!rushProject.packageJson.scripts) { - return undefined; - } - - const rawCommand: string = rushProject.packageJson.scripts[script]; - - if (rawCommand === undefined || rawCommand === null) { - return undefined; - } - - return rawCommand; - } -} diff --git a/apps/rush-lib/src/logic/TaskSelectorBase.ts b/apps/rush-lib/src/logic/TaskSelectorBase.ts new file mode 100644 index 00000000000..707533c8dc9 --- /dev/null +++ b/apps/rush-lib/src/logic/TaskSelectorBase.ts @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { BuildCacheConfiguration } from '../api/BuildCacheConfiguration'; +import { RushConfiguration } from '../api/RushConfiguration'; +import { RushConfigurationProject } from '../api/RushConfigurationProject'; +import { convertSlashesForWindows } from './taskExecution/ProjectTaskRunner'; +import { ProjectChangeAnalyzer } from './ProjectChangeAnalyzer'; +import { TaskCollection } from './taskExecution/TaskCollection'; + +export interface ITaskSelectorOptions { + rushConfiguration: RushConfiguration; + buildCacheConfiguration: BuildCacheConfiguration | undefined; + selection: ReadonlySet; + commandName: string; + commandToRun: string; + customParameterValues: string[]; + isQuietMode: boolean; + isDebugMode: boolean; + isIncrementalBuildAllowed: boolean; + ignoreMissingScript: boolean; + ignoreDependencyOrder: boolean; + packageDepsFilename: string; + projectChangeAnalyzer?: ProjectChangeAnalyzer; + allowWarningsInSuccessfulBuild?: boolean; +} + +/** + * This class is responsible for: + * - based on to/from flags, solving the dependency graph and figuring out which projects need to be run + * - creating a ProjectBuilder for each project that needs to be built + * - registering the necessary ProjectBuilders with the TaskExecutionManager, which actually orchestrates execution + */ +export abstract class TaskSelectorBase { + protected _options: ITaskSelectorOptions; + protected _projectChangeAnalyzer: ProjectChangeAnalyzer; + + public constructor(options: ITaskSelectorOptions) { + this._options = options; + + const { projectChangeAnalyzer = new ProjectChangeAnalyzer(options.rushConfiguration) } = options; + + this._projectChangeAnalyzer = projectChangeAnalyzer; + } + + public static getScriptToRun( + rushProject: RushConfigurationProject, + commandToRun: string, + customParameterValues: string[] + ): string | undefined { + const script: string | undefined = TaskSelectorBase._getScriptCommand(rushProject, commandToRun); + + if (script === undefined) { + return undefined; + } + + if (!script) { + return ''; + } else { + const taskCommand: string = `${script} ${customParameterValues.join(' ')}`; + return process.platform === 'win32' ? convertSlashesForWindows(taskCommand) : taskCommand; + } + } + + public abstract registerTasks(): TaskCollection; + + private static _getScriptCommand( + rushProject: RushConfigurationProject, + script: string + ): string | undefined { + if (!rushProject.packageJson.scripts) { + return undefined; + } + + const rawCommand: string = rushProject.packageJson.scripts[script]; + + if (rawCommand === undefined || rawCommand === null) { + return undefined; + } + + return rawCommand; + } +} diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts index 0716f145e33..87766564961 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts @@ -36,6 +36,10 @@ export class ProjectLogWritable extends TerminalWritable { this._logWriter = FileWriter.open(this._logPath); } + public static normalizeCommandNameForLogFilenames(commandName: string): string { + return commandName.replace(/[^a-zA-Z0-9]/g, '_'); + } + protected onWriteChunk(chunk: ITerminalChunk): void { if (!this._logWriter) { throw new InternalError('Output file was closed'); diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts index 4413b8dbb4a..283ff29aefa 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts @@ -54,6 +54,8 @@ export interface IProjectTaskRunnerOptions { projectChangeAnalyzer: ProjectChangeAnalyzer; packageDepsFilename: string; allowWarningsInSuccessfulBuild?: boolean; + taskName: string; + commandNameForLogFilenames: string; } function _areShallowEqual(object1: JsonObject, object2: JsonObject): boolean { @@ -75,9 +77,7 @@ function _areShallowEqual(object1: JsonObject, object2: JsonObject): boolean { * incremental state. */ export class ProjectTaskRunner extends BaseTaskRunner { - public get name(): string { - return ProjectTaskRunner.getTaskName(this._rushProject); - } + public readonly name: string; /** * This property is mutated by TaskExecutionManager, so is not readonly @@ -104,6 +104,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { public constructor(options: IProjectTaskRunnerOptions) { super(); + this.name = options.taskName; this._rushProject = options.rushProject; this._rushConfiguration = options.rushConfiguration; this._buildCacheConfiguration = options.buildCacheConfiguration; @@ -114,15 +115,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { this._projectChangeAnalyzer = options.projectChangeAnalyzer; this._packageDepsFilename = options.packageDepsFilename; this._allowWarningsInSuccessfulBuild = options.allowWarningsInSuccessfulBuild || false; - this._commandNameForLogFilenames = options.commandName; - } - - /** - * A helper method to determine the task name of a ProjectBuilder. Used when the task - * name is required before a task is created. - */ - public static getTaskName(rushProject: RushConfigurationProject): string { - return rushProject.packageName; + this._commandNameForLogFilenames = options.commandNameForLogFilenames; } public async executeAsync(context: ITaskRunnerContext): Promise { From 27b498e0bab042fe75fc3913de8df2863d1c6545 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Mon, 20 Dec 2021 18:22:12 -0800 Subject: [PATCH 3/5] Refactor the way log filenames are generated. --- .../rush-lib/src/cli/RushCommandLineParser.ts | 67 ++++--------------- .../src/cli/actions/WriteBuildCacheAction.ts | 2 +- .../src/cli/scriptActions/BulkScriptAction.ts | 16 ++--- .../src/logic/NonPhasedProjectTaskSelector.ts | 8 +-- .../logic/taskExecution/ProjectLogWritable.ts | 8 +-- .../logic/taskExecution/ProjectTaskRunner.ts | 8 +-- .../src/utilities/LogFilenameManager.ts | 51 ++++++++++++++ 7 files changed, 82 insertions(+), 78 deletions(-) create mode 100644 apps/rush-lib/src/utilities/LogFilenameManager.ts diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts index c47bd222af4..9e531a9e047 100644 --- a/apps/rush-lib/src/cli/RushCommandLineParser.ts +++ b/apps/rush-lib/src/cli/RushCommandLineParser.ts @@ -50,7 +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'; +import { LogFilenameManager } from '../utilities/LogFilenameManager'; /** * Options for `RushCommandLineParser`. @@ -68,11 +68,10 @@ export class RushCommandLineParser extends CommandLineParser { public readonly pluginManager: PluginManager; private _debugParameter!: CommandLineFlagParameter; - private _rushOptions: IRushCommandLineParserOptions; - private _terminalProvider: ConsoleTerminalProvider; - private _terminal: Terminal; - private readonly _commandNameForLogFilenamesToCommandNameMap: Map> = new Map(); - private readonly _commandNamesWithLogFilenameCollisions: Set = new Set(); + private readonly _rushOptions: IRushCommandLineParserOptions; + private readonly _terminalProvider: ConsoleTerminalProvider; + private readonly _terminal: Terminal; + private readonly _logFilenameManager: LogFilenameManager; public constructor(options?: Partial) { super({ @@ -91,8 +90,8 @@ export class RushCommandLineParser extends CommandLineParser { this._terminalProvider = new ConsoleTerminalProvider(); this._terminal = new Terminal(this._terminalProvider); - this._rushOptions = this._normalizeOptions(options || {}); + this._logFilenameManager = new LogFilenameManager(); try { const rushJsonFilename: string | undefined = RushConfiguration.tryFindRushJsonLocation({ @@ -254,23 +253,7 @@ export class RushCommandLineParser extends CommandLineParser { this._addCommandLineConfigActions(commandLineConfiguration); this._addDefaultBuildActions(commandLineConfiguration); - if (this._commandNamesWithLogFilenameCollisions.size > 0) { - const errorMessageParts: string[] = []; - for (const commandNameWithLogFilenameCollisions of this._commandNamesWithLogFilenameCollisions) { - const collidingCommands: string[] = Array.from( - this._commandNameForLogFilenamesToCommandNameMap.get(commandNameWithLogFilenameCollisions)! - ); - errorMessageParts.push( - `- [${collidingCommands.join(', ')}] will all write to ` + - `".${commandNameWithLogFilenameCollisions}.log" files` - ); - } - - throw new Error( - `The following command names will produce log files with names that will ` + - `collide:\n${errorMessageParts.join('\n')}` - ); - } + this._logFilenameManager.throwErrorIfLogFilenameCollisionsExist(); } private _addDefaultBuildActions(commandLineConfiguration?: CommandLineConfiguration): void { @@ -282,6 +265,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, @@ -304,7 +289,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( @@ -329,15 +314,12 @@ export class RushCommandLineParser extends CommandLineParser { switch (command.commandKind) { case RushConstants.bulkCommandKind: { - const commandNameForLogFilenames: string = this._getCommandNameForLogFilenames(command, commandToRun); + const logFilename: string = this._logFilenameManager.getLogFilename(commandToRun); this.addAction( new BulkScriptAction({ actionName: command.name, - commandNameForLogFilenames, - - // By default, the "rebuild" action runs the "build" script. However, if the command-line.json file - // overrides "rebuild," the "rebuild" script should be run. + logFilename: logFilename, commandToRun: commandToRun, summary: command.summary, @@ -412,31 +394,6 @@ export class RushCommandLineParser extends CommandLineParser { } } - private _getCommandNameForLogFilenames(command: CommandJson, commandToRun: string = command.name): string { - const commandNameForLogFilenames: string = ProjectLogWritable.normalizeCommandNameForLogFilenames( - // "rebuild" and "build" should write to the same log file ("build.log") - commandToRun - ); - - // Ensure two log files won't collide - let commandNamesForCommandNameForLogFilenames: Set | undefined = - this._commandNameForLogFilenamesToCommandNameMap.get(commandNameForLogFilenames); - if (!commandNamesForCommandNameForLogFilenames) { - commandNamesForCommandNameForLogFilenames = new Set(); - this._commandNameForLogFilenamesToCommandNameMap.set( - commandNameForLogFilenames, - commandNamesForCommandNameForLogFilenames - ); - } - - commandNamesForCommandNameForLogFilenames.add(commandToRun); - if (commandNamesForCommandNameForLogFilenames.size > 1) { - this._commandNamesWithLogFilenameCollisions.add(commandNameForLogFilenames); - } - - return commandNameForLogFilenames; - } - private _reportErrorAndSetExitCode(error: Error): void { if (!(error instanceof AlreadyReportedError)) { const prefix: string = 'ERROR: '; diff --git a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts index 445b1410b01..49a780b87ea 100644 --- a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts +++ b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts @@ -94,7 +94,7 @@ export class WriteBuildCacheAction extends BaseRushAction { isIncrementalBuildAllowed: false, projectChangeAnalyzer, packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(command), - commandNameForLogFilenames: ProjectLogWritable.normalizeCommandNameForLogFilenames(command) + logFilename: ProjectLogWritable.normalizeNameForLogFilenames(command) }); const trackedFiles: string[] = Array.from( diff --git a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts index 979462b846a..ae096bfd238 100644 --- a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts @@ -40,12 +40,8 @@ export interface IBulkScriptActionOptions extends IBaseScriptActionOptions { allowWarningsInSuccessfulBuild: boolean; watchForChanges: boolean; disableBuildCache: boolean; - commandNameForLogFilenames: string; - - /** - * Optional command to run. Otherwise, use the `actionName` as the command to run. - */ - commandToRun?: string; + logFilename: string; + commandToRun: string; } interface IExecuteInternalOptions { @@ -75,7 +71,7 @@ export class BulkScriptAction extends BaseScriptAction { private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined; private readonly _ignoreDependencyOrder: boolean; private readonly _allowWarningsInSuccessfulBuild: boolean; - private readonly _commandNameForLogFilenames: string; + private readonly _logFilename: string; private _changedProjectsOnly!: CommandLineFlagParameter; private _selectionParameters!: SelectionParameterSet; @@ -88,13 +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._commandNameForLogFilenames = options.commandNameForLogFilenames; + this._logFilename = options.logFilename; } public async runAsync(): Promise { @@ -162,7 +158,7 @@ export class BulkScriptAction extends BaseScriptAction { ignoreDependencyOrder: this._ignoreDependencyOrder, allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild, packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(this._commandToRun), - commandNameForLogFilenames: this._commandNameForLogFilenames + logFilename: this._logFilename }; const taskExecutionManagerOptions: ITaskExecutionManagerOptions = { diff --git a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts index 45d53f18219..c4d57d235c6 100644 --- a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts +++ b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts @@ -7,15 +7,15 @@ import { TaskCollection } from './taskExecution/TaskCollection'; import { ProjectTaskRunner } from './taskExecution/ProjectTaskRunner'; export interface INonPhasedProjectTaskSelectorOptions extends ITaskSelectorOptions { - commandNameForLogFilenames: string; + logFilename: string; } export class NonPhasedProjectTaskSelector extends TaskSelectorBase { - private readonly _commandNameForLogFilenames: string; + private readonly _logFilename: string; public constructor(options: INonPhasedProjectTaskSelectorOptions) { super(options); - this._commandNameForLogFilenames = options.commandNameForLogFilenames; + this._logFilename = options.logFilename; } public registerTasks(): TaskCollection { @@ -96,7 +96,7 @@ export class NonPhasedProjectTaskSelector extends TaskSelectorBase { projectChangeAnalyzer: this._projectChangeAnalyzer, packageDepsFilename: this._options.packageDepsFilename, allowWarningsInSuccessfulBuild: this._options.allowWarningsInSuccessfulBuild, - commandNameForLogFilenames: this._commandNameForLogFilenames + logFilename: this._logFilename }) ); } diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts index 87766564961..cc471f27c78 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts @@ -18,7 +18,7 @@ export class ProjectLogWritable extends TerminalWritable { private _logWriter: FileWriter | undefined = undefined; private _errorLogWriter: FileWriter | undefined = undefined; - public constructor(project: RushConfigurationProject, terminal: CollatedTerminal, commandName: string) { + public constructor(project: RushConfigurationProject, terminal: CollatedTerminal, logFilename: string) { super(); this._project = project; this._terminal = terminal; @@ -27,8 +27,8 @@ export class ProjectLogWritable extends TerminalWritable { this._project.packageName ); - this._logPath = `${this._project.projectFolder}/${unscopedProjectName}.${commandName}.log`; - this._errorLogPath = `${this._project.projectFolder}/${unscopedProjectName}.${commandName}.error.log`; + this._logPath = `${this._project.projectFolder}/${unscopedProjectName}.${logFilename}.log`; + this._errorLogPath = `${this._project.projectFolder}/${unscopedProjectName}.${logFilename}.error.log`; FileSystem.deleteFile(this._logPath); FileSystem.deleteFile(this._errorLogPath); @@ -36,7 +36,7 @@ export class ProjectLogWritable extends TerminalWritable { this._logWriter = FileWriter.open(this._logPath); } - public static normalizeCommandNameForLogFilenames(commandName: string): string { + public static normalizeNameForLogFilenames(commandName: string): string { return commandName.replace(/[^a-zA-Z0-9]/g, '_'); } diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts index 283ff29aefa..2384d9e7a21 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts @@ -55,7 +55,7 @@ export interface IProjectTaskRunnerOptions { packageDepsFilename: string; allowWarningsInSuccessfulBuild?: boolean; taskName: string; - commandNameForLogFilenames: string; + logFilename: string; } function _areShallowEqual(object1: JsonObject, object2: JsonObject): boolean { @@ -94,7 +94,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { private readonly _projectChangeAnalyzer: ProjectChangeAnalyzer; private readonly _packageDepsFilename: string; private readonly _allowWarningsInSuccessfulBuild: boolean; - private readonly _commandNameForLogFilenames: string; + private readonly _logFilename: string; /** * UNINITIALIZED === we haven't tried to initialize yet @@ -115,7 +115,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { this._projectChangeAnalyzer = options.projectChangeAnalyzer; this._packageDepsFilename = options.packageDepsFilename; this._allowWarningsInSuccessfulBuild = options.allowWarningsInSuccessfulBuild || false; - this._commandNameForLogFilenames = options.commandNameForLogFilenames; + this._logFilename = options.logFilename; } public async executeAsync(context: ITaskRunnerContext): Promise { @@ -153,7 +153,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { const projectLogWritable: ProjectLogWritable = new ProjectLogWritable( this._rushProject, context.collatedWriter.terminal, - this._commandNameForLogFilenames + this._logFilename ); try { diff --git a/apps/rush-lib/src/utilities/LogFilenameManager.ts b/apps/rush-lib/src/utilities/LogFilenameManager.ts new file mode 100644 index 00000000000..8a6959684ee --- /dev/null +++ b/apps/rush-lib/src/utilities/LogFilenameManager.ts @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { ProjectLogWritable } from '../logic/taskExecution/ProjectLogWritable'; + +export class LogFilenameManager { + private readonly _logFilenamesToCommandNameMap: Map> = new Map(); + private readonly _commandNamesWithLogFilenameCollisions: Set = new Set(); + + public getLogFilename(commandToRun: string): string { + const logFilename: string = ProjectLogWritable.normalizeNameForLogFilenames( + // "rebuild" and "build" should write to the same log file ("build.log") + commandToRun + ); + + // Ensure two log files won't collide + let commandsToRunForLogFilename: Set | undefined = + this._logFilenamesToCommandNameMap.get(logFilename); + if (!commandsToRunForLogFilename) { + commandsToRunForLogFilename = new Set(); + this._logFilenamesToCommandNameMap.set(logFilename, commandsToRunForLogFilename); + } + + commandsToRunForLogFilename.add(commandToRun); + if (commandsToRunForLogFilename.size > 1) { + this._commandNamesWithLogFilenameCollisions.add(logFilename); + } + + return logFilename; + } + + public throwErrorIfLogFilenameCollisionsExist(): void { + if (this._commandNamesWithLogFilenameCollisions.size > 0) { + const errorMessageParts: string[] = []; + for (const commandNameWithLogFilenameCollisions of this._commandNamesWithLogFilenameCollisions) { + const collidingCommands: string[] = Array.from( + this._logFilenamesToCommandNameMap.get(commandNameWithLogFilenameCollisions)! + ); + errorMessageParts.push( + `- [${collidingCommands.join(', ')}] will all write to ` + + `".${commandNameWithLogFilenameCollisions}.log" files` + ); + } + + throw new Error( + `The following command names will produce log files with names that will ` + + `collide:\n${errorMessageParts.join('\n')}` + ); + } + } +} From 7bb7e42567a30da765065bb6e3842ec31def81a5 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 21 Dec 2021 00:09:23 -0800 Subject: [PATCH 4/5] Tweak the log locations. --- .../rush-lib/src/cli/RushCommandLineParser.ts | 11 ++-- .../src/cli/actions/WriteBuildCacheAction.ts | 2 +- .../src/cli/scriptActions/BulkScriptAction.ts | 8 +-- .../cli/test/RushCommandLineParser.test.ts | 13 ----- .../a/package.json | 9 ---- .../b/package.json | 12 ----- .../common/config/rush/command-line.json | 16 ------ .../rush.json | 17 ------- .../src/logic/NonPhasedProjectTaskSelector.ts | 8 +-- apps/rush-lib/src/logic/RushConstants.ts | 5 ++ .../logic/taskExecution/ProjectLogWritable.ts | 46 ++++++++++++++--- .../logic/taskExecution/ProjectTaskRunner.ts | 8 +-- .../src/utilities/LogFilenameManager.ts | 51 ------------------- common/reviews/api/rush-lib.api.md | 1 + 14 files changed, 62 insertions(+), 145 deletions(-) delete mode 100644 apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/a/package.json delete mode 100644 apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/b/package.json delete mode 100644 apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/common/config/rush/command-line.json delete mode 100644 apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/rush.json delete mode 100644 apps/rush-lib/src/utilities/LogFilenameManager.ts diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts index 9e531a9e047..ead36b52fd3 100644 --- a/apps/rush-lib/src/cli/RushCommandLineParser.ts +++ b/apps/rush-lib/src/cli/RushCommandLineParser.ts @@ -50,7 +50,7 @@ import { SetupAction } from './actions/SetupAction'; import { EnvironmentConfiguration } from '../api/EnvironmentConfiguration'; import { ICustomCommandLineConfigurationInfo, PluginManager } from '../pluginFramework/PluginManager'; import { RushSession } from '../pluginFramework/RushSession'; -import { LogFilenameManager } from '../utilities/LogFilenameManager'; +import { ProjectLogWritable } from '../logic/taskExecution/ProjectLogWritable'; /** * Options for `RushCommandLineParser`. @@ -71,7 +71,6 @@ export class RushCommandLineParser extends CommandLineParser { private readonly _rushOptions: IRushCommandLineParserOptions; private readonly _terminalProvider: ConsoleTerminalProvider; private readonly _terminal: Terminal; - private readonly _logFilenameManager: LogFilenameManager; public constructor(options?: Partial) { super({ @@ -91,7 +90,6 @@ export class RushCommandLineParser extends CommandLineParser { this._terminalProvider = new ConsoleTerminalProvider(); this._terminal = new Terminal(this._terminalProvider); this._rushOptions = this._normalizeOptions(options || {}); - this._logFilenameManager = new LogFilenameManager(); try { const rushJsonFilename: string | undefined = RushConfiguration.tryFindRushJsonLocation({ @@ -252,8 +250,6 @@ export class RushCommandLineParser extends CommandLineParser { // Build actions from the command line configuration supersede default build actions. this._addCommandLineConfigActions(commandLineConfiguration); this._addDefaultBuildActions(commandLineConfiguration); - - this._logFilenameManager.throwErrorIfLogFilenameCollisionsExist(); } private _addDefaultBuildActions(commandLineConfiguration?: CommandLineConfiguration): void { @@ -314,12 +310,13 @@ export class RushCommandLineParser extends CommandLineParser { switch (command.commandKind) { case RushConstants.bulkCommandKind: { - const logFilename: string = this._logFilenameManager.getLogFilename(commandToRun); + const logFilenameIdentifier: string = + ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(commandToRun); this.addAction( new BulkScriptAction({ actionName: command.name, - logFilename: logFilename, + logFilenameIdentifier: logFilenameIdentifier, commandToRun: commandToRun, summary: command.summary, diff --git a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts index 49a780b87ea..b220e956fce 100644 --- a/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts +++ b/apps/rush-lib/src/cli/actions/WriteBuildCacheAction.ts @@ -94,7 +94,7 @@ export class WriteBuildCacheAction extends BaseRushAction { isIncrementalBuildAllowed: false, projectChangeAnalyzer, packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(command), - logFilename: ProjectLogWritable.normalizeNameForLogFilenames(command) + logFilenameIdentifier: ProjectLogWritable.normalizeNameForLogFilenameIdentifiers(command) }); const trackedFiles: string[] = Array.from( diff --git a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts index ae096bfd238..463f244d3e6 100644 --- a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts +++ b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts @@ -40,7 +40,7 @@ export interface IBulkScriptActionOptions extends IBaseScriptActionOptions { allowWarningsInSuccessfulBuild: boolean; watchForChanges: boolean; disableBuildCache: boolean; - logFilename: string; + logFilenameIdentifier: string; commandToRun: string; } @@ -71,7 +71,7 @@ export class BulkScriptAction extends BaseScriptAction { private readonly _repoCommandLineConfiguration: CommandLineConfiguration | undefined; private readonly _ignoreDependencyOrder: boolean; private readonly _allowWarningsInSuccessfulBuild: boolean; - private readonly _logFilename: string; + private readonly _logFilenameIdentifier: string; private _changedProjectsOnly!: CommandLineFlagParameter; private _selectionParameters!: SelectionParameterSet; @@ -90,7 +90,7 @@ export class BulkScriptAction extends BaseScriptAction { this._watchForChanges = options.watchForChanges; this._disableBuildCache = options.disableBuildCache; this._repoCommandLineConfiguration = options.commandLineConfiguration; - this._logFilename = options.logFilename; + this._logFilenameIdentifier = options.logFilenameIdentifier; } public async runAsync(): Promise { @@ -158,7 +158,7 @@ export class BulkScriptAction extends BaseScriptAction { ignoreDependencyOrder: this._ignoreDependencyOrder, allowWarningsInSuccessfulBuild: this._allowWarningsInSuccessfulBuild, packageDepsFilename: Utilities.getPackageDepsFilenameForCommand(this._commandToRun), - logFilename: this._logFilename + logFilenameIdentifier: this._logFilenameIdentifier }; const taskExecutionManagerOptions: ITaskExecutionManagerOptions = { diff --git a/apps/rush-lib/src/cli/test/RushCommandLineParser.test.ts b/apps/rush-lib/src/cli/test/RushCommandLineParser.test.ts index 59e942d4a0e..7abeaa9b6db 100644 --- a/apps/rush-lib/src/cli/test/RushCommandLineParser.test.ts +++ b/apps/rush-lib/src/cli/test/RushCommandLineParser.test.ts @@ -332,18 +332,5 @@ describe('RushCommandLineParser', () => { ); }); }); - - describe(`in repo with two commands that will produce colliding log filenames`, () => { - it('throws an error when starting Rush', async () => { - const repoName: string = 'commandsWithCollidingLogNamesRepo'; - - await expect(() => { - getCommandLineParserInstance(repoName, 'doesnt-matter'); - }).toThrowErrorMatchingInlineSnapshot(` - "The following command names will produce log files with names that will collide: - - [command:a, command-a] will all write to \\".command_a.log\\" files" - `); - }); - }); }); }); diff --git a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/a/package.json b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/a/package.json deleted file mode 100644 index 84b3f0dd05c..00000000000 --- a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/a/package.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "name": "a", - "version": "1.0.0", - "description": "Test package a", - "scripts": { - "command:a": "fake_build_task_but_works_with_mock", - "command-a": "fake_build_task_but_works_with_mock" - } -} diff --git a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/b/package.json b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/b/package.json deleted file mode 100644 index 859d606b22f..00000000000 --- a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/b/package.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "name": "b", - "version": "1.0.0", - "description": "Test package b", - "dependencies": { - "a": "1.0.0" - }, - "scripts": { - "command:a": "fake_build_task_but_works_with_mock", - "command-a": "fake_build_task_but_works_with_mock" - } -} diff --git a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/common/config/rush/command-line.json b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/common/config/rush/command-line.json deleted file mode 100644 index 43f9ad1bb71..00000000000 --- a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/common/config/rush/command-line.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "commands": [ - { - "name": "command:a", - "commandKind": "bulk", - "summary": "Test command", - "enableParallelism": true - }, - { - "name": "command-a", - "commandKind": "bulk", - "summary": "Test command", - "enableParallelism": true - } - ] -} diff --git a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/rush.json b/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/rush.json deleted file mode 100644 index f39da1606c4..00000000000 --- a/apps/rush-lib/src/cli/test/commandsWithCollidingLogNamesRepo/rush.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "npmVersion": "6.4.1", - "rushVersion": "5.5.2", - "projectFolderMinDepth": 1, - "projectFolderMaxDepth": 99, - - "projects": [ - { - "packageName": "a", - "projectFolder": "a" - }, - { - "packageName": "b", - "projectFolder": "b" - } - ] -} diff --git a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts index c4d57d235c6..743e059109c 100644 --- a/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts +++ b/apps/rush-lib/src/logic/NonPhasedProjectTaskSelector.ts @@ -7,15 +7,15 @@ import { TaskCollection } from './taskExecution/TaskCollection'; import { ProjectTaskRunner } from './taskExecution/ProjectTaskRunner'; export interface INonPhasedProjectTaskSelectorOptions extends ITaskSelectorOptions { - logFilename: string; + logFilenameIdentifier: string; } export class NonPhasedProjectTaskSelector extends TaskSelectorBase { - private readonly _logFilename: string; + private readonly _logFilenameIdentifier: string; public constructor(options: INonPhasedProjectTaskSelectorOptions) { super(options); - this._logFilename = options.logFilename; + this._logFilenameIdentifier = options.logFilenameIdentifier; } public registerTasks(): TaskCollection { @@ -96,7 +96,7 @@ export class NonPhasedProjectTaskSelector extends TaskSelectorBase { projectChangeAnalyzer: this._projectChangeAnalyzer, packageDepsFilename: this._options.packageDepsFilename, allowWarningsInSuccessfulBuild: this._options.allowWarningsInSuccessfulBuild, - logFilename: this._logFilename + logFilenameIdentifier: this._logFilenameIdentifier }) ); } diff --git a/apps/rush-lib/src/logic/RushConstants.ts b/apps/rush-lib/src/logic/RushConstants.ts index 5c2156dd1b4..8d7c8269459 100644 --- a/apps/rush-lib/src/logic/RushConstants.ts +++ b/apps/rush-lib/src/logic/RushConstants.ts @@ -229,4 +229,9 @@ export class RushConstants { * The name of the per-user Rush configuration data folder. */ public static readonly rushUserConfigurationFolderName: string = '.rush-user'; + + /** + * The name of the project `rush-logs` folder. + */ + public static readonly rushLogsFolderName: string = 'rush-logs'; } diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts index cc471f27c78..31cb4004a08 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectLogWritable.ts @@ -7,6 +7,7 @@ import { CollatedTerminal } from '@rushstack/stream-collator'; import { RushConfigurationProject } from '../../api/RushConfigurationProject'; import { PackageNameParsers } from '../../api/PackageNameParsers'; +import { RushConstants } from '../RushConstants'; export class ProjectLogWritable extends TerminalWritable { private readonly _project: RushConfigurationProject; @@ -18,17 +19,48 @@ export class ProjectLogWritable extends TerminalWritable { private _logWriter: FileWriter | undefined = undefined; private _errorLogWriter: FileWriter | undefined = undefined; - public constructor(project: RushConfigurationProject, terminal: CollatedTerminal, logFilename: string) { + public constructor( + project: RushConfigurationProject, + terminal: CollatedTerminal, + logFilenameIdentifier: string + ) { super(); this._project = project; this._terminal = terminal; - const unscopedProjectName: string = PackageNameParsers.permissive.getUnscopedName( - this._project.packageName + function getLogFilePaths( + basePath: string, + logFilenameIdentifier: string + ): { logPath: string; errorLogPath: string } { + const unscopedProjectName: string = PackageNameParsers.permissive.getUnscopedName(project.packageName); + + return { + logPath: `${basePath}/${unscopedProjectName}.${logFilenameIdentifier}.log`, + errorLogPath: `${basePath}/${unscopedProjectName}.${logFilenameIdentifier}.error.log` + }; + } + + const projectFolder: string = this._project.projectFolder; + const { logPath: legacyLogPath, errorLogPath: legacyErrorLogPath } = getLogFilePaths( + projectFolder, + 'build' ); + // If the multi-phase commands experiment is enabled, put logs under `rush-logs` + if (project.rushConfiguration.experimentsConfiguration.configuration._multiPhaseCommands) { + // Delete the legacy logs + FileSystem.deleteFile(legacyLogPath); + FileSystem.deleteFile(legacyErrorLogPath); - this._logPath = `${this._project.projectFolder}/${unscopedProjectName}.${logFilename}.log`; - this._errorLogPath = `${this._project.projectFolder}/${unscopedProjectName}.${logFilename}.error.log`; + const logPathPrefix: string = `${projectFolder}/${RushConstants.rushLogsFolderName}`; + FileSystem.ensureFolder(logPathPrefix); + + const { logPath, errorLogPath } = getLogFilePaths(logPathPrefix, logFilenameIdentifier); + this._logPath = logPath; + this._errorLogPath = errorLogPath; + } else { + this._logPath = legacyLogPath; + this._errorLogPath = legacyErrorLogPath; + } FileSystem.deleteFile(this._logPath); FileSystem.deleteFile(this._errorLogPath); @@ -36,8 +68,8 @@ export class ProjectLogWritable extends TerminalWritable { this._logWriter = FileWriter.open(this._logPath); } - public static normalizeNameForLogFilenames(commandName: string): string { - return commandName.replace(/[^a-zA-Z0-9]/g, '_'); + public static normalizeNameForLogFilenameIdentifiers(name: string): string { + return name.replace(/:/g, '_'); // Replace colons with underscores to be filesystem-safe } protected onWriteChunk(chunk: ITerminalChunk): void { diff --git a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts index 2384d9e7a21..32ffe88c847 100644 --- a/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts +++ b/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts @@ -55,7 +55,7 @@ export interface IProjectTaskRunnerOptions { packageDepsFilename: string; allowWarningsInSuccessfulBuild?: boolean; taskName: string; - logFilename: string; + logFilenameIdentifier: string; } function _areShallowEqual(object1: JsonObject, object2: JsonObject): boolean { @@ -94,7 +94,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { private readonly _projectChangeAnalyzer: ProjectChangeAnalyzer; private readonly _packageDepsFilename: string; private readonly _allowWarningsInSuccessfulBuild: boolean; - private readonly _logFilename: string; + private readonly _logFilenameIdentifier: string; /** * UNINITIALIZED === we haven't tried to initialize yet @@ -115,7 +115,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { this._projectChangeAnalyzer = options.projectChangeAnalyzer; this._packageDepsFilename = options.packageDepsFilename; this._allowWarningsInSuccessfulBuild = options.allowWarningsInSuccessfulBuild || false; - this._logFilename = options.logFilename; + this._logFilenameIdentifier = options.logFilenameIdentifier; } public async executeAsync(context: ITaskRunnerContext): Promise { @@ -153,7 +153,7 @@ export class ProjectTaskRunner extends BaseTaskRunner { const projectLogWritable: ProjectLogWritable = new ProjectLogWritable( this._rushProject, context.collatedWriter.terminal, - this._logFilename + this._logFilenameIdentifier ); try { diff --git a/apps/rush-lib/src/utilities/LogFilenameManager.ts b/apps/rush-lib/src/utilities/LogFilenameManager.ts deleted file mode 100644 index 8a6959684ee..00000000000 --- a/apps/rush-lib/src/utilities/LogFilenameManager.ts +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. -// See LICENSE in the project root for license information. - -import { ProjectLogWritable } from '../logic/taskExecution/ProjectLogWritable'; - -export class LogFilenameManager { - private readonly _logFilenamesToCommandNameMap: Map> = new Map(); - private readonly _commandNamesWithLogFilenameCollisions: Set = new Set(); - - public getLogFilename(commandToRun: string): string { - const logFilename: string = ProjectLogWritable.normalizeNameForLogFilenames( - // "rebuild" and "build" should write to the same log file ("build.log") - commandToRun - ); - - // Ensure two log files won't collide - let commandsToRunForLogFilename: Set | undefined = - this._logFilenamesToCommandNameMap.get(logFilename); - if (!commandsToRunForLogFilename) { - commandsToRunForLogFilename = new Set(); - this._logFilenamesToCommandNameMap.set(logFilename, commandsToRunForLogFilename); - } - - commandsToRunForLogFilename.add(commandToRun); - if (commandsToRunForLogFilename.size > 1) { - this._commandNamesWithLogFilenameCollisions.add(logFilename); - } - - return logFilename; - } - - public throwErrorIfLogFilenameCollisionsExist(): void { - if (this._commandNamesWithLogFilenameCollisions.size > 0) { - const errorMessageParts: string[] = []; - for (const commandNameWithLogFilenameCollisions of this._commandNamesWithLogFilenameCollisions) { - const collidingCommands: string[] = Array.from( - this._logFilenamesToCommandNameMap.get(commandNameWithLogFilenameCollisions)! - ); - errorMessageParts.push( - `- [${collidingCommands.join(', ')}] will all write to ` + - `".${commandNameWithLogFilenameCollisions}.log" files` - ); - } - - throw new Error( - `The following command names will produce log files with names that will ` + - `collide:\n${errorMessageParts.join('\n')}` - ); - } - } -} diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 9c62798a9c5..86fc8f7184b 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -636,6 +636,7 @@ export class RushConstants { static readonly projectShrinkwrapFilename: string; static readonly rebuildCommandName: string; static readonly repoStateFilename: string; + static readonly rushLogsFolderName: string; static readonly rushPackageName: string; static readonly rushPluginManifestFilename: string; static readonly rushPluginsConfigFilename: string; From 2780867a158eaa60adbfb9883aec44d5cb41898f Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 21 Dec 2021 00:10:41 -0800 Subject: [PATCH 5/5] Rush change. --- .../rush/pre-phased-refactors_2021-12-21-08-10.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@microsoft/rush/pre-phased-refactors_2021-12-21-08-10.json diff --git a/common/changes/@microsoft/rush/pre-phased-refactors_2021-12-21-08-10.json b/common/changes/@microsoft/rush/pre-phased-refactors_2021-12-21-08-10.json new file mode 100644 index 00000000000..bd7ff97cb34 --- /dev/null +++ b/common/changes/@microsoft/rush/pre-phased-refactors_2021-12-21-08-10.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file