Merge pull request microsoft#2802 from elliot-nelson/rebuild

octogonz · web-flow · commit 23124f5dfd2e · 2021-07-16T18:05:16.000-07:00
[rush] Rebuild ignores existing build cache entries
diff --git a/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/BulkScriptAction.ts
@@ -78,7 +78,6 @@ export class BulkScriptAction extends BaseScriptAction {
   private _verboseParameter!: CommandLineFlagParameter;
   private _parallelismParameter: CommandLineStringParameter | undefined;
   private _ignoreHooksParameter!: CommandLineFlagParameter;
-  private _disableBuildCacheFlag: CommandLineFlagParameter | undefined;
 
   public constructor(options: IBulkScriptActionOptions) {
     super(options);
@@ -126,7 +125,7 @@ export class BulkScriptAction extends BaseScriptAction {
 
     const terminal: Terminal = new Terminal(new ConsoleTerminalProvider());
     let buildCacheConfiguration: BuildCacheConfiguration | undefined;
-    if (!this._disableBuildCacheFlag?.value && !this._disableBuildCache) {
+    if (!this._disableBuildCache) {
       buildCacheConfiguration = await BuildCacheConfiguration.tryLoadAsync(terminal, this.rushConfiguration);
     }
 
@@ -300,11 +299,6 @@ export class BulkScriptAction extends BaseScriptAction {
       description: `Skips execution of the "eventHooks" scripts defined in rush.json. Make sure you know what you are skipping.`
     });
 
-    this._disableBuildCacheFlag = this.defineFlagParameter({
-      parameterLongName: '--disable-build-cache',
-      description: '(EXPERIMENTAL) Disables the build cache for this command invocation.'
-    });
-
     this.defineScriptParameters();
   }
 
diff --git a/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap b/apps/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap
@@ -119,7 +119,7 @@ exports[`CommandLineHelp prints the help for each action: build 1`] = `
                   [-o PROJECT] [-i PROJECT] [-I PROJECT]
                   [--to-version-policy VERSION_POLICY_NAME]
                   [--from-version-policy VERSION_POLICY_NAME] [-v] [-c]
-                  [--ignore-hooks] [--disable-build-cache] [-s] [-m]
+                  [--ignore-hooks] [-s] [-m]
                   
 
 This command is similar to \\"rush rebuild\\", except that \\"rush build\\" performs 
@@ -236,9 +236,6 @@ Optional arguments:
   --ignore-hooks        Skips execution of the \\"eventHooks\\" scripts defined 
                         in rush.json. Make sure you know what you are 
                         skipping.
-  --disable-build-cache
-                        (EXPERIMENTAL) Disables the build cache for this 
-                        command invocation.
   -s, --ship            Perform a production build, including minification 
                         and localization steps
   -m, --minimal         Perform a fast build, which disables certain tasks 
@@ -363,7 +360,7 @@ exports[`CommandLineHelp prints the help for each action: import-strings 1`] = `
                            [-f PROJECT] [-o PROJECT] [-i PROJECT] [-I PROJECT]
                            [--to-version-policy VERSION_POLICY_NAME]
                            [--from-version-policy VERSION_POLICY_NAME] [-v]
-                           [--ignore-hooks] [--disable-build-cache]
+                           [--ignore-hooks]
                            [--locale {en-us,fr-fr,es-es,zh-cn}]
                            
 
@@ -463,9 +460,6 @@ Optional arguments:
   --ignore-hooks        Skips execution of the \\"eventHooks\\" scripts defined 
                         in rush.json. Make sure you know what you are 
                         skipping.
-  --disable-build-cache
-                        (EXPERIMENTAL) Disables the build cache for this 
-                        command invocation.
   --locale {en-us,fr-fr,es-es,zh-cn}
                         Selects a single instead of the default locale 
                         (en-us) for non-ship builds or all locales for ship 
@@ -817,7 +811,7 @@ exports[`CommandLineHelp prints the help for each action: rebuild 1`] = `
                     [-o PROJECT] [-i PROJECT] [-I PROJECT]
                     [--to-version-policy VERSION_POLICY_NAME]
                     [--from-version-policy VERSION_POLICY_NAME] [-v]
-                    [--ignore-hooks] [--disable-build-cache] [-s] [-m]
+                    [--ignore-hooks] [-s] [-m]
                     
 
 This command assumes that the package.json file for each project contains a 
@@ -922,9 +916,6 @@ Optional arguments:
   --ignore-hooks        Skips execution of the \\"eventHooks\\" scripts defined 
                         in rush.json. Make sure you know what you are 
                         skipping.
-  --disable-build-cache
-                        (EXPERIMENTAL) Disables the build cache for this 
-                        command invocation.
   -s, --ship            Perform a production build, including minification 
                         and localization steps
   -m, --minimal         Perform a fast build, which disables certain tasks 
diff --git a/apps/rush-lib/src/logic/buildCache/test/__snapshots__/AzureStorageBuildCacheProvider.test.ts.snap b/apps/rush-lib/src/logic/buildCache/test/__snapshots__/AzureStorageBuildCacheProvider.test.ts.snap
@@ -14,6 +14,4 @@ Array [
 ]
 `;
 
-exports[`AzureStorageBuildCacheProvider Uses a correct list of Azure authority hosts 1`] = `"The specified Azure Environment (\\"INCORRECT_AZURE_ENVIRONMENT\\") is invalid. If it is specified, it must be one of: AzureChina, AzureGermany, AzureGovernment, AzurePublicCloud"`;
-
 exports[`AzureStorageBuildCacheProvider uses a correct list of Azure authority hosts 1`] = `"The specified Azure Environment (\\"INCORRECT_AZURE_ENVIRONMENT\\") is invalid. If it is specified, it must be one of: AzureChina, AzureGermany, AzureGovernment, AzurePublicCloud"`;
diff --git a/apps/rush-lib/src/logic/taskRunner/ProjectBuilder.ts b/apps/rush-lib/src/logic/taskRunner/ProjectBuilder.ts
@@ -10,14 +10,16 @@ import {
   JsonObject,
   NewlineKind,
   InternalError,
-  Terminal
+  Terminal,
+  ColorValue
 } from '@rushstack/node-core-library';
 import {
   TerminalChunkKind,
   TextRewriterTransform,
   StderrLineTransform,
   SplitterTransform,
-  DiscardStdoutTransform
+  DiscardStdoutTransform,
+  PrintUtilities
 } from '@rushstack/terminal';
 import { CollatedTerminal } from '@rushstack/stream-collator';
 
@@ -225,138 +227,155 @@ export class ProjectBuilder extends BaseBuilder {
             files,
             arguments: this._commandToRun
           };
-        } else {
-          terminal.writeLine(
-            'Unable to calculate incremental build state. Instead running full rebuild. Ensure Git is present.'
-          );
+        } else if (this.isIncrementalBuildAllowed) {
+          // To test this code path:
+          // Remove the `.git` folder then run "rush build --verbose"
+          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.'
+            ),
+            foregroundColor: ColorValue.Cyan
+          });
         }
       } catch (error) {
-        terminal.writeLine(
-          'Error calculating incremental build state. Instead running full rebuild. ' + error.toString()
-        );
+        // 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.toString());
+        terminal.writeLine({
+          text: 'Rush will proceed without incremental build, caching, and change detection.',
+          foregroundColor: ColorValue.Cyan
+        });
       }
 
