Revert "Add android license verification to doctor and some refactoring" (#14727)

Reports from that commit included:

* Doctor hanging
* Doctor prompting to install JDK
* AnsiStatus.cancel() printing elapsed time and extra newline
* Printing extra spinner character at the end of each line
This commit is contained in:
Todd Volkert 2018-02-15 09:13:56 -08:00 committed by GitHub
parent 615f411da7
commit d260294752
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 87 additions and 306 deletions

View file

@ -233,7 +233,7 @@ Future<String> _doctorText() async {
appContext.setVariable(Logger, logger);
await appContext.runInZone(() => doctor.diagnose(verbose: true));
await appContext.runInZone(() => doctor.diagnose());
return logger.statusText;
} catch (error, trace) {

View file

@ -12,11 +12,9 @@ import '../base/file_system.dart';
import '../base/io.dart' show ProcessResult;
import '../base/os.dart';
import '../base/platform.dart';
import '../base/process.dart';
import '../base/process_manager.dart';
import '../base/version.dart';
import '../globals.dart';
import 'android_studio.dart' as android_studio;
AndroidSdk get androidSdk => context[AndroidSdk];
@ -65,9 +63,6 @@ class AndroidSdk {
_init();
}
static const String _kJavaHomeEnvironmentVariable = 'JAVA_HOME';
static const String _kJavaExecutable = 'java';
/// The path to the Android SDK.
final String directory;
@ -296,51 +291,6 @@ class AndroidSdk {
return fs.path.join(directory, 'tools', 'bin', 'sdkmanager');
}
/// First try Java bundled with Android Studio, then sniff JAVA_HOME, then fallback to PATH.
static String findJavaBinary() {
if (android_studio.javaPath != null)
return fs.path.join(android_studio.javaPath, 'bin', 'java');
final String javaHomeEnv = platform.environment[_kJavaHomeEnvironmentVariable];
if (javaHomeEnv != null) {
// Trust JAVA_HOME.
return fs.path.join(javaHomeEnv, 'bin', 'java');
}
// MacOS specific logic to avoid popping up a dialog window.
// See: http://stackoverflow.com/questions/14292698/how-do-i-check-if-the-java-jdk-is-installed-on-mac.
if (platform.isMacOS) {
try {
final String javaHomeOutput = runCheckedSync(<String>['/usr/libexec/java_home'], hideStdout: true);
if (javaHomeOutput != null) {
final List<String> javaHomeOutputSplit = javaHomeOutput.split('\n');
if ((javaHomeOutputSplit != null) && (javaHomeOutputSplit.isNotEmpty)) {
final String javaHome = javaHomeOutputSplit[0].trim();
return fs.path.join(javaHome, 'bin', 'java');
}
}
} catch (_) { /* ignore */ }
}
// Fallback to PATH based lookup.
return os.which(_kJavaExecutable)?.path;
}
Map<String, String> _sdkManagerEnv;
Map<String, String> get sdkManagerEnv {
if (_sdkManagerEnv == null) {
// If we can locate Java, then add it to the path used to run the Android SDK manager.
final Map<String, String> _sdkManagerEnv = <String, String>{};
final String javaBinary = findJavaBinary();
if (javaBinary != null) {
_sdkManagerEnv['PATH'] =
fs.path.dirname(javaBinary) + os.pathVarSeparator + platform.environment['PATH'];
}
}
return _sdkManagerEnv;
}
/// Returns the version of the Android SDK manager tool or null if not found.
String get sdkManagerVersion {
if (!processManager.canRun(sdkManagerPath))

View file

@ -3,11 +3,12 @@
// found in the LICENSE file.
import 'dart:async';
import 'dart:convert';
import '../base/common.dart';
import '../base/context.dart';
import '../base/file_system.dart';
import '../base/io.dart';
import '../base/os.dart';
import '../base/platform.dart';
import '../base/process.dart';
import '../base/process_manager.dart';
@ -16,20 +17,10 @@ import '../base/version.dart';
import '../doctor.dart';
import '../globals.dart';
import 'android_sdk.dart';
import 'android_studio.dart' as android_studio;
AndroidWorkflow get androidWorkflow => context.putIfAbsent(AndroidWorkflow, () => new AndroidWorkflow());
enum LicensesAccepted {
none,
some,
all,
unknown,
}
final RegExp licenseCounts = new RegExp(r'(\d+) of (\d+) SDK package licenses? not accepted.');
final RegExp licenseNotAccepted = new RegExp(r'licenses? not accepted', caseSensitive: false);
final RegExp licenseAccepted = new RegExp(r'All SDK package licenses accepted.');
class AndroidWorkflow extends DoctorValidator implements Workflow {
AndroidWorkflow() : super('Android toolchain - develop for Android devices');
@ -42,8 +33,41 @@ class AndroidWorkflow extends DoctorValidator implements Workflow {
@override
bool get canLaunchDevices => androidSdk != null && androidSdk.validateSdkWellFormed().isEmpty;
static const String _kJavaHomeEnvironmentVariable = 'JAVA_HOME';
static const String _kJavaExecutable = 'java';
static const String _kJdkDownload = 'https://www.oracle.com/technetwork/java/javase/downloads/';
/// First try Java bundled with Android Studio, then sniff JAVA_HOME, then fallback to PATH.
static String _findJavaBinary() {
if (android_studio.javaPath != null)
return fs.path.join(android_studio.javaPath, 'bin', 'java');
final String javaHomeEnv = platform.environment[_kJavaHomeEnvironmentVariable];
if (javaHomeEnv != null) {
// Trust JAVA_HOME.
return fs.path.join(javaHomeEnv, 'bin', 'java');
}
// MacOS specific logic to avoid popping up a dialog window.
// See: http://stackoverflow.com/questions/14292698/how-do-i-check-if-the-java-jdk-is-installed-on-mac.
if (platform.isMacOS) {
try {
final String javaHomeOutput = runCheckedSync(<String>['/usr/libexec/java_home'], hideStdout: true);
if (javaHomeOutput != null) {
final List<String> javaHomeOutputSplit = javaHomeOutput.split('\n');
if ((javaHomeOutputSplit != null) && (javaHomeOutputSplit.isNotEmpty)) {
final String javaHome = javaHomeOutputSplit[0].trim();
return fs.path.join(javaHome, 'bin', 'java');
}
}
} catch (_) { /* ignore */ }
}
// Fallback to PATH based lookup.
return os.which(_kJavaExecutable)?.path;
}
/// Returns false if we cannot determine the Java version or if the version
/// is not compatible.
bool _checkJavaVersion(String javaBinary, List<ValidationMessage> messages) {
@ -130,7 +154,7 @@ class AndroidWorkflow extends DoctorValidator implements Workflow {
}
// Now check for the JDK.
final String javaBinary = AndroidSdk.findJavaBinary();
final String javaBinary = _findJavaBinary();
if (javaBinary == null) {
messages.add(new ValidationMessage.error(
'No Java Development Kit (JDK) found; You must have the environment '
@ -145,59 +169,10 @@ class AndroidWorkflow extends DoctorValidator implements Workflow {
return new ValidationResult(ValidationType.partial, messages, statusInfo: sdkVersionText);
}
// Check for licenses.
switch (await licensesAccepted) {
case LicensesAccepted.all:
messages.add(new ValidationMessage('All Android licenses accepted.'));
break;
case LicensesAccepted.some:
messages.add(new ValidationMessage.hint('Some Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses'));
return new ValidationResult(ValidationType.partial, messages, statusInfo: sdkVersionText);
case LicensesAccepted.none:
messages.add(new ValidationMessage.error('Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses'));
return new ValidationResult(ValidationType.partial, messages, statusInfo: sdkVersionText);
case LicensesAccepted.unknown:
messages.add(new ValidationMessage.error('Android license status unknown.'));
return new ValidationResult(ValidationType.partial, messages, statusInfo: sdkVersionText);
}
// Success.
return new ValidationResult(ValidationType.installed, messages, statusInfo: sdkVersionText);
}
Future<LicensesAccepted> get licensesAccepted async {
LicensesAccepted status = LicensesAccepted.unknown;
void _onLine(String line) {
if (licenseAccepted.hasMatch(line)) {
status = LicensesAccepted.all;
} else if (licenseCounts.hasMatch(line)) {
final Match match = licenseCounts.firstMatch(line);
if (match.group(1) != match.group(2)) {
status = LicensesAccepted.some;
} else {
status = LicensesAccepted.none;
}
} else if (licenseNotAccepted.hasMatch(line)) {
// In case the format changes, a more general match will keep doctor
// mostly working.
status = LicensesAccepted.none;
}
}
final Process process = await runDetachedWithIO(<String>[androidSdk.sdkManagerPath, '--licenses']);
process.stdin.write('n\n');
final Future<void> output = process.stdout.transform(const Utf8Decoder(allowMalformed: true)).transform(const LineSplitter()).listen(_onLine).asFuture<void>(null);
final Future<void> errors = process.stderr.transform(const Utf8Decoder(allowMalformed: true)).transform(const LineSplitter()).listen(_onLine).asFuture<void>(null);
try {
await Future.wait<void>(<Future<void>>[output, errors]).timeout(const Duration(seconds: 30));
} catch (TimeoutException) {
printTrace('Intentionally killing ${androidSdk.sdkManagerPath}');
processManager.killPid(process.pid);
}
return status;
}
/// Run the Android SDK manager tool in order to accept SDK licenses.
static Future<bool> runLicenseManager() async {
if (androidSdk == null) {
@ -205,6 +180,14 @@ class AndroidWorkflow extends DoctorValidator implements Workflow {
return false;
}
// If we can locate Java, then add it to the path used to run the Android SDK manager.
final Map<String, String> sdkManagerEnv = <String, String>{};
final String javaBinary = _findJavaBinary();
if (javaBinary != null) {
sdkManagerEnv['PATH'] =
fs.path.dirname(javaBinary) + os.pathVarSeparator + platform.environment['PATH'];
}
if (!processManager.canRun(androidSdk.sdkManagerPath))
throwToolExit(
'Android sdkmanager tool not found.\n'
@ -222,7 +205,7 @@ class AndroidWorkflow extends DoctorValidator implements Workflow {
final Process process = await runCommand(
<String>[androidSdk.sdkManagerPath, '--licenses'],
environment: androidSdk.sdkManagerEnv,
environment: sdkManagerEnv,
);
waitGroup<Null>(<Future<Null>>[

View file

@ -43,10 +43,10 @@ abstract class Logger {
Status startProgress(String message, { String progressId, bool expectSlowOperation: false });
}
/// A [Status] object includes functionality of a [Spinner], but may also display
/// diagnostic information like how long the spinner remained running between
/// [start] and [stop] (or [cancel]).
class Status extends Spinner {}
class Status {
void stop() { }
void cancel() { }
}
typedef void _FinishCallback();
@ -252,67 +252,30 @@ enum _LogType {
trace
}
class _AnsiStatus extends Status {
_AnsiStatus(this.message, this.expectSlowOperation, this.onFinish) {
stopwatch = new Stopwatch()..start();
/// A [Spinner] is a simple animation that does nothing but implement an ASCII
/// spinner. When stopped, the animation erases itself.
class Spinner {
Spinner();
/// Use this factory to generate AnsiSpinner or Spinner as necessary, and
/// start them.
factory Spinner.forContextTerminal() {
if (terminal.supportsColor)
return new AnsiSpinner()..start();
return new Spinner()..start();
}
void start() {}
void stop() {}
void cancel() {}
}
stdout.write('${message.padRight(52)} ');
stdout.write('${_progress[0]}');
/// Just a spinner, nothing more, nothing less.
class AnsiSpinner extends Spinner {
int index = 0;
bool live = true;
Timer timer;
static final List<String> _progress = <String>['-', r'\', '|', r'/', '-', r'\', '|', '/'];
void _callback(Timer _) {
stdout.write('\b${_progress[index]}');
index = ++index % _progress.length;
}
@override
void start() {
stdout.write(' ');
_callback(null);
timer = new Timer.periodic(const Duration(milliseconds: 100), _callback);
}
@override
void stop() {
if (!live)
return;
live = false;
timer.cancel();
stdout.write('\b');
}
static final List<String> _progress = <String>['-', r'\', '|', r'/', '-', r'\', '|', '/'];
@override
void cancel() => stop();
}
class _AnsiStatus extends Status with AnsiSpinner {
_AnsiStatus(this.message, this.expectSlowOperation, this.onFinish) {
stopwatch = new Stopwatch()..start();
stdout.write('${message.padRight(52)} ');
start();
}
Stopwatch stopwatch;
final String message;
final bool expectSlowOperation;
final _FinishCallback onFinish;
Stopwatch stopwatch;
Timer timer;
int index = 1;
bool live = true;
void _callback(Timer timer) {
stdout.write('\b${_progress[index]}');
index = ++index % _progress.length;
}
@override
void stop() {
@ -320,13 +283,15 @@ class _AnsiStatus extends Status with AnsiSpinner {
if (!live)
return;
super.stop();
live = false;
if (expectSlowOperation) {
print('\b\b\b\b\b${getElapsedAsSeconds(stopwatch.elapsed).padLeft(5)}');
} else {
print('\b\b\b\b\b${getElapsedAsMilliseconds(stopwatch.elapsed).padLeft(5)}');
}
timer.cancel();
}
@override
@ -335,8 +300,9 @@ class _AnsiStatus extends Status with AnsiSpinner {
if (!live)
return;
super.cancel();
live = false;
print(' ');
print('\b ');
timer.cancel();
}
}

View file

@ -17,10 +17,6 @@ typedef String StringConverter(String string);
typedef Future<dynamic> ShutdownHook();
// TODO(ianh): We have way too many ways to run subprocesses in this project.
// Convert most of these into one or more lightweight wrappers around the
// [ProcessManager] API using named parameters for the various options.
// See [here](https://github.com/flutter/flutter/pull/14535#discussion_r167041161)
// for more details.
/// The stage in which a [ShutdownHook] will be run. All shutdown hooks within
/// a given stage will be started in parallel and will be guaranteed to run to
@ -215,16 +211,6 @@ Future<Process> runDetached(List<String> cmd) {
return proc;
}
Future<Process> runDetachedWithIO(List<String> cmd, {
Map<String, String> environment
}) async {
_traceCommand(cmd);
return await processManager.start(
cmd,
mode: ProcessStartMode.DETACHED_WITH_STDIO,
);
}
Future<RunResult> runAsync(List<String> cmd, {
String workingDirectory,
bool allowReentrantFlutter: false,

View file

@ -906,9 +906,6 @@ class _AppLoggerStatus implements Status {
final int id;
final String progressId;
@override
void start() {}
@override
void stop() {
logger._status = null;

View file

@ -13,7 +13,6 @@ import 'artifacts.dart';
import 'base/common.dart';
import 'base/context.dart';
import 'base/file_system.dart';
import 'base/logger.dart';
import 'base/os.dart';
import 'base/platform.dart';
import 'base/process_manager.dart';
@ -28,12 +27,6 @@ import 'vscode/vscode_validator.dart';
Doctor get doctor => context[Doctor];
class ValidatorTask {
ValidatorTask(this.validator, this.result);
final DoctorValidator validator;
final Future<ValidationResult> result;
}
class Doctor {
List<DoctorValidator> _validators;
@ -63,16 +56,6 @@ class Doctor {
return _validators;
}
/// Return a list of [ValidatorTask] objects and starts validation on all
/// objects in [validators].
List<ValidatorTask> startValidatorTasks() {
final List<ValidatorTask> tasks = <ValidatorTask>[];
for (DoctorValidator validator in validators) {
tasks.add(new ValidatorTask(validator, validator.validate()));
}
return tasks;
}
List<Workflow> get workflows {
return new List<Workflow>.from(validators.where((DoctorValidator validator) => validator is Workflow));
}
@ -125,14 +108,9 @@ class Doctor {
bool doctorResult = true;
int issues = 0;
for (ValidatorTask validatorTask in startValidatorTasks()) {
final DoctorValidator validator = validatorTask.validator;
final Spinner status = new Spinner.forContextTerminal();
await (validatorTask.result).then<void>((_) {
status.stop();
}).whenComplete(status.cancel);
for (DoctorValidator validator in validators) {
final ValidationResult result = await validator.validate();
final ValidationResult result = await validatorTask.result;
if (result.type == ValidationType.missing) {
doctorResult = false;
}

View file

@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'dart:async';
import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
@ -16,7 +14,7 @@ import 'package:test/test.dart';
import '../src/common.dart';
import '../src/context.dart';
import '../src/mocks.dart' show MockAndroidSdk, MockProcess, MockProcessManager, MockStdio;
import '../src/mocks.dart' show MockAndroidSdk, MockProcessManager, MockStdio;
void main() {
AndroidSdk sdk;
@ -27,93 +25,12 @@ void main() {
setUp(() {
sdk = new MockAndroidSdk();
fs = new MemoryFileSystem();
fs.directory('/home/me').createSync(recursive: true);
processManager = new MockProcessManager();
stdio = new MockStdio();
});
MockProcess Function(List<String>) processMetaFactory(List<String> stdout) {
final Stream<List<int>> stdoutStream = new Stream<List<int>>.fromIterable(
stdout.map((String s) => s.codeUnits));
return (List<String> command) => new MockProcess(stdout: stdoutStream);
}
testUsingContext('licensesAccepted handles garbage/no output', () async {
MockAndroidSdk.createSdkDirectory();
when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
final AndroidWorkflow androidWorkflow = new AndroidWorkflow();
final LicensesAccepted result = await(androidWorkflow.licensesAccepted);
expect(result, equals(LicensesAccepted.unknown));
expect(processManager.commands.first, equals('/foo/bar/sdkmanager'));
expect(processManager.commands.last, equals('--licenses'));
}, overrides: <Type, Generator>{
AndroidSdk: () => sdk,
FileSystem: () => fs,
Platform: () => new FakePlatform()..environment = <String, String>{'HOME': '/home/me'},
ProcessManager: () => processManager,
Stdio: () => stdio,
});
testUsingContext('licensesAccepted works for all licenses accepted', () async {
MockAndroidSdk.createSdkDirectory();
when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
processManager.processFactory = processMetaFactory(<String>[
'[=======================================] 100% Computing updates... ',
'All SDK package licenses accepted.'
]);
final AndroidWorkflow androidWorkflow = new AndroidWorkflow();
final LicensesAccepted result = await(androidWorkflow.licensesAccepted);
expect(result, equals(LicensesAccepted.all));
}, overrides: <Type, Generator>{
AndroidSdk: () => sdk,
FileSystem: () => fs,
Platform: () => new FakePlatform()..environment = <String, String>{'HOME': '/home/me'},
ProcessManager: () => processManager,
Stdio: () => stdio,
});
testUsingContext('licensesAccepted works for some licenses accepted', () async {
MockAndroidSdk.createSdkDirectory();
when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
processManager.processFactory = processMetaFactory(<String>[
'[=======================================] 100% Computing updates... ',
'2 of 5 SDK package licenses not accepted.',
'Review licenses that have not been accepted (y/N)?',
]);
final AndroidWorkflow androidWorkflow = new AndroidWorkflow();
final LicensesAccepted result = await(androidWorkflow.licensesAccepted);
expect(result, equals(LicensesAccepted.some));
}, overrides: <Type, Generator>{
AndroidSdk: () => sdk,
FileSystem: () => fs,
Platform: () => new FakePlatform()..environment = <String, String>{'HOME': '/home/me'},
ProcessManager: () => processManager,
Stdio: () => stdio,
});
testUsingContext('licensesAccepted works for no licenses accepted', () async {
MockAndroidSdk.createSdkDirectory();
when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
processManager.processFactory = processMetaFactory(<String>[
'[=======================================] 100% Computing updates... ',
'5 of 5 SDK package licenses not accepted.',
'Review licenses that have not been accepted (y/N)?',
]);
final AndroidWorkflow androidWorkflow = new AndroidWorkflow();
final LicensesAccepted result = await(androidWorkflow.licensesAccepted);
expect(result, equals(LicensesAccepted.none));
}, overrides: <Type, Generator>{
AndroidSdk: () => sdk,
FileSystem: () => fs,
Platform: () => new FakePlatform()..environment = <String, String>{'HOME': '/home/me'},
ProcessManager: () => processManager,
Stdio: () => stdio,
});
testUsingContext('runLicenseManager succeeds for version >= 26', () async {
fs.directory('/home/me').createSync(recursive: true);
MockAndroidSdk.createSdkDirectory();
when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
when(sdk.sdkManagerVersion).thenReturn('26.0.0');
@ -128,6 +45,7 @@ void main() {
});
testUsingContext('runLicenseManager errors for version < 26', () async {
fs.directory('/home/me').createSync(recursive: true);
MockAndroidSdk.createSdkDirectory();
when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
when(sdk.sdkManagerVersion).thenReturn('25.0.0');
@ -142,6 +60,7 @@ void main() {
});
testUsingContext('runLicenseManager errors when sdkmanager is not found', () async {
fs.directory('/home/me').createSync(recursive: true);
MockAndroidSdk.createSdkDirectory();
when(sdk.sdkManagerPath).thenReturn('/foo/bar/sdkmanager');
processManager.succeed = false;

View file

@ -374,8 +374,9 @@ void main() {
final CommandRunner<Null> runner = createTestCommandRunner(command);
await runner.run(<String>['create', '--pub', '--offline', projectDir.path]);
expect(loggingProcessManager.commands.first, contains(matches(r'dart-sdk[\\/]bin[\\/]pub')));
expect(loggingProcessManager.commands.first, contains('--offline'));
final List<String> commands = loggingProcessManager.commands;
expect(commands, contains(matches(r'dart-sdk[\\/]bin[\\/]pub')));
expect(commands, contains('--offline'));
},
timeout: allowForCreateFlutterProject,
overrides: <Type, Generator>{
@ -390,8 +391,9 @@ void main() {
final CommandRunner<Null> runner = createTestCommandRunner(command);
await runner.run(<String>['create', '--pub', projectDir.path]);
expect(loggingProcessManager.commands.first, contains(matches(r'dart-sdk[\\/]bin[\\/]pub')));
expect(loggingProcessManager.commands.first, isNot(contains('--offline')));
final List<String> commands = loggingProcessManager.commands;
expect(commands, contains(matches(r'dart-sdk[\\/]bin[\\/]pub')));
expect(commands, isNot(contains('--offline')));
},
timeout: allowForCreateFlutterProject,
overrides: <Type, Generator>{
@ -486,9 +488,9 @@ Future<Null> _runFlutterTest(Directory workingDir, {String target}) async {
class MockFlutterVersion extends Mock implements FlutterVersion {}
/// A ProcessManager that invokes a real process manager, but keeps
/// track of all commands sent to it.
/// the last commands sent to it.
class LoggingProcessManager extends LocalProcessManager {
List<List<String>> commands = <List<String>>[];
List<String> commands;
@override
Future<Process> start(
@ -499,7 +501,7 @@ class LoggingProcessManager extends LocalProcessManager {
bool runInShell: false,
ProcessStartMode mode: ProcessStartMode.NORMAL,
}) {
commands.add(command);
commands = command;
return super.start(
command,
workingDirectory: workingDirectory,