Skip to content

Remove builder options nodes; store and check builder options by phase. #3983

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions build_runner/test/generate/watch_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ void main() {
final copyABuildApplication = applyToRoot(
TestBuilder(buildExtensions: appendExtension('.copy', from: '.txt')),
);
final defaultBuilderOptions = const BuilderOptions({});
final packageConfigId = makeAssetId('a|.dart_tool/package_config.json');
final packageGraph = buildPackageGraph({
rootPackage('a', path: path.absolute('a')): [],
Expand Down Expand Up @@ -362,12 +361,6 @@ void main() {
readerWriter,
);

var builderOptionsId = makeAssetId('a|Phase0.builderOptions');
var builderOptionsNode = AssetNode.builderOptions(
builderOptionsId,
lastKnownDigest: computeBuilderOptionsDigest(defaultBuilderOptions),
);

var bCopyId = makeAssetId('a|web/b.txt.copy');
var bTxtId = makeAssetId('a|web/b.txt');
var bCopyNode = AssetNode.generated(
Expand All @@ -377,7 +370,6 @@ void main() {
pendingBuildAction: PendingBuildAction.none,
wasOutput: true,
isFailure: false,
builderOptionsId: builderOptionsId,
lastKnownDigest: computeDigest(bCopyId, 'b2'),
inputs: [makeAssetId('a|web/b.txt')],
isHidden: false,
Expand All @@ -399,7 +391,6 @@ void main() {
pendingBuildAction: PendingBuildAction.none,
wasOutput: true,
isFailure: false,
builderOptionsId: builderOptionsId,
lastKnownDigest: computeDigest(cCopyId, 'c'),
inputs: [makeAssetId('a|web/c.txt')],
isHidden: false,
Expand All @@ -412,8 +403,6 @@ void main() {
], computeDigest(cTxtId, 'c')),
);

expectedGraph.add(builderOptionsNode);

// TODO: We dont have a shared way of computing the combined input
// hashes today, but eventually we should test those here too.
expect(
Expand Down
1 change: 0 additions & 1 deletion build_runner/test/server/asset_handler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ void main() {
graph.add(
AssetNode.generated(
AssetId('a', 'web/main.ddc.js'),
builderOptionsId: AssetId('_\$fake', 'options_id'),
phaseNumber: 0,
pendingBuildAction: PendingBuildAction.none,
isHidden: false,
Expand Down
1 change: 0 additions & 1 deletion build_runner/test/server/serve_handler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,6 @@ void main() {
assetGraph.add(
AssetNode.generated(
AssetId('a', 'web/main.ddc.js'),
builderOptionsId: AssetId('_\$fake', 'options_id'),
phaseNumber: 0,
pendingBuildAction: PendingBuildAction.none,
isHidden: false,
Expand Down
2 changes: 2 additions & 0 deletions build_runner_core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
- Track post process builder outputs separately from the main graph Instead of
in `postProcessAnchor` nodes.
- Compute outputs as needed instead of storing them in the asset graph.
- Check build options for changes in the phase setup instead of storing them
in the asset graph.

## 8.0.0

Expand Down
167 changes: 78 additions & 89 deletions build_runner_core/lib/src/asset_graph/graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:build/experiments.dart' as experiments_zone;
// ignore: implementation_imports
import 'package:build/src/internal.dart';
import 'package:built_collection/built_collection.dart';
import 'package:built_value/serializer.dart';
import 'package:crypto/crypto.dart';
import 'package:glob/glob.dart';
import 'package:meta/meta.dart';
Expand Down Expand Up @@ -61,8 +62,34 @@ class AssetGraph implements GeneratedAssetHider {
final Map<String, Map<PostProcessBuildStepId, Set<AssetId>>>
_postProcessBuildStepOutputs = {};

/// Digests from the previous build's [BuildPhases], or `null` if this is a
/// clean build.
BuiltList<Digest>? previousInBuildPhasesOptionsDigests;

/// Digests from the current build's [BuildPhases].
BuiltList<Digest> inBuildPhasesOptionsDigests;

/// Digests from the previous build's [BuildPhases], or `null` if this is a
/// clean build.
BuiltList<Digest>? previousPostBuildActionsOptionsDigests;

/// Digests from the current build's [BuildPhases].
BuiltList<Digest> postBuildActionsOptionsDigests;

AssetGraph._(
BuildPhases buildPhases,
this.dartVersion,
this.packageLanguageVersions,
this.enabledExperiments,
) : buildPhasesDigest = buildPhases.digest,
inBuildPhasesOptionsDigests = buildPhases.inBuildPhasesOptionsDigests,
postBuildActionsOptionsDigests =
buildPhases.postBuildActionsOptionsDigests;

AssetGraph._fromSerialized(
this.buildPhasesDigest,
this.inBuildPhasesOptionsDigests,
this.postBuildActionsOptionsDigests,
this.dartVersion,
this.packageLanguageVersions,
this.enabledExperiments,
Expand All @@ -80,21 +107,19 @@ class AssetGraph implements GeneratedAssetHider {
AssetReader digestReader,
) async {
var graph = AssetGraph._(
buildPhases.digest,
buildPhases,
Platform.version,
packageGraph.languageVersions,
experiments_zone.enabledExperiments.build(),
);
var placeholders = graph._addPlaceHolderNodes(packageGraph);
graph._addSources(sources);
graph
.._addBuilderOptionsNodes(buildPhases)
.._addOutputsForSources(
buildPhases,
sources,
packageGraph.root.name,
placeholders: placeholders,
);
graph._addOutputsForSources(
buildPhases,
sources,
packageGraph.root.name,
placeholders: placeholders,
);
// Pre-emptively compute digests for the nodes we know have outputs.
await graph._setLastKnownDigests(
sources.where((id) => graph.get(id)!.primaryOutputs.isNotEmpty),
Expand All @@ -112,6 +137,10 @@ class AssetGraph implements GeneratedAssetHider {
Map<String, Map<PostProcessBuildStepId, Set<AssetId>>>
get allPostProcessBuildStepOutputs => _postProcessBuildStepOutputs;

/// Whether this is a clean build, meaning there was no previous build state
/// loaded or it was discarded as incompatible.
bool get cleanBuild => previousInBuildPhasesOptionsDigests == null;

/// Checks if [id] exists in the graph.
bool contains(AssetId id) =>
_nodesByPackage[id.package]?.containsKey(id.path) ?? false;
Expand Down Expand Up @@ -217,36 +246,6 @@ class AssetGraph implements GeneratedAssetHider {
}
}

/// Adds [AssetNode.builderOptions] for all [buildPhases] to this graph.
void _addBuilderOptionsNodes(BuildPhases buildPhases) {
for (var phaseNum = 0; phaseNum < buildPhases.length; phaseNum++) {
var phase = buildPhases[phaseNum];
if (phase is InBuildPhase) {
add(
AssetNode.builderOptions(
builderOptionsIdForAction(phase, phaseNum),
lastKnownDigest: computeBuilderOptionsDigest(phase.builderOptions),
),
);
} else if (phase is PostBuildPhase) {
var actionNum = 0;
for (var builderAction in phase.builderActions) {
add(
AssetNode.builderOptions(
builderOptionsIdForAction(builderAction, actionNum),
lastKnownDigest: computeBuilderOptionsDigest(
builderAction.builderOptions,
),
),
);
actionNum++;
}
} else {
throw StateError('Invalid action type $phase');
}
}
}

/// Uses [digestReader] to compute the [Digest] for nodes with [ids] and set
/// the `lastKnownDigest` field.
Future<void> _setLastKnownDigests(
Expand Down Expand Up @@ -333,12 +332,6 @@ class AssetGraph implements GeneratedAssetHider {
for (final input in node.generatedNodeState!.inputs) {
result.putIfAbsent(input, () => {}).add(node.id);
}
result
.putIfAbsent(
node.generatedNodeConfiguration!.builderOptionsId,
() => {},
)
.add(node.id);
} else if (node.type == NodeType.glob) {
for (final input in node.globNodeState!.inputs) {
result.putIfAbsent(input, () => {}).add(node.id);
Expand Down Expand Up @@ -496,6 +489,8 @@ class AssetGraph implements GeneratedAssetHider {
var invalidatedIds = <AssetId>{};
final computedOutputs = computeOutputs();

// TODO(davidmorgan): it should be possible to track what needs building in
// the `Build` class instead of this invalidation logic.
void invalidateNodeAndDeps(AssetId startNodeId) {
if (!invalidatedIds.add(startNodeId)) return;
var nodesToInvalidate = [startNodeId];
Expand Down Expand Up @@ -536,6 +531,26 @@ class AssetGraph implements GeneratedAssetHider {
invalidateNodeAndDeps(changed);
}

// Invalidate nodes that depend on nodes that will be rebuilt due to changed
// build options.
if (previousInBuildPhasesOptionsDigests != null) {
final invalidatedPhases = <int>{};
for (var i = 0; i != buildPhases.inBuildPhases.length; i++) {
if (previousInBuildPhasesOptionsDigests![i] !=
inBuildPhasesOptionsDigests[i]) {
invalidatedPhases.add(i);
}
}
for (final node in allNodes) {
if (node.type == NodeType.generated &&
invalidatedPhases.contains(
node.generatedNodeConfiguration!.phaseNumber,
)) {
invalidateNodeAndDeps(node.id);
}
}
}

// For all new or deleted assets, check if they match any glob nodes and
// invalidate those.
for (var id in allNewAndDeletedIds) {
Expand Down Expand Up @@ -615,25 +630,22 @@ class AssetGraph implements GeneratedAssetHider {
}) {
var allInputs = Set<AssetId>.from(newSources);
if (placeholders != null) allInputs.addAll(placeholders);

for (var phaseNum = 0; phaseNum < buildPhases.length; phaseNum++) {
var phase = buildPhases[phaseNum];
if (phase is InBuildPhase) {
allInputs.addAll(
_addInBuildPhaseOutputs(
phase,
phaseNum,
allInputs,
buildPhases,
rootPackage,
),
);
} else if (phase is PostBuildPhase) {
_addPostBuildActionApplications(phase, allInputs);
} else {
throw StateError('Unrecognized phase type $phase');
}
for (
var phaseNum = 0;
phaseNum < buildPhases.inBuildPhases.length;
phaseNum++
) {
allInputs.addAll(
_addInBuildPhaseOutputs(
buildPhases.inBuildPhases[phaseNum],
phaseNum,
allInputs,
buildPhases,
rootPackage,
),
);
}
_addPostBuildActionApplications(buildPhases.postBuildPhase, allInputs);
return allInputs;
}

Expand All @@ -651,8 +663,6 @@ class AssetGraph implements GeneratedAssetHider {
String rootPackage,
) {
var phaseOutputs = <AssetId>{};
var buildOptionsNodeId = builderOptionsIdForAction(phase, phaseNum);
var builderOptionsNode = get(buildOptionsNodeId)!;
var inputs =
allInputs.where((input) => _actionMatches(phase, input)).toList();
for (var input in inputs) {
Expand All @@ -667,7 +677,6 @@ class AssetGraph implements GeneratedAssetHider {
var deleted = _addGeneratedOutputs(
outputs,
phaseNum,
builderOptionsNode,
buildPhases,
rootPackage,
primaryInput: input,
Expand Down Expand Up @@ -710,15 +719,11 @@ class AssetGraph implements GeneratedAssetHider {
Set<AssetId> _addGeneratedOutputs(
Iterable<AssetId> outputs,
int phaseNumber,
AssetNode builderOptionsNode,
BuildPhases buildPhases,
String rootPackage, {
required AssetId primaryInput,
required bool isHidden,
}) {
if (builderOptionsNode.type != NodeType.builderOptions) {
throw ArgumentError('Expected node of type NodeType.builderOptionsNode');
}
var removed = <AssetId>{};
Map<AssetId, Set<AssetId>>? computedOutputsBeforeRemoves;
for (var output in outputs) {
Expand All @@ -734,9 +739,10 @@ class AssetGraph implements GeneratedAssetHider {
throw DuplicateAssetNodeException(
rootPackage,
existing.id,
(buildPhases[existingConfiguration.phaseNumber] as InBuildPhase)
buildPhases
.inBuildPhases[existingConfiguration.phaseNumber]
.builderLabel,
(buildPhases[phaseNumber] as InBuildPhase).builderLabel,
buildPhases.inBuildPhases[phaseNumber].builderLabel,
);
}
_removeRecursive(output, removedIds: removed);
Expand All @@ -749,7 +755,6 @@ class AssetGraph implements GeneratedAssetHider {
pendingBuildAction: PendingBuildAction.build,
wasOutput: false,
isFailure: false,
builderOptionsId: builderOptionsNode.id,
isHidden: isHidden,
);
if (existing != null) {
Expand Down Expand Up @@ -833,22 +838,6 @@ class AssetGraph implements GeneratedAssetHider {
}
}

Digest computeBuilderOptionsDigest(BuilderOptions options) =>
md5.convert(utf8.encode(json.encode(options.config)));

AssetId builderOptionsIdForAction(BuildAction action, int actionNumber) {
if (action is InBuildPhase) {
return AssetId(action.package, 'Phase$actionNumber.builderOptions');
} else if (action is PostBuildAction) {
return PostProcessBuildStepId.builderOptionsIdFor(
package: action.package,
actionNumber: actionNumber,
);
} else {
throw StateError('Unsupported action type $action');
}
}

Set<AssetId> placeholderIdsFor(PackageGraph packageGraph) => Set<AssetId>.from(
packageGraph.allPackages.keys.expand(
(package) => [
Expand Down
12 changes: 12 additions & 0 deletions build_runner_core/lib/src/asset_graph/graph_loader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,18 @@ class AssetGraphLoader {
}
return null;
}

// Move old build phases digests to "previous" fields, set the new ones.
// These are used to check for phases to fully rerun due to changed options.
cachedGraph.previousInBuildPhasesOptionsDigests =
cachedGraph.inBuildPhasesOptionsDigests;
cachedGraph.inBuildPhasesOptionsDigests =
buildPhases.inBuildPhasesOptionsDigests;
cachedGraph.previousPostBuildActionsOptionsDigests =
cachedGraph.postBuildActionsOptionsDigests;
cachedGraph.postBuildActionsOptionsDigests =
buildPhases.postBuildActionsOptionsDigests;

return cachedGraph;
}

Expand Down
Loading
Loading