Update the delete files plugin to delete folder trees as well

spring6225 · Jul 19, 2023 · 51dc129 · 51dc129
1 parent 0a58294
commit 51dc129
Show file tree

Hide file tree

Showing 40 changed files with 177 additions and 151 deletions.
diff --git a/apps/heft/src/plugins/CopyFilesPlugin.ts b/apps/heft/src/plugins/CopyFilesPlugin.ts
@@ -1,13 +1,14 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
+import type * as fs from 'fs';
 import * as path from 'path';
 import { AlreadyExistsBehavior, FileSystem, Async, ITerminal } from '@rushstack/node-core-library';
 
 import { Constants } from '../utilities/Constants';
 import {
   normalizeFileSelectionSpecifier,
-  getFilePathsAsync,
+  getFileSelectionSpecifierPathsAsync,
   type IFileSelectionSpecifier
 } from './FileGlobSpecifier';
 import type { HeftConfiguration } from '../configuration/HeftConfiguration';
@@ -93,7 +94,7 @@ function _normalizeCopyOperation(rootPath: string, copyOperation: ICopyOperation
 async function _getCopyDescriptorsAsync(
   rootPath: string,
   copyConfigurations: Iterable<ICopyOperation>,
-  fs: WatchFileSystemAdapter | undefined
+  fileSystemAdapter: WatchFileSystemAdapter | undefined
 ): Promise<Map<string, ICopyDescriptor>> {
   // Create a map to deduplicate and prevent double-writes
   // resolvedDestinationFilePath -> descriptor
@@ -108,11 +109,15 @@ async function _getCopyDescriptorsAsync(
       // and the filename in "includeGlobs". Also, we know that the sourcePath will be set because of the above
       // call to _normalizeCopyOperation
       const sourceFolder: string = copyConfiguration.sourcePath!;
-      const sourceFilePaths: Set<string> | undefined = await getFilePathsAsync(copyConfiguration, fs);
+      const sourceFiles: Map<string, fs.Dirent> = await getFileSelectionSpecifierPathsAsync({
+        fileGlobSpecifier: copyConfiguration,
+        fileSystemAdapter
+      });
 
       // Dedupe and throw if a double-write is detected
       for (const destinationFolderPath of copyConfiguration.destinationFolders) {
-        for (const sourceFilePath of sourceFilePaths!) {
+        // We only need to care about the keys of the map since we know all the keys are paths to files
+        for (const sourceFilePath of sourceFiles.keys()) {
           // Only include the relative path from the sourceFolder if flatten is false
           const resolvedDestinationPath: string = path.resolve(
             destinationFolderPath,

diff --git a/apps/heft/src/plugins/DeleteFilesPlugin.ts b/apps/heft/src/plugins/DeleteFilesPlugin.ts
@@ -1,12 +1,12 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
-import * as path from 'path';
+import type * as fs from 'fs';
 import { FileSystem, Async, ITerminal } from '@rushstack/node-core-library';
 
 import { Constants } from '../utilities/Constants';
 import {
-  getFilePathsAsync,
+  getFileSelectionSpecifierPathsAsync,
   normalizeFileSelectionSpecifier,
   type IFileSelectionSpecifier
 } from './FileGlobSpecifier';
@@ -25,42 +25,64 @@ interface IDeleteFilesPluginOptions {
   deleteOperations: IDeleteOperation[];
 }
 
+interface IGetPathsToDeleteResult {
+  filesToDelete: Set<string>;
+  foldersToDelete: Set<string>;
+}
+
 async function _getPathsToDeleteAsync(
   rootPath: string,
   deleteOperations: Iterable<IDeleteOperation>
-): Promise<Set<string>> {
-  const pathsToDelete: Set<string> = new Set();
+): Promise<IGetPathsToDeleteResult> {
+  const result: IGetPathsToDeleteResult = {
+    filesToDelete: new Set<string>(),
+    foldersToDelete: new Set<string>()
+  };
+
   await Async.forEachAsync(
     deleteOperations,
     async (deleteOperation: IDeleteOperation) => {
       normalizeFileSelectionSpecifier(rootPath, deleteOperation);
 
       // Glob the files under the source path and add them to the set of files to delete
-      const sourceFilePaths: Set<string> = await getFilePathsAsync(deleteOperation);
-      for (const sourceFilePath of sourceFilePaths) {
-        pathsToDelete.add(sourceFilePath);
+      const sourcePaths: Map<string, fs.Dirent> = await getFileSelectionSpecifierPathsAsync({
+        fileGlobSpecifier: deleteOperation,
+        includeFolders: true
+      });
+      for (const [sourcePath, dirent] of sourcePaths) {
+        if (dirent.isDirectory()) {
+          result.foldersToDelete.add(sourcePath);
+        } else {
+          result.filesToDelete.add(sourcePath);
+        }
       }
     },
     { concurrency: Constants.maxParallelism }
   );
 
-  return pathsToDelete;
+  return result;
 }
 
 export async function deleteFilesAsync(
   rootPath: string,
   deleteOperations: Iterable<IDeleteOperation>,
   terminal: ITerminal
 ): Promise<void> {
-  const pathsToDelete: Set<string> = await _getPathsToDeleteAsync(rootPath, deleteOperations);
+  const pathsToDelete: IGetPathsToDeleteResult = await _getPathsToDeleteAsync(rootPath, deleteOperations);
   await _deleteFilesInnerAsync(pathsToDelete, terminal);
 }
 
-async function _deleteFilesInnerAsync(pathsToDelete: Set<string>, terminal: ITerminal): Promise<void> {
+async function _deleteFilesInnerAsync(
+  pathsToDelete: IGetPathsToDeleteResult,
+  terminal: ITerminal
+): Promise<void> {
   let deletedFiles: number = 0;
   let deletedFolders: number = 0;
+
+  const { filesToDelete, foldersToDelete } = pathsToDelete;
+
   await Async.forEachAsync(
-    pathsToDelete,
+    filesToDelete,
     async (pathToDelete: string) => {
       try {
         await FileSystem.deleteFileAsync(pathToDelete, { throwIfNotExists: true });
@@ -69,16 +91,25 @@ async function _deleteFilesInnerAsync(pathsToDelete: Set<string>, terminal: ITer
       } catch (error) {
         // If it doesn't exist, we can ignore the error.
         if (!FileSystem.isNotExistError(error)) {
-          // When we encounter an error relating to deleting a directory as if it was a file,
-          // attempt to delete the folder. Windows throws the unlink not permitted error, while
-          // linux throws the EISDIR error.
-          if (FileSystem.isUnlinkNotPermittedError(error) || FileSystem.isDirectoryError(error)) {
-            await FileSystem.deleteFolderAsync(pathToDelete);
-            terminal.writeVerboseLine(`Deleted folder "${pathToDelete}".`);
-            deletedFolders++;
-          } else {
-            throw error;
-          }
+          throw error;
+        }
+      }
+    },
+    { concurrency: Constants.maxParallelism }
+  );
+
+  // Clear out any folders that were encountered during the file deletion process.
+  await Async.forEachAsync(
+    foldersToDelete,
+    async (folderToDelete: string) => {
+      try {
+        await FileSystem.deleteFolderAsync(folderToDelete);
+        terminal.writeVerboseLine(`Deleted folder "${folderToDelete}".`);
+        deletedFolders++;
+      } catch (error) {
+        // If it doesn't exist, we can ignore the error.
+        if (!FileSystem.isNotExistError(error)) {
+          throw error;
         }
       }
     },

diff --git a/apps/heft/src/plugins/FileGlobSpecifier.ts b/apps/heft/src/plugins/FileGlobSpecifier.ts
@@ -1,8 +1,9 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
+import type * as fs from 'fs';
 import * as path from 'path';
-import glob, { FileSystemAdapter } from 'fast-glob';
+import glob, { type FileSystemAdapter, type Entry } from 'fast-glob';
 
 import { Async } from '@rushstack/node-core-library';
 
@@ -71,6 +72,12 @@ export interface IGlobOptions {
   dot?: boolean;
 }
 
+export interface IGetFileSelectionSpecifierPathsOptions {
+  fileGlobSpecifier: IFileSelectionSpecifier;
+  includeFolders?: boolean;
+  fileSystemAdapter?: FileSystemAdapter;
+}
+
 /**
  * Glob a set of files and return a list of paths that match the provided patterns.
  *
@@ -129,36 +136,46 @@ export async function watchGlobAsync(
   return results;
 }
 
-export async function getFilePathsAsync(
-  fileGlobSpecifier: IFileSelectionSpecifier,
-  fs?: FileSystemAdapter
-): Promise<Set<string>> {
-  const rawFiles: string[] = await glob(fileGlobSpecifier.includeGlobs!, {
-    fs,
+export async function getFileSelectionSpecifierPathsAsync(
+  options: IGetFileSelectionSpecifierPathsOptions
+): Promise<Map<string, fs.Dirent>> {
+  const { fileGlobSpecifier, includeFolders, fileSystemAdapter } = options;
+  const rawEntries: Entry[] = await glob(fileGlobSpecifier.includeGlobs!, {
+    fs: fileSystemAdapter,
     cwd: fileGlobSpecifier.sourcePath,
     ignore: fileGlobSpecifier.excludeGlobs,
     dot: true,
-    absolute: true
+    absolute: true,
+    onlyFiles: !includeFolders,
+    stats: true
   });
 
-  if (fs && isWatchFileSystemAdapter(fs)) {
-    const changedFiles: Set<string> = new Set();
+  let results: Map<string, fs.Dirent>;
+  if (fileSystemAdapter && isWatchFileSystemAdapter(fileSystemAdapter)) {
+    results = new Map();
     await Async.forEachAsync(
-      rawFiles,
-      async (file: string) => {
-        const state: IWatchedFileState = await fs.getStateAndTrackAsync(path.normalize(file));
+      rawEntries,
+      async (entry: Entry) => {
+        const { path: filePath, dirent } = entry;
+        if (entry.dirent.isDirectory()) {
+          return;
+        }
+        const state: IWatchedFileState = await fileSystemAdapter.getStateAndTrackAsync(
+          path.normalize(filePath)
+        );
         if (state.changed) {
-          changedFiles.add(file);
+          results.set(filePath, dirent);
         }
       },
       {
         concurrency: 20
       }
     );
-    return changedFiles;
+  } else {
+    results = new Map(rawEntries.map((entry) => [entry.path, entry.dirent]));
   }
 
-  return new Set(rawFiles);
+  return results;
 }
 
 export function normalizeFileSelectionSpecifier(

diff --git a/build-tests-samples/heft-node-basic-tutorial/config/heft.json b/build-tests-samples/heft-node-basic-tutorial/config/heft.json
@@ -7,7 +7,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist" }, { "sourcePath": "lib" }],
+      "cleanFiles": [{ "includeGlobs": ["dist", "lib"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {

diff --git a/build-tests-samples/heft-node-jest-tutorial/config/heft.json b/build-tests-samples/heft-node-jest-tutorial/config/heft.json
@@ -4,7 +4,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist" }, { "sourcePath": "lib" }],
+      "cleanFiles": [{ "includeGlobs": ["dist", "lib"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {

diff --git a/build-tests-samples/heft-serverless-stack-tutorial/config/heft.json b/build-tests-samples/heft-serverless-stack-tutorial/config/heft.json
@@ -7,7 +7,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist" }, { "sourcePath": "lib" }, { "sourcePath": ".build" }],
+      "cleanFiles": [{ "includeGlobs": ["dist", "lib", ".build"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {

diff --git a/build-tests-samples/heft-storybook-react-tutorial/config/heft.json b/build-tests-samples/heft-storybook-react-tutorial/config/heft.json
@@ -7,7 +7,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist" }, { "sourcePath": "lib" }, { "sourcePath": "lib-commonjs" }],
+      "cleanFiles": [{ "includeGlobs": ["dist", "lib", "lib-commonjs"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {

diff --git a/build-tests-samples/heft-webpack-basic-tutorial/config/heft.json b/build-tests-samples/heft-webpack-basic-tutorial/config/heft.json
@@ -7,7 +7,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist" }, { "sourcePath": "lib" }, { "sourcePath": "lib-commonjs" }],
+      "cleanFiles": [{ "includeGlobs": ["dist", "lib", "lib-commonjs"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {

diff --git a/build-tests-samples/packlets-tutorial/config/heft.json b/build-tests-samples/packlets-tutorial/config/heft.json
@@ -7,7 +7,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist" }, { "sourcePath": "lib" }],
+      "cleanFiles": [{ "includeGlobs": ["dist", "lib"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {

diff --git a/build-tests/hashed-folder-copy-plugin-webpack4-test/config/heft.json b/build-tests/hashed-folder-copy-plugin-webpack4-test/config/heft.json
@@ -7,7 +7,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist-dev" }, { "sourcePath": "dist-prod" }, { "sourcePath": "lib" }],
+      "cleanFiles": [{ "includeGlobs": ["dist-dev", "dist-prod", "lib"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {

diff --git a/build-tests/hashed-folder-copy-plugin-webpack5-test/config/heft.json b/build-tests/hashed-folder-copy-plugin-webpack5-test/config/heft.json
@@ -7,7 +7,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist-dev" }, { "sourcePath": "dist-prod" }, { "sourcePath": "lib" }],
+      "cleanFiles": [{ "includeGlobs": ["dist-dev", "dist-prod", "lib"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {

diff --git a/build-tests/heft-copy-files-test/config/heft.json b/build-tests/heft-copy-files-test/config/heft.json
@@ -9,31 +9,17 @@
     "build": {
       "cleanFiles": [
         {
-          "sourcePath": "out-all"
-        },
-        {
-          "sourcePath": "out-all-linked"
-        },
-        {
-          "sourcePath": "out-all-flattened"
-        },
-        {
-          "sourcePath": "out-all-except-for-images"
-        },
-        {
-          "sourcePath": "out-images1"
-        },
-        {
-          "sourcePath": "out-images2"
-        },
-        {
-          "sourcePath": "out-images3"
-        },
-        {
-          "sourcePath": "out-images4"
-        },
-        {
-          "sourcePath": "out-images5"
+          "includeGlobs": [
+            "out-all",
+            "out-all-linked",
+            "out-all-flattened",
+            "out-all-except-for-images",
+            "out-images1",
+            "out-images2",
+            "out-images3",
+            "out-images4",
+            "out-images5"
+          ]
         }
       ],
 

diff --git a/build-tests/heft-example-plugin-01/config/heft.json b/build-tests/heft-example-plugin-01/config/heft.json
@@ -4,7 +4,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist" }, { "sourcePath": "lib" }],
+      "cleanFiles": [{ "includeGlobs": ["dist", "lib"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {

diff --git a/build-tests/heft-example-plugin-02/config/heft.json b/build-tests/heft-example-plugin-02/config/heft.json
@@ -4,7 +4,8 @@
   // TODO: Add comments
   "phasesByName": {
     "build": {
-      "cleanFiles": [{ "sourcePath": "dist" }, { "sourcePath": "lib" }],
+      "cleanFiles": [{ "includeGlobs": ["dist", "lib"] }],
+
       "tasksByName": {
         "typescript": {
           "taskPlugin": {