Skip to content
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

Simpler in memory filesystem #3823

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion build_resolvers/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
## 2.4.4-wip

- Refactor `BuildAssetUriResolver` into `AnalysisDriverModel` and
`AnalysisDriverModelUriResolver`. Add new implementation of
`AnalysisDriverFilesystem`. Add new implementation of
`AnalysisDriverModel`.

## 2.4.3
Expand Down
7 changes: 3 additions & 4 deletions build_resolvers/lib/src/analysis_driver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';

import 'analysis_driver_model.dart';
import 'analysis_driver_model_uri_resolver.dart';
import 'build_asset_uri_resolver.dart';

/// Builds an [AnalysisDriverForPackageBuild] backed by a summary SDK.
Expand All @@ -29,12 +28,12 @@ Future<AnalysisDriverForPackageBuild> analysisDriver(
analysisOptions: analysisOptions,
packages: _buildAnalyzerPackages(
packageConfig,
analysisDriverModel.resourceProvider,
analysisDriverModel.filesystem,
),
resourceProvider: analysisDriverModel.resourceProvider,
resourceProvider: analysisDriverModel.filesystem,
sdkSummaryBytes: File(sdkSummaryPath).readAsBytesSync(),
uriResolvers: [
AnalysisDriverModelUriResolver(analysisDriverModel),
analysisDriverModel.filesystem,
],
);
}
Expand Down
175 changes: 175 additions & 0 deletions build_resolvers/lib/src/analysis_driver_filesystem.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:convert';
import 'dart:typed_data';

import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/source/file_source.dart';
// ignore: implementation_imports
import 'package:analyzer/src/clients/build_resolvers/build_resolvers.dart';
import 'package:build/build.dart' hide Resource;
import 'package:path/path.dart' as p;

/// The in-memory filesystem that is the analyzer's view of the build.
///
/// Tracks modified paths, which should be passed to
/// `AnalysisDriver.changeFile` to update the analyzer state.
class AnalysisDriverFilesystem implements UriResolver, ResourceProvider {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ouch. I did not expect that clients will start implementing ResourceProvider.
This will potentially complicate making any change to this interface.

final Map<String, String> _data = {};
final Set<String> _changedPaths = {};
davidmorgan marked this conversation as resolved.
Show resolved Hide resolved

// Methods for use by `AnalysisDriverModel`.

/// Whether [path] exists.
bool exists(String path) => _data.containsKey(path);

/// Reads the data previously written to [path].
///
/// Throws if ![exists].
String read(String path) => _data[path]!;

/// Deletes the data previously written to [path].
///
/// Records the change in [changedPaths].
///
/// Or, if it's missing, does nothing.
void deleteFile(String path) {
if (_data.remove(path) != null) _changedPaths.add(path);
}

/// Writes [content] to [path].
///
/// Records the change in [changedPaths], only if the content actually
/// changed.
bool writeFile(String path, String content) {
final oldContent = _data[path];
_data[path] = content;
if (content != oldContent) _changedPaths.add(path);
davidmorgan marked this conversation as resolved.
Show resolved Hide resolved
return oldContent != content;
}

/// Paths that were modified by [deleteFile] or [writeFile] since the last
/// call to [clearChangedPaths].
Iterable<String> get changedPaths => _changedPaths;

/// Clears [changedPaths].
void clearChangedPaths() => _changedPaths.clear();

// `UriResolver` methods.

@override
Uri pathToUri(String path) {
davidmorgan marked this conversation as resolved.
Show resolved Hide resolved
var pathSegments = p.posix.split(path);
var packageName = pathSegments[1];
if (pathSegments[2] == 'lib') {
return Uri(
scheme: 'package',
pathSegments: [packageName].followedBy(pathSegments.skip(3)),
);
} else {
return Uri(
scheme: 'asset',
pathSegments: [packageName].followedBy(pathSegments.skip(2)),
);
}
}

@override
Source? resolveAbsolute(Uri uri, [Uri? actualUri]) {
final assetId = parseAsset(uri);
if (assetId == null) return null;

var file = getFile(assetPath(assetId));
return FileSource(file, assetId.uri);
}

/// Path of [assetId] for the in-memory filesystem.
static String assetPath(AssetId assetId) =>
p.posix.join('/${assetId.package}', assetId.path);
davidmorgan marked this conversation as resolved.
Show resolved Hide resolved

/// Attempts to parse [uri] into an [AssetId].
///
/// Handles 'package:' or 'asset:' URIs, as well as 'file:' URIs that have the
/// same pattern used by [assetPath].
///
/// Returns null if the Uri cannot be parsed.
static AssetId? parseAsset(Uri uri) {
if (const ['dart', 'dart-ext'].any(uri.isScheme)) return null;
Copy link

Choose a reason for hiding this comment

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

Can't we write this similar to the line below instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, actually this line is not needed at all, it will anyway fall through and return null. (This was copied existing code but with the consts inlined).

Removed. Thanks.

if (uri.isScheme('package') || uri.isScheme('asset')) {
return AssetId.resolve(uri);
}
if (uri.isScheme('file')) {
final parts = p.split(uri.path);
return AssetId(parts[1], p.posix.joinAll(parts.skip(2)));
}
return null;
}

// `ResourceProvider` methods.

@override
p.Context get pathContext => p.posix;
davidmorgan marked this conversation as resolved.
Show resolved Hide resolved

@override
File getFile(String path) => _Resource(this, path);

@override
Folder getFolder(String path) => _Resource(this, path);

// `ResourceProvider` methods that are not needed.

@override
Link getLink(String path) => throw UnimplementedError();

@override
Resource getResource(String path) => throw UnimplementedError();

@override
Folder? getStateLocation(String pluginId) => throw UnimplementedError();
}

/// Minimal implementation of [File] and [Folder].
///
/// Provides only what the analyzer actually uses during analysis.
class _Resource implements File, Folder {
final AnalysisDriverFilesystem filesystem;
@override
final String path;

_Resource(this.filesystem, this.path);

@override
bool get exists => filesystem.exists(path);

@override
String get shortName => filesystem.pathContext.basename(path);

@override
Uint8List readAsBytesSync() {
// TODO(davidmorgan): the analyzer reads as bytes in `FileContentCache`
// then converts back to `String` and hashes. It should be possible to save
// that work, for example by injecting a custom `FileContentCache`.
return utf8.encode(filesystem.read(path));
}

@override
String readAsStringSync() => filesystem.read(path);

// `Folder` methods.

@override
bool contains(String path) =>
filesystem.pathContext.isWithin(this.path, path);

// Most `File` and/or `Folder` methods are not needed.

@override
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);

// Needs an explicit override to satisfy both `File.copyTo` and
// `Folder.copyTo`.
@override
_Resource copyTo(Folder _) => throw UnimplementedError();
}
61 changes: 21 additions & 40 deletions build_resolvers/lib/src/analysis_driver_model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@ import 'dart:collection';

import 'package:analyzer/dart/analysis/utilities.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/file_system/memory_file_system.dart';
// ignore: implementation_imports
import 'package:analyzer/src/clients/build_resolvers/build_resolvers.dart';
import 'package:build/build.dart';
import 'package:path/path.dart' as p;

import 'analysis_driver_model_uri_resolver.dart';
import 'analysis_driver_filesystem.dart';

