Return exit codes from run, and send them to the parent isolate. (#878)

Author: jakemac53

analysis_options.yaml

@@ -6,6 +6,8 @@ analyzer:
     unused_local_variable: error
     dead_code: error
     override_on_non_overriding_method: error
+  exclude:
+    - "test/goldens/generated_build_script.dart"
 linter:
   rules:
     # Errors

build_runner/CHANGELOG.md

@@ -1,3 +1,11 @@
+## 0.7.7
+
+- The top level `run` method now returns an `int` which represents an `exitCode`
+  for the command that was executed.
+  - For now we still set the exitCode manually as well but this will likely
+    change in the next breaking release. In manual scripts you should `await`
+    the call to `run` and assign that to `exitCode` to be future-proofed.
+
 ## 0.7.6
 
 - Update to package:build version `0.12.0`.

build_runner/bin/build_runner.dart

@@ -41,17 +41,27 @@ Future<Null> main(List<String> args) async {
 
   var exitPort = new ReceivePort();
   var errorPort = new ReceivePort();
-  var errorListener = errorPort.listen((e) => stderr.writeAll(e as List, '\n'));
-  await Isolate.spawnUri(new Uri.file(p.absolute(scriptLocation)), args, null,
+  var messagePort = new ReceivePort();
+  var errorListener = errorPort.listen((e) {
+    stderr.writeAll(e as List, '\n');
+    if (exitCode == 0) exitCode = 1;
+  });
+  await Isolate.spawnUri(
+      new Uri.file(p.absolute(scriptLocation)), args, messagePort.sendPort,
       onExit: exitPort.sendPort, onError: errorPort.sendPort);
+  try {
+    exitCode = await messagePort.first as int;
+  } on StateError catch (_) {
+    if (exitCode == 0) exitCode = 1;
+  }
   await exitPort.first;
   await errorListener.cancel();
   await logListener?.cancel();
 }
 
 const _generateCommand = 'generate-build-script';
 
-class _GenerateBuildScript extends Command {
+class _GenerateBuildScript extends Command<int> {
   @override
   final description = 'Generate a script to run builds and print the file path '
       'with no other logging. Useful for wrapping builds with other tools.';

build_runner/lib/src/build_script_generate/build_script_generate.dart

@@ -61,14 +61,25 @@ Future<Iterable<Expression>> _findBuilderApplications() async {
 /// A method forwarding to `run`.
 Method _main() => new Method((b) => b
   ..name = 'main'
-  ..lambda = true
+  ..modifier = MethodModifier.async
   ..requiredParameters.add(new Parameter((b) => b
     ..name = 'args'
     ..type = new TypeReference((b) => b
       ..symbol = 'List'
       ..types.add(refer('String')))))
-  ..body = refer('run', 'package:build_runner/build_runner.dart')
-      .call([refer('args'), refer('_builders')]).code);
+  ..optionalParameters.add(new Parameter((b) => b
+    ..name = 'sendPort'
+    ..type = refer('SendPort', 'dart:isolate')))
+  ..body = new Block.of([
+    refer('run', 'package:build_runner/build_runner.dart')
+        .call([refer('args'), refer('_builders')])
+        .awaited
+        .assignVar('result')
+        .statement,
+    refer('sendPort')
+        .nullSafeProperty('send')
+        .call([refer('result')]).statement,
+  ]));
 
 /// An expression calling `apply` with appropriate setup for a Builder.
 Expression _applyBuilder(BuilderDefinition definition) =>

build_runner/lib/src/entrypoint/options.dart

@@ -23,7 +23,7 @@ const _verbose = 'verbose';
 final _pubBinary = Platform.isWindows ? 'pub.bat' : 'pub';
 
 /// Unified command runner for all build_runner commands.
-class BuildCommandRunner extends CommandRunner {
+class BuildCommandRunner extends CommandRunner<int> {
   final List<BuilderApplication> builderApplications;
 
   BuildCommandRunner(List<BuilderApplication> builderApplications)
@@ -139,7 +139,7 @@ class _ServeTarget {
   _ServeTarget(this.dir, this.port);
 }
 
-abstract class _BaseCommand extends Command {
+abstract class _BaseCommand extends Command<int> {
   List<BuilderApplication> get builderApplications =>
       (runner as BuildCommandRunner).builderApplications;
 
@@ -201,14 +201,19 @@ class _BuildCommand extends _BaseCommand {
       'Performs a single build on the specified targets and then exits.';
 
   @override
-  Future<Null> run() async {
+  Future<int> run() async {
     var options = _readOptions();
-    await build(builderApplications,
+    var result = await build(builderApplications,
         deleteFilesByDefault: options.deleteFilesByDefault,
         enableLowResourcesMode: options.enableLowResourcesMode,
         assumeTty: options.assumeTty,
         outputDir: options.outputDir,
         verbose: options.verbose);
+    if (result.status == BuildStatus.success) {
+      return ExitCode.success.code;
+    } else {
+      return 1;
+    }
   }
 }
 
@@ -224,7 +229,7 @@ class _WatchCommand extends _BaseCommand {
       'rebuilding as appropriate.';
 
   @override
-  Future<Null> run() async {
+  Future<int> run() async {
     var options = _readOptions();
     var handler = await watch(builderApplications,
         deleteFilesByDefault: options.deleteFilesByDefault,
@@ -234,6 +239,7 @@ class _WatchCommand extends _BaseCommand {
         verbose: options.verbose);
     await handler.currentBuild;
     await handler.buildResults.drain();
+    return ExitCode.success.code;
   }
 }
 
@@ -257,7 +263,7 @@ class _ServeCommand extends _WatchCommand {
   _ServeOptions _readOptions() => new _ServeOptions.fromParsedArgs(argResults);
 
   @override
-  Future<Null> run() async {
+  Future<int> run() async {
     var options = _readOptions();
     var handler = await watch(builderApplications,
         deleteFilesByDefault: options.deleteFilesByDefault,
@@ -273,6 +279,8 @@ class _ServeCommand extends _WatchCommand {
     }
     await handler.buildResults.drain();
     await Future.wait(servers.map((server) => server.close()));
+
+    return ExitCode.success.code;
   }
 }
 
@@ -291,39 +299,52 @@ class _TestCommand extends _BaseCommand {
       'using the compiled assets.';
 
   @override
-  Future<Null> run() async {
-    var packageGraph = new PackageGraph.forThisPackage();
-    _ensureBuildTestDependency(packageGraph);
-    var options = _readOptions();
-    // We always need an output dir when running tests, so we create a tmp dir
-    // if the user didn't specify one.
-    var outputDir = options.outputDir ??
-        Directory.systemTemp
-            .createTempSync('build_runner_test')
-            .absolute
-            .uri
-            .toFilePath();
-    await build(builderApplications,
-        deleteFilesByDefault: options.deleteFilesByDefault,
-        enableLowResourcesMode: options.enableLowResourcesMode,
-        assumeTty: options.assumeTty,
-        outputDir: outputDir,
-        verbose: options.verbose,
-        packageGraph: packageGraph);
-
-    // Run the tests!
-    await _runTests(outputDir);
-
-    // Clean up the output dir if one wasn't explicitly asked for.
-    if (options.outputDir == null) {
-      await new Directory(outputDir).delete(recursive: true);
+  Future<int> run() async {
+    _SharedOptions options;
+    String outputDir;
+    try {
+      var packageGraph = new PackageGraph.forThisPackage();
+      _ensureBuildTestDependency(packageGraph);
+      options = _readOptions();
+      // We always need an output dir when running tests, so we create a tmp dir
+      // if the user didn't specify one.
+      outputDir = options.outputDir ??
+          Directory.systemTemp
+              .createTempSync('build_runner_test')
+              .absolute
+              .uri
+              .toFilePath();
+      var result = await build(builderApplications,
+          deleteFilesByDefault: options.deleteFilesByDefault,
+          enableLowResourcesMode: options.enableLowResourcesMode,
+          assumeTty: options.assumeTty,
+          outputDir: outputDir,
+          verbose: options.verbose,
+          packageGraph: packageGraph);
+
+      if (result.status == BuildStatus.failure) {
+        stdout.writeln('Skipping tests due to build failure');
+        return 1;
+      }
+
+      var testExitCode = await _runTests(outputDir);
+      if (testExitCode != 0) {
+        // No need to log - should see failed tests in the console.
+        exitCode = testExitCode;
+      }
+      return testExitCode;
+    } finally {
+      // Clean up the output dir if one wasn't explicitly asked for.
+      if (options.outputDir == null && outputDir != null) {
+        await new Directory(outputDir).delete(recursive: true);
+      }
+
+      await ProcessManager.terminateStdIn();
     }
-
-    await ProcessManager.terminateStdIn();
   }
 
   /// Runs tests using [precompiledPath] as the precompiled test directory.
-  Future<Null> _runTests(String precompiledPath) async {
+  Future<int> _runTests(String precompiledPath) async {
     stdout.writeln('Running tests...\n');
     var extraTestArgs = argResults.rest;
     var testProcess = await new ProcessManager().spawn(
@@ -334,11 +355,7 @@ class _TestCommand extends _BaseCommand {
           '--precompiled',
           precompiledPath,
         ]..addAll(extraTestArgs));
-    var testExitCode = await testProcess.exitCode;
-    if (testExitCode != 0) {
-      // No need to log - should see failed tests in the console.
-      exitCode = testExitCode;
-    }
+    return testProcess.exitCode;
   }
 }
 

build_runner/lib/src/entrypoint/run.dart

@@ -10,7 +10,7 @@ import 'options.dart';
 
 /// A common entrypoint to parse command line arguments and build or serve with
 /// [builders].
-Future run(List<String> args, List<BuilderApplication> builders) async {
+Future<int> run(List<String> args, List<BuilderApplication> builders) {
   var runner = new BuildCommandRunner(builders);
-  await runner.run(args);
+  return runner.run(args);
 }

build_runner/pubspec.yaml

@@ -1,5 +1,5 @@
 name: build_runner
-version: 0.7.6
+version: 0.7.7
 description: Tools to write binaries that run builders.
 author: Dart Team <misc@dartlang.org>
 homepage: https://github.com/dart-lang/build

e2e_example/test/common/utils.dart

@@ -206,6 +206,7 @@ Future<Null> expectTestsFail({bool useManualScript}) async {
   var result =
       useManualScript ? await _runManualTests() : await _runAutoTests();
   expect(result.stdout, contains('Some tests failed'));
+  expect(result.exitCode, isNot(0));
 }
 
 Future<Null> expectTestsPass(

e2e_example/test/goldens/generated_build_script.dart

@@ -3,6 +3,7 @@ import 'package:provides_builder/builders.dart' as _i2;
 import 'package:build_test/builder.dart' as _i3;
 import 'package:build_config/build_config.dart' as _i4;
 import 'package:build_web_compilers/builders.dart' as _i5;
+import 'dart:isolate' as _i6;
 
 final _builders = [
   _i1.apply('provides_builder|some_not_applied_builder', [_i2.notApplied],
@@ -33,4 +34,7 @@ final _builders = [
           include: const ['web/**', 'test/**_test.dart'],
           exclude: const ['test/**.node_test.dart', 'test/**.vm_test.dart']))
 ];
-main(List<String> args) => _i1.run(args, _builders);
+main(List<String> args, [_i6.SendPort sendPort]) async {
+  var result = await _i1.run(args, _builders);
+  sendPort?.send(result);
+}

e2e_example/tool/build.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:io';
 
 import 'package:build/build.dart';
 import 'package:build_web_compilers/build_web_compilers.dart';
@@ -38,7 +39,7 @@ Future main(List<String> args) async {
             include: const ['web/**', 'test/**.browser_test.dart']))
   ];
 
-  await run(args, builders);
+  exitCode = await run(args, builders);
 }
 
 class ThrowingBuilder extends Builder {