-      const isPackageUnchanged: boolean = !!(
-        lastProjectBuildDeps &&
-        projectBuildDeps &&
-        projectBuildDeps.arguments === lastProjectBuildDeps.arguments &&
-        _areShallowEqual(projectBuildDeps.files, lastProjectBuildDeps.files)
-      );
+      // 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) {
+        const projectBuildCache: ProjectBuildCache | undefined = await this._getProjectBuildCacheAsync(
+          terminal,
+          trackedFiles,
+          context.repoCommandLineConfiguration
+        );
+        const restoreFromCacheSuccess: boolean | undefined =
+          await projectBuildCache?.tryRestoreFromCacheAsync(terminal);
 
-      const projectBuildCache: ProjectBuildCache | undefined = await this._getProjectBuildCacheAsync(
-        terminal,
-        trackedFiles,
-        context.repoCommandLineConfiguration
-      );
-      const restoreFromCacheSuccess: boolean | undefined = await projectBuildCache?.tryRestoreFromCacheAsync(
-        terminal
-      );
+        if (restoreFromCacheSuccess) {
+          return TaskStatus.FromCache;
+        }
 
-      if (restoreFromCacheSuccess) {
-        return TaskStatus.FromCache;
-      } else if (isPackageUnchanged && this.isIncrementalBuildAllowed) {
-        return TaskStatus.Skipped;
-      } else {
-        // If the deps file exists, remove it before starting a build.
-        FileSystem.deleteFile(currentDepsPath);
-
-        // TODO: Remove legacyDepsPath with the next major release of Rush
-        const legacyDepsPath: string = path.join(this._rushProject.projectFolder, 'package-deps.json');
-        // Delete the legacy package-deps.json
-        FileSystem.deleteFile(legacyDepsPath);
-
-        if (!this._commandToRun) {
-          // Write deps on success.
-          if (projectBuildDeps) {
-            JsonFile.save(projectBuildDeps, currentDepsPath, {
-              ensureFolderExists: true
-            });
-          }
+        const isPackageUnchanged: boolean = !!(
+          lastProjectBuildDeps &&
+          projectBuildDeps &&
+          projectBuildDeps.arguments === lastProjectBuildDeps.arguments &&
+          _areShallowEqual(projectBuildDeps.files, lastProjectBuildDeps.files)
+        );
 
-          return TaskStatus.Success;
+        if (isPackageUnchanged) {
+          return TaskStatus.Skipped;
         }
+      }
 
-        // Run the task
-        terminal.writeLine('Invoking: ' + this._commandToRun);
+      // If the deps file exists, remove it before starting a build.
+      FileSystem.deleteFile(currentDepsPath);
 
-        const task: child_process.ChildProcess = Utilities.executeLifecycleCommandAsync(this._commandToRun, {
-          rushConfiguration: this._rushConfiguration,
-          workingDirectory: projectFolder,
-          initCwd: this._rushConfiguration.commonTempFolder,
-          handleOutput: true,
-          environmentPathOptions: {
-            includeProjectBin: true
-          }
-        });
+      // TODO: Remove legacyDepsPath with the next major release of Rush
+      const legacyDepsPath: string = path.join(this._rushProject.projectFolder, 'package-deps.json');
+      // Delete the legacy package-deps.json
+      FileSystem.deleteFile(legacyDepsPath);
 
-        // Hook into events, in order to get live streaming of build log
-        if (task.stdout !== null) {
-          task.stdout.on('data', (data: Buffer) => {
-            const text: string = data.toString();
-            collatedTerminal.writeChunk({ text, kind: TerminalChunkKind.Stdout });
+      if (!this._commandToRun) {
+        // Write deps on success.
+        if (projectBuildDeps) {
+          JsonFile.save(projectBuildDeps, currentDepsPath, {
+            ensureFolderExists: true
           });
         }
-        if (task.stderr !== null) {
-          task.stderr.on('data', (data: Buffer) => {
-            const text: string = data.toString();
-            collatedTerminal.writeChunk({ text, kind: TerminalChunkKind.Stderr });
-            hasWarningOrError = true;
-          });
+
+        return TaskStatus.Success;
+      }
+
+      // Run the task
+      terminal.writeLine('Invoking: ' + this._commandToRun);
+
+      const task: child_process.ChildProcess = Utilities.executeLifecycleCommandAsync(this._commandToRun, {
+        rushConfiguration: this._rushConfiguration,
+        workingDirectory: projectFolder,
+        initCwd: this._rushConfiguration.commonTempFolder,
+        handleOutput: true,
+        environmentPathOptions: {
+          includeProjectBin: true
         }
+      });
+
+      // Hook into events, in order to get live streaming of build log
+      if (task.stdout !== null) {
+        task.stdout.on('data', (data: Buffer) => {
+          const text: string = data.toString();
+          collatedTerminal.writeChunk({ text, kind: TerminalChunkKind.Stdout });
+        });
+      }
+      if (task.stderr !== null) {
+        task.stderr.on('data', (data: Buffer) => {
+          const text: string = data.toString();
+          collatedTerminal.writeChunk({ text, kind: TerminalChunkKind.Stderr });
+          hasWarningOrError = true;
+        });
+      }
 
-        let status: TaskStatus = await new Promise(
-          (resolve: (status: TaskStatus) => void, reject: (error: TaskError) => void) => {
-            task.on('close', (code: number) => {
-              try {
-                if (code !== 0) {
-                  reject(new TaskError('error', `Returned error code: ${code}`));
-                } else if (hasWarningOrError) {
-                  resolve(TaskStatus.SuccessWithWarning);
-                } else {
-                  resolve(TaskStatus.Success);
-                }
-              } catch (error) {
-                reject(error);
+      let status: TaskStatus = await new Promise(
+        (resolve: (status: TaskStatus) => void, reject: (error: TaskError) => void) => {
+          task.on('close', (code: number) => {
+            try {
+              if (code !== 0) {
+                reject(new TaskError('error', `Returned error code: ${code}`));
+              } else if (hasWarningOrError) {
+                resolve(TaskStatus.SuccessWithWarning);
+              } else {
+                resolve(TaskStatus.Success);
               }
-            });
+            } catch (error) {
+              reject(error);
+            }
+          });
+        }
+      );
+
+      if (status === TaskStatus.Success && projectBuildDeps) {
+        // Write deps on success.
+        const writeProjectStatePromise: Promise<boolean> = JsonFile.saveAsync(
+          projectBuildDeps,
+          currentDepsPath,
+          {
+            ensureFolderExists: true
           }
         );
 
-        if (status === TaskStatus.Success && projectBuildDeps) {
-          // Write deps on success.
-          const writeProjectStatePromise: Promise<boolean> = JsonFile.saveAsync(
-            projectBuildDeps,
-            currentDepsPath,
-            {
-              ensureFolderExists: true
-            }
-          );
-
-          const setCacheEntryPromise: Promise<boolean | undefined> = this.tryWriteCacheEntryAsync(
-            terminal,
-            trackedFiles,
-            context.repoCommandLineConfiguration
-          );
+        // 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.
+        const setCacheEntryPromise: Promise<boolean | undefined> = this.tryWriteCacheEntryAsync(
+          terminal,
+          trackedFiles,
+          context.repoCommandLineConfiguration
+        );
 
-          const [, cacheWriteSuccess] = await Promise.all([writeProjectStatePromise, setCacheEntryPromise]);
+        const [, cacheWriteSuccess] = await Promise.all([writeProjectStatePromise, setCacheEntryPromise]);
 
-          if (terminalProvider.hasErrors) {
-            status = TaskStatus.Failure;
-          } else if (cacheWriteSuccess === false) {
-            status = TaskStatus.SuccessWithWarning;
-          }
+        if (terminalProvider.hasErrors) {
+          status = TaskStatus.Failure;
+        } else if (cacheWriteSuccess === false) {
+          status = TaskStatus.SuccessWithWarning;
         }
+      }
 
-        normalizeNewlineTransform.close();
+      normalizeNewlineTransform.close();
 
-        // If the pipeline is wired up correctly, then closing normalizeNewlineTransform should
-        // have closed projectLogWritable.
-        if (projectLogWritable.isOpen) {
-          throw new InternalError('The output file handle was not closed');
-        }
-
-        return status;
+      // If the pipeline is wired up correctly, then closing normalizeNewlineTransform should
+      // have closed projectLogWritable.
+      if (projectLogWritable.isOpen) {
+        throw new InternalError('The output file handle was not closed');
       }
+
+      return status;
     } finally {
       projectLogWritable.close();
     }
diff --git a/common/changes/@microsoft/rush/rebuild_2021-07-09-20-31.json b/common/changes/@microsoft/rush/rebuild_2021-07-09-20-31.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "When the experimental build cache is enabled, \"rush rebuild\" now forces cached projects to be rebuilt  (GitHub #2802)",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "elliot.nelson@users.noreply.github.com"
+}
diff --git a/common/changes/@microsoft/rush/rebuild_2021-07-13-21-32.json b/common/changes/@microsoft/rush/rebuild_2021-07-13-21-32.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "(Breaking change) Remove the experimental \"--disable-build-cache\" command line parameter.",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "elliot-nelson@users.noreply.github.com"
+}

Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,4 @@ Array [`
`14`	`14`	`]`
`15`	`15`	`;
`16`	`16`
`17`		-exports[`AzureStorageBuildCacheProvider Uses a correct list of Azure authority hosts 1`] = `"The specified Azure Environment (\\"INCORRECT_AZURE_ENVIRONMENT\\") is invalid. If it is specified, it must be one of: AzureChina, AzureGermany, AzureGovernment, AzurePublicCloud"`;
`18`		`-`
`19`	`17`	exports[`AzureStorageBuildCacheProvider uses a correct list of Azure authority hosts 1`] = `"The specified Azure Environment (\\"INCORRECT_AZURE_ENVIRONMENT\\") is invalid. If it is specified, it must be one of: AzureChina, AzureGermany, AzureGovernment, AzurePublicCloud"`;