Skip to content

Commit fe432fd

Browse files
committed
Remove builder options nodes; store and check builder options by phase.
1 parent 79d87ec commit fe432fd

24 files changed

+279
-402
lines changed

build_runner/test/generate/watch_test.dart

-11
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ void main() {
2626
final copyABuildApplication = applyToRoot(
2727
TestBuilder(buildExtensions: appendExtension('.copy', from: '.txt')),
2828
);
29-
final defaultBuilderOptions = const BuilderOptions({});
3029
final packageConfigId = makeAssetId('a|.dart_tool/package_config.json');
3130
final packageGraph = buildPackageGraph({
3231
rootPackage('a', path: path.absolute('a')): [],
@@ -362,12 +361,6 @@ void main() {
362361
readerWriter,
363362
);
364363

365-
var builderOptionsId = makeAssetId('a|Phase0.builderOptions');
366-
var builderOptionsNode = AssetNode.builderOptions(
367-
builderOptionsId,
368-
lastKnownDigest: computeBuilderOptionsDigest(defaultBuilderOptions),
369-
);
370-
371364
var bCopyId = makeAssetId('a|web/b.txt.copy');
372365
var bTxtId = makeAssetId('a|web/b.txt');
373366
var bCopyNode = AssetNode.generated(
@@ -377,7 +370,6 @@ void main() {
377370
pendingBuildAction: PendingBuildAction.none,
378371
wasOutput: true,
379372
isFailure: false,
380-
builderOptionsId: builderOptionsId,
381373
lastKnownDigest: computeDigest(bCopyId, 'b2'),
382374
inputs: [makeAssetId('a|web/b.txt')],
383375
isHidden: false,
@@ -399,7 +391,6 @@ void main() {
399391
pendingBuildAction: PendingBuildAction.none,
400392
wasOutput: true,
401393
isFailure: false,
402-
builderOptionsId: builderOptionsId,
403394
lastKnownDigest: computeDigest(cCopyId, 'c'),
404395
inputs: [makeAssetId('a|web/c.txt')],
405396
isHidden: false,
@@ -412,8 +403,6 @@ void main() {
412403
], computeDigest(cTxtId, 'c')),
413404
);
414405

415-
expectedGraph.add(builderOptionsNode);
416-
417406
// TODO: We dont have a shared way of computing the combined input
418407
// hashes today, but eventually we should test those here too.
419408
expect(

build_runner/test/server/asset_handler_test.dart

-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ void main() {
136136
graph.add(
137137
AssetNode.generated(
138138
AssetId('a', 'web/main.ddc.js'),
139-
builderOptionsId: AssetId('_\$fake', 'options_id'),
140139
phaseNumber: 0,
141140
pendingBuildAction: PendingBuildAction.none,
142141
isHidden: false,

build_runner/test/server/serve_handler_test.dart

-1
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,6 @@ void main() {
209209
assetGraph.add(
210210
AssetNode.generated(
211211
AssetId('a', 'web/main.ddc.js'),
212-
builderOptionsId: AssetId('_\$fake', 'options_id'),
213212
phaseNumber: 0,
214213
pendingBuildAction: PendingBuildAction.none,
215214
isHidden: false,

build_runner_core/CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
- Track post process builder outputs separately from the main graph Instead of
3131
in `postProcessAnchor` nodes.
3232
- Compute outputs as needed instead of storing them in the asset graph.
33+
- Check build options for changes in the phase setup instead of storing them
34+
in the asset graph.
3335

3436
## 8.0.0
3537

build_runner_core/lib/src/asset_graph/graph.dart

+52-89
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import 'package:build/experiments.dart' as experiments_zone;
1212
// ignore: implementation_imports
1313
import 'package:build/src/internal.dart';
1414
import 'package:built_collection/built_collection.dart';
15+
import 'package:built_value/serializer.dart';
1516
import 'package:crypto/crypto.dart';
1617
import 'package:glob/glob.dart';
1718
import 'package:meta/meta.dart';
@@ -61,8 +62,34 @@ class AssetGraph implements GeneratedAssetHider {
6162
final Map<String, Map<PostProcessBuildStepId, Set<AssetId>>>
6263
_postProcessBuildStepOutputs = {};
6364

65+
/// Digests from the previous build's [BuildPhases], or `null` if this is a
66+
/// clean build.
67+
BuiltList<Digest>? previousInBuildPhasesOptionsDigests;
68+
69+
/// Digests from the current build's [BuildPhases].
70+
BuiltList<Digest> inBuildPhasesOptionsDigests;
71+
72+
/// Digests from the previous build's [BuildPhases], or `null` if this is a
73+
/// clean build.
74+
BuiltList<Digest>? previousPostBuildOptionsDigests;
75+
76+
/// Digests from the current build's [BuildPhases].
77+
BuiltList<Digest> postBuildActionsOptionsDigests;
78+
6479
AssetGraph._(
80+
BuildPhases buildPhases,
81+
this.dartVersion,
82+
this.packageLanguageVersions,
83+
this.enabledExperiments,
84+
) : buildPhasesDigest = buildPhases.digest,
85+
inBuildPhasesOptionsDigests = buildPhases.inBuildPhasesOptionsDigests,
86+
postBuildActionsOptionsDigests =
87+
buildPhases.postBuildActionsOptionsDigests;
88+
89+
AssetGraph._fromSerialized(
6590
this.buildPhasesDigest,
91+
this.inBuildPhasesOptionsDigests,
92+
this.postBuildActionsOptionsDigests,
6693
this.dartVersion,
6794
this.packageLanguageVersions,
6895
this.enabledExperiments,
@@ -80,21 +107,19 @@ class AssetGraph implements GeneratedAssetHider {
80107
AssetReader digestReader,
81108
) async {
82109
var graph = AssetGraph._(
83-
buildPhases.digest,
110+
buildPhases,
84111
Platform.version,
85112
packageGraph.languageVersions,
86113
experiments_zone.enabledExperiments.build(),
87114
);
88115
var placeholders = graph._addPlaceHolderNodes(packageGraph);
89116
graph._addSources(sources);
90-
graph
91-
.._addBuilderOptionsNodes(buildPhases)
92-
.._addOutputsForSources(
93-
buildPhases,
94-
sources,
95-
packageGraph.root.name,
96-
placeholders: placeholders,
97-
);
117+
graph._addOutputsForSources(
118+
buildPhases,
119+
sources,
120+
packageGraph.root.name,
121+
placeholders: placeholders,
122+
);
98123
// Pre-emptively compute digests for the nodes we know have outputs.
99124
await graph._setLastKnownDigests(
100125
sources.where((id) => graph.get(id)!.primaryOutputs.isNotEmpty),
@@ -217,36 +242,6 @@ class AssetGraph implements GeneratedAssetHider {
217242
}
218243
}
219244

220-
/// Adds [AssetNode.builderOptions] for all [buildPhases] to this graph.
221-
void _addBuilderOptionsNodes(BuildPhases buildPhases) {
222-
for (var phaseNum = 0; phaseNum < buildPhases.length; phaseNum++) {
223-
var phase = buildPhases[phaseNum];
224-
if (phase is InBuildPhase) {
225-
add(
226-
AssetNode.builderOptions(
227-
builderOptionsIdForAction(phase, phaseNum),
228-
lastKnownDigest: computeBuilderOptionsDigest(phase.builderOptions),
229-
),
230-
);
231-
} else if (phase is PostBuildPhase) {
232-
var actionNum = 0;
233-
for (var builderAction in phase.builderActions) {
234-
add(
235-
AssetNode.builderOptions(
236-
builderOptionsIdForAction(builderAction, actionNum),
237-
lastKnownDigest: computeBuilderOptionsDigest(
238-
builderAction.builderOptions,
239-
),
240-
),
241-
);
242-
actionNum++;
243-
}
244-
} else {
245-
throw StateError('Invalid action type $phase');
246-
}
247-
}
248-
}
249-
250245
/// Uses [digestReader] to compute the [Digest] for nodes with [ids] and set
251246
/// the `lastKnownDigest` field.
252247
Future<void> _setLastKnownDigests(
@@ -333,12 +328,6 @@ class AssetGraph implements GeneratedAssetHider {
333328
for (final input in node.generatedNodeState!.inputs) {
334329
result.putIfAbsent(input, () => {}).add(node.id);
335330
}
336-
result
337-
.putIfAbsent(
338-
node.generatedNodeConfiguration!.builderOptionsId,
339-
() => {},
340-
)
341-
.add(node.id);
342331
} else if (node.type == NodeType.glob) {
343332
for (final input in node.globNodeState!.inputs) {
344333
result.putIfAbsent(input, () => {}).add(node.id);
@@ -615,25 +604,22 @@ class AssetGraph implements GeneratedAssetHider {
615604
}) {
616605
var allInputs = Set<AssetId>.from(newSources);
617606
if (placeholders != null) allInputs.addAll(placeholders);
618-
619-
for (var phaseNum = 0; phaseNum < buildPhases.length; phaseNum++) {
620-
var phase = buildPhases[phaseNum];
621-
if (phase is InBuildPhase) {
622-
allInputs.addAll(
623-
_addInBuildPhaseOutputs(
624-
phase,
625-
phaseNum,
626-
allInputs,
627-
buildPhases,
628-
rootPackage,
629-
),
630-
);
631-
} else if (phase is PostBuildPhase) {
632-
_addPostBuildActionApplications(phase, allInputs);
633-
} else {
634-
throw StateError('Unrecognized phase type $phase');
635-
}
607+
for (
608+
var phaseNum = 0;
609+
phaseNum < buildPhases.inBuildPhases.length;
610+
phaseNum++
611+
) {
612+
allInputs.addAll(
613+
_addInBuildPhaseOutputs(
614+
buildPhases.inBuildPhases[phaseNum],
615+
phaseNum,
616+
allInputs,
617+
buildPhases,
618+
rootPackage,
619+
),
620+
);
636621
}
622+
_addPostBuildActionApplications(buildPhases.postBuildPhase, allInputs);
637623
return allInputs;
638624
}
639625

@@ -651,8 +637,6 @@ class AssetGraph implements GeneratedAssetHider {
651637
String rootPackage,
652638
) {
653639
var phaseOutputs = <AssetId>{};
654-
var buildOptionsNodeId = builderOptionsIdForAction(phase, phaseNum);
655-
var builderOptionsNode = get(buildOptionsNodeId)!;
656640
var inputs =
657641
allInputs.where((input) => _actionMatches(phase, input)).toList();
658642
for (var input in inputs) {
@@ -667,7 +651,6 @@ class AssetGraph implements GeneratedAssetHider {
667651
var deleted = _addGeneratedOutputs(
668652
outputs,
669653
phaseNum,
670-
builderOptionsNode,
671654
buildPhases,
672655
rootPackage,
673656
primaryInput: input,
@@ -710,15 +693,11 @@ class AssetGraph implements GeneratedAssetHider {
710693
Set<AssetId> _addGeneratedOutputs(
711694
Iterable<AssetId> outputs,
712695
int phaseNumber,
713-
AssetNode builderOptionsNode,
714696
BuildPhases buildPhases,
715697
String rootPackage, {
716698
required AssetId primaryInput,
717699
required bool isHidden,
718700
}) {
719-
if (builderOptionsNode.type != NodeType.builderOptions) {
720-
throw ArgumentError('Expected node of type NodeType.builderOptionsNode');
721-
}
722701
var removed = <AssetId>{};
723702
Map<AssetId, Set<AssetId>>? computedOutputsBeforeRemoves;
724703
for (var output in outputs) {
@@ -734,9 +713,10 @@ class AssetGraph implements GeneratedAssetHider {
734713
throw DuplicateAssetNodeException(
735714
rootPackage,
736715
existing.id,
737-
(buildPhases[existingConfiguration.phaseNumber] as InBuildPhase)
716+
buildPhases
717+
.inBuildPhases[existingConfiguration.phaseNumber]
738718
.builderLabel,
739-
(buildPhases[phaseNumber] as InBuildPhase).builderLabel,
719+
buildPhases.inBuildPhases[phaseNumber].builderLabel,
740720
);
741721
}
742722
_removeRecursive(output, removedIds: removed);
@@ -749,7 +729,6 @@ class AssetGraph implements GeneratedAssetHider {
749729
pendingBuildAction: PendingBuildAction.build,
750730
wasOutput: false,
751731
isFailure: false,
752-
builderOptionsId: builderOptionsNode.id,
753732
isHidden: isHidden,
754733
);
755734
if (existing != null) {
@@ -833,22 +812,6 @@ class AssetGraph implements GeneratedAssetHider {
833812
}
834813
}
835814

836-
Digest computeBuilderOptionsDigest(BuilderOptions options) =>
837-
md5.convert(utf8.encode(json.encode(options.config)));
838-
839-
AssetId builderOptionsIdForAction(BuildAction action, int actionNumber) {
840-
if (action is InBuildPhase) {
841-
return AssetId(action.package, 'Phase$actionNumber.builderOptions');
842-
} else if (action is PostBuildAction) {
843-
return PostProcessBuildStepId.builderOptionsIdFor(
844-
package: action.package,
845-
actionNumber: actionNumber,
846-
);
847-
} else {
848-
throw StateError('Unsupported action type $action');
849-
}
850-
}
851-
852815
Set<AssetId> placeholderIdsFor(PackageGraph packageGraph) => Set<AssetId>.from(
853816
packageGraph.allPackages.keys.expand(
854817
(package) => [

build_runner_core/lib/src/asset_graph/graph_loader.dart

+12
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,18 @@ class AssetGraphLoader {
133133
}
134134
return null;
135135
}
136+
137+
// Move old build phases digests to "previous" fields, set the new ones.
138+
// These are used to check for phases to fully rerun due to changed options.
139+
cachedGraph.previousInBuildPhasesOptionsDigests =
140+
cachedGraph.inBuildPhasesOptionsDigests;
141+
cachedGraph.inBuildPhasesOptionsDigests =
142+
buildPhases.inBuildPhasesOptionsDigests;
143+
cachedGraph.previousPostBuildOptionsDigests =
144+
cachedGraph.postBuildActionsOptionsDigests;
145+
cachedGraph.postBuildActionsOptionsDigests =
146+
buildPhases.postBuildActionsOptionsDigests;
147+
136148
return cachedGraph;
137149
}
138150

build_runner_core/lib/src/asset_graph/node.dart

-18
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ part 'node.g.dart';
1818
class NodeType extends EnumClass {
1919
static Serializer<NodeType> get serializer => _$nodeTypeSerializer;
2020

21-
static const NodeType builderOptions = _$builderOptions;
2221
static const NodeType generated = _$generated;
2322
static const NodeType glob = _$glob;
2423
static const NodeType internal = _$internal;
@@ -122,17 +121,6 @@ abstract class AssetNode implements Built<AssetNode, AssetNodeBuilder> {
122121
b.lastKnownDigest = lastKnownDigest;
123122
});
124123

125-
/// A [BuilderOptions] object.
126-
///
127-
/// Each [AssetNode.generated] has one describing its configuration, so it
128-
/// rebuilds when the configuration changes.
129-
factory AssetNode.builderOptions(AssetId id, {Digest? lastKnownDigest}) =>
130-
AssetNode((b) {
131-
b.id = id;
132-
b.type = NodeType.builderOptions;
133-
b.lastKnownDigest = lastKnownDigest;
134-
});
135-
136124
/// A missing source file.
137125
///
138126
/// Created when a builder tries to read a non-existent file.
@@ -164,7 +152,6 @@ abstract class AssetNode implements Built<AssetNode, AssetNodeBuilder> {
164152
AssetId id, {
165153
Digest? lastKnownDigest,
166154
required AssetId primaryInput,
167-
required AssetId builderOptionsId,
168155
required int phaseNumber,
169156
required bool isHidden,
170157
Iterable<AssetId>? inputs,
@@ -175,7 +162,6 @@ abstract class AssetNode implements Built<AssetNode, AssetNodeBuilder> {
175162
b.id = id;
176163
b.type = NodeType.generated;
177164
b.generatedNodeConfiguration.primaryInput = primaryInput;
178-
b.generatedNodeConfiguration.builderOptionsId = builderOptionsId;
179165
b.generatedNodeConfiguration.phaseNumber = phaseNumber;
180166
b.generatedNodeConfiguration.isHidden = isHidden;
181167
b.generatedNodeState.inputs.replace(inputs ?? []);
@@ -258,10 +244,6 @@ abstract class GeneratedNodeConfiguration
258244
/// The primary input which generated this node.
259245
AssetId get primaryInput;
260246

261-
/// The [AssetId] of the node representing the [BuilderOptions] used to create
262-
/// this node.
263-
AssetId get builderOptionsId;
264-
265247
/// The phase in which this node is generated.
266248
///
267249
/// The generator that produces this node can only read files from earlier

0 commit comments

Comments
 (0)