Don't indefinitely persist file hashes, handle more error conditions (#43602)

packages/flutter_tools/lib/src/build_system/build_system.dart

@@ -406,7 +406,7 @@ class BuildSystem {
     environment.outputDir.createSync(recursive: true);
 
     // Load file hash store from previous builds.
-    final FileHashStore fileCache = FileHashStore(environment)
+    final FileHashStore fileCache = FileHashStore(environment, fs)
       ..initialize();
 
     // Perform sanity checks on build.

packages/flutter_tools/lib/src/build_system/file_hash_store.dart

@@ -69,12 +69,12 @@ class FileHash {
 /// operation, and persisted to cache in the root build directory.
 ///
 /// The format of the file store is subject to change and not part of its API.
-///
-// TODO(jonahwilliams): find a better way to clear out old entries, perhaps
-// track the last access or modification date?
 class FileHashStore {
-  FileHashStore(this.environment);
+  FileHashStore(this.environment, this.fileSystem) :
+    _cachePath = environment.buildDir.childFile(_kFileCache).path;
 
+  final FileSystem fileSystem;
+  final String _cachePath;
   final Environment environment;
   final HashMap<String, String> previousHashes = HashMap<String, String>();
   final HashMap<String, String> currentHashes = HashMap<String, String>();
@@ -88,20 +88,33 @@ class FileHashStore {
   /// Read file hashes from disk.
   void initialize() {
     printTrace('Initializing file store');
-    if (!_cacheFile.existsSync()) {
+    final File cacheFile = fileSystem.file(_cachePath);
+    if (!cacheFile.existsSync()) {
       return;
     }
-    final List<int> data = _cacheFile.readAsBytesSync();
+    List<int> data;
+    try {
+      data = cacheFile.readAsBytesSync();
+    } on FileSystemException catch (err) {
+      printError(
+        'Failed to read file store at ${cacheFile.path} due to $err.\n'
+        'Build artifacts will not be cached. Try clearing the cache directories '
+        'with "flutter clean"',
+      );
+      return;
+    }
+
     FileStorage fileStorage;
     try {
       fileStorage = FileStorage.fromBuffer(data);
     } catch (err) {
       printTrace('Filestorage format changed');
-      _cacheFile.deleteSync();
+      cacheFile.deleteSync();
       return;
     }
     if (fileStorage.version != _kVersion) {
-      _cacheFile.deleteSync();
+      printTrace('file cache format updating, clearing old hashes.');
+      cacheFile.deleteSync();
       return;
     }
     for (FileHash fileHash in fileStorage.files) {
@@ -113,15 +126,12 @@ class FileHashStore {
   /// Persist file hashes to disk.
   void persist() {
     printTrace('Persisting file store');
-    final File file = _cacheFile;
-    if (!file.existsSync()) {
-      file.createSync(recursive: true);
+    final File cacheFile = fileSystem.file(_cachePath);
+    if (!cacheFile.existsSync()) {
+      cacheFile.createSync(recursive: true);
     }
     final List<FileHash> fileHashes = <FileHash>[];
     for (MapEntry<String, String> entry in currentHashes.entries) {
-      previousHashes[entry.key] = entry.value;
-    }
-    for (MapEntry<String, String> entry in previousHashes.entries) {
       fileHashes.add(FileHash(entry.key, entry.value));
     }
     final FileStorage fileStorage = FileStorage(
@@ -129,7 +139,15 @@ class FileHashStore {
       fileHashes,
     );
     final Uint8List buffer = fileStorage.toBuffer();
-    file.writeAsBytesSync(buffer);
+    try {
+      cacheFile.writeAsBytesSync(buffer);
+    } on FileSystemException catch (err) {
+      printError(
+        'Failed to persist file store at ${cacheFile.path} due to $err.\n'
+        'Build artifacts will not be cached. Try clearing the cache directories '
+        'with "flutter clean"',
+      );
+    }
     printTrace('Done persisting file store');
   }
 
@@ -166,6 +184,4 @@ class FileHashStore {
       resource.release();
     }
   }
-
-  File get _cacheFile => environment.buildDir.childFile(_kFileCache);
 }

packages/flutter_tools/test/general.shard/build_system/filecache_test.dart

@@ -3,8 +3,11 @@
 // found in the LICENSE file.
 
 import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/build_system/build_system.dart';
 import 'package:flutter_tools/src/build_system/file_hash_store.dart';
+import 'package:flutter_tools/src/globals.dart';
+import 'package:mockito/mockito.dart';
 
 import '../../src/common.dart';
 import '../../src/testbed.dart';
@@ -25,7 +28,7 @@ void main() {
   });
 
   test('Initializes file cache', () => testbed.run(() {
-    final FileHashStore fileCache = FileHashStore(environment);
+    final FileHashStore fileCache = FileHashStore(environment, fs);
     fileCache.initialize();
     fileCache.persist();
 
@@ -43,7 +46,7 @@ void main() {
     final File file = fs.file('foo.dart')
       ..createSync()
       ..writeAsStringSync('hello');
-    final FileHashStore fileCache = FileHashStore(environment);
+    final FileHashStore fileCache = FileHashStore(environment, fs);
     fileCache.initialize();
     await fileCache.hashFiles(<File>[file]);
     fileCache.persist();
@@ -56,7 +59,7 @@ void main() {
     expect(fileStorage.files.single.path, file.path);
 
 
-    final FileHashStore newFileCache = FileHashStore(environment);
+    final FileHashStore newFileCache = FileHashStore(environment, fs);
     newFileCache.initialize();
     expect(newFileCache.currentHashes, isEmpty);
     expect(newFileCache.previousHashes['foo.dart'],  currentHash);
@@ -73,7 +76,7 @@ void main() {
     final File file = fs.file('foo.dart')
       ..createSync()
       ..writeAsStringSync('hello');
-    final FileHashStore fileCache = FileHashStore(environment);
+    final FileHashStore fileCache = FileHashStore(environment, fs);
     fileCache.initialize();
     environment.buildDir.deleteSync(recursive: true);
 
@@ -83,7 +86,7 @@ void main() {
   }));
 
   test('handles hashing missing files', () => testbed.run(() async {
-    final FileHashStore fileCache = FileHashStore(environment);
+    final FileHashStore fileCache = FileHashStore(environment, fs);
     fileCache.initialize();
 
     final List<File> results = await fileCache.hashFiles(<File>[fs.file('hello.dart')]);
@@ -92,4 +95,45 @@ void main() {
     expect(results.single.path, 'hello.dart');
     expect(fileCache.currentHashes, isNot(contains(fs.path.absolute('hello.dart'))));
   }));
+
+  test('handles failure to persist file cache', () => testbed.run(() async {
+    final BufferLogger bufferLogger = logger;
+    final FakeForwardingFileSystem fakeForwardingFileSystem = FakeForwardingFileSystem(fs);
+    final FileHashStore fileCache = FileHashStore(environment, fakeForwardingFileSystem);
+    final String cacheFile = environment.buildDir.childFile('.filecache').path;
+    final MockFile mockFile = MockFile();
+    when(mockFile.writeAsBytesSync(any)).thenThrow(const FileSystemException('Out of space!'));
+    when(mockFile.existsSync()).thenReturn(true);
+
+    fileCache.initialize();
+    fakeForwardingFileSystem.files[cacheFile] = mockFile;
+    fileCache.persist();
+
+    expect(bufferLogger.errorText, contains('Out of space!'));
+  }));
+
+  test('handles failure to restore file cache', () => testbed.run(() async {
+    final BufferLogger bufferLogger = logger;
+    final FakeForwardingFileSystem fakeForwardingFileSystem = FakeForwardingFileSystem(fs);
+    final FileHashStore fileCache = FileHashStore(environment, fakeForwardingFileSystem);
+    final String cacheFile = environment.buildDir.childFile('.filecache').path;
+    final MockFile mockFile = MockFile();
+    when(mockFile.readAsBytesSync()).thenThrow(const FileSystemException('Out of space!'));
+    when(mockFile.existsSync()).thenReturn(true);
+
+    fakeForwardingFileSystem.files[cacheFile] = mockFile;
+    fileCache.initialize();
+
+    expect(bufferLogger.errorText, contains('Out of space!'));
+  }));
+}
+
+class FakeForwardingFileSystem extends ForwardingFileSystem {
+  FakeForwardingFileSystem(FileSystem fileSystem) : super(fileSystem);
+
+  final Map<String, FileSystemEntity> files = <String, FileSystemEntity>{};
+
+  @override
+  File file(dynamic path) => files[path] ?? super.file(path);
 }
+class MockFile extends Mock implements File {}