diff --git a/.analysis_options b/.analysis_options index f6381c536ce..ad2d78a009b 100644 --- a/.analysis_options +++ b/.analysis_options @@ -26,6 +26,7 @@ analyzer: todo: ignore exclude: - 'bin/cache/**' + - 'packages/flutter_tools/test/data/dart_dependencies_test/**' linter: rules: diff --git a/.analysis_options_repo b/.analysis_options_repo index 1fe4c4f9b3f..7f3fdd3e507 100644 --- a/.analysis_options_repo +++ b/.analysis_options_repo @@ -27,6 +27,7 @@ analyzer: todo: ignore exclude: - 'bin/cache/**' + - 'packages/flutter_tools/test/data/dart_dependencies_test/**' linter: rules: diff --git a/packages/flutter_tools/lib/src/dart/dependencies.dart b/packages/flutter_tools/lib/src/dart/dependencies.dart new file mode 100644 index 00000000000..04240ce0345 --- /dev/null +++ b/packages/flutter_tools/lib/src/dart/dependencies.dart @@ -0,0 +1,46 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:convert'; + +import 'package:path/path.dart' as path; + +import '../base/process.dart'; +import '../toolchain.dart'; + +class DartDependencySetBuilder { + DartDependencySetBuilder(this.mainScriptPath, + this.projectRootPath, + this.packagesFilePath); + + final String mainScriptPath; + final String projectRootPath; + final String packagesFilePath; + + Set build() { + final String skySnapshotPath = + ToolConfiguration.instance.getHostToolPath(HostTool.SkySnapshot); + + final List args = [ + skySnapshotPath, + '--packages=$packagesFilePath', + '--print-deps', + mainScriptPath + ]; + + String output = runSyncAndThrowStdErrOnError(args); + + final List lines = LineSplitter.split(output).toList(); + final Set minimalDependencies = new Set(); + for (String line in lines) { + if (!line.startsWith('package:')) { + // We convert the uris so that they are relative to the project + // root. + line = path.relative(line, from: projectRootPath); + } + minimalDependencies.add(line); + } + return minimalDependencies; + } +} diff --git a/packages/flutter_tools/lib/src/dart/package_map.dart b/packages/flutter_tools/lib/src/dart/package_map.dart index 53ad511284a..8ea0dbc4053 100644 --- a/packages/flutter_tools/lib/src/dart/package_map.dart +++ b/packages/flutter_tools/lib/src/dart/package_map.dart @@ -29,12 +29,27 @@ class PackageMap { final String packagesPath; - Map get map { + /// Load and parses the .packages file. + void load() { _map ??= _parse(packagesPath); + } + + Map get map { + load(); return _map; } Map _map; + /// Returns the path to [packageUri]. + String pathForPackage(Uri packageUri) { + assert(packageUri.scheme == 'package'); + List pathSegments = packageUri.pathSegments.toList(); + String packageName = pathSegments.removeAt(0); + Uri packageBase = map[packageName]; + String packageRelativePath = path.joinAll(pathSegments); + return packageBase.resolve(packageRelativePath).path; + } + String checkValid() { if (FileSystemEntity.isFileSync(packagesPath)) return null; diff --git a/packages/flutter_tools/lib/src/dependency_checker.dart b/packages/flutter_tools/lib/src/dependency_checker.dart new file mode 100644 index 00000000000..88deffd6ef8 --- /dev/null +++ b/packages/flutter_tools/lib/src/dependency_checker.dart @@ -0,0 +1,67 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; + +import 'globals.dart'; + +import 'dart/dependencies.dart'; +import 'dart/package_map.dart'; +import 'asset.dart'; + +import 'package:path/path.dart' as pathos; + +class DependencyChecker { + final DartDependencySetBuilder builder; + final Set _dependencies = new Set(); + final AssetBundle assets; + DependencyChecker(this.builder, this.assets); + + /// Returns [true] if any components have been modified after [threshold] or + /// if it cannot be determined. + bool check(DateTime threshold) { + _dependencies.clear(); + PackageMap packageMap; + // Parse the package map. + try { + packageMap = new PackageMap(builder.packagesFilePath)..load(); + _dependencies.add(builder.packagesFilePath); + } catch (e, st) { + printTrace('DependencyChecker: could not parse .packages file:\n$e\n$st'); + return true; + } + // Build the set of Dart dependencies. + try { + Set dependencies = builder.build(); + for (String path in dependencies) { + // Ensure all paths are absolute. + if (path.startsWith('package:')) { + path = packageMap.pathForPackage(Uri.parse(path)); + } else { + path = pathos.join(builder.projectRootPath, path); + } + _dependencies.add(path); + } + } catch (e, st) { + printTrace('DependencyChecker: error determining .dart dependencies:\n$e\n$st'); + return true; + } + // TODO(johnmccutchan): Extract dependencies from the AssetBundle too. + + // Check all dependency modification times. + for (String path in _dependencies) { + File file = new File(path); + FileStat stat = file.statSync(); + if (stat.type == FileSystemEntityType.NOT_FOUND) { + printTrace('DependencyChecker: Error stating $path.'); + return true; + } + if (stat.modified.isAfter(threshold)) { + printTrace('DependencyChecker: $path is newer than $threshold'); + return true; + } + } + return false; + } +} diff --git a/packages/flutter_tools/lib/src/hot.dart b/packages/flutter_tools/lib/src/hot.dart index e7d545b1641..0ec3de28204 100644 --- a/packages/flutter_tools/lib/src/hot.dart +++ b/packages/flutter_tools/lib/src/hot.dart @@ -3,7 +3,6 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:convert'; import 'dart:io'; import 'package:meta/meta.dart'; @@ -14,15 +13,15 @@ import 'application_package.dart'; import 'asset.dart'; import 'base/context.dart'; import 'base/logger.dart'; -import 'base/process.dart'; import 'base/utils.dart'; import 'build_info.dart'; import 'dart/package_map.dart'; +import 'dart/dependencies.dart'; import 'devfs.dart'; import 'device.dart'; import 'globals.dart'; import 'resident_runner.dart'; -import 'toolchain.dart'; + import 'vmservice.dart'; import 'usage.dart'; @@ -37,47 +36,6 @@ HotRunnerConfig get hotRunnerConfig => context[HotRunnerConfig]; const bool kHotReloadDefault = true; -class DartDependencySetBuilder { - DartDependencySetBuilder(this.mainScriptPath, - this.projectRootPath, - this.packagesFilePath); - - final String mainScriptPath; - final String projectRootPath; - final String packagesFilePath; - - Set build() { - final String skySnapshotPath = - ToolConfiguration.instance.getHostToolPath(HostTool.SkySnapshot); - - final List args = [ - skySnapshotPath, - '--packages=$packagesFilePath', - '--print-deps', - mainScriptPath - ]; - - String output = runSyncAndThrowStdErrOnError(args); - - final List lines = LineSplitter.split(output).toList(); - final Set minimalDependencies = new Set(); - for (String line in lines) { - // We need to convert the uris so that they are relative to the project - // root and tweak package: uris so that they reflect their devFS location. - if (line.startsWith('package:')) { - // Swap out package: for packages/ because we place all package sources - // under packages/. - line = line.replaceFirst('package:', 'packages/'); - } else { - // Ensure paths are relative to the project root. - line = path.relative(line, from: projectRootPath); - } - minimalDependencies.add(line); - } - return minimalDependencies; - } -} - class HotRunner extends ResidentRunner { HotRunner( Device device, { @@ -151,7 +109,18 @@ class HotRunner extends ResidentRunner { new DartDependencySetBuilder( _mainPath, _projectRootPath, _packagesFilePath); try { - _dartDependencies = dartDependencySetBuilder.build(); + Set dependencies = dartDependencySetBuilder.build(); + _dartDependencies = new Set(); + for (String path in dependencies) { + // We need to tweak package: uris so that they reflect their devFS + // location. + if (path.startsWith('package:')) { + // Swap out package: for packages/ because we place all package + // sources under packages/. + path = path.replaceFirst('package:', 'packages/'); + } + _dartDependencies.add(path); + } } catch (error) { printStatus('Error detected in application source code:', emphasis: true); printError(error); diff --git a/packages/flutter_tools/test/all.dart b/packages/flutter_tools/test/all.dart index de2c8a95d8b..c47203e3672 100644 --- a/packages/flutter_tools/test/all.dart +++ b/packages/flutter_tools/test/all.dart @@ -18,6 +18,8 @@ import 'android_sdk_test.dart' as android_sdk_test; import 'asset_bundle_test.dart' as asset_bundle_test; import 'application_package_test.dart' as application_package_test; import 'base_utils_test.dart' as base_utils_test; +import 'dart_dependencies_test.dart' as dart_dependencies_test; +import 'dependency_checker_test.dart' as dependency_checker_test; import 'channel_test.dart' as channel_test; import 'config_test.dart' as config_test; import 'context_test.dart' as context_test; @@ -55,11 +57,13 @@ void main() { application_package_test.main(); asset_bundle_test.main(); base_utils_test.main(); + dart_dependencies_test.main(); channel_test.main(); config_test.main(); context_test.main(); create_test.main(); daemon_test.main(); + dependency_checker_test.main(); devfs_test.main(); device_test.main(); devices_test.main(); diff --git a/packages/flutter_tools/test/dart_dependencies_test.dart b/packages/flutter_tools/test/dart_dependencies_test.dart new file mode 100644 index 00000000000..068bded723d --- /dev/null +++ b/packages/flutter_tools/test/dart_dependencies_test.dart @@ -0,0 +1,41 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; + +import 'package:flutter_tools/src/dart/dependencies.dart'; +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; +import 'src/context.dart'; + +void main() { + group('DartDependencySetBuilder', () { + final String basePath = path.dirname(Platform.script.path); + final String dataPath = path.join(basePath, 'data', 'dart_dependencies_test'); + testUsingContext('good', () { + final String testPath = path.join(dataPath, 'good'); + final String mainPath = path.join(testPath, 'main.dart'); + final String packagesPath = path.join(testPath, '.packages'); + DartDependencySetBuilder builder = + new DartDependencySetBuilder(mainPath, testPath, packagesPath); + Set dependencies = builder.build(); + expect(dependencies.contains('main.dart'), isTrue); + expect(dependencies.contains('foo.dart'), isTrue); + }); + testUsingContext('syntax_error', () { + final String testPath = path.join(dataPath, 'syntax_error'); + final String mainPath = path.join(testPath, 'main.dart'); + final String packagesPath = path.join(testPath, '.packages'); + DartDependencySetBuilder builder = + new DartDependencySetBuilder(mainPath, testPath, packagesPath); + try { + builder.build(); + fail('expect an assertion to be thrown.'); + } catch (e) { + expect(e, const isInstanceOf()); + expect(e.contains('unexpected token \'bad\''), isTrue); + } + }); + }); +} diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/.dartignore b/packages/flutter_tools/test/data/dart_dependencies_test/.dartignore new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/.gitignore b/packages/flutter_tools/test/data/dart_dependencies_test/.gitignore new file mode 100644 index 00000000000..160ff889221 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/.gitignore @@ -0,0 +1 @@ +!.packages diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/.analysis_options b/packages/flutter_tools/test/data/dart_dependencies_test/good/.analysis_options new file mode 100644 index 00000000000..cb5e559ceaf --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/good/.analysis_options @@ -0,0 +1,3 @@ +analyzer: + exclude: + - '**' diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/.packages b/packages/flutter_tools/test/data/dart_dependencies_test/good/.packages new file mode 100644 index 00000000000..453dc36e857 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/good/.packages @@ -0,0 +1 @@ +self:lib/ diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/foo.dart b/packages/flutter_tools/test/data/dart_dependencies_test/good/foo.dart new file mode 100644 index 00000000000..6853db73b68 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/good/foo.dart @@ -0,0 +1,3 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/lib/bar.dart b/packages/flutter_tools/test/data/dart_dependencies_test/good/lib/bar.dart new file mode 100644 index 00000000000..6853db73b68 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/good/lib/bar.dart @@ -0,0 +1,3 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/main.dart b/packages/flutter_tools/test/data/dart_dependencies_test/good/main.dart new file mode 100644 index 00000000000..659a48df0ec --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/good/main.dart @@ -0,0 +1,6 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'foo.dart'; +import 'package:self/bar.dart'; diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/pubspec.yaml b/packages/flutter_tools/test/data/dart_dependencies_test/good/pubspec.yaml new file mode 100644 index 00000000000..99a0109ab60 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/good/pubspec.yaml @@ -0,0 +1 @@ +name: self diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.analysis_options b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.analysis_options new file mode 100644 index 00000000000..cb5e559ceaf --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.analysis_options @@ -0,0 +1,3 @@ +analyzer: + exclude: + - '**' diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.packages b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.packages new file mode 100644 index 00000000000..453dc36e857 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.packages @@ -0,0 +1 @@ +self:lib/ diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/foo.dart b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/foo.dart new file mode 100644 index 00000000000..b72678b64c0 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/foo.dart @@ -0,0 +1,5 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +bad programmer! diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/main.dart b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/main.dart new file mode 100644 index 00000000000..43d48c01bae --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/main.dart @@ -0,0 +1,5 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'foo.dart'; diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/pubspec.yaml b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/pubspec.yaml new file mode 100644 index 00000000000..99a0109ab60 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/pubspec.yaml @@ -0,0 +1 @@ +name: self diff --git a/packages/flutter_tools/test/dependency_checker_test.dart b/packages/flutter_tools/test/dependency_checker_test.dart new file mode 100644 index 00000000000..5ae8fa51bab --- /dev/null +++ b/packages/flutter_tools/test/dependency_checker_test.dart @@ -0,0 +1,72 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; + +import 'package:flutter_tools/src/dart/dependencies.dart'; +import 'package:flutter_tools/src/dependency_checker.dart'; +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; +import 'src/common.dart'; +import 'src/context.dart'; + +void main() { + group('DependencyChecker', () { + final String basePath = path.dirname(Platform.script.path); + final String dataPath = path.join(basePath, 'data', 'dart_dependencies_test'); + testUsingContext('good', () { + final String testPath = path.join(dataPath, 'good'); + final String mainPath = path.join(testPath, 'main.dart'); + final String fooPath = path.join(testPath, 'foo.dart'); + final String barPath = path.join(testPath, 'lib', 'bar.dart'); + final String packagesPath = path.join(testPath, '.packages'); + DartDependencySetBuilder builder = + new DartDependencySetBuilder(mainPath, testPath, packagesPath); + DependencyChecker dependencyChecker = + new DependencyChecker(builder, null); + + // Set file modification time on all dependencies to be in the past. + DateTime baseTime = new DateTime.now(); + updateFileModificationTime(packagesPath, baseTime, -10); + updateFileModificationTime(mainPath, baseTime, -10); + updateFileModificationTime(fooPath, baseTime, -10); + updateFileModificationTime(barPath, baseTime, -10); + expect(dependencyChecker.check(baseTime), isFalse); + + // Set .packages file modification time to be in the future. + updateFileModificationTime(packagesPath, baseTime, 20); + expect(dependencyChecker.check(baseTime), isTrue); + + // Reset .packages file modification time. + updateFileModificationTime(packagesPath, baseTime, 0); + expect(dependencyChecker.check(baseTime), isFalse); + + // Set 'package:self/bar.dart' file modification time to be in the future. + updateFileModificationTime(barPath, baseTime, 10); + expect(dependencyChecker.check(baseTime), isTrue); + }); + testUsingContext('syntax error', () { + final String testPath = path.join(dataPath, 'syntax_error'); + final String mainPath = path.join(testPath, 'main.dart'); + final String fooPath = path.join(testPath, 'foo.dart'); + final String packagesPath = path.join(testPath, '.packages'); + + DartDependencySetBuilder builder = + new DartDependencySetBuilder(mainPath, testPath, packagesPath); + DependencyChecker dependencyChecker = + new DependencyChecker(builder, null); + + DateTime baseTime = new DateTime.now(); + + // Set file modification time on all dependencies to be in the past. + updateFileModificationTime(packagesPath, baseTime, -10); + updateFileModificationTime(mainPath, baseTime, -10); + updateFileModificationTime(fooPath, baseTime, -10); + + // Dependencies are considered dirty because there is a syntax error in + // the .dart file. + expect(dependencyChecker.check(baseTime), isTrue); + }); + }); +} diff --git a/packages/flutter_tools/test/devfs_test.dart b/packages/flutter_tools/test/devfs_test.dart index eb6907dca7a..179ed90327f 100644 --- a/packages/flutter_tools/test/devfs_test.dart +++ b/packages/flutter_tools/test/devfs_test.dart @@ -10,6 +10,7 @@ import 'package:flutter_tools/src/devfs.dart'; import 'package:path/path.dart' as path; import 'package:test/test.dart'; +import 'src/common.dart'; import 'src/context.dart'; import 'src/mocks.dart'; @@ -48,8 +49,8 @@ void main() { }); testUsingContext('modify existing file on local file system', () async { File file = new File(path.join(basePath, filePath)); - // Set the last modified time to 5 seconds ago. - Process.runSync('touch', ['-A', '-05', file.path]); + // Set the last modified time to 5 seconds in the past. + updateFileModificationTime(file.path, new DateTime.now(), -5); await devFS.update(); await file.writeAsBytes([1, 2, 3, 4, 5, 6]); await devFS.update(); diff --git a/packages/flutter_tools/test/src/common.dart b/packages/flutter_tools/test/src/common.dart index f85f2d996c1..a5d6733da1e 100644 --- a/packages/flutter_tools/test/src/common.dart +++ b/packages/flutter_tools/test/src/common.dart @@ -3,6 +3,9 @@ // found in the LICENSE file. import 'package:args/command_runner.dart'; + +import 'package:flutter_tools/src/base/context.dart'; +import 'package:flutter_tools/src/base/process_manager.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; import 'package:flutter_tools/src/runner/flutter_command_runner.dart'; @@ -12,3 +15,19 @@ CommandRunner createTestCommandRunner([FlutterCommand command]) { runner.addCommand(command); return runner; } + +/// Updates [path] to have a modification time [seconds] from now. +void updateFileModificationTime(String path, + DateTime baseTime, + int seconds) { + DateTime modificationTime = baseTime.add(new Duration(seconds: seconds)); + String argument = + '${modificationTime.year}' + '${modificationTime.month.toString().padLeft(2, "0")}' + '${modificationTime.day.toString().padLeft(2, "0")}' + '${modificationTime.hour.toString().padLeft(2, "0")}' + '${modificationTime.minute.toString().padLeft(2, "0")}' + '.${modificationTime.second.toString().padLeft(2, "0")}'; + ProcessManager processManager = context[ProcessManager]; + processManager.runSync('touch', ['-t', argument, path]); +} diff --git a/packages/flutter_tools/test/src/context.dart b/packages/flutter_tools/test/src/context.dart index 35a286d13a1..92eeb461053 100644 --- a/packages/flutter_tools/test/src/context.dart +++ b/packages/flutter_tools/test/src/context.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:io'; import 'package:flutter_tools/src/base/config.dart'; import 'package:flutter_tools/src/base/context.dart'; @@ -20,6 +21,7 @@ import 'package:flutter_tools/src/toolchain.dart'; import 'package:flutter_tools/src/usage.dart'; import 'package:mockito/mockito.dart'; +import 'package:path/path.dart' as path; import 'package:test/test.dart'; /// Return the test logger. This assumes that the current Logger is a BufferLogger. @@ -64,6 +66,9 @@ void testUsingContext(String description, dynamic testMethod(), { testContext.putIfAbsent(SimControl, () => new MockSimControl()); testContext.putIfAbsent(Usage, () => new MockUsage()); + final String basePath = path.dirname(Platform.script.path); + final String flutterRoot = + path.normalize(path.join(basePath, '..', '..', '..')); try { return await testContext.runInZone(() { // Apply the overrides to the test context in the zone since their @@ -71,7 +76,9 @@ void testUsingContext(String description, dynamic testMethod(), { overrides.forEach((Type type, dynamic value()) { context.setVariable(type, value()); }); - + // Provide a sane default for the flutterRoot directory. Individual + // tests can override this. + Cache.flutterRoot = flutterRoot; return testMethod(); }); } catch (error) {