Skip to content

Commit 6f4c629

Browse files
authored
Fix library cycle graph update. (#4004)
1 parent c0f78f3 commit 6f4c629

File tree

7 files changed

+111
-29
lines changed

7 files changed

+111
-29
lines changed

build/lib/src/library_cycle_graph/asset_deps_loader.dart

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,22 +41,26 @@ class AssetDepsLoader {
4141
final result =
4242
ExpiringValueBuilder<AssetDeps>()..expiresAfter = content.expiresAfter;
4343

44-
final parsed =
45-
parseString(content: content.value, throwIfDiagnostics: false).unit;
46-
4744
final depsNodeBuilder = AssetDepsBuilder();
4845

49-
for (final directive in parsed.directives) {
50-
if (directive is! UriBasedDirective) continue;
51-
final uri = directive.uri.stringValue;
52-
if (uri == null) continue;
53-
final parsedUri = Uri.parse(uri);
54-
if (_ignoredSchemes.any(parsedUri.isScheme)) continue;
55-
final assetId = AssetId.resolve(parsedUri, from: id);
56-
depsNodeBuilder.deps.add(assetId);
46+
if (content.value == '') {
47+
result.value = AssetDeps.empty;
48+
} else {
49+
final parsed =
50+
parseString(content: content.value, throwIfDiagnostics: false).unit;
51+
52+
for (final directive in parsed.directives) {
53+
if (directive is! UriBasedDirective) continue;
54+
final uri = directive.uri.stringValue;
55+
if (uri == null) continue;
56+
final parsedUri = Uri.parse(uri);
57+
if (_ignoredSchemes.any(parsedUri.isScheme)) continue;
58+
final assetId = AssetId.resolve(parsedUri, from: id);
59+
depsNodeBuilder.deps.add(assetId);
60+
}
61+
62+
result.value = depsNodeBuilder.build();
5763
}
58-
59-
result.value = depsNodeBuilder.build();
6064
return result.build();
6165
}
6266
}

build/lib/src/library_cycle_graph/phased_asset_deps.dart

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,23 @@ abstract class PhasedAssetDeps
3232
factory PhasedAssetDeps.of(Map<AssetId, PhasedValue<AssetDeps>> assetDeps) =>
3333
_$PhasedAssetDeps._(assetDeps: assetDeps.build());
3434

35-
/// Returns this data with [other] added to it.
36-
PhasedAssetDeps addAll(PhasedAssetDeps other) {
35+
/// Returns `this` data with [other] added to it.
36+
///
37+
/// For each asset: if [other] has a complete value for that asset, use the
38+
/// new value. Otherwise, use the old value from `this`.
39+
PhasedAssetDeps update(PhasedAssetDeps other) {
3740
final result = toBuilder();
3841
for (final entry in other.assetDeps.entries) {
39-
result.assetDeps[entry.key] = entry.value;
42+
final updatedValue = entry.value;
43+
if (updatedValue.isComplete) {
44+
result.assetDeps[entry.key] = updatedValue;
45+
} else {
46+
// The only allow "not available yet" value is `AssetDeps.empty`.
47+
if (updatedValue.values.length != 1 ||
48+
updatedValue.values.single.value != AssetDeps.empty) {
49+
throw StateError('Unexpected value: $updatedValue');
50+
}
51+
}
4052
}
4153
return result.build();
4254
}

build/lib/src/library_cycle_graph/phased_value.dart

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ abstract class PhasedValue<T>
5858
b.values.add(ExpiringValue<T>(value));
5959
});
6060

61-
/// A value that will be generated during [untilAfterPhase].
61+
/// A value that will be generated in phase [expiresAfter].
6262
///
6363
/// Pass the "missing" value for T as [before].
6464
factory PhasedValue.unavailable({
65-
required int untilAfterPhase,
65+
required int expiresAfter,
6666
required T before,
6767
}) => PhasedValue((b) {
68-
b.values.add(ExpiringValue<T>(before, expiresAfter: untilAfterPhase));
68+
b.values.add(ExpiringValue<T>(before, expiresAfter: expiresAfter));
6969
});
7070

7171
/// A value that is generated during [atPhase], changing from [before] to
@@ -108,7 +108,7 @@ abstract class PhasedValue<T>
108108
return value;
109109
}
110110
}
111-
throw StateError('No value for phase $phase in $this.');
111+
throw StateError('No value for phase $phase.');
112112
}
113113

114114
/// The value at [phase].
@@ -121,7 +121,7 @@ abstract class PhasedValue<T>
121121
///
122122
/// Throws if not [isComplete], meaning the last value is not known.
123123
T get lastValue {
124-
if (!isComplete) throw StateError('Not complete, no last value: $this');
124+
if (!isComplete) throw StateError('Not complete, no last value.');
125125
return values.last.value;
126126
}
127127

