Skip to content

Refactor BuildCacheReader to BuildCacheAssetPathProvider. #3874

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
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
12 changes: 5 additions & 7 deletions _test_common/lib/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ export 'sdk.dart';
export 'test_phases.dart';

Digest computeDigest(AssetId id, String contents) {
// Special handling for `$$` assets, these are generated under the .dart_tool
// dir and unfortunately that leaks into the digest computations.
if (id.package.startsWith(r'$$')) {
var package = id.package.substring(2);
id = AssetId(package, '.dart_tool/build/generated/$package/${id.path}');
}
return md5.convert([...utf8.encode(contents), ...id.toString().codeUnits]);
// Tests use `$$` at the start of an ID to signal "generated", remove it.
var idString = id.toString();
if (idString.startsWith(r'$$')) idString = idString.substring(2);

return md5.convert([...utf8.encode(contents), ...idString.codeUnits]);
}

class PlaceholderBuilder extends Builder {
Expand Down
2 changes: 2 additions & 0 deletions build/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## 2.4.3-wip

- `AssetNotFoundException` now also reports the missing `path`.
- Bump the min sdk to 3.7.0.
- Use `build_test` 3.0.0.
- Add `package:build/src/internal.dart` for use by `build_resolvers`,
Expand All @@ -9,6 +10,7 @@
- Add internal `Filesystem` that backs `AssetReader` and `AssetWriter`
implementations.
- Refactor `CachingAssetReader` to `FilesystemCache`.
- Refactor `BuildCacheReader` to `BuildCacheAssetPathProvider`.

## 2.4.2

Expand Down
8 changes: 6 additions & 2 deletions build/lib/src/asset/exceptions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ import 'id.dart';

class AssetNotFoundException implements Exception {
final AssetId assetId;
final String? path;

AssetNotFoundException(this.assetId);
AssetNotFoundException(this.assetId, {this.path});

@override
String toString() => 'AssetNotFoundException: $assetId';
String toString() {
if (path == null) return 'AssetNotFoundException: $assetId';
return 'AssetNotFoundException: $assetId ($path)';
}
}

class PackageNotFoundException implements Exception {
Expand Down
9 changes: 6 additions & 3 deletions build/lib/src/builder/build_step_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,13 @@ class BuildStepImpl implements BuildStep, AssetReaderState {
_reportUnusedAssets = reportUnusedAssets;

@override
BuildStepImpl copyWith({FilesystemCache? cache}) => BuildStepImpl(
BuildStepImpl copyWith({
AssetPathProvider? assetPathProvider,
FilesystemCache? cache,
}) => BuildStepImpl(
inputId,
allowedOutputs,
_reader.copyWith(cache: cache),
_reader.copyWith(assetPathProvider: assetPathProvider, cache: cache),
_writer,
_resolvers,
_resourceManager,
Expand All @@ -107,7 +110,7 @@ class BuildStepImpl implements BuildStep, AssetReaderState {
AssetFinder get assetFinder => _reader.assetFinder;

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

@override
InputTracker? get inputTracker => _reader.inputTracker;
Expand Down
13 changes: 1 addition & 12 deletions build/lib/src/state/asset_path_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,7 @@

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

/// Converts [AssetId] to paths.
/// Converts [AssetId]s to paths.
abstract interface class AssetPathProvider {
String pathFor(AssetId id);
}

/// Applies a function to an existing [AssetPathProvider].
class OverlayAssetPathProvider implements AssetPathProvider {
AssetPathProvider delegate;
AssetId Function(AssetId) overlay;

OverlayAssetPathProvider({required this.delegate, required this.overlay});

@override
String pathFor(AssetId id) => delegate.pathFor(overlay(id));
}
32 changes: 20 additions & 12 deletions build/lib/src/state/reader_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,15 @@ import 'input_tracker.dart';
/// Provides access to the state backing an [AssetReader].
extension AssetReaderStateExtension on AssetReader {
/// Returns a new instance with optionally updated [cache].
AssetReader copyWith({FilesystemCache? cache}) {
AssetReader copyWith({
AssetPathProvider? assetPathProvider,
FilesystemCache? cache,
}) {
_requireIsAssetReaderState();
return (this as AssetReaderState).copyWith(cache: cache);
return (this as AssetReaderState).copyWith(
assetPathProvider: assetPathProvider,
cache: cache,
);
}

Filesystem get filesystem {
Expand All @@ -35,6 +41,11 @@ extension AssetReaderStateExtension on AssetReader {
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;
Expand All @@ -47,11 +58,6 @@ extension AssetReaderStateExtension on AssetReader {
return result;
}

AssetPathProvider? get assetPathProvider =>
this is AssetReaderState
? (this as AssetReaderState).assetPathProvider
: null;

/// Throws if `this` is not an [AssetReaderState].
void _requireIsAssetReaderState() {
if (this is! AssetReaderState) {
Expand All @@ -64,8 +70,11 @@ extension AssetReaderStateExtension on AssetReader {

/// The state backing an [AssetReader].
abstract interface class AssetReaderState {
/// Returns a new instance with optionally updated [cache].
AssetReader copyWith({FilesystemCache? cache});
/// Returns a new instance with optionally updated [assetPathProvider] and/or [cache].
AssetReader copyWith({
AssetPathProvider? assetPathProvider,
FilesystemCache? cache,
});

/// The [Filesystem] that this reader reads from.
///
Expand All @@ -86,7 +95,6 @@ abstract interface class AssetReaderState {
/// not have one.
InputTracker? get inputTracker;

/// The [AssetPathProvider] associated with this reader, or `null` if it does
/// not have one.
AssetPathProvider? get assetPathProvider;
/// The [AssetPathProvider] associated with this reader.
AssetPathProvider get assetPathProvider;
}
1 change: 1 addition & 0 deletions build_runner_core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Add internal `Filesystem` that backs `AssetReader` and `AssetWriter`
implementations.
- Refactor `CachingAssetReader` to `FilesystemCache`.
- Refactor `BuildCacheReader` to `BuildCacheAssetPathProvider`.

## 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 @@ -4,7 +4,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/writer.dart';
Expand Down
28 changes: 5 additions & 23 deletions build_runner_core/lib/src/asset/batch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import 'writer.dart';
///
/// The default [IOEnvironment] uses readers and writes that are batch-aware
/// outside of low-memory mode.
///
/// TODO(davidmorgan): this is not currently used, refactor into `Filesystem`.
final class _FileSystemWriteBatch {
final Map<AssetId, _PendingFileState> _pendingWrites = {};

Expand Down Expand Up @@ -54,7 +56,7 @@ final class _FileSystemWriteBatch {
///
/// The returned reader will see pending writes by the returned writer before
/// they are flushed to the file system.
(AssetReader, RunnerAssetWriter) wrapInBatch({
(BatchReader, BatchWriter) wrapInBatch({
required AssetReader reader,
required RunnerAssetWriter writer,
}) {
Expand All @@ -72,35 +74,18 @@ final class _PendingFileState {
}

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

final AssetReader _inner;
final _FileSystemWriteBatch _batch;

BatchReader(this._inner, this._batch);

@override
BatchReader copyWith({FilesystemCache? cache}) =>
BatchReader(_inner.copyWith(cache: cache), _batch);

@override
Filesystem get filesystem => _inner.filesystem;

@override
FilesystemCache get cache => _inner.cache;

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

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

_PendingFileState? _stateFor(AssetId id) {
return _batch._pendingWrites[id];
}

@override
Future<bool> canRead(AssetId id) async {
if (_stateFor(id) case final state?) {
return !state.isDeleted;
Expand All @@ -110,7 +95,6 @@ final class BatchReader extends AssetReader implements AssetReaderState {
}

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

Stream<AssetId> _findAssets(Glob glob, String? package) {
Expand All @@ -119,7 +103,6 @@ final class BatchReader extends AssetReader implements AssetReaderState {
.where((asset) => _stateFor(asset)?.isDeleted != true);
}

@override
Future<List<int>> readAsBytes(AssetId id) async {
if (_stateFor(id) case final state?) {
if (state.isDeleted) {
Expand All @@ -132,7 +115,6 @@ final class BatchReader extends AssetReader implements AssetReaderState {
}
}

@override
Future<String> readAsString(AssetId id, {Encoding encoding = utf8}) async {
if (_stateFor(id) case final state?) {
if (state.isDeleted) {
Expand Down
83 changes: 18 additions & 65 deletions build_runner_core/lib/src/asset/build_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,84 +4,37 @@ import 'dart:convert';
import 'package:build/build.dart';
// ignore: implementation_imports
import 'package:build/src/internal.dart';
import 'package:crypto/crypto.dart';
import 'package:glob/glob.dart';

import '../asset_graph/graph.dart';
import '../asset_graph/node.dart';
import '../util/constants.dart';
import 'writer.dart';

/// Wraps an [AssetReader] and translates reads for generated files into reads
/// from the build cache directory
class BuildCacheReader implements AssetReader, AssetReaderState {
@override
final AssetPathProvider? assetPathProvider;

final AssetReader _delegate;
/// Wraps an [AssetPathProvider] to place hidden generated files in the build
/// cache directory.
///
/// Assets that are in the provided [AssetGraph], have node type
/// [GeneratedAssetNode] and have `isHidden == true` are mapped to
/// [generatedOutputDirectory].
class BuildCacheAssetPathProvider implements AssetPathProvider {
final AssetPathProvider _delegate;
final AssetGraph _assetGraph;
final String _rootPackage;

BuildCacheReader(
AssetReader delegate,
AssetGraph assetGraph,
String rootPackage,
) : _delegate = delegate,
_assetGraph = assetGraph,
_rootPackage = rootPackage,
assetPathProvider =
delegate.assetPathProvider == null
? null
: OverlayAssetPathProvider(
delegate: delegate.assetPathProvider!,
overlay: (id) => _cacheLocation(id, assetGraph, rootPackage),
);

@override
BuildCacheReader copyWith({FilesystemCache? cache}) => BuildCacheReader(
_delegate.copyWith(cache: cache),
_assetGraph,
_rootPackage,
);

@override
Filesystem get filesystem => _delegate.filesystem;

@override
FilesystemCache get cache => _delegate.cache;

@override
AssetFinder get assetFinder => _delegate.assetFinder;

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

@override
Future<bool> canRead(AssetId id) =>
_delegate.canRead(_cacheLocation(id, _assetGraph, _rootPackage));

@override
Future<Digest> digest(AssetId id) =>
_delegate.digest(_cacheLocation(id, _assetGraph, _rootPackage));

@override
Future<List<int>> readAsBytes(AssetId id) =>
_delegate.readAsBytes(_cacheLocation(id, _assetGraph, _rootPackage));

@override
Future<String> readAsString(AssetId id, {Encoding encoding = utf8}) =>
_delegate.readAsString(
_cacheLocation(id, _assetGraph, _rootPackage),
encoding: encoding,
);
BuildCacheAssetPathProvider({
required AssetPathProvider delegate,
required AssetGraph assetGraph,
required String rootPackage,
}) : _delegate = delegate,
_assetGraph = assetGraph,
_rootPackage = rootPackage;

@override
Stream<AssetId> findAssets(Glob glob) =>
throw UnimplementedError(
'Asset globbing should be done per phase with the AssetGraph',
);
String pathFor(AssetId id) =>
_delegate.pathFor(_cacheLocation(id, _assetGraph, _rootPackage));
}

// TODO(davidmorgan): refactor in the same way as the reader.
class BuildCacheWriter implements RunnerAssetWriter {
final AssetGraph _assetGraph;
final RunnerAssetWriter _delegate;
Expand Down
Loading
Loading