diff --git a/docs/output/cleaning.md b/docs/output/cleaning.md index 2c0130c4e..5f35aef94 100644 --- a/docs/output/cleaning.md +++ b/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 ` directory that are either: @@ -45,6 +45,12 @@ The `--clean-exclude ` 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 ` 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. diff --git a/src/modules/argumentsParser.ts b/src/modules/argumentsParser.ts index 6fe1d3e27..a278f6108 100644 --- a/src/modules/argumentsParser.ts +++ b/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`); diff --git a/src/modules/directoryCleaner.ts b/src/modules/directoryCleaner.ts index 9a9577164..190cd5b22 100644 --- a/src/modules/directoryCleaner.ts +++ b/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 { - 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 { + 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 { diff --git a/src/types/options.ts b/src/types/options.ts index 8efa6c138..5b62202f8 100644 --- a/src/types/options.ts +++ b/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; } diff --git a/test/modules/argumentsParser.test.ts b/test/modules/argumentsParser.test.ts index 37670f2b3..da09f911e 100644 --- a/test/modules/argumentsParser.test.ts +++ b/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); diff --git a/test/modules/directoryCleaner.test.ts b/test/modules/directoryCleaner.test.ts index dd19f45de..dbd10c5d0 100644 --- a/test/modules/directoryCleaner.test.ts +++ b/test/modules/directoryCleaner.test.ts @@ -9,6 +9,12 @@ import ProgressBarFake from '../console/progressBarFake.js'; const ROM_FIXTURES_DIR = path.join('test', 'fixtures', 'roms'); +interface CleanResult { + cleanedPaths: string[], + remainingPaths: string[], + movedPaths: string[], +} + /** * NOTE(cemmer): for some entirely unexplainable reason, these tests will start to fail on Windows * if there are more than three of them running. See the absolute mad head-banging commit log in @@ -19,58 +25,71 @@ async function runOutputCleaner( optionsProps: OptionsProps, cleanExclude: string[], writtenFilePathsToExclude: string[], -): Promise { +): Promise { // Copy the fixture files to a temp directory - const tempDir = await fsPoly.mkdtemp(Temp.getTempDir()); - await fsPoly.copyDir(ROM_FIXTURES_DIR, tempDir); - - const writtenRomFilesToExclude = await Promise.all(writtenFilePathsToExclude - .map(async (filePath) => File.fileOf({ filePath: path.join(tempDir, filePath) }))); - - const before = await fsPoly.walk(tempDir); - expect(before.length).toBeGreaterThan(0); + const tempInputDir = await fsPoly.mkdtemp(Temp.getTempDir()); + await fsPoly.copyDir(ROM_FIXTURES_DIR, tempInputDir); - await new DirectoryCleaner( - new Options({ - ...optionsProps, - commands: ['move', 'clean'], - cleanExclude: cleanExclude.map((filePath) => path.join(tempDir, filePath)), - }), - new ProgressBarFake(), - ).clean([tempDir], writtenRomFilesToExclude); - const after = await fsPoly.walk(tempDir); + try { + const writtenRomFilesToExclude = await Promise.all(writtenFilePathsToExclude + .map(async (filePath) => File.fileOf({ filePath: path.join(tempInputDir, filePath) }))); - // Test cleanup - await fsPoly.rm(tempDir, { recursive: true, force: true }); + const before = await fsPoly.walk(tempInputDir); + expect(before.length).toBeGreaterThan(0); - return after - .map((pathLike) => pathLike.replace(tempDir + path.sep, '')) - .sort(); + const cleanedPaths = (await new DirectoryCleaner( + new Options({ + ...optionsProps, + commands: ['move', 'clean'], + cleanExclude: cleanExclude.map((filePath) => path.join(tempInputDir, filePath)), + }), + new ProgressBarFake(), + ).clean([tempInputDir], writtenRomFilesToExclude)) + .map((pathLike) => pathLike.replace(tempInputDir + path.sep, '')) + .sort(); + + const after = await fsPoly.walk(tempInputDir); + const remainingPaths = after + .map((pathLike) => pathLike.replace(tempInputDir + path.sep, '')) + .sort(); + + const movedPaths = optionsProps.cleanBackup !== undefined + ? await fsPoly.walk(optionsProps.cleanBackup) + : []; + + return { + cleanedPaths, + remainingPaths, + movedPaths, + }; + } finally { + await fsPoly.rm(tempInputDir, { recursive: true, force: true }); + } } it('should delete nothing if nothing written', async () => { const existingFiles = (await fsPoly.walk(ROM_FIXTURES_DIR)) .map((filePath) => filePath.replace(/^test[\\/]fixtures[\\/]roms[\\/]/, '')) .sort(); - const filesRemaining = await runOutputCleaner({}, [], []); - expect(filesRemaining).toEqual(existingFiles); + const { remainingPaths } = await runOutputCleaner({}, [], []); + expect(remainingPaths).toEqual(existingFiles); }); it('should delete nothing if no excess files', async () => { const existingFiles = (await fsPoly.walk(ROM_FIXTURES_DIR)) .map((filePath) => filePath.replace(/^test[\\/]fixtures[\\/]roms[\\/]/, '')) .sort(); - const filesRemaining = await runOutputCleaner({}, [], existingFiles); - expect(filesRemaining).toEqual(existingFiles); + const { remainingPaths } = await runOutputCleaner({}, [], existingFiles); + expect(remainingPaths).toEqual(existingFiles); }); it('should delete some if all unmatched and some excluded', async () => { - const filesRemaining = await runOutputCleaner({}, [ + const { remainingPaths } = await runOutputCleaner({}, [ path.join('**', 'foobar.*'), ], [ 'non-existent file', ]); - expect(filesRemaining).toEqual([ + expect(remainingPaths).toEqual([ path.join('7z', 'foobar.7z'), 'foobar.lnx', path.join('rar', 'foobar.rar'), @@ -81,13 +100,13 @@ it('should delete some if all unmatched and some excluded', async () => { }); it('should delete some if some matched and nothing excluded', async () => { - const filesRemaining = await runOutputCleaner({}, [], [ + const { remainingPaths } = await runOutputCleaner({}, [], [ path.join('7z', 'empty.7z'), path.join('raw', 'fizzbuzz.nes'), path.join('zip', 'foobar.zip'), 'non-existent file', ]); - expect(filesRemaining).toEqual([ + expect(remainingPaths).toEqual([ path.join('7z', 'empty.7z'), path.join('raw', 'fizzbuzz.nes'), path.join('zip', 'foobar.zip'), @@ -95,21 +114,40 @@ it('should delete some if some matched and nothing excluded', async () => { }); it('should delete everything if all unmatched and nothing excluded', async () => { - await expect(runOutputCleaner({}, [], [ + const { cleanedPaths, remainingPaths, movedPaths } = await runOutputCleaner({}, [], [ 'non-existent file', - ])).resolves.toHaveLength(0); + ]); + expect(cleanedPaths.length).toBeGreaterThan(0); + expect(remainingPaths).toHaveLength(0); + expect(movedPaths).toHaveLength(0); +}); + +it('should move everything if all unmatched and nothing excluded', async () => { + const tempMoveDir = await fsPoly.mkdtemp(Temp.getTempDir()); + try { + const { cleanedPaths, remainingPaths, movedPaths } = await runOutputCleaner({ + cleanBackup: tempMoveDir, + }, [], [ + 'non-existent file', + ]); + expect(cleanedPaths.length).toBeGreaterThan(0); + expect(remainingPaths).toHaveLength(0); + expect(movedPaths).toHaveLength(cleanedPaths.length); + } finally { + await fsPoly.rm(tempMoveDir, { recursive: true, force: true }); + } }); it('should delete nothing if all unmatched but doing a dry run', async () => { const existingFiles = (await fsPoly.walk(ROM_FIXTURES_DIR)) .map((filePath) => filePath.replace(/^test[\\/]fixtures[\\/]roms[\\/]/, '')) .sort(); - const filesRemaining = await runOutputCleaner({ + const { remainingPaths } = await runOutputCleaner({ cleanDryRun: true, }, [], [ 'non-existent file', ]); - expect(filesRemaining).toEqual(existingFiles); + expect(remainingPaths).toEqual(existingFiles); }); it('should delete hard links', async () => { @@ -137,8 +175,8 @@ it('should delete hard links', async () => { new ProgressBarFake(), ).clean([linksDir], [await File.fileOf({ filePath: tempLinkOne })]); - const filesRemaining = await fsPoly.walk(tempDir); - expect(filesRemaining).toEqual([ + const remainingPaths = await fsPoly.walk(tempDir); + expect(remainingPaths).toEqual([ // Original files were preserved tempFileOne, tempFileTwo, @@ -175,8 +213,8 @@ it('should delete symlinks', async () => { new ProgressBarFake(), ).clean([linksDir], [await File.fileOf({ filePath: tempLinkOne })]); - const filesRemaining = await fsPoly.walk(tempDir); - expect(filesRemaining).toEqual([ + const remainingPaths = await fsPoly.walk(tempDir); + expect(remainingPaths).toEqual([ // Original files were preserved tempFileOne, tempFileTwo,