Unverified Commit c8466d04 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] handle case where file is deleted by other program or running...

[flutter_tools] handle case where file is deleted by other program or running on read only volume (#66708)

* [flutter_tools] handle case where file is deleted by other program

* Add test cases

* Update file_system.dart

* Update file_system_test.dart

* fix import

* make a static on ErrorHandligFS

* add support for no exit on failure

* address comments

* update doc comment to file or directory
parent 64c845c5
...@@ -63,6 +63,40 @@ class ErrorHandlingFileSystem extends ForwardingFileSystem { ...@@ -63,6 +63,40 @@ class ErrorHandlingFileSystem extends ForwardingFileSystem {
} }
} }
/// Delete the file or directory and return true if it exists, take no
/// action and return false if it does not.
///
/// This method should be prefered to checking if it exists and
/// then deleting, because it handles the edge case where the file or directory
/// is deleted by a different program between the two calls.
static bool deleteIfExists(FileSystemEntity file, {bool recursive = false}) {
if (!file.existsSync()) {
return false;
}
try {
file.deleteSync(recursive: recursive);
} on FileSystemException catch (err) {
// Certain error codes indicate the file could not be found. It could have
// been deleted by a different program while the tool was running.
// if it still exists, the file likely exists on a read-only volume.
//
// On windows this is error code 2: ERROR_FILE_NOT_FOUND, and on
// macOS/Linux it is error code 2/ENOENT: No such file or directory.
const int kSystemCannotFindFile = 2;
if (err?.osError?.errorCode != kSystemCannotFindFile || _noExitOnFailure) {
rethrow;
}
if (file.existsSync()) {
throwToolExit(
'The Flutter tool tried to delete the file or directory ${file.path} but was '
'unable to. This may be due to the file and/or project\'s location on a read-only '
'volume. Consider relocating the project and trying again',
);
}
}
return true;
}
static bool _noExitOnFailure = false; static bool _noExitOnFailure = false;
@override @override
......
...@@ -10,6 +10,7 @@ import 'package:pool/pool.dart'; ...@@ -10,6 +10,7 @@ import 'package:pool/pool.dart';
import 'package:process/process.dart'; import 'package:process/process.dart';
import '../artifacts.dart'; import '../artifacts.dart';
import '../base/error_handling_io.dart';
import '../base/file_system.dart'; import '../base/file_system.dart';
import '../base/logger.dart'; import '../base/logger.dart';
import '../base/platform.dart'; import '../base/platform.dart';
...@@ -160,9 +161,7 @@ abstract class Target { ...@@ -160,9 +161,7 @@ abstract class Target {
/// Invoke to remove the stamp file if the [buildAction] threw an exception. /// Invoke to remove the stamp file if the [buildAction] threw an exception.
void clearStamp(Environment environment) { void clearStamp(Environment environment) {
final File stamp = _findStampFile(environment); final File stamp = _findStampFile(environment);
if (stamp.existsSync()) { ErrorHandlingFileSystem.deleteIfExists(stamp);
stamp.deleteSync();
}
} }
void _writeStamp( void _writeStamp(
...@@ -697,9 +696,7 @@ class FlutterBuildSystem extends BuildSystem { ...@@ -697,9 +696,7 @@ class FlutterBuildSystem extends BuildSystem {
for (final String lastOutput in lastOutputs) { for (final String lastOutput in lastOutputs) {
if (!currentOutputs.containsKey(lastOutput)) { if (!currentOutputs.containsKey(lastOutput)) {
final File lastOutputFile = fileSystem.file(lastOutput); final File lastOutputFile = fileSystem.file(lastOutput);
if (lastOutputFile.existsSync()) { ErrorHandlingFileSystem.deleteIfExists(lastOutputFile);
lastOutputFile.deleteSync();
}
} }
} }
} }
...@@ -820,9 +817,7 @@ class _BuildInstance { ...@@ -820,9 +817,7 @@ class _BuildInstance {
continue; continue;
} }
final File previousFile = fileSystem.file(previousOutput); final File previousFile = fileSystem.file(previousOutput);
if (previousFile.existsSync()) { ErrorHandlingFileSystem.deleteIfExists(previousFile);
previousFile.deleteSync();
}
} }
} on Exception catch (exception, stackTrace) { } on Exception catch (exception, stackTrace) {
// TODO(jonahwilliams): throw specific exception for expected errors to mark // TODO(jonahwilliams): throw specific exception for expected errors to mark
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import '../base/error_handling_io.dart';
import '../base/file_system.dart'; import '../base/file_system.dart';
import '../base/logger.dart'; import '../base/logger.dart';
...@@ -26,9 +27,7 @@ class DepfileService { ...@@ -26,9 +27,7 @@ class DepfileService {
/// exist. /// exist.
void writeToFile(Depfile depfile, File output) { void writeToFile(Depfile depfile, File output) {
if (depfile.inputs.isEmpty || depfile.outputs.isEmpty) { if (depfile.inputs.isEmpty || depfile.outputs.isEmpty) {
if (output.existsSync()) { ErrorHandlingFileSystem.deleteIfExists(output);
output.deleteSync();
}
return; return;
} }
final StringBuffer buffer = StringBuffer(); final StringBuffer buffer = StringBuffer();
......
...@@ -10,6 +10,7 @@ import 'package:process/process.dart'; ...@@ -10,6 +10,7 @@ import 'package:process/process.dart';
import 'android/gradle_utils.dart'; import 'android/gradle_utils.dart';
import 'base/common.dart'; import 'base/common.dart';
import 'base/error_handling_io.dart';
import 'base/file_system.dart'; import 'base/file_system.dart';
import 'base/io.dart' show HttpClient, HttpClientRequest, HttpClientResponse, HttpStatus, ProcessException, SocketException; import 'base/io.dart' show HttpClient, HttpClientRequest, HttpClientResponse, HttpStatus, ProcessException, SocketException;
import 'base/logger.dart'; import 'base/logger.dart';
...@@ -436,9 +437,7 @@ class Cache { ...@@ -436,9 +437,7 @@ class Cache {
getStampFileFor('flutter_tools').deleteSync(); getStampFileFor('flutter_tools').deleteSync();
for (final ArtifactSet artifact in _artifacts) { for (final ArtifactSet artifact in _artifacts) {
final File file = getStampFileFor(artifact.stampName); final File file = getStampFileFor(artifact.stampName);
if (file.existsSync()) { ErrorHandlingFileSystem.deleteIfExists(file);
file.deleteSync();
}
} }
} on FileSystemException catch (err) { } on FileSystemException catch (err) {
_logger.printError('Failed to delete some stamp files: $err'); _logger.printError('Failed to delete some stamp files: $err');
......
...@@ -7,6 +7,7 @@ import 'package:meta/meta.dart'; ...@@ -7,6 +7,7 @@ import 'package:meta/meta.dart';
import 'package:process/process.dart'; import 'package:process/process.dart';
import '../base/common.dart'; import '../base/common.dart';
import '../base/error_handling_io.dart';
import '../base/file_system.dart'; import '../base/file_system.dart';
import '../base/io.dart'; import '../base/io.dart';
import '../base/logger.dart'; import '../base/logger.dart';
...@@ -307,9 +308,7 @@ class CocoaPods { ...@@ -307,9 +308,7 @@ class CocoaPods {
/// Ensures that pod install is deemed needed on next check. /// Ensures that pod install is deemed needed on next check.
void invalidatePodInstallOutput(XcodeBasedProject xcodeProject) { void invalidatePodInstallOutput(XcodeBasedProject xcodeProject) {
final File manifestLock = xcodeProject.podManifestLock; final File manifestLock = xcodeProject.podManifestLock;
if (manifestLock.existsSync()) { ErrorHandlingFileSystem.deleteIfExists(manifestLock);
manifestLock.deleteSync();
}
} }
// Check if you need to run pod install. // Check if you need to run pod install.
......
...@@ -9,6 +9,7 @@ import 'package:yaml/yaml.dart'; ...@@ -9,6 +9,7 @@ import 'package:yaml/yaml.dart';
import 'android/gradle.dart'; import 'android/gradle.dart';
import 'base/common.dart'; import 'base/common.dart';
import 'base/error_handling_io.dart';
import 'base/file_system.dart'; import 'base/file_system.dart';
import 'convert.dart'; import 'convert.dart';
import 'dart/package_map.dart'; import 'dart/package_map.dart';
...@@ -435,11 +436,7 @@ const String _kFlutterPluginsDependenciesKey = 'dependencies'; ...@@ -435,11 +436,7 @@ const String _kFlutterPluginsDependenciesKey = 'dependencies';
bool _writeFlutterPluginsList(FlutterProject project, List<Plugin> plugins) { bool _writeFlutterPluginsList(FlutterProject project, List<Plugin> plugins) {
final File pluginsFile = project.flutterPluginsDependenciesFile; final File pluginsFile = project.flutterPluginsDependenciesFile;
if (plugins.isEmpty) { if (plugins.isEmpty) {
if (pluginsFile.existsSync()) { return ErrorHandlingFileSystem.deleteIfExists(pluginsFile);
pluginsFile.deleteSync();
return true;
}
return false;
} }
final String iosKey = project.ios.pluginConfigKey; final String iosKey = project.ios.pluginConfigKey;
...@@ -506,11 +503,7 @@ List<dynamic> _createPluginLegacyDependencyGraph(List<Plugin> plugins) { ...@@ -506,11 +503,7 @@ List<dynamic> _createPluginLegacyDependencyGraph(List<Plugin> plugins) {
bool _writeFlutterPluginsListLegacy(FlutterProject project, List<Plugin> plugins) { bool _writeFlutterPluginsListLegacy(FlutterProject project, List<Plugin> plugins) {
final File pluginsFile = project.flutterPluginsFile; final File pluginsFile = project.flutterPluginsFile;
if (plugins.isEmpty) { if (plugins.isEmpty) {
if (pluginsFile.existsSync()) { return ErrorHandlingFileSystem.deleteIfExists(pluginsFile);
pluginsFile.deleteSync();
return true;
}
return false;
} }
const String info = 'This is a generated file; do not edit or check into version control.'; const String info = 'This is a generated file; do not edit or check into version control.';
...@@ -1051,9 +1044,7 @@ Future<void> _writeWebPluginRegistrant(FlutterProject project, List<Plugin> plug ...@@ -1051,9 +1044,7 @@ Future<void> _writeWebPluginRegistrant(FlutterProject project, List<Plugin> plug
final String filePath = globals.fs.path.join(registryDirectory, 'generated_plugin_registrant.dart'); final String filePath = globals.fs.path.join(registryDirectory, 'generated_plugin_registrant.dart');
if (webPlugins.isEmpty) { if (webPlugins.isEmpty) {
final File file = globals.fs.file(filePath); final File file = globals.fs.file(filePath);
if (file.existsSync()) { return ErrorHandlingFileSystem.deleteIfExists(file);
file.deleteSync();
}
} else { } else {
_renderTemplateToFile( _renderTemplateToFile(
_dartPluginRegistryTemplate, _dartPluginRegistryTemplate,
......
...@@ -3,14 +3,15 @@ ...@@ -3,14 +3,15 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async'; import 'dart:async';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'package:vm_service/vm_service.dart' as vm_service; import 'package:vm_service/vm_service.dart' as vm_service;
import 'base/common.dart'; import 'base/common.dart';
import 'base/error_handling_io.dart';
import 'base/file_system.dart'; import 'base/file_system.dart';
import 'base/logger.dart'; import 'base/logger.dart';
import 'base/utils.dart'; import 'base/utils.dart';
import 'vmservice.dart'; import 'vmservice.dart';
// Names of some of the Timeline events we care about. // Names of some of the Timeline events we care about.
...@@ -94,9 +95,7 @@ Future<void> downloadStartupTrace(vm_service.VmService vmService, { ...@@ -94,9 +95,7 @@ Future<void> downloadStartupTrace(vm_service.VmService vmService, {
final File traceInfoFile = output.childFile('start_up_info.json'); final File traceInfoFile = output.childFile('start_up_info.json');
// Delete old startup data, if any. // Delete old startup data, if any.
if (traceInfoFile.existsSync()) { ErrorHandlingFileSystem.deleteIfExists(traceInfoFile);
traceInfoFile.deleteSync();
}
// Create "build" directory, if missing. // Create "build" directory, if missing.
if (!traceInfoFile.parent.existsSync()) { if (!traceInfoFile.parent.existsSync()) {
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
// 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 'package:flutter_tools/src/base/error_handling_io.dart';
import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/globals.dart' as globals;
import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/common.dart';
...@@ -246,9 +247,7 @@ flutter_project:lib/ ...@@ -246,9 +247,7 @@ flutter_project:lib/
exitCode: 1, exitCode: 1,
); );
} finally { } finally {
if (optionsFile.existsSync()) { ErrorHandlingFileSystem.deleteIfExists(optionsFile);
optionsFile.deleteSync();
}
} }
}); });
......
...@@ -6,6 +6,7 @@ import 'dart:convert'; ...@@ -6,6 +6,7 @@ import 'dart:convert';
import 'package:args/command_runner.dart'; import 'package:args/command_runner.dart';
import 'package:flutter_tools/src/base/bot_detector.dart'; import 'package:flutter_tools/src/base/bot_detector.dart';
import 'package:flutter_tools/src/base/error_handling_io.dart';
import 'package:flutter_tools/src/base/file_system.dart' hide IOSink; import 'package:flutter_tools/src/base/file_system.dart' hide IOSink;
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/cache.dart';
...@@ -182,9 +183,7 @@ void main() { ...@@ -182,9 +183,7 @@ void main() {
].expand<String>((List<String> list) => list); ].expand<String>((List<String> list) => list);
for (final String path in allFiles) { for (final String path in allFiles) {
final File file = globals.fs.file(globals.fs.path.join(projectPath, path)); final File file = globals.fs.file(globals.fs.path.join(projectPath, path));
if (file.existsSync()) { ErrorHandlingFileSystem.deleteIfExists(file);
file.deleteSync();
}
} }
} }
......
...@@ -111,6 +111,58 @@ void setupDirectoryMocks({ ...@@ -111,6 +111,58 @@ void setupDirectoryMocks({
} }
void main() { void main() {
testWithoutContext('deleteIfExists does not delete if file does not exist', () {
final File file = MockFile();
when(file.existsSync()).thenReturn(false);
expect(ErrorHandlingFileSystem.deleteIfExists(file), false);
});
testWithoutContext('deleteIfExists deletes if file exists', () {
final File file = MockFile();
when(file.existsSync()).thenReturn(true);
expect(ErrorHandlingFileSystem.deleteIfExists(file), true);
});
testWithoutContext('deleteIfExists handles separate program deleting file', () {
final File file = MockFile();
bool exists = true;
// Return true for the first call, false for any subsequent calls.
when(file.existsSync()).thenAnswer((Invocation _) {
final bool result = exists;
exists = false;
return result;
});
when(file.deleteSync(recursive: false))
.thenThrow(const FileSystemException('', '', OSError('', 2)));
expect(ErrorHandlingFileSystem.deleteIfExists(file), true);
});
testWithoutContext('deleteIfExists throws tool exit if file exists on read-only volume', () {
final File file = MockFile();
when(file.existsSync()).thenReturn(true);
when(file.deleteSync(recursive: false))
.thenThrow(const FileSystemException('', '', OSError('', 2)));
expect(() => ErrorHandlingFileSystem.deleteIfExists(file), throwsA(isA<ToolExit>()));
});
testWithoutContext('deleteIfExists does not tool exit if file exists on read-only '
'volume and it is run under noExitOnFailure', () {
final File file = MockFile();
when(file.existsSync()).thenReturn(true);
when(file.deleteSync(recursive: false))
.thenThrow(const FileSystemException('', '', OSError('', 2)));
expect(() {
ErrorHandlingFileSystem.noExitOnFailure(() {
ErrorHandlingFileSystem.deleteIfExists(file);
});
}, throwsA(isA<FileSystemException>()));
});
group('throws ToolExit on Windows', () { group('throws ToolExit on Windows', () {
const int kDeviceFull = 112; const int kDeviceFull = 112;
const int kUserMappedSectionOpened = 1224; const int kUserMappedSectionOpened = 1224;
......
...@@ -180,3 +180,4 @@ void main() { ...@@ -180,3 +180,4 @@ void main() {
} }
class MockIoProcessSignal extends Mock implements io.ProcessSignal {} class MockIoProcessSignal extends Mock implements io.ProcessSignal {}
class MockFile extends Mock implements File {}
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