@@ -138,8 +138,7 @@ abstract class PhasedValue<T>
138138
if (value.expiresAfter != null &&
139139
value.expiresAfter! <= values.last.expiresAfter!) {
140140
throw StateError(
141-
"Can't follow with a value expiring before or at the existing value."
142-
' This: $this, followedBy: $value',
141+
"Can't follow with a value expiring before or at the existing value.",
143142
);
144143
}
145144
return rebuild((b) {

build/test/library_cycle_graph/library_cycle_graph_loader_test.dart

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,7 @@ void main() {
151151
group('with generated nodes', () {
152152
test('single generated node', () async {
153153
final nodeLoader0 = TestAssetDepsLoader(0, {
154-
a1: PhasedValue.unavailable(
155-
untilAfterPhase: 1,
156-
before: AssetDeps.empty,
157-
),
154+
a1: PhasedValue.unavailable(expiresAfter: 1, before: AssetDeps.empty),
158155
});
159156
final nodeLoader2 = TestAssetDepsLoader(2, {
160157
a1: PhasedValue.generated(

build_runner_core/lib/src/generate/build.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,10 +263,13 @@ class Build {
263263
_logger,
264264
'Caching finalized dependency graph',
265265
() async {
266+
// Combine previous phased asset deps, if any, with the newly loaded
267+
// deps. Because of skipped builds, the newly loaded deps might just
268+
// say "not generated yet", in which case the old value is retained.
266269
final updatedPhasedAssetDeps =
267270
assetGraph.previousPhasedAssetDeps == null
268271
? AnalysisDriverModel.sharedInstance.phasedAssetDeps()
269-
: assetGraph.previousPhasedAssetDeps!.addAll(
272+
: assetGraph.previousPhasedAssetDeps!.update(
270273
AnalysisDriverModel.sharedInstance.phasedAssetDeps(),
271274
);
272275
assetGraph.previousPhasedAssetDeps = updatedPhasedAssetDeps;

build_runner_core/lib/src/generate/single_step_reader_writer.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ class SingleStepReaderWriter extends AssetReader
457457
if (node.type == NodeType.generated) {
458458
final nodePhase = node.generatedNodeConfiguration!.phaseNumber;
459459
if (nodePhase >= phase) {
460-
return PhasedValue.unavailable(before: '', untilAfterPhase: nodePhase);
460+
return PhasedValue.unavailable(before: '', expiresAfter: nodePhase);
461461
} else {
462462
// If needed, trigger a build at an earlier phase.
463463
if (!_runningBuild.assetIsProcessedOutput(id)) {
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:test/test.dart';
6+
7+
import 'invalidation_tester.dart';
8+
9+
void main() {
10+
late InvalidationTester tester;
11+
12+
setUp(() {
13+
tester = InvalidationTester();
14+
});
15+
16+
// Builds a.2 and a.3 both resolve z; the resolved library cycle graph
17+
// differs because z.4 is generated between a.2 and a.3.
18+
group('a.1 <== a.2, a.1 <== a.3, z.4 <== z.5, '
19+
'a.2 resolves: z --> z.5, a.3 resolves: z --> z.5 --> z2', () {
20+
setUp(() {
21+
tester = InvalidationTester();
22+
tester.sources(['a.1', 'x1', 'z', 'z.4', 'z2']);
23+
tester.importGraph({
24+
'z': ['z.5'],
25+
'z.5': ['z2'],
26+
});
27+
28+
tester.builder(from: '.1', to: '.2')
29+
..reads('.1')
30+
..readsOther('x1')
31+
..resolvesOther('z')
32+
..writes('.2');
33+
34+
tester.builder(from: '.4', to: '.5')
35+
..reads('.4')
36+
..writes('.5');
37+
38+
tester.builder(from: '.1', to: '.3')
39+
..reads('.1')
40+
..resolvesOther('z')
41+
..writes('.3');
42+
});
43+
44+
test('a.2+a.3 are built', () async {
45+
expect(await tester.build(), Result(written: ['a.2', 'z.5', 'a.3']));
46+
});
47+
48+
test('change z2, a.3 is rebuilt', () async {
49+
await tester.build();
50+
expect(await tester.build(change: 'z2'), Result(written: ['a.3']));
51+
});
52+
53+
test('partial library cycle graph update does not lose data', () async {
54+
// Full build: full library cycle graph is computed.
55+
expect(await tester.build(), Result(written: ['a.2', 'z.5', 'a.3']));
56+
// Trigger only a.2: library cycle graph is only computed up to a.2 phase.
57+
expect(await tester.build(change: 'x1'), Result(written: ['a.2']));
58+
59+
// Next build still needs the full library cycle graph, if it succeeds
60+
// part of the graph was retained from the first build.
61+
expect(await tester.build(), Result());
62+
63+
// Change using the graph still rebuilds as expected.
64+
expect(await tester.build(change: 'z2'), Result(written: ['a.3']));
65+
});
66+
});
67+
}

0 commit comments

Comments
 (0)