Merge pull request #3193 from dmichon-msft/optimize-tar

[rush-lib] Optimizations for interacting with `tar` utility

Author: iclanton

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

@@ -6,7 +6,7 @@ import events from 'events';
 import * as crypto from 'crypto';
 import type * as stream from 'stream';
 import * as tar from 'tar';
-import { FileSystem, Path, ITerminal, FolderItem } from '@rushstack/node-core-library';
+import { Async, FileSystem, Path, ITerminal, FolderItem } from '@rushstack/node-core-library';
 
 import { RushConfigurationProject } from '../../api/RushConfigurationProject';
 import { ProjectChangeAnalyzer } from '../ProjectChangeAnalyzer';
@@ -268,16 +268,25 @@ export class ProjectBuildCache {
 
     const tarUtility: TarExecutable | undefined = await ProjectBuildCache._tryGetTarUtility(terminal);
     if (tarUtility) {
-      const tempLocalCacheEntryPath: string = this._localBuildCacheProvider.getCacheEntryPath(cacheId);
+      const finalLocalCacheEntryPath: string = this._localBuildCacheProvider.getCacheEntryPath(cacheId);
+      // Derive the temp file from the destination path to ensure they are on the same volume
+      const tempLocalCacheEntryPath: string = `${finalLocalCacheEntryPath}.temp`;
       const logFilePath: string = this._getTarLogFilePath();
       const tarExitCode: number = await tarUtility.tryCreateArchiveFromProjectPathsAsync({
         archivePath: tempLocalCacheEntryPath,
         paths: filesToCache.outputFilePaths,
         project: this._project,
         logFilePath
       });
+
       if (tarExitCode === 0) {
-        localCacheEntryPath = tempLocalCacheEntryPath;
+        // Move after the archive is finished so that if the process is interrupted we aren't left with an invalid file
+        await FileSystem.moveAsync({
+          sourcePath: tempLocalCacheEntryPath,
+          destinationPath: finalLocalCacheEntryPath,
+          overwrite: true
+        });
+        localCacheEntryPath = finalLocalCacheEntryPath;
       } else {
         terminal.writeWarningLine(
           `"tar" exited with code ${tarExitCode} while attempting to create the cache entry. ` +
@@ -358,80 +367,82 @@ export class ProjectBuildCache {
     return success;
   }
 
+  /**
+   * Walks the declared output folders of the project and collects a list of files.
+   * @returns The list of output files as project-relative paths, or `undefined` if a
+   *   symbolic link was encountered.
+   */
   private async _tryCollectPathsToCacheAsync(terminal: ITerminal): Promise<IPathsToCache | undefined> {
-    const projectFolderPath: string = this._project.projectFolder;
-    const outputFolderNamesThatExist: boolean[] = await Promise.all(
-      this._projectOutputFolderNames.map((outputFolderName) =>
-        FileSystem.existsAsync(`${projectFolderPath}/${outputFolderName}`)
-      )
-    );
+    const posixPrefix: string = this._project.projectFolder;
+    const outputFilePaths: string[] = [];
+    const queue: [string, string][] = [];
+
     const filteredOutputFolderNames: string[] = [];
-    for (let i: number = 0; i < outputFolderNamesThatExist.length; i++) {
-      if (outputFolderNamesThatExist[i]) {
-        filteredOutputFolderNames.push(this._projectOutputFolderNames[i]);
-      }
-    }
 
-    let encounteredEnumerationIssue: boolean = false;
-    function symbolicLinkPathCallback(entryPath: string): void {
-      terminal.writeError(`Unable to include "${entryPath}" in build cache. It is a symbolic link.`);
-      encounteredEnumerationIssue = true;
-    }
+    let hasSymbolicLinks: boolean = false;
 
-    const outputFilePaths: string[] = [];
-    for (const filteredOutputFolderName of filteredOutputFolderNames) {
-      if (encounteredEnumerationIssue) {
-        return undefined;
+    // Adds child directories to the queue, files to the path list, and bails on symlinks
+    function processChildren(relativePath: string, diskPath: string, children: FolderItem[]): void {
+      for (const child of children) {
+        const childRelativePath: string = `${relativePath}/${child.name}`;
+        if (child.isSymbolicLink()) {
+          terminal.writeError(
+            `Unable to include "${childRelativePath}" in build cache. It is a symbolic link.`
+          );
+          hasSymbolicLinks = true;
+        } else if (child.isDirectory()) {
+          queue.push([childRelativePath, `${diskPath}/${child.name}`]);
+        } else {
+          outputFilePaths.push(childRelativePath);
+        }
       }
+    }
 
-      const outputFilePathsForFolder: AsyncIterableIterator<string> = this._getPathsInFolder(
-        terminal,
-        symbolicLinkPathCallback,
-        filteredOutputFolderName,
-        `${projectFolderPath}/${filteredOutputFolderName}`
-      );
+    // Handle declared output folders.
+    for (const outputFolder of this._projectOutputFolderNames) {
+      const diskPath: string = `${posixPrefix}/${outputFolder}`;
+      try {
+        const children: FolderItem[] = await FileSystem.readFolderItemsAsync(diskPath);
+        processChildren(outputFolder, diskPath, children);
+        // The folder exists, record it
+        filteredOutputFolderNames.push(outputFolder);
+      } catch (error) {
+        if (!FileSystem.isNotExistError(error as Error)) {
+          throw error;
+        }
 
-      for await (const outputFilePath of outputFilePathsForFolder) {
-        outputFilePaths.push(outputFilePath);
+        // If the folder does not exist, ignore it.
       }
     }
 
-    if (encounteredEnumerationIssue) {
+    // Walk the tree in parallel
+    await Async.forEachAsync(
+      queue,
+      async ([relativePath, diskPath]: [string, string]) => {
+        const children: FolderItem[] = await FileSystem.readFolderItemsAsync(diskPath);
+        processChildren(relativePath, diskPath, children);
+      },
+      {
+        concurrency: 10
+      }
+    );
+
+    if (hasSymbolicLinks) {
+      // Symbolic links do not round-trip safely.
       return undefined;
     }
 
+    // Ensure stable output path order.
+    outputFilePaths.sort();
+
     return {
-      filteredOutputFolderNames,
-      outputFilePaths
+      outputFilePaths,
+      filteredOutputFolderNames
     };
   }
 
-  private async *_getPathsInFolder(
-    terminal: ITerminal,
-    symbolicLinkPathCallback: (path: string) => void,
-    posixPrefix: string,
-    folderPath: string
-  ): AsyncIterableIterator<string> {
-    const folderEntries: FolderItem[] = await FileSystem.readFolderItemsAsync(folderPath);
-    for (const folderEntry of folderEntries) {
-      const entryPath: string = `${posixPrefix}/${folderEntry.name}`;
-      if (folderEntry.isSymbolicLink()) {
-        symbolicLinkPathCallback(entryPath);
-      } else if (folderEntry.isDirectory()) {
-        yield* this._getPathsInFolder(
-          terminal,
-          symbolicLinkPathCallback,
-          entryPath,
-          `${folderPath}/${folderEntry.name}`
-        );
-      } else {
-        yield entryPath;
-      }
-    }
-  }
-
   private _getTarLogFilePath(): string {
-    return path.join(this._project.projectRushTempFolder, 'build-cache-tar.log');
+    return path.join(this._project.projectRushTempFolder, `${this._cacheId}.log`);
   }
 
   private static async _getCacheId(options: IProjectBuildCacheOptions): Promise<string | undefined> {

apps/rush-lib/src/utilities/TarExecutable.ts

@@ -71,8 +71,8 @@ export class TarExecutable {
    */
   public async tryCreateArchiveFromProjectPathsAsync(options: ICreateArchiveOptions): Promise<number> {
     const { project, archivePath, paths, logFilePath } = options;
-    const pathsListFilePath: string = `${project.projectRushTempFolder}/tarPaths_${Date.now()}`;
-    await FileSystem.writeFileAsync(pathsListFilePath, paths.join('\n'));
+
+    const tarInput: string = paths.join('\n');
 
     // On Windows, tar.exe will report a "Failed to clean up compressor" error if the target folder
     // does not exist (GitHub #2622)
@@ -89,27 +89,24 @@ export class TarExecutable {
         archivePath,
         // [Windows bsdtar 3.3.2] -z, -j, -J, --lzma  Compress archive with gzip/bzip2/xz/lzma
         '-z',
-        // [Windows bsdtar 3.3.2] -C <dir>  Change to <dir> before processing remaining files
-        '-C',
-        projectFolderPath,
         // [GNU tar 1.33] -T, --files-from=FILE      get names to extract or create from FILE
         //
         // Windows bsdtar does not document this parameter, but seems to accept it.
-        '--files-from',
-        pathsListFilePath
+        '--files-from=-'
       ],
       projectFolderPath,
-      logFilePath
+      logFilePath,
+      tarInput
     );
-    await FileSystem.deleteFileAsync(pathsListFilePath);
 
     return tarExitCode;
   }
 
   private async _spawnTarWithLoggingAsync(
     args: string[],
     currentWorkingDirectory: string,
-    logFilePath: string
+    logFilePath: string,
+    input?: string
   ): Promise<number> {
     // Runs "tar" with the specified args and logs its output to the specified location.
     // The log file looks like this:
@@ -143,6 +140,9 @@ export class TarExecutable {
         `Start time: ${new Date().toString()}`,
         `Invoking "${this._tarExecutablePath} ${args.join(' ')}"`,
         '',
+        `======= BEGIN PROCESS INPUT ======`,
+        input || '',
+        '======== END PROCESS INPUT =======',
         '======= BEGIN PROCESS OUTPUT =======',
         ''
       ].join('\n')
@@ -152,8 +152,13 @@ export class TarExecutable {
       currentWorkingDirectory: currentWorkingDirectory
     });
 
-    childProcess.stdout?.on('data', (chunk) => fileWriter.write(`[stdout] ${chunk}`));
-    childProcess.stderr?.on('data', (chunk) => fileWriter.write(`[stderr] ${chunk}`));
+    childProcess.stdout!.on('data', (chunk) => fileWriter.write(`[stdout] ${chunk}`));
+    childProcess.stderr!.on('data', (chunk) => fileWriter.write(`[stderr] ${chunk}`));
+
+    if (input !== undefined) {
+      childProcess.stdin!.write(input, 'utf-8');
+      childProcess.stdin!.end();
+    }
 
     const [tarExitCode] = await events.once(childProcess, 'exit');
 

common/changes/@microsoft/rush/optimize-tar_2021-11-09-22-50.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Optimize invocation of tar to use stdin instead of a temporary file.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}

common/changes/@microsoft/rush/tar-stdin_2022-02-02-22-00.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Revise architecture of symbolic link scan to use a queue and parallel file system calls.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}

common/changes/@microsoft/rush/tar-stdin_2022-02-02-22-27.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Create separate tar logs per phase based on cache id.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}

common/changes/@microsoft/rush/tar-stdin_2022-02-02-22-38.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Pack tar to a temp file, then move into the cache to ensure cache integrity.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}