From 82ecbb5cbd01e391e988baca328654269857be6a Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Fri, 13 Dec 2024 10:51:20 -0800 Subject: [PATCH] Refactor gradle task runner to share error handler code (#159452) fixes https://github.com/flutter/flutter/issues/153893 ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Gray Mackall <34871572+gmackall@users.noreply.github.com> --- .../flutter_tools/lib/src/android/gradle.dart | 402 ++++++++++-------- .../android/android_gradle_builder_test.dart | 3 +- 2 files changed, 216 insertions(+), 189 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/gradle.dart b/packages/flutter_tools/lib/src/android/gradle.dart index f2cc1e95f8..df69207bdf 100644 --- a/packages/flutter_tools/lib/src/android/gradle.dart +++ b/packages/flutter_tools/lib/src/android/gradle.dart @@ -53,6 +53,8 @@ final RegExp _kBuildVariantRegex = RegExp('^BuildVariant: (?<$_kBuildVariantRege const String _kBuildVariantRegexGroupName = 'variant'; const String _kBuildVariantTaskName = 'printBuildVariants'; +typedef _OutputParser = void Function(String line); + String _getOutputAppLinkSettingsTaskFor(String buildVariant) { return _taskForBuildVariant('output', buildVariant, 'AppLinkSettings'); } @@ -264,32 +266,135 @@ class AndroidGradleBuilder implements AndroidBuilder { ); } - Future _runGradleTask( + Future _runGradleTask( String taskName, { List options = const [], - required FlutterProject project + required FlutterProject project, + required List localGradleErrors, + required String gradleExecutablePath, + int retry = 0, + VoidCallback? preRunTask, + VoidCallback? postRunTask, + int? maxRetries, + _OutputParser? outputParser, }) async { + final bool usesAndroidX = isAppUsingAndroidX(project.android.hostAppGradleRoot); + if (usesAndroidX) { + _analytics.send(Event.flutterBuildInfo(label: 'app-using-android-x', buildType: 'gradle')); + } else if (!usesAndroidX) { + _analytics.send(Event.flutterBuildInfo(label: 'app-not-using-android-x', buildType: 'gradle')); + + _logger.printStatus("${_logger.terminal.warningMark} Your app isn't using AndroidX.", emphasis: true); + _logger.printStatus( + 'To avoid potential build failures, you can quickly migrate your app ' + 'by following the steps on https://goo.gl/CP92wY .', + indent: 4, + ); + } + + GradleHandledError? detectedGradleError; + String? detectedGradleErrorLine; + String? consumeLog(String line) { + if (outputParser != null) { + outputParser(line); + } + // The log lines that trigger incompatibleKotlinVersionHandler don't + // always indicate an error, and there are times that that handler + // covers up a more important error handler. Uniquely set it to be + // the lowest priority handler by allowing it to be overridden. + if (detectedGradleError != null + && detectedGradleError != incompatibleKotlinVersionHandler) { + // Pipe stdout/stderr from Gradle. + return line; + } + for (final GradleHandledError gradleError in localGradleErrors) { + if (gradleError.test(line)) { + detectedGradleErrorLine = line; + detectedGradleError = gradleError; + // The first error match wins. + break; + } + } + // Pipe stdout/stderr from Gradle. + return line; + } final Status status = _logger.startProgress( "Running Gradle task '$taskName'...", ); final List command = [ - _gradleUtils.getExecutable(project), + gradleExecutablePath, ...options, // suppresses gradle output. taskName, ]; + preRunTask?.call(); - RunResult result; + int exitCode = 1; try { - result = await _processUtils.run( + exitCode = await _processUtils.stream( command, workingDirectory: project.android.hostAppGradleRoot.path, allowReentrantFlutter: true, environment: _java?.environment, + mapFunction: consumeLog, ); + } on ProcessException catch (exception) { + consumeLog(exception.toString()); + // Rethrow the exception if the error isn't handled by any of the + // `localGradleErrors`. + if (detectedGradleError == null) { + rethrow; + } } finally { status.stop(); } - return result; + postRunTask?.call(); + if (exitCode != 0) { + if (detectedGradleError == null) { + _analytics.send(Event.flutterBuildInfo(label: 'gradle-unknown-failure', buildType: 'gradle')); + return exitCode; + } + final GradleBuildStatus status = await detectedGradleError!.handler( + line: detectedGradleErrorLine!, + project: project, + usesAndroidX: usesAndroidX, + ); + + if (maxRetries == null || retry < maxRetries) { + switch (status) { + case GradleBuildStatus.retry: + // Use binary exponential backoff before retriggering the build. + // The expected wait times are: 100ms, 200ms, 400ms, and so on... + final int waitTime = min(pow(2, retry).toInt() * 100, kMaxRetryTime.inMicroseconds); + retry += 1; + _logger.printStatus('Retrying Gradle Build: #$retry, wait time: ${waitTime}ms'); + await Future.delayed(Duration(milliseconds: waitTime)); + exitCode = await _runGradleTask( + taskName, + options: options, + preRunTask: preRunTask, + postRunTask: postRunTask, + localGradleErrors: localGradleErrors, + gradleExecutablePath: gradleExecutablePath, + retry: retry, + project: project, + maxRetries: maxRetries, + ); + if (exitCode == 0) { + final String successEventLabel = 'gradle-${detectedGradleError! + .eventLabel}-success'; + _analytics.send(Event.flutterBuildInfo( + label: successEventLabel, buildType: 'gradle')); + return exitCode; + } + case GradleBuildStatus.exit: + // Continue and throw tool exit. + } + } + final String usageLabel = 'gradle-${detectedGradleError!.eventLabel}-failure'; + _analytics.send(Event.flutterBuildInfo(label: usageLabel, buildType: 'gradle')); + } + + return exitCode; } /// Builds an app. @@ -333,29 +438,14 @@ class AndroidGradleBuilder implements AndroidBuilder { final ProjectMigration migration = ProjectMigration(migrators); await migration.run(); - final bool usesAndroidX = isAppUsingAndroidX(project.android.hostAppGradleRoot); - if (usesAndroidX) { - _analytics.send(Event.flutterBuildInfo(label: 'app-using-android-x', buildType: 'gradle')); - } else if (!usesAndroidX) { - _analytics.send(Event.flutterBuildInfo(label: 'app-not-using-android-x', buildType: 'gradle')); - - _logger.printStatus("${_logger.terminal.warningMark} Your app isn't using AndroidX.", emphasis: true); - _logger.printStatus( - 'To avoid potential build failures, you can quickly migrate your app ' - 'by following the steps on https://goo.gl/CP92wY .', - indent: 4, - ); - } // The default Gradle script reads the version name and number // from the local.properties file. updateLocalProperties( project: project, buildInfo: androidBuildInfo.buildInfo); - final List command = [ - // This does more than get gradlewrapper. It creates the file, ensures it - // exists and verifies the file is executable. - _gradleUtils.getExecutable(project), - ]; + final List options = []; + + final String gradleExecutablePath = _gradleUtils.getExecutable(project); // All automatically created files should exist. if (configOnly) { @@ -369,22 +459,18 @@ class AndroidGradleBuilder implements AndroidBuilder { ? getBundleTaskFor(buildInfo) : getAssembleTaskFor(buildInfo); - final Status status = _logger.startProgress( - "Running Gradle task '$assembleTask'...", - ); - if (_logger.isVerbose) { - command.add('--full-stacktrace'); - command.add('--info'); - command.add('-Pverbose=true'); + options.add('--full-stacktrace'); + options.add('--info'); + options.add('-Pverbose=true'); } else { - command.add('-q'); + options.add('-q'); } if (!buildInfo.androidGradleDaemon) { - command.add('--no-daemon'); + options.add('--no-daemon'); } if (buildInfo.androidSkipBuildDependencyValidation) { - command.add('-PskipDependencyChecks=true'); + options.add('-PskipDependencyChecks=true'); } final LocalEngineInfo? localEngineInfo = _artifacts.localEngineInfo; if (localEngineInfo != null) { @@ -397,29 +483,29 @@ class AndroidGradleBuilder implements AndroidBuilder { 'Using local engine: ${localEngineInfo.targetOutPath}\n' 'Local Maven repo: ${localEngineRepo.path}' ); - command.add('-Plocal-engine-repo=${localEngineRepo.path}'); - command.add('-Plocal-engine-build-mode=${buildInfo.modeName}'); - command.add('-Plocal-engine-out=${localEngineInfo.targetOutPath}'); - command.add('-Plocal-engine-host-out=${localEngineInfo.hostOutPath}'); - command.add('-Ptarget-platform=${_getTargetPlatformByLocalEnginePath( + options.add('-Plocal-engine-repo=${localEngineRepo.path}'); + options.add('-Plocal-engine-build-mode=${buildInfo.modeName}'); + options.add('-Plocal-engine-out=${localEngineInfo.targetOutPath}'); + options.add('-Plocal-engine-host-out=${localEngineInfo.hostOutPath}'); + options.add('-Ptarget-platform=${_getTargetPlatformByLocalEnginePath( localEngineInfo.targetOutPath)}'); } else if (androidBuildInfo.targetArchs.isNotEmpty) { final String targetPlatforms = androidBuildInfo .targetArchs .map((AndroidArch e) => e.platformName).join(','); - command.add('-Ptarget-platform=$targetPlatforms'); + options.add('-Ptarget-platform=$targetPlatforms'); } - command.add('-Ptarget=$target'); + options.add('-Ptarget=$target'); // If using v1 embedding, we want to use FlutterApplication as the base app. final String baseApplicationName = project.android.getEmbeddingVersion() == AndroidEmbeddingVersion.v2 ? 'android.app.Application' : 'io.flutter.app.FlutterApplication'; - command.add('-Pbase-application-name=$baseApplicationName'); + options.add('-Pbase-application-name=$baseApplicationName'); final List? deferredComponents = project.manifest.deferredComponents; if (deferredComponents != null) { if (deferredComponentsEnabled) { - command.add('-Pdeferred-components=true'); + options.add('-Pdeferred-components=true'); androidBuildInfo.buildInfo.dartDefines.add('validate-deferred-components=$validateDeferredComponents'); } // Pass in deferred components regardless of building split aot to satisfy @@ -429,130 +515,51 @@ class AndroidGradleBuilder implements AndroidBuilder { componentNames.add(component.name); } if (componentNames.isNotEmpty) { - command.add('-Pdeferred-component-names=${componentNames.join(',')}'); + options.add('-Pdeferred-component-names=${componentNames.join(',')}'); // Multi-apk applications cannot use shrinking. This is only relevant when using // android dynamic feature modules. _logger.printStatus( 'Shrinking has been disabled for this build due to deferred components. Shrinking is ' 'not available for multi-apk applications. This limitation is expected to be removed ' 'when Gradle plugin 4.2+ is available in Flutter.', color: TerminalColor.yellow); - command.add('-Pshrink=false'); + options.add('-Pshrink=false'); } } - command.addAll(androidBuildInfo.buildInfo.toGradleConfig()); + options.addAll(androidBuildInfo.buildInfo.toGradleConfig()); if (buildInfo.fileSystemRoots.isNotEmpty) { - command.add('-Pfilesystem-roots=${buildInfo.fileSystemRoots.join('|')}'); + options.add('-Pfilesystem-roots=${buildInfo.fileSystemRoots.join('|')}'); } if (buildInfo.fileSystemScheme != null) { - command.add('-Pfilesystem-scheme=${buildInfo.fileSystemScheme}'); + options.add('-Pfilesystem-scheme=${buildInfo.fileSystemScheme}'); } if (androidBuildInfo.splitPerAbi) { - command.add('-Psplit-per-abi=true'); + options.add('-Psplit-per-abi=true'); } if (androidBuildInfo.fastStart) { - command.add('-Pfast-start=true'); + options.add('-Pfast-start=true'); } - command.add(assembleTask); - - GradleHandledError? detectedGradleError; - String? detectedGradleErrorLine; - String? consumeLog(String line) { - // The log lines that trigger incompatibleKotlinVersionHandler don't - // always indicate an error, and there are times that that handler - // covers up a more important error handler. Uniquely set it to be - // the lowest priority handler by allowing it to be overridden. - if (detectedGradleError != null - && detectedGradleError != incompatibleKotlinVersionHandler) { - // Pipe stdout/stderr from Gradle. - return line; - } - for (final GradleHandledError gradleError in localGradleErrors) { - if (gradleError.test(line)) { - detectedGradleErrorLine = line; - detectedGradleError = gradleError; - // The first error match wins. - break; - } - } - // Pipe stdout/stderr from Gradle. - return line; - } - - final Stopwatch sw = Stopwatch() - ..start(); - int exitCode = 1; - try { - exitCode = await _processUtils.stream( - command, - workingDirectory: project.android.hostAppGradleRoot.path, - allowReentrantFlutter: true, - environment: _java?.environment, - mapFunction: consumeLog, - ); - } on ProcessException catch (exception) { - consumeLog(exception.toString()); - // Rethrow the exception if the error isn't handled by any of the - // `localGradleErrors`. - if (detectedGradleError == null) { - rethrow; - } - } finally { - status.stop(); - } - - final Duration elapsedDuration = sw.elapsed; - _analytics.send(Event.timing( - workflow: 'build', - variableName: 'gradle', - elapsedMilliseconds: elapsedDuration.inMilliseconds, - )); + late Stopwatch sw; + final int exitCode = await _runGradleTask( + assembleTask, + preRunTask: () { + sw = Stopwatch()..start(); + }, + postRunTask: () { + final Duration elapsedDuration = sw.elapsed; + _analytics.send(Event.timing( + workflow: 'build', + variableName: 'gradle', + elapsedMilliseconds: elapsedDuration.inMilliseconds, + )); + }, + options: options, + project: project, + maxRetries: maxRetries, + localGradleErrors: localGradleErrors, + gradleExecutablePath: gradleExecutablePath, + ); if (exitCode != 0) { - if (detectedGradleError == null) { - _analytics.send(Event.flutterBuildInfo(label: 'gradle-unknown-failure', buildType: 'gradle')); - - throwToolExit( - 'Gradle task $assembleTask failed with exit code $exitCode', - exitCode: exitCode, - ); - } - final GradleBuildStatus status = await detectedGradleError!.handler( - line: detectedGradleErrorLine!, - project: project, - usesAndroidX: usesAndroidX, - ); - - if (maxRetries == null || retry < maxRetries) { - switch (status) { - case GradleBuildStatus.retry: - // Use binary exponential backoff before retriggering the build. - // The expected wait times are: 100ms, 200ms, 400ms, and so on... - final int waitTime = min(pow(2, retry).toInt() * 100, kMaxRetryTime.inMicroseconds); - retry += 1; - _logger.printStatus('Retrying Gradle Build: #$retry, wait time: ${waitTime}ms'); - await Future.delayed(Duration(milliseconds: waitTime)); - await buildGradleApp( - project: project, - androidBuildInfo: androidBuildInfo, - target: target, - isBuildingBundle: isBuildingBundle, - localGradleErrors: localGradleErrors, - retry: retry, - maxRetries: maxRetries, - configOnly: configOnly, - ); - final String successEventLabel = 'gradle-${detectedGradleError!.eventLabel}-success'; - - _analytics.send(Event.flutterBuildInfo(label: successEventLabel, buildType: 'gradle')); - - return; - case GradleBuildStatus.exit: - // Continue and throw tool exit. - } - } - final String usageLabel = 'gradle-${detectedGradleError?.eventLabel}-failure'; - _analytics.send(Event.flutterBuildInfo(label: usageLabel, buildType: 'gradle')); - throwToolExit( 'Gradle task $assembleTask failed with exit code $exitCode', exitCode: exitCode, @@ -796,31 +803,43 @@ class AndroidGradleBuilder implements AndroidBuilder { @override Future> getBuildVariants({required FlutterProject project}) async { - final Stopwatch sw = Stopwatch() - ..start(); - final RunResult result = await _runGradleTask( - _kBuildVariantTaskName, - options: const ['-q'], - project: project, - ); + late Stopwatch sw; + int exitCode = 1; + final List results = []; - final Duration elapsedDuration = sw.elapsed; - _analytics.send(Event.timing( - workflow: 'print', - variableName: 'android build variants', - elapsedMilliseconds: elapsedDuration.inMilliseconds, - )); + try { + exitCode = await _runGradleTask( + _kBuildVariantTaskName, + preRunTask: () { + sw = Stopwatch()..start(); + }, + postRunTask: () { + final Duration elapsedDuration = sw.elapsed; + _analytics.send(Event.timing( + workflow: 'print', + variableName: 'android build variants', + elapsedMilliseconds: elapsedDuration.inMilliseconds, + )); + }, + options: const ['-q'], + project: project, + localGradleErrors: gradleErrors, + gradleExecutablePath: _gradleUtils.getExecutable(project), + outputParser: (String line) { + if (_kBuildVariantRegex.firstMatch(line) case final RegExpMatch match) { + results.add(match.namedGroup(_kBuildVariantRegexGroupName)!); + } + } + ); + } on Error catch (error) { + _logger.printError(error.toString()); + } - if (result.exitCode != 0) { - _logger.printStatus(result.stdout, wrap: false); - _logger.printError(result.stderr, wrap: false); + if (exitCode != 0) { return const []; } - return [ - for (final String line in LineSplitter.split(result.stdout)) - if (_kBuildVariantRegex.firstMatch(line) case final RegExpMatch match) - match.namedGroup(_kBuildVariantRegexGroupName)!, - ]; + + return results; } @override @@ -835,24 +854,33 @@ class AndroidGradleBuilder implements AndroidBuilder { directory.absolute.path, 'app-link-settings-$buildVariant.json', ); - final Stopwatch sw = Stopwatch() - ..start(); - final RunResult result = await _runGradleTask( - taskName, - options: ['-q', '-PoutputPath=$outputPath'], - project: project, - ); - final Duration elapsedDuration = sw.elapsed; - _analytics.send(Event.timing( - workflow: 'outputs', - variableName: 'app link settings', - elapsedMilliseconds: elapsedDuration.inMilliseconds, - )); + late Stopwatch sw; + int exitCode = 1; + try { + exitCode = await _runGradleTask( + taskName, + preRunTask: () { + sw = Stopwatch()..start(); + }, + postRunTask: () { + final Duration elapsedDuration = sw.elapsed; + _analytics.send(Event.timing( + workflow: 'outputs', + variableName: 'app link settings', + elapsedMilliseconds: elapsedDuration.inMilliseconds, + )); + }, + options: ['-q', '-PoutputPath=$outputPath'], + project: project, + localGradleErrors: gradleErrors, + gradleExecutablePath: _gradleUtils.getExecutable(project), + ); + } on Error catch (error) { + _logger.printError(error.toString()); + } - if (result.exitCode != 0) { - _logger.printStatus(result.stdout, wrap: false); - _logger.printError(result.stderr, wrap: false); - throwToolExit(result.stderr); + if (exitCode != 0) { + throwToolExit('Gradle task $taskName failed with exit code $exitCode'); } return outputPath; } diff --git a/packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart b/packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart index a80a0e336e..36c3f9ade6 100644 --- a/packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart +++ b/packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart @@ -339,8 +339,7 @@ void main() { expect(logger.statusText, contains('Retrying Gradle Build: #2, wait time: 200ms')); expect(testFnCalled, equals(maxRetries + 1)); - - expect(fakeAnalytics.sentEvents, hasLength(7)); + expect(fakeAnalytics.sentEvents, hasLength(9)); expect(fakeAnalytics.sentEvents, contains( Event.flutterBuildInfo( label: 'gradle-random-event-label-failure',