diff --git a/.gitignore b/.gitignore index 046cfabbc..85cea2750 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ pubspec.lock # Files generated by dart tools .dart_tool doc/ +build.lock # Generated by tool/create_all_test.dart tool/test_all.dart diff --git a/build_runner/CHANGELOG.md b/build_runner/CHANGELOG.md index d02b45d74..e4d171bdc 100644 --- a/build_runner/CHANGELOG.md +++ b/build_runner/CHANGELOG.md @@ -1,8 +1,12 @@ -## 0.7.1+1 +## 0.7.2 - **BUG FIX**: Running the `build_runner` binary without arguments no longer causes a crash saying `Could not find an option named "assume-tty".`. +- Builds now automatically generate a `build.lock` file in the package + directory while a build is in progress (including watchers and servers), and + fail if a lock file is already created. + ## 0.7.1 - Run Builders which write to the source tree before those which write to the diff --git a/build_runner/bin/build_runner.dart b/build_runner/bin/build_runner.dart index 41561153d..054327c51 100644 --- a/build_runner/bin/build_runner.dart +++ b/build_runner/bin/build_runner.dart @@ -9,11 +9,18 @@ import 'package:io/io.dart'; import 'package:logging/logging.dart'; import 'package:build_runner/src/build_script_generate/build_script_generate.dart'; +import 'package:build_runner/src/generate/exceptions.dart'; import 'package:build_runner/src/logging/std_io_logging.dart'; Future main(List args) async { var logListener = Logger.root.onRecord.listen(stdIOLogListener); - await ensureBuildScript(); + try { + await ensureBuildScript(); + } on ConcurrentBuildException catch (e) { + Logger.root.log(Level.SEVERE, '$e'); + exitCode = 1; + return; + } var dart = Platform.resolvedExecutable; final innerArgs = [scriptLocation]..addAll(args); if (stdioType(stdin) == StdioType.TERMINAL && diff --git a/build_runner/lib/build_runner.dart b/build_runner/lib/build_runner.dart index 138975a0a..273fb00b8 100644 --- a/build_runner/lib/build_runner.dart +++ b/build_runner/lib/build_runner.dart @@ -7,6 +7,7 @@ export 'src/asset/writer.dart'; export 'src/entrypoint/run.dart' show run; export 'src/generate/build.dart'; export 'src/generate/build_result.dart'; +export 'src/generate/lock_file.dart' show clearLock, createLock; export 'src/generate/performance_tracker.dart' show BuildPerformance, BuildActionPerformance; export 'src/package_graph/apply_builders.dart' diff --git a/build_runner/lib/src/build_script_generate/build_script_generate.dart b/build_runner/lib/src/build_script_generate/build_script_generate.dart index d3be71592..6a590a349 100644 --- a/build_runner/lib/src/build_script_generate/build_script_generate.dart +++ b/build_runner/lib/src/build_script_generate/build_script_generate.dart @@ -38,11 +38,12 @@ Future _generateBuildScript() async { /// Finds expressions to create all the [BuilderApplication] instances that /// should be applied packages in the build. /// -/// Adds `apply` expressions based on the BuildefDefinitions from any package +/// Adds `apply` expressions based on the BuilderDefinitions from any package /// which has a `build.yaml`. Future> _findBuilderApplications() async { final builderApplications = []; final packageGraph = new PackageGraph.forThisPackage(); + await createLock(packageGraph); final orderedPackages = stronglyConnectedComponents( [packageGraph.root], (node) => node.name, (node) => node.dependencies) .expand((c) => c); diff --git a/build_runner/lib/src/generate/build_impl.dart b/build_runner/lib/src/generate/build_impl.dart index 2dc0deeb9..56da94673 100644 --- a/build_runner/lib/src/generate/build_impl.dart +++ b/build_runner/lib/src/generate/build_impl.dart @@ -30,6 +30,7 @@ import 'build_result.dart'; import 'exceptions.dart'; import 'fold_frames.dart'; import 'heartbeat.dart'; +import 'lock_file.dart'; import 'options.dart'; import 'performance_tracker.dart'; import 'phase.dart'; @@ -71,6 +72,7 @@ Future build( await terminator.cancel(); await options.logListener.cancel(); + await clearLock(options.packageGraph); return result; } diff --git a/build_runner/lib/src/generate/exceptions.dart b/build_runner/lib/src/generate/exceptions.dart index 910b1779c..367b29692 100644 --- a/build_runner/lib/src/generate/exceptions.dart +++ b/build_runner/lib/src/generate/exceptions.dart @@ -8,11 +8,14 @@ import 'package:build_runner/src/generate/phase.dart'; import 'phase.dart'; class ConcurrentBuildException implements Exception { - const ConcurrentBuildException(); + final String lockFilePath; + + const ConcurrentBuildException(this.lockFilePath); @override - String toString() => - 'ConcurrentBuildException: Only one build may be running at a time.'; + String toString() => 'Only one build may be running at a time!\n' + 'If a build process has failed, or you want to remove the lock \n' + 'manually simple delete the file "$lockFilePath".'; } abstract class FatalBuildException implements Exception { diff --git a/build_runner/lib/src/generate/lock_file.dart b/build_runner/lib/src/generate/lock_file.dart new file mode 100644 index 000000000..3cd7f1107 --- /dev/null +++ b/build_runner/lib/src/generate/lock_file.dart @@ -0,0 +1,58 @@ +// Copyright (c) 2017, 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:crypto/crypto.dart'; +import 'package:path/path.dart' as p; + +import '../generate/exceptions.dart'; +import '../package_graph/package_graph.dart'; + +/// Returns a simple string hash used to determine if a lock file is owned. +String _createHash() => md5 + .convert(pid.toString().codeUnits) + .bytes + .map((b) => b.toRadixString(16)) + .join(''); + +/// Returns a future that completes with a hash when [graph] could be locked. +/// +/// This same hash is expected to be used with [clearLock] when the build +/// process is complete. Throws a `ConcurrentBuildException` if the lock could +/// not be created. +Future createLock(PackageGraph graph, {String hash}) async { + // For package graphs without a root (usually for testing), just succeed. + hash ??= _createHash(); + if (graph?.root?.path == null) { + return hash; + } + final path = p.join(graph.root.path, 'build.lock'); + final file = new File(path); + if (await file.exists()) { + throw new ConcurrentBuildException(file.absolute.path); + } + await file.writeAsString(hash); + return hash; +} + +/// Returns a future that completes after clearing the lock file for [graph]. +Future clearLock(PackageGraph graph, {String hash}) async { + // For package graphs without a root (usually for testing), just succeed. + if (graph?.root?.path == null) { + return; + } + final path = p.join(graph.root.path, 'build.lock'); + final file = new File(path); + if (!await file.exists()) { + return; + } + final input = await file.readAsString(); + hash ??= _createHash(); + if (hash != input) { + throw new ConcurrentBuildException(file.absolute.path); + } + await file.delete(); +} diff --git a/build_runner/lib/src/generate/watch_impl.dart b/build_runner/lib/src/generate/watch_impl.dart index 0743a2c2e..a47f1f5a8 100644 --- a/build_runner/lib/src/generate/watch_impl.dart +++ b/build_runner/lib/src/generate/watch_impl.dart @@ -25,6 +25,7 @@ import 'build_definition.dart'; import 'build_impl.dart'; import 'build_result.dart'; import 'directory_watcher_factory.dart'; +import 'lock_file.dart'; import 'options.dart'; import 'phase.dart'; import 'terminator.dart'; @@ -65,12 +66,14 @@ Future watch( final buildActions = await createBuildActions(options.packageGraph, builders, overrideBuildConfig: overrideBuildConfig); - var watch = runWatch(options, buildActions, terminator.shouldTerminate); + var watch = runWatch(options, buildActions, + terminator.shouldTerminate.then((_) => clearLock(options.packageGraph))); // ignore: unawaited_futures watch.buildResults.drain().then((_) async { await terminator.cancel(); await options.logListener.cancel(); + await clearLock(options.packageGraph); }); return createServeHandler(watch); @@ -149,6 +152,7 @@ class WatchImpl implements BuildState { .hasBeenUpdated(mergedChanges.keys.toSet())) { fatalBuildCompleter.complete(); _logger.severe('Terminating builds due to build script update'); + await clearLock(options.packageGraph); return new BuildResult(BuildStatus.failure, []); } } @@ -182,6 +186,7 @@ class WatchImpl implements BuildState { await currentBuild; await _buildDefinition.resourceManager.beforeExit(); await controller.close(); + await clearLock(options.packageGraph); _logger.info('Builds finished. Safe to exit\n'); }); diff --git a/build_runner/pubspec.yaml b/build_runner/pubspec.yaml index e9123abbd..c41ad4a39 100644 --- a/build_runner/pubspec.yaml +++ b/build_runner/pubspec.yaml @@ -1,5 +1,5 @@ name: build_runner -version: 0.7.1+1 +version: 0.7.2 description: Tools to write binaries that run builders. author: Dart Team homepage: https://github.com/dart-lang/build diff --git a/build_runner/test/generate/lock_file_test.dart b/build_runner/test/generate/lock_file_test.dart new file mode 100644 index 000000000..e4eb2d7f2 --- /dev/null +++ b/build_runner/test/generate/lock_file_test.dart @@ -0,0 +1,78 @@ +// Copyright (c) 2017, 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:build_runner/src/generate/exceptions.dart'; +import 'package:build_runner/src/generate/lock_file.dart'; +import 'package:build_runner/src/package_graph/package_graph.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; + +import '../common/descriptors.dart'; + +void main() { + PackageGraph graph; + + setUp(() async { + await d.dir('pkg', [ + d.dir('lib'), + d.file('.packages', 'pkg:./lib/'), + await pubspec('pkg'), + ]).create(); + + final root = new PackageNode('pkg', p.join(d.sandbox, 'pkg'), isRoot: true); + graph = new PackageGraph.fromRoot(root); + expect(graph.root.path, isNotNull); + }); + + test('should create a lock file', () async { + // Start with no lock file. + await d.dir('pkg', [ + d.nothing('build.lock'), + ]).validate(); + + // Create a lock file. + await createLock(graph, hash: '1234'); + + // Expect a lock file. + await d.dir('pkg', [ + d.file('build.lock', '1234'), + ]).validate(); + }); + + test('should throw if a lock file already exists', () async { + await d.dir('pkg', [ + d.file('build.lock', '1234'), + ]).create(); + + expect(createLock(graph), throwsConcurrentBuildException); + }); + + test('should remove a lock file', () async { + await d.dir('pkg', [ + d.file('build.lock', '1234'), + ]).create(); + + await clearLock(graph, hash: '1234'); + + await d.dir('pkg', [ + d.nothing('build.lock'), + ]).validate(); + }); + + test('should throw on removing a lock file with a wrong hash', () async { + await d.dir('pkg', [ + d.file('build.lock', '1234'), + ]).create(); + + expect(clearLock(graph, hash: 'ABCD'), throwsConcurrentBuildException); + + await d.dir('pkg', [ + d.file('build.lock', '1234'), + ]).validate(); + }); +} + +final throwsConcurrentBuildException = + throwsA(const isInstanceOf());