Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions build_runner/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down
16 changes: 13 additions & 3 deletions build_runner/bin/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down
97 changes: 57 additions & 40 deletions build_runner/lib/src/entrypoint/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
}
}

Expand All @@ -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,
Expand All @@ -234,6 +239,7 @@ class _WatchCommand extends _BaseCommand {
verbose: options.verbose);
await handler.currentBuild;
await handler.buildResults.drain();
return ExitCode.success.code;
}
}

Expand All @@ -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,
Expand All @@ -273,6 +279,8 @@ class _ServeCommand extends _WatchCommand {
}
await handler.buildResults.drain();
await Future.wait(servers.map((server) => server.close()));

return ExitCode.success.code;
}
}

Expand All @@ -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(
Expand All @@ -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;
}
}

Expand Down
4 changes: 2 additions & 2 deletions build_runner/lib/src/entrypoint/run.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 1 addition & 1 deletion build_runner/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 1 addition & 0 deletions e2e_example/test/common/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 5 additions & 1 deletion e2e_example/test/goldens/generated_build_script.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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);
}
3 changes: 2 additions & 1 deletion e2e_example/tool/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 {
Expand Down