Revert: Update prepare_package.dart to use the process_runner package instead...

Revert: Update prepare_package.dart to use the process_runner package instead having duplicate code. #62710 (#63608)

This reverts #62710, since it has a noSuchMethodError in some circumstances.

This isn't just a straight git revert, because I had to resolve the conflicts in the reverted pubspec.yaml files and re-run flutter update-packages --force-upgrade to make the checksums match.

dev/bots/prepare_package.dart

@@ -14,7 +14,6 @@ import 'package:http/http.dart' as http;
 import 'package:path/path.dart' as path;
 import 'package:platform/platform.dart' show Platform, LocalPlatform;
 import 'package:process/process.dart';
-import 'package:process_runner/process_runner.dart';
 
 const String chromiumRepo = 'https://chromium.googlesource.com/external/github.com/flutter/flutter';
 const String githubRepo = 'https://github.com/flutter/flutter.git';
@@ -28,12 +27,24 @@ const String baseUrl = 'https://storage.googleapis.com/flutter_infra';
 /// Exception class for when a process fails to run, so we can catch
 /// it and provide something more readable than a stack trace.
 class PreparePackageException implements Exception {
-  PreparePackageException(this.message);
+  PreparePackageException(this.message, [this.result]);
 
   final String message;
+  final ProcessResult result;
+  int get exitCode => result?.exitCode ?? -1;
 
   @override
-  String toString() => '$runtimeType: $message';
+  String toString() {
+    String output = runtimeType.toString();
+    if (message != null) {
+      output += ': $message';
+    }
+    final String stderr = result?.stderr as String ?? '';
+    if (stderr.isNotEmpty) {
+      output += ':\n$stderr';
+    }
+    return output;
+  }
 }
 
 enum Branch { dev, beta, stable }
@@ -63,6 +74,109 @@ Branch fromBranchName(String name) {
   }
 }
 
+/// A helper class for classes that want to run a process, optionally have the
+/// stderr and stdout reported as the process runs, and capture the stdout
+/// properly without dropping any.
+class ProcessRunner {
+  ProcessRunner({
+    ProcessManager processManager,
+    this.subprocessOutput = true,
+    this.defaultWorkingDirectory,
+    this.platform = const LocalPlatform(),
+  }) : processManager = processManager ?? const LocalProcessManager() {
+    environment = Map<String, String>.from(platform.environment);
+  }
+
+  /// The platform to use for a starting environment.
+  final Platform platform;
+
+  /// Set [subprocessOutput] to show output as processes run. Stdout from the
+  /// process will be printed to stdout, and stderr printed to stderr.
+  final bool subprocessOutput;
+
+  /// Set the [processManager] in order to inject a test instance to perform
+  /// testing.
+  final ProcessManager processManager;
+
+  /// Sets the default directory used when `workingDirectory` is not specified
+  /// to [runProcess].
+  final Directory defaultWorkingDirectory;
+
+  /// The environment to run processes with.
+  Map<String, String> environment;
+
+  /// Run the command and arguments in `commandLine` as a sub-process from
+  /// `workingDirectory` if set, or the [defaultWorkingDirectory] if not. Uses
+  /// [Directory.current] if [defaultWorkingDirectory] is not set.
+  ///
+  /// Set `failOk` if [runProcess] should not throw an exception when the
+  /// command completes with a non-zero exit code.
+  Future<String> runProcess(
+    List<String> commandLine, {
+    Directory workingDirectory,
+    bool failOk = false,
+  }) async {
+    workingDirectory ??= defaultWorkingDirectory ?? Directory.current;
+    if (subprocessOutput) {
+      stderr.write('Running "${commandLine.join(' ')}" in ${workingDirectory.path}.\n');
+    }
+    final List<int> output = <int>[];
+    final Completer<void> stdoutComplete = Completer<void>();
+    final Completer<void> stderrComplete = Completer<void>();
+    Process process;
+    Future<int> allComplete() async {
+      await stderrComplete.future;
+      await stdoutComplete.future;
+      return process.exitCode;
+    }
+
+    try {
+      process = await processManager.start(
+        commandLine,
+        workingDirectory: workingDirectory.absolute.path,
+        environment: environment,
+      );
+      process.stdout.listen(
+        (List<int> event) {
+          output.addAll(event);
+          if (subprocessOutput) {
+            stdout.add(event);
+          }
+        },
+        onDone: () async => stdoutComplete.complete(),
+      );
+      if (subprocessOutput) {
+        process.stderr.listen(
+          (List<int> event) {
+            stderr.add(event);
+          },
+          onDone: () async => stderrComplete.complete(),
+        );
+      } else {
+        stderrComplete.complete();
+      }
+    } on ProcessException catch (e) {
+      final String message = 'Running "${commandLine.join(' ')}" in ${workingDirectory.path} '
+          'failed with:\n${e.toString()}';
+      throw PreparePackageException(message);
+    } on ArgumentError catch (e) {
+      final String message = 'Running "${commandLine.join(' ')}" in ${workingDirectory.path} '
+          'failed with:\n${e.toString()}';
+      throw PreparePackageException(message);
+    }
+
+    final int exitCode = await allComplete();
+    if (exitCode != 0 && !failOk) {
+      final String message = 'Running "${commandLine.join(' ')}" in ${workingDirectory.path} failed';
+      throw PreparePackageException(
+        message,
+        ProcessResult(0, exitCode, null, 'returned $exitCode'),
+      );
+    }
+    return utf8.decoder.convert(output).trim();
+  }
+}
+
 typedef HttpReader = Future<Uint8List> Function(Uri url, {Map<String, String> headers});
 
 /// Creates a pre-populated Flutter archive from a git repo.
