Roll forward #8467 (#8477)

* Revert "Revert "Simplify path handling logic in dependency checker and devFS (#8414)" (#8467)"

This reverts commit 96ba7f76d2.

* Intentionally use a self-package URI in flutter_gallery

* tests to catch problems with self-package imports
This commit is contained in:
Michael Goderbauer 2017-02-28 17:21:17 -08:00 committed by GitHub
parent 6571b41f40
commit 65835af4e6
8 changed files with 64 additions and 103 deletions

View file

@ -33,6 +33,8 @@ export 'snack_bar_demo.dart';
export 'tabs_demo.dart'; export 'tabs_demo.dart';
export 'tabs_fab_demo.dart'; export 'tabs_fab_demo.dart';
export 'text_field_demo.dart'; export 'text_field_demo.dart';
export 'tooltip_demo.dart';
export 'two_level_list_demo.dart'; export 'two_level_list_demo.dart';
export 'typography_demo.dart'; export 'typography_demo.dart';
// Intentionally use a self-package URL to make sure we don't break these.
export 'package:flutter_gallery/demo/tooltip_demo.dart';

View file

@ -5,7 +5,6 @@
import 'dart:convert'; import 'dart:convert';
import '../artifacts.dart'; import '../artifacts.dart';
import '../base/file_system.dart';
import '../base/process.dart'; import '../base/process.dart';
class DartDependencySetBuilder { class DartDependencySetBuilder {
@ -30,16 +29,6 @@ class DartDependencySetBuilder {
String output = runSyncAndThrowStdErrOnError(args); String output = runSyncAndThrowStdErrOnError(args);
final List<String> lines = LineSplitter.split(output).toList(); return new Set<String>.from(LineSplitter.split(output));
final Set<String> minimalDependencies = new Set<String>();
for (String line in lines) {
if (!line.startsWith('package:')) {
// We convert the uris so that they are relative to the project
// root.
line = fs.path.relative(line, from: projectRootPath);
}
minimalDependencies.add(line);
}
return minimalDependencies;
} }
} }

View file

