diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index f94384eecd..dc77d0abf5 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/packages/flutter_tools/lib/src/base/process.dart @@ -231,16 +231,88 @@ Future runAsync( String workingDirectory, bool allowReentrantFlutter = false, Map environment, + Duration timeout, + int timeoutRetries = 0, }) async { _traceCommand(cmd, workingDirectory: workingDirectory); - final ProcessResult results = await processManager.run( - cmd, - workingDirectory: workingDirectory, - environment: _environment(allowReentrantFlutter, environment), - ); - final RunResult runResults = RunResult(results, cmd); - printTrace(runResults.toString()); - return runResults; + + // When there is no timeout, there's no need to kill a running process, so + // we can just use processManager.run(). + if (timeout == null) { + final ProcessResult results = await processManager.run( + cmd, + workingDirectory: workingDirectory, + environment: _environment(allowReentrantFlutter, environment), + ); + final RunResult runResults = RunResult(results, cmd); + printTrace(runResults.toString()); + return runResults; + } + + // When there is a timeout, we have to kill the running process, so we have + // to use processManager.start() through runCommand() above. + while (true) { + assert(timeoutRetries >= 0); + timeoutRetries = timeoutRetries - 1; + + final Process process = await runCommand( + cmd, + workingDirectory: workingDirectory, + allowReentrantFlutter: allowReentrantFlutter, + environment: environment, + ); + + final StringBuffer stdoutBuffer = StringBuffer(); + final StringBuffer stderrBuffer = StringBuffer(); + final Future stdoutFuture = process.stdout + .transform(const Utf8Decoder(reportErrors: false)) + .listen(stdoutBuffer.write) + .asFuture(null); + final Future stderrFuture = process.stderr + .transform(const Utf8Decoder(reportErrors: false)) + .listen(stderrBuffer.write) + .asFuture(null); + + int exitCode; + exitCode = await process.exitCode.timeout(timeout, onTimeout: () { + return null; + }); + + String stdoutString; + String stderrString; + try { + await Future.wait(>[stdoutFuture, stderrFuture]); + } catch (_) { + // Ignore errors on the process' stdout and stderr streams. Just capture + // whatever we got, and use the exit code + } + stdoutString = stdoutBuffer.toString(); + stderrString = stderrBuffer.toString(); + + final ProcessResult result = ProcessResult( + process.pid, exitCode ?? -1, stdoutString, stderrString); + final RunResult runResult = RunResult(result, cmd); + + // If the process did not timeout. We are done. + if (exitCode != null) { + printTrace(runResult.toString()); + return runResult; + } + + // The process timed out. Kill it. + processManager.killPid(process.pid); + + // If we are out of timeoutRetries, throw a ProcessException. + if (timeoutRetries < 0) { + throw ProcessException(cmd[0], cmd.sublist(1), + 'Process "${cmd[0]}" timed out: $runResult', exitCode); + } + + // Log the timeout with a trace message in verbose mode. + printTrace('Process "${cmd[0]}" timed out. $timeoutRetries attempts left: $runResult'); + } + + // Unreachable. } typedef RunResultChecker = bool Function(int); @@ -251,12 +323,16 @@ Future runCheckedAsync( bool allowReentrantFlutter = false, Map environment, RunResultChecker whiteListFailures, + Duration timeout, + int timeoutRetries = 0, }) async { final RunResult result = await runAsync( cmd, workingDirectory: workingDirectory, allowReentrantFlutter: allowReentrantFlutter, environment: environment, + timeout: timeout, + timeoutRetries: timeoutRetries, ); if (result.exitCode != 0) { if (whiteListFailures == null || !whiteListFailures(result.exitCode)) { diff --git a/packages/flutter_tools/lib/src/ios/xcodeproj.dart b/packages/flutter_tools/lib/src/ios/xcodeproj.dart index ccd5c02d7b..5acc1c132d 100644 --- a/packages/flutter_tools/lib/src/ios/xcodeproj.dart +++ b/packages/flutter_tools/lib/src/ios/xcodeproj.dart @@ -10,6 +10,7 @@ import '../artifacts.dart'; import '../base/context.dart'; import '../base/file_system.dart'; import '../base/io.dart'; +import '../base/logger.dart'; import '../base/os.dart'; import '../base/platform.dart'; import '../base/process.dart'; @@ -249,6 +250,8 @@ class XcodeProjectInterpreter { return _minorVersion; } + /// Synchronously retrieve xcode build settings. Prefer using the async + /// version below. Map getBuildSettings(String projectPath, String target) { try { final String out = runCheckedSync([ @@ -266,6 +269,41 @@ class XcodeProjectInterpreter { } } + /// Asynchronously retrieve xcode build settings. This one is preferred for + /// new call-sites. + Future> getBuildSettingsAsync( + String projectPath, String target, { + Duration timeout = const Duration(minutes: 1), + }) async { + final Status status = Status.withSpinner( + timeout: timeoutConfiguration.fastOperation, + ); + try { + // showBuildSettings is reported to ocassionally timeout. Here, we give it + // a lot of wiggle room (locally on Flutter Gallery, this takes ~1s). + // When there is a timeout, we retry once. + final RunResult result = await runCheckedAsync([ + _executable, + '-project', + fs.path.absolute(projectPath), + '-target', + target, + '-showBuildSettings', + ], + workingDirectory: projectPath, + timeout: timeout, + timeoutRetries: 1, + ); + final String out = result.stdout.trim(); + return parseXcodeBuildSettings(out); + } catch(error) { + printTrace('Unexpected failure to get the build settings: $error.'); + return const {}; + } finally { + status.stop(); + } + } + void cleanWorkspace(String workspacePath, String scheme) { runSync([ _executable, diff --git a/packages/flutter_tools/lib/src/macos/cocoapods.dart b/packages/flutter_tools/lib/src/macos/cocoapods.dart index ac006fac44..fac60cafe5 100644 --- a/packages/flutter_tools/lib/src/macos/cocoapods.dart +++ b/packages/flutter_tools/lib/src/macos/cocoapods.dart @@ -192,7 +192,7 @@ class CocoaPods { /// Ensures the given Xcode-based sub-project of a parent Flutter project /// contains a suitable `Podfile` and that its `Flutter/Xxx.xcconfig` files /// include pods configuration. - void setupPodfile(XcodeBasedProject xcodeProject) { + Future setupPodfile(XcodeBasedProject xcodeProject) async { if (!xcodeProjectInterpreter.isInstalled) { // Don't do anything for iOS when host platform doesn't support it. return; @@ -202,27 +202,29 @@ class CocoaPods { return; } final File podfile = xcodeProject.podfile; - if (!podfile.existsSync()) { - String podfileTemplateName; - if (xcodeProject is MacOSProject) { - podfileTemplateName = 'Podfile-macos'; - } else { - final bool isSwift = xcodeProjectInterpreter.getBuildSettings( - runnerProject.path, - 'Runner', - ).containsKey('SWIFT_VERSION'); - podfileTemplateName = isSwift ? 'Podfile-ios-swift' : 'Podfile-ios-objc'; - } - final File podfileTemplate = fs.file(fs.path.join( - Cache.flutterRoot, - 'packages', - 'flutter_tools', - 'templates', - 'cocoapods', - podfileTemplateName, - )); - podfileTemplate.copySync(podfile.path); + if (podfile.existsSync()) { + addPodsDependencyToFlutterXcconfig(xcodeProject); + return; } + String podfileTemplateName; + if (xcodeProject is MacOSProject) { + podfileTemplateName = 'Podfile-macos'; + } else { + final bool isSwift = (await xcodeProjectInterpreter.getBuildSettingsAsync( + runnerProject.path, + 'Runner', + )).containsKey('SWIFT_VERSION'); + podfileTemplateName = isSwift ? 'Podfile-ios-swift' : 'Podfile-ios-objc'; + } + final File podfileTemplate = fs.file(fs.path.join( + Cache.flutterRoot, + 'packages', + 'flutter_tools', + 'templates', + 'cocoapods', + podfileTemplateName, + )); + podfileTemplate.copySync(podfile.path); addPodsDependencyToFlutterXcconfig(xcodeProject); } diff --git a/packages/flutter_tools/lib/src/plugins.dart b/packages/flutter_tools/lib/src/plugins.dart index 021ec84dc3..db87e5f41a 100644 --- a/packages/flutter_tools/lib/src/plugins.dart +++ b/packages/flutter_tools/lib/src/plugins.dart @@ -371,7 +371,7 @@ Future injectPlugins(FlutterProject project, {bool checkProjects = false}) if (!project.isModule && (!checkProjects || subproject.existsSync())) { final CocoaPods cocoaPods = CocoaPods(); if (plugins.isNotEmpty) { - cocoaPods.setupPodfile(subproject); + await cocoaPods.setupPodfile(subproject); } /// The user may have a custom maintained Podfile that they're running `pod install` /// on themselves. diff --git a/packages/flutter_tools/lib/src/project.dart b/packages/flutter_tools/lib/src/project.dart index 1e99c899a2..dc2552c9a8 100644 --- a/packages/flutter_tools/lib/src/project.dart +++ b/packages/flutter_tools/lib/src/project.dart @@ -22,24 +22,29 @@ import 'ios/xcodeproj.dart' as xcode; import 'plugins.dart'; import 'template.dart'; -FlutterProjectFactory get projectFactory => context.get() ?? const FlutterProjectFactory(); +FlutterProjectFactory get projectFactory => context.get() ?? FlutterProjectFactory(); class FlutterProjectFactory { - const FlutterProjectFactory(); + FlutterProjectFactory(); + + final Map _projects = + {}; /// Returns a [FlutterProject] view of the given directory or a ToolExit error, /// if `pubspec.yaml` or `example/pubspec.yaml` is invalid. FlutterProject fromDirectory(Directory directory) { assert(directory != null); - final FlutterManifest manifest = FlutterProject._readManifest( - directory.childFile(bundle.defaultManifestPath).path, - ); - final FlutterManifest exampleManifest = FlutterProject._readManifest( - FlutterProject._exampleDirectory(directory) - .childFile(bundle.defaultManifestPath) - .path, - ); - return FlutterProject(directory, manifest, exampleManifest); + return _projects.putIfAbsent(directory.path, /* ifAbsent */ () { + final FlutterManifest manifest = FlutterProject._readManifest( + directory.childFile(bundle.defaultManifestPath).path, + ); + final FlutterManifest exampleManifest = FlutterProject._readManifest( + FlutterProject._exampleDirectory(directory) + .childFile(bundle.defaultManifestPath) + .path, + ); + return FlutterProject(directory, manifest, exampleManifest); + }); } } @@ -394,9 +399,14 @@ class IosProject implements XcodeBasedProject { Map get buildSettings { if (!xcode.xcodeProjectInterpreter.isInstalled) return null; - return xcode.xcodeProjectInterpreter.getBuildSettings(xcodeProject.path, _hostAppBundleName); + _buildSettings ??= + xcode.xcodeProjectInterpreter.getBuildSettings(xcodeProject.path, + _hostAppBundleName); + return _buildSettings; } + Map _buildSettings; + Future ensureReadyForPlatformSpecificTooling() async { _regenerateFromTemplateIfNeeded(); if (!_flutterLibRoot.existsSync()) diff --git a/packages/flutter_tools/test/general.shard/base/process_test.dart b/packages/flutter_tools/test/general.shard/base/process_test.dart index 8ffca5e785..6d23e467d3 100644 --- a/packages/flutter_tools/test/general.shard/base/process_test.dart +++ b/packages/flutter_tools/test/general.shard/base/process_test.dart @@ -12,7 +12,9 @@ import 'package:process/process.dart'; import '../../src/common.dart'; import '../../src/context.dart'; -import '../../src/mocks.dart' show MockProcess, MockProcessManager; +import '../../src/mocks.dart' show MockProcess, + MockProcessManager, + flakyProcessFactory; void main() { group('process exceptions', () { @@ -28,6 +30,7 @@ void main() { expect(() async => await runCheckedAsync(['false']), throwsA(isInstanceOf())); }, overrides: {ProcessManager: () => mockProcessManager}); }); + group('shutdownHooks', () { testUsingContext('runInExpectedOrder', () async { int i = 1; @@ -60,6 +63,7 @@ void main() { expect(cleanup, 4); }); }); + group('output formatting', () { MockProcessManager mockProcessManager; BufferLogger mockLogger; @@ -90,6 +94,49 @@ void main() { Platform: () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false, }); }); + + group('runAsync timeout and retry', () { + const Duration delay = Duration(seconds: 2); + MockProcessManager flakyProcessManager; + + setUp(() { + // MockProcessManager has an implementation of start() that returns the + // result of processFactory. + flakyProcessManager = MockProcessManager(); + flakyProcessManager.processFactory = flakyProcessFactory(1, delay: delay); + }); + + testUsingContext('flaky process fails without retry', () async { + final RunResult result = await runAsync( + ['dummy'], + timeout: delay + const Duration(seconds: 1), + ); + expect(result.exitCode, -9); + }, overrides: { + ProcessManager: () => flakyProcessManager, + }); + + testUsingContext('flaky process succeeds with retry', () async { + final RunResult result = await runAsync( + ['dummy'], + timeout: delay - const Duration(milliseconds: 500), + timeoutRetries: 1, + ); + expect(result.exitCode, 0); + }, overrides: { + ProcessManager: () => flakyProcessManager, + }); + + testUsingContext('flaky process generates ProcessException on timeout', () async { + expect(() async => await runAsync( + ['dummy'], + timeout: delay - const Duration(milliseconds: 500), + timeoutRetries: 0, + ), throwsA(isInstanceOf())); + }, overrides: { + ProcessManager: () => flakyProcessManager, + }); + }); } class PlainMockProcessManager extends Mock implements ProcessManager {} diff --git a/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart b/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart index ef36055a3e..05a611264f 100644 --- a/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart +++ b/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart @@ -17,19 +17,20 @@ import 'package:process/process.dart'; import '../../src/common.dart'; import '../../src/context.dart'; +import '../../src/mocks.dart' as mocks; import '../../src/pubspec_schema.dart'; const String xcodebuild = '/usr/bin/xcodebuild'; void main() { group('xcodebuild versioning', () { - MockProcessManager mockProcessManager; + mocks.MockProcessManager mockProcessManager; XcodeProjectInterpreter xcodeProjectInterpreter; FakePlatform macOS; FileSystem fs; setUp(() { - mockProcessManager = MockProcessManager(); + mockProcessManager = mocks.MockProcessManager(); xcodeProjectInterpreter = XcodeProjectInterpreter(); macOS = fakePlatform('macos'); fs = MemoryFileSystem(); @@ -149,6 +150,23 @@ void main() { .thenReturn(ProcessResult(0, 1, '', '')); expect(xcodeProjectInterpreter.getBuildSettings('', ''), const {}); }); + + testUsingContext('build settings flakes', () async { + const Duration delay = Duration(seconds: 1); + mockProcessManager.processFactory = + mocks.flakyProcessFactory(1, delay: delay + const Duration(seconds: 1)); + expect(await xcodeProjectInterpreter.getBuildSettingsAsync( + '', '', timeout: delay), + const {}); + // build settings times out and is killed once, then succeeds. + verify(mockProcessManager.killPid(any)).called(1); + // The verbose logs should tell us something timed out. + expect(testLogger.traceText, contains('timed out')); + }, overrides: { + Platform: () => macOS, + FileSystem: () => fs, + ProcessManager: () => mockProcessManager, + }); }); group('Xcode project properties', () { test('properties from default project can be parsed', () { diff --git a/packages/flutter_tools/test/general.shard/macos/cocoapods_test.dart b/packages/flutter_tools/test/general.shard/macos/cocoapods_test.dart index 6ebccdb2e8..6828bcaadc 100644 --- a/packages/flutter_tools/test/general.shard/macos/cocoapods_test.dart +++ b/packages/flutter_tools/test/general.shard/macos/cocoapods_test.dart @@ -164,7 +164,7 @@ void main() { group('Setup Podfile', () { testUsingContext('creates objective-c Podfile when not present', () async { - cocoaPodsUnderTest.setupPodfile(projectUnderTest.ios); + await cocoaPodsUnderTest.setupPodfile(projectUnderTest.ios); expect(projectUnderTest.ios.podfile.readAsStringSync(), 'Objective-C iOS podfile template'); }, overrides: { @@ -173,12 +173,13 @@ void main() { testUsingContext('creates swift Podfile if swift', () async { when(mockXcodeProjectInterpreter.isInstalled).thenReturn(true); - when(mockXcodeProjectInterpreter.getBuildSettings(any, any)).thenReturn({ + when(mockXcodeProjectInterpreter.getBuildSettingsAsync(any, any)) + .thenAnswer((_) async => { 'SWIFT_VERSION': '4.0', }); final FlutterProject project = FlutterProject.fromPath('project'); - cocoaPodsUnderTest.setupPodfile(project.ios); + await cocoaPodsUnderTest.setupPodfile(project.ios); expect(projectUnderTest.ios.podfile.readAsStringSync(), 'Swift iOS podfile template'); }, overrides: { @@ -188,7 +189,7 @@ void main() { testUsingContext('creates macOS Podfile when not present', () async { projectUnderTest.macos.xcodeProject.createSync(recursive: true); - cocoaPodsUnderTest.setupPodfile(projectUnderTest.macos); + await cocoaPodsUnderTest.setupPodfile(projectUnderTest.macos); expect(projectUnderTest.macos.podfile.readAsStringSync(), 'macOS podfile template'); }, overrides: { @@ -199,7 +200,7 @@ void main() { projectUnderTest.ios.podfile..createSync()..writeAsStringSync('Existing Podfile'); final FlutterProject project = FlutterProject.fromPath('project'); - cocoaPodsUnderTest.setupPodfile(project.ios); + await cocoaPodsUnderTest.setupPodfile(project.ios); expect(projectUnderTest.ios.podfile.readAsStringSync(), 'Existing Podfile'); }, overrides: { @@ -210,7 +211,7 @@ void main() { when(mockXcodeProjectInterpreter.isInstalled).thenReturn(false); final FlutterProject project = FlutterProject.fromPath('project'); - cocoaPodsUnderTest.setupPodfile(project.ios); + await cocoaPodsUnderTest.setupPodfile(project.ios); expect(projectUnderTest.ios.podfile.existsSync(), false); }, overrides: { @@ -228,7 +229,7 @@ void main() { ..writeAsStringSync('Existing release config'); final FlutterProject project = FlutterProject.fromPath('project'); - cocoaPodsUnderTest.setupPodfile(project.ios); + await cocoaPodsUnderTest.setupPodfile(project.ios); final String debugContents = projectUnderTest.ios.xcodeConfigFor('Debug').readAsStringSync(); expect(debugContents, contains( diff --git a/packages/flutter_tools/test/general.shard/project_test.dart b/packages/flutter_tools/test/general.shard/project_test.dart index d16d4951e0..33cd5f68c4 100644 --- a/packages/flutter_tools/test/general.shard/project_test.dart +++ b/packages/flutter_tools/test/general.shard/project_test.dart @@ -244,9 +244,11 @@ void main() { group('language', () { MockXcodeProjectInterpreter mockXcodeProjectInterpreter; MemoryFileSystem fs; + FlutterProjectFactory flutterProjectFactory; setUp(() { fs = MemoryFileSystem(); mockXcodeProjectInterpreter = MockXcodeProjectInterpreter(); + flutterProjectFactory = FlutterProjectFactory(); }); testInMemory('default host app language', () async { @@ -273,6 +275,7 @@ apply plugin: 'kotlin-android' }, overrides: { FileSystem: () => fs, XcodeProjectInterpreter: () => mockXcodeProjectInterpreter, + FlutterProjectFactory: () => flutterProjectFactory, }); }); @@ -280,10 +283,12 @@ apply plugin: 'kotlin-android' MemoryFileSystem fs; MockPlistUtils mockPlistUtils; MockXcodeProjectInterpreter mockXcodeProjectInterpreter; + FlutterProjectFactory flutterProjectFactory; setUp(() { fs = MemoryFileSystem(); mockPlistUtils = MockPlistUtils(); mockXcodeProjectInterpreter = MockXcodeProjectInterpreter(); + flutterProjectFactory = FlutterProjectFactory(); }); void testWithMocks(String description, Future testMethod()) { @@ -291,6 +296,7 @@ apply plugin: 'kotlin-android' FileSystem: () => fs, PlistParser: () => mockPlistUtils, XcodeProjectInterpreter: () => mockXcodeProjectInterpreter, + FlutterProjectFactory: () => flutterProjectFactory, }); } @@ -425,9 +431,11 @@ apply plugin: 'kotlin-android' group('Regression test for invalid pubspec', () { Testbed testbed; + FlutterProjectFactory flutterProjectFactory; setUp(() { testbed = Testbed(); + flutterProjectFactory = FlutterProjectFactory(); }); test('Handles asking for builders from an invalid pubspec', () => testbed.run(() { @@ -439,6 +447,8 @@ apply plugin: 'kotlin-android' final FlutterProject flutterProject = FlutterProject.current(); expect(flutterProject.builders, null); + }, overrides: { + FlutterProjectFactory: () => flutterProjectFactory, })); test('Handles asking for builders from a trivial pubspec', () => testbed.run(() { @@ -451,6 +461,8 @@ name: foo_bar final FlutterProject flutterProject = FlutterProject.current(); expect(flutterProject.builders, null); + }, overrides: { + FlutterProjectFactory: () => flutterProjectFactory, })); }); } @@ -510,12 +522,16 @@ void testInMemory(String description, Future testMethod()) { .childDirectory('packages') .childDirectory('flutter_tools') .childDirectory('schema'), testFileSystem); + + final FlutterProjectFactory flutterProjectFactory = FlutterProjectFactory(); + testUsingContext( description, testMethod, overrides: { FileSystem: () => testFileSystem, Cache: () => Cache(), + FlutterProjectFactory: () => flutterProjectFactory, }, ); } diff --git a/packages/flutter_tools/test/src/context.dart b/packages/flutter_tools/test/src/context.dart index 090680e32f..3f516b6712 100644 --- a/packages/flutter_tools/test/src/context.dart +++ b/packages/flutter_tools/test/src/context.dart @@ -335,6 +335,15 @@ class FakeXcodeProjectInterpreter implements XcodeProjectInterpreter { return {}; } + @override + Future> getBuildSettingsAsync( + String projectPath, + String target, { + Duration timeout = const Duration(minutes: 1), + }) async { + return {}; + } + @override void cleanWorkspace(String workspacePath, String scheme) { } diff --git a/packages/flutter_tools/test/src/mocks.dart b/packages/flutter_tools/test/src/mocks.dart index 92d184d01e..feb71d9790 100644 --- a/packages/flutter_tools/test/src/mocks.dart +++ b/packages/flutter_tools/test/src/mocks.dart @@ -182,6 +182,26 @@ class MockProcessManager extends Mock implements ProcessManager { } } +/// A function that generates a process factory that gives processes that fail +/// a given number of times before succeeding. The returned processes will +/// fail after a delay if one is supplied. +ProcessFactory flakyProcessFactory(int flakes, {Duration delay}) { + int flakesLeft = flakes; + return (List command) { + if (flakesLeft == 0) { + return MockProcess(exitCode: Future.value(0)); + } + flakesLeft = flakesLeft - 1; + Future exitFuture; + if (delay == null) { + exitFuture = Future.value(-9); + } else { + exitFuture = Future.delayed(delay, () => Future.value(-9)); + } + return MockProcess(exitCode: exitFuture); + }; +} + /// A process that exits successfully with no output and ignores all input. class MockProcess extends Mock implements Process { MockProcess({