build(broccoli): add tree-stabilizer plugin to deal with unstable trees

Previously we assumed that all input and ouput paths for broccoli trees are immutable, that turned out to be
incorrect.

By adding a tree stabilizer plugin in front of each diffing plugin, we ensure that the input trees
are stable. The stabilization is done via symlinks which is super cheap on platforms that support
symlinks. On Windows we currently copy the whole input directory, which is far from ideal. We should
investagate if using move operation on Windows is ok in the future to improve performance.

Closes #2051
This commit is contained in:
Igor Minar 2015-05-24 17:27:38 -07:00
parent 01fb8e6635
commit 7b1e9286d8
5 changed files with 65 additions and 33 deletions

View File

@ -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);
}

View File

@ -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);
@ -101,9 +109,15 @@ class DiffingPluginWrapper implements BroccoliTree {
}
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);
}
}

View File

@ -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);
};

View File

@ -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);
};

View File

@ -96,11 +96,6 @@ module.exports = function makeNodeTree(destinationPath) {
}
});
// 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
traceurCompatibleTsModulesTree = stew.mv(traceurCompatibleTsModulesTree, '');
var typescriptTree = compileWithTypescript(traceurCompatibleTsModulesTree, {
allowNonTsExtensions: false,
emitDecoratorMetadata: true,
@ -131,12 +126,5 @@ module.exports = function makeNodeTree(destinationPath) {
}]
});
// 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
nodeTree = stew.mv(nodeTree, '');
return destCopy(nodeTree, destinationPath);
};