Skip to content

Refactor MultiPackagesAssetReader to AssetFinder #3859

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
Feb 17, 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
2 changes: 1 addition & 1 deletion _test_common/lib/in_memory_reader_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import 'package:path/path.dart' as p;
import 'package:watcher/watcher.dart';

class InMemoryRunnerAssetReaderWriter extends InMemoryAssetReaderWriter
implements RunnerAssetReader, RunnerAssetWriter {
implements AssetReader, RunnerAssetWriter {
final _onCanReadController = StreamController<AssetId>.broadcast();
Stream<AssetId> get onCanRead => _onCanReadController.stream;
void Function(AssetId)? onDelete;
Expand Down
3 changes: 2 additions & 1 deletion _test_common/lib/test_environment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'dart:async';

import 'package:build/build.dart';
import 'package:build_runner_core/build_runner_core.dart';
import 'package:logging/logging.dart';

Expand All @@ -21,7 +22,7 @@ class TestBuildEnvironment extends BuildEnvironment {
final InMemoryRunnerAssetReaderWriter _readerWriter;

@override
RunnerAssetReader get reader => _readerWriter;
AssetReader get reader => _readerWriter;
@override
RunnerAssetWriter get writer => _readerWriter;

Expand Down
1 change: 1 addition & 0 deletions build/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
`build_runner_core` and `build_test`.
- Use `build_test` 3.0.0.
- Refactor `PathProvidingAssetReader` to `AssetPathProvider`.
- Refactor `MultiPackageAssetReader` to internal `AssetFinder`.

## 2.4.2

Expand Down
14 changes: 0 additions & 14 deletions build/lib/src/asset/reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,3 @@ abstract class AssetReader {
return digestSink.events.first;
}
}

/// The same as an `AssetReader`, except that `findAssets` takes an optional
/// argument `package` which allows you to glob any package.
///
/// This should not be exposed to end users generally, but can be used by
/// different build system implementations.
abstract class MultiPackageAssetReader extends AssetReader {
/// Returns all readable assets matching [glob] under [package].
///
/// Some implementations may require the [package] argument, while others
/// may have a sane default.
@override
Stream<AssetId> findAssets(Glob glob, {String? package});
}
10 changes: 5 additions & 5 deletions build/lib/src/builder/build_step_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import '../asset/id.dart';
import '../asset/reader.dart';
import '../asset/writer.dart';
import '../resource/resource.dart';
import '../state/asset_finder.dart';
import '../state/asset_path_provider.dart';
import '../state/input_tracker.dart';
import '../state/reader_state.dart';
Expand Down Expand Up @@ -81,6 +82,9 @@ class BuildStepImpl implements BuildStep, AssetReaderState {
_stageTracker = stageTracker ?? NoOpStageTracker.instance,
_reportUnusedAssets = reportUnusedAssets;

@override
AssetFinder get assetFinder => _reader.assetFinder;

@override
AssetPathProvider? get assetPathProvider => _reader.assetPathProvider;

Expand Down Expand Up @@ -135,11 +139,7 @@ class BuildStepImpl implements BuildStep, AssetReaderState {
@override
Stream<AssetId> findAssets(Glob glob) {
if (_isComplete) throw BuildStepCompletedException();
if (_reader is MultiPackageAssetReader) {
return _reader.findAssets(glob, package: inputId.package);
} else {
return _reader.findAssets(glob);
}
return _reader.assetFinder.find(glob, package: inputId.package);
}

@override
Expand Down
5 changes: 3 additions & 2 deletions build/lib/src/internal.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
// 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.

/// Internal build state for `build_resolvers`, `build_runner_core` and
/// `build_test` only.
/// Internal build state for `build_resolvers`, `build_runner`,
/// `build_runner_core` and `build_test` only.
library;

export 'state/asset_finder.dart';
export 'state/asset_path_provider.dart';
export 'state/input_tracker.dart';
export 'state/reader_state.dart';
25 changes: 25 additions & 0 deletions build/lib/src/state/asset_finder.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// 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:async';

import 'package:glob/glob.dart';

import '../asset/id.dart';

abstract interface class AssetFinder {
/// Returns all readable assets matching [glob] under [package].
///
/// `Reader.findAssets` exposes this funcionality without allowing controlling
/// [package].
Stream<AssetId> find(Glob glob, {String? package});
}

class FunctionAssetFinder implements AssetFinder {
final Stream<AssetId> Function(Glob, String?) function;

FunctionAssetFinder(this.function);

@override
Stream<AssetId> find(Glob glob, {String? package}) => function(glob, package);
}
12 changes: 12 additions & 0 deletions build/lib/src/state/reader_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@
// BSD-style license that can be found in the LICENSE file.

import '../asset/reader.dart';
import 'asset_finder.dart';
import 'asset_path_provider.dart';
import 'input_tracker.dart';

/// Provides access to the state backing an [AssetReader].
extension AssetReaderStateExtension on AssetReader {
AssetFinder get assetFinder {
_requireIsAssetReaderState();
return (this as AssetReaderState).assetFinder;
}

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

Expand Down Expand Up @@ -37,6 +43,12 @@ extension AssetReaderStateExtension on AssetReader {

/// The state backing an [AssetReader].
abstract interface class AssetReaderState {
/// The [AssetFinder] associated with this reader.
///
/// All readers have an [AssetFinder], but the functionality it provides,
/// 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;
Expand Down
2 changes: 2 additions & 0 deletions build_runner/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## 2.4.16-wip

- Use `build_test` 3.0.0.
- Start using `package:build/src/internal.dart'.
- Refactor `MultiPackageAssetReader` to internal `AssetFinder`.

## 2.4.15

Expand Down
4 changes: 2 additions & 2 deletions build_runner/lib/src/generate/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Future<BuildResult> build(List<BuilderApplication> builders,
bool? assumeTty,
String? configKey,
PackageGraph? packageGraph,
RunnerAssetReader? reader,
AssetReader? reader,
RunnerAssetWriter? writer,
Resolvers? resolvers,
Level? logLevel,
Expand Down Expand Up @@ -148,7 +148,7 @@ Future<ServeHandler> watch(List<BuilderApplication> builders,
bool? assumeTty,
String? configKey,
PackageGraph? packageGraph,
RunnerAssetReader? reader,
AssetReader? reader,
RunnerAssetWriter? writer,
Resolvers? resolvers,
Level? logLevel,
Expand Down
2 changes: 1 addition & 1 deletion build_runner/lib/src/generate/watch_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Future<ServeHandler> watch(
bool? assumeTty,
String? configKey,
PackageGraph? packageGraph,
RunnerAssetReader? reader,
AssetReader? reader,
RunnerAssetWriter? writer,
Resolvers? resolvers,
Level? logLevel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import 'dart:async';

import 'package:build/build.dart';
// ignore: implementation_imports
import 'package:build/src/internal.dart';
import 'package:build_config/build_config.dart';
import 'package:build_runner_core/build_runner_core.dart';
import 'package:glob/glob.dart';
Expand All @@ -14,11 +16,11 @@ import 'package:path/path.dart' as p;
final _log = Logger('BuildConfigOverrides');

Future<Map<String, BuildConfig>> findBuildConfigOverrides(
PackageGraph packageGraph, RunnerAssetReader reader,
PackageGraph packageGraph, AssetReader reader,
{String? configKey}) async {
final configs = <String, BuildConfig>{};
final configFiles =
reader.findAssets(Glob('*.build.yaml'), package: packageGraph.root.name);
final configFiles = reader.assetFinder
.find(Glob('*.build.yaml'), package: packageGraph.root.name);
await for (final id in configFiles) {
final packageName = p.basename(id.path).split('.').first;
final packageNode = packageGraph.allPackages[packageName];
Expand Down
2 changes: 1 addition & 1 deletion build_runner/lib/src/server/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ class AssetHandler {
var directoryPath = p.url.dirname(from.path);
var glob = p.url.join(directoryPath, '*');
var result =
await _reader.findAssets(Glob(glob)).map((a) => a.path).toList();
await _reader.assetFinder.find(Glob(glob)).map((a) => a.path).toList();
var message = StringBuffer('Could not find ${from.path}');
if (result.isEmpty) {
message.write(' or any files in $directoryPath. ');
Expand Down
2 changes: 1 addition & 1 deletion build_runner/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ dependencies:
analyzer: '>=4.4.0 <8.0.0'
args: ^2.0.0
async: ^2.5.0
build: ">=2.1.0 <2.5.0"
build: ^2.3.4-wip
build_config: ">=1.1.0 <1.2.0"
build_daemon: ^4.0.0
build_resolvers: ^2.4.4
Expand Down
1 change: 1 addition & 0 deletions build_runner_core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- Start using `package:build/src/internal.dart'.
- Use `build_test` 3.0.0.
- Refactor `PathProvidingAssetReader` to `AssetPathProvider`.
- Refactor `MultiPackageAssetReader` to internal `AssetFinder`.

## 8.0.0

Expand Down
1 change: 0 additions & 1 deletion build_runner_core/lib/build_runner_core.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ export 'package:build/build.dart' show PostProcessBuildStep, PostProcessBuilder;
export 'src/asset/batch.dart' show wrapInBatch;
export 'src/asset/file_based.dart';
export 'src/asset/finalized_reader.dart';
export 'src/asset/reader.dart' show RunnerAssetReader;
export 'src/asset/writer.dart';
export 'src/environment/build_environment.dart';
export 'src/environment/io_environment.dart';
Expand Down
23 changes: 13 additions & 10 deletions build_runner_core/lib/src/asset/batch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import 'package:glob/glob.dart';
import 'package:meta/meta.dart';

import '../environment/io_environment.dart';
import 'reader.dart';
import 'writer.dart';

/// A batch of file system writes that should be committed at once instead of
Expand Down Expand Up @@ -46,15 +45,15 @@ final class _FileSystemWriteBatch {
}
}

/// Wraps a pair of a [RunnerAssetReader] with path-prividing capabilities and
/// Wraps a pair of a [AssetReader] with path-providing capabilities and
/// a [RunnerAssetWriter] into a pair of readers and writers that will
/// internally buffer writes and only flush them in
/// [RunnerAssetWriter.completeBuild].
///
/// The returned reader will see pending writes by the returned writer before
/// they are flushed to the file system.
(RunnerAssetReader, RunnerAssetWriter) wrapInBatch({
required RunnerAssetReader reader,
(AssetReader, RunnerAssetWriter) wrapInBatch({
required AssetReader reader,
required RunnerAssetWriter writer,
}) {
final batch = _FileSystemWriteBatch._();
Expand All @@ -74,9 +73,10 @@ final class _PendingFileState {
}

@internal
final class BatchReader extends AssetReader
implements AssetReaderState, RunnerAssetReader {
final RunnerAssetReader _inner;
final class BatchReader extends AssetReader implements AssetReaderState {
@override
late final AssetFinder assetFinder = FunctionAssetFinder(_findAssets);
final AssetReader _inner;
final _FileSystemWriteBatch _batch;

BatchReader(this._inner, this._batch);
Expand All @@ -100,10 +100,13 @@ final class BatchReader extends AssetReader
}
}

// This is only for generators, so only `BuildStep` needs to implement it.
@override
Stream<AssetId> findAssets(Glob glob, {String? package}) {
return _inner
.findAssets(glob, package: package)
Stream<AssetId> findAssets(Glob glob) => throw UnimplementedError();

Stream<AssetId> _findAssets(Glob glob, String? package) {
return _inner.assetFinder
.find(glob, package: package)
.where((asset) => _stateFor(asset)?.isDeleted != true);
}

Expand Down
3 changes: 3 additions & 0 deletions build_runner_core/lib/src/asset/build_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ class BuildCacheReader implements AssetReader, AssetReaderState {
delegate: delegate.assetPathProvider!,
overlay: (id) => _cacheLocation(id, assetGraph, rootPackage));

@override
AssetFinder get assetFinder => _delegate.assetFinder;

@override
InputTracker? get inputTracker => _delegate.inputTracker;

Expand Down
9 changes: 5 additions & 4 deletions build_runner_core/lib/src/asset/cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ import 'lru_cache.dart';
/// An [AssetReader] that caches all results from the delegate.
///
/// Assets are cached until [invalidate] is invoked.
///
/// Does not implement [findAssets].
class CachingAssetReader implements AssetReader, AssetReaderState {
/// Cached results of [readAsBytes].
final _bytesContentCache = LruCache<AssetId, List<int>>(
Expand Down Expand Up @@ -55,16 +53,19 @@ class CachingAssetReader implements AssetReader, AssetReaderState {
@override
InputTracker? get inputTracker => _delegate.inputTracker;

@override
AssetFinder get assetFinder => _delegate.assetFinder;

@override
Future<bool> canRead(AssetId id) =>
_canReadCache.putIfAbsent(id, () => _delegate.canRead(id));

@override
Future<Digest> digest(AssetId id) => _delegate.digest(id);

// This is only for generators, so only `BuildStep` needs to implement it.
@override
Stream<AssetId> findAssets(Glob glob) =>
throw UnimplementedError('unimplemented!');
Stream<AssetId> findAssets(Glob glob) => throw UnimplementedError();

@override
Future<List<int>> readAsBytes(AssetId id, {bool cache = true}) {
Expand Down
12 changes: 8 additions & 4 deletions build_runner_core/lib/src/asset/file_based.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,17 @@ import 'package:path/path.dart' as path;
import 'package:pool/pool.dart';

import '../package_graph/package_graph.dart';
import 'reader.dart';
import 'writer.dart';

/// Pool for async file operations, we don't want to use too many file handles.
final _descriptorPool = Pool(32);

/// Basic [AssetReader] which uses a [PackageGraph] to look up where to read
/// files from disk.
class FileBasedAssetReader extends AssetReader
implements AssetReaderState, RunnerAssetReader {
class FileBasedAssetReader extends AssetReader implements AssetReaderState {
@override
late final AssetFinder assetFinder = FunctionAssetFinder(_findAssets);

final PackageGraph packageGraph;

FileBasedAssetReader(this.packageGraph);
Expand All @@ -47,8 +48,11 @@ class FileBasedAssetReader extends AssetReader
_fileForOrThrow(id, packageGraph).then((file) => _descriptorPool
.withResource(() => file.readAsString(encoding: encoding)));

// This is only for generators, so only `BuildStep` needs to implement it.
@override
Stream<AssetId> findAssets(Glob glob, {String? package}) {
Stream<AssetId> findAssets(Glob glob) => throw UnimplementedError();

Stream<AssetId> _findAssets(Glob glob, String? package) {
var packageNode =
package == null ? packageGraph.root : packageGraph[package];
if (packageNode == null) {
Expand Down
Loading
Loading