Convert to using process_runner package

Author: gspencergoog

ci/bin/lint.dart

@@ -7,267 +7,11 @@
 
 import 'dart:async' show Completer;
 import 'dart:convert' show jsonDecode, utf8, LineSplitter;
-import 'dart:io'
-    show
-        File,
-        Process,
-        ProcessResult,
-        ProcessException,
-        exit,
-        Directory,
-        FileSystemEntity,
-        Platform,
-        stderr,
-        stdout;
+import 'dart:io' show File, exit, Directory, FileSystemEntity, Platform, stderr;
 
 import 'package:args/args.dart';
 import 'package:path/path.dart' as path;
-
-Platform defaultPlatform = Platform();
-
-/// Exception class for when a process fails to run, so we can catch
-/// it and provide something more readable than a stack trace.
-class ProcessRunnerException implements Exception {
-  ProcessRunnerException(this.message, {this.result});
-
-  final String message;
-  final ProcessResult result;
-
-  int get exitCode => result?.exitCode ?? -1;
-
-  @override
-  String toString() {
-    String output = runtimeType.toString();
-    output += ': $message';
-    final String stderr = (result?.stderr ?? '') as String;
-    if (stderr.isNotEmpty) {
-      output += ':\n$stderr';
-    }
-    return output;
-  }
-}
-
-class ProcessRunnerResult {
-  const ProcessRunnerResult(this.exitCode, this.stdout, this.stderr, this.output);
-  final int exitCode;
-  final List<int> stdout;
-  final List<int> stderr;
-  final List<int> output;
-}
-
-/// 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({
-    this.defaultWorkingDirectory,
-  });
-
-  /// 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 = Map<String, String>.from(Platform.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 a non-zero exit code.
-  Future<ProcessRunnerResult> runProcess(
-    List<String> commandLine, {
-    Directory workingDirectory,
-    bool printOutput = false,
-    bool failOk = false,
-    Stream<List<int>> stdin,
-  }) async {
-    workingDirectory ??= defaultWorkingDirectory ?? Directory.current;
-    if (printOutput) {
-      stderr.write('Running "${commandLine.join(' ')}" in ${workingDirectory.path}.\n');
-    }
-    final List<int> stdoutOutput = <int>[];
-    final List<int> stderrOutput = <int>[];
-    final List<int> combinedOutput = <int>[];
-    final Completer<void> stdoutComplete = Completer<void>();
-    final Completer<void> stderrComplete = Completer<void>();
-    final Completer<void> stdinComplete = Completer<void>();
-
-    Process process;
-    Future<int> allComplete() async {
-      if (stdin != null) {
-        await stdinComplete.future;
-        await process?.stdin?.close();
-      }
-      await stderrComplete.future;
-      await stdoutComplete.future;
-      return process?.exitCode ?? Future<int>.value(0);
-    }
-
-    try {
-      process = await Process.start(
-        commandLine.first,
-        commandLine.sublist(1),
-        workingDirectory: workingDirectory.absolute.path,
-        environment: environment,
-        runInShell: false,
-      );
-      if (stdin != null) {
-        stdin.listen((List<int> data) {
-          process?.stdin?.add(data);
-        }, onDone: () async => stdinComplete.complete());
-      }
-      process.stdout.listen(
-        (List<int> event) {
-          stdoutOutput.addAll(event);
-          combinedOutput.addAll(event);
-          if (printOutput) {
-            stdout.add(event);
-          }
-        },
-        onDone: () async => stdoutComplete.complete(),
-      );
-      process.stderr.listen(
-        (List<int> event) {
-          stderrOutput.addAll(event);
-          combinedOutput.addAll(event);
-          if (printOutput) {
-            stderr.add(event);
-          }
-        },
-        onDone: () async => stderrComplete.complete(),
-      );
-    } on ProcessException catch (e) {
-      final String message = 'Running "${commandLine.join(' ')}" in ${workingDirectory.path} '
-          'failed with:\n${e.toString()}';
-      throw ProcessRunnerException(message);
-    } on ArgumentError catch (e) {
-      final String message = 'Running "${commandLine.join(' ')}" in ${workingDirectory.path} '
-          'failed with:\n${e.toString()}';
-      throw ProcessRunnerException(message);
-    }
-
-    final int exitCode = await allComplete();
-    if (exitCode != 0 && !failOk) {
-      final String message =
-          'Running "${commandLine.join(' ')}" in ${workingDirectory.path} failed';
-      throw ProcessRunnerException(
-        message,
-        result: ProcessResult(
-            0, exitCode, null, 'exited with code $exitCode\n${utf8.decode(combinedOutput)}'),
-      );
-    }
-    return ProcessRunnerResult(exitCode, stdoutOutput, stderrOutput, combinedOutput);
-  }
-}
-
-class WorkerJob {
-  WorkerJob(
-    this.name,
-    this.args, {
-    this.workingDirectory,
-    this.printOutput = false,
-  });
-
-  /// The name of the job.
-  final String name;
-
-  /// The arguments for the process, including the command name as args[0].
-  final List<String> args;
-
-  /// The working directory that the command should be executed in.
-  final Directory workingDirectory;
-
-  /// Whether or not this command should print it's stdout when it runs.
-  final bool printOutput;
-
-  @override
-  String toString() {
-    return args.join(' ');
-  }
-}
-
-/// A pool of worker processes that will keep [numWorkers] busy until all of the
-/// (presumably single-threaded) processes are finished.
-class ProcessPool {
-  ProcessPool({int numWorkers}) : numWorkers = numWorkers ?? Platform.numberOfProcessors;
-
-  ProcessRunner processRunner = ProcessRunner();
-  int numWorkers;
-  List<WorkerJob> pendingJobs = <WorkerJob>[];
-  List<WorkerJob> failedJobs = <WorkerJob>[];
-  Map<WorkerJob, Future<List<int>>> inProgressJobs = <WorkerJob, Future<List<int>>>{};
-  Map<WorkerJob, ProcessRunnerResult> completedJobs = <WorkerJob, ProcessRunnerResult>{};
-  Completer<Map<WorkerJob, ProcessRunnerResult>> completer =
-      Completer<Map<WorkerJob, ProcessRunnerResult>>();
-
-  void _printReport() {
-    final int totalJobs = completedJobs.length + inProgressJobs.length + pendingJobs.length;
-    final String percent =
-        totalJobs == 0 ? '100' : ((100 * completedJobs.length) ~/ totalJobs).toString().padLeft(3);
-    final String completed = completedJobs.length.toString().padLeft(3);
-    final String total = totalJobs.toString().padRight(3);
-    final String inProgress = inProgressJobs.length.toString().padLeft(2);
-    final String pending = pendingJobs.length.toString().padLeft(3);
-    stdout.write('Jobs: $percent% done, $completed/$total completed, $inProgress in '
-        'progress, $pending pending.  \r');
-  }
-
-  Future<List<int>> _scheduleJob(WorkerJob job) async {
-    final Completer<List<int>> jobDone = Completer<List<int>>();
-    final List<int> output = <int>[];
-    try {
-      completedJobs[job] = await processRunner.runProcess(
-        job.args,
-        workingDirectory: job.workingDirectory,
-        printOutput: job.printOutput,
-      );
-    } catch (e) {
-      failedJobs.add(job);
-      if (e is ProcessRunnerException) {
-        print(e.toString());
-        print('${utf8.decode(output)}');
-      } else {
-        print('\nJob $job failed: $e');
-      }
-    } finally {
-      inProgressJobs.remove(job);
-      if (pendingJobs.isNotEmpty) {
-        final WorkerJob newJob = pendingJobs.removeAt(0);
-        inProgressJobs[newJob] = _scheduleJob(newJob);
-      } else {
-        if (inProgressJobs.isEmpty) {
-          completer.complete(completedJobs);
-        }
-      }
-      jobDone.complete(output);
-      _printReport();
-    }
-    return jobDone.future;
-  }
-
-  Future<Map<WorkerJob, ProcessRunnerResult>> startWorkers(List<WorkerJob> jobs) async {
-    assert(inProgressJobs.isEmpty);
-    assert(failedJobs.isEmpty);
-    assert(completedJobs.isEmpty);
-    if (jobs.isEmpty) {
-      return <WorkerJob, ProcessRunnerResult>{};
-    }
-    pendingJobs = jobs;
-    for (int i = 0; i < numWorkers; ++i) {
-      if (pendingJobs.isEmpty) {
-        break;
-      }
-      final WorkerJob job = pendingJobs.removeAt(0);
-      inProgressJobs[job] = _scheduleJob(job);
-    }
-    return completer.future.then((Map<WorkerJob, ProcessRunnerResult> result) {
-      stdout.flush();
-      return result;
-    });
-  }
-}
+import 'package:process_runner/process_runner.dart';
 
 String _linterOutputHeader = '''
 ┌──────────────────────────┐
@@ -313,20 +57,33 @@ bool containsAny(File file, Iterable<File> queries) {
   return queries.where((File query) => path.equals(query.path, file.path)).isNotEmpty;
 }
 
-/// Returns a list of all files with current changes or differ from `master`.
+/// Returns a list of all non-deleted files which differ from the nearest
+/// merge-base with `master`. If it can't find a fork point, uses the default
+/// merge-base.
 Future<List<File>> getListOfChangedFiles(Directory repoPath) async {
   final ProcessRunner processRunner = ProcessRunner(defaultWorkingDirectory: repoPath);
-  String branch = 'upstream/master';
-  final ProcessResult fetchResult = Process.runSync('git', <String>['fetch', 'upstream', 'master']);
+  final ProcessRunnerResult fetchResult = await processRunner.runProcess(
+    <String>['git', 'fetch', 'upstream', 'master'],
+    failOk: true,
+  );
   if (fetchResult.exitCode != 0) {
-    branch = 'origin/master';
-    Process.runSync('git', <String>['fetch', 'origin', 'master']);
+    await processRunner.runProcess(<String>['git', 'fetch', 'origin', 'master']);
   }
   final Set<String> result = <String>{};
-  final ProcessRunnerResult diffResult = await processRunner.runProcess(
-      <String>['git', 'diff', '--name-only', '--diff-filter=ACMRT', if (branch.isNotEmpty) branch]);
-
-  result.addAll(utf8.decode(diffResult.stdout).split('\n').where(isNonEmptyString));
+  ProcessRunnerResult mergeBaseResult = await processRunner.runProcess(
+      <String>['git', 'merge-base', '--fork-point', 'FETCH_HEAD', 'HEAD'],
+      failOk: true);
+  if (mergeBaseResult.exitCode != 0) {
+    if (verbose) {
+      stderr.writeln("Didn't find a fork point, falling back to default merge base.");
+    }
+    mergeBaseResult = await processRunner
+        .runProcess(<String>['git', 'merge-base', 'FETCH_HEAD', 'HEAD'], failOk: false);
+  }
+  final String mergeBase = mergeBaseResult.stdout.trim();
+  final ProcessRunnerResult masterResult = await processRunner
+      .runProcess(<String>['git', 'diff', '--name-only', '--diff-filter=ACMRT', mergeBase]);
+  result.addAll(masterResult.stdout.split('\n').where(isNonEmptyString));
   return result.map<File>((String filePath) => File(path.join(repoPath.path, filePath))).toList();
 }
 
@@ -409,6 +166,8 @@ void main(List<String> arguments) async {
     _usage(parser);
   }
 
+  print(_linterOutputHeader);
+
   final String checksArg = options.wasParsed('checks') ? options['checks'] as String : '';
   final String checks = checksArg.isNotEmpty ? '--checks=$checksArg' : '--config=';
   final bool lintAll =
@@ -439,12 +198,17 @@ void main(List<String> arguments) async {
   final List<Command> changedFileBuildCommands =
       buildCommands.where((Command x) => containsAny(x.file, changedFiles)).toList();
 
+  if (changedFileBuildCommands.isEmpty) {
+    print('No changed files that have build commands associated with them '
+        'were found.');
+    exit(0);
+  }
+
   if (verbose) {
     print('Found ${changedFileBuildCommands.length} files that have build '
         'commands associated with them and can be lint checked.');
   }
 
-  print(_linterOutputHeader);
   int exitCode = 0;
   final List<WorkerJob> jobs = <WorkerJob>[];
   for (Command command in changedFileBuildCommands) {
@@ -453,23 +217,23 @@ void main(List<String> arguments) async {
       final List<String> args = <String>[command.file.path, checks, '--'];
       args.addAll(tidyArgs?.split(' ') ?? <String>[]);
       print('🔶 linting ${command.file}');
-      jobs.add(WorkerJob(command.file.path, <String>[tidyPath, ...args],
-          workingDirectory: command.directory));
+      jobs.add(WorkerJob(<String>[tidyPath, ...args],
+          workingDirectory: command.directory, name: 'clang-tidy on ${command.file.path}'));
     } else {
       print('🔷 ignoring ${command.file}');
     }
   }
   final ProcessPool pool = ProcessPool();
-  final Map<WorkerJob, ProcessRunnerResult> results = await pool.startWorkers(jobs);
-  print('\n');
-  for (final WorkerJob job in results.keys) {
-    if (results[job].stdout.isEmpty) {
+
+  await for (final WorkerJob job in pool.startWorkers(jobs)) {
+    if (job.result.stdout.isEmpty) {
       continue;
     }
     print('❌ Failures for ${job.name}:');
-    print(utf8.decode(results[job].stdout));
+    print(job.result.stdout);
     exitCode = 1;
   }
+  print('\n');
   if (exitCode == 0) {
     print('No lint problems found.');
   }

ci/pubspec.yaml

@@ -3,6 +3,7 @@ name: ci_scripts
 dependencies:
   args: ^1.6.0
   path: ^1.7.0
+  process_runner: ^2.0.3
 
 environment:
-  sdk: '>=2.8.0 <3.0.0'
+  sdk: '>=2.8.0 <3.0.0'