Feature: cleaning backup directory option (#1221)

docs/output/cleaning.md

@@ -1,4 +1,4 @@
-# Output Cleaning
+# Output Directory Cleaning
 
 The `igir clean` [command](../commands.md) can be used when writing (`igir copy`, `igir move`, and `igir link`) to delete files from the `--output <path>` directory that are either:
 
@@ -45,6 +45,12 @@ The `--clean-exclude <path>` option exists so that one or more paths (with suppo
 
 See the [Analogue Pocket](../usage/hardware/analogue-pocket.md) page for a practical example.
 
+## Backing up cleaned files
+
+By default, `igir` will recycle cleaned files, and if recycle fails then it will delete them. This is potentially destructive, so a `--clean-backup <path>` option is provided to instead move files to a backup directory.
+
+The input directory structure is not maintained, no subdirectories will be created in the backup directory. Files of conflicting names will have a number appended to their name, e.g. `File (1).rom`.
+
 ## Dry run
 
 The `--clean-dry-run` option exists to see what paths `igir clean` would delete, without _actually_ deleting them.

src/modules/argumentsParser.ts

@@ -456,6 +456,13 @@ export default class ArgumentsParser {
         type: 'array',
         requiresArg: true,
       })
+      .option('clean-backup', {
+        group: groupRomClean,
+        description: 'Move cleaned files to a directory for backup',
+        type: 'string',
+        coerce: ArgumentsParser.getLastValue, // don't allow string[] values
+        requiresArg: true,
+      })
       .option('clean-dry-run', {
         group: groupRomClean,
         description: 'Don\'t clean any files and instead only print what files would be cleaned',
@@ -465,7 +472,7 @@ export default class ArgumentsParser {
         if (checkArgv.help) {
           return true;
         }
-        const needClean = ['clean-exclude', 'clean-dry-run'].filter((option) => checkArgv[option]);
+        const needClean = ['clean-exclude', 'clean-backup', 'clean-dry-run'].filter((option) => checkArgv[option]);
         if (!checkArgv._.includes('clean') && needClean.length > 0) {
           // TODO(cememr): print help message
           throw new ExpectedError(`Missing required command for option${needClean.length !== 1 ? 's' : ''} ${needClean.join(', ')}: clean`);

src/modules/directoryCleaner.ts

@@ -1,6 +1,7 @@
 import fs from 'node:fs';
 import path from 'node:path';
 
+import { Semaphore } from 'async-mutex';
 import { isNotJunk } from 'junk';
 import trash from 'trash';
 
@@ -55,8 +56,16 @@ export default class DirectoryCleaner extends Module {
     try {
       this.progressBar.logTrace(`cleaning ${filesToClean.length.toLocaleString()} file${filesToClean.length !== 1 ? 's' : ''}`);
       await this.progressBar.reset(filesToClean.length);
-      // TODO(cemmer): don't trash save files
-      await this.trashOrDelete(filesToClean);
+      if (this.options.getCleanDryRun()) {
+        this.progressBar.logInfo(`paths skipped from cleaning (dry run):\n${filesToClean.map((filePath) => `  ${filePath}`).join('\n')}`);
+      } else {
+        const cleanBackupDir = this.options.getCleanBackup();
+        if (cleanBackupDir !== undefined) {
+          await this.backupFiles(cleanBackupDir, filesToClean);
+        } else {
+          await this.trashOrDelete(filesToClean);
+        }
+      }
     } catch (error) {
       this.progressBar.logError(`failed to clean unmatched files: ${error}`);
       return [];
@@ -67,7 +76,11 @@ export default class DirectoryCleaner extends Module {
       while (emptyDirs.length > 0) {
         await this.progressBar.reset(emptyDirs.length);
         this.progressBar.logTrace(`cleaning ${emptyDirs.length.toLocaleString()} empty director${emptyDirs.length !== 1 ? 'ies' : 'y'}`);
-        await this.trashOrDelete(emptyDirs);
+        if (this.options.getCleanDryRun()) {
+          this.progressBar.logInfo(`paths skipped from cleaning (dry run):\n${emptyDirs.map((filePath) => `  ${filePath}`).join('\n')}`);
+        } else {
+          await this.trashOrDelete(emptyDirs);
+        }
         // Deleting some empty directories could leave others newly empty
         emptyDirs = await DirectoryCleaner.getEmptyDirs(dirsToClean);
       }
@@ -80,15 +93,10 @@ export default class DirectoryCleaner extends Module {
   }
 
   private async trashOrDelete(filePaths: string[]): Promise<void> {
-    if (this.options.getCleanDryRun()) {
-      this.progressBar.logInfo(`paths skipped from cleaning (dry run):\n${filePaths.map((filePath) => `  ${filePath}`).join('\n')}`);
-      return;
-    }
-
     // Prefer recycling...
     for (let i = 0; i < filePaths.length; i += Defaults.OUTPUT_CLEANER_BATCH_SIZE) {
       const filePathsChunk = filePaths.slice(i, i + Defaults.OUTPUT_CLEANER_BATCH_SIZE);
-      this.progressBar.logInfo(`cleaning path${filePathsChunk.length !== 1 ? 's' : ''}:\n${filePathsChunk.map((filePath) => `  ${filePath}`).join('\n')}`);
+      this.progressBar.logInfo(`recycling cleaned path${filePathsChunk.length !== 1 ? 's' : ''}:\n${filePathsChunk.map((filePath) => `  ${filePath}`).join('\n')}`);
       try {
         await trash(filePathsChunk);
       } catch (error) {
@@ -98,20 +106,47 @@ export default class DirectoryCleaner extends Module {
     }
 
     // ...but if that doesn't work, delete the leftovers
-    const filePathsExist = await Promise.all(
-      filePaths.map(async (filePath) => fsPoly.exists(filePath)),
-    );
-    await Promise.all(
-      filePaths
-        .filter((filePath, idx) => filePathsExist.at(idx))
-        .map(async (filePath) => {
-          try {
-            await fsPoly.rm(filePath, { force: true });
-          } catch (error) {
-            this.progressBar.logError(`failed to delete ${filePath}: ${error}`);
-          }
-        }),
-    );
+    const existSemaphore = new Semaphore(Defaults.OUTPUT_CLEANER_BATCH_SIZE);
+    const existingFilePathsCheck = await Promise.all(filePaths
+      .map(async (filePath) => existSemaphore.runExclusive(async () => fsPoly.exists(filePath))));
+    const existingFilePaths = filePaths.filter((filePath, idx) => existingFilePathsCheck.at(idx));
+    for (let i = 0; i < existingFilePaths.length; i += Defaults.OUTPUT_CLEANER_BATCH_SIZE) {
+      const filePathsChunk = existingFilePaths.slice(i, i + Defaults.OUTPUT_CLEANER_BATCH_SIZE);
+      this.progressBar.logInfo(`deleting cleaned path${filePathsChunk.length !== 1 ? 's' : ''}:\n${filePathsChunk.map((filePath) => `  ${filePath}`).join('\n')}`);
+      try {
+        await Promise.all(filePathsChunk
+          .map(async (filePath) => fsPoly.rm(filePath, { force: true })));
+      } catch (error) {
+        this.progressBar.logWarn(`failed to delete ${filePathsChunk.length} path${filePathsChunk.length !== 1 ? 's' : ''}: ${error}`);
+      }
+    }
+  }
+
+  private async backupFiles(backupDir: string, filePaths: string[]): Promise<void> {
+    const semaphore = new Semaphore(this.options.getWriterThreads());
+    await Promise.all(filePaths.map(async (filePath) => {
+      await semaphore.runExclusive(async () => {
+        let backupPath = path.join(backupDir, path.basename(filePath));
+        let increment = 0;
+        while (await fsPoly.exists(backupPath)) {
+          increment += 1;
+          const { name, ext } = path.parse(filePath);
+          backupPath = path.join(backupDir, `${name} (${increment})${ext}`);
+        }
+
+        this.progressBar.logInfo(`moving cleaned path: ${filePath} -> ${backupPath}`);
+        const backupPathDir = path.dirname(backupPath);
+        if (!await fsPoly.exists(backupPathDir)) {
+          await fsPoly.mkdir(backupPathDir, { recursive: true });
+        }
+        try {
+          await fsPoly.mv(filePath, backupPath);
+        } catch (error) {
+          this.progressBar.logWarn(`failed to move ${filePath} -> ${backupPath}: ${error}`);
+        }
+        await this.progressBar.incrementProgress();
+      });
+    }));
   }
 
   private static async getEmptyDirs(dirsToClean: string | string[]): Promise<string[]> {

src/types/options.ts

@@ -95,6 +95,7 @@ export interface OptionsProps {
   readonly overwriteInvalid?: boolean,
 
   readonly cleanExclude?: string[],
+  readonly cleanBackup?: string,
   readonly cleanDryRun?: boolean,
 
   readonly zipExclude?: string,
@@ -237,6 +238,8 @@ export default class Options implements OptionsProps {
 
   readonly cleanExclude: string[];
 
+  readonly cleanBackup?: string;
+
   readonly cleanDryRun: boolean;
 
   readonly zipExclude: string;
@@ -407,6 +410,7 @@ export default class Options implements OptionsProps {
     this.overwrite = options?.overwrite ?? false;
     this.overwriteInvalid = options?.overwriteInvalid ?? false;
     this.cleanExclude = options?.cleanExclude ?? [];
+    this.cleanBackup = options?.cleanBackup;
     this.cleanDryRun = options?.cleanDryRun ?? false;
 
     this.zipExclude = options?.zipExclude ?? '';
@@ -954,6 +958,10 @@ export default class Options implements OptionsProps {
       .sort();
   }
 
+  getCleanBackup(): string | undefined {
+    return this.cleanBackup;
+  }
+
   getCleanDryRun(): boolean {
     return this.cleanDryRun;
   }

test/modules/argumentsParser.test.ts

@@ -145,6 +145,7 @@ describe('options', () => {
     expect(options.getOverwriteInvalid()).toEqual(false);
     expect(options.getRomFixExtension()).toEqual(RomFixExtension.AUTO);
 
+    expect(options.getCleanBackup()).toBeUndefined();
     expect(options.getCleanDryRun()).toEqual(false);
 
     expect(options.getZipDatName()).toEqual(false);
@@ -577,6 +578,12 @@ describe('options', () => {
     expect((await argumentsParser.parse([...argv, 'clean', '--clean-exclude', outputDir]).scanOutputFilesWithoutCleanExclusions([outputDir], [])).length).toEqual(0);
   });
 
+  it('should parse "clean-backup"', () => {
+    expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--clean-backup', 'foo'])).toThrow(/missing required command/i);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, 'clean', '--clean-backup', 'foo']).getCleanBackup()).toEqual('foo');
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, 'clean', '--clean-backup', 'foo', '--clean-backup', 'bar']).getCleanBackup()).toEqual('bar');
+  });
+
   it('should parse "clean-dry-run"', () => {
     expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--clean-dry-run'])).toThrow(/missing required command/i);
     expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, 'clean', '--clean-dry-run']).getCleanDryRun()).toEqual(true);