IgorMinar · May 28, 2015 · May 25, 2015 · May 28, 2015 · May 28, 2015 · May 27, 2015
diff --git a/gulpfile.js b/gulpfile.js
@@ -407,13 +407,6 @@ gulp.task('test.unit.js', ['build.js.dev'], function (neverDone) {
   );
 });
 
-gulp.task('watch.js.dev', ['build.js.dev'], function (neverDone) {
-  watch('modules/**', [
-    '!broccoli.js.dev',
-    '!test.unit.js/karma-run',
-  ]);
-});
-
 
 gulp.task('!test.unit.js/karma-server', function() {
   karma.server.start({configFile: __dirname + '/karma-js.conf.js', reporters: 'dots'});

diff --git a/tools/broccoli/broccoli-tree-stabilizer.ts b/tools/broccoli/broccoli-tree-stabilizer.ts
@@ -0,0 +1,45 @@
+/// <reference path="broccoli.d.ts" />
+/// <reference path="../typings/node/node.d.ts" />
+
+import fs = require('fs');
+let symlinkOrCopy = require('symlink-or-copy');
+
+
+/**
+ * Stabilizes the inputPath for the following plugins in the build tree.
+ *
+ * All broccoli plugins that inherit from `broccoli-writer` or `broccoli-filter` change their
+ * outputPath during each rebuild.
+ *
+ * This means that all following plugins in the build tree can't rely on their inputPath being
+ * immutable. This results in breakage of any plugin that is not expecting such behavior.
+ *
+ * For example all `DiffingBroccoliPlugin`s expect their inputPath to be stable.
+ *
+ * By inserting this plugin into the tree after any misbehaving plugin, we can stabilize the
+ * inputPath for the following plugin in the tree and correct the surprising behavior.
+ */
+class TreeStabilizer implements BroccoliTree {
+  inputPath: string;
+  outputPath: string;
+
+
+  constructor(public inputTree: BroccoliTree) {}
+
+
+  rebuild() {
+    fs.rmdirSync(this.outputPath);
+
+    // TODO: investigate if we can use rename the directory instead to improve performance on
+    // Windows
+    symlinkOrCopy.sync(this.inputPath, this.outputPath);
+  }
+
+
+  cleanup() {}
+}
+
+
+export default function stabilizeTree(inputTree) {
+  return new TreeStabilizer(inputTree);
+}
diff --git a/tools/broccoli/broccoli.d.ts b/tools/broccoli/broccoli.d.ts
@@ -17,7 +17,8 @@ interface BroccoliTree {
    * For plugins that take only one input tree, it might be more convenient to use the `inputPath`
    *property instead.
    *
-   * This property is set just before the first rebuild and doesn't change afterwards.
+   * This property is set just before the first rebuild and doesn't change afterwards, unless
+   * plugins themselves change it.
    *
    * If the inputPath is outside of broccoli's temp directory, then it's lifetime is not managed by
    *the builder.
@@ -31,7 +32,8 @@ interface BroccoliTree {
   /**
    * Contains the fs paths for the output trees.
    *
-   * This property is set just before the first rebuild and doesn't change afterwards.
+   * This property is set just before the first rebuild and doesn't change afterwards, unless the
+   * plugins themselves change it.
    *
    * The underlying directory is also created by the builder just before the first rebuild.
    * This directory is destroyed and recreated upon each rebuild.

diff --git a/tools/broccoli/diffing-broccoli-plugin.ts b/tools/broccoli/diffing-broccoli-plugin.ts
@@ -6,6 +6,7 @@ import fs = require('fs');
 import fse = require('fs-extra');
 import path = require('path');
 import {TreeDiffer, DiffResult} from './tree-differ';
+import stabilizeTree from './broccoli-tree-stabilizer';
 let symlinkOrCopy = require('symlink-or-copy');
 
 
@@ -51,9 +52,9 @@ class DiffingPluginWrapper implements BroccoliTree {
 
   constructor(private pluginClass, private wrappedPluginArguments) {
     if (Array.isArray(wrappedPluginArguments[0])) {
-      this.inputTrees = wrappedPluginArguments[0];
+      this.inputTrees = this.stabilizeTrees(wrappedPluginArguments[0]);
     } else {
-      this.inputTree = wrappedPluginArguments[0];
+      this.inputTree = this.stabilizeTree(wrappedPluginArguments[0]);
     }
 
     this.description = this.pluginClass.name;
@@ -82,6 +83,13 @@ class DiffingPluginWrapper implements BroccoliTree {
   }
 
 
+  cleanup() {
+    if (this.wrappedPlugin.cleanup) {
+      this.wrappedPlugin.cleanup();
+    }
+  }
+
+
   private relinkOutputAndCachePaths() {
     // just symlink the cache and output tree
     fs.rmdirSync(this.outputPath);
@@ -94,16 +102,23 @@ class DiffingPluginWrapper implements BroccoliTree {
       let includeExtensions = this.pluginClass.includeExtensions || [];
       let excludeExtensions = this.pluginClass.excludeExtensions || [];
       this.initialized = true;
-      this.treeDiffer = new TreeDiffer(this.inputPath, includeExtensions, excludeExtensions);
+      this.treeDiffer =
+          new TreeDiffer(this.description, this.inputPath, includeExtensions, excludeExtensions);
       this.wrappedPlugin =
           new this.pluginClass(this.inputPath, this.cachePath, this.wrappedPluginArguments[1]);
     }
   }
 
 
-  cleanup() {
-    if (this.wrappedPlugin.cleanup) {
-      this.wrappedPlugin.cleanup();
-    }
+  private stabilizeTrees(trees: BroccoliTree[]) {
+    return trees.map((tree) => this.stabilizeTree(tree));
+  }
+
+
+  private stabilizeTree(tree: BroccoliTree) {
+    // Ignore all DiffingPlugins as they are already stable, for others we don't know for sure
+    // so we need to stabilize them.
+    // Since it's not safe to use instanceof operator in node, we are checking the constructor.name.
+    return (tree.constructor['name'] === 'DiffingPluginWrapper') ? tree : stabilizeTree(tree);
   }
 }
diff --git a/tools/broccoli/tree-differ.spec.ts b/tools/broccoli/tree-differ.spec.ts
@@ -26,7 +26,7 @@ describe('TreeDiffer', () => {
       };
       mockfs(testDir);
 
-      let differ = new TreeDiffer('dir1');
+      let differ = new TreeDiffer('testLabel', 'dir1');
 
       let diffResult = differ.diffTree();
 
@@ -49,7 +49,7 @@ describe('TreeDiffer', () => {
       };
       mockfs(testDir);
 
-      let differ = new TreeDiffer('dir1');
+      let differ = new TreeDiffer('testLabel', 'dir1');
 
       let diffResult = differ.diffTree();
 
@@ -76,7 +76,7 @@ describe('TreeDiffer', () => {
       };
       mockfs(testDir);
 
-      let differ = new TreeDiffer('dir1');
+      let differ = new TreeDiffer('testLabel', 'dir1');
 
       let diffResult = differ.diffTree();
 
@@ -123,7 +123,7 @@ describe('TreeDiffer', () => {
       };
       mockfs(testDir);
 
-      let differ = new TreeDiffer('symlinks');
+      let differ = new TreeDiffer('testLabel', 'symlinks');
 
       let diffResult = differ.diffTree();
 
@@ -177,6 +177,17 @@ describe('TreeDiffer', () => {
     });
 
 
+    it("should throw an error if an extension isn't prefixed with doc", () => {
+      // includeExtensions
+      expect(() => new TreeDiffer('testLabel', 'dir1', ['js']))
+          .toThrowError("Extension must begin with '.'. Was: 'js'");
+
+      // excludeExtentions
+      expect(() => new TreeDiffer('testLabel', 'dir1', [], ['js']))
+          .toThrowError("Extension must begin with '.'. Was: 'js'");
+    });
+
+
     it('should ignore files with extensions not listed in includeExtensions', () => {
       let testDir = {
         'dir1': {
@@ -190,7 +201,7 @@ describe('TreeDiffer', () => {
       };
       mockfs(testDir);
 
-      let differ = new TreeDiffer('dir1', ['.js', '.coffee']);
+      let differ = new TreeDiffer('testLabel', 'dir1', ['.js', '.coffee']);
 
       let diffResult = differ.diffTree();
 
@@ -236,7 +247,7 @@ describe('TreeDiffer', () => {
       };
       mockfs(testDir);
 
-      let differ = new TreeDiffer('dir1', ['.ts', '.cs'], ['.d.ts', '.d.cs']);
+      let differ = new TreeDiffer('testLabel', 'dir1', ['.ts', '.cs'], ['.d.ts', '.d.cs']);
 
       let diffResult = differ.diffTree();
 
@@ -277,7 +288,7 @@ describe('TreeDiffer', () => {
       };
       mockfs(testDir);
 
-      let differ = new TreeDiffer('dir1');
+      let differ = new TreeDiffer('testLabel', 'dir1');
       differ.diffTree();
 
       testDir['dir1']['file-2.txt'] = 'new file';
@@ -286,24 +297,24 @@ describe('TreeDiffer', () => {
       let diffResult = differ.diffTree();
       expect(diffResult.changedPaths).toEqual(['file-2.txt']);
     });
-  });
 
 
-  it('should detect file additions mixed with file changes', () => {
-    let testDir = {
-      'dir1': {'file-1.txt': mockfs.file({content: 'file-1.txt content', mtime: new Date(1000)})}
-    };
-    mockfs(testDir);
+    it('should detect file additions mixed with file changes', () => {
+      let testDir = {
+        'dir1': {'file-1.txt': mockfs.file({content: 'file-1.txt content', mtime: new Date(1000)})}
+      };
+      mockfs(testDir);
 
-    let differ = new TreeDiffer('dir1');
-    differ.diffTree();
+      let differ = new TreeDiffer('testLabel', 'dir1');
+      differ.diffTree();
 
-    testDir['dir1']['file-1.txt'] = 'new content';
-    testDir['dir1']['file-2.txt'] = 'new file';
-    mockfs(testDir);
+      testDir['dir1']['file-1.txt'] = 'new content';
+      testDir['dir1']['file-2.txt'] = 'new file';
+      mockfs(testDir);
 
-    let diffResult = differ.diffTree();
-    expect(diffResult.changedPaths).toEqual(['file-1.txt', 'file-2.txt']);
+      let diffResult = differ.diffTree();
+      expect(diffResult.changedPaths).toEqual(['file-1.txt', 'file-2.txt']);
+    });
   });
 
 
@@ -316,7 +327,7 @@ describe('TreeDiffer', () => {
       };
       mockfs(testDir);
 
-      let differ = new TreeDiffer('dir1');
+      let differ = new TreeDiffer('testLabel', 'dir1');
       differ.diffTree();
 
       delete testDir['dir1']['file-1.txt'];
@@ -326,29 +337,29 @@ describe('TreeDiffer', () => {
       expect(diffResult.changedPaths).toEqual([]);
       expect(diffResult.removedPaths).toEqual(['file-1.txt']);
     });
-  });
 
 
-  it('should detect file removals mixed with file changes and additions', () => {
-    let testDir = {
-      'dir1': {
-        'file-1.txt': mockfs.file({content: 'file-1.txt content', mtime: new Date(1000)}),
-        'file-2.txt': mockfs.file({content: 'file-1.txt content', mtime: new Date(1000)})
-      }
-    };
+    it('should detect file removals mixed with file changes and additions', () => {
+      let testDir = {
+        'dir1': {
+          'file-1.txt': mockfs.file({content: 'file-1.txt content', mtime: new Date(1000)}),
+          'file-2.txt': mockfs.file({content: 'file-1.txt content', mtime: new Date(1000)})
+        }
+      };
 
-    mockfs(testDir);
+      mockfs(testDir);
 
-    let differ = new TreeDiffer('dir1');
-    differ.diffTree();
+      let differ = new TreeDiffer('testLabel', 'dir1');
+      differ.diffTree();
 
-    testDir['dir1']['file-1.txt'] = 'changed content';
-    delete testDir['dir1']['file-2.txt'];
-    testDir['dir1']['file-3.txt'] = 'new content';
-    mockfs(testDir);
+      testDir['dir1']['file-1.txt'] = 'changed content';
+      delete testDir['dir1']['file-2.txt'];
+      testDir['dir1']['file-3.txt'] = 'new content';
+      mockfs(testDir);
 
-    let diffResult = differ.diffTree();
-    expect(diffResult.changedPaths).toEqual(['file-1.txt', 'file-3.txt']);
-    expect(diffResult.removedPaths).toEqual(['file-2.txt']);
+      let diffResult = differ.diffTree();
+      expect(diffResult.changedPaths).toEqual(['file-1.txt', 'file-3.txt']);
+      expect(diffResult.removedPaths).toEqual(['file-2.txt']);
+    });
   });
 });
diff --git a/tools/broccoli/tree-differ.ts b/tools/broccoli/tree-differ.ts
@@ -21,7 +21,7 @@ export class TreeDiffer {
   private include: RegExp = null;
   private exclude: RegExp = null;
 
-  constructor(private rootPath: string, includeExtensions?: string[],
+  constructor(private label: string, private rootPath: string, includeExtensions?: string[],
               excludeExtensions?: string[]) {
     this.rootDirName = path.basename(rootPath);
 
@@ -31,7 +31,9 @@ export class TreeDiffer {
     this.exclude = (excludeExtensions || []).length ? buildRegexp(excludeExtensions) : null;
 
     function combine(prev, curr) {
-      if (curr.charAt(0) !== ".") throw new TypeError("Extension must begin with '.'");
+      if (curr.charAt(0) !== ".") {
+        throw new Error(`Extension must begin with '.'. Was: '${curr}'`);
+      }
       let kSpecialRegexpChars = /[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g;
       curr = '(' + curr.replace(kSpecialRegexpChars, '\\$&') + ')';
       return prev ? (prev + '|' + curr) : curr;
@@ -40,7 +42,7 @@ export class TreeDiffer {
 
 
   public diffTree(): DiffResult {
-    let result = new DirtyCheckingDiffResult(this.rootDirName);
+    let result = new DirtyCheckingDiffResult(this.label, this.rootDirName);
     this.dirtyCheckPath(this.rootPath, result);
     this.detectDeletionsAndUpdateFingerprints(result);
     result.endTime = Date.now();
@@ -127,12 +129,12 @@ class DirtyCheckingDiffResult {
   public startTime: number = Date.now();
   public endTime: number = null;
 
-  constructor(public name: string) {}
+  constructor(public label:string, public directoryName: string) {}
 
   toString() {
-    return `${pad(this.name, 40)}, duration: ${pad(this.endTime - this.startTime, 5)}ms, ` +
-           `${pad(this.changedPaths.length + this.removedPaths.length, 5)} changes detected ` +
-           `(files: ${pad(this.filesChecked, 5)}, directories: ${pad(this.directoriesChecked, 4)})`;
+    return `${pad(this.label, 30)}, ${pad(this.endTime - this.startTime, 5)}ms, ` +
+           `${pad(this.changedPaths.length + this.removedPaths.length, 5)} changes ` +
+           `(files: ${pad(this.filesChecked, 5)}, dirs: ${pad(this.directoriesChecked, 4)})`;
   }
 
   log(verbose) {

diff --git a/tools/broccoli/trees/browser_tree.ts b/tools/broccoli/trees/browser_tree.ts
@@ -56,11 +56,6 @@ module.exports = function makeBrowserTree(options, destinationPath) {
 
   var es6Tree = mergeTrees([traceurTree, typescriptTree]);
 
-  // TODO(iminar): tree differ seems to have issues with trees created by mergeTrees, investigate!
-  //   ENOENT error is thrown while doing fs.readdirSync on inputRoot
-  //   in the meantime, we just do noop mv to create a new tree
-  es6Tree = stew.mv(es6Tree, '');
-
   // Call Traceur again to lower the ES6 build tree to ES5
   var es5Tree = transpileWithTraceur(es6Tree, {
     destExtension: '.js',
@@ -152,10 +147,5 @@ module.exports = function makeBrowserTree(options, destinationPath) {
 
   var mergedTree = mergeTrees([stew.mv(es6Tree, '/es6'), stew.mv(es5Tree, '/es5')]);
 
-  // TODO(iminar): tree differ seems to have issues with trees created by mergeTrees, investigate!
-  //   ENOENT error is thrown while doing fs.readdirSync on inputRoot
-  //   in the meantime, we just do noop mv to create a new tree
-  mergedTree = stew.mv(mergedTree, '');
-
   return destCopy(mergedTree, destinationPath);
 };
diff --git a/tools/broccoli/trees/dart_tree.ts b/tools/broccoli/trees/dart_tree.ts
@@ -144,10 +144,5 @@ module.exports = function makeDartTree(destinationPath) {
 
   var dartTree = mergeTrees([sourceTree, getTemplatedPubspecsTree(), getDocsTree()]);
 
-  // TODO(iminar): tree differ seems to have issues with trees created by mergeTrees, investigate!
-  //   ENOENT error is thrown while doing fs.readdirSync on inputRoot
-  //   in the meantime, we just do noop mv to create a new tree
-  dartTree = stew.mv(dartTree, '');
-
   return destCopy(dartTree, destinationPath);
 };