From 6f36cdac565607e834bd12b0e52b4a2103db4630 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Fri, 11 Mar 2016 10:37:21 -0800 Subject: [PATCH] add option to skip the prompt to delete existing outputs --- lib/src/generate/build.dart | 16 ++++-- lib/src/generate/build_impl.dart | 88 ++++++++++++++++++++------------ lib/src/generate/options.dart | 15 +++--- test/common/common.dart | 4 +- test/generate/build_test.dart | 6 +-- 5 files changed, 82 insertions(+), 47 deletions(-) diff --git a/lib/src/generate/build.dart b/lib/src/generate/build.dart index 4565959a2..95cdc3f34 100644 --- a/lib/src/generate/build.dart +++ b/lib/src/generate/build.dart @@ -20,6 +20,10 @@ import 'watch_impl.dart'; /// Runs all of the [Phases] in [phaseGroups]. /// +/// By default, the user will be prompted to delete any files which already +/// exist but were not generated by this specific build script. The +/// [deleteFilesByDefault] option can be set to [true] to skip this prompt. +/// /// A [packageGraph] may be supplied, otherwise one will be constructed using /// [PackageGraph.forThisPackage]. The default functionality assumes you are /// running in the root directory of a package, with both a `pubspec.yaml` and @@ -38,13 +42,15 @@ import 'watch_impl.dart'; /// will simply consume the first event and allow the build to continue. /// Multiple termination events will cause a normal shutdown. Future build(PhaseGroup phaseGroup, - {PackageGraph packageGraph, + {bool deleteFilesByDefault, + PackageGraph packageGraph, AssetReader reader, AssetWriter writer, Level logLevel, onLog(LogRecord record), Stream terminateEventStream}) async { var options = new BuildOptions( + deleteFilesByDefault: deleteFilesByDefault, packageGraph: packageGraph, reader: reader, writer: writer, @@ -82,7 +88,8 @@ Future build(PhaseGroup phaseGroup, /// will complete normally. Subsequent events are not handled (and will /// typically cause a shutdown). Stream watch(PhaseGroup phaseGroup, - {PackageGraph packageGraph, + {bool deleteFilesByDefault, + PackageGraph packageGraph, AssetReader reader, AssetWriter writer, Level logLevel, @@ -91,6 +98,7 @@ Stream watch(PhaseGroup phaseGroup, DirectoryWatcherFactory directoryWatcherFactory, Stream terminateEventStream}) { var options = new BuildOptions( + deleteFilesByDefault: deleteFilesByDefault, packageGraph: packageGraph, reader: reader, writer: writer, @@ -122,7 +130,8 @@ Stream watch(PhaseGroup phaseGroup, /// [address]:[port], but instead a [requestHandler] may be provided for custom /// behavior. Stream serve(PhaseGroup phaseGroup, - {PackageGraph packageGraph, + {bool deleteFilesByDefault, + PackageGraph packageGraph, AssetReader reader, AssetWriter writer, Level logLevel, @@ -135,6 +144,7 @@ Stream serve(PhaseGroup phaseGroup, int port, Handler requestHandler}) { var options = new BuildOptions( + deleteFilesByDefault: deleteFilesByDefault, packageGraph: packageGraph, reader: reader, writer: writer, diff --git a/lib/src/generate/build_impl.dart b/lib/src/generate/build_impl.dart index 62633eed3..7658555c6 100644 --- a/lib/src/generate/build_impl.dart +++ b/lib/src/generate/build_impl.dart @@ -35,6 +35,7 @@ import 'phase.dart'; class BuildImpl { final AssetId _assetGraphId; final List> _buildActions; + final bool _deleteFilesByDefault; final _inputsByPackage = >{}; final _logger = new Logger('Build'); final PackageGraph _packageGraph; @@ -50,6 +51,7 @@ class BuildImpl { : _assetGraphId = new AssetId(options.packageGraph.root.name, assetGraphPath), _buildActions = phaseGroup.buildActions, + _deleteFilesByDefault = options.deleteFilesByDefault, _packageGraph = options.packageGraph, _reader = options.reader, _writer = options.writer; @@ -77,16 +79,22 @@ class BuildImpl { Chain.capture(() async { if (_buildRunning) throw const ConcurrentBuildException(); _buildRunning = true; + var isNewAssetGraph = false; /// Initialize the [assetGraph] if its not yet set up. if (_assetGraph == null) { await logWithTime(_logger, 'Reading cached dependency graph', () async { _assetGraph = await _readAssetGraph(); - - /// Collect updates since the asset graph was last created. This only - /// handles updates and deletes, not adds. We list the file system for - /// all inputs later on (in [_initializeInputsByPackage]). - updates.addAll(await _getUpdates()); + if (_assetGraph.allNodes.isEmpty && + !(await _reader.hasInput(_assetGraphId))) { + isNewAssetGraph = true; + buildType = BuildType.Full; + } else { + /// Collect updates since the asset graph was last created. This only + /// handles updates and deletes, not adds. We list the file system for + /// all inputs later on (in [_initializeInputsByPackage]). + updates.addAll(await _getUpdates()); + } }); } @@ -97,27 +105,27 @@ class BuildImpl { /// /// The [_isFirstBuild] flag is used as a proxy for "has this script /// been updated since it started running". - /// - /// TODO(jakemac): Come up with a better way of telling if the script - /// has been updated since it started running. - await logWithTime(_logger, 'Checking build script for updates', () async { - if (await _buildScriptUpdated()) { - buildType = BuildType.Full; - if (_isFirstBuild) { - _logger - .warning('Invalidating asset graph due to build script update'); - _assetGraph.allNodes - .where((node) => node is GeneratedAssetNode) - .forEach( - (node) => (node as GeneratedAssetNode).needsUpdate = true); - } else { - done.complete(new BuildResult(BuildStatus.Failure, buildType, [], - exception: new BuildScriptUpdatedException())); + if (!isNewAssetGraph) { + await logWithTime(_logger, 'Checking build script for updates', + () async { + if (await _buildScriptUpdated()) { + buildType = BuildType.Full; + if (_isFirstBuild) { + _logger.warning( + 'Invalidating asset graph due to build script update'); + _assetGraph.allNodes + .where((node) => node is GeneratedAssetNode) + .forEach((node) => + (node as GeneratedAssetNode).needsUpdate = true); + } else { + done.complete(new BuildResult(BuildStatus.Failure, buildType, [], + exception: new BuildScriptUpdatedException())); + } } - } - }); - // Bail if the previous step completed the build. - if (done.isCompleted) return; + }); + // Bail if the previous step completed the build. + if (done.isCompleted) return; + } await logWithTime(_logger, 'Finalizing build setup', () async { /// Applies all [updates] to the [_assetGraph] as well as doing other @@ -132,7 +140,7 @@ class BuildImpl { /// Delete all previous outputs! _logger.info('Deleting previous outputs'); - await _deletePreviousOutputs(); + await _deletePreviousOutputs(isNewAssetGraph); }); /// Run a fresh build. @@ -185,6 +193,9 @@ class BuildImpl { /// Checks if the current running program has been updated since the asset /// graph was last built. + /// + /// TODO(jakemac): Come up with a better way of telling if the script + /// has been updated since it started running. Future _buildScriptUpdated() async { var completer = new Completer(); Future @@ -299,10 +310,8 @@ class BuildImpl { } /// Deletes all previous output files that are in need of an update. - Future _deletePreviousOutputs() async { - /// TODO(jakemac): need a cleaner way of telling if the current graph was - /// generated from cache or if its just a brand new graph. - if (await _reader.hasInput(_assetGraphId)) { + Future _deletePreviousOutputs(bool isNewAssetGraph) async { + if (!isNewAssetGraph) { await _writer.delete(_assetGraphId); _inputsByPackage[_assetGraphId.package]?.remove(_assetGraphId); @@ -357,6 +366,20 @@ class BuildImpl { // Check conflictingOuputs, prompt user to delete files. if (conflictingOutputs.isEmpty) return; + Future deleteConflictingOutputs() { + return Future.wait(conflictingOutputs.map/**/((output) { + _inputsByPackage[output.package]?.remove(output); + return _writer.delete(output); + })); + } + + // Skip the prompt if using this option. + if (_deleteFilesByDefault) { + await deleteConflictingOutputs(); + return; + } + + // Prompt the user to delete files that are declared as outputs. stdout.writeln('\n\nFound ${conflictingOutputs.length} declared outputs ' 'which already exist on disk. This is likely because the' '`$cacheDir` folder was deleted, or you are submitting generated ' @@ -368,10 +391,7 @@ class BuildImpl { switch (input.toLowerCase()) { case 'y': stdout.writeln('Deleting files...'); - await Future.wait(conflictingOutputs.map/**/((output) { - _inputsByPackage[output.package]?.remove(output); - return _writer.delete(output); - })); + await deleteConflictingOutputs(); done = true; break; case 'n': diff --git a/lib/src/generate/options.dart b/lib/src/generate/options.dart index 4c3e3fc64..51cec37c4 100644 --- a/lib/src/generate/options.dart +++ b/lib/src/generate/options.dart @@ -22,6 +22,7 @@ class BuildOptions { PackageGraph packageGraph; AssetReader reader; AssetWriter writer; + bool deleteFilesByDefault; /// Watch mode options. Duration debounceDelay; @@ -34,17 +35,18 @@ class BuildOptions { Handler requestHandler; BuildOptions( - {this.debounceDelay, + {this.address, + this.debounceDelay, + this.deleteFilesByDefault, + this.directory, this.directoryWatcherFactory, Level logLevel, onLog(LogRecord record), this.packageGraph, - this.reader, - this.writer, - this.directory, - this.address, this.port, - this.requestHandler}) { + this.reader, + this.requestHandler, + this.writer}) { /// Set up logging logLevel ??= Level.INFO; Logger.root.level = logLevel; @@ -84,6 +86,7 @@ class BuildOptions { writer ??= new CachedAssetWriter(cache, new FileBasedAssetWriter(packageGraph)); directoryWatcherFactory ??= defaultDirectoryWatcherFactory; + deleteFilesByDefault ??= false; } } diff --git a/test/common/common.dart b/test/common/common.dart index 0993269ed..d05ff2bf4 100644 --- a/test/common/common.dart +++ b/test/common/common.dart @@ -72,7 +72,8 @@ Future nextResult(results) { } testPhases(PhaseGroup phases, Map inputs, - {Map outputs, + {bool deleteFilesByDefault, + Map outputs, PackageGraph packageGraph, BuildStatus status: BuildStatus.Success, exceptionMatcher, @@ -91,6 +92,7 @@ testPhases(PhaseGroup phases, Map inputs, } var result = await build(phases, + deleteFilesByDefault: deleteFilesByDefault, reader: reader, writer: writer, packageGraph: packageGraph, diff --git a/test/generate/build_test.dart b/test/generate/build_test.dart index fdbcf6ffa..01081d8fd 100644 --- a/test/generate/build_test.dart +++ b/test/generate/build_test.dart @@ -198,13 +198,13 @@ main() { await writer.delete(outputId); // Second run, should have no extra outputs. - var done = - testPhases(copyAPhaseGroup, inputs, outputs: outputs, writer: writer); + var done = testPhases(copyAPhaseGroup, inputs, + outputs: outputs, writer: writer, deleteFilesByDefault: true); // Should block on user input. await new Future.delayed(new Duration(seconds: 1)); // Now it should complete! await done; - }, skip: 'Need to manually add a `y` to stdin for this test to run.'); + }); group('incremental builds with cached graph', () { test('one new asset, one modified asset, one unchanged asset', () async {