From 0a157cf0e7c0cc5367785297f20326993151baf8 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Tue, 20 Mar 2018 14:25:20 -0700 Subject: [PATCH] Initial validation of build_runner dependency and version Also change the commands to return the exitCode from build_runner --- webdev/bin/webdev.dart | 12 ++- webdev/lib/src/command/build_command.dart | 2 +- .../command/build_runner_command_base.dart | 14 ++- webdev/lib/src/command/serve_command.dart | 2 +- webdev/lib/src/pubspec.dart | 90 +++++++++++++++++++ webdev/lib/src/util.dart | 11 +++ webdev/lib/src/webdev_command_runner.dart | 18 ++-- webdev/pubspec.yaml | 4 + webdev/test/integration_test.dart | 46 ++++++++-- 9 files changed, 183 insertions(+), 16 deletions(-) create mode 100644 webdev/lib/src/pubspec.dart create mode 100644 webdev/lib/src/util.dart diff --git a/webdev/bin/webdev.dart b/webdev/bin/webdev.dart index d88116ca4..b032ab62d 100644 --- a/webdev/bin/webdev.dart +++ b/webdev/bin/webdev.dart @@ -12,11 +12,21 @@ import 'package:webdev/src/webdev_command_runner.dart'; Future main(List args) async { try { - await webdevCommandRunner().run(args); + exitCode = await run(args); } on UsageException catch (e) { print(yellow.wrap(e.message)); print(' '); print(e.usage); exitCode = ExitCode.usage.code; + } on PackageException catch (e) { + print(yellow.wrap('Could not run in the current directory.')); + for (var detail in e.details) { + print(detail.error); + if (detail.description != null) { + print(' ${detail.description}'); + } + } + + exitCode = ExitCode.config.code; } } diff --git a/webdev/lib/src/command/build_command.dart b/webdev/lib/src/command/build_command.dart index d98c5fbe3..002c6327b 100644 --- a/webdev/lib/src/command/build_command.dart +++ b/webdev/lib/src/command/build_command.dart @@ -14,5 +14,5 @@ class BuildCommand extends BuildRunnerCommandBase { final description = 'Run builders to build a package.'; @override - Future run() => runCore('build'); + Future run() => runCore('build'); } diff --git a/webdev/lib/src/command/build_runner_command_base.dart b/webdev/lib/src/command/build_runner_command_base.dart index a03bace6a..8c47bf136 100644 --- a/webdev/lib/src/command/build_runner_command_base.dart +++ b/webdev/lib/src/command/build_runner_command_base.dart @@ -3,15 +3,17 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; -import 'dart:io'; +import 'dart:io' hide exitCode, exit; import 'dart:isolate'; import 'package:args/command_runner.dart'; import 'package:stack_trace/stack_trace.dart'; +import '../pubspec.dart'; + /// Extend to get a command with the arguments common to all build_runner /// commands. -abstract class BuildRunnerCommandBase extends Command { +abstract class BuildRunnerCommandBase extends Command { BuildRunnerCommandBase() { // TODO(nshahan) Expose more common args passed to build_runner commands. // build_runner might expose args for use in wrapping scripts like this one. @@ -25,9 +27,13 @@ abstract class BuildRunnerCommandBase extends Command { help: 'Enables verbose logging.'); } - Future runCore(String command) async { + Future runCore(String command) async { + await checkPubspecLock(); + final arguments = [command]..addAll(argResults.arguments); + var exitCode = 0; + // Heavily inspired by dart-lang/build @ 0c77443dd7 // /build_runner/bin/build_runner.dart#L58-L85 var exitPort = new ReceivePort(); @@ -62,6 +68,8 @@ abstract class BuildRunnerCommandBase extends Command { await exitPort.first; await errorListener.cancel(); await exitCodeListener?.cancel(); + + return exitCode; } } diff --git a/webdev/lib/src/command/serve_command.dart b/webdev/lib/src/command/serve_command.dart index 790d52b97..23a5dc728 100644 --- a/webdev/lib/src/command/serve_command.dart +++ b/webdev/lib/src/command/serve_command.dart @@ -31,5 +31,5 @@ class ServeCommand extends BuildRunnerCommandBase { } @override - Future run() => runCore('serve'); + Future run() => runCore('serve'); } diff --git a/webdev/lib/src/pubspec.dart b/webdev/lib/src/pubspec.dart new file mode 100644 index 000000000..fe0ca56e7 --- /dev/null +++ b/webdev/lib/src/pubspec.dart @@ -0,0 +1,90 @@ +// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:async'; +import 'dart:io'; + +import 'package:pub_semver/pub_semver.dart'; +import 'package:yaml/yaml.dart'; + +import 'util.dart'; + +class PackageException implements Exception { + final List details; + + PackageException._(this.details); +} + +class PackageExceptionDetails { + final String error; + final String description; + + const PackageExceptionDetails._(this.error, {this.description}); + + static const noPubspecLock = const PackageExceptionDetails._( + '`pubspec.lock` does not exist.', + description: + 'Run `webdev` in a Dart package directory. Run `pub get` first.'); + + static const noBuildRunnerDep = const PackageExceptionDetails._( + 'A dependency on `build_runner` was not found.', + description: + 'You must have a dependency on `build_runner` in `pubspec.yaml`. ' + 'It can be in either `dependencies` or `dev_dependencies`.'); + + @override + String toString() => [error, description].join('\n'); +} + +Future checkPubspecLock() async { + var file = new File('pubspec.lock'); + if (!file.existsSync()) { + throw new PackageException._([PackageExceptionDetails.noPubspecLock]); + } + + var pubspecLock = loadYaml(await file.readAsString()) as YamlMap; + + var packages = pubspecLock['packages'] as YamlMap; + + var issues = []; + + var buildRunner = packages['build_runner'] as YamlMap; + if (buildRunner == null) { + issues.add(PackageExceptionDetails.noBuildRunnerDep); + } else { + var dependency = buildRunner['dependency'] as String; + if (!dependency.startsWith('direct ')) { + issues.add(PackageExceptionDetails.noBuildRunnerDep); + } + + var version = buildRunner['version'] as String; + if (version == null) { + // TODO: warning? + } else { + var buildRunnerVersion = new Version.parse(version); + + if (!supportedBuildRunnerVersionRange.allows(buildRunnerVersion)) { + var error = 'The `build_runner` version – $buildRunnerVersion – is not ' + 'within the supported range – $supportedBuildRunnerVersionRange.'; + issues.add(new PackageExceptionDetails._(error)); + } + } + + var source = buildRunner['source'] as String; + if (source == 'hosted') { + //var description = buildRunner['description'] as YamlMap; + // TODO: check for `{url: https://pub.dartlang.org, name: build_runner}` + // If not, print a warning + } else { + // TODO: print a warning that we're assuming hosted + } + } + + // TODO: validate build_web_compilers + //var buldWebCompilers = packages['build_web_compilers']; + + if (issues.isNotEmpty) { + throw new PackageException._(issues); + } +} diff --git a/webdev/lib/src/util.dart b/webdev/lib/src/util.dart new file mode 100644 index 000000000..99be8d4db --- /dev/null +++ b/webdev/lib/src/util.dart @@ -0,0 +1,11 @@ +// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:pub_semver/pub_semver.dart'; + +final supportedBuildRunnerVersionRange = new VersionRange( + min: new Version(0, 8, 0), + includeMin: true, + max: new Version(0, 9, 0), + includeMax: false); diff --git a/webdev/lib/src/webdev_command_runner.dart b/webdev/lib/src/webdev_command_runner.dart index c2a7c02b5..cb99066f2 100644 --- a/webdev/lib/src/webdev_command_runner.dart +++ b/webdev/lib/src/webdev_command_runner.dart @@ -2,13 +2,21 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:async'; + import 'package:args/command_runner.dart'; import 'command/build_command.dart'; import 'command/serve_command.dart'; -/// All available top level commands. -CommandRunner webdevCommandRunner() => - new CommandRunner('webdev', 'A tool to develop Dart web projects.') - ..addCommand(new BuildCommand()) - ..addCommand(new ServeCommand()); +export 'pubspec.dart' show PackageException; + +Future run(List args) async { + var runner = + new CommandRunner('webdev', 'A tool to develop Dart web projects.') + ..addCommand(new BuildCommand()) + ..addCommand(new ServeCommand()); + + // In the case of `help`, `null` is returned. Treat that as success. + return await runner.run(args) ?? 0; +} diff --git a/webdev/pubspec.yaml b/webdev/pubspec.yaml index 13b761f49..7a8bb7e49 100644 --- a/webdev/pubspec.yaml +++ b/webdev/pubspec.yaml @@ -10,10 +10,14 @@ environment: dependencies: args: ^1.2.0 io: ^0.3.2+1 + pub_semver: ^1.3.2 stack_trace: ^1.9.2 + yaml: ^2.1.13 dev_dependencies: + path: ^1.5.1 test: ^0.12.0 + test_descriptor: ^1.0.3 test_process: ^1.0.1 executables: diff --git a/webdev/test/integration_test.dart b/webdev/test/integration_test.dart index 4d8aca451..f42ed62bf 100644 --- a/webdev/test/integration_test.dart +++ b/webdev/test/integration_test.dart @@ -4,12 +4,16 @@ import 'dart:io'; +import 'package:path/path.dart' as p; import 'package:test/test.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; import 'package:test_process/test_process.dart'; +final _webdevBin = p.absolute('bin/webdev.dart'); + void main() { test('README contains help output', () async { - var process = await TestProcess.start('dart', ['bin/webdev.dart']); + var process = await TestProcess.start('dart', [_webdevBin]); var output = (await process.stdoutStream().join('\n')).trim(); @@ -22,8 +26,7 @@ void main() { }); test('non-existant commands create errors', () async { - var process = - await TestProcess.start('dart', ['bin/webdev.dart', 'monkey']); + var process = await TestProcess.start('dart', [_webdevBin, 'monkey']); var output = (await process.stdoutStream().join('\n')).trim(); @@ -33,7 +36,40 @@ void main() { }); test('should fail in a package without a build_runner dependency', () async { - var process = await TestProcess.start('dart', ['bin/webdev.dart', 'serve']); - await process.shouldExit(255); + var process = await TestProcess.start('dart', [_webdevBin, 'serve']); + var output = (await process.stdoutStream().join('\n')).trim(); + + expect(output, contains(r'''Could not run in the current directory. +A dependency on `build_runner` was not found.''')); + await process.shouldExit(78); + }); + + group('should fail when `build_runner` is the wrong version', () { + for (var version in ['0.7.13+1', '0.9.0']) { + test(version, () async { + await d.file('pubspec.lock', ''' +# Copy-pasted from a valid run +packages: + build_runner: + dependency: "direct main" + description: + name: build_runner + url: "https://pub.dartlang.org" + source: hosted + version: "$version" +''').create(); + + var process = await TestProcess.start('dart', [_webdevBin, 'build'], + workingDirectory: d.sandbox); + var output = (await process.stdoutStream().join('\n')).trim(); + + expect(output, contains('Could not run in the current directory.')); + expect( + output, + contains('The `build_runner` version – $version – ' + 'is not within the supported range – >=0.8.0 <0.9.0.')); + await process.shouldExit(78); + }); + } }); }