Skip to content

Commit

Permalink
Move input tracking out of AssetReader to SingleStepReader.
Browse files Browse the repository at this point in the history
`BuildStepImpl` always uses a `SingleStepReader`.
  • Loading branch information
davidmorgan committed Feb 26, 2025
1 parent c663785 commit 9e96d43
Show file tree
Hide file tree
Showing 21 changed files with 181 additions and 117 deletions.
7 changes: 3 additions & 4 deletions _test_common/lib/in_memory_reader_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,36 +24,35 @@ class InMemoryRunnerAssetReaderWriter extends InMemoryAssetReaderWriter
factory InMemoryRunnerAssetReaderWriter({String? rootPackage}) {
final filesystem = InMemoryFilesystem();
return InMemoryRunnerAssetReaderWriter.using(
assetsRead: {},
rootPackage: rootPackage ?? 'unset',
assetFinder: InMemoryAssetFinder(filesystem, rootPackage),
assetPathProvider: const InMemoryAssetPathProvider(),
filesystem: filesystem,
cache: const PassthroughFilesystemCache(),
inputTracker: InputTracker(),
);
}

InMemoryRunnerAssetReaderWriter.using({
required super.assetsRead,
required super.rootPackage,
required super.assetFinder,
required super.assetPathProvider,
required super.filesystem,
required super.cache,
required super.inputTracker,
}) : super.using();

@override
InMemoryRunnerAssetReaderWriter copyWith({
AssetPathProvider? assetPathProvider,
FilesystemCache? cache,
InputTracker? inputTracker,
}) => InMemoryRunnerAssetReaderWriter.using(
assetsRead: assetsRead,
rootPackage: rootPackage,
assetFinder: assetFinder,
assetPathProvider: assetPathProvider ?? this.assetPathProvider,
filesystem: filesystem,
cache: cache ?? this.cache,
inputTracker: inputTracker ?? this.inputTracker,
);

@override
Expand Down
2 changes: 0 additions & 2 deletions build/lib/src/builder/build_step.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import 'dart:async';
import 'dart:convert';

import 'package:analyzer/dart/element/element.dart';
import 'package:meta/meta.dart';
import 'package:package_config/package_config_types.dart';

