From aad5795408bb32f48c3dc066dc7a60cf7fe71b44 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Mon, 18 May 2015 13:39:28 -0400 Subject: [PATCH] fix(tree-differ): treat symlinks to deleted paths as removals Previously, tree-differ would not correctly handle symlinks to deleted files, resulting in an ENOENT errno being tossed by libuv. This change fixes this to ensure that symlinks are safely handled, performantly. Closes #1961 --- tools/broccoli/tree-differ.spec.ts | 73 ++++++++++++++++++++++++++++++ tools/broccoli/tree-differ.ts | 16 ++++++- 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/tools/broccoli/tree-differ.spec.ts b/tools/broccoli/tree-differ.spec.ts index d54e5cf24c..46421e8413 100644 --- a/tools/broccoli/tree-differ.spec.ts +++ b/tools/broccoli/tree-differ.spec.ts @@ -104,6 +104,79 @@ describe('TreeDiffer', () => { }); + it('should handle changes via symbolic links', () => { + let testDir = { + 'orig_path': { + 'file-1.txt': mockfs.file({content: 'file-1.txt content', mtime: new Date(1000)}), + 'file-2.txt': mockfs.file({content: 'file-2.txt content', mtime: new Date(1000)}), + 'subdir-1': { + 'file-1.1.txt': + mockfs.file({content: 'file-1.1.txt content', mtime: new Date(1000)}) + } + }, + 'symlinks': { + 'file-1.txt': mockfs.symlink({path: '../orig_path/file-1.txt'}), + 'file-2.txt': mockfs.symlink({path: '../orig_path/file-2.txt'}), + 'subdir-1': + {'file-1.1.txt': mockfs.symlink({path: '../../orig_path/subdir-1/file-1.1.txt'})} + } + }; + mockfs(testDir); + + let differ = new TreeDiffer('symlinks'); + + let diffResult = differ.diffTree(); + + expect(diffResult.changedPaths) + .toEqual(['file-1.txt', 'file-2.txt', 'subdir-1/file-1.1.txt']); + + // change two files + testDir['orig_path']['file-1.txt'] = + mockfs.file({content: 'new content', mtime: new Date(1000)}); + testDir['orig_path']['subdir-1']['file-1.1.txt'] = + mockfs.file({content: 'file-1.1.txt content', mtime: new Date(9999)}); + mockfs(testDir); + + diffResult = differ.diffTree(); + + expect(diffResult.changedPaths).toEqual(['file-1.txt', 'subdir-1/file-1.1.txt']); + + expect(diffResult.removedPaths).toEqual([]); + + // change one file + testDir['orig_path']['file-1.txt'] = + mockfs.file({content: 'super new', mtime: new Date(1000)}); + mockfs(testDir); + + diffResult = differ.diffTree(); + expect(diffResult.changedPaths).toEqual(['file-1.txt']); + + // remove a link + delete testDir['orig_path']['file-1.txt']; + mockfs(testDir); + + diffResult = differ.diffTree(); + expect(diffResult.changedPaths).toEqual([]); + expect(diffResult.removedPaths).toEqual(['file-1.txt']); + + // don't report it as a removal twice + mockfs(testDir); + + diffResult = differ.diffTree(); + expect(diffResult.changedPaths).toEqual([]); + expect(diffResult.removedPaths).toEqual([]); + + // re-add it. + testDir['orig_path']['file-1.txt'] = + mockfs.file({content: 'super new', mtime: new Date(1000)}); + mockfs(testDir); + + diffResult = differ.diffTree(); + expect(diffResult.changedPaths).toEqual(['file-1.txt']); + expect(diffResult.removedPaths).toEqual([]); + }); + + it('should ignore files with extensions not listed in includeExtensions', () => { let testDir = { 'dir1': { diff --git a/tools/broccoli/tree-differ.ts b/tools/broccoli/tree-differ.ts index 20a7cce363..efcaabee70 100644 --- a/tools/broccoli/tree-differ.ts +++ b/tools/broccoli/tree-differ.ts @@ -4,6 +4,16 @@ import fs = require('fs'); import path = require('path'); +function tryStatSync(path) { + try { + return fs.statSync(path); + } catch (e) { + if (e.code === "ENOENT") return null; + throw e; + } +} + + export class TreeDiffer { private fingerprints: {[key: string]: string} = Object.create(null); private nextFingerprints: {[key: string]: string} = Object.create(null); @@ -41,7 +51,11 @@ export class TreeDiffer { private dirtyCheckPath(rootDir: string, result: DirtyCheckingDiffResult) { fs.readdirSync(rootDir).forEach((segment) => { let absolutePath = path.join(rootDir, segment); - let pathStat = fs.statSync(absolutePath); + let pathStat = fs.lstatSync(absolutePath); + if (pathStat.isSymbolicLink()) { + pathStat = tryStatSync(absolutePath); + if (pathStat === null) return; + } if (pathStat.isDirectory()) { result.directoriesChecked++;