From ed5482de1981a6bb3c90230a8162a4e355043543 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 19 Oct 2020 09:33:52 -0700 Subject: [PATCH] Revert "[flutter_tools] add --android-gradle-daemon option, use in devicelab (#68409)" (#68489) This reverts commit ddab72e79f72b7896d8f62b2062d46c764c224f6. --- .../tasks/complex_layout_semantics_perf.dart | 1 - dev/devicelab/lib/framework/runner.dart | 41 +++++++++++++ dev/devicelab/lib/tasks/defines_task.dart | 1 - dev/devicelab/lib/tasks/gallery.dart | 1 - dev/devicelab/lib/tasks/hot_mode_tests.dart | 2 +- .../lib/tasks/integration_tests.dart | 1 - dev/devicelab/lib/tasks/perf_tests.dart | 4 -- .../track_widget_creation_enabled_task.dart | 2 - .../flutter_tools/lib/src/android/gradle.dart | 6 -- .../flutter_tools/lib/src/build_info.dart | 15 ----- .../lib/src/commands/build_aar.dart | 1 - .../lib/src/commands/build_apk.dart | 1 - .../lib/src/commands/build_appbundle.dart | 1 - .../flutter_tools/lib/src/commands/run.dart | 1 - .../lib/src/runner/flutter_command.dart | 17 ------ .../general.shard/android/gradle_test.dart | 57 +------------------ 16 files changed, 44 insertions(+), 108 deletions(-) diff --git a/dev/devicelab/bin/tasks/complex_layout_semantics_perf.dart b/dev/devicelab/bin/tasks/complex_layout_semantics_perf.dart index ade3b84217..83316b706b 100644 --- a/dev/devicelab/bin/tasks/complex_layout_semantics_perf.dart +++ b/dev/devicelab/bin/tasks/complex_layout_semantics_perf.dart @@ -21,7 +21,6 @@ void main() { await inDirectory(complexLayoutPath, () async { await flutter('drive', options: [ - '--no-android-gradle-daemon', '-v', '--profile', '--trace-startup', // Enables "endless" timeline event buffering. diff --git a/dev/devicelab/lib/framework/runner.dart b/dev/devicelab/lib/framework/runner.dart index 6b7cd7a910..5fa5295ffe 100644 --- a/dev/devicelab/lib/framework/runner.dart +++ b/dev/devicelab/lib/framework/runner.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:path/path.dart' as path; import 'package:vm_service_client/vm_service_client.dart'; import 'package:flutter_devicelab/framework/utils.dart'; @@ -87,6 +88,7 @@ Future runTask( } finally { if (!runnerFinished) runner.kill(ProcessSignal.sigkill); + await cleanupSystem(); await stdoutSub.cancel(); await stderrSub.cancel(); } @@ -122,3 +124,42 @@ Future _connectToRunnerIsolate(Uri vmServiceUri) async { } } } + +Future cleanupSystem() async { + if (deviceOperatingSystem == null || deviceOperatingSystem == DeviceOperatingSystem.android) { + print('\n\nCleaning up system after task...'); + final String javaHome = await findJavaHome(); + if (javaHome != null) { + // To shut gradle down, we have to call "gradlew --stop". + // To call gradlew, we need to have a gradle-wrapper.properties file along + // with a shell script, a .jar file, etc. We get these from various places + // as you see in the code below, and we save them all into a temporary dir + // which we can then delete after. + // All the steps below are somewhat tolerant of errors, because it doesn't + // really matter if this succeeds every time or not. + print('\nTelling Gradle to shut down (JAVA_HOME=$javaHome)'); + final String gradlewBinaryName = Platform.isWindows ? 'gradlew.bat' : 'gradlew'; + final Directory tempDir = Directory.systemTemp.createTempSync('flutter_devicelab_shutdown_gradle.'); + recursiveCopy(Directory(path.join(flutterDirectory.path, 'bin', 'cache', 'artifacts', 'gradle_wrapper')), tempDir); + copy(File(path.join(path.join(flutterDirectory.path, 'packages', 'flutter_tools'), 'templates', 'app', 'android.tmpl', 'gradle', 'wrapper', 'gradle-wrapper.properties')), Directory(path.join(tempDir.path, 'gradle', 'wrapper'))); + if (!Platform.isWindows) { + await exec( + 'chmod', + ['a+x', path.join(tempDir.path, gradlewBinaryName)], + canFail: true, + ); + } + await exec( + path.join(tempDir.path, gradlewBinaryName), + ['--stop'], + environment: { 'JAVA_HOME': javaHome}, + workingDirectory: tempDir.path, + canFail: true, + ); + rmTree(tempDir); + print('\n'); + } else { + print('Could not determine JAVA_HOME; not shutting down Gradle.'); + } + } +} diff --git a/dev/devicelab/lib/tasks/defines_task.dart b/dev/devicelab/lib/tasks/defines_task.dart index 8a9cb60c20..044542fc23 100644 --- a/dev/devicelab/lib/tasks/defines_task.dart +++ b/dev/devicelab/lib/tasks/defines_task.dart @@ -17,7 +17,6 @@ Future runDartDefinesTask() async { await flutter('packages', options: ['get']); await flutter('drive', options: [ - '--no-android-gradle-daemon', '--verbose', '-d', deviceId, diff --git a/dev/devicelab/lib/tasks/gallery.dart b/dev/devicelab/lib/tasks/gallery.dart index 226e374fdc..b06beae752 100644 --- a/dev/devicelab/lib/tasks/gallery.dart +++ b/dev/devicelab/lib/tasks/gallery.dart @@ -70,7 +70,6 @@ class GalleryTransitionTest { await flutter( 'build', options: [ - '--no-android-gradle-daemon', 'apk', '--profile', '-t', diff --git a/dev/devicelab/lib/tasks/hot_mode_tests.dart b/dev/devicelab/lib/tasks/hot_mode_tests.dart index fc3f721c18..c5b76c0d82 100644 --- a/dev/devicelab/lib/tasks/hot_mode_tests.dart +++ b/dev/devicelab/lib/tasks/hot_mode_tests.dart @@ -26,7 +26,7 @@ TaskFunction createHotModeTest({String deviceIdOverride, Map env final File benchmarkFile = file(path.join(_editedFlutterGalleryDir.path, 'hot_benchmark.json')); rm(benchmarkFile); final List options = [ - '--hot', '-d', deviceIdOverride, '--benchmark', '--verbose', '--resident', '--no-android-gradle-daemon', + '--hot', '-d', deviceIdOverride, '--benchmark', '--verbose', '--resident', ]; int hotReloadCount = 0; Map twoReloadsData; diff --git a/dev/devicelab/lib/tasks/integration_tests.dart b/dev/devicelab/lib/tasks/integration_tests.dart index c66ab74558..bd0d08787a 100644 --- a/dev/devicelab/lib/tasks/integration_tests.dart +++ b/dev/devicelab/lib/tasks/integration_tests.dart @@ -130,7 +130,6 @@ class DriverTest { await flutter('packages', options: ['get']); final List options = [ - '--no-android-gradle-daemon', '-v', '-t', testTarget, diff --git a/dev/devicelab/lib/tasks/perf_tests.dart b/dev/devicelab/lib/tasks/perf_tests.dart index 3b00a813fe..bb8e19d0ba 100644 --- a/dev/devicelab/lib/tasks/perf_tests.dart +++ b/dev/devicelab/lib/tasks/perf_tests.dart @@ -347,7 +347,6 @@ TaskFunction createsScrollSmoothnessPerfTest() { await flutter('packages', options: ['get']); await flutter('drive', options: [ - '--no-android-gradle-daemon', '-v', '--verbose-system-logs', '--profile', @@ -396,7 +395,6 @@ TaskFunction createFramePolicyIntegrationTest() { await flutter('packages', options: ['get']); await flutter('drive', options: [ - '--no-android-gradle-daemon', '-v', '--verbose-system-logs', '--profile', @@ -461,7 +459,6 @@ class StartupTest { final List> results = >[]; for (int i = 0; i < iterations; ++i) { await flutter('run', options: [ - '--no-android-gradle-daemon', '--verbose', '--profile', '--trace-startup', @@ -578,7 +575,6 @@ class PerfTest { await flutter('packages', options: ['get']); await flutter('drive', options: [ - '--no-android-gradle-daemon', '-v', '--verbose-system-logs', '--profile', diff --git a/dev/devicelab/lib/tasks/track_widget_creation_enabled_task.dart b/dev/devicelab/lib/tasks/track_widget_creation_enabled_task.dart index 04ab69ee09..620dda6a2d 100644 --- a/dev/devicelab/lib/tasks/track_widget_creation_enabled_task.dart +++ b/dev/devicelab/lib/tasks/track_widget_creation_enabled_task.dart @@ -43,7 +43,6 @@ class TrackWidgetCreationEnabledTask { final Process runProcess = await startProcess( path.join(flutterDirectory.path, 'bin', 'flutter'), flutterCommandArgs('run', [ - '--no-android-gradle-daemon', ...?additionalArgs, '--vmservice-out-file=info', '--track-widget-creation', @@ -79,7 +78,6 @@ class TrackWidgetCreationEnabledTask { final Process runProcess = await startProcess( path.join(flutterDirectory.path, 'bin', 'flutter'), flutterCommandArgs('run', [ - '--no-android-gradle-daemon', ...?additionalArgs, '--vmservice-out-file=info', '--no-track-widget-creation', diff --git a/packages/flutter_tools/lib/src/android/gradle.dart b/packages/flutter_tools/lib/src/android/gradle.dart index b1846b61f7..9645a4eb2c 100644 --- a/packages/flutter_tools/lib/src/android/gradle.dart +++ b/packages/flutter_tools/lib/src/android/gradle.dart @@ -279,9 +279,6 @@ Future buildGradleApp({ } else { command.add('-q'); } - if (!buildInfo.androidGradleDaemon) { - command.add('--no-daemon'); - } if (globals.artifacts is LocalEngineArtifacts) { final LocalEngineArtifacts localEngineArtifacts = globals.artifacts as LocalEngineArtifacts; final Directory localEngineRepo = _getLocalEngineRepo( @@ -595,9 +592,6 @@ Future buildGradleAar({ } else { command.add('-q'); } - if (!buildInfo.androidGradleDaemon) { - command.add('--no-daemon'); - } if (target != null && target.isNotEmpty) { command.add('-Ptarget=$target'); diff --git a/packages/flutter_tools/lib/src/build_info.dart b/packages/flutter_tools/lib/src/build_info.dart index 16927d33e6..17d9b9d8ba 100644 --- a/packages/flutter_tools/lib/src/build_info.dart +++ b/packages/flutter_tools/lib/src/build_info.dart @@ -34,7 +34,6 @@ class BuildInfo { this.packagesPath = '.packages', this.nullSafetyMode = NullSafetyMode.autodetect, this.codeSizeDirectory, - this.androidGradleDaemon = true, }); final BuildMode mode; @@ -119,20 +118,6 @@ class BuildInfo { /// will be written for code size profiling. final String codeSizeDirectory; - /// Whether to enable the Gradle daemon when performing an Android build. - /// - /// Starting the daemon is the default behavior of the gradle wrapper script created - /// in a Flutter project. Setting this value to false will cause the tool to pass - /// `--no-daemon` to the gradle wrapper script, preventing it from spawning a daemon - /// process. - /// - /// For one-off builds or CI systems, preventing the daemon from spawning will - /// reduce system resource usage, at the cost of any subsequent builds starting - /// up slightly slower. - /// - /// The Gradle daemon may also be disabled in the Android application's properties file. - final bool androidGradleDaemon; - static const BuildInfo debug = BuildInfo(BuildMode.debug, null, treeShakeIcons: false); static const BuildInfo profile = BuildInfo(BuildMode.profile, null, treeShakeIcons: kIconTreeShakerEnabledDefault); static const BuildInfo jitRelease = BuildInfo(BuildMode.jitRelease, null, treeShakeIcons: kIconTreeShakerEnabledDefault); diff --git a/packages/flutter_tools/lib/src/commands/build_aar.dart b/packages/flutter_tools/lib/src/commands/build_aar.dart index 1f256e5688..5824a7375a 100644 --- a/packages/flutter_tools/lib/src/commands/build_aar.dart +++ b/packages/flutter_tools/lib/src/commands/build_aar.dart @@ -43,7 +43,6 @@ class BuildAarCommand extends BuildSubCommand { usesTrackWidgetCreation(verboseHelp: false); addNullSafetyModeOptions(hide: !verboseHelp); addEnableExperimentation(hide: !verboseHelp); - addAndroidSpecificBuildOptions(hide: !verboseHelp); argParser ..addMultiOption( 'target-platform', diff --git a/packages/flutter_tools/lib/src/commands/build_apk.dart b/packages/flutter_tools/lib/src/commands/build_apk.dart index 750fc15ec4..7cdee98a22 100644 --- a/packages/flutter_tools/lib/src/commands/build_apk.dart +++ b/packages/flutter_tools/lib/src/commands/build_apk.dart @@ -33,7 +33,6 @@ class BuildApkCommand extends BuildSubCommand { addBuildPerformanceFile(hide: !verboseHelp); addNullSafetyModeOptions(hide: !verboseHelp); usesAnalyzeSizeFlag(); - addAndroidSpecificBuildOptions(hide: !verboseHelp); argParser ..addFlag('split-per-abi', negatable: false, diff --git a/packages/flutter_tools/lib/src/commands/build_appbundle.dart b/packages/flutter_tools/lib/src/commands/build_appbundle.dart index 97c1ccf27e..c84ef66f43 100644 --- a/packages/flutter_tools/lib/src/commands/build_appbundle.dart +++ b/packages/flutter_tools/lib/src/commands/build_appbundle.dart @@ -33,7 +33,6 @@ class BuildAppBundleCommand extends BuildSubCommand { addNullSafetyModeOptions(hide: !verboseHelp); addEnableExperimentation(hide: !verboseHelp); usesAnalyzeSizeFlag(); - addAndroidSpecificBuildOptions(hide: !verboseHelp); argParser.addMultiOption('target-platform', splitCommas: true, defaultsTo: ['android-arm', 'android-arm64', 'android-x64'], diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index fb0a27c587..34ae7f04bc 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -88,7 +88,6 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment usesDeviceUserOption(); usesDeviceTimeoutOption(); addDdsOptions(verboseHelp: verboseHelp); - addAndroidSpecificBuildOptions(hide: !verboseHelp); } bool get traceStartup => boolArg('trace-startup'); diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 449468cb66..7cfbf063ce 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -114,7 +114,6 @@ class FlutterOptions { static const String kDeviceTimeout = 'device-timeout'; static const String kAnalyzeSize = 'analyze-size'; static const String kNullAssertions = 'null-assertions'; - static const String kAndroidGradleDaemon = 'android-gradle-daemon'; } abstract class FlutterCommand extends Command { @@ -612,18 +611,6 @@ abstract class FlutterCommand extends Command { ); } - void addAndroidSpecificBuildOptions({ bool hide = false }) { - argParser.addFlag( - FlutterOptions.kAndroidGradleDaemon, - help: 'Whether to enable the Gradle daemon when performing an Android build. ' - 'Starting the daemon is the default behavior of the gradle wrapper script created ' - ' in a Flutter project. Setting this flag to false corresponds to passing ' - "'--no-daemon' to the gradle wrapper script. This flag will cause the daemon " - 'process to terminate after the build is completed', - defaultsTo: true, - ); - } - /// Adds build options common to all of the desktop build commands. void addCommonDesktopBuildOptions({ bool verboseHelp = false }) { addBuildModeFlags(verboseHelp: verboseHelp); @@ -777,9 +764,6 @@ abstract class FlutterCommand extends Command { ? stringArg(FlutterOptions.kSplitDebugInfoOption) : null; - final bool androidGradleDaemon = !argParser.options.containsKey(FlutterOptions.kAndroidGradleDaemon) - || boolArg(FlutterOptions.kAndroidGradleDaemon); - if (dartObfuscation && (splitDebugInfoPath == null || splitDebugInfoPath.isEmpty)) { throwToolExit( '"--${FlutterOptions.kDartObfuscationOption}" can only be used in ' @@ -843,7 +827,6 @@ abstract class FlutterCommand extends Command { packagesPath: globalResults['packages'] as String ?? '.packages', nullSafetyMode: nullSafetyMode, codeSizeDirectory: codeSizeDirectory, - androidGradleDaemon: androidGradleDaemon, ); } 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 be389dfd40..0ad0805e2d 100644 --- a/packages/flutter_tools/test/general.shard/android/gradle_test.dart +++ b/packages/flutter_tools/test/general.shard/android/gradle_test.dart @@ -1328,7 +1328,7 @@ plugin1=${plugin1.path} ProcessManager: () => mockProcessManager, }); - testUsingContext('logs success event after a successful retry', () async { + testUsingContext('logs success event after a sucessful retry', () async { int testFnCalled = 0; when(mockProcessManager.start(any, workingDirectory: anyNamed('workingDirectory'), @@ -1415,7 +1415,7 @@ plugin1=${plugin1.path} Usage: () => mockUsage, }); - testUsingContext('performs code size analysis and sends analytics', () async { + testUsingContext('performs code size analyis and sends analytics', () async { when(mockProcessManager.start(any, workingDirectory: anyNamed('workingDirectory'), environment: anyNamed('environment'))) @@ -2059,59 +2059,6 @@ plugin1=${plugin1.path} ProcessManager: () => mockProcessManager, }); - testUsingContext('honors --no-android-gradle-daemon setting', () async { - (globals.processManager as FakeProcessManager).addCommand( - const FakeCommand(command: [ - '/android/gradlew', - '-q', - '--no-daemon', - '-Ptarget-platform=android-arm,android-arm64,android-x64', - '-Ptarget=lib/main.dart', - '-Ptrack-widget-creation=false', - 'assembleRelease' - ], - )); - fileSystem.file('android/gradlew').createSync(recursive: true); - - fileSystem.directory('android') - .childFile('gradle.properties') - .createSync(recursive: true); - - fileSystem.file('android/build.gradle') - .createSync(recursive: true); - - fileSystem.directory('android') - .childDirectory('app') - .childFile('build.gradle') - ..createSync(recursive: true) - ..writeAsStringSync('apply from: irrelevant/flutter.gradle'); - - await expectLater(() async { - await buildGradleApp( - project: FlutterProject.current(), - androidBuildInfo: const AndroidBuildInfo( - BuildInfo( - BuildMode.release, - null, - treeShakeIcons: false, - androidGradleDaemon: false, - ), - ), - target: 'lib/main.dart', - isBuildingBundle: false, - localGradleErrors: const [], - ); - }, throwsToolExit()); - }, overrides: { - AndroidSdk: () => mockAndroidSdk, - AndroidStudio: () => mockAndroidStudio, - Artifacts: () => Artifacts.test(), - Cache: () => cache, - Platform: () => android, - FileSystem: () => fileSystem, - ProcessManager: () => FakeProcessManager.list([]), - }); - testUsingContext('build aar uses selected local engineļ¼Œthe engine abi is arm', () async { when(mockArtifacts.getArtifactPath(Artifact.flutterFramework, platform: TargetPlatform.android_arm, mode: anyNamed('mode'))).thenReturn('engine');