[rush] Split ProjectChangeAnalyzer, fix build cache hashes #4476

dmichon-msft · 2024-01-09T22:33:58Z

Summary

Makes #3994 more feasible.

Details

Splits the functionality of ProjectChangeAnalyzer into two components:

ProjectChangeAnalyzer#_tryGetSnapshotProviderAsync - Performs all the async I/O operations to determine the current state of the repository, including taking a snapshot of process.env so that per-operation hashes can be lazily computed in a stable manner.
InputSnapshot - Synchronous data structure representing the state of the repository, which can be queried for operation-level state hashes, or tracked file lists. Performs the evaluation of input/output collisions as part of evaluating said queries. Includes evaluation of incrementalBuildIgnoredGlobs and dependsOnEnvVars, but does not include the hashes from dependencies, since the Operation graph is not provided to this API.

Moves build cache hash computation to the beforeExecuteOperations tap in CacheableOperationPlugin and updates the computation to factor in the runtime operation graph, rather than only the npm dependencies.

For a specific example of how this alters the build cache, consider a repository with one project and two phases in rush build:
Project A
Phases _phase:first and _phase:second, with _phase:second depends on self: ['_phase:first']

Define a command line parameter --some-flag-for-first that is forwarded only to _phase:first.

Before this change, running rush build and rush build --some-flag-for-first would produce different cache IDs for A (_phase:first), but the same cache id for A (_phase:second).

With this change the cache IDs for A (_phase:second) are also impacted by the presence of the --some-flag-for-first, since it can change the output of A (_phase:first).

How it was tested

For the splitting, added unit tests for InputSnapshot and ProjectChangeAnalyzer#_tryGetSnapshotProviderAsync.

For the build cache IDs, temporarily removed applicability of the --production CLI parameter from _phase:test and validated that running node ./apps/rush/lib/start-dev.js test and node ./apps/rush/lib/start-dev.js test --production still produced different cache keys for _phase:test.

For comparison, validated that the command-line.json alternation and existing rush test and rush test --production had a collision on the cache keys for _phase:test.

Impacted documentation

Any documentation about how build cache keys are computed.

libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts

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

libraries/rush-lib/src/logic/snapshots/InputSnapshot.ts

D4N14L

We should do a pre-publish to verify that everything that is changing here doesn't break our flow, since we're very likely the largest customer on this tool.

common/changes/@microsoft/rush/split-project-change-analyzer_2024-01-09-22-11.json

common/changes/@microsoft/rush/split-project-change-analyzer_2024-01-09-22-06.json

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

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

libraries/rush-lib/src/logic/buildCache/ProjectBuildCache.ts

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

libraries/rush-lib/src/logic/snapshots/InputSnapshot.ts

libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts

iclanton · 2024-09-27T22:58:27Z

common/changes/@rushstack/lookup-by-path/split-project-change-analyzer_2024-09-24-23-54.json

+  "changes": [
+    {
+      "packageName": "@rushstack/lookup-by-path",
+      "comment": "Add \"IReadonlyLookupByPath\" interface to help unit tests for functions that consume \"LookupByPath\".",


Suggested change

"comment": "Add \"IReadonlyLookupByPath\" interface to help unit tests for functions that consume \"LookupByPath\".",

"comment": "Add `IReadonlyLookupByPath` interface to help unit tests for functions that consume `LookupByPath`.",

iclanton · 2024-09-29T21:28:33Z

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

-    await projectChangeAnalyzer._ensureInitializedAsync(terminal);
+
+    const analyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(this.rushConfiguration);
+    const snapshotProvider: (() => Promise<IInputSnapshot | undefined>) | undefined =


Suggested change

const snapshotProvider: (() => Promise<IInputSnapshot | undefined>) | undefined =

const snapshotProvider: IInputSnapshotProvider | undefined =

iclanton · 2024-09-29T21:29:29Z

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

+      terminal.writeLine(
+        `The Rush monorepo is not in a Git repository. Rush will proceed without incremental build support.`
+      );


Is this the only reason why the snapshot provider or initial snapshot would be undefined?

iclanton · 2024-09-29T21:31:56Z

libraries/rush-lib/src/logic/ProjectWatcher.ts

 import { Colorize, type ITerminal } from '@rushstack/terminal';

 import { Git } from './Git';
-import { ProjectChangeAnalyzer } from './ProjectChangeAnalyzer';
+import type { IInputSnapshot, IInputSnapshotProvider } from './snapshots/InputSnapshot';


The "snapshot" terminology may be a little confusing because Jest snapshots are stored in a similarly-named folder.

iclanton · 2024-09-29T21:32:42Z

libraries/rush-lib/src/logic/ProjectWatcher.ts

 * is responsible for change detection in all incremental builds) to determine what actually chanaged.
 *
 * Calling `waitForChange()` will return a promise that resolves when the package-deps of one or
 * more projects differ from the value the previous time it was invoked. The first time will always resolve with the full selection.
 */
 export class ProjectWatcher {
+  private readonly _snapshotProvider: IInputSnapshotProvider;


Suggested change

private readonly _snapshotProvider: IInputSnapshotProvider;

private readonly _getSnapshotAsync: IInputSnapshotProvider;

iclanton · 2024-09-29T21:38:41Z

libraries/rush-lib/src/logic/buildCache/ProjectBuildCache.ts

+    return options.buildCacheConfiguration.getCacheEntryId({
+      projectName: options.project.packageName,
+      projectStateHash: options.operationStateHash,
+      phaseName: options.phaseName


Suggested change

return options.buildCacheConfiguration.getCacheEntryId({

projectName: options.project.packageName,

projectStateHash: options.operationStateHash,

phaseName: options.phaseName

const { buildCacheConfiguration, project: { packageName }, operationStateHash, phaseName } = options;

return buildCacheConfiguration.getCacheEntryId({

projectName: packageName,

projectStateHash: operationStateHash,

phaseName

iclanton · 2024-09-29T21:40:47Z

libraries/rush-lib/src/logic/snapshots/InputSnapshot.ts

+import * as path from 'path';
+import { createHash, type Hash } from 'crypto';


Suggested change

import * as path from 'path';

import { createHash, type Hash } from 'crypto';

import * as path from 'node:path';

import { createHash, type Hash } from 'node:crypto';

iclanton · 2024-09-29T21:41:44Z

libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts

-      for (const projectDeps of projectHashDeps.values()) {
-        if (shrinkwrapHash) {
-          projectDeps.set(shrinkwrapFile, shrinkwrapHash);
+      for (const project of rushConfiguration.projects) {


Always analyzing all projects?

iclanton · 2024-09-29T22:03:33Z

libraries/rush-lib/src/logic/snapshots/InputSnapshot.ts

+ * The methods on this interface are idempotent and will return the same result regardless of when they are executed.
+ * @beta
+ */
+export interface IInputSnapshot {


Suggested change

export interface IInputSnapshot {

export interface IInputsSnapshot {

iclanton · 2024-09-29T22:03:58Z

libraries/rush-lib/src/logic/snapshots/InputSnapshot.ts

+ *
+ * @internal
+ */
+export class InputSnapshot implements IInputSnapshot {


Suggested change

export class InputSnapshot implements IInputSnapshot {

export class InputsSnapshot implements IInputsSnapshot {

dmichon-msft requested review from iclanton, octogonz, apostolisms and D4N14L as code owners January 9, 2024 22:33