Run pub in interactive mode in flutter packages pub (#11700)

packages/flutter_tools/lib/executable.dart

@@ -118,6 +118,7 @@ Future<int> run(List<String> args, List<FlutterCommand> subCommands, {
     // in those locations as well to see if you need a similar update there.
 
     // Seed these context entries first since others depend on them
+    context.putIfAbsent(Stdio, () => const Stdio());
     context.putIfAbsent(Platform, () => const LocalPlatform());
     context.putIfAbsent(FileSystem, () => const LocalFileSystem());
     context.putIfAbsent(ProcessManager, () => const LocalProcessManager());

packages/flutter_tools/lib/src/base/io.dart

@@ -26,10 +26,11 @@
 /// increase the API surface that we have to test in Flutter tools, and the APIs
 /// in `dart:io` can sometimes be hard to use in tests.
 import 'dart:async';
-import 'dart:io' as io show exit, ProcessSignal;
+import 'dart:io' as io show exit, IOSink, ProcessSignal, stderr, stdin, stdout;
 
 import 'package:meta/meta.dart';
 
+import 'context.dart';
 import 'platform.dart';
 import 'process.dart';
 
@@ -62,10 +63,11 @@ export 'dart:io'
         ProcessStartMode,
         // RandomAccessFile  NO! Use `file_system.dart`
         ServerSocket,
-        stderr,
-        stdin,
+        // stderr,           NO! Use `io.dart`
+        // stdin,            NO! Use `io.dart`
+        Stdin,
         StdinException,
-        stdout,
+        // stdout,           NO! Use `io.dart`
         Socket,
         SocketException,
         SYSTEM_ENCODING,
@@ -143,3 +145,32 @@ class _PosixProcessSignal extends ProcessSignal {
     return super.watch();
   }
 }
+
+class Stdio {
+  const Stdio();
+
+  Stream<List<int>> get stdin => io.stdin;
+  io.IOSink get stdout => io.stdout;
+  io.IOSink get stderr => io.stderr;
+}
+
+io.IOSink get stderr {
+  if (context == null)
+    return io.stderr;
+  final Stdio contextStreams = context[Stdio];
+  return contextStreams.stderr;
+}
+
+Stream<List<int>> get stdin {
+  if (context == null)
+    return io.stdin;
+  final Stdio contextStreams = context[Stdio];
+  return contextStreams.stdin;
+}
+
+io.IOSink get stdout {
+  if (context == null)
+    return io.stdout;
+  final Stdio contextStreams = context[Stdio];
+  return contextStreams.stdout;
+}

packages/flutter_tools/lib/src/base/process.dart

@@ -166,6 +166,30 @@ Future<int> runCommandAndStreamOutput(List<String> cmd, {
   return await process.exitCode;
 }
 
+/// Runs the [command] interactively, connecting the stdin/stdout/stderr
+/// streams of this process to those of the child process. Completes with
+/// the exit code of the child process.
+Future<int> runInteractively(List<String> command, {
+  String workingDirectory,
+  bool allowReentrantFlutter: false,
+  Map<String, String> environment
+}) async {
+  final Process process = await runCommand(
+    command,
+    workingDirectory: workingDirectory,
+    allowReentrantFlutter: allowReentrantFlutter,
+    environment: environment,
+  );
+  process.stdin.addStream(stdin);
+  // Wait for stdout and stderr to be fully processed, because process.exitCode
+  // may complete first.
+  Future.wait<dynamic>(<Future<dynamic>>[
+    stdout.addStream(process.stdout),
+    stderr.addStream(process.stderr),
+  ]);
+  return await process.exitCode;
+}
+
 Future<Null> runAndKill(List<String> cmd, Duration timeout) {
   final Future<Process> proc = runDetached(cmd);
   return new Future<Null>.delayed(timeout, () async {

packages/flutter_tools/lib/src/base/terminal.dart

@@ -9,7 +9,7 @@ import 'package:quiver/strings.dart';
 
 import '../globals.dart';
 import 'context.dart';
-import 'io.dart';
+import 'io.dart' as io;
 import 'platform.dart';
 
 final AnsiTerminal _kAnsiTerminal = new AnsiTerminal();
@@ -61,18 +61,21 @@ class AnsiTerminal {
     // [_ENOTTY] or [_INVALID_HANDLE], we should check beforehand if stdin is
     // connected to a terminal or not.
     // (Requires https://github.com/dart-lang/sdk/issues/29083 to be resolved.)
-    try {
-      // The order of setting lineMode and echoMode is important on Windows.
-      if (value) {
-        stdin.echoMode = false;
-        stdin.lineMode = false;
-      } else {
-        stdin.lineMode = true;
-        stdin.echoMode = true;
+    final Stream<List<int>> stdin = io.stdin;
+    if (stdin is io.Stdin) {
+      try {
+        // The order of setting lineMode and echoMode is important on Windows.
+        if (value) {
+          stdin.echoMode = false;
+          stdin.lineMode = false;
+        } else {
+          stdin.lineMode = true;
+          stdin.echoMode = true;
+        }
+      } on io.StdinException catch (error) {
+        if (!_lineModeIgnorableErrors.contains(error.osError?.errorCode))
+          rethrow;
       }
-    } on StdinException catch (error) {
-      if (!_lineModeIgnorableErrors.contains(error.osError?.errorCode))
-        rethrow;
     }
   }
 
@@ -83,7 +86,7 @@ class AnsiTerminal {
   /// Useful when the console is in [singleCharMode].
   Stream<String> get onCharInput {
     if (_broadcastStdInString == null)
-      _broadcastStdInString = stdin.transform(ASCII.decoder).asBroadcastStream();
+      _broadcastStdInString = io.stdin.transform(ASCII.decoder).asBroadcastStream();
     return _broadcastStdInString;
   }
 

packages/flutter_tools/lib/src/commands/packages.dart

@@ -124,5 +124,5 @@ class PackagesPassthroughCommand extends FlutterCommand {
   }
 
   @override
-  Future<Null> runCommand() => pub(argResults.rest, retry: false);
+  Future<Null> runCommand() => pubInteractively(argResults.rest);
 }

packages/flutter_tools/lib/src/dart/pub.dart

@@ -78,23 +78,25 @@ Future<Null> pubGet({
 
 typedef String MessageFilter(String message);
 
+/// Runs pub in 'batch' mode, forwarding complete lines written by pub to its
+/// stdout/stderr streams to the corresponding stream of this process, optionally
+/// applying filtering. The pub process will not receive anything on its stdin stream.
 Future<Null> pub(List<String> arguments, {
   String directory,
   MessageFilter filter,
   String failureMessage: 'pub failed',
   @required bool retry,
 }) async {
-  final List<String> command = <String>[ sdkBinaryName('pub') ]..addAll(arguments);
   int attempts = 0;
   int duration = 1;
   int code;
   while (true) {
     attempts += 1;
     code = await runCommandAndStreamOutput(
-      command,
+      _pubCommand(arguments),
       workingDirectory: directory,
       mapFunction: filter,
-      environment: <String, String>{ 'FLUTTER_ROOT': Cache.flutterRoot, _pubEnvironmentKey: _getPubEnvironmentValue() }
+      environment: _pubEnvironment,
     );
     if (code != 69) // UNAVAILABLE in https://github.com/dart-lang/pub/blob/master/lib/src/exit_codes.dart
       break;
@@ -108,6 +110,32 @@ Future<Null> pub(List<String> arguments, {
     throwToolExit('$failureMessage ($code)', exitCode: code);
 }
 
+/// Runs pub in 'interactive' mode, directly piping the stdin stream of this
+/// process to that of pub, and the stdout/stderr stream of pub to the corresponding
+/// streams of this process.
+Future<Null> pubInteractively(List<String> arguments, {
+  String directory,
+}) async {
+  final int code = await runInteractively(
+    _pubCommand(arguments),
+    workingDirectory: directory,
+    environment: _pubEnvironment,
+  );
+  if (code != 0)
+    throwToolExit('pub finished with exit code $code', exitCode: code);
+}
+
+/// The command used for running pub.
+List<String> _pubCommand(List<String> arguments) {
+  return <String>[ sdkBinaryName('pub') ]..addAll(arguments);
+}
+
+/// The full environment used when running pub.
+Map<String, String> get _pubEnvironment => <String, String>{
+  'FLUTTER_ROOT': Cache.flutterRoot,
+  _pubEnvironmentKey: _getPubEnvironmentValue(),
+};
+
 final RegExp _analyzerWarning = new RegExp(r'^! \w+ [^ ]+ from path \.\./\.\./bin/cache/dart-sdk/lib/\w+$');
 
 /// The console environment key used by the pub tool.

packages/flutter_tools/test/commands/packages_test.dart

@@ -3,9 +3,11 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:convert';
+import 'dart:io' show IOSink;
 
 import 'package:args/command_runner.dart';
-import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/file_system.dart' hide IOSink;
 import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/commands/packages.dart';
@@ -69,47 +71,72 @@ void main() {
   });
 
   group('packages test/pub', () {
-    final List<List<dynamic>> log = <List<dynamic>>[];
+    MockProcessManager mockProcessManager;
+    MockStdio mockStdio;
+
+    setUp(() {
+      mockProcessManager = new MockProcessManager();
+      mockStdio = new MockStdio();
+    });
+
     testUsingContext('test', () async {
-      log.clear();
       await createTestCommandRunner(new PackagesCommand()).run(<String>['packages', 'test']);
-      expect(log, hasLength(1));
-      expect(log[0], hasLength(3));
-      expect(log[0][0], matches(r'dart-sdk[\\/]bin[\\/]pub'));
-      expect(log[0][1], 'run');
-      expect(log[0][2], 'test');
+      final List<String> commands = mockProcessManager.commands;
+      expect(commands, hasLength(3));
+      expect(commands[0], matches(r'dart-sdk[\\/]bin[\\/]pub'));
+      expect(commands[1], 'run');
+      expect(commands[2], 'test');
     }, overrides: <Type, Generator>{
-      ProcessManager: () {
-        return new MockProcessManager((List<dynamic> command) {
-          log.add(command);
-        });
-      },
+      ProcessManager: () => mockProcessManager,
+      Stdio: () => mockStdio,
     });
+
     testUsingContext('run', () async {
-      log.clear();
       await createTestCommandRunner(new PackagesCommand()).run(<String>['packages', '--verbose', 'pub', 'run', '--foo', 'bar']);
-      expect(log, hasLength(1));
-      expect(log[0], hasLength(4));
-      expect(log[0][0], matches(r'dart-sdk[\\/]bin[\\/]pub'));
-      expect(log[0][1], 'run');
-      expect(log[0][2], '--foo');
-      expect(log[0][3], 'bar');
+      final List<String> commands = mockProcessManager.commands;
+      expect(commands, hasLength(4));
+      expect(commands[0], matches(r'dart-sdk[\\/]bin[\\/]pub'));
+      expect(commands[1], 'run');
+      expect(commands[2], '--foo');
+      expect(commands[3], 'bar');
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => mockProcessManager,
+      Stdio: () => mockStdio,
+    });
+
+    testUsingContext('publish', () async {
+      final PromptingProcess process = new PromptingProcess();
+      mockProcessManager.processFactory = (List<String> commands) => process;
+      final Future<Null> runPackages = createTestCommandRunner(new PackagesCommand()).run(<String>['packages', 'pub', 'publish']);
+      final Future<Null> runPrompt = process.showPrompt('Proceed (y/n)? ', <String>['hello', 'world']);
+      final Future<Null> simulateUserInput = new Future<Null>(() {
+        mockStdio.simulateStdin('y');
+      });
+      await Future.wait(<Future<Null>>[runPackages, runPrompt, simulateUserInput]);
+      final List<String> commands = mockProcessManager.commands;
+      expect(commands, hasLength(2));
+      expect(commands[0], matches(r'dart-sdk[\\/]bin[\\/]pub'));
+      expect(commands[1], 'publish');
+      final List<String> stdout = mockStdio.writtenToStdout;
+      expect(stdout, hasLength(4));
+      expect(stdout.sublist(0, 2), contains('Proceed (y/n)? '));
+      expect(stdout.sublist(0, 2), contains('y\n'));
+      expect(stdout[2], 'hello\n');
+      expect(stdout[3], 'world\n');
     }, overrides: <Type, Generator>{
-      ProcessManager: () {
-        return new MockProcessManager((List<dynamic> command) {
-          log.add(command);
-        });
-      },
+      ProcessManager: () => mockProcessManager,
+      Stdio: () => mockStdio,
     });
   });
 }
 
-typedef void StartCallback(List<dynamic> command);
+/// A strategy for creating Process objects from a list of commands.
+typedef Process ProcessFactory(List<String> command);
 
+/// A ProcessManager that starts Processes by delegating to a ProcessFactory.
 class MockProcessManager implements ProcessManager {
-  MockProcessManager(this.onStart);
-
-  final StartCallback onStart;
+  ProcessFactory processFactory = (List<String> commands) => new MockProcess();
+  List<String> commands;
 
   @override
   Future<Process> start(
@@ -120,20 +147,63 @@ class MockProcessManager implements ProcessManager {
     bool runInShell: false,
     ProcessStartMode mode: ProcessStartMode.NORMAL,
   }) {
-    onStart(command);
-    return new Future<Process>.value(new MockProcess());
+    commands = command;
+    return new Future<Process>.value(processFactory(command));
+  }
+
+  @override
+  dynamic noSuchMethod(Invocation invocation) => null;
+}
+
+/// A process that prompts the user to proceed, then asynchronously writes
+/// some lines to stdout before it exits.
+class PromptingProcess implements Process {
+  Future<Null> showPrompt(String prompt, List<String> outputLines) async {
+    _stdoutController.add(UTF8.encode(prompt));
+    final List<int> bytesOnStdin = await _stdin.future;
+    // Echo stdin to stdout.
+    _stdoutController.add(bytesOnStdin);
+    if (bytesOnStdin[0] == UTF8.encode('y')[0]) {
+      for (final String line in outputLines)
+        _stdoutController.add(UTF8.encode('$line\n'));
+    }
+    await _stdoutController.close();
+  }
+
+  final StreamController<List<int>> _stdoutController = new StreamController<List<int>>();
+  final CompleterIOSink _stdin = new CompleterIOSink();
+
+  @override
+  Stream<List<int>> get stdout => _stdoutController.stream;
+
+  @override
+  Stream<List<int>> get stderr => const Stream<List<int>>.empty();
+
+  @override
+  IOSink get stdin => _stdin;
+
+  @override
+  Future<int> get exitCode async {
+    await _stdoutController.done;
+    return 0;
   }
 
   @override
   dynamic noSuchMethod(Invocation invocation) => null;
 }
 
+/// An inactive process that collects stdin and produces no output.
 class MockProcess implements Process {
+  final IOSink _stdin = new MemoryIOSink();
+
+  @override
+  Stream<List<int>> get stdout => const Stream<List<int>>.empty();
+
   @override
-  Stream<List<int>> get stdout => new MockStream<List<int>>();
+  Stream<List<int>> get stderr => const Stream<List<int>>.empty();
 
   @override
-  Stream<List<int>> get stderr => new MockStream<List<int>>();
+  IOSink get stdin => _stdin;
 
   @override
   Future<int> get exitCode => new Future<int>.value(0);
@@ -142,29 +212,97 @@ class MockProcess implements Process {
   dynamic noSuchMethod(Invocation invocation) => null;
 }
 
-class MockStream<T> implements Stream<T> {
+/// An IOSink that completes a future with the first line written to it.
+class CompleterIOSink extends MemoryIOSink {
+  final Completer<List<int>> _completer = new Completer<List<int>>();
+
+  Future<List<int>> get future => _completer.future;
+
   @override
-  Stream<S> transform<S>(StreamTransformer<T, S> streamTransformer) => new MockStream<S>();
+  void add(List<int> data) {
+    if (!_completer.isCompleted)
+      _completer.complete(data);
+    super.add(data);
+  }
+}
+
+/// A Stdio that collects stdout and supports simulated stdin.
+class MockStdio extends Stdio {
+  final MemoryIOSink _stdout = new MemoryIOSink();
+  final StreamController<List<int>> _stdin = new StreamController<List<int>>();
 
   @override
-  Stream<T> where(bool test(T event)) => new MockStream<T>();
+  IOSink get stdout => _stdout;
 
   @override
-  StreamSubscription<T> listen(void onData(T event), {Function onError, void onDone(), bool cancelOnError}) {
-    return new MockStreamSubscription<T>();
+  Stream<List<int>> get stdin => _stdin.stream;
+
+  void simulateStdin(String line) {
+    _stdin.add(UTF8.encode('$line\n'));
   }
 
-  @override
-  dynamic noSuchMethod(Invocation invocation) => null;
+  List<String> get writtenToStdout => _stdout.writes.map(_stdout.encoding.decode).toList();
 }
 
-class MockStreamSubscription<T> implements StreamSubscription<T> {
+/// An IOSink that collects whatever is written to it.
+class MemoryIOSink implements IOSink {
   @override
-  Future<E> asFuture<E>([E futureValue]) => new Future<E>.value();
+  Encoding encoding = UTF8;
+
+  final List<List<int>> writes = <List<int>>[];
 
   @override
-  Future<Null> cancel() => null;
+  void add(List<int> data) {
+    writes.add(data);
+  }
 
   @override
-  dynamic noSuchMethod(Invocation invocation) => null;
+  Future<Null> addStream(Stream<List<int>> stream) {
+    final Completer<Null> completer = new Completer<Null>();
+    stream.listen((List<int> data) {
+      add(data);
+    }).onDone(() => completer.complete(null));
+    return completer.future;
+  }
+
+  @override
+  void writeCharCode(int charCode) {
+    add(<int>[charCode]);
+  }
+
+  @override
+  void write(Object obj) {
+    add(encoding.encode('$obj'));
+  }
+
+  @override
+  void writeln([Object obj = ""]) {
+    add(encoding.encode('$obj\n'));
+  }
+
+  @override
+  void writeAll(Iterable<dynamic> objects, [String separator = ""]) {
+    bool addSeparator = false;
+    for (dynamic object in objects) {
+      if (addSeparator) {
+        write(separator);
+      }
+      write(object);
+      addSeparator = true;
+    }
+  }
+
+  @override
+  void addError(dynamic error, [StackTrace stackTrace]) {
+    throw new UnimplementedError();
+  }
+
+  @override
+  Future<Null> get done => close();
+
+  @override
+  Future<Null> close() async => null;
+
+  @override
+  Future<Null> flush() async => null;
 }

packages/flutter_tools/test/src/context.dart

@@ -8,6 +8,7 @@ import 'package:flutter_tools/src/artifacts.dart';
 import 'package:flutter_tools/src/base/config.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/os.dart';
 import 'package:flutter_tools/src/base/platform.dart';
@@ -73,6 +74,7 @@ void testUsingContext(String description, dynamic testMethod(), {
 
     // The context always starts with these value since others depend on them.
     testContext
+      ..putIfAbsent(Stdio, () => const Stdio())
       ..putIfAbsent(Platform, () => const LocalPlatform())
       ..putIfAbsent(FileSystem, () => const LocalFileSystem())
       ..putIfAbsent(ProcessManager, () => const LocalProcessManager())