Unverified Commit 99d66f27 authored by Zachary Anderson's avatar Zachary Anderson Committed by GitHub

[flutter_tool] Make a couple file operations synchronous (#37044)

parent 6160c765
...@@ -141,12 +141,21 @@ class Cache { ...@@ -141,12 +141,21 @@ class Cache {
if (!_lockEnabled) if (!_lockEnabled)
return; return;
assert(_lock == null); assert(_lock == null);
_lock = await fs.file(fs.path.join(flutterRoot, 'bin', 'cache', 'lockfile')).open(mode: FileMode.write); final File lockFile =
fs.file(fs.path.join(flutterRoot, 'bin', 'cache', 'lockfile'));
try {
_lock = lockFile.openSync(mode: FileMode.write);
} on FileSystemException catch (e) {
printError('Failed to open or create the artifact cache lockfile: "$e"');
printError('Please ensure you have permissions to create or open '
'${lockFile.path}');
throwToolExit('Failed to open or create the lockfile');
}
bool locked = false; bool locked = false;
bool printed = false; bool printed = false;
while (!locked) { while (!locked) {
try { try {
await _lock.lock(); _lock.lockSync();
locked = true; locked = true;
} on FileSystemException { } on FileSystemException {
if (!printed) { if (!printed) {
......
...@@ -367,7 +367,13 @@ class FlutterCommandRunner extends CommandRunner<void> { ...@@ -367,7 +367,13 @@ class FlutterCommandRunner extends CommandRunner<void> {
flutterUsage.suppressAnalytics = true; flutterUsage.suppressAnalytics = true;
_checkFlutterCopy(); _checkFlutterCopy();
await FlutterVersion.instance.ensureVersionFile(); try {
await FlutterVersion.instance.ensureVersionFile();
} on FileSystemException catch (e) {
printError('Failed to write the version file to the artifact cache: "$e".');
printError('Please ensure you have permissions in the artifact cache directory.');
throwToolExit('Failed to write the version file');
}
if (topLevelResults.command?.name != 'upgrade' && topLevelResults['version-check']) { if (topLevelResults.command?.name != 'upgrade' && topLevelResults['version-check']) {
await FlutterVersion.instance.checkFlutterVersionFreshness(); await FlutterVersion.instance.checkFlutterVersionFreshness();
} }
......
...@@ -99,8 +99,8 @@ class FlutterVersion { ...@@ -99,8 +99,8 @@ class FlutterVersion {
String get engineRevision => Cache.instance.engineRevision; String get engineRevision => Cache.instance.engineRevision;
String get engineRevisionShort => _shortGitRevision(engineRevision); String get engineRevisionShort => _shortGitRevision(engineRevision);
Future<void> ensureVersionFile() { Future<void> ensureVersionFile() async {
return fs.file(fs.path.join(Cache.flutterRoot, 'version')).writeAsString(_frameworkVersion); fs.file(fs.path.join(Cache.flutterRoot, 'version')).writeAsStringSync(_frameworkVersion);
} }
@override @override
......
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async';
import 'package:file/file.dart'; import 'package:file/file.dart';
import 'package:file/memory.dart'; import 'package:file/memory.dart';
import 'package:file_testing/file_testing.dart'; import 'package:file_testing/file_testing.dart';
...@@ -11,6 +9,7 @@ import 'package:meta/meta.dart'; ...@@ -11,6 +9,7 @@ import 'package:meta/meta.dart';
import 'package:mockito/mockito.dart'; import 'package:mockito/mockito.dart';
import 'package:platform/platform.dart'; import 'package:platform/platform.dart';
import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart' show InternetAddress, SocketException; import 'package:flutter_tools/src/base/io.dart' show InternetAddress, SocketException;
...@@ -23,7 +22,18 @@ import '../src/testbed.dart'; ...@@ -23,7 +22,18 @@ import '../src/testbed.dart';
void main() { void main() {
group('$Cache.checkLockAcquired', () { group('$Cache.checkLockAcquired', () {
MockFileSystem mockFileSystem;
MemoryFileSystem memoryFileSystem;
MockFile mockFile;
MockRandomAccessFile mockRandomAccessFile;
setUp(() { setUp(() {
mockFileSystem = MockFileSystem();
memoryFileSystem = MemoryFileSystem();
mockFile = MockFile();
mockRandomAccessFile = MockRandomAccessFile();
when(mockFileSystem.path).thenReturn(memoryFileSystem.path);
Cache.enableLocking(); Cache.enableLocking();
}); });
...@@ -31,6 +41,7 @@ void main() { ...@@ -31,6 +41,7 @@ void main() {
// Restore locking to prevent potential side-effects in // Restore locking to prevent potential side-effects in
// tests outside this group (this option is globally shared). // tests outside this group (this option is globally shared).
Cache.enableLocking(); Cache.enableLocking();
Cache.releaseLockEarly();
}); });
test('should throw when locking is not acquired', () { test('should throw when locking is not acquired', () {
...@@ -43,10 +54,21 @@ void main() { ...@@ -43,10 +54,21 @@ void main() {
}); });
testUsingContext('should not throw when lock is acquired', () async { testUsingContext('should not throw when lock is acquired', () async {
when(mockFileSystem.file(argThat(endsWith('lockfile')))).thenReturn(mockFile);
when(mockFile.openSync(mode: anyNamed('mode'))).thenReturn(mockRandomAccessFile);
await Cache.lock(); await Cache.lock();
Cache.checkLockAcquired(); Cache.checkLockAcquired();
Cache.releaseLockEarly();
}, overrides: <Type, Generator>{
FileSystem: () => mockFileSystem,
});
testUsingContext('throws tool exit when lockfile open fails', () async {
when(mockFileSystem.file(argThat(endsWith('lockfile')))).thenReturn(mockFile);
when(mockFile.openSync(mode: anyNamed('mode'))).thenThrow(const FileSystemException());
expect(() async => await Cache.lock(), throwsA(isA<ToolExit>()));
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
FileSystem: () => MockFileSystem(), FileSystem: () => mockFileSystem,
}); });
testUsingContext('should not throw when FLUTTER_ALREADY_LOCKED is set', () async { testUsingContext('should not throw when FLUTTER_ALREADY_LOCKED is set', () async {
...@@ -166,7 +188,7 @@ void main() { ...@@ -166,7 +188,7 @@ void main() {
expect(flattenNameSubdirs(Uri.parse('http://docs.flutter.io/foo/bar')), 'docs.flutter.io/foo/bar'); expect(flattenNameSubdirs(Uri.parse('http://docs.flutter.io/foo/bar')), 'docs.flutter.io/foo/bar');
expect(flattenNameSubdirs(Uri.parse('https://www.flutter.dev')), 'www.flutter.dev'); expect(flattenNameSubdirs(Uri.parse('https://www.flutter.dev')), 'www.flutter.dev');
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
FileSystem: () => MockFileSystem(), FileSystem: () => MemoryFileSystem(),
}); });
test('Unstable artifacts', () { test('Unstable artifacts', () {
...@@ -247,21 +269,8 @@ class FakeCachedArtifact extends EngineCachedArtifact { ...@@ -247,21 +269,8 @@ class FakeCachedArtifact extends EngineCachedArtifact {
List<String> getPackageDirs() => packageDirs; List<String> getPackageDirs() => packageDirs;
} }
class MockFileSystem extends ForwardingFileSystem { class MockFileSystem extends Mock implements FileSystem {}
MockFileSystem() : super(MemoryFileSystem()); class MockFile extends Mock implements File {}
@override
File file(dynamic path) {
return MockFile();
}
}
class MockFile extends Mock implements File {
@override
Future<RandomAccessFile> open({ FileMode mode = FileMode.read }) async {
return MockRandomAccessFile();
}
}
class MockRandomAccessFile extends Mock implements RandomAccessFile {} class MockRandomAccessFile extends Mock implements RandomAccessFile {}
class MockCachedArtifact extends Mock implements CachedArtifact {} class MockCachedArtifact extends Mock implements CachedArtifact {}
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'package:file/memory.dart'; import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/base/context.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
...@@ -69,6 +70,17 @@ void main() { ...@@ -69,6 +70,17 @@ void main() {
Platform: () => platform, Platform: () => platform,
}, initializeFlutterRoot: false); }, initializeFlutterRoot: false);
testUsingContext('throw tool exit if the version file cannot be written', () async {
final MockFlutterVersion version = FlutterVersion.instance;
when(version.ensureVersionFile()).thenThrow(const FileSystemException());
expect(() async => await runner.run(<String>['dummy']), throwsA(isA<ToolExit>()));
}, overrides: <Type, Generator>{
FileSystem: () => fs,
Platform: () => platform,
}, initializeFlutterRoot: false);
testUsingContext('works if --local-engine is specified and --local-engine-src-path is determined by sky_engine', () async { testUsingContext('works if --local-engine is specified and --local-engine-src-path is determined by sky_engine', () async {
fs.directory('$_kArbitraryEngineRoot/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/').createSync(recursive: true); fs.directory('$_kArbitraryEngineRoot/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/').createSync(recursive: true);
fs.directory('$_kArbitraryEngineRoot/src/out/host_debug').createSync(recursive: true); fs.directory('$_kArbitraryEngineRoot/src/out/host_debug').createSync(recursive: true);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment