Skip to content

Switch to new resolver, remove old resolver. #3962

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 9, 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
10 changes: 10 additions & 0 deletions build/lib/src/library_cycle_graph/phased_reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,14 @@ abstract class PhasedReader {
/// its content. Note that generation might output nothing, in which case an
/// empty string is returned for its content.
Future<PhasedValue<String>> readPhased(AssetId id);

/// Whether [id] is a generated asset that changes between [phase] and
/// [comparedToPhase].
///
/// Returns `true` iff the generated file is hidden to one of the two phases
/// and not hidden to the other. Do not check actual file content or
/// generation state.
///
/// Returns `false` for all other types of asset, including unknown assets.
bool hasChanged(AssetId id, {required int comparedToPhase});
}
1 change: 1 addition & 0 deletions build_resolvers/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- Bump the min SDK to 3.7.0.
- Use `build_test` 3.0.0.
- Use `build_runner_core` 9.0.0.
- Use new resolver always; remove `--use-experimental-resolver` flag.
- Start using `package:build/src/internal.dart`.
- Switch `BuildAssetUriResolver` dependency crawl to an iterative
algorithm, preventing stack overflows.
Expand Down
20 changes: 17 additions & 3 deletions build_resolvers/lib/builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

import 'dart:async';

import 'package:analyzer/dart/analysis/utilities.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:build/build.dart';
import 'package:convert/convert.dart';
import 'package:crypto/crypto.dart';

import 'src/build_asset_uri_resolver.dart';

const transitiveDigestExtension = '.transitive_digest';

Builder transitiveDigestsBuilder(BuilderOptions _) =>
Expand Down Expand Up @@ -64,7 +64,7 @@ https://github.com/dart-lang/build/blob/master/docs/faq.md#unable-to-read-asset-
byteSink.add((await buildStep.digest(next)).bytes);

// We know this isn't null since we already checked if we can read `next`.
final deps = (await dependenciesOf(next, buildStep))!;
final deps = _parseDependencies(await buildStep.readAsString(next), next);

// Add all previously unseen deps to the queue.
for (final dep in deps) {
Expand All @@ -83,3 +83,17 @@ https://github.com/dart-lang/build/blob/master/docs/faq.md#unable-to-read-asset-
'.dart': ['.dart$transitiveDigestExtension'],
};
}

const _ignoredSchemes = ['dart', 'dart-ext'];

Iterable<AssetId> _parseDependencies(String content, AssetId from) =>
parseString(content: content, throwIfDiagnostics: false).unit.directives
.whereType<UriBasedDirective>()
.map((directive) => directive.uri.stringValue)
// Filter out nulls. uri.stringValue can be null for strings that use
// interpolation.
.whereType<String>()
.where(
(uriContent) => !_ignoredSchemes.any(Uri.parse(uriContent).isScheme),
)
.map((content) => AssetId.resolve(Uri.parse(content), from: from));
17 changes: 13 additions & 4 deletions build_resolvers/lib/src/analysis_driver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@ import 'dart:io';
import 'package:analyzer/file_system/file_system.dart' show ResourceProvider;
// ignore: implementation_imports
import 'package:analyzer/src/clients/build_resolvers/build_resolvers.dart';
import 'package:build/build.dart';
import 'package:package_config/package_config.dart' show PackageConfig;
import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';

import 'analysis_driver_filesystem.dart';
import 'analysis_driver_model.dart';
import 'build_asset_uri_resolver.dart';

