From 6f5e88a59f5ac01fd55b5c3a11b7c2ddc6c7402c Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 21 Feb 2019 15:57:20 -0800 Subject: [PATCH] Allow for gradle downloading missing SDK assets (#28097) * Allow for gradle downloading missing SDK assets if SDK licenses are present. * Fix license path for missing sdkmanager * Cirrus re-run... * Fix condition check * rename props, add docs, rename method * fix tests after param changes * Fix weird path for flutter run and add tests * remove print, fix tests --- .../lib/src/android/android_device.dart | 2 +- .../lib/src/android/android_sdk.dart | 36 +++++++-- .../lib/src/android/android_workflow.dart | 18 +++-- .../flutter_tools/lib/src/android/apk.dart | 12 +-- .../flutter_tools/lib/src/android/gradle.dart | 15 ++++ .../lib/src/application_package.dart | 3 + .../lib/src/base/user_messages.dart | 17 +++- .../test/android/android_device_test.dart | 9 ++- .../test/android/android_sdk_test.dart | 1 + .../test/android/android_workflow_test.dart | 24 +++++- .../test/application_package_test.dart | 77 +++++++++++++++++++ packages/flutter_tools/test/src/mocks.dart | 20 +++-- 12 files changed, 201 insertions(+), 33 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index 73aa44b6bb6..45e46bcde89 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -372,7 +372,7 @@ class AndroidDevice extends Device { if (buildInfo.targetPlatform == null && devicePlatform == TargetPlatform.android_arm64) buildInfo = buildInfo.withTargetPlatform(TargetPlatform.android_arm64); - if (!prebuiltApplication) { + if (!prebuiltApplication || androidSdk.licensesAvailable && androidSdk.latestVersion == null) { printTrace('Building APK'); final FlutterProject project = await FlutterProject.current(); await buildApk( diff --git a/packages/flutter_tools/lib/src/android/android_sdk.dart b/packages/flutter_tools/lib/src/android/android_sdk.dart index 0bb5823b3c6..081cce5976d 100644 --- a/packages/flutter_tools/lib/src/android/android_sdk.dart +++ b/packages/flutter_tools/lib/src/android/android_sdk.dart @@ -263,7 +263,7 @@ class AndroidNdk { class AndroidSdk { AndroidSdk(this.directory, [this.ndk]) { - _init(); + reinitialize(); } static const String _javaHomeEnvironmentVariable = 'JAVA_HOME'; @@ -278,6 +278,23 @@ class AndroidSdk { List _sdkVersions; AndroidSdkVersion _latestVersion; + /// Whether the `platform-tools` directory exists in the Android SDK. + /// + /// It is possible to have an Android SDK folder that is missing this with + /// the expectation that it will be downloaded later, e.g. by gradle or the + /// sdkmanager. The [licensesAvailable] property should be used to determine + /// whether the licenses are at least possibly accepted. + bool get platformToolsAvailable => fs.directory(fs.path.join(directory, 'platform-tools')).existsSync(); + + /// Whether the `licenses` directory exists in the Android SDK. + /// + /// The existence of this folder normally indicates that the SDK licenses have + /// been accepted, e.g. via the sdkmanager, Android Studio, or by copying them + /// from another workstation such as in CI scenarios. If these files are valid + /// gradle or the sdkmanager will be able to download and use other parts of + /// the SDK on demand. + bool get licensesAvailable => fs.directory(fs.path.join(directory, 'licenses')).existsSync(); + static AndroidSdk locateAndroidSdk() { String findAndroidHomeDir() { String androidHomeDir; @@ -348,7 +365,7 @@ class AndroidSdk { } static bool validSdkDirectory(String dir) { - return fs.isDirectorySync(fs.path.join(dir, 'platform-tools')); + return fs.isDirectorySync(fs.path.join(dir, 'licenses')); } List get sdkVersions => _sdkVersions; @@ -376,8 +393,8 @@ class AndroidSdk { /// Validate the Android SDK. This returns an empty list if there are no /// issues; otherwise, it returns a list of issues found. List validateSdkWellFormed() { - if (!processManager.canRun(adbPath)) - return ['Android SDK file not found: $adbPath.']; + if (adbPath == null || !processManager.canRun(adbPath)) + return ['Android SDK file not found: ${adbPath ?? 'adb'}.']; if (sdkVersions.isEmpty || latestVersion == null) { final StringBuffer msg = StringBuffer('No valid Android SDK platforms found in ${_platformsDir.path}.'); @@ -396,7 +413,10 @@ class AndroidSdk { } String getPlatformToolsPath(String binaryName) { - return fs.path.join(directory, 'platform-tools', binaryName); + final String path = fs.path.join(directory, 'platform-tools', binaryName); + if (fs.file(path).existsSync()) + return path; + return null; } String getEmulatorPath() { @@ -420,7 +440,11 @@ class AndroidSdk { return null; } - void _init() { + /// Sets up various paths used internally. + /// + /// This method should be called in a case where the tooling may have updated + /// SDK artifacts, such as after running a gradle build. + void reinitialize() { List buildTools = []; // 19.1.0, 22.0.1, ... final Directory buildToolsDir = fs.directory(fs.path.join(directory, 'build-tools')); diff --git a/packages/flutter_tools/lib/src/android/android_workflow.dart b/packages/flutter_tools/lib/src/android/android_workflow.dart index d7376fbad44..5da192c1216 100644 --- a/packages/flutter_tools/lib/src/android/android_workflow.dart +++ b/packages/flutter_tools/lib/src/android/android_workflow.dart @@ -105,6 +105,11 @@ class AndroidValidator extends DoctorValidator { return ValidationResult(ValidationType.missing, messages); } + if (androidSdk.licensesAvailable && !androidSdk.platformToolsAvailable) { + messages.add(ValidationMessage.hint(userMessages.androidSdkLicenseOnly(kAndroidHome))); + return ValidationResult(ValidationType.partial, messages); + } + messages.add(ValidationMessage(userMessages.androidSdkLocation(androidSdk.directory))); messages.add(ValidationMessage(androidSdk.ndk == null @@ -249,7 +254,9 @@ class AndroidLicenseValidator extends DoctorValidator { } } - _ensureCanRunSdkManager(); + if (!_canRunSdkManager()) { + return LicensesAccepted.unknown; + } final Process process = await runCommand( [androidSdk.sdkManagerPath, '--licenses'], @@ -279,7 +286,9 @@ class AndroidLicenseValidator extends DoctorValidator { return false; } - _ensureCanRunSdkManager(); + if (!_canRunSdkManager()) { + throwToolExit(userMessages.androidMissingSdkManager(androidSdk.sdkManagerPath)); + } final Version sdkManagerVersion = Version.parse(androidSdk.sdkManagerVersion); if (sdkManagerVersion == null || sdkManagerVersion.major < 26) { @@ -306,10 +315,9 @@ class AndroidLicenseValidator extends DoctorValidator { return exitCode == 0; } - static void _ensureCanRunSdkManager() { + static bool _canRunSdkManager() { assert(androidSdk != null); final String sdkManagerPath = androidSdk.sdkManagerPath; - if (!processManager.canRun(sdkManagerPath)) - throwToolExit(userMessages.androidMissingSdkManager(sdkManagerPath)); + return processManager.canRun(sdkManagerPath); } } diff --git a/packages/flutter_tools/lib/src/android/apk.dart b/packages/flutter_tools/lib/src/android/apk.dart index 46de467f217..4a0ded0dcdf 100644 --- a/packages/flutter_tools/lib/src/android/apk.dart +++ b/packages/flutter_tools/lib/src/android/apk.dart @@ -8,7 +8,6 @@ import 'package:meta/meta.dart'; import '../base/common.dart'; import '../build_info.dart'; -import '../globals.dart'; import '../project.dart'; import 'android_sdk.dart'; @@ -32,18 +31,11 @@ Future buildApk({ if (androidSdk == null) throwToolExit('No Android SDK found. Try setting the ANDROID_SDK_ROOT environment variable.'); - final List validationResult = androidSdk.validateSdkWellFormed(); - if (validationResult.isNotEmpty) { - for (String message in validationResult) { - printError(message, wrap: false); - } - throwToolExit('Try re-installing or updating your Android SDK.'); - } - - return buildGradleProject( + await buildGradleProject( project: project, buildInfo: buildInfo, target: target, isBuildingBundle: false ); + androidSdk.reinitialize(); } diff --git a/packages/flutter_tools/lib/src/android/gradle.dart b/packages/flutter_tools/lib/src/android/gradle.dart index ae2b9fbf8e1..de9f2c0bd05 100644 --- a/packages/flutter_tools/lib/src/android/gradle.dart +++ b/packages/flutter_tools/lib/src/android/gradle.dart @@ -111,6 +111,21 @@ Future _gradleProject() async { return _cachedGradleProject; } +/// Runs `gradlew dependencies`, ensuring that dependencies are resolved and +/// potentially downloaded. +Future checkGradleDependencies() async { + final Status progress = logger.startProgress('Ensuring gradle dependencies are up to date...', timeout: kSlowOperation); + final FlutterProject flutterProject = await FlutterProject.current(); + final String gradle = await _ensureGradle(flutterProject); + await runCheckedAsync( + [gradle, 'dependencies'], + workingDirectory: flutterProject.android.hostAppGradleRoot.path, + environment: _gradleEnv, + ); + androidSdk.reinitialize(); + progress.stop(); +} + // Note: Dependencies are resolved and possibly downloaded as a side-effect // of calculating the app properties using Gradle. This may take minutes. Future _readGradleProject() async { diff --git a/packages/flutter_tools/lib/src/application_package.dart b/packages/flutter_tools/lib/src/application_package.dart index e274519f3c8..3822ea260be 100644 --- a/packages/flutter_tools/lib/src/application_package.dart +++ b/packages/flutter_tools/lib/src/application_package.dart @@ -34,6 +34,9 @@ class ApplicationPackageFactory { case TargetPlatform.android_arm64: case TargetPlatform.android_x64: case TargetPlatform.android_x86: + if (androidSdk?.licensesAvailable == true && androidSdk.latestVersion == null) { + await checkGradleDependencies(); + } return applicationBinary == null ? await AndroidApk.fromAndroidProject((await FlutterProject.current()).android) : AndroidApk.fromApk(applicationBinary); diff --git a/packages/flutter_tools/lib/src/base/user_messages.dart b/packages/flutter_tools/lib/src/base/user_messages.dart index f7ceef6ea41..230e7e4e08a 100644 --- a/packages/flutter_tools/lib/src/base/user_messages.dart +++ b/packages/flutter_tools/lib/src/base/user_messages.dart @@ -45,6 +45,15 @@ class UserMessages { String androidCantRunJavaBinary(String javaBinary) => 'Cannot execute $javaBinary to determine the version'; String get androidUnknownJavaVersion => 'Could not determine java version'; String androidJavaVersion(String javaVersion) => 'Java version $javaVersion'; + String androidSdkLicenseOnly(String envKey) => + 'Android SDK contains licenses only.\n' + 'Your first build of an Android application will take longer than usual, ' + 'while gradle downloads the missing components. This functionality will ' + 'only work if the licenses in the licenses folder in $envKey are valid.\n' + 'If the Android SDK has been installed to another location, set $envKey to that location.\n' + 'You may also want to add it to your PATH environment variable.\n\n' + 'Certain features, such as `flutter emulators` and `flutter devices`, will ' + 'not work without the currently missing SDK components.'; String androidBadSdkDir(String envKey, String homeDir) => '$envKey = $homeDir\n' 'but Android SDK not found at this location.'; @@ -53,7 +62,7 @@ class UserMessages { 'Install Android Studio from: https://developer.android.com/studio/index.html\n' 'On first launch it will assist you in installing the Android SDK components.\n' '(or visit https://flutter.io/setup/#android-setup for detailed instructions).\n' - 'If Android SDK has been installed to a custom location, set $envKey to that location.\n' + 'If the Android SDK has been installed to a custom location, set $envKey to that location.\n' 'You may also want to add it to your PATH environment variable.\n'; String androidSdkLocation(String directory) => 'Android SDK at $directory'; String androidSdkPlatformToolsVersion(String platform, String tools) => @@ -75,7 +84,11 @@ class UserMessages { String get androidLicensesAll => 'All Android licenses accepted.'; String get androidLicensesSome => 'Some Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses'; String get androidLicensesNone => 'Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses'; - String get androidLicensesUnknown => 'Android license status unknown.'; + String get androidLicensesUnknown => + 'Android license status unknown.\n' + 'Try re-installing or updating your Android SDK Manager.\n' + 'See https://developer.android.com/studio/#downloads or visit ' + 'https://flutter.io/setup/#android-setup for detailed instructions.'; String androidSdkManagerOutdated(String managerPath) => 'A newer version of the Android SDK is required. To update, run:\n' '$managerPath --update\n'; diff --git a/packages/flutter_tools/test/android/android_device_test.dart b/packages/flutter_tools/test/android/android_device_test.dart index b6988f5425f..d7945e535a1 100644 --- a/packages/flutter_tools/test/android/android_device_test.dart +++ b/packages/flutter_tools/test/android/android_device_test.dart @@ -27,16 +27,23 @@ void main() { }); group('getAdbDevices', () { + final MockProcessManager mockProcessManager = MockProcessManager(); testUsingContext('throws on missing adb path', () { final Directory sdkDir = MockAndroidSdk.createSdkDirectory(); Config.instance.setValue('android-sdk', sdkDir.path); final File adbExe = fs.file(getAdbPath(androidSdk)); - adbExe.deleteSync(); + when(mockProcessManager.runSync( + [adbExe.path, 'devices', '-l'], + )) + .thenAnswer( + (_) => throw ArgumentError(adbExe.path), + ); expect(() => getAdbDevices(), throwsToolExit(message: RegExp('Unable to run "adb".*${adbExe.path}'))); }, overrides: { AndroidSdk: () => MockAndroidSdk(), FileSystem: () => MemoryFileSystem(), + ProcessManager: () => mockProcessManager, }); testUsingContext('physical devices', () { diff --git a/packages/flutter_tools/test/android/android_sdk_test.dart b/packages/flutter_tools/test/android/android_sdk_test.dart index 9ef1711d998..1fe7f32ffc3 100644 --- a/packages/flutter_tools/test/android/android_sdk_test.dart +++ b/packages/flutter_tools/test/android/android_sdk_test.dart @@ -226,6 +226,7 @@ class MockBrokenAndroidSdk extends Mock implements AndroidSdk { }) { final Directory dir = fs.systemTempDirectory.createTempSync('flutter_mock_android_sdk.'); + _createSdkFile(dir, 'licenses/dummy'); _createSdkFile(dir, 'platform-tools/adb'); _createSdkFile(dir, 'build-tools/sda/aapt'); diff --git a/packages/flutter_tools/test/android/android_workflow_test.dart b/packages/flutter_tools/test/android/android_workflow_test.dart index 4764b1bf3ac..c98847449ca 100644 --- a/packages/flutter_tools/test/android/android_workflow_test.dart +++ b/packages/flutter_tools/test/android/android_workflow_test.dart @@ -42,11 +42,12 @@ void main() { return (List command) => MockProcess(stdout: stdoutStream); } - testUsingContext('licensesAccepted throws if cannot run sdkmanager', () async { + testUsingContext('licensesAccepted returns LicensesAccepted.unknown if cannot run sdkmanager', () async { processManager.succeed = false; when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager'); final AndroidLicenseValidator licenseValidator = AndroidLicenseValidator(); - expect(licenseValidator.licensesAccepted, throwsToolExit()); + final LicensesAccepted licenseStatus = await licenseValidator.licensesAccepted; + expect(licenseStatus, LicensesAccepted.unknown); }, overrides: { AndroidSdk: () => sdk, FileSystem: () => fs, @@ -178,8 +179,27 @@ void main() { Stdio: () => stdio, }); + testUsingContext('detects license-only SDK installation', () async { + when(sdk.licensesAvailable).thenReturn(true); + when(sdk.platformToolsAvailable).thenReturn(false); + final ValidationResult validationResult = await AndroidValidator().validate(); + expect(validationResult.type, ValidationType.partial); + expect( + validationResult.messages.last.message, + userMessages.androidSdkLicenseOnly(kAndroidHome), + ); + }, overrides: { + AndroidSdk: () => sdk, + FileSystem: () => fs, + Platform: () => FakePlatform()..environment = {'HOME': '/home/me'}, + ProcessManager: () => processManager, + Stdio: () => stdio, + }); + testUsingContext('detects minium required SDK and buildtools', () async { final AndroidSdkVersion mockSdkVersion = MockAndroidSdkVersion(); + when(sdk.licensesAvailable).thenReturn(true); + when(sdk.platformToolsAvailable).thenReturn(true); // Test with invalid SDK and build tools when(mockSdkVersion.sdkLevel).thenReturn(26); diff --git a/packages/flutter_tools/test/application_package_test.dart b/packages/flutter_tools/test/application_package_test.dart index 48a1396debc..099e8a20f3d 100644 --- a/packages/flutter_tools/test/application_package_test.dart +++ b/packages/flutter_tools/test/application_package_test.dart @@ -3,18 +3,24 @@ // found in the LICENSE file. import 'dart:convert'; +import 'dart:io' show ProcessResult; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/platform.dart'; +import 'package:flutter_tools/src/build_info.dart'; +import 'package:flutter_tools/src/cache.dart'; +import 'package:flutter_tools/src/project.dart'; import 'package:mockito/mockito.dart'; import 'package:flutter_tools/src/application_package.dart'; +import 'package:flutter_tools/src/android/android_sdk.dart'; import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/os.dart'; import 'package:flutter_tools/src/ios/ios_workflow.dart'; +import 'package:process/process.dart'; import 'src/common.dart'; import 'src/context.dart'; @@ -24,7 +30,78 @@ final Map noColorTerminalOverride = { Platform: _kNoColorTerminalPlatform, }; +class MockitoProcessManager extends Mock implements ProcessManager {} +class MockitoAndroidSdk extends Mock implements AndroidSdk {} +class MockitoAndroidSdkVersion extends Mock implements AndroidSdkVersion {} + void main() { + group('Apk with partial Android SDK works', () { + AndroidSdk sdk; + ProcessManager mockProcessManager; + MemoryFileSystem fs; + File gradle; + final Map overrides = { + AndroidSdk: () => sdk, + ProcessManager: () => mockProcessManager, + FileSystem: () => fs, + }; + + setUp(() async { + sdk = MockitoAndroidSdk(); + mockProcessManager = MockitoProcessManager(); + fs = MemoryFileSystem(); + Cache.flutterRoot = '../..'; + when(sdk.licensesAvailable).thenReturn(true); + when(mockProcessManager.canRun(any)).thenReturn(true); + when(mockProcessManager.run( + any, + workingDirectory: anyNamed('workingDirectory'), + environment: anyNamed('environment'), + )).thenAnswer((_) async => ProcessResult(1, 0, 'stdout', 'stderr')); + when(mockProcessManager.runSync(any)).thenReturn(ProcessResult(1, 0, 'stdout', 'stderr')); + final FlutterProject project = await FlutterProject.current(); + gradle = fs.file(project.android.hostAppGradleRoot.childFile( + platform.isWindows ? 'gradlew.bat' : 'gradlew', + ).path)..createSync(recursive: true); + }); + + testUsingContext('Licenses available, build tools not, apk exists', () async { + when(sdk.latestVersion).thenReturn(null); + final FlutterProject project = await FlutterProject.current(); + final File gradle = project.android.hostAppGradleRoot.childFile( + platform.isWindows ? 'gradlew.bat' : 'gradlew', + )..createSync(recursive: true); + + await ApplicationPackageFactory.instance.getPackageForPlatform( + TargetPlatform.android_arm, + applicationBinary: fs.file('app.apk'), + ); + verify( + mockProcessManager.run( + argThat(equals([gradle.path, 'dependencies'])), + workingDirectory: anyNamed('workingDirectory'), + environment: anyNamed('environment'), + ), + ).called(1); + }, overrides: overrides); + + testUsingContext('Licenses available, build tools available, does not call gradle dependencies', () async { + final AndroidSdkVersion sdkVersion = MockitoAndroidSdkVersion(); + when(sdk.latestVersion).thenReturn(sdkVersion); + + await ApplicationPackageFactory.instance.getPackageForPlatform( + TargetPlatform.android_arm, + ); + verifyNever( + mockProcessManager.run( + argThat(equals([gradle.path, 'dependencies'])), + workingDirectory: anyNamed('workingDirectory'), + environment: anyNamed('environment'), + ), + ); + }, overrides: overrides); + }); + group('ApkManifestData', () { test('Parses manifest with an Activity that has enabled set to true, action set to android.intent.action.MAIN and category set to android.intent.category.LAUNCHER', () { final ApkManifestData data = ApkManifestData.parseFromXmlDump(_aaptDataWithExplicitEnabledAndMainLauncherActivity); diff --git a/packages/flutter_tools/test/src/mocks.dart b/packages/flutter_tools/test/src/mocks.dart index 015b96e7ff3..861c78ca340 100644 --- a/packages/flutter_tools/test/src/mocks.dart +++ b/packages/flutter_tools/test/src/mocks.dart @@ -44,16 +44,24 @@ class MockAndroidSdk extends Mock implements AndroidSdk { int ndkVersion = 16, bool withNdkSysroot = false, bool withSdkManager = true, + bool withPlatformTools = true, + bool withBuildTools = true, }) { final Directory dir = fs.systemTempDirectory.createTempSync('flutter_mock_android_sdk.'); - _createSdkFile(dir, 'platform-tools/adb'); + _createDir(dir, 'licenses'); - _createSdkFile(dir, 'build-tools/19.1.0/aapt'); - _createSdkFile(dir, 'build-tools/22.0.1/aapt'); - _createSdkFile(dir, 'build-tools/23.0.2/aapt'); - if (withAndroidN) - _createSdkFile(dir, 'build-tools/24.0.0-preview/aapt'); + if (withPlatformTools) { + _createSdkFile(dir, 'platform-tools/adb'); + } + + if (withBuildTools) { + _createSdkFile(dir, 'build-tools/19.1.0/aapt'); + _createSdkFile(dir, 'build-tools/22.0.1/aapt'); + _createSdkFile(dir, 'build-tools/23.0.2/aapt'); + if (withAndroidN) + _createSdkFile(dir, 'build-tools/24.0.0-preview/aapt'); + } _createSdkFile(dir, 'platforms/android-22/android.jar'); _createSdkFile(dir, 'platforms/android-23/android.jar');