Skip to content

Commit 5b59745

Browse files
davidmorgandavidmorgan
authored
Add to resolver_test, add to AnalysisDriverModel as needed to satisfy the tests. (#3822)
* Add to `resolver_test`, add to `AnalysisDriverModel` as needed to satisfy the tests. Run the tests with both "shared" instances, re-used between tests (as before) and "new" instances. * Remove unneeded null check. * Address review comments.
1 parent 4e5a83c commit 5b59745

File tree

4 files changed

+397
-67
lines changed

4 files changed

+397
-67
lines changed

‎build_resolvers/lib/src/analysis_driver_model.dart

Lines changed: 185 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,24 @@ class AnalysisDriverModel {
3434
final MemoryResourceProvider resourceProvider =
3535
MemoryResourceProvider(context: p.posix);
3636

37+
/// The import graph of all sources needed for analysis.
38+
final _graph = _Graph();
39+
40+
/// Assets that have been synced into the in-memory filesystem
41+
/// [resourceProvider].
42+
final _syncedOntoResourceProvider = <AssetId>{};
43+
3744
/// Notifies that [step] has completed.
3845
///
3946
/// All build steps must complete before [reset] is called.
4047
void notifyComplete(BuildStep step) {
41-
// TODO(davidmorgan): add test coverage, fix implementation.
48+
// This implementation doesn't keep state per `BuildStep`, nothing to do.
4249
}
4350

4451
/// Clear cached information specific to an individual build.
4552
void reset() {
46-
// TODO(davidmorgan): add test coverage, fix implementation.
53+
_graph.clear();
54+
_syncedOntoResourceProvider.clear();
4755
}
4856

4957
/// Attempts to parse [uri] into an [AssetId] and returns it if it is cached.
@@ -82,66 +90,85 @@ class AnalysisDriverModel {
8290
FutureOr<void> Function(AnalysisDriverForPackageBuild))
8391
withDriverResource,
8492
{required bool transitive}) async {
85-
/// TODO(davidmorgan): add test coverage for whether transitive
86-
/// sources are read when [transitive] is false, fix the implementation
87-
/// here.
88-
/// TODO(davidmorgan): add test coverage for whether
89-
/// `.transitive_deps` files cut off the reporting of deps to the
90-
/// [buildStep], fix the implementation here.
91-
92-
// Find transitive deps, this also informs [buildStep] of all inputs).
93-
final ids = await _expandToTransitive(buildStep, entryPoints);
94-
95-
// Apply changes to in-memory filesystem.
96-
for (final id in ids) {
97-
if (await buildStep.canRead(id)) {
98-
final content = await buildStep.readAsString(id);
99-
100-
/// TODO(davidmorgan): add test coverage for when a file is
101-
/// modified rather than added, fix the implementation here.
102-
resourceProvider.newFile(id.asPath, content);
103-
} else {
104-
if (resourceProvider.getFile(id.asPath).exists) {
105-
resourceProvider.deleteFile(id.asPath);
106-
}
107-
}
108-
}
109-
110-
// Notify the analyzer of changes.
93+
// Immediately take the lock on `driver` so that the whole class state,
94+
// `_graph` and `_readForAnalyzir`, is only mutated by one build step at a
95+
// time. Otherwise, interleaved access complicates processing significantly.
11196
await withDriverResource((driver) async {
112-
for (final id in ids) {
113-
// TODO(davidmorgan): add test coverage for over-notification of
114-
// changes, fix the implementaion here.
115-
driver.changeFile(id.asPath);
116-
}
117-
await driver.applyPendingFileChanges();
97+
return _performResolve(driver, buildStep, entryPoints, withDriverResource,
98+
transitive: transitive);
11899
});
119100
}
120101

121-
/// Walks the import graph from [ids], returns full transitive deps.
122-
Future<Set<AssetId>> _expandToTransitive(
123-
AssetReader reader, Iterable<AssetId> ids) async {
124-
final result = ids.toSet();
125-
final nextIds = Queue.of(ids);
126-
while (nextIds.isNotEmpty) {
127-
final nextId = nextIds.removeFirst();
102+
Future<void> _performResolve(
103+
AnalysisDriverForPackageBuild driver,
104+
BuildStep buildStep,
105+
List<AssetId> entryPoints,
106+
Future<void> Function(
107+
FutureOr<void> Function(AnalysisDriverForPackageBuild))
108+
withDriverResource,
109+
{required bool transitive}) async {
110+
var idsToSyncOntoResourceProvider = entryPoints;
111+
Iterable<AssetId> inputIds = entryPoints;
128112

129-
// Skip if not readable. Note that calling `canRead` still makes it a
130-
// dependency of the `BuildStep`.
131-
if (!await reader.canRead(nextId)) continue;
113+
// If requested, find transitive imports.
114+
if (transitive) {
115+
await _graph.load(buildStep, entryPoints);
116+
idsToSyncOntoResourceProvider = _graph.nodes.keys.toList();
117+
inputIds = _graph.inputsFor(entryPoints);
132118

133-
final content = await reader.readAsString(nextId);
134-
final deps = _parseDependencies(content, nextId);
119+
// Check for inputs that were missing when the directive graph was read
120+
// but have since been written by another build action.
121+
for (final id in inputIds
122+
.where((id) => !id.path.endsWith(_transitiveDigestExtension))) {
123+
if (_graph.nodes[id]!.isMissing) {
124+
if (await buildStep.canRead(id)) {
125+
idsToSyncOntoResourceProvider.add(id);
126+
_syncedOntoResourceProvider.remove(id);
127+
}
128+
}
129+
}
130+
}
135131

136-
// For each dep, if it's not in `result` yet, it's newly-discovered:
137-
// add it to `nextIds`.
138-
for (final dep in deps) {
139-
if (result.add(dep)) {
140-
nextIds.add(dep);
132+
// Notify [buildStep] of its inputs.
133+
for (final id in inputIds) {
134+
await buildStep.canRead(id);
135+
}
136+
137+
// Sync changes onto the "URI resolver", the in-memory filesystem.
138+
final changedIds = <AssetId>[];
139+
for (final id in idsToSyncOntoResourceProvider) {
140+
if (!_syncedOntoResourceProvider.add(id)) continue;
141+
final content =
142+
await buildStep.canRead(id) ? await buildStep.readAsString(id) : null;
143+
final inMemoryFile = resourceProvider.getFile(id.asPath);
144+
final inMemoryContent =
145+
inMemoryFile.exists ? inMemoryFile.readAsStringSync() : null;
146+
147+
if (content != inMemoryContent) {
148+
if (content == null) {
149+
// TODO(davidmorgan): per "globallySeenAssets" in
150+
// BuildAssetUriResolver, deletes should only be applied at the end
151+
// of the build, in case the file is actually there but not visible
152+
// to the current reader.
153+
resourceProvider.deleteFile(id.asPath);
154+
changedIds.add(id);
155+
} else {
156+
if (inMemoryContent == null) {
157+
resourceProvider.newFile(id.asPath, content);
158+
} else {
159+
resourceProvider.modifyFile(id.asPath, content);
160+
}
161+
changedIds.add(id);
141162
}
142163
}
143164
}
144-
return result;
165+
166+
// Notify the analyzer of changes and wait for it to update its internal
167+
// state.
168+
for (final id in changedIds) {
169+
driver.changeFile(id.asPath);
170+
}
171+
await driver.applyPendingFileChanges();
145172
}
146173
}
147174

@@ -167,3 +194,109 @@ extension _AssetIdExtensions on AssetId {
167194
/// Asset path for the in-memory filesystem.
168195
String get asPath => AnalysisDriverModelUriResolver.assetPath(this);
169196
}
197+
198+
/// The directive graph of all known sources.
199+
///
200+
/// Also tracks whether there is a `.transitive_digest` file next to each source
201+
/// asset, and tracks missing files.
202+
class _Graph {
203+
final Map<AssetId, _Node> nodes = {};
204+
205+
/// Walks the import graph from [ids] loading into [nodes].
206+
Future<void> load(AssetReader reader, Iterable<AssetId> ids) async {
207+
// TODO(davidmorgan): check if List is faster.
208+
final nextIds = Queue.of(ids);
209+
while (nextIds.isNotEmpty) {
210+
final nextId = nextIds.removeFirst();
211+
212+
// Skip if already seen.
213+
if (nodes.containsKey(nextId)) continue;
214+
215+
final hasTransitiveDigestAsset =
216+
await reader.canRead(nextId.addExtension(_transitiveDigestExtension));
217+
218+
// Skip if not readable.
219+
if (!await reader.canRead(nextId)) {
220+
nodes[nextId] = _Node.missing(
221+
id: nextId, hasTransitiveDigestAsset: hasTransitiveDigestAsset);
222+
continue;
223+
}
224+
225+
final content = await reader.readAsString(nextId);
226+
final deps = _parseDependencies(content, nextId);
227+
nodes[nextId] = _Node(
228+
id: nextId,
229+
deps: deps,
230+
hasTransitiveDigestAsset: hasTransitiveDigestAsset);
231+
nextIds.addAll(deps.where((id) => !nodes.containsKey(id)));
232+
}
233+
}
234+
235+
void clear() {
236+
nodes.clear();
237+
}
238+
239+
/// The inputs for a build action analyzing [entryPoints].
240+
///
241+
/// This is transitive deps, but cut off by the presence of any
242+
/// `.transitive_digest` file next to an asset.
243+
Set<AssetId> inputsFor(Iterable<AssetId> entryPoints) {
244+
final result = entryPoints.toSet();
245+
final nextIds = Queue.of(entryPoints);
246+
247+
while (nextIds.isNotEmpty) {
248+
final nextId = nextIds.removeFirst();
249+
final node = nodes[nextId]!;
250+
251+
// Add the transitive digest file as an input. If it exists, skip deps.
252+
result.add(nextId.addExtension(_transitiveDigestExtension));
253+
if (node.hasTransitiveDigestAsset) {
254+
continue;
255+
}
256+
257+
// Skip if there are no deps because the file is missing.
258+
if (node.isMissing) continue;
259+
260+
// For each dep, if it's not in `result` yet, it's newly-discovered:
261+
// add it to `nextIds`.
262+
for (final dep in node.deps) {
263+
if (result.add(dep)) {
264+
nextIds.add(dep);
265+
}
266+
}
267+
}
268+
return result;
269+
}
270+
271+
@override
272+
String toString() => nodes.toString();
273+
}
274+
275+
/// A node in the directive graph.
276+
class _Node {
277+
final AssetId id;
278+
final List<AssetId> deps;
279+
final bool isMissing;
280+
final bool hasTransitiveDigestAsset;
281+
282+
_Node(
283+
{required this.id,
284+
required this.deps,
285+
required this.hasTransitiveDigestAsset})
286+
: isMissing = false;
287+
288+
_Node.missing({required this.id, required this.hasTransitiveDigestAsset})
289+
: isMissing = true,
290+
deps = const [];
291+
292+
@override
293+
String toString() => '$id:'
294+
'${hasTransitiveDigestAsset ? 'digest:' : ''}'
295+
'${isMissing ? 'missing' : deps}';
296+
}
297+
298+
// Transitive digest files are built next to source inputs. As the name
299+
// suggests, they contain the transitive digest of all deps of the file.
300+
// So, establishing a dependency on a transitive digest file is equivalent
301+
// to establishing a dependency on all deps of the file.
302+
const _transitiveDigestExtension = '.transitive_digest';

0 commit comments

Comments
 (0)