[flutter_tools] avoid creating Android Devices if AndroidSDK cannot be found (#64524)

Avoid creating AndroidDevice discovery if the SDK cannot be located. Previously the tool would use which/where adb, however this required us to handle the AndroidSdk class being potentially null - which required an additional layer of indirection around all access. Sometimes these were forgotten leading to NPEs.

In general, not much can be done with an Android Device if the actual SDK is not installed.
This commit is contained in:
Jonah Williams 2020-08-25 11:08:27 -07:00 committed by GitHub
parent f0a3cbace3
commit 47596c6203
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 22 additions and 26 deletions

View file

@ -255,7 +255,7 @@ class AndroidDevice extends Device {
AndroidDevicePortForwarder _portForwarder; AndroidDevicePortForwarder _portForwarder;
List<String> adbCommandForDevice(List<String> args) { List<String> adbCommandForDevice(List<String> args) {
return <String>[getAdbPath(_androidSdk), '-s', id, ...args]; return <String>[_androidSdk.adbPath, '-s', id, ...args];
} }
String runAdbCheckedSync( String runAdbCheckedSync(
@ -318,13 +318,13 @@ class AndroidDevice extends Device {
try { try {
final RunResult adbVersion = await _processUtils.run( final RunResult adbVersion = await _processUtils.run(
<String>[getAdbPath(_androidSdk), 'version'], <String>[_androidSdk.adbPath, 'version'],
throwOnError: true, throwOnError: true,
); );
if (_isValidAdbVersion(adbVersion.stdout)) { if (_isValidAdbVersion(adbVersion.stdout)) {
return true; return true;
} }
_logger.printError('The ADB at "${getAdbPath(_androidSdk)}" is too old; please install version 1.0.39 or later.'); _logger.printError('The ADB at "${_androidSdk.adbPath}" is too old; please install version 1.0.39 or later.');
} on Exception catch (error, trace) { } on Exception catch (error, trace) {
_logger.printError('Error running ADB: $error', stackTrace: trace); _logger.printError('Error running ADB: $error', stackTrace: trace);
} }
@ -339,7 +339,7 @@ class AndroidDevice extends Device {
// adb server is out of date. killing.. // adb server is out of date. killing..
// * daemon started successfully * // * daemon started successfully *
await _processUtils.run( await _processUtils.run(
<String>[getAdbPath(_androidSdk), 'start-server'], <String>[_androidSdk.adbPath, 'start-server'],
throwOnError: true, throwOnError: true,
); );

View file

@ -55,14 +55,13 @@ class AndroidDevices extends PollingDeviceDiscovery {
@override @override
Future<List<Device>> pollingGetDevices({ Duration timeout }) async { Future<List<Device>> pollingGetDevices({ Duration timeout }) async {
final String adbPath = getAdbPath(_androidSdk); if (_androidSdk == null) {
if (adbPath == null) {
return <AndroidDevice>[]; return <AndroidDevice>[];
} }
String text; String text;
try { try {
text = (await _processUtils.run( text = (await _processUtils.run(
<String>[adbPath, 'devices', '-l'], <String>[_androidSdk.adbPath, 'devices', '-l'],
throwOnError: true, throwOnError: true,
)).stdout.trim(); )).stdout.trim();
} on ArgumentError catch (exception) { } on ArgumentError catch (exception) {
@ -88,12 +87,11 @@ class AndroidDevices extends PollingDeviceDiscovery {
@override @override
Future<List<String>> getDiagnostics() async { Future<List<String>> getDiagnostics() async {
final String adbPath = getAdbPath(_androidSdk); if (_androidSdk.adbPath == null) {
if (adbPath == null) {
return <String>[]; return <String>[];
} }
final RunResult result = await _processUtils.run(<String>[adbPath, 'devices', '-l']); final RunResult result = await _processUtils.run(<String>[_androidSdk.adbPath, 'devices', '-l']);
if (result.exitCode != 0) { if (result.exitCode != 0) {
return <String>[]; return <String>[];
} else { } else {

View file

@ -22,20 +22,6 @@ const String kAndroidSdkRoot = 'ANDROID_SDK_ROOT';
final RegExp _numberedAndroidPlatformRe = RegExp(r'^android-([0-9]+)$'); final RegExp _numberedAndroidPlatformRe = RegExp(r'^android-([0-9]+)$');
final RegExp _sdkVersionRe = RegExp(r'^ro.build.version.sdk=([0-9]+)$'); final RegExp _sdkVersionRe = RegExp(r'^ro.build.version.sdk=([0-9]+)$');
/// Locate ADB. Prefer to use one from an Android SDK, if we can locate that.
/// This should be used over accessing androidSdk.adbPath directly because it
/// will work for those users who have Android Platform Tools installed but
/// not the full SDK.
String getAdbPath(AndroidSdk existingSdk) {
if (existingSdk?.adbPath != null) {
return existingSdk.adbPath;
}
if (existingSdk?.latestVersion == null) {
return globals.os.which('adb')?.path;
}
return existingSdk?.adbPath;
}
// Android SDK layout: // Android SDK layout:
// $ANDROID_SDK_ROOT/platform-tools/adb // $ANDROID_SDK_ROOT/platform-tools/adb

View file

@ -58,13 +58,13 @@ class AndroidWorkflow implements Workflow {
bool get appliesToHostPlatform => _featureFlags.isAndroidEnabled; bool get appliesToHostPlatform => _featureFlags.isAndroidEnabled;
@override @override
bool get canListDevices => getAdbPath(_androidSdk) != null; bool get canListDevices => _androidSdk != null && _androidSdk.adbPath != null;
@override @override
bool get canLaunchDevices => _androidSdk != null && _androidSdk.validateSdkWellFormed().isEmpty; bool get canLaunchDevices => _androidSdk != null && _androidSdk.validateSdkWellFormed().isEmpty;
@override @override
bool get canListEmulators => _androidSdk.emulatorPath != null; bool get canListEmulators => _androidSdk != null && _androidSdk.emulatorPath != null;
} }
class AndroidValidator extends DoctorValidator { class AndroidValidator extends DoctorValidator {

View file

@ -22,6 +22,7 @@ import 'package:process/process.dart';
import '../../src/common.dart'; import '../../src/common.dart';
import '../../src/context.dart'; import '../../src/context.dart';
import '../../src/mocks.dart' show MockAndroidSdk, MockProcess, MockProcessManager, MockStdio; import '../../src/mocks.dart' show MockAndroidSdk, MockProcess, MockProcessManager, MockStdio;
import '../../src/testbed.dart';
class MockAndroidSdkVersion extends Mock implements AndroidSdkVersion {} class MockAndroidSdkVersion extends Mock implements AndroidSdkVersion {}
class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {} class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {}
@ -56,6 +57,17 @@ void main() {
return (List<String> command) => MockProcess(stdout: stdoutStream); return (List<String> command) => MockProcess(stdout: stdoutStream);
} }
testWithoutContext('AndroidWorkflow handles a null AndroidSDK', () {
final AndroidWorkflow androidWorkflow = AndroidWorkflow(
featureFlags: TestFeatureFlags(),
androidSdk: null,
);
expect(androidWorkflow.canLaunchDevices, false);
expect(androidWorkflow.canListDevices, false);
expect(androidWorkflow.canListEmulators, false);
});
testUsingContext('licensesAccepted returns LicensesAccepted.unknown if cannot find sdkmanager', () async { testUsingContext('licensesAccepted returns LicensesAccepted.unknown if cannot find sdkmanager', () async {
processManager.canRunSucceeds = false; processManager.canRunSucceeds = false;
when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager'); when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');