Do not lower-case paths during canonicalization. (#9571)

* Do not lower-case paths during canonicalization.

This breaks hot reload on some platfroms with case insensitive file systems.

* Add unit tests
This commit is contained in:
Michael Goderbauer 2017-04-25 10:34:43 -07:00 committed by GitHub
parent 31828609dc
commit ad1c497c03
11 changed files with 86 additions and 17 deletions

View file

@ -134,3 +134,11 @@ Directory getReplaySource(String dirname, String basename) {
throwToolExit('Invalid replay-from location: $dirname ("$basename" does not exist)');
return dir;
}
/// Canonicalizes [path].
///
/// This function implements the behaviour of `canonicalize` from
/// `package:path`. However, unlike the original, it does not change the ASCII
/// case of the path. Changing the case can break hot reload in some situations,
/// for an example see: https://github.com/flutter/flutter/issues/9539.
String canonicalizePath(String path) => fs.path.normalize(fs.path.absolute(path));

View file

@ -9,9 +9,9 @@ import '../dart/package_map.dart';
class DartDependencySetBuilder {
DartDependencySetBuilder(String mainScriptPath, String packagesFilePath) :
_mainScriptPath = fs.path.canonicalize(mainScriptPath),
_mainScriptPath = canonicalizePath(mainScriptPath),
_mainScriptUri = fs.path.toUri(mainScriptPath),
_packagesFilePath = fs.path.canonicalize(packagesFilePath);
_packagesFilePath = canonicalizePath(packagesFilePath);
final String _mainScriptPath;
final String _packagesFilePath;
@ -48,7 +48,7 @@ class DartDependencySetBuilder {
}
resolvedUri = newResolvedUri;
}
final String path = fs.path.canonicalize(resolvedUri.toFilePath());
final String path = canonicalizePath(resolvedUri.toFilePath());
if (!dependencies.contains(path)) {
if (!fs.isFileSync(path)) {
throw new DartDependencyException(

View file

@ -499,7 +499,6 @@ class DevFS {
String relativePath,
Uri directoryUriOnDevice, {
bool ignoreDotFiles: true,
Set<String> fileFilter
}) {
if (file is Directory) {
// Skip non-files.
@ -510,13 +509,6 @@ class DevFS {
// Skip dot files.
return true;
}
if (fileFilter != null) {
final String canonicalizeFilePath = fs.path.canonicalize(file.absolute.path);
if ((fileFilter != null) && !fileFilter.contains(canonicalizeFilePath)) {
// Skip files that are not included in the filter.
return true;
}
}
return false;
}
@ -542,8 +534,7 @@ class DevFS {
directoryUriOnDevice =
_directoryUriOnDevice(directoryUriOnDevice, directory);
try {
final String absoluteDirectoryPath =
fs.path.canonicalize(fs.path.absolute(directory.path));
final String absoluteDirectoryPath = canonicalizePath(directory.path);
// For each file in the file filter.
for (String filePath in fileFilter) {
if (!filePath.startsWith(absoluteDirectoryPath)) {
@ -600,7 +591,7 @@ class DevFS {
}
final String relativePath =
fs.path.relative(file.path, from: directory.path);
if (_shouldSkip(file, relativePath, directoryUriOnDevice, ignoreDotFiles: ignoreDotFiles, fileFilter: fileFilter)) {
if (_shouldSkip(file, relativePath, directoryUriOnDevice, ignoreDotFiles: ignoreDotFiles)) {
continue;
}
final Uri deviceUri = directoryUriOnDevice.resolveUri(fs.path.toUri(relativePath));

View file

@ -27,8 +27,8 @@ void main() {
final DartDependencySetBuilder builder =
new DartDependencySetBuilder(mainPath, packagesPath);
final Set<String> dependencies = builder.build();
expect(dependencies.contains(fs.path.canonicalize(mainPath)), isTrue);
expect(dependencies.contains(fs.path.canonicalize(fs.path.join(testPath, 'foo.dart'))), isTrue);
expect(dependencies.contains(canonicalizePath(mainPath)), isTrue);
expect(dependencies.contains(canonicalizePath(fs.path.join(testPath, 'foo.dart'))), isTrue);
});
testUsingContext('syntax_error', () {
@ -73,5 +73,14 @@ void main() {
expect(error.toString(), contains('pubspec.yaml'));
}
});
testUsingContext('does not change ASCI casing of path', () {
final String testPath = fs.path.join(dataPath, 'asci_casing');
final String mainPath = fs.path.join(testPath, 'main.dart');
final String packagesPath = fs.path.join(testPath, '.packages');
final DartDependencySetBuilder builder = new DartDependencySetBuilder(mainPath, packagesPath);
final Set<String> deps = builder.build();
expect(deps, contains(endsWith('This_Import_Has_fuNNy_casING.dart')));
});
});
}

View file

@ -0,0 +1,3 @@
analyzer:
exclude:
- '**'

View file

@ -0,0 +1,5 @@
// Copyright 2017 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.
String dummy = 'Hello';

View file

@ -0,0 +1,9 @@
// Copyright 2017 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 'This_Import_Has_fuNNy_casING.dart';
void main() {
print(dummy);
}

View file

@ -9,6 +9,7 @@ import 'package:file/file.dart';
import 'package:file/memory.dart';
import 'package:flutter_tools/src/asset.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/devfs.dart';
import 'package:flutter_tools/src/vmservice.dart';
@ -121,6 +122,21 @@ void main() {
FileSystem: () => fs,
});
testUsingContext('add new file to local file system and preserve unusal file name casing', () async {
final String filePathWithUnusalCasing = fs.path.join('FooBar', 'TEST.txt');
final File file = fs.file(fs.path.join(basePath, filePathWithUnusalCasing));
await file.parent.create(recursive: true);
file.writeAsBytesSync(<int>[1, 2, 3, 4, 5, 6, 7]);
final int bytes = await devFS.update();
devFSOperations.expectMessages(<String>[
'writeFile test FooBar/TEST.txt',
]);
expect(devFS.assetPathsToEvict, isEmpty);
expect(bytes, 7);
}, overrides: <Type, Generator>{
FileSystem: () => fs,
});
testUsingContext('modify existing file on local file system', () async {
await devFS.update();
final File file = fs.file(fs.path.join(basePath, filePath));
@ -181,7 +197,7 @@ void main() {
fileFilter.addAll(fs.directory(pkgUri)
.listSync(recursive: true)
.where((FileSystemEntity file) => file is File)
.map((FileSystemEntity file) => fs.path.canonicalize(file.path))
.map((FileSystemEntity file) => canonicalizePath(file.path))
.toList());
}
final int bytes = await devFS.update(fileFilter: fileFilter);

View file

@ -59,4 +59,30 @@ void main() {
expect(sourceMemoryFs.directory(sourcePath).listSync().length, 3);
});
});
group('canonicalizePath', () {
test('does not lowercase on Windows', () {
String path = 'C:\\Foo\\bAr\\cOOL.dart';
expect(canonicalizePath(path), path);
// fs.path.canonicalize does lowercase on Windows
expect(fs.path.canonicalize(path), isNot(path));
path = '..\\bar\\.\\\\Foo';
final String expected = fs.path.join(fs.currentDirectory.parent.absolute.path, 'bar', 'Foo');
expect(canonicalizePath(path), expected);
// fs.path.canonicalize should return the same result (modulo casing)
expect(fs.path.canonicalize(path), expected.toLowerCase());
}, testOn: 'windows');
test('does not lowercase on posix', () {
String path = '/Foo/bAr/cOOL.dart';
expect(canonicalizePath(path), path);
// fs.path.canonicalize and canonicalizePath should be the same on Posix
expect(fs.path.canonicalize(path), path);
path = '../bar/.//Foo';
final String expected = fs.path.join(fs.currentDirectory.parent.absolute.path, 'bar', 'Foo');
expect(canonicalizePath(path), expected);
}, testOn: 'posix');
});
}