code review feedback

Author: Ben Keen

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

@@ -946,6 +946,72 @@ export class Operation {
     weight: number;
 }
 
+// Warning: (ae-internal-missing-underscore) The name "OperationExecutionRecord" should be prefixed with an underscore because the declaration is marked as @internal
+//
+// @internal
+export class OperationExecutionRecord implements IOperationRunnerContext, IOperationExecutionResult {
+    // Warning: (ae-forgotten-export) The symbol "IOperationExecutionRecordContext" needs to be exported by the entry point index.d.ts
+    constructor(operation: Operation, context: IOperationExecutionRecordContext);
+    // (undocumented)
+    readonly associatedPhase: IPhase;
+    // (undocumented)
+    readonly associatedProject: RushConfigurationProject;
+    // (undocumented)
+    get cobuildRunnerId(): string | undefined;
+    // (undocumented)
+    get collatedWriter(): CollatedWriter;
+    readonly consumers: Set<OperationExecutionRecord>;
+    criticalPathLength: number | undefined;
+    // (undocumented)
+    get debugMode(): boolean;
+    readonly dependencies: Set<OperationExecutionRecord>;
+    // (undocumented)
+    get environment(): IEnvironment | undefined;
+    error: Error | undefined;
+    // (undocumented)
+    executeAsync({ onStart, onResult }: {
+        onStart: (record: OperationExecutionRecord) => Promise<OperationStatus | undefined>;
+        onResult: (record: OperationExecutionRecord) => Promise<void>;
+    }): Promise<void>;
+    // (undocumented)
+    getStateHash(): string;
+    // (undocumented)
+    getStateHashComponents(): ReadonlyArray<string>;
+    // (undocumented)
+    get isTerminal(): boolean;
+    // (undocumented)
+    logFilePaths: ILogFilePaths | undefined;
+    // (undocumented)
+    get metadataFolderPath(): string | undefined;
+    // (undocumented)
+    get name(): string;
+    // (undocumented)
+    get nonCachedDurationMs(): number | undefined;
+    readonly operation: Operation;
+    // (undocumented)
+    readonly _operationMetadataManager: _OperationMetadataManager;
+    // (undocumented)
+    get quietMode(): boolean;
+    // (undocumented)
+    readonly runner: IOperationRunner;
+    runWithTerminalAsync<T>(callback: (terminal: ITerminal, terminalProvider: ITerminalProvider) => Promise<T>, options: {
+        createLogFile: boolean;
+        logFileSuffix: string;
+    }): Promise<T>;
+    // (undocumented)
+    get silent(): boolean;
+    get status(): OperationStatus;
+    set status(newStatus: OperationStatus);
+    // (undocumented)
+    readonly stdioSummarizer: StdioSummarizer;
+    // Warning: (ae-forgotten-export) The symbol "Stopwatch" needs to be exported by the entry point index.d.ts
+    //
+    // (undocumented)
+    readonly stopwatch: Stopwatch;
+    // (undocumented)
+    get weight(): number;
+}
+
 // @internal
 export class _OperationMetadataManager {
     constructor(options: _IOperationMetadataManagerOptions);
@@ -1134,7 +1200,6 @@ export type PnpmStoreOptions = PnpmStoreLocation;
 export class ProjectBuildCache {
     // (undocumented)
     get cacheId(): string | undefined;
-    // Warning: (ae-forgotten-export) The symbol "OperationExecutionRecord" needs to be exported by the entry point index.d.ts
     // Warning: (ae-forgotten-export) The symbol "IOperationBuildCacheOptions" needs to be exported by the entry point index.d.ts
     //
     // (undocumented)

libraries/rush-lib/src/index.ts

@@ -135,6 +135,7 @@ export type {
   IExecutionResult,
   IOperationExecutionResult
 } from './logic/operations/IOperationExecutionResult';
+export type { OperationExecutionRecord } from './logic/operations/OperationExecutionRecord';
 export { type IOperationOptions, Operation } from './logic/operations/Operation';
 export { OperationStatus } from './logic/operations/OperationStatus';
 export type { ILogFilePaths } from './logic/operations/ProjectLogWritable';

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

@@ -128,6 +128,8 @@ function spliceShards(existingOperations: Set<Operation>, context: ICreateOperat
       ];
 
       const { scripts } = project.packageJson;
+
+      //
       const commandToRun: string | undefined = phase.shellCommand ?? scripts?.[phase.name];
 
       operation.logFilenameIdentifier = `${baseLogFilenameIdentifier}_collate`;

rush-plugins/rush-bridge-cache-plugin/README.md

@@ -1,44 +1,45 @@
 # @rushstack/rush-bridge-cache-plugin
 
-This plugin allows for interaction with the Rush cache. It exposes some methods to set the cache.
+This is a Rush plugin that adds an optional `--set-cache-only` flag to Rush's phased commands, so you can other tools to actual run the scripts and generate the build artifacts on disk, then separately populate the Rush cache for particular actions(s). For integrations with other build orchestrators, this allows the best of both worlds: using a different build tool to orchestrate the work, but still populate the Rush cache for benefiting local use of Rush.
 
+## Here be dragons!
 
-## Installation
-
-`npm install @rushstack/rush-bridge-cache-plugin`
+This is a power-user sort of plugin. It assumes that the work for a particular task has already been completed and the build artifacts have been generated on disk. **If you run this command on a package where the command hasn't already been ran and the build artifacts are missing or incorrect, you will cache invalid content**. Be careful and beware!
 
-<!--
-## Usage
-The package contains a binary you can run from the command line. It will auto-detect the location of your rush.json file.
 
-`populate-rush-cache --packageName=[package name] --phase=test:unit`
+## Installation
 
-- `--packageName` (required) - this should map to the `packageName` entry in your `rush.json` file.
-- `--phase` (required): the name of the phased command whose command has already been ran, and you want to cache the result on disk.
--->
+(TODO)
 
---------------------
+`npm install @rushstack/rush-bridge-cache-plugin`
 
-So this will:
-- tap into the hooks and do all the necessary shit to figure out how to po
-- expose a simple API for external users to tap into, e.g. if you want to populate the cache externally you'd import this plugin and use the methods to do so. You could then wrap that in a rush function, or however you want to do it.
 
+## Configuration
 
+First you need to update your `command-line.json` file to add the new flag. Configure it to target whatever specific commands you want to have this feature. Example:
 
+```json
+{
+  "associatedCommands": ["build", "test", "lint", "a11y", "typecheck"],
+  "description": "When the flag is added to any associated command, it'll bypass running the command itself, but cache the result of a previous run. Beware! Only run when you know the build artifacts are in a valid state for the command.",
+  "parameterKind": "flag",
+  "longName": "--set-cache-only",
+  "required": false
+}
+```
 
 
---------------------
+## Usage
 
-Discussion about the solution for BuildXL here:
-  https://teams.microsoft.com/l/message/19:d85f52548ec74e8f8a0f107bd4e5ceb6@thread.v2/1740610046597?context=%7B%22contextType%22%3A%22chat%22%7D
+Any of the rush command can now just be given a `--set-cache-only` property, e.g.
 
+`rush build --to your-packageX --set-cache-only`
 
-"populate-cache": "..." <--- called after any cacheable unit of work is complete
+That will examine `your-packageX` and all of its dependencies, then populate the cache.
 
-OperationExecutionRecord -> the smallest unit of work to be done.
-  - looks like it's strongly coupled to the runner. We need one-off method calls.
 
-CacheableOperationPlugin
-  _tryGetProjectBuildCache()  -> this returns the project build cache
+## Performance
 
+When running within a pipeline, you'll want to populate the cache as fast as possible. So instead of waiting until the full build graph has been processed, you'll wnt to run it after each successful task. For that, just use Rush's `--only` and target whatever task had just completed, for example:
 
+`rush lint --only your-packageY --set-cache-only`

rush-plugins/rush-bridge-cache-plugin/command-line.json

@@ -2,7 +2,7 @@
   "name": "@rushstack/rush-bridge-cache-plugin",
   "version": "0.0.1",
   "private": true,
-  "description": "A plugin to expose methods to interact with the Rush cache.",
+  "description": "Rush plugin that provides a --set-cache-only command flag to populates the cache from previous runs.",
   "license": "MIT",
   "main": "./lib/index.js",
   "repository": {

rush-plugins/rush-bridge-cache-plugin/package.json

@@ -3,9 +3,8 @@
   "plugins": [
     {
       "pluginName": "rush-bridge-cache-plugin",
-      "description": "Rush plugin to allow interactions with the Rush cache.",
-      "entryPoint": "./lib/index.js",
-      "commandLineJsonFilePath": "./command-line.json"
+      "description": "Rush plugin that provides a --set-cache-only command flag to populates the cache from previous runs.",
+      "entryPoint": "./lib/index.js"
     }
   ]
 }

rush-plugins/rush-bridge-cache-plugin/rush-plugin-manifest.json

@@ -1,14 +1,16 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
-import { BuildCacheConfiguration, ProjectBuildCache } from '@rushstack/rush-sdk';
+import { ProjectBuildCache } from '@rushstack/rush-sdk';
 import type {
+  BuildCacheConfiguration,
+  IExecuteOperationsContext,
   ILogger,
   IOperationExecutionResult,
   IPhasedCommand,
   IRushPlugin,
   Operation,
-  RushConfiguration,
+  OperationExecutionRecord,
   RushSession
 } from '@rushstack/rush-sdk';
 
@@ -17,90 +19,96 @@ const PLUGIN_NAME: 'RushBridgeCachePlugin' = 'RushBridgeCachePlugin';
 export class BridgeCachePlugin implements IRushPlugin {
   public readonly pluginName: string = PLUGIN_NAME;
 
-  public apply(session: RushSession, rushConfiguration: RushConfiguration): void {
-    // klutzy. Better way?
-    const actionName: string = process.argv[2];
-
-    // const isSetCacheOnly: boolean = process.argv.includes('--set-cache-only'); // has to be allowed for ANY phased command, or be customizable
-    // if (!isSetCacheOnly) {
-    //   return;
-    // }
-
-    // tracks the projects being targeted by the command (--to, --only etc.)
-    const targetProjects: string[] = [];
+  public apply(session: RushSession): void {
+    const isSetCacheOnly: boolean = process.argv.includes('--set-cache-only');
+    if (!isSetCacheOnly) {
+      return;
+    }
 
     const cancelOperations = (operations: Set<Operation>): Set<Operation> => {
       operations.forEach((operation: Operation) => {
-        if (operation.enabled) {
-          targetProjects.push(operation.associatedProject.packageName);
-        }
-
         operation.enabled = false;
       });
       return operations;
     };
 
-    session.hooks.runPhasedCommand
-      .for(actionName)
-      .tapPromise(PLUGIN_NAME, async (command: IPhasedCommand) => {
-        // cancel the actual operations. We don't want to actually run the command, just cache the output folders from a previous run
-        command.hooks.createOperations.tap(
-          { name: PLUGIN_NAME, stage: Number.MAX_SAFE_INTEGER },
-          cancelOperations
-        );
-
-        // now populate the cache for each operation
-        command.hooks.beforeExecuteOperations.tap(
-          PLUGIN_NAME,
-          async (recordByOperation: Map<Operation, IOperationExecutionResult>): Promise<void> => {
-            await this._setCacheAsync(session, rushConfiguration, recordByOperation, targetProjects);
+    session.hooks.runAnyPhasedCommand.tapPromise(PLUGIN_NAME, async (command: IPhasedCommand) => {
+      // tracks the projects being targeted by the command (--to, --only etc.)
+      const targetProjects: Set<Operation> = new Set<Operation>();
+
+      // cancel the actual operations. We don't want to actually run the command, just cache the output folders from a previous run
+      command.hooks.createOperations.tap(
+        { name: PLUGIN_NAME, stage: Number.MAX_SAFE_INTEGER },
+        (operations: Set<Operation>): Set<Operation> => {
+          operations.forEach((operation: Operation) => {
+            if (operation.enabled) {
+              targetProjects.add(operation);
+            }
+          });
+          return cancelOperations(operations);
+        }
+      );
+
+      // now populate the cache for each operation
+      command.hooks.beforeExecuteOperations.tap(
+        PLUGIN_NAME,
+        async (
+          recordByOperation: Map<Operation, IOperationExecutionResult>,
+          context: IExecuteOperationsContext
+        ): Promise<void> => {
+          if (!context.buildCacheConfiguration) {
+            return;
           }
-        );
-      });
+
+          await this._setCacheAsync(
+            session,
+            context.buildCacheConfiguration,
+            recordByOperation,
+            targetProjects
+          );
+        }
+      );
+    });
   }
 
   private async _setCacheAsync(
     session: RushSession,
-    rushConfiguration: RushConfiguration,
+    buildCacheConfiguration: BuildCacheConfiguration,
     recordByOperation: Map<Operation, IOperationExecutionResult>,
-    targetProjects: string[]
+    targetProjects: Set<Operation>
   ): Promise<void> {
     const logger: ILogger = session.getLogger(PLUGIN_NAME);
 
-    // const isSetCacheOnly: boolean = process.argv.includes('--set-cache-only');
-
     recordByOperation.forEach(
       async (operationExecutionResult: IOperationExecutionResult, operation: Operation) => {
-        const { associatedProject, associatedPhase, settings } = operation;
+        const { associatedProject, associatedPhase } = operation;
 
-        if (!targetProjects.includes(associatedProject.packageName)) {
+        // omit operations that aren't targeted, or packages without a command for this phase
+        const hasCommand: boolean = !!associatedProject.packageJson.scripts?.[associatedPhase.name];
+        if (!targetProjects.has(operation) || !hasCommand) {
           return;
         }
 
-        const buildCacheConfiguration: BuildCacheConfiguration | undefined =
-          await BuildCacheConfiguration.tryLoadAsync(logger.terminal, rushConfiguration, session);
-
-        if (!buildCacheConfiguration) {
-          return;
-        }
-
-        const projectBuildCache: ProjectBuildCache = ProjectBuildCache.getProjectBuildCache({
-          project: associatedProject,
-          projectOutputFolderNames: settings?.outputFolderNames || [],
-          buildCacheConfiguration,
-          terminal: logger.terminal,
-          operationStateHash: operationExecutionResult.getStateHash(),
-          phaseName: associatedPhase.name
-        });
+        const projectBuildCache: ProjectBuildCache = ProjectBuildCache.forOperation(
+          operationExecutionResult as OperationExecutionRecord,
+          {
+            buildCacheConfiguration,
+            terminal: logger.terminal
+          }
+        );
 
         const success: boolean = await projectBuildCache.trySetCacheEntryAsync(logger.terminal);
 
-        // eslint-disable-next-line no-console
-        console.log('- setting cache for', {
-          success,
-          name: associatedPhase.name,
-          package: associatedProject.packageName
-        });
+        if (success) {
+          logger.terminal.writeLine(
+            `Cache entry set for ${associatedPhase.name} (${associatedProject.packageName}) from previously generated output folders\n`
+          );
+        } else {
+          logger.terminal.writeErrorLine(
+            `Error creating a cache entry set for ${associatedPhase.name} (${associatedProject.packageName}) from previously generated output folders\n`
+          );
+          process.exit(1);
+        }
       }
     );
   }