import '../analyzer/resolver.dart';
Expand All @@ -19,7 +18,6 @@ import '../resource/resource.dart';
/// This represents a single [inputId], logic around resolving as a library,
/// and the ability to read and write assets as allowed by the underlying build
/// system.
@sealed
abstract class BuildStep implements AssetReader, AssetWriter {
/// The primary for this build step.
AssetId get inputId;
Expand Down
3 changes: 2 additions & 1 deletion build/lib/src/generate/run_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
import 'dart:isolate';

// ignore: implementation_imports
import 'package:build_runner_core/src/generate/build_step_impl.dart';
import 'package:logging/logging.dart';
import 'package:package_config/package_config.dart';

Expand All @@ -11,7 +13,6 @@ import '../asset/id.dart';
import '../asset/reader.dart';
import '../asset/writer.dart';
import '../builder/build_step.dart';
import '../builder/build_step_impl.dart';
import '../builder/builder.dart';
import '../builder/logging.dart';
import '../resource/resource.dart';
Expand Down
1 change: 0 additions & 1 deletion build/lib/src/internal.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,4 @@ export 'state/asset_finder.dart';
export 'state/asset_path_provider.dart';
export 'state/filesystem.dart';
export 'state/filesystem_cache.dart';
export 'state/input_tracker.dart';
export 'state/reader_state.dart';
12 changes: 0 additions & 12 deletions build/lib/src/state/input_tracker.dart

This file was deleted.

20 changes: 0 additions & 20 deletions build/lib/src/state/reader_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import 'asset_finder.dart';
import 'asset_path_provider.dart';
import 'filesystem.dart';
import 'filesystem_cache.dart';
import 'input_tracker.dart';

/// Provides access to the state backing an [AssetReader].
extension AssetReaderStateExtension on AssetReader {
Expand Down Expand Up @@ -38,26 +37,11 @@ extension AssetReaderStateExtension on AssetReader {
return (this as AssetReaderState).assetFinder;
}

InputTracker? get inputTracker =>
this is AssetReaderState ? (this as AssetReaderState).inputTracker : null;

AssetPathProvider get assetPathProvider {
_requireIsAssetReaderState();
return (this as AssetReaderState).assetPathProvider;
}

/// Gets [inputTracker] or throws a descriptive error if it is `null`.
InputTracker get requireInputTracker {
final result = inputTracker;
if (result == null) {
_requireIsAssetReaderState();
throw StateError(
'`AssetReader` is missing required `inputTracker`: $this',
);
}
return result;
}

/// Throws if `this` is not an [AssetReaderState].
void _requireIsAssetReaderState() {
if (this is! AssetReaderState) {
Expand Down Expand Up @@ -91,10 +75,6 @@ abstract interface class AssetReaderState {
/// globbing in arbitrary packages, is hidden from generators.
AssetFinder get assetFinder;

/// The [InputTracker] that this reader records reads to; or `null` if it does
/// not have one.
InputTracker? get inputTracker;

/// The [AssetPathProvider] associated with this reader.
AssetPathProvider get assetPathProvider;
}
1 change: 1 addition & 0 deletions build/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ environment:
dependencies:
analyzer: '>=6.9.0 <8.0.0'
async: ^2.5.0
build_runner_core: ^8.0.1-wip
convert: ^3.0.0
crypto: ^3.0.0
glob: ^2.0.0
Expand Down
2 changes: 1 addition & 1 deletion build/test/builder/build_step_impl_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import 'dart:convert';

import 'package:build/build.dart';
import 'package:build/src/builder/build_step.dart';
import 'package:build/src/builder/build_step_impl.dart';
import 'package:build_resolvers/build_resolvers.dart';
import 'package:build_runner_core/src/generate/build_step_impl.dart';
import 'package:build_test/build_test.dart';
import 'package:package_config/package_config.dart';
import 'package:test/test.dart';
Expand Down
11 changes: 7 additions & 4 deletions build_resolvers/lib/src/analysis_driver_model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/src/clients/build_resolvers/build_resolvers.dart';
import 'package:build/build.dart';
// ignore: implementation_imports
import 'package:build/src/internal.dart';
import 'package:build_runner_core/src/generate/build_step_impl.dart';

import 'analysis_driver_filesystem.dart';

Expand Down Expand Up @@ -97,7 +97,7 @@ class AnalysisDriverModel {
await withDriverResource((driver) async {
return _performResolve(
driver,
buildStep,
buildStep as BuildStepImpl,
entryPoints,
withDriverResource,
transitive: transitive,
Expand All @@ -107,7 +107,7 @@ class AnalysisDriverModel {

Future<void> _performResolve(
AnalysisDriverForPackageBuild driver,
BuildStep buildStep,
BuildStepImpl buildStep,
List<AssetId> entryPoints,
Future<void> Function(
FutureOr<void> Function(AnalysisDriverForPackageBuild),
Expand All @@ -127,7 +127,10 @@ class AnalysisDriverModel {
}

// Notify [buildStep] of its inputs.
buildStep.requireInputTracker.assetsRead.addAll(inputIds);
buildStep.inputTracker.addAll(
primaryInput: buildStep.inputId,
inputs: inputIds,
);

// Sync changes onto the "URI resolver", the in-memory filesystem.
for (final id in idsToSyncOntoFilesystem) {
Expand Down
1 change: 1 addition & 0 deletions build_resolvers/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ dependencies:
analyzer: '>=6.9.0 <8.0.0'
async: ^2.5.0
build: ^2.4.3-wip
build_runner_core: ^8.0.1-wip
collection: ^1.17.0
convert: ^3.1.1
crypto: ^3.0.0
Expand Down
6 changes: 3 additions & 3 deletions build_resolvers/test/resolver_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ void runTests(ResolversFactory resolversFactory) {
await resolver.libraryFor(entryPoint);
},
assetReaderChecks: (reader) {
expect(reader.testing.assetsRead, {
expect(reader.testing.inputsTracked, {
AssetId('a', 'web/main.dart'),
AssetId('a', 'web/main.dart.transitive_digest'),
AssetId('a', 'web/a.dart'),
Expand Down Expand Up @@ -194,7 +194,7 @@ void runTests(ResolversFactory resolversFactory) {
await resolver.libraryFor(entryPoint);
},
assetReaderChecks: (reader) {
expect(reader.testing.assetsRead, {
expect(reader.testing.inputsTracked, {
AssetId('a', 'web/main.dart'),
AssetId('a', 'web/main.dart.transitive_digest'),
AssetId('a', 'web/a.dart'),
Expand All @@ -212,7 +212,7 @@ void runTests(ResolversFactory resolversFactory) {
await resolver.libraryFor(entryPoint);
},
assetReaderChecks: (reader) {
expect(reader.testing.assetsRead, {
expect(reader.testing.inputsTracked, {
AssetId('a', 'web/main.dart'),
AssetId('a', 'web/main.dart.transitive_digest'),
AssetId('a', 'web/a.dart'),
Expand Down
6 changes: 0 additions & 6 deletions build_runner_core/lib/src/asset/reader_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ class ReaderWriter extends AssetReader
final AssetFinder assetFinder;
@override
final FilesystemCache cache;
@override
final InputTracker? inputTracker;

/// A [ReaderWriter] suitable for real builds.
///
Expand All @@ -46,8 +44,6 @@ class ReaderWriter extends AssetReader
assetPathProvider: packageGraph,
filesystem: IoFilesystem(),
cache: const PassthroughFilesystemCache(),
// In real builds input tracking is done per generator.
inputTracker: null,
);

ReaderWriter.using({
Expand All @@ -56,7 +52,6 @@ class ReaderWriter extends AssetReader
required this.assetPathProvider,
required this.filesystem,
required this.cache,
required this.inputTracker,
});

@override
Expand All @@ -69,7 +64,6 @@ class ReaderWriter extends AssetReader
assetPathProvider: assetPathProvider ?? this.assetPathProvider,
filesystem: filesystem,
cache: cache ?? this.cache,
inputTracker: inputTracker,
);

@override
Expand Down
24 changes: 15 additions & 9 deletions build_runner_core/lib/src/generate/build_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import 'package:pool/pool.dart';
import 'package:watcher/watcher.dart';

import '../asset/finalized_reader.dart';
import '../asset/reader.dart';
import '../asset/writer.dart';
import '../asset_graph/graph.dart';
import '../asset_graph/node.dart';
Expand All @@ -40,9 +39,11 @@ import 'build_directory.dart';
import 'build_result.dart';
import 'finalized_assets_view.dart';
import 'heartbeat.dart';
import 'input_tracker.dart';
import 'options.dart';
import 'performance_tracker.dart';
import 'phase.dart';
import 'single_step_reader.dart';

final _logger = Logger('Build');

Expand Down Expand Up @@ -561,12 +562,14 @@ class _SingleBuild {
var wrappedWriter = AssetWriterSpy(_writer);

var wrappedReader = SingleStepReader(
input,
_reader,
_assetGraph,
phaseNumber,
input.package,
_isReadableNode,
_checkInvalidInput,
InputTracker(_reader.filesystem),
_getUpdatedGlobNode,
wrappedWriter,
);
Expand All @@ -581,9 +584,8 @@ class _SingleBuild {
await _cleanUpStaleOutputs(builderOutputs);
await FailureReporter.clean(phaseNumber, input);

// We may have read some inputs in the call to `_buildShouldRun`, we
// want to remove those.
wrappedReader.inputTracker.assetsRead.clear();
// Clear input tracking accumulated during `_buildShouldRun`.
wrappedReader.inputTracker.clear();

var actionDescription = _actionLoggerName(
phase,
Expand Down Expand Up @@ -625,6 +627,7 @@ class _SingleBuild {
await tracker.trackStage(
'Finalize',
() => _setOutputsState(
input,
builderOutputs,
wrappedReader,
wrappedReader.inputTracker,
Expand Down Expand Up @@ -701,22 +704,23 @@ class _SingleBuild {
var inputNode = _assetGraph.get(input)!;
var wrappedWriter = AssetWriterSpy(_writer);
var wrappedReader = SingleStepReader(
input,
_reader,
_assetGraph,
phaseNum,
input.package,
_isReadableNode,
_checkInvalidInput,
InputTracker(_reader.filesystem),
null,
wrappedWriter,
);

if (!await _postProcessBuildShouldRun(anchorNode, wrappedReader)) {
return <AssetId>[];
}
// We may have read some inputs in the call to `_buildShouldRun`, we want
// to remove those.
wrappedReader.inputTracker.assetsRead.clear();
// Clear input tracking accumulated during `_buildShouldRun`.
wrappedReader.inputTracker.clear();

// Clean out the impacts of the previous run
await FailureReporter.clean(phaseNum, input);
Expand Down Expand Up @@ -782,6 +786,7 @@ class _SingleBuild {
// written.
inputNode.primaryOutputs.addAll(assetsWritten);
await _setOutputsState(
input,
assetsWritten,
wrappedReader,
wrappedReader.inputTracker,
Expand Down Expand Up @@ -999,6 +1004,7 @@ class _SingleBuild {
/// - Setting the `previousInputsDigest` on each output based on the inputs.
/// - Storing the error message with the [_failureReporter].
Future<void> _setOutputsState(
AssetId input,
Iterable<AssetId> outputs,
AssetReader reader,
InputTracker inputTracker,
Expand All @@ -1010,8 +1016,8 @@ class _SingleBuild {
if (outputs.isEmpty) return;
var usedInputs =
unusedAssets != null
? inputTracker.assetsRead.difference(unusedAssets)
: inputTracker.assetsRead;
? inputTracker.inputs.difference(unusedAssets)
: inputTracker.inputs;

final inputsDigest = await _computeCombinedDigest(
usedInputs,
Expand Down
Loading

0 comments on commit 9e96d43

Please sign in to comment.