Skip to content

Split post process builder state out of the main graph. #3959

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 1 commit into from
Apr 11, 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
4 changes: 4 additions & 0 deletions build_runner/test/generate/watch_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,10 @@ void main() {
cachedGraph,
equalsAssetGraph(expectedGraph, checkPreviousInputsDigest: false),
);
expect(
cachedGraph.allPostProcessBuildStepOutputs,
expectedGraph.allPostProcessBuildStepOutputs,
);
});

test('ignores events from nested packages', () async {
Expand Down
9 changes: 6 additions & 3 deletions build_runner/test/server/asset_handler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:build_runner/src/server/server.dart';
import 'package:build_runner_core/build_runner_core.dart';
import 'package:build_runner_core/src/asset_graph/graph.dart';
import 'package:build_runner_core/src/asset_graph/node.dart';
import 'package:build_runner_core/src/asset_graph/post_process_build_step_id.dart';
import 'package:build_runner_core/src/generate/build_phases.dart';
import 'package:build_runner_core/src/generate/options.dart';
import 'package:build_runner_core/src/package_graph/target_graph.dart';
Expand Down Expand Up @@ -48,9 +49,11 @@ void main() {
void addAsset(String id, String content, {bool deleted = false}) {
var node = makeAssetNode(id, [], computeDigest(AssetId.parse(id), 'a'));
if (deleted) {
node = node.rebuild(
(b) => b..deletedBy.add(node.id.addExtension('.post_anchor.1')),
);
node = node.rebuild((b) {
b.deletedBy.add(
PostProcessBuildStepId(input: node.id, actionNumber: 1),
);
});
}
graph.add(node);
delegate.testing.writeString(node.id, content);
Expand Down
9 changes: 6 additions & 3 deletions build_runner/test/server/serve_handler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import 'package:build_runner/src/server/server.dart';
import 'package:build_runner_core/build_runner_core.dart';
import 'package:build_runner_core/src/asset_graph/graph.dart';
import 'package:build_runner_core/src/asset_graph/node.dart';
import 'package:build_runner_core/src/asset_graph/post_process_build_step_id.dart';
import 'package:build_runner_core/src/generate/build_phases.dart';
import 'package:build_runner_core/src/generate/options.dart';
import 'package:build_runner_core/src/generate/performance_tracker.dart';
Expand Down Expand Up @@ -139,9 +140,11 @@ void main() {
void addSource(String id, String content, {bool deleted = false}) {
var node = makeAssetNode(id, [], computeDigest(AssetId.parse(id), content));
if (deleted) {
node = node.rebuild(
(b) => b..deletedBy.add(node.id.addExtension('.post_anchor.1')),
);
node = node.rebuild((b) {
b.deletedBy.add(
PostProcessBuildStepId(input: node.id, actionNumber: 1),
);
});
}
assetGraph.add(node);
readerWriter.testing.writeString(node.id, content);
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 @@ -27,6 +27,8 @@
- Add `reportUnusedAssetsForInput` to `BuildOptions`, to listen for when
a builder notifies that an asset is unused.
- Use `LibraryCycleGraphLoader` to load transitive deps for analysis.
- Track post process builder outputs separately from the main graph Instead of
in `postProcessAnchor` nodes.

## 8.0.0

Expand Down
86 changes: 63 additions & 23 deletions build_runner_core/lib/src/asset_graph/graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import 'package:build/src/internal.dart';
import 'package:built_collection/built_collection.dart';
import 'package:crypto/crypto.dart';
import 'package:glob/glob.dart';
import 'package:meta/meta.dart';
import 'package:package_config/package_config.dart';
import 'package:watcher/watcher.dart';

Expand All @@ -23,6 +24,8 @@ import '../generate/phase.dart';
import '../util/constants.dart';
import 'exceptions.dart';
import 'node.dart';
import 'post_process_build_step_id.dart';
import 'serializers.dart';

part 'serialization.dart';

Expand All @@ -46,6 +49,14 @@ class AssetGraph implements GeneratedAssetHider {

final BuiltMap<String, LanguageVersion?> packageLanguageVersions;

/// All post process build steps outputs, indexed by package then
/// [PostProcessBuildStepId].
///
/// Created with empty outputs at the start of the build if it's a new build
/// step; or deserialized with previous build outputs if it has run before.
final Map<String, Map<PostProcessBuildStepId, Set<AssetId>>>
_postProcessBuildStepOutputs = {};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this really the indentation provided by dart format? (also below)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is ... formatting gets a bit surprising when there are very long identifiers.

There's a format presubmit :) that's checked before the tests run.


AssetGraph._(
this.buildPhasesDigest,
this.dartVersion,
Expand Down Expand Up @@ -93,6 +104,10 @@ class AssetGraph implements GeneratedAssetHider {

List<int> serialize() => _AssetGraphSerializer(this).serialize();

@visibleForTesting
Map<String, Map<PostProcessBuildStepId, Set<AssetId>>>
get allPostProcessBuildStepOutputs => _postProcessBuildStepOutputs;

/// Checks if [id] exists in the graph.
bool contains(AssetId id) =>
_nodesByPackage[id.package]?.containsKey(id.path) ?? false;
Expand Down Expand Up @@ -243,9 +258,6 @@ class AssetGraph implements GeneratedAssetHider {
var node = get(id);
if (node == null) return removedIds;
removedIds.add(id);
for (var anchor in node.anchorOutputs.toList()) {
_removeRecursive(anchor, removedIds: removedIds);
}
for (var output in node.primaryOutputs.toList()) {
_removeRecursive(output, removedIds: removedIds);
}
Expand Down Expand Up @@ -290,6 +302,12 @@ class AssetGraph implements GeneratedAssetHider {
if (node.type != NodeType.missingSource) {
_nodesByPackage[id.package]!.remove(id.path);
}

// Remove post build action applications with removed assets as inputs.
for (final packageOutputs in _postProcessBuildStepOutputs.values) {
packageOutputs.removeWhere((id, _) => removedIds!.contains(id.input));
}

return removedIds;
}

Expand All @@ -301,6 +319,31 @@ class AssetGraph implements GeneratedAssetHider {
Iterable<AssetNode> packageNodes(String package) =>
_nodesByPackage[package]?.values ?? [];

/// All the post process build steps for `package`.
Iterable<PostProcessBuildStepId> postProcessBuildStepIds({
required String package,
}) => _postProcessBuildStepOutputs[package]?.keys ?? const [];

/// Creates or updates state for a [PostProcessBuildStepId].
void updatePostProcessBuildStep(
PostProcessBuildStepId buildStepId, {
required Set<AssetId> outputs,
}) {
_postProcessBuildStepOutputs.putIfAbsent(
buildStepId.input.package,
() => {},
)[buildStepId] =
outputs;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you must just have forgotten to format it right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this is the output :)

}

/// Gets outputs of a [PostProcessBuildStepId].
///
/// These are set using [updatePostProcessBuildStep] during the build, then
/// used to clean up prior outputs in the next build.
Iterable<AssetId> postProcessBuildStepOutputs(PostProcessBuildStepId action) {
return _postProcessBuildStepOutputs[action.input.package]![action]!;
}

/// All the generated outputs in the graph.
Iterable<AssetId> get outputs =>
allNodes.where((n) => n.type == NodeType.generated).map((n) => n.id);
Expand Down Expand Up @@ -547,7 +590,7 @@ class AssetGraph implements GeneratedAssetHider {
),
);
} else if (phase is PostBuildPhase) {
_addPostBuildPhaseAnchors(phase, allInputs);
_addPostBuildActionApplications(phase, allInputs);
} else {
throw StateError('Unrecognized phase type $phase');
}
Expand Down Expand Up @@ -600,27 +643,21 @@ class AssetGraph implements GeneratedAssetHider {
return phaseOutputs;
}

/// Adds all [AssetNode.postProcessAnchor]s for [phase] given [allInputs];
///
/// Does not return anything because [AssetNode.postProcessAnchor]s are
/// synthetic and should not be treated as inputs.
void _addPostBuildPhaseAnchors(PostBuildPhase phase, Set<AssetId> allInputs) {
var actionNum = 0;
/// Adds all [PostProcessBuildStepId]s for [phase] given [allInputs];
void _addPostBuildActionApplications(
PostBuildPhase phase,
Set<AssetId> allInputs,
) {
var actionNumber = 0;
for (var action in phase.builderActions) {
var inputs = allInputs.where((input) => _actionMatches(action, input));
for (var input in inputs) {
var buildOptionsNodeId = builderOptionsIdForAction(action, actionNum);
var anchor = AssetNode.postProcessAnchorForInputAndAction(
input,
actionNum,
buildOptionsNodeId,
updatePostProcessBuildStep(
PostProcessBuildStepId(input: input, actionNumber: actionNumber),
outputs: {},
);
add(anchor);
updateNode(input, (nodeBuilder) {
nodeBuilder.anchorOutputs.add(anchor.id);
});
}
actionNum++;
actionNumber++;
}
}

Expand Down Expand Up @@ -762,11 +799,14 @@ class AssetGraph implements GeneratedAssetHider {
Digest computeBuilderOptionsDigest(BuilderOptions options) =>
md5.convert(utf8.encode(json.encode(options.config)));

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