Merge pull request microsoft#2856 from dmichon-msft/fix-build-cache-read

iclanton · web-flow · commit bcf6702ba2b8 · 2021-08-13T15:34:24.000-07:00
[rush] Allow build cache reads downstream of built projects during `rush build`
diff --git a/apps/rush-lib/src/logic/taskRunner/BaseBuilder.ts b/apps/rush-lib/src/logic/taskRunner/BaseBuilder.ts
@@ -27,9 +27,9 @@ export abstract class BaseBuilder {
   abstract readonly name: string;
 
   /**
-   * This flag determines if an incremental build is allowed for the task.
+   * This flag determines if the task is allowed to be skipped if up to date.
    */
-  abstract isIncrementalBuildAllowed: boolean;
+  abstract isSkipAllowed: boolean;
 
   /**
    * Assigned by execute().  True if the build script was an empty string.  Operationally an empty string is
diff --git a/apps/rush-lib/src/logic/taskRunner/ProjectBuilder.ts b/apps/rush-lib/src/logic/taskRunner/ProjectBuilder.ts
@@ -77,14 +77,18 @@ export class ProjectBuilder extends BaseBuilder {
     return ProjectBuilder.getTaskName(this._rushProject);
   }
 
-  public readonly isIncrementalBuildAllowed: boolean;
+  /**
+   * This property is mutated by TaskRunner, so is not readonly
+   */
+  public isSkipAllowed: boolean;
   public hadEmptyScript: boolean = false;
 
   private readonly _rushProject: RushConfigurationProject;
   private readonly _rushConfiguration: RushConfiguration;
   private readonly _buildCacheConfiguration: BuildCacheConfiguration | undefined;
   private readonly _commandName: string;
   private readonly _commandToRun: string;
+  private readonly _isCacheReadAllowed: boolean;
   private readonly _projectChangeAnalyzer: ProjectChangeAnalyzer;
   private readonly _packageDepsFilename: string;
 
@@ -101,7 +105,8 @@ export class ProjectBuilder extends BaseBuilder {
     this._buildCacheConfiguration = options.buildCacheConfiguration;
     this._commandName = options.commandName;
     this._commandToRun = options.commandToRun;
-    this.isIncrementalBuildAllowed = options.isIncrementalBuildAllowed;
+    this._isCacheReadAllowed = options.isIncrementalBuildAllowed;
+    this.isSkipAllowed = options.isIncrementalBuildAllowed;
     this._projectChangeAnalyzer = options.projectChangeAnalyzer;
     this._packageDepsFilename = options.packageDepsFilename;
   }
@@ -229,7 +234,7 @@ export class ProjectBuilder extends BaseBuilder {
             files,
             arguments: this._commandToRun
           };
-        } else if (this.isIncrementalBuildAllowed) {
+        } else if (this.isSkipAllowed) {
           // To test this code path:
           // Remove the `.git` folder then run "rush build --verbose"
           terminal.writeLine({
@@ -250,9 +255,8 @@ export class ProjectBuilder extends BaseBuilder {
         });
       }
 
-      // If the current command is allowed to do incremental builds, attempt to retrieve
-      // the project from the build cache or skip building, if appropriate.
-      if (this.isIncrementalBuildAllowed) {
+      // If allowed to read from the build cache, try retrieving the cache entry.
+      if (this._isCacheReadAllowed) {
         const projectBuildCache: ProjectBuildCache | undefined = await this._getProjectBuildCacheAsync(
           terminal,
           trackedFiles,
@@ -264,7 +268,10 @@ export class ProjectBuilder extends BaseBuilder {
         if (restoreFromCacheSuccess) {
           return TaskStatus.FromCache;
         }
+      }
 
+      // If allowed, attempt to skip building.
+      if (this.isSkipAllowed) {
         const isPackageUnchanged: boolean = !!(
           lastProjectBuildDeps &&
           projectBuildDeps &&
diff --git a/apps/rush-lib/src/logic/taskRunner/TaskRunner.ts b/apps/rush-lib/src/logic/taskRunner/TaskRunner.ts
@@ -338,7 +338,7 @@ export class TaskRunner {
 
     task.dependents.forEach((dependent: Task) => {
       if (!this._changedProjectsOnly) {
-        dependent.builder.isIncrementalBuildAllowed = false;
+        dependent.builder.isSkipAllowed = false;
       }
       dependent.dependencies.delete(task);
     });
@@ -355,7 +355,7 @@ export class TaskRunner {
     task.status = TaskStatus.SuccessWithWarning;
     task.dependents.forEach((dependent: Task) => {
       if (!this._changedProjectsOnly) {
-        dependent.builder.isIncrementalBuildAllowed = false;
+        dependent.builder.isSkipAllowed = false;
       }
       dependent.dependencies.delete(task);
     });
diff --git a/apps/rush-lib/src/logic/taskRunner/test/MockBuilder.ts b/apps/rush-lib/src/logic/taskRunner/test/MockBuilder.ts
@@ -10,7 +10,7 @@ export class MockBuilder extends BaseBuilder {
   private readonly _action: ((terminal: CollatedTerminal) => Promise<TaskStatus>) | undefined;
   public readonly name: string;
   public readonly hadEmptyScript: boolean = false;
-  public readonly isIncrementalBuildAllowed: boolean = false;
+  public readonly isSkipAllowed: boolean = false;
 
   public constructor(name: string, action?: (terminal: CollatedTerminal) => Promise<TaskStatus>) {
     super();
diff --git a/common/changes/@microsoft/rush/fix-rush-build-cache_2021-08-13-22-04.json b/common/changes/@microsoft/rush/fix-rush-build-cache_2021-08-13-22-04.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "When build cache is enabled in `rush build`, allow projects downstream to be satisfied from the cache if applicable. Cache reads will still be disabled for `rush rebuild`.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "dmichon-msft@users.noreply.github.com"
+}
