diff --git a/src/vs/base/node/config.ts b/src/vs/base/node/config.ts index 6fac5849f9615..6c97609d87516 100644 --- a/src/vs/base/node/config.ts +++ b/src/vs/base/node/config.ts @@ -126,8 +126,8 @@ export class ConfigWatcher extends Disposable implements IConfigWatcher { } private async handleSymbolicLink(): Promise { - const { stat, isSymbolicLink } = await statLink(this._path); - if (isSymbolicLink && !stat.isDirectory()) { + const { stat, symbolicLink } = await statLink(this._path); + if (symbolicLink && !stat.isDirectory()) { const realPath = await realpath(this._path); this.watch(realPath, false); diff --git a/src/vs/base/node/pfs.ts b/src/vs/base/node/pfs.ts index 1b294073eed2a..5d09a037d7361 100644 --- a/src/vs/base/node/pfs.ts +++ b/src/vs/base/node/pfs.ts @@ -178,30 +178,52 @@ export function stat(path: string): Promise { } export interface IStatAndLink { + + // The stats of the file. If the file is a symbolic + // link, the stats will be of that target file and + // not the link itself. + // If the file is a symbolic link pointing to a non + // existing file, the stat will be of the link and + // the `dangling` flag will indicate this. stat: fs.Stats; - isSymbolicLink: boolean; + + // Will be provided if the resource is a symbolic link + // on disk. Use the `dangling` flag to find out if it + // points to a resource that does not exist on disk. + symbolicLink?: { dangling: boolean }; } export async function statLink(path: string): Promise { // First stat the link - let linkStat: fs.Stats | undefined; - let linkStatError: NodeJS.ErrnoException | undefined; + let lstats: fs.Stats | undefined; try { - linkStat = await lstat(path); + lstats = await lstat(path); + + // Return early if the stat is not a symbolic link at all + if (!lstats.isSymbolicLink()) { + return { stat: lstats }; + } } catch (error) { - linkStatError = error; + /* ignore - use stat() instead */ } - // Then stat the target and return that - const isLink = !!(linkStat && linkStat.isSymbolicLink()); - if (linkStatError || isLink) { - const fileStat = await stat(path); + // If the stat is a symbolic link or failed to stat, use fs.stat() + // which for symbolic links will stat the target they point to + try { + const stats = await stat(path); - return { stat: fileStat, isSymbolicLink: isLink }; - } + return { stat: stats, symbolicLink: lstats?.isSymbolicLink() ? { dangling: false } : undefined }; + } catch (error) { - return { stat: linkStat!, isSymbolicLink: false }; + // If the link points to a non-existing file we still want + // to return it as result while setting dangling: true flag + if (error.code === 'ENOENT' && lstats) { + return { stat: lstats, symbolicLink: { dangling: true } }; + } + + throw error; + } } export function lstat(path: string): Promise { diff --git a/src/vs/base/test/node/pfs/pfs.test.ts b/src/vs/base/test/node/pfs/pfs.test.ts index aa4ca198eb04d..13ff45194c4fc 100644 --- a/src/vs/base/test/node/pfs/pfs.test.ts +++ b/src/vs/base/test/node/pfs/pfs.test.ts @@ -334,7 +334,7 @@ suite('PFS', function () { test('stat link', async () => { if (isWindows) { - return Promise.resolve(); // Symlinks are not the same on win, and we can not create them programitically without admin privileges + return; // Symlinks are not the same on win, and we can not create them programitically without admin privileges } const id1 = uuid.generateUuid(); @@ -349,14 +349,38 @@ suite('PFS', function () { fs.symlinkSync(directory, symbolicLink); let statAndIsLink = await pfs.statLink(directory); - assert.ok(!statAndIsLink!.isSymbolicLink); + assert.ok(!statAndIsLink?.symbolicLink); statAndIsLink = await pfs.statLink(symbolicLink); - assert.ok(statAndIsLink!.isSymbolicLink); + assert.ok(statAndIsLink?.symbolicLink); + assert.ok(!statAndIsLink?.symbolicLink?.dangling); pfs.rimrafSync(directory); }); + test('stat link (non existing target)', async () => { + if (isWindows) { + return; // Symlinks are not the same on win, and we can not create them programitically without admin privileges + } + + const id1 = uuid.generateUuid(); + const parentDir = path.join(os.tmpdir(), 'vsctests', id1); + const directory = path.join(parentDir, 'pfs', id1); + + const id2 = uuid.generateUuid(); + const symbolicLink = path.join(parentDir, 'pfs', id2); + + await pfs.mkdirp(directory, 493); + + fs.symlinkSync(directory, symbolicLink); + + pfs.rimrafSync(directory); + + const statAndIsLink = await pfs.statLink(symbolicLink); + assert.ok(statAndIsLink?.symbolicLink); + assert.ok(statAndIsLink?.symbolicLink?.dangling); + }); + test('readdir', async () => { if (canNormalize && typeof process.versions['electron'] !== 'undefined' /* needs electron */) { const id = uuid.generateUuid(); @@ -546,7 +570,7 @@ suite('PFS', function () { test('writeFile (stream, error handling EACCES)', async () => { if (isLinux) { - return Promise.resolve(); // somehow this test fails on Linux in our TFS builds + return; // somehow this test fails on Linux in our TFS builds } const id = uuid.generateUuid(); diff --git a/src/vs/platform/files/node/diskFileSystemProvider.ts b/src/vs/platform/files/node/diskFileSystemProvider.ts index bccd1c8d1b05a..9382fd312250e 100644 --- a/src/vs/platform/files/node/diskFileSystemProvider.ts +++ b/src/vs/platform/files/node/diskFileSystemProvider.ts @@ -76,10 +76,10 @@ export class DiskFileSystemProvider extends Disposable implements async stat(resource: URI): Promise { try { - const { stat, isSymbolicLink } = await statLink(this.toFilePath(resource)); // cannot use fs.stat() here to support links properly + const { stat, symbolicLink } = await statLink(this.toFilePath(resource)); // cannot use fs.stat() here to support links properly return { - type: this.toType(stat, isSymbolicLink), + type: this.toType(stat, symbolicLink), ctime: stat.birthtime.getTime(), // intentionally not using ctime here, we want the creation time mtime: stat.mtime.getTime(), size: stat.size @@ -98,18 +98,7 @@ export class DiskFileSystemProvider extends Disposable implements try { let type: FileType; if (child.isSymbolicLink()) { - try { - type = (await this.stat(joinPath(resource, child.name))).type; // always resolve target the link points to if any - } catch (error) { - if (error.code !== FileSystemProviderErrorCode.FileNotFound) { - throw error; // any error that is not file not found is unexpected - } - - // a symbolic link can point to a target that does - // not exist on the file system. in that case we - // still want to return the element as UNKNOWN. - type = FileType.SymbolicLink | FileType.Unknown; - } + type = (await this.stat(joinPath(resource, child.name))).type; // always resolve target the link points to if any } else { type = this.toType(child); } @@ -126,9 +115,24 @@ export class DiskFileSystemProvider extends Disposable implements } } - private toType(entry: Stats | Dirent, isSymbolicLink = entry.isSymbolicLink()): FileType { - let type = entry.isFile() ? FileType.File : entry.isDirectory() ? FileType.Directory : FileType.Unknown; - if (isSymbolicLink) { + private toType(entry: Stats | Dirent, symbolicLink?: { dangling: boolean }): FileType { + + // Signal file type by checking for file / directory, except: + // - symbolic links pointing to non-existing files are FileType.Unknown + // - files that are neither file nor directory are FileType.Unknown + let type: FileType; + if (symbolicLink?.dangling) { + type = FileType.Unknown; + } else if (entry.isFile()) { + type = FileType.File; + } else if (entry.isDirectory()) { + type = FileType.Directory; + } else { + type = FileType.Unknown; + } + + // Always signal symbolic link as file type additionally + if (symbolicLink) { type |= FileType.SymbolicLink; } diff --git a/src/vs/platform/files/node/watcher/nodejs/watcherService.ts b/src/vs/platform/files/node/watcher/nodejs/watcherService.ts index b2bf3498400f1..b25613a214602 100644 --- a/src/vs/platform/files/node/watcher/nodejs/watcherService.ts +++ b/src/vs/platform/files/node/watcher/nodejs/watcherService.ts @@ -35,14 +35,14 @@ export class FileWatcher extends Disposable { private async startWatching(): Promise { try { - const { stat, isSymbolicLink } = await statLink(this.path); + const { stat, symbolicLink } = await statLink(this.path); if (this.isDisposed) { return; } let pathToWatch = this.path; - if (isSymbolicLink) { + if (symbolicLink) { try { pathToWatch = await realpath(pathToWatch); } catch (error) { diff --git a/src/vs/platform/files/test/node/diskFileService.test.ts b/src/vs/platform/files/test/node/diskFileService.test.ts index c58e7045e14be..ad0f3d22529fe 100644 --- a/src/vs/platform/files/test/node/diskFileService.test.ts +++ b/src/vs/platform/files/test/node/diskFileService.test.ts @@ -445,8 +445,7 @@ suite('Disk File Service', function () { return; // not reliable on windows } - const link = URI.file(join(testDir, 'foo')); - await symlink(link.fsPath, join(testDir, 'bar')); + await symlink(join(testDir, 'foo'), join(testDir, 'bar')); const resolved = await service.resolve(URI.file(testDir)); assert.equal(resolved.isDirectory, true); @@ -485,6 +484,52 @@ suite('Disk File Service', function () { assert.equal((error).fileOperationResult, FileOperationResult.FILE_NOT_FOUND); }); + test('deleteFile - symbolic link (exists)', async () => { + if (isWindows) { + return; // not reliable on windows + } + + const target = URI.file(join(testDir, 'lorem.txt')); + const link = URI.file(join(testDir, 'lorem.txt-linked')); + await symlink(target.fsPath, link.fsPath); + + const source = await service.resolve(link); + + let event: FileOperationEvent; + disposables.add(service.onAfterOperation(e => event = e)); + + await service.del(source.resource); + + assert.equal(existsSync(source.resource.fsPath), false); + + assert.ok(event!); + assert.equal(event!.resource.fsPath, link.fsPath); + assert.equal(event!.operation, FileOperation.DELETE); + + assert.equal(existsSync(target.fsPath), true); // target the link pointed to is never deleted + }); + + test('deleteFile - symbolic link (pointing to non-existing file)', async () => { + if (isWindows) { + return; // not reliable on windows + } + + const target = URI.file(join(testDir, 'foo')); + const link = URI.file(join(testDir, 'bar')); + await symlink(target.fsPath, link.fsPath); + + let event: FileOperationEvent; + disposables.add(service.onAfterOperation(e => event = e)); + + await service.del(link); + + assert.equal(existsSync(link.fsPath), false); + + assert.ok(event!); + assert.equal(event!.resource.fsPath, link.fsPath); + assert.equal(event!.operation, FileOperation.DELETE); + }); + test('deleteFolder (recursive)', async () => { let event: FileOperationEvent; disposables.add(service.onAfterOperation(e => event = e));