Skip to content

Commit

Permalink
add option to skip the prompt to delete existing outputs
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakemac53 @kevmoo
jakemac53 authored and kevmoo committed Mar 11, 2016
1 parent 54d6522 commit 6f36cda
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 47 deletions.
16 changes: 13 additions & 3 deletions lib/src/generate/build.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<BuildResult> 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,
Expand Down Expand Up @@ -82,7 +88,8 @@ Future<BuildResult> build(PhaseGroup phaseGroup,
/// will complete normally. Subsequent events are not handled (and will
/// typically cause a shutdown).
Stream<BuildResult> watch(PhaseGroup phaseGroup,
{PackageGraph packageGraph,
{bool deleteFilesByDefault,
PackageGraph packageGraph,
AssetReader reader,
AssetWriter writer,
Level logLevel,
Expand All @@ -91,6 +98,7 @@ Stream<BuildResult> watch(PhaseGroup phaseGroup,
DirectoryWatcherFactory directoryWatcherFactory,
Stream terminateEventStream}) {
var options = new BuildOptions(
deleteFilesByDefault: deleteFilesByDefault,
packageGraph: packageGraph,
reader: reader,
writer: writer,
Expand Down Expand Up @@ -122,7 +130,8 @@ Stream<BuildResult> watch(PhaseGroup phaseGroup,
/// [address]:[port], but instead a [requestHandler] may be provided for custom
/// behavior.
Stream<BuildResult> serve(PhaseGroup phaseGroup,
{PackageGraph packageGraph,
{bool deleteFilesByDefault,
PackageGraph packageGraph,
AssetReader reader,
AssetWriter writer,
Level logLevel,
Expand All @@ -135,6 +144,7 @@ Stream<BuildResult> serve(PhaseGroup phaseGroup,
int port,
Handler requestHandler}) {
var options = new BuildOptions(
deleteFilesByDefault: deleteFilesByDefault,
packageGraph: packageGraph,
reader: reader,
writer: writer,
Expand Down
88 changes: 54 additions & 34 deletions lib/src/generate/build_impl.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import 'phase.dart';
class BuildImpl {
final AssetId _assetGraphId;
final List<List<BuildAction>> _buildActions;
final bool _deleteFilesByDefault;
final _inputsByPackage = <String, Set<AssetId>>{};
final _logger = new Logger('Build');
final PackageGraph _packageGraph;
Expand All @@ -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;
Expand Down Expand Up @@ -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());
}
});
}

Expand All @@ -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
Expand All @@ -132,7 +140,7 @@ class BuildImpl {

/// Delete all previous outputs!
_logger.info('Deleting previous outputs');
await _deletePreviousOutputs();
await _deletePreviousOutputs(isNewAssetGraph);
});

/// Run a fresh build.
Expand Down Expand Up @@ -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<bool> _buildScriptUpdated() async {
var completer = new Completer<bool>();
Future
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -357,6 +366,20 @@ class BuildImpl {
// Check conflictingOuputs, prompt user to delete files.
if (conflictingOutputs.isEmpty) return;

Future deleteConflictingOutputs() {
return Future.wait(conflictingOutputs.map/*<Future>*/((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 '
Expand All @@ -368,10 +391,7 @@ class BuildImpl {
switch (input.toLowerCase()) {
case 'y':
stdout.writeln('Deleting files...');
await Future.wait(conflictingOutputs.map/*<Future>*/((output) {
_inputsByPackage[output.package]?.remove(output);
return _writer.delete(output);
}));
await deleteConflictingOutputs();
done = true;
break;
case 'n':
Expand Down
15 changes: 9 additions & 6 deletions lib/src/generate/options.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class BuildOptions {
PackageGraph packageGraph;
AssetReader reader;
AssetWriter writer;
bool deleteFilesByDefault;

/// Watch mode options.
Duration debounceDelay;
Expand All @@ -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;
Expand Down Expand Up @@ -84,6 +86,7 @@ class BuildOptions {
writer ??=
new CachedAssetWriter(cache, new FileBasedAssetWriter(packageGraph));
directoryWatcherFactory ??= defaultDirectoryWatcherFactory;
deleteFilesByDefault ??= false;
}
}

Expand Down
4 changes: 3 additions & 1 deletion test/common/common.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ Future<BuildResult> nextResult(results) {
}

testPhases(PhaseGroup phases, Map<String, String> inputs,
{Map<String, String> outputs,
{bool deleteFilesByDefault,
Map<String, String> outputs,
PackageGraph packageGraph,
BuildStatus status: BuildStatus.Success,
exceptionMatcher,
Expand All @@ -91,6 +92,7 @@ testPhases(PhaseGroup phases, Map<String, String> inputs,
}

var result = await build(phases,
deleteFilesByDefault: deleteFilesByDefault,
reader: reader,
writer: writer,
packageGraph: packageGraph,
Expand Down
6 changes: 3 additions & 3 deletions test/generate/build_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 6f36cda

Please sign in to comment.