build(broccoli): add tree-stabilizer plugin to deal with unstable trees#2143
build(broccoli): add tree-stabilizer plugin to deal with unstable trees#2143IgorMinar merged 7 commits intoangular:masterfrom
Conversation
There was a problem hiding this comment.
rmdirSync will fail if non-empty, which it probably is. use rimraf or fs-extra instead?
There was a problem hiding this comment.
Also, (using rimraf), if outputPath is a symlink of inputPath, isn't that going to remove everything in the inputPath? That seems like a not good thing to happen
There was a problem hiding this comment.
FYI: Calling rimraf (or fs-extra, which calls rimraf) on a symlink will only remove the symlink, not descend into the target directory.
There was a problem hiding this comment.
@caitp unless there is a bug in the system, we can't get into a situation where the underlying directory doesn't exist. output path is created by broccoli builder for each rebuild unless it already exists.
There was a problem hiding this comment.
That doesn't address what my comment is saying. "rmdirSync" will fail (read: throw) if the directory is non-empty. We aren't talking about whether it exists or not, we're talking about it being non-empty, which it probably is. This most likely works correctly on unix systems where symlinkOrCopy is not copying, but on windows machines it's likely to break
|
For the record, I think we see better perf on win32 by avoiding the deep copy and just dealing with the property changing. Deep copy could be quite expensive in some cases |
|
Another option is to just re-initialize the wrapped plugin if the inputPath(s) change, so you still get to keep your happy lifecycle management, without having to do unnecessary copying |
IgorMinar
force-pushed
the
stabilize-trees
branch
from
20fbe15 to
a260b3f
Compare
|
@caitp we can't re-init the plugin as that would kill the incremental build. |
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 angular#2051
…than inputPath Usually we don't care what we are diffing, but why we are diffing it. With this change we see what is causing build slowdown due to diffing.
IgorMinar
force-pushed
the
stabilize-trees
branch
from
8de36f3 to
b144174
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
5 participants
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