From 4d10c16310269cf4d3cec120a7d44f2ea1dfd3ca Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 25 May 2023 17:14:37 +0200 Subject: [PATCH] files - :lipstick: atomic support --- src/vs/base/node/pfs.ts | 16 ++++++++-------- src/vs/base/test/node/pfs/pfs.test.ts | 2 +- src/vs/platform/files/common/fileService.ts | 4 ++++ src/vs/platform/files/common/files.ts | 13 +++++++++---- .../test/node/diskFileService.integrationTest.ts | 2 +- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/vs/base/node/pfs.ts b/src/vs/base/node/pfs.ts index d49fd3f10c6..0cdb1809b52 100644 --- a/src/vs/base/node/pfs.ts +++ b/src/vs/base/node/pfs.ts @@ -37,13 +37,13 @@ export enum RimRafMode { * - `UNLINK`: direct removal from disk * - `MOVE`: faster variant that first moves the target to temp dir and then * deletes it in the background without waiting for that to finish. - * the optional `target` allows to override where to rename the + * the optional `moveToPath` allows to override where to rename the * path to before deleting it. */ async function rimraf(path: string, mode: RimRafMode.UNLINK): Promise; -async function rimraf(path: string, mode: RimRafMode.MOVE, target?: string): Promise; -async function rimraf(path: string, mode?: RimRafMode, target?: string): Promise; -async function rimraf(path: string, mode = RimRafMode.UNLINK, target?: string): Promise { +async function rimraf(path: string, mode: RimRafMode.MOVE, moveToPath?: string): Promise; +async function rimraf(path: string, mode?: RimRafMode, moveToPath?: string): Promise; +async function rimraf(path: string, mode = RimRafMode.UNLINK, moveToPath?: string): Promise { if (isRootOrDriveLetter(path)) { throw new Error('rimraf - will refuse to recursively delete root'); } @@ -54,10 +54,10 @@ async function rimraf(path: string, mode = RimRafMode.UNLINK, target?: string): } // delete: via move - return rimrafMove(path, target); + return rimrafMove(path, moveToPath); } -async function rimrafMove(path: string, target = randomPath(tmpdir())): Promise { +async function rimrafMove(path: string, moveToPath = randomPath(tmpdir())): Promise { try { try { // Intentionally using `fs.promises` here to skip @@ -68,7 +68,7 @@ async function rimrafMove(path: string, target = randomPath(tmpdir())): Promise< // than necessary and we have a fallback to delete // via unlink. // https://github.com/microsoft/vscode/issues/139908 - await fs.promises.rename(path, target); + await fs.promises.rename(path, moveToPath); } catch (error) { if (error.code === 'ENOENT') { return; // ignore - path to delete did not exist @@ -78,7 +78,7 @@ async function rimrafMove(path: string, target = randomPath(tmpdir())): Promise< } // Delete but do not return as promise - rimrafUnlink(target).catch(error => {/* ignore */ }); + rimrafUnlink(moveToPath).catch(error => {/* ignore */ }); } catch (error) { if (error.code !== 'ENOENT') { throw error; diff --git a/src/vs/base/test/node/pfs/pfs.test.ts b/src/vs/base/test/node/pfs/pfs.test.ts index 3a92872b81e..cefdc23af5b 100644 --- a/src/vs/base/test/node/pfs/pfs.test.ts +++ b/src/vs/base/test/node/pfs/pfs.test.ts @@ -91,7 +91,7 @@ flakySuite('PFS', function () { assert.ok(!fs.existsSync(testDir)); }); - test('rimraf - simple - move (with target)', async () => { + test('rimraf - simple - move (with moveToPath)', async () => { fs.writeFileSync(join(testDir, 'somefile.txt'), 'Contents'); fs.writeFileSync(join(testDir, 'someOtherFile.txt'), 'Contents'); diff --git a/src/vs/platform/files/common/fileService.ts b/src/vs/platform/files/common/fileService.ts index a2ac9629762..067d076db82 100644 --- a/src/vs/platform/files/common/fileService.ts +++ b/src/vs/platform/files/common/fileService.ts @@ -987,6 +987,10 @@ export class FileService extends Disposable implements IFileService { throw new Error(localize('deleteFailedAtomicUnsupported', "Unable to delete file '{0}' atomically because provider does not support it.", this.resourceForError(resource))); } + if (useTrash && atomic) { + throw new Error(localize('deleteFailedTrashAndAtomicUnsupported', "Unable to delete file '{0}' with option to use trash and atomic both enabled.", this.resourceForError(resource))); + } + // Validate delete let stat: IStat | undefined = undefined; try { diff --git a/src/vs/platform/files/common/files.ts b/src/vs/platform/files/common/files.ts index 794a06e3bb2..1f051fdf5e0 100644 --- a/src/vs/platform/files/common/files.ts +++ b/src/vs/platform/files/common/files.ts @@ -317,7 +317,8 @@ export interface IFileAtomicDeleteOptions { /** * The optional `atomic` flag can be used to make sure * the `delete` method deletes the target atomically by - * first renaming it to a temporary resource and then deleting it. + * first renaming it to a temporary resource in the same + * folder and then deleting it. */ readonly atomic: IFileAtomicOptions | false; } @@ -394,15 +395,19 @@ export interface IFileDeleteOptions { /** * Set to `true` to attempt to move the file to trash - * instead of deleting it permanently from disk. This - * option maybe not be supported on all providers. + * instead of deleting it permanently from disk. + * + * This option maybe not be supported on all providers. */ readonly useTrash: boolean; /** * The optional `atomic` flag can be used to make sure * the `delete` method deletes the target atomically by - * first renaming it to a temporary resource and then deleting it. + * first renaming it to a temporary resource in the same + * folder and then deleting it. + * + * This option maybe not be supported on all providers. */ readonly atomic: IFileAtomicOptions | false; } diff --git a/src/vs/platform/files/test/node/diskFileService.integrationTest.ts b/src/vs/platform/files/test/node/diskFileService.integrationTest.ts index 0d9d44ad973..fdf58c22472 100644 --- a/src/vs/platform/files/test/node/diskFileService.integrationTest.ts +++ b/src/vs/platform/files/test/node/diskFileService.integrationTest.ts @@ -589,7 +589,7 @@ flakySuite('Disk File Service', function () { const resource = URI.file(join(testDir, 'deep')); const source = await service.resolve(resource); - assert.strictEqual(await service.canDelete(source.resource, { recursive: true, useTrash }), true); + assert.strictEqual(await service.canDelete(source.resource, { recursive: true, useTrash, atomic }), true); await service.del(source.resource, { recursive: true, useTrash, atomic }); assert.strictEqual(existsSync(source.resource.fsPath), false);