From dce8f712288e7ebea55b0af37bb2affdcc21f349 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 12 Feb 2021 17:38:52 -0800 Subject: [PATCH] [flutter_tools] do not use context logger in gradle (#75940) --- .../flutter_tools/lib/src/android/gradle.dart | 83 +++++++++---------- .../lib/src/application_package.dart | 2 +- .../flutter_tools/lib/src/base/logger.dart | 4 +- .../flutter_tools/lib/src/base/terminal.dart | 6 +- .../flutter_tools/lib/src/context_runner.dart | 4 +- .../android/android_gradle_builder_test.dart | 13 ++- .../general.shard/android/gradle_test.dart | 8 +- .../test/general.shard/base/logger_test.dart | 26 ++++++ 8 files changed, 90 insertions(+), 56 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/gradle.dart b/packages/flutter_tools/lib/src/android/gradle.dart index 27505184ffb..cb83ae5c0a1 100644 --- a/packages/flutter_tools/lib/src/android/gradle.dart +++ b/packages/flutter_tools/lib/src/android/gradle.dart @@ -21,7 +21,7 @@ import '../build_info.dart'; import '../cache.dart'; import '../convert.dart'; import '../flutter_manifest.dart'; -import '../globals.dart' as globals; +import '../globals.dart' as globals hide logger, printStatus, printTrace, printError; import '../project.dart'; import '../reporting/reporting.dart'; import 'android_builder.dart'; @@ -136,8 +136,8 @@ Future getGradleAppOut(AndroidProject androidProject) async { /// Runs `gradlew dependencies`, ensuring that dependencies are resolved and /// potentially downloaded. -Future checkGradleDependencies() async { - final Status progress = globals.logger.startProgress( +Future checkGradleDependencies(Logger logger) async { + final Status progress = logger.startProgress( 'Ensuring gradle dependencies are up to date...', ); final FlutterProject flutterProject = FlutterProject.current(); @@ -157,7 +157,7 @@ Future checkGradleDependencies() async { /// from the existing `settings.gradle` file. This operation will fail if the existing /// `settings.gradle` file has local edits. @visibleForTesting -void createSettingsAarGradle(Directory androidDirectory) { +void createSettingsAarGradle(Directory androidDirectory, Logger logger) { final File newSettingsFile = androidDirectory.childFile('settings_aar.gradle'); if (newSettingsFile.existsSync()) { return; @@ -169,7 +169,7 @@ void createSettingsAarGradle(Directory androidDirectory) { final String currentFileContent = currentSettingsFile.readAsStringSync(); final String newSettingsRelativeFile = globals.fs.path.relative(newSettingsFile.path); - final Status status = globals.logger.startProgress('✏️ Creating `$newSettingsRelativeFile`...'); + final Status status = logger.startProgress('✏️ Creating `$newSettingsRelativeFile`...'); final String flutterRoot = globals.fs.path.absolute(Cache.flutterRoot); final File legacySettingsDotGradleFiles = globals.fs.file(globals.fs.path.join(flutterRoot, 'packages','flutter_tools', @@ -191,20 +191,26 @@ void createSettingsAarGradle(Directory androidDirectory) { } if (!exactMatch) { status.cancel(); - globals.printStatus('$warningMark Flutter tried to create the file `$newSettingsRelativeFile`, but failed.'); + logger.printStatus('$warningMark Flutter tried to create the file `$newSettingsRelativeFile`, but failed.'); // Print how to manually update the file. - globals.printStatus(globals.fs.file(globals.fs.path.join(flutterRoot, 'packages','flutter_tools', + logger.printStatus(globals.fs.file(globals.fs.path.join(flutterRoot, 'packages','flutter_tools', 'gradle', 'manual_migration_settings.gradle.md')).readAsStringSync()); throwToolExit('Please create the file and run this command again.'); } // Copy the new file. newSettingsFile.writeAsStringSync(settingsAarContent); status.stop(); - globals.printStatus('$successMark `$newSettingsRelativeFile` created successfully.'); + logger.printStatus('$successMark `$newSettingsRelativeFile` created successfully.'); } /// An implementation of the [AndroidBuilder] that delegates to gradle. class AndroidGradleBuilder implements AndroidBuilder { + AndroidGradleBuilder({ + @required Logger logger, + }) : _logger = logger; + + final Logger _logger; + /// Builds the AAR and POM files for the current Flutter module or plugin. @override Future buildAar({ @@ -238,7 +244,7 @@ class AndroidGradleBuilder implements AndroidBuilder { androidPackage: project.manifest.androidPackage, repoDirectory: getRepoDirectory(outputDirectory), buildNumber: buildNumber, - logger: globals.logger, + logger: _logger, fileSystem: globals.fs, ); } finally { @@ -326,8 +332,8 @@ class AndroidGradleBuilder implements AndroidBuilder { BuildEvent('app-using-android-x', flutterUsage: globals.flutterUsage).send(); } else if (!usesAndroidX) { BuildEvent('app-not-using-android-x', flutterUsage: globals.flutterUsage).send(); - globals.printStatus("$warningMark Your app isn't using AndroidX.", emphasis: true); - globals.printStatus( + _logger.printStatus("$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, @@ -339,7 +345,7 @@ class AndroidGradleBuilder implements AndroidBuilder { if (shouldBuildPluginAsAar) { // Create a settings.gradle that doesn't import the plugins as subprojects. - createSettingsAarGradle(project.android.hostAppGradleRoot); + createSettingsAarGradle(project.android.hostAppGradleRoot, _logger); await buildPluginsAsAar( project, androidBuildInfo, @@ -352,7 +358,7 @@ class AndroidGradleBuilder implements AndroidBuilder { ? getBundleTaskFor(buildInfo) : getAssembleTaskFor(buildInfo); - final Status status = globals.logger.startProgress( + final Status status = _logger.startProgress( "Running Gradle task '$assembleTask'...", multilineOutput: true, ); @@ -360,7 +366,7 @@ class AndroidGradleBuilder implements AndroidBuilder { final List command = [ gradleUtils.getExecutable(project), ]; - if (globals.logger.isVerbose) { + if (_logger.isVerbose) { command.add('-Pverbose=true'); } else { command.add('-q'); @@ -374,7 +380,7 @@ class AndroidGradleBuilder implements AndroidBuilder { engineOutPath: localEngineArtifacts.engineOutPath, androidBuildInfo: androidBuildInfo, ); - globals.printTrace( + _logger.printTrace( 'Using local engine: ${localEngineArtifacts.engineOutPath}\n' 'Local Maven repo: ${localEngineRepo.path}' ); @@ -528,7 +534,7 @@ class AndroidGradleBuilder implements AndroidBuilder { await _performCodeSizeAnalysis('aab', bundleFile, androidBuildInfo); } - globals.printStatus( + _logger.printStatus( '$successMark Built ${globals.fs.path.relative(bundleFile.path)}$appSize.', color: TerminalColor.green, ); @@ -552,7 +558,7 @@ class AndroidGradleBuilder implements AndroidBuilder { apkFile.copySync(apkDirectory .childFile('app.apk') .path); - globals.printTrace('calculateSha: $apkDirectory/app.apk'); + _logger.printTrace('calculateSha: $apkDirectory/app.apk'); final File apkShaFile = apkDirectory.childFile('app.apk.sha1'); apkShaFile.writeAsStringSync(_calculateSha(apkFile)); @@ -560,7 +566,7 @@ class AndroidGradleBuilder implements AndroidBuilder { final String appSize = (buildInfo.mode == BuildMode.debug) ? '' // Don't display the size when building a debug variant. : ' (${getSizeAsMB(apkFile.lengthSync())})'; - globals.printStatus( + _logger.printStatus( '$successMark Built ${globals.fs.path.relative(apkFile.path)}$appSize.', color: TerminalColor.green, ); @@ -575,7 +581,7 @@ class AndroidGradleBuilder implements AndroidBuilder { AndroidBuildInfo androidBuildInfo,) async { final SizeAnalyzer sizeAnalyzer = SizeAnalyzer( fileSystem: globals.fs, - logger: globals.logger, + logger: _logger, flutterUsage: globals.flutterUsage, ); final String archName = getNameForAndroidArch(androidBuildInfo.targetArchs.single); @@ -597,7 +603,7 @@ class AndroidGradleBuilder implements AndroidBuilder { ) ..writeAsStringSync(jsonEncode(output)); // This message is used as a sentinel in analyze_apk_size_test.dart - globals.printStatus( + _logger.printStatus( 'A summary of your ${kind.toUpperCase()} analysis can be found at: ${outputFile.path}', ); @@ -606,7 +612,7 @@ class AndroidGradleBuilder implements AndroidBuilder { .split('.flutter-devtools/') .last .trim(); - globals.printStatus( + _logger.printStatus( '\nTo analyze your app size in Dart DevTools, run the following command:\n' 'flutter pub global activate devtools; flutter pub global run devtools ' '--appSizeBase=$relativeAppSizePath' @@ -639,7 +645,7 @@ class AndroidGradleBuilder implements AndroidBuilder { final BuildInfo buildInfo = androidBuildInfo.buildInfo; final String aarTask = getAarTaskFor(buildInfo); - final Status status = globals.logger.startProgress( + final Status status = _logger.startProgress( "Running Gradle task '$aarTask'...", multilineOutput: true, ); @@ -660,7 +666,7 @@ class AndroidGradleBuilder implements AndroidBuilder { '-Pis-plugin=${manifest.isPlugin}', '-PbuildNumber=$buildNumber' ]; - if (globals.logger.isVerbose) { + if (_logger.isVerbose) { command.add('-Pverbose=true'); } else { command.add('-q'); @@ -674,7 +680,7 @@ class AndroidGradleBuilder implements AndroidBuilder { } command.addAll(androidBuildInfo.buildInfo.toGradleConfig()); if (buildInfo.dartObfuscation && buildInfo.mode != BuildMode.release) { - globals.printStatus( + _logger.printStatus( 'Dart obfuscation is not supported in ${toTitleCase(buildInfo.friendlyModeName)}' ' mode, building as un-obfuscated.', ); @@ -686,9 +692,9 @@ class AndroidGradleBuilder implements AndroidBuilder { engineOutPath: localEngineArtifacts.engineOutPath, androidBuildInfo: androidBuildInfo, ); - globals.printTrace( - 'Using local engine: ${localEngineArtifacts.engineOutPath}\n' - 'Local Maven repo: ${localEngineRepo.path}' + _logger.printTrace( + 'Using local engine: ${localEngineArtifacts.engineOutPath}\n' + 'Local Maven repo: ${localEngineRepo.path}' ); command.add('-Plocal-engine-repo=${localEngineRepo.path}'); command.add('-Plocal-engine-build-mode=${buildInfo.modeName}'); @@ -732,8 +738,8 @@ class AndroidGradleBuilder implements AndroidBuilder { globals.flutterUsage.sendTiming('build', 'gradle-aar', sw.elapsed); if (result.exitCode != 0) { - globals.printStatus(result.stdout, wrap: false); - globals.printError(result.stderr, wrap: false); + _logger.printStatus(result.stdout, wrap: false); + _logger.printError(result.stderr, wrap: false); throwToolExit( 'Gradle task $aarTask failed with exit code ${result.exitCode}.', exitCode: result.exitCode, @@ -741,14 +747,14 @@ class AndroidGradleBuilder implements AndroidBuilder { } final Directory repoDirectory = getRepoDirectory(outputDirectory); if (!repoDirectory.existsSync()) { - globals.printStatus(result.stdout, wrap: false); - globals.printError(result.stderr, wrap: false); + _logger.printStatus(result.stdout, wrap: false); + _logger.printError(result.stderr, wrap: false); throwToolExit( 'Gradle task $aarTask failed to produce $repoDirectory.', exitCode: exitCode, ); } - globals.printStatus( + _logger.printStatus( '$successMark Built ${globals.fs.path.relative(repoDirectory.path)}.', color: TerminalColor.green, ); @@ -777,10 +783,10 @@ class AndroidGradleBuilder implements AndroidBuilder { final String pluginName = pluginParts.first; final File buildGradleFile = pluginDirectory.childDirectory('android').childFile('build.gradle'); if (!buildGradleFile.existsSync()) { - globals.printTrace("Skipping plugin $pluginName since it doesn't have a android/build.gradle file"); + _logger.printTrace("Skipping plugin $pluginName since it doesn't have a android/build.gradle file"); continue; } - globals.logger.printStatus('Building plugin $pluginName...'); + _logger.printStatus('Building plugin $pluginName...'); try { await buildGradleAar( project: FlutterProject.fromDirectory(pluginDirectory), @@ -876,15 +882,8 @@ String _hex(List bytes) { } String _calculateSha(File file) { - final Stopwatch sw = Stopwatch()..start(); final List bytes = file.readAsBytesSync(); - globals.printTrace('calculateSha: reading file took ${sw.elapsedMilliseconds}us'); - globals.flutterUsage.sendTiming('build', 'apk-sha-read', sw.elapsed); - sw.reset(); - final String sha = _hex(sha1.convert(bytes).bytes); - globals.printTrace('calculateSha: computing sha took ${sw.elapsedMilliseconds}us'); - globals.flutterUsage.sendTiming('build', 'apk-sha-calc', sw.elapsed); - return sha; + return _hex(sha1.convert(bytes).bytes); } void _exitWithUnsupportedProjectMessage() { diff --git a/packages/flutter_tools/lib/src/application_package.dart b/packages/flutter_tools/lib/src/application_package.dart index c028e25db9e..7454e1f37a9 100644 --- a/packages/flutter_tools/lib/src/application_package.dart +++ b/packages/flutter_tools/lib/src/application_package.dart @@ -65,7 +65,7 @@ class ApplicationPackageFactory { case TargetPlatform.android_x64: case TargetPlatform.android_x86: if (_androidSdk?.licensesAvailable == true && _androidSdk?.latestVersion == null) { - await checkGradleDependencies(); + await checkGradleDependencies(_logger); } if (applicationBinary == null) { return await AndroidApk.fromAndroidProject( diff --git a/packages/flutter_tools/lib/src/base/logger.dart b/packages/flutter_tools/lib/src/base/logger.dart index 4ab30fac29e..5df4481f940 100644 --- a/packages/flutter_tools/lib/src/base/logger.dart +++ b/packages/flutter_tools/lib/src/base/logger.dart @@ -413,7 +413,6 @@ class WindowsStdoutLogger extends StdoutLogger { @override void writeToStdOut(String message) { - // TODO(jcollins-g): wrong abstraction layer for this, move to [Stdio]. final String windowsMessage = _terminal.supportsEmoji ? message : message.replaceAll('🔥', '') @@ -421,7 +420,8 @@ class WindowsStdoutLogger extends StdoutLogger { .replaceAll('✗', 'X') .replaceAll('✓', '√') .replaceAll('🔨', '') - .replaceAll('💪', ''); + .replaceAll('💪', '') + .replaceAll('✏️', ''); _stdio.stdoutWrite(windowsMessage); } } diff --git a/packages/flutter_tools/lib/src/base/terminal.dart b/packages/flutter_tools/lib/src/base/terminal.dart index d2820b82419..19c3a01cf5f 100644 --- a/packages/flutter_tools/lib/src/base/terminal.dart +++ b/packages/flutter_tools/lib/src/base/terminal.dart @@ -84,7 +84,7 @@ abstract class Terminal { /// Create a new test [Terminal]. /// /// If not specified, [supportsColor] defaults to `false`. - factory Terminal.test({bool supportsColor}) = _TestTerminal; + factory Terminal.test({bool supportsColor, bool supportsEmoji}) = _TestTerminal; /// Whether the current terminal supports color escape codes. bool get supportsColor; @@ -317,7 +317,7 @@ class AnsiTerminal implements Terminal { } class _TestTerminal implements Terminal { - _TestTerminal({this.supportsColor = false}); + _TestTerminal({this.supportsColor = false, this.supportsEmoji = false}); @override bool usesTerminalUi; @@ -351,7 +351,7 @@ class _TestTerminal implements Terminal { final bool supportsColor; @override - bool get supportsEmoji => false; + final bool supportsEmoji; @override bool get stdinHasTerminal => false; diff --git a/packages/flutter_tools/lib/src/context_runner.dart b/packages/flutter_tools/lib/src/context_runner.dart index 1d69fcfbc6e..19a30051d8b 100644 --- a/packages/flutter_tools/lib/src/context_runner.dart +++ b/packages/flutter_tools/lib/src/context_runner.dart @@ -79,7 +79,9 @@ Future runInContext( body: runnerWrapper, overrides: overrides, fallbacks: { - AndroidBuilder: () => AndroidGradleBuilder(), + AndroidBuilder: () => AndroidGradleBuilder( + logger: globals.logger, + ), AndroidLicenseValidator: () => AndroidLicenseValidator( operatingSystemUtils: globals.os, platform: globals.platform, 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 02b8bc12528..b8d9584b58f 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 @@ -14,6 +14,7 @@ import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart'; +import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/cache.dart'; @@ -29,6 +30,7 @@ import '../../src/mocks.dart'; void main() { group('gradle build', () { + BufferLogger logger; TestUsage testUsage; MockAndroidSdk mockAndroidSdk; MockAndroidStudio mockAndroidStudio; @@ -41,6 +43,7 @@ void main() { AndroidGradleBuilder builder; setUp(() { + logger = BufferLogger.test(); testUsage = TestUsage(); fileSystem = MemoryFileSystem.test(); fileSystemUtils = MockFileSystemUtils(); @@ -49,7 +52,9 @@ void main() { mockArtifacts = MockLocalEngineArtifacts(); mockProcessManager = MockProcessManager(); android = fakePlatform('android'); - builder = AndroidGradleBuilder(); + builder = AndroidGradleBuilder( + logger: logger, + ); when(mockAndroidSdk.directory).thenReturn('irrelevant'); @@ -660,7 +665,7 @@ void main() { ); expect( - testLogger.statusText, + logger.statusText, contains('Built build/app/outputs/flutter-apk/app-release.apk (0.0MB)'), ); @@ -708,11 +713,11 @@ void main() { ); expect( - testLogger.statusText, + logger.statusText, contains('Built build/outputs/repo'), ); expect( - testLogger.statusText.contains('Consuming the Module'), + logger.statusText.contains('Consuming the Module'), isFalse, ); diff --git a/packages/flutter_tools/test/general.shard/android/gradle_test.dart b/packages/flutter_tools/test/general.shard/android/gradle_test.dart index ecefbee8408..4a90beba596 100644 --- a/packages/flutter_tools/test/general.shard/android/gradle_test.dart +++ b/packages/flutter_tools/test/general.shard/android/gradle_test.dart @@ -462,7 +462,7 @@ include ':app' globals.fs.file(globals.fs.path.join(toolGradlePath, 'settings_aar.gradle.tmpl')) .writeAsStringSync(settingsAarFile); - createSettingsAarGradle(tempDir); + createSettingsAarGradle(tempDir, testLogger); expect(testLogger.statusText, contains('created successfully')); expect(tempDir.childFile('settings_aar.gradle').existsSync(), isTrue); @@ -495,7 +495,7 @@ include ':app' globals.fs.file(globals.fs.path.join(toolGradlePath, 'settings_aar.gradle.tmpl')) .writeAsStringSync(settingsAarFile); - createSettingsAarGradle(tempDir); + createSettingsAarGradle(tempDir, testLogger); expect(testLogger.statusText, contains('created successfully')); expect(tempDir.childFile('settings_aar.gradle').existsSync(), isTrue); @@ -834,13 +834,15 @@ flutter: FakeProcessManager fakeProcessManager; MockAndroidSdk mockAndroidSdk; AndroidGradleBuilder builder; + BufferLogger logger; setUp(() { + logger = BufferLogger.test(); fs = MemoryFileSystem.test(); fakeProcessManager = FakeProcessManager.list([]); mockAndroidSdk = MockAndroidSdk(); when(mockAndroidSdk.directory).thenReturn('irrelevant'); - builder = AndroidGradleBuilder(); + builder = AndroidGradleBuilder(logger: logger); }); testUsingContext('calls gradle', () async { diff --git a/packages/flutter_tools/test/general.shard/base/logger_test.dart b/packages/flutter_tools/test/general.shard/base/logger_test.dart index b9a346a3455..b04f790a741 100644 --- a/packages/flutter_tools/test/general.shard/base/logger_test.dart +++ b/packages/flutter_tools/test/general.shard/base/logger_test.dart @@ -87,6 +87,32 @@ void main() { ), isA()); }); + testWithoutContext('WindowsStdoutLogger rewrites emojis when terminal does not support emoji', () { + final FakeStdio stdio = FakeStdio(); + final WindowsStdoutLogger logger = WindowsStdoutLogger( + outputPreferences: OutputPreferences.test(), + stdio: stdio, + terminal: Terminal.test(supportsColor: false, supportsEmoji: false), + ); + + logger.printStatus('🔥🖼️✗✓🔨💪✏️'); + + expect(stdio.writtenToStdout, ['X√\n']); + }); + + testWithoutContext('WindowsStdoutLogger does not rewrite emojis when terminal does support emoji', () { + final FakeStdio stdio = FakeStdio(); + final WindowsStdoutLogger logger = WindowsStdoutLogger( + outputPreferences: OutputPreferences.test(), + stdio: stdio, + terminal: Terminal.test(supportsColor: true, supportsEmoji: true), + ); + + logger.printStatus('🔥🖼️✗✓🔨💪✏️'); + + expect(stdio.writtenToStdout, ['🔥🖼️✗✓🔨💪✏️\n']); + }); + testWithoutContext('DelegatingLogger delegates', () { final FakeLogger fakeLogger = FakeLogger(); final DelegatingLogger delegatingLogger = DelegatingLogger(fakeLogger);