@ -5,7 +5,6 @@
import 'asset.dart'; import 'asset.dart';
import 'base/file_system.dart'; import 'base/file_system.dart';
import 'dart/dependencies.dart'; import 'dart/dependencies.dart';
import 'dart/package_map.dart';
import 'globals.dart'; import 'globals.dart';
class DependencyChecker { class DependencyChecker {
@ -18,10 +17,7 @@ class DependencyChecker {
/// if it cannot be determined. /// if it cannot be determined.
bool check(DateTime threshold) { bool check(DateTime threshold) {
_dependencies.clear(); _dependencies.clear();
PackageMap packageMap;
// Parse the package map.
try { try {
packageMap = new PackageMap(builder.packagesFilePath)..load();
_dependencies.add(builder.packagesFilePath); _dependencies.add(builder.packagesFilePath);
} catch (e, st) { } catch (e, st) {
printTrace('DependencyChecker: could not parse .packages file:\n$e\n$st'); printTrace('DependencyChecker: could not parse .packages file:\n$e\n$st');
@ -29,16 +25,7 @@ class DependencyChecker {
} }
// Build the set of Dart dependencies. // Build the set of Dart dependencies.
try { try {
Set<String> dependencies = builder.build(); _dependencies.addAll(builder.build());
for (String path in dependencies) {
// Ensure all paths are absolute.
if (path.startsWith('package:')) {
path = packageMap.pathForPackage(Uri.parse(path));
} else {
path = fs.path.join(builder.projectRootPath, path);
}
_dependencies.add(path);
}
} catch (e, st) { } catch (e, st) {
printTrace('DependencyChecker: error determining .dart dependencies:\n$e\n$st'); printTrace('DependencyChecker: error determining .dart dependencies:\n$e\n$st');
return true; return true;

View file

@ -461,10 +461,10 @@ class DevFS {
} }
bool _shouldIgnore(String devicePath) { bool _shouldIgnore(String devicePath) {
List<String> ignoredPrefixes = <String>['android/', List<String> ignoredPrefixes = <String>['android' + fs.path.separator,
getBuildDirectory(), getBuildDirectory(),
'ios/', 'ios' + fs.path.separator,
'.pub/']; '.pub' + fs.path.separator];
for (String ignoredPrefix in ignoredPrefixes) { for (String ignoredPrefix in ignoredPrefixes) {
if (devicePath.startsWith(ignoredPrefix)) if (devicePath.startsWith(ignoredPrefix))
return true; return true;
@ -473,16 +473,14 @@ class DevFS {
} }
Future<bool> _scanDirectory(Directory directory, Future<bool> _scanDirectory(Directory directory,
{String directoryName, {String directoryNameOnDevice,
bool recursive: false, bool recursive: false,
bool ignoreDotFiles: true, bool ignoreDotFiles: true,
String packagesDirectoryName,
Set<String> fileFilter}) async { Set<String> fileFilter}) async {
String prefix = directoryName; if (directoryNameOnDevice == null) {
if (prefix == null) { directoryNameOnDevice = fs.path.relative(directory.path, from: rootDirectory.path);
prefix = fs.path.relative(directory.path, from: rootDirectory.path); if (directoryNameOnDevice == '.')
if (prefix == '.') directoryNameOnDevice = '';
prefix = '';
} }
try { try {
Stream<FileSystemEntity> files = Stream<FileSystemEntity> files =
@ -508,24 +506,8 @@ class DevFS {
} }
final String relativePath = final String relativePath =
fs.path.relative(file.path, from: directory.path); fs.path.relative(file.path, from: directory.path);
final String devicePath = fs.path.join(prefix, relativePath); final String devicePath = fs.path.join(directoryNameOnDevice, relativePath);
bool filtered = false; if ((fileFilter != null) && !fileFilter.contains(file.absolute.path)) {
if ((fileFilter != null) &&
!fileFilter.contains(devicePath)) {
if (packagesDirectoryName != null) {
// Double check the filter for packages/packagename/
final String packagesDevicePath =
fs.path.join(packagesDirectoryName, relativePath);
if (!fileFilter.contains(packagesDevicePath)) {
// File was not in the filter set.
filtered = true;
}
} else {
// File was not in the filter set.
filtered = true;
}
}
if (filtered) {
// Skip files that are not included in the filter. // Skip files that are not included in the filter.
continue; continue;
} }
@ -548,27 +530,26 @@ class DevFS {
PackageMap packageMap = new PackageMap(_packagesFilePath); PackageMap packageMap = new PackageMap(_packagesFilePath);
for (String packageName in packageMap.map.keys) { for (String packageName in packageMap.map.keys) {
Uri uri = packageMap.map[packageName]; Uri packageUri = packageMap.map[packageName];
// This project's own package. String packagePath = fs.path.fromUri(packageUri);
final bool isProjectPackage = uri.toString() == 'lib/'; Directory packageDirectory = fs.directory(packageUri);
final String directoryName = String directoryNameOnDevice = fs.path.join('packages', packageName);
isProjectPackage ? 'lib' : fs.path.join('packages', packageName); bool packageExists;
// If this is the project's package, we need to pass both
// package:<package_name> and lib/ as paths to be checked against if (fs.path.isWithin(rootDirectory.path, packagePath)) {
// the filter because we must support both package: imports and relative // We already scanned everything under the root directory.
// path imports within the project's own code. packageExists = packageDirectory.existsSync();
final String packagesDirectoryName = directoryNameOnDevice = fs.path.relative(packagePath, from: rootDirectory.path);
isProjectPackage ? fs.path.join('packages', packageName) : null; } else {
Directory directory = fs.directory(uri); packageExists =
bool packageExists = await _scanDirectory(packageDirectory,
await _scanDirectory(directory, directoryNameOnDevice: directoryNameOnDevice,
directoryName: directoryName, recursive: true,
recursive: true, fileFilter: fileFilter);
packagesDirectoryName: packagesDirectoryName, }
fileFilter: fileFilter);
if (packageExists) { if (packageExists) {
sb ??= new StringBuffer(); sb ??= new StringBuffer();
sb.writeln('$packageName:$directoryName'); sb.writeln('$packageName:$directoryNameOnDevice');
} }
} }
if (sb != null) { if (sb != null) {

View file

@ -98,18 +98,7 @@ class HotRunner extends ResidentRunner {
new DartDependencySetBuilder( new DartDependencySetBuilder(
mainPath, projectRootPath, packagesFilePath); mainPath, projectRootPath, packagesFilePath);
try { try {
Set<String> dependencies = dartDependencySetBuilder.build(); _dartDependencies = new Set<String>.from(dartDependencySetBuilder.build());
_dartDependencies = new Set<String>();
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) { } catch (error) {
printStatus('Error detected in application source code:', emphasis: true); printStatus('Error detected in application source code:', emphasis: true);
printError('$error'); printError('$error');

View file

@ -21,8 +21,8 @@ void main() {
DartDependencySetBuilder builder = DartDependencySetBuilder builder =
new DartDependencySetBuilder(mainPath, testPath, packagesPath); new DartDependencySetBuilder(mainPath, testPath, packagesPath);
Set<String> dependencies = builder.build(); Set<String> dependencies = builder.build();
expect(dependencies.contains('main.dart'), isTrue); expect(dependencies.contains(mainPath), isTrue);
expect(dependencies.contains('foo.dart'), isTrue); expect(dependencies.contains(fs.path.join(testPath, 'foo.dart')), isTrue);
}); });
testUsingContext('syntax_error', () { testUsingContext('syntax_error', () {
final String testPath = fs.path.join(dataPath, 'syntax_error'); final String testPath = fs.path.join(dataPath, 'syntax_error');

View file

@ -4,6 +4,7 @@
import 'dart:async'; import 'dart:async';
import 'dart:convert'; import 'dart:convert';
import 'dart:io' as io;
import 'package:flutter_tools/src/asset.dart'; import 'package:flutter_tools/src/asset.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
@ -18,7 +19,7 @@ import 'src/context.dart';
import 'src/mocks.dart'; import 'src/mocks.dart';
void main() { void main() {
final String filePath = fs.path.join('bar', 'foo.txt'); final String filePath = fs.path.join('lib', 'foo.txt');
final String filePath2 = fs.path.join('foo', 'bar.txt'); final String filePath2 = fs.path.join('foo', 'bar.txt');
Directory tempDir; Directory tempDir;
String basePath; String basePath;
@ -69,6 +70,7 @@ void main() {
File file = fs.file(fs.path.join(basePath, filePath)); File file = fs.file(fs.path.join(basePath, filePath));
await file.parent.create(recursive: true); await file.parent.create(recursive: true);
file.writeAsBytesSync(<int>[1, 2, 3]); file.writeAsBytesSync(<int>[1, 2, 3]);
_packages['my_project'] = 'lib';
// simulate package // simulate package
await _createPackage('somepkg', 'somefile.txt'); await _createPackage('somepkg', 'somefile.txt');
@ -80,12 +82,20 @@ void main() {
int bytes = await devFS.update(); int bytes = await devFS.update();
devFSOperations.expectMessages(<String>[ devFSOperations.expectMessages(<String>[
'writeFile test .packages', 'writeFile test ${fs.path.join('lib', 'foo.txt')}',
'writeFile test ${fs.path.join('bar', 'foo.txt')}',
'writeFile test ${fs.path.join('packages', 'somepkg', 'somefile.txt')}', 'writeFile test ${fs.path.join('packages', 'somepkg', 'somefile.txt')}',
'writeFile test .packages',
]); ]);
expect(devFS.assetPathsToEvict, isEmpty); expect(devFS.assetPathsToEvict, isEmpty);
expect(bytes, 31);
List<String> packageSpecOnDevice = LineSplitter.split(UTF8.decode(
await devFSOperations.devicePathToContent['.packages'].contentsAsBytes()
)).toList();
expect(packageSpecOnDevice,
unorderedEquals(['my_project:lib', 'somepkg:packages/somepkg'])
);
expect(bytes, 46);
}); });
testUsingContext('add new file to local file system', () async { testUsingContext('add new file to local file system', () async {
File file = fs.file(fs.path.join(basePath, filePath2)); File file = fs.file(fs.path.join(basePath, filePath2));
@ -110,7 +120,7 @@ void main() {
await file.writeAsBytes(<int>[1, 2, 3, 4, 5, 6]); await file.writeAsBytes(<int>[1, 2, 3, 4, 5, 6]);
bytes = await devFS.update(); bytes = await devFS.update();
devFSOperations.expectMessages(<String>[ devFSOperations.expectMessages(<String>[
'writeFile test ${fs.path.join('bar', 'foo.txt')}', 'writeFile test ${fs.path.join('lib', 'foo.txt')}',
]); ]);
expect(devFS.assetPathsToEvict, isEmpty); expect(devFS.assetPathsToEvict, isEmpty);
expect(bytes, 6); expect(bytes, 6);
@ -120,7 +130,7 @@ void main() {
await file.delete(); await file.delete();
int bytes = await devFS.update(); int bytes = await devFS.update();
devFSOperations.expectMessages(<String>[ devFSOperations.expectMessages(<String>[
'deleteFile test ${fs.path.join('bar', 'foo.txt')}', 'deleteFile test ${fs.path.join('lib', 'foo.txt')}',
]); ]);
expect(devFS.assetPathsToEvict, isEmpty); expect(devFS.assetPathsToEvict, isEmpty);
expect(bytes, 0); expect(bytes, 0);
@ -133,7 +143,7 @@ void main() {
'writeFile test ${fs.path.join('packages', 'newpkg', 'anotherfile.txt')}', 'writeFile test ${fs.path.join('packages', 'newpkg', 'anotherfile.txt')}',
]); ]);
expect(devFS.assetPathsToEvict, isEmpty); expect(devFS.assetPathsToEvict, isEmpty);
expect(bytes, 51); expect(bytes, 66);
}); });
testUsingContext('add an asset bundle', () async { testUsingContext('add an asset bundle', () async {
assetBundle.entries['a.txt'] = new DevFSStringContent('abc'); assetBundle.entries['a.txt'] = new DevFSStringContent('abc');
@ -197,7 +207,7 @@ void main() {
devFSOperations.expectMessages(<String>['destroy test']); devFSOperations.expectMessages(<String>['destroy test']);
expect(devFS.assetPathsToEvict, isEmpty); expect(devFS.assetPathsToEvict, isEmpty);
}); });
}); }, skip: io.Platform.isWindows);
group('devfs remote', () { group('devfs remote', () {
MockVMService vmService; MockVMService vmService;
@ -230,11 +240,11 @@ void main() {
int bytes = await devFS.update(); int bytes = await devFS.update();
vmService.expectMessages(<String>[ vmService.expectMessages(<String>[
'writeFile test .packages', 'writeFile test .packages',
'writeFile test ${fs.path.join('bar', 'foo.txt')}', 'writeFile test ${fs.path.join('lib', 'foo.txt')}',
'writeFile test ${fs.path.join('packages', 'somepkg', 'somefile.txt')}', 'writeFile test ${fs.path.join('packages', 'somepkg', 'somefile.txt')}',
]); ]);
expect(devFS.assetPathsToEvict, isEmpty); expect(devFS.assetPathsToEvict, isEmpty);
expect(bytes, 31); expect(bytes, 46);
}, timeout: const Timeout(const Duration(seconds: 5))); }, timeout: const Timeout(const Duration(seconds: 5)));
testUsingContext('delete dev file system', () async { testUsingContext('delete dev file system', () async {
@ -242,7 +252,7 @@ void main() {
vmService.expectMessages(<String>['_deleteDevFS {fsName: test}']); vmService.expectMessages(<String>['_deleteDevFS {fsName: test}']);
expect(devFS.assetPathsToEvict, isEmpty); expect(devFS.assetPathsToEvict, isEmpty);
}); });
}); }, skip: io.Platform.isWindows);
} }
class MockVMService extends BasicMock implements VMService { class MockVMService extends BasicMock implements VMService {
@ -311,7 +321,7 @@ class MockVM implements VM {
final List<Directory> _tempDirs = <Directory>[]; final List<Directory> _tempDirs = <Directory>[];
final Map <String, Directory> _packages = <String, Directory>{}; final Map <String, String> _packages = <String, String>{};
Directory _newTempDir() { Directory _newTempDir() {
Directory tempDir = fs.systemTempDirectory.createTempSync('devfs${_tempDirs.length}'); Directory tempDir = fs.systemTempDirectory.createTempSync('devfs${_tempDirs.length}');
@ -330,10 +340,9 @@ Future<Null> _createPackage(String pkgName, String pkgFileName) async {
File pkgFile = fs.file(fs.path.join(pkgTempDir.path, pkgName, 'lib', pkgFileName)); File pkgFile = fs.file(fs.path.join(pkgTempDir.path, pkgName, 'lib', pkgFileName));
await pkgFile.parent.create(recursive: true); await pkgFile.parent.create(recursive: true);
pkgFile.writeAsBytesSync(<int>[11, 12, 13]); pkgFile.writeAsBytesSync(<int>[11, 12, 13]);
_packages[pkgName] = pkgTempDir; _packages[pkgName] = pkgFile.parent.path;
StringBuffer sb = new StringBuffer(); StringBuffer sb = new StringBuffer();
_packages.forEach((String pkgName, Directory pkgTempDir) { _packages.forEach((String pkgName, String pkgPath) {
Uri pkgPath = fs.path.toUri(fs.path.join(pkgTempDir.path, pkgName, 'lib'));
sb.writeln('$pkgName:$pkgPath'); sb.writeln('$pkgName:$pkgPath');
}); });
fs.file(fs.path.join(_tempDirs[0].path, '.packages')).writeAsStringSync(sb.toString()); fs.file(fs.path.join(_tempDirs[0].path, '.packages')).writeAsStringSync(sb.toString());

View file

@ -95,6 +95,8 @@ class BasicMock {
} }
class MockDevFSOperations extends BasicMock implements DevFSOperations { class MockDevFSOperations extends BasicMock implements DevFSOperations {
Map<String, DevFSContent> devicePathToContent = <String, DevFSContent>{};
@override @override
Future<Uri> create(String fsName) async { Future<Uri> create(String fsName) async {
messages.add('create $fsName'); messages.add('create $fsName');
@ -109,10 +111,12 @@ class MockDevFSOperations extends BasicMock implements DevFSOperations {
@override @override
Future<dynamic> writeFile(String fsName, String devicePath, DevFSContent content) async { Future<dynamic> writeFile(String fsName, String devicePath, DevFSContent content) async {
messages.add('writeFile $fsName $devicePath'); messages.add('writeFile $fsName $devicePath');
devicePathToContent[devicePath] = content;
} }
@override @override
Future<dynamic> deleteFile(String fsName, String devicePath) async { Future<dynamic> deleteFile(String fsName, String devicePath) async {
messages.add('deleteFile $fsName $devicePath'); messages.add('deleteFile $fsName $devicePath');
devicePathToContent.remove(devicePath);
} }
} }