/// Builds an [AnalysisDriverForPackageBuild] backed by a summary SDK.
///
Expand Down Expand Up @@ -55,10 +54,20 @@ Packages _buildAnalyzerPackages(
// make them match the paths that we give it, so we use the
// `assetPath` function to create those.
rootFolder: resourceProvider.getFolder(
p.url.normalize(assetPath(AssetId(package.name, ''))),
p.url.normalize(
AnalysisDriverFilesystem.assetPathFor(
package: package.name,
path: '',
),
),
),
libFolder: resourceProvider.getFolder(
p.url.normalize(assetPath(AssetId(package.name, 'lib'))),
p.url.normalize(
AnalysisDriverFilesystem.assetPathFor(
package: package.name,
path: 'lib',
),
),
),
),
});
Expand Down
4 changes: 4 additions & 0 deletions build_resolvers/lib/src/analysis_driver_filesystem.dart
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ class AnalysisDriverFilesystem implements UriResolver, ResourceProvider {
static String assetPath(AssetId assetId) =>
'/${assetId.package}/${assetId.path}';

/// Path of the asset with [package] and [path] for the in-memory filesystem.
static String assetPathFor({required String package, required String path}) =>
'/$package/$path';

/// Attempts to parse [uri] into an [AssetId].
///
/// Handles 'package:' or 'asset:' URIs, as well as 'file:' URIs that have the
Expand Down
100 changes: 57 additions & 43 deletions build_resolvers/lib/src/analysis_driver_model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ import 'analysis_driver_filesystem.dart';
/// anywhere; finish it. See `build_asset_uri_resolver.dart` for the current
/// implementation.
class AnalysisDriverModel {
/// The instance used by the shared `AnalyzerResolvers` instance.
static AnalysisDriverModel sharedInstance = AnalysisDriverModel();

/// In-memory filesystem for the analyzer.
final AnalysisDriverFilesystem filesystem = AnalysisDriverFilesystem();

Expand All @@ -37,7 +40,7 @@ class AnalysisDriverModel {

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

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

/// Attempts to parse [uri] into an [AssetId] and returns it if it is cached.
Expand Down Expand Up @@ -86,25 +89,17 @@ class AnalysisDriverModel {
withDriverResource, {
required bool transitive,
}) async {
// Immediately take the lock on `driver` so that the whole class state,
// is only mutated by one build step at a time.
await withDriverResource((driver) async {
// TODO(davidmorgan): it looks like this is only ever called with a single
// entrypoint, consider doing a breaking release to simplify the API.
for (final entrypoint in entrypoints) {
await _performResolve(
driver,
buildStep as BuildStepImpl,
entrypoint,
withDriverResource,
transitive: transitive,
);
}
});
for (final entrypoint in entrypoints) {
await _performResolve(
buildStep as BuildStepImpl,
entrypoint,
withDriverResource,
transitive: transitive,
);
}
}

Future<void> _performResolve(
AnalysisDriverForPackageBuild driver,
BuildStepImpl buildStep,
AssetId entrypoint,
Future<void> Function(
Expand All @@ -113,17 +108,18 @@ class AnalysisDriverModel {
withDriverResource, {
required bool transitive,
}) async {
var idsToSyncOntoFilesystem = [entrypoint];
Iterable<AssetId> idsToSyncOntoFilesystem = [entrypoint];
Iterable<AssetId> inputIds = [entrypoint];

// If requested, find transitive imports.
if (transitive) {
// Note: `transitiveDepsOf` can cause loads that cause builds that cause a
// recursive `_performResolve` on this same `AnalysisDriver` instance.
final nodeLoader = AssetDepsLoader(buildStep.phasedReader);
idsToSyncOntoFilesystem =
(await _graphLoader.transitiveDepsOf(
nodeLoader,
entrypoint,
)).toList();
idsToSyncOntoFilesystem = await _graphLoader.transitiveDepsOf(
nodeLoader,
entrypoint,
);
inputIds = idsToSyncOntoFilesystem;
}

Expand All @@ -133,27 +129,45 @@ class AnalysisDriverModel {
inputs: inputIds,
);

// Sync changes onto the "URI resolver", the in-memory filesystem.
for (final id in idsToSyncOntoFilesystem) {
if (!_syncedOntoFilesystem.add(id)) {
continue;
}
final content =
await buildStep.canRead(id) ? await buildStep.readAsString(id) : null;
if (content == null) {
filesystem.deleteFile(id.asPath);
} else {
filesystem.writeFile(id.asPath, content);
await withDriverResource((driver) async {
// Sync changes onto the "URI resolver", the in-memory filesystem.
final phase = buildStep.phasedReader.phase;
for (final id in idsToSyncOntoFilesystem) {
final wasSyncedAt = _syncedOntoFilesystemAtPhase[id];
if (wasSyncedAt != null) {
// Skip if already synced at this phase.
if (wasSyncedAt == phase) {
continue;
}
// Skip if synced at an equivalent other phase.
if (!buildStep.phasedReader.hasChanged(
id,
comparedToPhase: wasSyncedAt,
)) {
continue;
}
}

_syncedOntoFilesystemAtPhase[id] = phase;
final content =
await buildStep.canRead(id)
? await buildStep.readAsString(id)
: null;
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 path in filesystem.changedPaths) {
driver.changeFile(path);
}
filesystem.clearChangedPaths();
await driver.applyPendingFileChanges();
// Notify the analyzer of changes and wait for it to update its internal
// state.
for (final path in filesystem.changedPaths) {
driver.changeFile(path);
}
filesystem.clearChangedPaths();
await driver.applyPendingFileChanges();
});
}
}

Expand Down
Loading
Loading