/// Manages analysis driver and related build state.
///
Expand All @@ -31,15 +29,14 @@ import 'analysis_driver_model_uri_resolver.dart';
/// implementation.
class AnalysisDriverModel {
/// In-memory filesystem for the analyzer.
final MemoryResourceProvider resourceProvider =
MemoryResourceProvider(context: p.posix);
final AnalysisDriverFilesystem filesystem = AnalysisDriverFilesystem();

/// The import graph of all sources needed for analysis.
final _graph = _Graph();

/// Assets that have been synced into the in-memory filesystem
/// [resourceProvider].
final _syncedOntoResourceProvider = <AssetId>{};
/// [filesystem].
final _syncedOntoFilesystem = <AssetId>{};

/// Notifies that [step] has completed.
///
Expand All @@ -51,7 +48,7 @@ class AnalysisDriverModel {
/// Clear cached information specific to an individual build.
void reset() {
_graph.clear();
_syncedOntoResourceProvider.clear();
_syncedOntoFilesystem.clear();
}

/// Attempts to parse [uri] into an [AssetId] and returns it if it is cached.
Expand All @@ -61,16 +58,16 @@ class AnalysisDriverModel {
///
/// Returns null if the `Uri` cannot be parsed or is not cached.
AssetId? lookupCachedAsset(Uri uri) {
final assetId = AnalysisDriverModelUriResolver.parseAsset(uri);
final assetId = AnalysisDriverFilesystem.parseAsset(uri);
// TODO(davidmorgan): not clear if this is the right "exists" check.
if (assetId == null || !resourceProvider.getFile(assetId.asPath).exists) {
if (assetId == null || !filesystem.getFile(assetId.asPath).exists) {
return null;
}

return assetId;
}

/// Updates [resourceProvider] and the analysis driver given by
/// Updates [filesystem] and the analysis driver given by
/// `withDriverResource` with updated versions of [entryPoints].
///
/// If [transitive], then all the transitive imports from [entryPoints] are
Expand Down Expand Up @@ -107,14 +104,14 @@ class AnalysisDriverModel {
FutureOr<void> Function(AnalysisDriverForPackageBuild))
withDriverResource,
{required bool transitive}) async {
var idsToSyncOntoResourceProvider = entryPoints;
var idsToSyncOntoFilesystem = entryPoints;
Iterable<AssetId> inputIds = entryPoints;

// If requested, find transitive imports.
if (transitive) {
final previouslyMissingFiles = await _graph.load(buildStep, entryPoints);
_syncedOntoResourceProvider.removeAll(previouslyMissingFiles);
idsToSyncOntoResourceProvider = _graph.nodes.keys.toList();
_syncedOntoFilesystem.removeAll(previouslyMissingFiles);
idsToSyncOntoFilesystem = _graph.nodes.keys.toList();
inputIds = _graph.inputsFor(entryPoints);
}

Expand All @@ -124,39 +121,23 @@ class AnalysisDriverModel {
}

// Sync changes onto the "URI resolver", the in-memory filesystem.
final changedIds = <AssetId>[];
for (final id in idsToSyncOntoResourceProvider) {
if (!_syncedOntoResourceProvider.add(id)) continue;
for (final id in idsToSyncOntoFilesystem) {
if (!_syncedOntoFilesystem.add(id)) continue;
final content =
await buildStep.canRead(id) ? await buildStep.readAsString(id) : null;
final inMemoryFile = resourceProvider.getFile(id.asPath);
final inMemoryContent =
inMemoryFile.exists ? inMemoryFile.readAsStringSync() : null;

if (content != inMemoryContent) {
if (content == null) {
// TODO(davidmorgan): per "globallySeenAssets" in
// BuildAssetUriResolver, deletes should only be applied at the end
// of the build, in case the file is actually there but not visible
// to the current reader.
resourceProvider.deleteFile(id.asPath);
changedIds.add(id);
} else {
if (inMemoryContent == null) {
resourceProvider.newFile(id.asPath, content);
} else {
resourceProvider.modifyFile(id.asPath, content);
}
changedIds.add(id);
}
if (content == null) {
filesystem.deleteFile(id.asPath);
} else {
filesystem.writeFile(id.asPath, content);
}
}

// Notify the analyzer of changes and wait for it to update its internal
// state.
for (final id in changedIds) {
driver.changeFile(id.asPath);
for (final path in filesystem.changedPaths) {
driver.changeFile(path);
}
filesystem.clearChangedPaths();
await driver.applyPendingFileChanges();
}
}
Expand All @@ -181,7 +162,7 @@ List<AssetId> _parseDependencies(String content, AssetId from) =>

extension _AssetIdExtensions on AssetId {
/// Asset path for the in-memory filesystem.
String get asPath => AnalysisDriverModelUriResolver.assetPath(this);
String get asPath => AnalysisDriverFilesystem.assetPath(this);
}

/// The directive graph of all known sources.
Expand Down
Loading
Loading