@@ -89,8 +203,9 @@ class ArchiveCreator {
         flutterRoot = Directory(path.join(tempDir.path, 'flutter')),
         httpReader = httpReader ?? http.readBytes,
         _processRunner = ProcessRunner(
-          processManager: processManager ?? const LocalProcessManager(),
-          printOutputDefault: subprocessOutput,
+          processManager: processManager,
+          subprocessOutput: subprocessOutput,
+          platform: platform,
         ) {
     _flutter = path.join(
       flutterRoot.absolute.path,
@@ -189,11 +304,12 @@ class ArchiveCreator {
     if (strict) {
       try {
         return _runGit(<String>['describe', '--tags', '--exact-match', revision]);
-      } on ProcessRunnerException catch (exception) {
+      } on PreparePackageException catch (exception) {
         throw PreparePackageException(
-            'Git error when checking for a version tag attached to revision $revision.\n'
-            'Perhaps there is no tag at that revision?:\n'
-            '$exception');
+          'Git error when checking for a version tag attached to revision $revision.\n'
+          'Perhaps there is no tag at that revision?:\n'
+          '$exception'
+        );
       }
     } else {
       return _runGit(<String>['describe', '--tags', '--abbrev=0', revision]);
@@ -252,10 +368,8 @@ class ArchiveCreator {
     // the archive, but some are checked in, and we don't want to skip
     // those.
     await _runGit(<String>['clean', '-f', '-X', '**/.packages']);
-
     /// Remove package_config files and any contents in .dart_tool
     await _runGit(<String>['clean', '-f', '-X', '**/.dart_tool']);
-
     /// Remove git subfolder from .pub-cache, this contains the flutter goldens
     /// and new flutter_gallery.
     final Directory gitCache = Directory(path.join(flutterRoot.absolute.path, '.pub-cache', 'git'));
@@ -273,25 +387,23 @@ class ArchiveCreator {
     }
   }
 
-  Future<String> _runCommand(List<String> commandline, {Directory workingDirectory}) async {
-    final ProcessRunnerResult result = await _processRunner.runProcess(
-      commandline,
+  Future<String> _runFlutter(List<String> args, {Directory workingDirectory}) {
+    return _processRunner.runProcess(
+      <String>[_flutter, ...args],
       workingDirectory: workingDirectory ?? flutterRoot,
     );
-    return result.stdout.trim();
-  }
-
-  Future<String> _runFlutter(List<String> args, {Directory workingDirectory}) {
-    return _runCommand(<String>[_flutter, ...args], workingDirectory: workingDirectory);
   }
 
   Future<String> _runGit(List<String> args, {Directory workingDirectory}) {
-    return _runCommand(<String>['git', ...args], workingDirectory: workingDirectory);
+    return _processRunner.runProcess(
+      <String>['git', ...args],
+      workingDirectory: workingDirectory ?? flutterRoot,
+    );
   }
 
   /// Unpacks the given zip file into the currentDirectory (if set), or the
   /// same directory as the archive.
-  Future<String> _unzipArchive(File archive, {Directory workingDirectory}) async {
+  Future<String> _unzipArchive(File archive, {Directory workingDirectory}) {
     workingDirectory ??= Directory(path.dirname(archive.absolute.path));
     List<String> commandLine;
     if (platform.isWindows) {
@@ -306,7 +418,7 @@ class ArchiveCreator {
         archive.absolute.path,
       ];
     }
-    return _runCommand(commandLine, workingDirectory: workingDirectory);
+    return _processRunner.runProcess(commandLine, workingDirectory: workingDirectory);
   }
 
   /// Create a zip archive from the directory source.
@@ -314,7 +426,7 @@ class ArchiveCreator {
     List<String> commandLine;
     if (platform.isWindows) {
       // Unhide the .git folder, https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/attrib.
-      await _runCommand(
+      await _processRunner.runProcess(
         <String>['attrib', '-h', '.git'],
         workingDirectory: Directory(source.absolute.path),
       );
@@ -335,24 +447,20 @@ class ArchiveCreator {
         path.basename(source.path),
       ];
     }
-    return _runCommand(
+    return _processRunner.runProcess(
       commandLine,
       workingDirectory: Directory(path.dirname(source.absolute.path)),
     );
   }
 
   /// Create a tar archive from the directory source.
-  Future<String> _createTarArchive(File output, Directory source) async {
-    final ProcessRunnerResult result = await _processRunner.runProcess(
-      <String>[
-        'tar',
-        'cJf',
-        output.absolute.path,
-        path.basename(source.absolute.path),
-      ],
-      workingDirectory: Directory(path.dirname(source.absolute.path)),
-    );
-    return result.stdout.trim();
+  Future<String> _createTarArchive(File output, Directory source) {
+    return _processRunner.runProcess(<String>[
+      'tar',
+      'cJf',
+      output.absolute.path,
+      path.basename(source.absolute.path),
+    ], workingDirectory: Directory(path.dirname(source.absolute.path)));
   }
 }
 
@@ -371,7 +479,7 @@ class ArchivePublisher {
         metadataGsPath = '$gsReleaseFolder/${getMetadataFilename(platform)}',
         _processRunner = ProcessRunner(
           processManager: processManager,
-          printOutputDefault: subprocessOutput,
+          subprocessOutput: subprocessOutput,
         );
 
   final Platform platform;
@@ -474,25 +582,18 @@ class ArchivePublisher {
     bool failOk = false,
   }) async {
     if (platform.isWindows) {
-      final ProcessRunnerResult result = await _processRunner.runProcess(
-        <String>[
-          'python',
-          path.join(platform.environment['DEPOT_TOOLS'], 'gsutil.py'),
-          '--',
-          ...args
-        ],
+      return _processRunner.runProcess(
+        <String>['python', path.join(platform.environment['DEPOT_TOOLS'], 'gsutil.py'), '--', ...args],
         workingDirectory: workingDirectory,
         failOk: failOk,
       );
-      return result.stdout.trim();
     }
 
-    final ProcessRunnerResult result = await _processRunner.runProcess(
+    return _processRunner.runProcess(
       <String>['gsutil.py', '--', ...args],
       workingDirectory: workingDirectory,
       failOk: failOk,
     );
-    return result.stdout.trim();
   }
 
   Future<String> _cloudCopy(String src, String dest) async {
@@ -619,13 +720,7 @@ Future<void> main(List<String> rawArguments) async {
   }
 
   final Branch branch = fromBranchName(parsedArguments['branch'] as String);
-  final ArchiveCreator creator = ArchiveCreator(
-    tempDir,
-    outputDir,
-    revision,
-    branch,
-    strict: parsedArguments['publish'] as bool,
-  );
+  final ArchiveCreator creator = ArchiveCreator(tempDir, outputDir, revision, branch, strict: parsedArguments['publish'] as bool);
   int exitCode = 0;
   String message;
   try {
@@ -641,12 +736,12 @@ Future<void> main(List<String> rawArguments) async {
       );
       await publisher.publishArchive();
     }
-  } on ProcessRunnerException catch (e, stack) {
+  } on PreparePackageException catch (e) {
     exitCode = e.exitCode;
-    message = e.message + '\n' + stack.toString();
-  } catch (e, stack) {
+    message = e.message;
+  } catch (e) {
     exitCode = -1;
-    message = e.toString() + '\n' + stack.toString();
+    message = e.toString();
   } finally {
     if (removeTempDir) {
       tempDir.deleteSync(recursive: true);

dev/bots/pubspec.yaml

@@ -18,7 +18,6 @@ dependencies:
   path: 1.8.0-nullsafety
   platform: 2.2.1
   process: 3.0.13
-  process_runner: 2.0.4
   test: 1.16.0-nullsafety.1
 
   _discoveryapis_commons: 0.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
@@ -78,4 +77,4 @@ dev_dependencies:
   mockito: 4.1.1
   test_api: 0.2.19-nullsafety
 
-# PUBSPEC CHECKSUM: 52ec
+# PUBSPEC CHECKSUM: d9a1

dev/bots/test/prepare_package_test.dart

@@ -9,7 +9,6 @@ import 'dart:typed_data';
 
 import 'package:mockito/mockito.dart';
 import 'package:path/path.dart' as path;
-import 'package:process_runner/process_runner.dart';
 import 'package:platform/platform.dart' show FakePlatform;
 
 import '../prepare_package.dart';
@@ -21,15 +20,15 @@ void main() {
   test('Throws on missing executable', () async {
     // Uses a *real* process manager, since we want to know what happens if
     // it can't find an executable.
-    final ProcessRunner processRunner = ProcessRunner(printOutputDefault: false);
+    final ProcessRunner processRunner = ProcessRunner(subprocessOutput: false);
     expect(
         expectAsync1((List<String> commandLine) async {
           return processRunner.runProcess(commandLine);
         })(<String>['this_executable_better_not_exist_2857632534321']),
-        throwsA(isA<ProcessRunnerException>()));
+        throwsA(isA<PreparePackageException>()));
     try {
       await processRunner.runProcess(<String>['this_executable_better_not_exist_2857632534321']);
-    } on ProcessRunnerException catch (e) {
+    } on PreparePackageException catch (e) {
       expect(
         e.message,
         contains('Invalid argument(s): Cannot find executable for this_executable_better_not_exist_2857632534321.'),
@@ -50,8 +49,8 @@ void main() {
           'echo test': <ProcessResult>[ProcessResult(0, 0, 'output', 'error')],
         };
         final ProcessRunner processRunner = ProcessRunner(
-            printOutputDefault: false, processManager: fakeProcessManager);
-        final String output = (await processRunner.runProcess(<String>['echo', 'test'])).stdout;
+            subprocessOutput: false, platform: platform, processManager: fakeProcessManager);
+        final String output = await processRunner.runProcess(<String>['echo', 'test']);
         expect(output, equals('output'));
       });
       test('Throws on process failure', () async {
@@ -60,12 +59,12 @@ void main() {
           'echo test': <ProcessResult>[ProcessResult(0, -1, 'output', 'error')],
         };
         final ProcessRunner processRunner = ProcessRunner(
-            printOutputDefault: false, processManager: fakeProcessManager);
+            subprocessOutput: false, platform: platform, processManager: fakeProcessManager);
         expect(
             expectAsync1((List<String> commandLine) async {
               return processRunner.runProcess(commandLine);
             })(<String>['echo', 'test']),
-            throwsA(isA<ProcessRunnerException>()));
+            throwsA(isA<PreparePackageException>()));
       });
     });
     group('ArchiveCreator for $platformName', () {
@@ -188,7 +187,7 @@ void main() {
           'git reset --hard $testRef': <ProcessResult>[ProcessResult(0, -1, 'output2', '')],
         };
         processManager.fakeResults = calls;
-        expect(expectAsync0(creator.initializeRepo), throwsA(isA<ProcessRunnerException>()));
+        expect(expectAsync0(creator.initializeRepo), throwsA(isA<PreparePackageException>()));
       });
 
       test('non-strict mode calls the right commands', () async {