Skip to content

Commit 686e203

Browse files
committed
Error if the build script gets updated after it starts running
1 parent 7ca1fab commit 686e203

File tree

2 files changed

+67
-20
lines changed

2 files changed

+67
-20
lines changed

lib/src/generate/build_impl.dart

+46-20
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ class BuildImpl {
4040
bool _buildRunning = false;
4141
final _logger = new Logger('Build');
4242

43+
bool _isFirstBuild = true;
44+
4345
BuildImpl(this._reader, this._writer, this._packageGraph, this._phaseGroups);
4446

4547
/// Runs a build
@@ -57,6 +59,9 @@ class BuildImpl {
5759
{DateTime validAsOf, Map<AssetId, ChangeType> updates}) async {
5860
validAsOf ??= new DateTime.now();
5961
updates ??= <AssetId, ChangeType>{};
62+
63+
/// Assume incremental, change if necessary.
64+
var buildType = BuildType.Incremental;
6065
try {
6166
if (_buildRunning) throw const ConcurrentBuildException();
6267
_buildRunning = true;
@@ -72,21 +77,41 @@ class BuildImpl {
7277
updates.addAll(await _getUpdates());
7378
}
7479

80+
/// If the build script gets updated, we need to either fully invalidate
81+
/// the graph (if the script current running is up to date), or we need to
82+
/// terminate and ask the user to restart the script (if the currently
83+
/// running script is out of date).
84+
///
85+
/// The [_isFirstBuild] flag is used as a proxy for "has this script
86+
/// been updated since it started running".
87+
///
88+
/// TODO(jakemac): Come up with a better way of telling if the script
89+
/// has been updated since it started running.
90+
_logger.info('Checking if the build script has been updated');
91+
if (await _buildScriptUpdated()) {
92+
buildType = BuildType.Full;
93+
if (_isFirstBuild) {
94+
_logger.info('Invalidating asset graph due to build script update');
95+
_assetGraph.allNodes
96+
.where((node) => node is GeneratedAssetNode)
97+
.forEach(
98+
(node) => (node as GeneratedAssetNode).needsUpdate = true);
99+
} else {
100+
var message = 'Build abandoned due to change to the build script or '
101+
'one of its dependencies. This could be caused by a pub get or '
102+
'any other change. Please terminate the build script and restart '
103+
'it.';
104+
_logger.warning(message);
105+
return new BuildResult(BuildStatus.Failure, buildType, [],
106+
exception: message);
107+
}
108+
}
109+
75110
/// Applies all [updates] to the [_assetGraph] as well as doing other
76111
/// necessary cleanup.
77112
_logger.info('Updating dependency graph with changes since last build.');
78113
await _updateWithChanges(updates);
79114

80-
/// Sometimes we need to fully invalidate the graph, such as when the
81-
/// build script itself is updated.
82-
_logger.info('Checking if asset graph needs to be rebuilt');
83-
if (await _shouldInvalidateAssetGraph()) {
84-
_logger.info('Invalidating asset graph due to build script update');
85-
_assetGraph.allNodes
86-
.where((node) => node is GeneratedAssetNode)
87-
.forEach((node) => (node as GeneratedAssetNode).needsUpdate = true);
88-
}
89-
90115
/// Wait while all inputs are collected.
91116
_logger.info('Initializing inputs');
92117
await _initializeInputsByPackage();
@@ -108,10 +133,11 @@ class BuildImpl {
108133

109134
return result;
110135
} catch (e, s) {
111-
return new BuildResult(BuildStatus.Failure, BuildType.Full, [],
136+
return new BuildResult(BuildStatus.Failure, buildType, [],
112137
exception: e, stackTrace: s);
113138
} finally {
114139
_buildRunning = false;
140+
_isFirstBuild = false;
115141
}
116142
}
117143

@@ -133,13 +159,12 @@ class BuildImpl {
133159
}
134160
}
135161

136-
/// Checks if the [_assetGraph] needs to be completely invalidated.
137-
///
138-
/// For now this just means looking at the current running program, and seeing
139-
/// if any of its sources are newer than the asset graph itself.
140-
Future<bool> _shouldInvalidateAssetGraph() async {
162+
/// Checks if the current running program has been updated since the asset
163+
/// graph was last built.
164+
Future<bool> _buildScriptUpdated() async {
141165
var completer = new Completer<bool>();
142-
Future.wait(currentMirrorSystem().libraries.keys.map((Uri uri) async {
166+
Future
167+
.wait(currentMirrorSystem().libraries.keys.map((Uri uri) async {
143168
/// Short-circuit
144169
if (completer.isCompleted) return;
145170
var lastModified;
@@ -162,11 +187,11 @@ class BuildImpl {
162187
lastModified = await file.lastModified();
163188
break;
164189
case 'data':
190+
165191
/// Test runner uses a `data` scheme, don't invalidate for those.
166192
if (uri.path.contains('package:test')) return;
167193
continue unknownUri;
168-
unknownUri:
169-
default:
194+
unknownUri: default:
170195
_logger.info('Unrecognized uri scheme `${uri.scheme}` found for '
171196
'library in build script, falling back on full rebuild.');
172197
if (!completer.isCompleted) completer.complete(true);
@@ -176,7 +201,8 @@ class BuildImpl {
176201
if (lastModified.compareTo(_assetGraph.validAsOf) > 0) {
177202
if (!completer.isCompleted) completer.complete(true);
178203
}
179-
})).then((_) {
204+
}))
205+
.then((_) {
180206
if (!completer.isCompleted) completer.complete(false);
181207
});
182208
return completer.future;

test/generate/watch_test.dart

+21
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,27 @@ main() {
129129

130130
expect(cachedGraph, equalsAssetGraph(expectedGraph));
131131
});
132+
133+
test('build fails if script is updated after the first build starts',
134+
() async {
135+
var writer = new InMemoryAssetWriter();
136+
var results = [];
137+
startWatch(copyAPhaseGroup, {'a|web/a.txt': 'a'}, writer)
138+
.listen(results.add);
139+
140+
var result = await nextResult(results);
141+
checkOutputs({'a|web/a.txt.copy': 'a',}, result, writer.assets);
142+
143+
/// Pretend like a part of the dart script got updated.
144+
await writer.writeAsString(makeAsset('test|lib/test.dart', ''),
145+
lastModified: new DateTime.now().add(new Duration(days: 1)));
146+
await writer.writeAsString(makeAsset('a|web/a.txt', 'b'));
147+
FakeWatcher
148+
.notifyWatchers(new WatchEvent(ChangeType.MODIFY, 'a/web/a.txt'));
149+
150+
result = await nextResult(results);
151+
expect(result.status, BuildStatus.Failure);
152+
});
132153
});
133154

134155
group('multiple phases', () {

0 commit comments

Comments
 (0)