diff --git a/src/vs/base/node/pfs.ts b/src/vs/base/node/pfs.ts index 28e6ca7e88743..0cdb1809b524f 100644 --- a/src/vs/base/node/pfs.ts +++ b/src/vs/base/node/pfs.ts @@ -37,8 +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 `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.UNLINK): 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'); } @@ -49,12 +54,11 @@ async function rimraf(path: string, mode = RimRafMode.UNLINK): Promise { } // delete: via move - return rimrafMove(path); + return rimrafMove(path, moveToPath); } -async function rimrafMove(path: string): Promise { +async function rimrafMove(path: string, moveToPath = randomPath(tmpdir())): Promise { try { - const pathInTemp = randomPath(tmpdir()); try { // Intentionally using `fs.promises` here to skip // the patched graceful-fs method that can result @@ -64,7 +68,7 @@ async function rimrafMove(path: string): Promise { // than necessary and we have a fallback to delete // via unlink. // https://github.com/microsoft/vscode/issues/139908 - await fs.promises.rename(path, pathInTemp); + await fs.promises.rename(path, moveToPath); } catch (error) { if (error.code === 'ENOENT') { return; // ignore - path to delete did not exist @@ -74,7 +78,7 @@ async function rimrafMove(path: string): Promise { } // Delete but do not return as promise - rimrafUnlink(pathInTemp).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 8114adb657134..cefdc23af5b2e 100644 --- a/src/vs/base/test/node/pfs/pfs.test.ts +++ b/src/vs/base/test/node/pfs/pfs.test.ts @@ -10,7 +10,7 @@ import { timeout } from 'vs/base/common/async'; import { VSBuffer } from 'vs/base/common/buffer'; import { randomPath } from 'vs/base/common/extpath'; import { FileAccess } from 'vs/base/common/network'; -import { join, sep } from 'vs/base/common/path'; +import { basename, dirname, join, sep } from 'vs/base/common/path'; import { isWindows } from 'vs/base/common/platform'; import { configureFlushOnWrite, Promises, RimRafMode, rimrafSync, SymlinkSupport, writeFileSync } from 'vs/base/node/pfs'; import { flakySuite, getRandomTestPath } from 'vs/base/test/node/testUtils'; @@ -91,6 +91,14 @@ flakySuite('PFS', function () { assert.ok(!fs.existsSync(testDir)); }); + test('rimraf - simple - move (with moveToPath)', async () => { + fs.writeFileSync(join(testDir, 'somefile.txt'), 'Contents'); + fs.writeFileSync(join(testDir, 'someOtherFile.txt'), 'Contents'); + + await Promises.rm(testDir, RimRafMode.MOVE, join(dirname(testDir), `${basename(testDir)}.vsctmp`)); + assert.ok(!fs.existsSync(testDir)); + }); + test('rimraf - path does not exist - move', async () => { const nonExistingDir = join(testDir, 'unknown-move'); await Promises.rm(nonExistingDir, RimRafMode.MOVE); diff --git a/src/vs/platform/files/browser/htmlFileSystemProvider.ts b/src/vs/platform/files/browser/htmlFileSystemProvider.ts index ee51cb1ac23c4..382bb7a7e6ca4 100644 --- a/src/vs/platform/files/browser/htmlFileSystemProvider.ts +++ b/src/vs/platform/files/browser/htmlFileSystemProvider.ts @@ -269,8 +269,8 @@ export class HTMLFileSystemProvider implements IFileSystemProviderWithFileReadWr const file = await fileHandle.getFile(); const contents = new Uint8Array(await file.arrayBuffer()); - await this.writeFile(to, contents, { create: true, overwrite: opts.overwrite, unlock: false }); - await this.delete(from, { recursive: false, useTrash: false }); + await this.writeFile(to, contents, { create: true, overwrite: opts.overwrite, unlock: false, atomic: false }); + await this.delete(from, { recursive: false, useTrash: false, atomic: false }); } // File API does not support any real rename otherwise diff --git a/src/vs/platform/files/browser/indexedDBFileSystemProvider.ts b/src/vs/platform/files/browser/indexedDBFileSystemProvider.ts index 9cd73f93be9e8..a83c046c715e9 100644 --- a/src/vs/platform/files/browser/indexedDBFileSystemProvider.ts +++ b/src/vs/platform/files/browser/indexedDBFileSystemProvider.ts @@ -311,7 +311,7 @@ export class IndexedDBFileSystemProvider extends Disposable implements IFileSyst throw createFileSystemProviderError('Cannot rename files with different types', FileSystemProviderErrorCode.Unknown); } // delete the target file if exists - await this.delete(to, { recursive: true, useTrash: false }); + await this.delete(to, { recursive: true, useTrash: false, atomic: false }); } const toTargetResource = (path: string): URI => this.extUri.joinPath(to, this.extUri.relativePath(from, from.with({ path })) || ''); @@ -339,7 +339,7 @@ export class IndexedDBFileSystemProvider extends Disposable implements IFileSyst await this.bulkWrite(targetFiles); } - await this.delete(from, { recursive: true, useTrash: false }); + await this.delete(from, { recursive: true, useTrash: false, atomic: false }); } async delete(resource: URI, opts: IFileDeleteOptions): Promise { diff --git a/src/vs/platform/files/common/diskFileSystemProviderClient.ts b/src/vs/platform/files/common/diskFileSystemProviderClient.ts index 4eeed662440b3..f277238bbd35d 100644 --- a/src/vs/platform/files/common/diskFileSystemProviderClient.ts +++ b/src/vs/platform/files/common/diskFileSystemProviderClient.ts @@ -53,6 +53,8 @@ export class DiskFileSystemProviderClient extends Disposable implements FileSystemProviderCapabilities.FileFolderCopy | FileSystemProviderCapabilities.FileWriteUnlock | FileSystemProviderCapabilities.FileAtomicRead | + FileSystemProviderCapabilities.FileAtomicWrite | + FileSystemProviderCapabilities.FileAtomicDelete | FileSystemProviderCapabilities.FileClone; if (this.extraCapabilities.pathCaseSensitive) { diff --git a/src/vs/platform/files/common/fileService.ts b/src/vs/platform/files/common/fileService.ts index cae3edd0d6016..07da158ea7ede 100644 --- a/src/vs/platform/files/common/fileService.ts +++ b/src/vs/platform/files/common/fileService.ts @@ -14,7 +14,7 @@ import { Disposable, DisposableStore, dispose, IDisposable, toDisposable } from import { TernarySearchTree } from 'vs/base/common/ternarySearchTree'; import { Schemas } from 'vs/base/common/network'; import { mark } from 'vs/base/common/performance'; -import { extUri, extUriIgnorePathCase, IExtUri, isAbsolutePath } from 'vs/base/common/resources'; +import { basename, dirname, extUri, extUriIgnorePathCase, IExtUri, isAbsolutePath, joinPath } from 'vs/base/common/resources'; import { consumeStream, isReadableBufferedStream, isReadableStream, listenStream, newWriteableStream, peekReadable, peekStream, transform } from 'vs/base/common/stream'; import { URI } from 'vs/base/common/uri'; import { localize } from 'vs/nls'; @@ -396,7 +396,17 @@ export class FileService extends Disposable implements IFileService { // write file: buffered else { - await this.doWriteBuffered(provider, resource, options, bufferOrReadableOrStreamOrBufferedStream instanceof VSBuffer ? bufferToReadable(bufferOrReadableOrStreamOrBufferedStream) : bufferOrReadableOrStreamOrBufferedStream); + const contents = bufferOrReadableOrStreamOrBufferedStream instanceof VSBuffer ? bufferToReadable(bufferOrReadableOrStreamOrBufferedStream) : bufferOrReadableOrStreamOrBufferedStream; + + // atomic write + if (options?.atomic !== false && options?.atomic?.postfix) { + await this.doWriteBufferedAtomic(provider, resource, joinPath(dirname(resource), `${basename(resource)}${options.atomic.postfix}`), options, contents); + } + + // non-atomic write + else { + await this.doWriteBuffered(provider, resource, options, contents); + } } // events @@ -416,6 +426,18 @@ export class FileService extends Disposable implements IFileService { throw new Error(localize('writeFailedUnlockUnsupported', "Unable to unlock file '{0}' because provider does not support it.", this.resourceForError(resource))); } + // Validate atomic support + const atomic = !!options?.atomic; + if (atomic) { + if (!(provider.capabilities & FileSystemProviderCapabilities.FileAtomicWrite)) { + throw new Error(localize('writeFailedAtomicUnsupported', "Unable to atomically write file '{0}' because provider does not support it.", this.resourceForError(resource))); + } + + if (unlock) { + throw new Error(localize('writeFailedAtomicUnlock', "Unable to unlock file '{0}' because atomic write is enabled.", this.resourceForError(resource))); + } + } + // Validate via file stat meta data let stat: IStat | undefined = undefined; try { @@ -579,7 +601,7 @@ export class FileService extends Disposable implements IFileService { } if (error instanceof TooLargeFileOperationError) { - return new TooLargeFileOperationError(message, error.fileOperationResult, error.size, error.options); + return new TooLargeFileOperationError(message, error.fileOperationResult, error.size, error.options as IReadFileOptions); } return new FileOperationError(message, toFileOperationResult(error), options); @@ -959,6 +981,16 @@ export class FileService extends Disposable implements IFileService { throw new Error(localize('deleteFailedTrashUnsupported', "Unable to delete file '{0}' via trash because provider does not support it.", this.resourceForError(resource))); } + // Validate atomic support + const atomic = options?.atomic; + if (atomic && !(provider.capabilities & FileSystemProviderCapabilities.FileAtomicDelete)) { + 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 atomically delete file '{0}' because using trash is enabled.", this.resourceForError(resource))); + } + // Validate delete let stat: IStat | undefined = undefined; try { @@ -990,9 +1022,10 @@ export class FileService extends Disposable implements IFileService { const useTrash = !!options?.useTrash; const recursive = !!options?.recursive; + const atomic = options?.atomic ?? false; // Delete through provider - await provider.delete(resource, { recursive, useTrash }); + await provider.delete(resource, { recursive, useTrash, atomic }); // Events this._onDidRunOperation.fire(new FileOperationEvent(resource, FileOperation.DELETE)); @@ -1122,6 +1155,28 @@ export class FileService extends Disposable implements IFileService { private readonly writeQueue = this._register(new ResourceQueue()); + private async doWriteBufferedAtomic(provider: IFileSystemProviderWithOpenReadWriteCloseCapability, resource: URI, tempResource: URI, options: IWriteFileOptions | undefined, readableOrStreamOrBufferedStream: VSBufferReadable | VSBufferReadableStream | VSBufferReadableBufferedStream): Promise { + + // Write to temp resource first + await this.doWriteBuffered(provider, tempResource, options, readableOrStreamOrBufferedStream); + + try { + + // Rename over existing to ensure atomic replace + await provider.rename(tempResource, resource, { overwrite: true }); + } catch (error) { + + // Cleanup in case of rename error + try { + await provider.delete(tempResource, { recursive: false, useTrash: false, atomic: false }); + } catch (error) { + // ignore - we want the outer error to bubble up + } + + throw error; + } + } + private async doWriteBuffered(provider: IFileSystemProviderWithOpenReadWriteCloseCapability, resource: URI, options: IWriteFileOptions | undefined, readableOrStreamOrBufferedStream: VSBufferReadable | VSBufferReadableStream | VSBufferReadableBufferedStream): Promise { return this.writeQueue.queueFor(resource, this.getExtUri(provider).providerExtUri).queue(async () => { @@ -1237,7 +1292,7 @@ export class FileService extends Disposable implements IFileService { } // Write through the provider - await provider.writeFile(resource, buffer.buffer, { create: true, overwrite: true, unlock: options?.unlock ?? false }); + await provider.writeFile(resource, buffer.buffer, { create: true, overwrite: true, unlock: options?.unlock ?? false, atomic: options?.atomic ?? false }); } private async doPipeBuffered(sourceProvider: IFileSystemProviderWithOpenReadWriteCloseCapability, source: URI, targetProvider: IFileSystemProviderWithOpenReadWriteCloseCapability, target: URI): Promise { @@ -1291,7 +1346,7 @@ export class FileService extends Disposable implements IFileService { } private async doPipeUnbufferedQueued(sourceProvider: IFileSystemProviderWithFileReadWriteCapability, source: URI, targetProvider: IFileSystemProviderWithFileReadWriteCapability, target: URI): Promise { - return targetProvider.writeFile(target, await sourceProvider.readFile(source), { create: true, overwrite: true, unlock: false }); + return targetProvider.writeFile(target, await sourceProvider.readFile(source), { create: true, overwrite: true, unlock: false, atomic: false }); } private async doPipeUnbufferedToBuffered(sourceProvider: IFileSystemProviderWithFileReadWriteCapability, source: URI, targetProvider: IFileSystemProviderWithOpenReadWriteCloseCapability, target: URI): Promise { diff --git a/src/vs/platform/files/common/files.ts b/src/vs/platform/files/common/files.ts index b58fe33019dfa..1f051fdf5e065 100644 --- a/src/vs/platform/files/common/files.ts +++ b/src/vs/platform/files/common/files.ts @@ -283,7 +283,44 @@ export interface IFileAtomicReadOptions { * to from a different process. If you need such atomic * operations, you better use a real database as storage. */ - readonly atomic: true; + readonly atomic: boolean; +} + +export interface IFileAtomicOptions { + + /** + * The postfix is used to create a temporary file based + * on the original resource. The resulting temporary + * file will be in the same folder as the resource and + * have `postfix` appended to the resource name. + * + * Example: given a file resource `file:///some/path/foo.txt` + * and a postfix `.vsctmp`, the temporary file will be + * created as `file:///some/path/foo.txt.vsctmp`. + */ + readonly postfix: string; +} + +export interface IFileAtomicWriteOptions { + + /** + * The optional `atomic` flag can be used to make sure + * the `writeFile` method updates the target file atomically + * by first writing to a temporary file in the same folder + * and then renaming it over the target. + */ + readonly atomic: IFileAtomicOptions | false; +} + +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 in the same + * folder and then deleting it. + */ + readonly atomic: IFileAtomicOptions | false; } export interface IFileReadLimits { @@ -316,7 +353,7 @@ export interface IFileReadStreamOptions { readonly limits?: IFileReadLimits; } -export interface IFileWriteOptions extends IFileOverwriteOptions, IFileUnlockOptions { +export interface IFileWriteOptions extends IFileOverwriteOptions, IFileUnlockOptions, IFileAtomicWriteOptions { /** * Set to `true` to create a file when it does not exist. Will @@ -358,10 +395,21 @@ 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 in the same + * folder and then deleting it. + * + * This option maybe not be supported on all providers. + */ + readonly atomic: IFileAtomicOptions | false; } export enum FileType { @@ -515,10 +563,21 @@ export const enum FileSystemProviderCapabilities { */ FileAtomicRead = 1 << 14, + /** + * Provider support to write files atomically. This implies the + * provider provides the `FileReadWrite` capability too. + */ + FileAtomicWrite = 1 << 15, + + /** + * Provider support to delete atomically. + */ + FileAtomicDelete = 1 << 16, + /** * Provider support to clone files atomically. */ - FileClone = 1 << 15 + FileClone = 1 << 17 } export interface IFileSystemProvider { @@ -607,6 +666,26 @@ export function hasFileAtomicReadCapability(provider: IFileSystemProvider): prov return !!(provider.capabilities & FileSystemProviderCapabilities.FileAtomicRead); } +export interface IFileSystemProviderWithFileAtomicWriteCapability extends IFileSystemProvider { + writeFile(resource: URI, contents: Uint8Array, opts?: IFileAtomicWriteOptions): Promise; +} + +export function hasFileAtomicWriteCapability(provider: IFileSystemProvider): provider is IFileSystemProviderWithFileAtomicWriteCapability { + if (!hasReadWriteCapability(provider)) { + return false; // we require the `FileReadWrite` capability too + } + + return !!(provider.capabilities & FileSystemProviderCapabilities.FileAtomicWrite); +} + +export interface IFileSystemProviderWithFileAtomicDeleteCapability extends IFileSystemProvider { + delete(resource: URI, opts: IFileAtomicDeleteOptions): Promise; +} + +export function hasFileAtomicDeleteCapability(provider: IFileSystemProvider): provider is IFileSystemProviderWithFileAtomicDeleteCapability { + return !!(provider.capabilities & FileSystemProviderCapabilities.FileAtomicDelete); +} + export enum FileSystemProviderErrorCode { FileExists = 'EntryExists', FileNotFound = 'EntryNotFound', @@ -1146,6 +1225,14 @@ export interface IWriteFileOptions { * Whether to attempt to unlock a file before writing. */ readonly unlock?: boolean; + + /** + * The optional `atomic` flag can be used to make sure + * the `writeFile` method updates the target file atomically + * by first writing to a temporary file in the same folder + * and then renaming it over the target. + */ + readonly atomic?: IFileAtomicOptions | false; } export interface IResolveFileOptions { @@ -1185,7 +1272,7 @@ export class FileOperationError extends Error { constructor( message: string, readonly fileOperationResult: FileOperationResult, - readonly options?: IReadFileOptions & IWriteFileOptions & ICreateFileOptions + readonly options?: IReadFileOptions | IWriteFileOptions | ICreateFileOptions ) { super(message); } diff --git a/src/vs/platform/files/node/diskFileSystemProvider.ts b/src/vs/platform/files/node/diskFileSystemProvider.ts index 8a4a63c4d814d..0142abfb605b7 100644 --- a/src/vs/platform/files/node/diskFileSystemProvider.ts +++ b/src/vs/platform/files/node/diskFileSystemProvider.ts @@ -12,14 +12,14 @@ import { CancellationToken } from 'vs/base/common/cancellation'; import { Event } from 'vs/base/common/event'; import { isEqual } from 'vs/base/common/extpath'; import { DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; -import { basename, dirname } from 'vs/base/common/path'; +import { basename, dirname, join } from 'vs/base/common/path'; import { isLinux, isWindows } from 'vs/base/common/platform'; -import { extUriBiasedIgnorePathCase, joinPath } from 'vs/base/common/resources'; +import { extUriBiasedIgnorePathCase, joinPath, basename as resourcesBasename, dirname as resourcesDirname } from 'vs/base/common/resources'; import { newWriteableStream, ReadableStreamEvents } from 'vs/base/common/stream'; import { URI } from 'vs/base/common/uri'; import { IDirent, Promises, RimRafMode, SymlinkSupport } from 'vs/base/node/pfs'; import { localize } from 'vs/nls'; -import { createFileSystemProviderError, IFileAtomicReadOptions, IFileDeleteOptions, IFileOpenOptions, IFileOverwriteOptions, IFileReadStreamOptions, FileSystemProviderCapabilities, FileSystemProviderError, FileSystemProviderErrorCode, FileType, IFileWriteOptions, IFileSystemProviderWithFileAtomicReadCapability, IFileSystemProviderWithFileCloneCapability, IFileSystemProviderWithFileFolderCopyCapability, IFileSystemProviderWithFileReadStreamCapability, IFileSystemProviderWithFileReadWriteCapability, IFileSystemProviderWithOpenReadWriteCloseCapability, isFileOpenForWriteOptions, IStat, FilePermission } from 'vs/platform/files/common/files'; +import { createFileSystemProviderError, IFileAtomicReadOptions, IFileDeleteOptions, IFileOpenOptions, IFileOverwriteOptions, IFileReadStreamOptions, FileSystemProviderCapabilities, FileSystemProviderError, FileSystemProviderErrorCode, FileType, IFileWriteOptions, IFileSystemProviderWithFileAtomicReadCapability, IFileSystemProviderWithFileCloneCapability, IFileSystemProviderWithFileFolderCopyCapability, IFileSystemProviderWithFileReadStreamCapability, IFileSystemProviderWithFileReadWriteCapability, IFileSystemProviderWithOpenReadWriteCloseCapability, isFileOpenForWriteOptions, IStat, FilePermission, IFileSystemProviderWithFileAtomicWriteCapability, IFileSystemProviderWithFileAtomicDeleteCapability } from 'vs/platform/files/common/files'; import { readFileIntoStream } from 'vs/platform/files/common/io'; import { AbstractNonRecursiveWatcherClient, AbstractUniversalWatcherClient, IDiskFileChange, ILogMessage } from 'vs/platform/files/common/watcher'; import { ILogService } from 'vs/platform/log/common/log'; @@ -46,6 +46,8 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple IFileSystemProviderWithFileReadStreamCapability, IFileSystemProviderWithFileFolderCopyCapability, IFileSystemProviderWithFileAtomicReadCapability, + IFileSystemProviderWithFileAtomicWriteCapability, + IFileSystemProviderWithFileAtomicDeleteCapability, IFileSystemProviderWithFileCloneCapability { private static TRACE_LOG_RESOURCE_LOCKS = false; // not enabled by default because very spammy @@ -71,6 +73,8 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple FileSystemProviderCapabilities.FileFolderCopy | FileSystemProviderCapabilities.FileWriteUnlock | FileSystemProviderCapabilities.FileAtomicRead | + FileSystemProviderCapabilities.FileAtomicWrite | + FileSystemProviderCapabilities.FileAtomicDelete | FileSystemProviderCapabilities.FileClone; if (isLinux) { @@ -101,6 +105,14 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple } } + private async statIgnoreError(resource: URI): Promise { + try { + return await this.stat(resource); + } catch (error) { + return undefined; + } + } + async readdir(resource: URI): Promise<[string, FileType][]> { try { const children = await Promises.readdir(this.toFilePath(resource), { withFileTypes: true }); @@ -231,6 +243,37 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple } async writeFile(resource: URI, content: Uint8Array, opts: IFileWriteOptions): Promise { + if (opts?.atomic !== false && opts?.atomic?.postfix) { + return this.doWriteFileAtomic(resource, joinPath(resourcesDirname(resource), `${resourcesBasename(resource)}${opts.atomic.postfix}`), content, opts); + } else { + return this.doWriteFile(resource, content, opts); + } + } + + private async doWriteFileAtomic(resource: URI, tempResource: URI, content: Uint8Array, opts: IFileWriteOptions): Promise { + + // Write to temp resource first + await this.doWriteFile(tempResource, content, opts); + + try { + + // Rename over existing to ensure atomic replace + await this.rename(tempResource, resource, { overwrite: true }); + + } catch (error) { + + // Cleanup in case of rename error + try { + await this.delete(tempResource, { recursive: false, useTrash: false, atomic: false }); + } catch (error) { + // ignore - we want the outer error to bubble up + } + + throw error; + } + } + + private async doWriteFile(resource: URI, content: Uint8Array, opts: IFileWriteOptions): Promise { let handle: number | undefined = undefined; try { const filePath = this.toFilePath(resource); @@ -296,7 +339,9 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple await Promises.chmod(filePath, stat.mode | 0o200); } } catch (error) { - this.logService.trace(error); // ignore any errors here and try to just write + if (error.code !== 'ENOENT') { + this.logService.trace(error); // ignore any errors here and try to just write + } } } @@ -542,7 +587,12 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple try { const filePath = this.toFilePath(resource); if (opts.recursive) { - await Promises.rm(filePath, RimRafMode.MOVE); + let rmMoveToPath: string | undefined = undefined; + if (opts?.atomic !== false && opts.atomic.postfix) { + rmMoveToPath = join(dirname(filePath), `${basename(filePath)}${opts.atomic.postfix}`); + } + + await Promises.rm(filePath, RimRafMode.MOVE, rmMoveToPath); } else { try { await Promises.unlink(filePath); @@ -587,8 +637,8 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple try { - // Ensure target does not exist - await this.validateTargetDeleted(from, to, 'move', opts.overwrite); + // Validate the move operation can perform + await this.validateMoveCopy(from, to, 'move', opts.overwrite); // Move await Promises.move(fromFilePath, toFilePath); @@ -614,8 +664,8 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple try { - // Ensure target does not exist - await this.validateTargetDeleted(from, to, 'copy', opts.overwrite); + // Validate the copy operation can perform + await this.validateMoveCopy(from, to, 'copy', opts.overwrite); // Copy await Promises.copy(fromFilePath, toFilePath, { preserveSymlinks: true }); @@ -631,7 +681,7 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple } } - private async validateTargetDeleted(from: URI, to: URI, mode: 'move' | 'copy', overwrite?: boolean): Promise { + private async validateMoveCopy(from: URI, to: URI, mode: 'move' | 'copy', overwrite?: boolean): Promise { const fromFilePath = this.toFilePath(from); const toFilePath = this.toFilePath(to); @@ -641,18 +691,44 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple isSameResourceWithDifferentPathCase = isEqual(fromFilePath, toFilePath, true /* ignore case */); } - if (isSameResourceWithDifferentPathCase && mode === 'copy') { - throw createFileSystemProviderError(localize('fileCopyErrorPathCase', "'File cannot be copied to same path with different path case"), FileSystemProviderErrorCode.FileExists); - } + if (isSameResourceWithDifferentPathCase) { - // Handle existing target (unless this is a case change) - if (!isSameResourceWithDifferentPathCase && await Promises.exists(toFilePath)) { - if (!overwrite) { - throw createFileSystemProviderError(localize('fileCopyErrorExists', "File at target already exists"), FileSystemProviderErrorCode.FileExists); + // You cannot copy the same file to the same location with different + // path case unless you are on a case sensitive file system + if (mode === 'copy') { + throw createFileSystemProviderError(localize('fileCopyErrorPathCase', "File cannot be copied to same path with different path case"), FileSystemProviderErrorCode.FileExists); } - // Delete target - await this.delete(to, { recursive: true, useTrash: false }); + // You can move the same file to the same location with different + // path case on case insensitive file systems + else if (mode === 'move') { + return; + } + } + + // Here we have to see if the target to move/copy to exists or not. + // We need to respect the `overwrite` option to throw in case the + // target exists. + + const fromStat = await this.statIgnoreError(from); + if (!fromStat) { + throw createFileSystemProviderError(localize('fileMoveCopyErrorNotFound', "File to move/copy does not exist"), FileSystemProviderErrorCode.FileNotFound); + } + + const toStat = await this.statIgnoreError(to); + if (!toStat) { + return; // target does not exist so we are good + } + + if (!overwrite) { + throw createFileSystemProviderError(localize('fileMoveCopyErrorExists', "File at target already exists and thus will not be moved/copied to unless overwrite is specified"), FileSystemProviderErrorCode.FileExists); + } + + // Handle existing target for move/copy + if ((fromStat.type & FileType.File) !== 0 && (toStat.type & FileType.File) !== 0) { + return; // node.js can move/copy a file over an existing file without having to delete it first + } else { + await this.delete(to, { recursive: true, useTrash: false, atomic: false }); } } diff --git a/src/vs/platform/files/test/browser/indexedDBFileService.integrationTest.ts b/src/vs/platform/files/test/browser/indexedDBFileService.integrationTest.ts index a12b43189f0ff..e88991ab7fcae 100644 --- a/src/vs/platform/files/test/browser/indexedDBFileService.integrationTest.ts +++ b/src/vs/platform/files/test/browser/indexedDBFileService.integrationTest.ts @@ -82,7 +82,7 @@ flakySuite('IndexedDBFileSystemProvider', function () { test('root is always present', async () => { assert.strictEqual((await userdataFileProvider.stat(userdataURIFromPaths([]))).type, FileType.Directory); - await userdataFileProvider.delete(userdataURIFromPaths([]), { recursive: true, useTrash: false }); + await userdataFileProvider.delete(userdataURIFromPaths([]), { recursive: true, useTrash: false, atomic: false }); assert.strictEqual((await userdataFileProvider.stat(userdataURIFromPaths([]))).type, FileType.Directory); }); @@ -230,7 +230,7 @@ flakySuite('IndexedDBFileSystemProvider', function () { let creationPromises: Promise | undefined = undefined; return { async create() { - return creationPromises = Promise.all(batch.map(entry => userdataFileProvider.writeFile(entry.resource, VSBuffer.fromString(entry.contents).buffer, { create: true, overwrite: true, unlock: false }))); + return creationPromises = Promise.all(batch.map(entry => userdataFileProvider.writeFile(entry.resource, VSBuffer.fromString(entry.contents).buffer, { create: true, overwrite: true, unlock: false, atomic: false }))); }, async assertContentsCorrect() { if (!creationPromises) { throw Error('read called before create'); } diff --git a/src/vs/platform/files/test/node/diskFileService.integrationTest.ts b/src/vs/platform/files/test/node/diskFileService.integrationTest.ts index 48016404b7ece..fdf58c22472e1 100644 --- a/src/vs/platform/files/test/node/diskFileService.integrationTest.ts +++ b/src/vs/platform/files/test/node/diskFileService.integrationTest.ts @@ -16,7 +16,7 @@ import { joinPath } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; import { Promises } from 'vs/base/node/pfs'; import { flakySuite, getRandomTestPath } from 'vs/base/test/node/testUtils'; -import { etag, IFileAtomicReadOptions, FileOperation, FileOperationError, FileOperationEvent, FileOperationResult, FilePermission, FileSystemProviderCapabilities, hasFileAtomicReadCapability, hasOpenReadWriteCloseCapability, IFileStat, IFileStatWithMetadata, IReadFileOptions, IStat, NotModifiedSinceFileOperationError, TooLargeFileOperationError } from 'vs/platform/files/common/files'; +import { etag, IFileAtomicReadOptions, FileOperation, FileOperationError, FileOperationEvent, FileOperationResult, FilePermission, FileSystemProviderCapabilities, hasFileAtomicReadCapability, hasOpenReadWriteCloseCapability, IFileStat, IFileStatWithMetadata, IReadFileOptions, IStat, NotModifiedSinceFileOperationError, TooLargeFileOperationError, IFileAtomicOptions } from 'vs/platform/files/common/files'; import { FileService } from 'vs/platform/files/common/fileService'; import { DiskFileSystemProvider } from 'vs/platform/files/node/diskFileSystemProvider'; import { NullLogService } from 'vs/platform/log/common/log'; @@ -70,6 +70,8 @@ export class TestDiskFileSystemProvider extends DiskFileSystemProvider { FileSystemProviderCapabilities.FileFolderCopy | FileSystemProviderCapabilities.FileWriteUnlock | FileSystemProviderCapabilities.FileAtomicRead | + FileSystemProviderCapabilities.FileAtomicWrite | + FileSystemProviderCapabilities.FileAtomicDelete | FileSystemProviderCapabilities.FileClone; if (isLinux) { @@ -569,22 +571,26 @@ flakySuite('Disk File Service', function () { }); test('deleteFolder (recursive)', async () => { - return testDeleteFolderRecursive(false); + return testDeleteFolderRecursive(false, false); + }); + + test('deleteFolder (recursive, atomic)', async () => { + return testDeleteFolderRecursive(false, { postfix: '.vsctmp' }); }); (isLinux /* trash is unreliable on Linux */ ? test.skip : test)('deleteFolder (recursive, useTrash)', async () => { - return testDeleteFolderRecursive(true); + return testDeleteFolderRecursive(true, false); }); - async function testDeleteFolderRecursive(useTrash: boolean): Promise { + async function testDeleteFolderRecursive(useTrash: boolean, atomic: IFileAtomicOptions | false): Promise { let event: FileOperationEvent; disposables.add(service.onDidRunOperation(e => event = e)); const resource = URI.file(join(testDir, 'deep')); const source = await service.resolve(resource); - assert.strictEqual(await service.canDelete(source.resource, { recursive: true, useTrash }), true); - await service.del(source.resource, { recursive: true, useTrash }); + 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); assert.ok(event!); @@ -1772,13 +1778,13 @@ flakySuite('Disk File Service', function () { }); test('writeFile - default', async () => { - return testWriteFile(); + return testWriteFile(false); }); test('writeFile - flush on write', async () => { DiskFileSystemProvider.configureFlushOnWrite(true); try { - return await testWriteFile(); + return await testWriteFile(false); } finally { DiskFileSystemProvider.configureFlushOnWrite(false); } @@ -1787,16 +1793,41 @@ flakySuite('Disk File Service', function () { test('writeFile - buffered', async () => { setCapabilities(fileProvider, FileSystemProviderCapabilities.FileOpenReadWriteClose); - return testWriteFile(); + return testWriteFile(false); }); test('writeFile - unbuffered', async () => { setCapabilities(fileProvider, FileSystemProviderCapabilities.FileReadWrite); - return testWriteFile(); + return testWriteFile(false); + }); + + test('writeFile - default (atomic)', async () => { + return testWriteFile(true); }); - async function testWriteFile() { + test('writeFile - flush on write (atomic)', async () => { + DiskFileSystemProvider.configureFlushOnWrite(true); + try { + return await testWriteFile(true); + } finally { + DiskFileSystemProvider.configureFlushOnWrite(false); + } + }); + + test('writeFile - buffered (atomic)', async () => { + setCapabilities(fileProvider, FileSystemProviderCapabilities.FileOpenReadWriteClose | FileSystemProviderCapabilities.FileAtomicWrite); + + return testWriteFile(true); + }); + + test('writeFile - unbuffered (atomic)', async () => { + setCapabilities(fileProvider, FileSystemProviderCapabilities.FileReadWrite | FileSystemProviderCapabilities.FileAtomicWrite); + + return testWriteFile(true); + }); + + async function testWriteFile(atomic: boolean) { let event: FileOperationEvent; disposables.add(service.onDidRunOperation(e => event = e)); @@ -1806,7 +1837,7 @@ flakySuite('Disk File Service', function () { assert.strictEqual(content, 'Small File'); const newContent = 'Updates to the small file'; - await service.writeFile(resource, VSBuffer.fromString(newContent)); + await service.writeFile(resource, VSBuffer.fromString(newContent), { atomic: atomic ? { postfix: '.vsctmp' } : false }); assert.ok(event!); assert.strictEqual(event!.resource.fsPath, resource.fsPath); @@ -1816,28 +1847,44 @@ flakySuite('Disk File Service', function () { } test('writeFile (large file) - default', async () => { - return testWriteFileLarge(); + return testWriteFileLarge(false); }); test('writeFile (large file) - buffered', async () => { setCapabilities(fileProvider, FileSystemProviderCapabilities.FileOpenReadWriteClose); - return testWriteFileLarge(); + return testWriteFileLarge(false); }); test('writeFile (large file) - unbuffered', async () => { setCapabilities(fileProvider, FileSystemProviderCapabilities.FileReadWrite); - return testWriteFileLarge(); + return testWriteFileLarge(false); + }); + + test('writeFile (large file) - default (atomic)', async () => { + return testWriteFileLarge(true); + }); + + test('writeFile (large file) - buffered (atomic)', async () => { + setCapabilities(fileProvider, FileSystemProviderCapabilities.FileOpenReadWriteClose | FileSystemProviderCapabilities.FileAtomicWrite); + + return testWriteFileLarge(true); + }); + + test('writeFile (large file) - unbuffered (atomic)', async () => { + setCapabilities(fileProvider, FileSystemProviderCapabilities.FileReadWrite | FileSystemProviderCapabilities.FileAtomicWrite); + + return testWriteFileLarge(true); }); - async function testWriteFileLarge() { + async function testWriteFileLarge(atomic: boolean) { const resource = URI.file(join(testDir, 'lorem.txt')); const content = readFileSync(resource.fsPath); const newContent = content.toString() + content.toString(); - const fileStat = await service.writeFile(resource, VSBuffer.fromString(newContent)); + const fileStat = await service.writeFile(resource, VSBuffer.fromString(newContent), { atomic: atomic ? { postfix: '.vsctmp' } : false }); assert.strictEqual(fileStat.name, 'lorem.txt'); assert.strictEqual(readFileSync(resource.fsPath).toString(), newContent); diff --git a/src/vs/platform/state/node/stateService.ts b/src/vs/platform/state/node/stateService.ts index 073ec83ef3991..ebe9318124a14 100644 --- a/src/vs/platform/state/node/stateService.ts +++ b/src/vs/platform/state/node/stateService.ts @@ -139,7 +139,7 @@ export class FileStorage { // Write to disk try { - await this.fileService.writeFile(this.storagePath, VSBuffer.fromString(serializedDatabase)); + await this.fileService.writeFile(this.storagePath, VSBuffer.fromString(serializedDatabase), { atomic: { postfix: '.vsctmp' } }); this.lastSavedStorageContents = serializedDatabase; } catch (error) { this.logService.error(error); diff --git a/src/vs/workbench/api/common/extHostFileSystemConsumer.ts b/src/vs/workbench/api/common/extHostFileSystemConsumer.ts index 43adfc90baefa..70af8b12e39de 100644 --- a/src/vs/workbench/api/common/extHostFileSystemConsumer.ts +++ b/src/vs/workbench/api/common/extHostFileSystemConsumer.ts @@ -107,7 +107,7 @@ export class ExtHostConsumerFileSystem { await that._proxy.$ensureActivation(uri.scheme); return await provider.delete(uri, { recursive: false, ...options }); } else { - return await that._proxy.$delete(uri, { recursive: false, useTrash: false, ...options }); + return await that._proxy.$delete(uri, { recursive: false, useTrash: false, atomic: false, ...options }); } } catch (err) { return ExtHostConsumerFileSystem._handleError(err); diff --git a/src/vs/workbench/api/node/extHostDiskFileSystemProvider.ts b/src/vs/workbench/api/node/extHostDiskFileSystemProvider.ts index 32906c3292a4f..b5e9e5d095eac 100644 --- a/src/vs/workbench/api/node/extHostDiskFileSystemProvider.ts +++ b/src/vs/workbench/api/node/extHostDiskFileSystemProvider.ts @@ -55,11 +55,11 @@ class DiskFileSystemProviderAdapter implements vscode.FileSystemProvider { } writeFile(uri: vscode.Uri, content: Uint8Array, options: { readonly create: boolean; readonly overwrite: boolean }): Promise { - return this.impl.writeFile(uri, content, { ...options, unlock: false }); + return this.impl.writeFile(uri, content, { ...options, unlock: false, atomic: false }); } delete(uri: vscode.Uri, options: { readonly recursive: boolean }): Promise { - return this.impl.delete(uri, { ...options, useTrash: false }); + return this.impl.delete(uri, { ...options, useTrash: false, atomic: false }); } rename(oldUri: vscode.Uri, newUri: vscode.Uri, options: { readonly overwrite: boolean }): Promise { diff --git a/src/vs/workbench/contrib/files/browser/editors/textFileSaveErrorHandler.ts b/src/vs/workbench/contrib/files/browser/editors/textFileSaveErrorHandler.ts index 52fd1affaabd9..cb1e773949d92 100644 --- a/src/vs/workbench/contrib/files/browser/editors/textFileSaveErrorHandler.ts +++ b/src/vs/workbench/contrib/files/browser/editors/textFileSaveErrorHandler.ts @@ -8,7 +8,7 @@ import { toErrorMessage } from 'vs/base/common/errorMessage'; import { basename, isEqual } from 'vs/base/common/resources'; import { Action } from 'vs/base/common/actions'; import { URI } from 'vs/base/common/uri'; -import { FileOperationError, FileOperationResult } from 'vs/platform/files/common/files'; +import { FileOperationError, FileOperationResult, IWriteFileOptions } from 'vs/platform/files/common/files'; import { ITextFileService, ISaveErrorHandler, ITextFileEditorModel, ITextFileSaveAsOptions } from 'vs/workbench/services/textfile/common/textfiles'; import { ServicesAccessor, IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IDisposable, dispose, Disposable } from 'vs/base/common/lifecycle'; @@ -134,7 +134,7 @@ export class TextFileSaveErrorHandler extends Disposable implements ISaveErrorHa // Any other save error else { const isWriteLocked = fileOperationError.fileOperationResult === FileOperationResult.FILE_WRITE_LOCKED; - const triedToUnlock = isWriteLocked && fileOperationError.options?.unlock; + const triedToUnlock = isWriteLocked && (fileOperationError.options as IWriteFileOptions | undefined)?.unlock; const isPermissionDenied = fileOperationError.fileOperationResult === FileOperationResult.FILE_PERMISSION_DENIED; const canSaveElevated = resource.scheme === Schemas.file; // currently only supported for local schemes (https://github.com/microsoft/vscode/issues/48659) diff --git a/src/vs/workbench/contrib/webview/browser/resourceLoading.ts b/src/vs/workbench/contrib/webview/browser/resourceLoading.ts index 602e3610c1b98..065187469ca63 100644 --- a/src/vs/workbench/contrib/webview/browser/resourceLoading.ts +++ b/src/vs/workbench/contrib/webview/browser/resourceLoading.ts @@ -9,7 +9,7 @@ import { isUNC } from 'vs/base/common/extpath'; import { Schemas } from 'vs/base/common/network'; import { normalize, sep } from 'vs/base/common/path'; import { URI } from 'vs/base/common/uri'; -import { FileOperationError, FileOperationResult, IFileService } from 'vs/platform/files/common/files'; +import { FileOperationError, FileOperationResult, IFileService, IWriteFileOptions } from 'vs/platform/files/common/files'; import { ILogService } from 'vs/platform/log/common/log'; import { getWebviewContentMimeType } from 'vs/platform/webview/common/mimeTypes'; @@ -73,7 +73,7 @@ export async function loadLocalResource( // NotModified status is expected and can be handled gracefully if (result === FileOperationResult.FILE_NOT_MODIFIED_SINCE) { - return new WebviewResourceResponse.NotModified(mime, err.options?.mtime); + return new WebviewResourceResponse.NotModified(mime, (err.options as IWriteFileOptions | undefined)?.mtime); } } diff --git a/src/vs/workbench/services/textfile/test/browser/browserTextFileService.io.test.ts b/src/vs/workbench/services/textfile/test/browser/browserTextFileService.io.test.ts index e28f5718cef59..58d93d4b78630 100644 --- a/src/vs/workbench/services/textfile/test/browser/browserTextFileService.io.test.ts +++ b/src/vs/workbench/services/textfile/test/browser/browserTextFileService.io.test.ts @@ -57,7 +57,7 @@ if (isWeb) { await fileProvider.writeFile( URI.file(join(testDir, fileName)), files[fileName], - { create: true, overwrite: false, unlock: false } + { create: true, overwrite: false, unlock: false, atomic: false } ); } diff --git a/src/vs/workbench/services/textfile/test/electron-sandbox/nativeTextFileService.io.test.ts b/src/vs/workbench/services/textfile/test/electron-sandbox/nativeTextFileService.io.test.ts index b1d9d93829f76..707780e4c2a57 100644 --- a/src/vs/workbench/services/textfile/test/electron-sandbox/nativeTextFileService.io.test.ts +++ b/src/vs/workbench/services/textfile/test/electron-sandbox/nativeTextFileService.io.test.ts @@ -53,7 +53,7 @@ suite('Files - NativeTextFileService i/o', function () { await fileProvider.writeFile( URI.file(join(testDir, fileName)), files[fileName], - { create: true, overwrite: false, unlock: false } + { create: true, overwrite: false, unlock: false, atomic: false } ); } diff --git a/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts b/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts index bfe4dbb973dc1..c6c0d91e0d1cd 100644 --- a/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts +++ b/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts @@ -1074,7 +1074,7 @@ export class StoredFileWorkingCopy extend // Any other save error else { const isWriteLocked = fileOperationError.fileOperationResult === FileOperationResult.FILE_WRITE_LOCKED; - const triedToUnlock = isWriteLocked && fileOperationError.options?.unlock; + const triedToUnlock = isWriteLocked && (fileOperationError.options as IWriteFileOptions | undefined)?.unlock; const isPermissionDenied = fileOperationError.fileOperationResult === FileOperationResult.FILE_PERMISSION_DENIED; const canSaveElevated = this.elevatedFileService.isSupported(this.resource);