From d510733e88bf0dbea05f21352b4c9df59f042622 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Tue, 21 Jun 2016 12:56:37 -0700 Subject: [PATCH 1/3] Fix #109, convert packages paths to absolute paths in file watcher --- CHANGELOG.md | 4 ++++ lib/src/generate/watch_impl.dart | 38 +++++++++++++++++++++++++++----- pubspec.yaml | 2 +- test/generate/watch_test.dart | 17 ++++++++++++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a39b38a2d9..53664c0412 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.3.0+6 +- Convert `packages` paths in the file watcher to their absolute paths. This + fixes [#109](https://github.com/dart-lang/build/issues/109). + ## 0.3.0+5 - Fix duplicate logs issue when running as a BuilderTransformer. diff --git a/lib/src/generate/watch_impl.dart b/lib/src/generate/watch_impl.dart index df458d2873..b8dd89b66e 100644 --- a/lib/src/generate/watch_impl.dart +++ b/lib/src/generate/watch_impl.dart @@ -195,17 +195,45 @@ class WatchImpl { _logger.fine('Setting up watcher at $absolutePackagePath'); // Ignore all subfolders which are other packages. - var pathsToIgnore = absolutePackagePaths.values.where((path) => - path != absolutePackagePath && - path.startsWith(absolutePackagePath)); + var pathsToIgnore = absolutePackagePaths.values + .where((path) => + path != absolutePackagePath && + path.startsWith(absolutePackagePath)) + .toList(); var watcher = _directoryWatcherFactory(absolutePackagePath); watchers.add(watcher); _allListeners.add(watcher.events.listen((WatchEvent e) { + var changePath = e.path; + + // Convert `packages` paths to absolute paths. + () { + var changePathParts = path.split(changePath); + var packagesIndex = changePathParts.indexOf('packages'); + if (packagesIndex != -1) { + if (changePathParts.length < packagesIndex + 2) { + _logger.severe('Invalid change path: $changePath'); + return; + } + + var packageName = changePathParts[packagesIndex + 1]; + var packageNode = _packageGraph[packageName]; + if (packageNode == null) { + _logger.severe('Got update for invalid package: $packageName'); + return; + } + var packagePath = absolutePackagePaths[packageNode]; + var libPath = path.joinAll(['lib'] + ..addAll(changePathParts.getRange( + packagesIndex + 2, changePathParts.length))); + changePath = path.join(packagePath, libPath); + } + }(); + // Check for ignored paths and immediately bail. - if (pathsToIgnore.any((path) => e.path.startsWith(path))) return; + if (pathsToIgnore.any((path) => changePath.startsWith(path))) return; - var relativePath = path.relative(e.path, from: absolutePackagePath); + var relativePath = path.relative(changePath, from: absolutePackagePath); _logger.finest( 'Got ${e.type} event for path $relativePath from ${watcher.path}'); var id = new AssetId(package.name, relativePath); diff --git a/pubspec.yaml b/pubspec.yaml index 06926248fe..274f77d6e9 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: build -version: 0.3.0+5 +version: 0.3.0+6 description: A build system for Dart. author: Dart Team homepage: https://github.com/dart-lang/build diff --git a/test/generate/watch_test.dart b/test/generate/watch_test.dart index df6433f19e..00c8462f41 100644 --- a/test/generate/watch_test.dart +++ b/test/generate/watch_test.dart @@ -181,6 +181,23 @@ void main() { // matches the input set. checkOutputs({'a|web/a.txt.copy': 'b',}, result, writer.assets); }); + + test('converts packages paths to absolute ones', () async { + var writer = new InMemoryAssetWriter(); + var results = []; + startWatch(copyAPhaseGroup, {'a|lib/a.txt': 'a'}, writer) + .listen(results.add); + + var result = await nextResult(results); + checkOutputs({'a|lib/a.txt.copy': 'a',}, result, writer.assets); + + await writer.writeAsString(makeAsset('a|lib/a.txt', 'b')); + FakeWatcher.notifyWatchers(new WatchEvent(ChangeType.MODIFY, + path.absolute('a', 'packages', 'a', 'a.txt'))); + + result = await nextResult(results); + checkOutputs({'a|lib/a.txt.copy': 'b',}, result, writer.assets); + }); }); group('multiple phases', () { From 134ee9808e8097b83ce7e6c49579c6c29adb7c12 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Tue, 21 Jun 2016 14:50:17 -0700 Subject: [PATCH 2/3] code review updates --- lib/src/generate/watch_impl.dart | 56 +++++++++++++++++--------------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/lib/src/generate/watch_impl.dart b/lib/src/generate/watch_impl.dart index b8dd89b66e..2014d027b0 100644 --- a/lib/src/generate/watch_impl.dart +++ b/lib/src/generate/watch_impl.dart @@ -204,36 +204,14 @@ class WatchImpl { var watcher = _directoryWatcherFactory(absolutePackagePath); watchers.add(watcher); _allListeners.add(watcher.events.listen((WatchEvent e) { - var changePath = e.path; - - // Convert `packages` paths to absolute paths. - () { - var changePathParts = path.split(changePath); - var packagesIndex = changePathParts.indexOf('packages'); - if (packagesIndex != -1) { - if (changePathParts.length < packagesIndex + 2) { - _logger.severe('Invalid change path: $changePath'); - return; - } - - var packageName = changePathParts[packagesIndex + 1]; - var packageNode = _packageGraph[packageName]; - if (packageNode == null) { - _logger.severe('Got update for invalid package: $packageName'); - return; - } - var packagePath = absolutePackagePaths[packageNode]; - var libPath = path.joinAll(['lib'] - ..addAll(changePathParts.getRange( - packagesIndex + 2, changePathParts.length))); - changePath = path.join(packagePath, libPath); - } - }(); + var changePath = _normalizeChangePath(e.path, absolutePackagePaths); + if (changePath == null) return; // Check for ignored paths and immediately bail. if (pathsToIgnore.any((path) => changePath.startsWith(path))) return; - var relativePath = path.relative(changePath, from: absolutePackagePath); + var relativePath = + path.relative(changePath, from: absolutePackagePath); _logger.finest( 'Got ${e.type} event for path $relativePath from ${watcher.path}'); var id = new AssetId(package.name, relativePath); @@ -261,4 +239,30 @@ class WatchImpl { var node = _assetGraph.get(id); return node is GeneratedAssetNode && type != ChangeType.REMOVE; } + + // Convert `packages` paths to absolute paths. Returns null if it finds an + // invalid package path. + String _normalizeChangePath( + String changePath, Map absolutePackagePaths) { + var changePathParts = path.split(changePath); + var packagesIndex = changePathParts.indexOf('packages'); + if (packagesIndex == -1) return changePath; + + if (changePathParts.length < packagesIndex + 2) { + _logger.severe('Invalid change path: $changePath'); + return null; + } + + var packageName = changePathParts[packagesIndex + 1]; + var packageNode = _packageGraph[packageName]; + if (packageNode == null) { + _logger.severe('Got update for invalid package: $packageName'); + return null; + } + var packagePath = absolutePackagePaths[packageNode]; + var libPath = path.joinAll(['lib'] + ..addAll( + changePathParts.getRange(packagesIndex + 2, changePathParts.length))); + return path.join(packagePath, libPath); + } } From 04b15715ba3f316b6c63787bad44aece7816ac42 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Tue, 21 Jun 2016 14:50:43 -0700 Subject: [PATCH 3/3] format --- test/generate/watch_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/generate/watch_test.dart b/test/generate/watch_test.dart index 00c8462f41..5d2741a8d8 100644 --- a/test/generate/watch_test.dart +++ b/test/generate/watch_test.dart @@ -192,8 +192,8 @@ void main() { checkOutputs({'a|lib/a.txt.copy': 'a',}, result, writer.assets); await writer.writeAsString(makeAsset('a|lib/a.txt', 'b')); - FakeWatcher.notifyWatchers(new WatchEvent(ChangeType.MODIFY, - path.absolute('a', 'packages', 'a', 'a.txt'))); + FakeWatcher.notifyWatchers(new WatchEvent( + ChangeType.MODIFY, path.absolute('a', 'packages', 'a', 'a.txt'))); result = await nextResult(results); checkOutputs({'a|lib/a.txt.copy': 'b',}, result, writer.assets);