Skip to content

Commit

Permalink
Stop using transitive digest files for resolvers. (#3897)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidmorgan authored Mar 3, 2025
1 parent b2f939c commit a49bbef
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 173 deletions.
25 changes: 6 additions & 19 deletions _test/test/goldens/generated_build_script.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ import 'package:build_test/builder.dart' as _i4;
import 'package:build_config/build_config.dart' as _i5;
import 'package:build_modules/builders.dart' as _i6;
import 'package:build/build.dart' as _i7;
import 'package:build_resolvers/builder.dart' as _i8;
import 'dart:isolate' as _i9;
import 'package:build_runner/build_runner.dart' as _i10;
import 'dart:io' as _i11;
import 'dart:isolate' as _i8;
import 'package:build_runner/build_runner.dart' as _i9;
import 'dart:io' as _i10;

final _builders = <_i1.BuilderApplication>[
_i1.apply(
Expand Down Expand Up @@ -145,18 +144,6 @@ final _builders = <_i1.BuilderApplication>[
const _i7.BuilderOptions(<String, dynamic>{r'compiler': r'dart2js'}),
appliesBuilders: const [r'build_web_compilers:dart2js_archive_extractor'],
),
_i1.apply(
r'build_resolvers:transitive_digests',
[_i8.transitiveDigestsBuilder],
_i1.toAllPackages(),
isOptional: true,
hideOutput: true,
appliesBuilders: const [r'build_resolvers:transitive_digest_cleanup'],
),
_i1.applyPostProcess(
r'build_resolvers:transitive_digest_cleanup',
_i8.transitiveDigestCleanup,
),
_i1.applyPostProcess(
r'build_modules:module_cleanup',
_i6.moduleCleanup,
Expand All @@ -180,12 +167,12 @@ final _builders = <_i1.BuilderApplication>[
];
void main(
List<String> args, [
_i9.SendPort? sendPort,
_i8.SendPort? sendPort,
]) async {
var result = await _i10.run(
var result = await _i9.run(
args,
_builders,
);
sendPort?.send(result);
_i11.exitCode = result;
_i10.exitCode = result;
}
1 change: 1 addition & 0 deletions build_resolvers/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Switch `BuildAssetUriResolver` dependency crawl to an iterative
algorithm, preventing stack overflows.
- Move `BuildStepImpl` to `build_runner_core`, use `SingleStepReader` directly.
- Stop building `transitive_digest` files by default.

## 2.4.4

Expand Down
18 changes: 0 additions & 18 deletions build_resolvers/build.yaml

This file was deleted.

5 changes: 5 additions & 0 deletions build_resolvers/lib/builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import 'package:crypto/crypto.dart';

import 'src/build_asset_uri_resolver.dart';

const transitiveDigestExtension = '.transitive_digest';

Builder transitiveDigestsBuilder(BuilderOptions _) =>
_TransitiveDigestsBuilder();

Expand All @@ -24,6 +26,9 @@ PostProcessBuilder transitiveDigestCleanup(BuilderOptions options) =>
/// For any dependency that has a transitive digest already written, we just use
/// that and don't crawl its transitive deps, as the transitive digest includes
/// all the information we need.
///
/// TODO(davidmorgan): this is not used by `build_resolvers` any more, consider
/// deprecating and removing.
class _TransitiveDigestsBuilder extends Builder {
@override
Future<void> build(BuildStep buildStep) async {
Expand Down
42 changes: 4 additions & 38 deletions build_resolvers/lib/src/analysis_driver_model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ class AnalysisDriverModel {
/// Notifies [buildStep] of all inputs that result from analysis. If
/// [transitive], this includes all transitive dependencies.
///
/// If while finding transitive deps a `.transitive_deps` file is
/// encountered next to a source file then this cuts off the reporting
/// of deps to the [buildStep], but does not affect the reporting of
/// files to the analysis driver.
Future<void> performResolve(
BuildStep buildStep,
List<AssetId> entryPoints,
Expand Down Expand Up @@ -176,9 +172,6 @@ extension _AssetIdExtensions on AssetId {
}

/// The directive graph of all known sources.
///
/// Also tracks whether there is a `.transitive_digest` file next to each source
/// asset, and tracks missing files.
class _Graph {
final Map<AssetId, _Node> nodes = {};

Expand Down Expand Up @@ -208,18 +201,11 @@ class _Graph {
previouslyMissingFiles.add(id);
}
// Load the node.
final hasTransitiveDigestAsset = await reader.canRead(
id.addExtension(_transitiveDigestExtension),
);
final content = await reader.readAsString(id);
final deps = _parseDependencies(content, id);
node = _Node(
id: id,
deps: deps,
hasTransitiveDigestAsset: hasTransitiveDigestAsset,
);
node = _Node(id: id, deps: deps);
} else {
node ??= _Node.missing(id: id, hasTransitiveDigestAsset: false);
node ??= _Node.missing(id: id);
}
nodes[id] = node;
}
Expand Down Expand Up @@ -247,12 +233,6 @@ class _Graph {
final nextId = nextIds.removeFirst();
final node = nodes[nextId]!;

// Add the transitive digest file as an input. If it exists, skip deps.
result.add(nextId.addExtension(_transitiveDigestExtension));
if (node.hasTransitiveDigestAsset) {
continue;
}

// Skip if there are no deps because the file is missing.
if (node.isMissing) continue;

Expand All @@ -276,27 +256,13 @@ class _Node {
final AssetId id;
final List<AssetId> deps;
final bool isMissing;
final bool hasTransitiveDigestAsset;

_Node({
required this.id,
required this.deps,
required this.hasTransitiveDigestAsset,
}) : isMissing = false;
_Node({required this.id, required this.deps}) : isMissing = false;

_Node.missing({required this.id, required this.hasTransitiveDigestAsset})
: isMissing = true,
deps = const [];
_Node.missing({required this.id}) : isMissing = true, deps = const [];

@override
String toString() =>
'$id:'
'${hasTransitiveDigestAsset ? 'digest:' : ''}'
'${isMissing ? 'missing' : deps}';
}

// Transitive digest files are built next to source inputs. As the name
// suggests, they contain the transitive digest of all deps of the file.
// So, establishing a dependency on a transitive digest file is equivalent
// to establishing a dependency on all deps of the file.
const _transitiveDigestExtension = '.transitive_digest';
14 changes: 1 addition & 13 deletions build_resolvers/lib/src/build_asset_uri_resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import 'package:analyzer/dart/ast/ast.dart';
// ignore: implementation_imports
import 'package:analyzer/src/clients/build_resolvers/build_resolvers.dart';
import 'package:build/build.dart' show AssetId, BuildStep;
import 'package:collection/collection.dart';
import 'package:crypto/crypto.dart';
import 'package:path/path.dart' as p;

Expand All @@ -21,8 +20,6 @@ import 'crawl_async.dart';

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

const transitiveDigestExtension = '.transitive_digest';

/// Switches [BuildAssetUriResolver.sharedInstance] from [BuildAssetUriResolver]
/// to [AnalysisDriverModel].
void useExperimentalResolver() {
Expand Down Expand Up @@ -99,16 +96,7 @@ class BuildAssetUriResolver implements AnalysisDriverModel {
),
(id, state) async {
if (state == null) return const [];
// Establishes a dependency on the transitive deps digest.
final hasTransitiveDigestAsset = await buildStep.canRead(
id.addExtension(transitiveDigestExtension),
);
return hasTransitiveDigestAsset
// Only crawl assets that we haven't yet loaded into the
// analyzer if we are using transitive digests for invalidation.
? state.dependencies.whereNot(_cachedAssetDigests.containsKey)
// Otherwise fall back on crawling all source deps.
: state.dependencies.where(notCrawled);
return state.dependencies.where(notCrawled);
},
).drain<void>();
} else {
Expand Down
85 changes: 0 additions & 85 deletions build_resolvers/test/resolver_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ void runTests(ResolversFactory resolversFactory) {
final entryPoint = AssetId('a', 'web/main.dart');
Resolvers createResolvers({PackageConfig? packageConfig}) =>
resolversFactory.create(packageConfig: packageConfig);
final isSharedResolvers = resolversFactory.isInstanceShared;

test('should handle initial files', () {
return resolveSources({'a|web/main.dart': ' main() {}'}, (resolver) async {
var lib = await resolver.libraryFor(entryPoint);
Expand Down Expand Up @@ -146,84 +144,14 @@ void runTests(ResolversFactory resolversFactory) {
assetReaderChecks: (reader) {
expect(reader.testing.inputsTracked, {
AssetId('a', 'web/main.dart'),
AssetId('a', 'web/main.dart.transitive_digest'),
AssetId('a', 'web/a.dart'),
AssetId('a', 'web/a.dart.transitive_digest'),
AssetId('a', 'web/b.dart'),
AssetId('a', 'web/b.dart.transitive_digest'),
});
},
resolvers: createResolvers(),
);
});

test(
'transitive_digest file cuts off deps reported to buildStep',
// This test requires a new resolvers instance.
skip: isSharedResolvers,
() async {
// BuildAssetUriResolver cuts off a buildStep's deps tree if it encounters
// a `.transitive_digest` file, because that file stands in for the
// transitive deps from that point.
//
// But, the _first_ time it is reading files it continues to read them,
// because the analyzer still needs all the files. So the first build
// action will see all the files as deps.
final resolvers = createResolvers();
final sources = {
'a|web/main.dart': '''
import 'a.dart';
main() {
} ''',
'a|web/a.dart': '''
library a;
import 'b.dart';
''',
'a|web/a.dart.transitive_digest': '''
library a;
import 'b.dart';
''',
'a|web/b.dart': '''
library b;
''',
};
// First action sees all the files as inputs.
await resolveSources(
sources,
(resolver) async {
await resolver.libraryFor(entryPoint);
},
assetReaderChecks: (reader) {
expect(reader.testing.inputsTracked, {
AssetId('a', 'web/main.dart'),
AssetId('a', 'web/main.dart.transitive_digest'),
AssetId('a', 'web/a.dart'),
AssetId('a', 'web/a.dart.transitive_digest'),
AssetId('a', 'web/b.dart'),
AssetId('a', 'web/b.dart.transitive_digest'),
});
},
resolvers: resolvers,
);
// Second action has inputs cut off at the `transitive_digest` file.
await resolveSources(
sources,
(resolver) async {
await resolver.libraryFor(entryPoint);
},
assetReaderChecks: (reader) {
expect(reader.testing.inputsTracked, {
AssetId('a', 'web/main.dart'),
AssetId('a', 'web/main.dart.transitive_digest'),
AssetId('a', 'web/a.dart'),
AssetId('a', 'web/a.dart.transitive_digest'),
});
},
resolvers: resolvers,
);
},
);

test('updates following a change to source', () async {
var resolvers = createResolvers();
await resolveSources(
Expand Down Expand Up @@ -1398,14 +1326,10 @@ final _skipOnPreRelease =

abstract class ResolversFactory {
/// Whether [create] returns a shared instance that persists between tests.
bool get isInstanceShared;
Resolvers create({PackageConfig? packageConfig});
}

class BuildAssetUriResolversFactory implements ResolversFactory {
@override
bool get isInstanceShared => false;

@override
Resolvers create({PackageConfig? packageConfig}) => AnalyzerResolvers.custom(
packageConfig: packageConfig,
Expand All @@ -1417,9 +1341,6 @@ class BuildAssetUriResolversFactory implements ResolversFactory {
}

class SharedBuildAssetUriResolversFactory implements ResolversFactory {
@override
bool get isInstanceShared => true;

@override
Resolvers create({PackageConfig? packageConfig}) => AnalyzerResolvers.custom(
packageConfig: packageConfig,
Expand All @@ -1431,9 +1352,6 @@ class SharedBuildAssetUriResolversFactory implements ResolversFactory {
}

class AnalysisDriverModelFactory implements ResolversFactory {
@override
bool get isInstanceShared => false;

@override
Resolvers create({PackageConfig? packageConfig}) => AnalyzerResolvers.custom(
packageConfig: packageConfig,
Expand All @@ -1447,9 +1365,6 @@ class AnalysisDriverModelFactory implements ResolversFactory {
class SharedAnalysisDriverModelFactory implements ResolversFactory {
static final AnalysisDriverModel sharedInstance = AnalysisDriverModel();

@override
bool get isInstanceShared => true;

@override
Resolvers create({PackageConfig? packageConfig}) {
final result = AnalyzerResolvers.custom(
Expand Down

0 comments on commit a49bbef

Please sign in to comment.