Skip to content

File based asset system #22

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 3 commits into from
Feb 2, 2016
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 .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ build/
pubspec.lock

# Include .packages files from tests which are hand coded
!test/package_graph/**/.packages
!test/fixtures/**/.packages
1 change: 1 addition & 0 deletions e2e_example/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
**/*.txt.copy
1 change: 1 addition & 0 deletions lib/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
export 'src/asset/asset.dart';
export 'src/asset/exceptions.dart';
export 'src/asset/file_based.dart';
export 'src/asset/id.dart';
export 'src/asset/reader.dart';
export 'src/asset/writer.dart';
Expand Down
30 changes: 30 additions & 0 deletions lib/src/asset/exceptions.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) 2016, 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 'asset.dart';
import 'id.dart';

class AssetNotFoundException implements Exception {
Expand All @@ -11,3 +12,32 @@ class AssetNotFoundException implements Exception {
@override
String toString() => 'AssetNotFoundException: $assetId';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider reusing Error#safeToString in exception toString methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried that out but the message doesn't seem very good? (all you get is the type)

}

class PackageNotFoundException implements Exception {
final String name;

PackageNotFoundException(this.name);

@override
String toString() => 'PackageNotFoundException: $name';
}

class InvalidOutputException implements Exception {
final Asset asset;

InvalidOutputException(this.asset);

@override
String toString() => 'InvalidOutputException: $asset\n'
'Files may only be output in the root (application) package.';
}

class InvalidInputException implements Exception {
final AssetId assetId;

InvalidInputException(this.assetId);

@override
String toString() => 'InvalidInputException: $assetId\n'
'For package dependencies, only files under `lib` may be used as inputs.';
}
77 changes: 77 additions & 0 deletions lib/src/asset/file_based.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright (c) 2016, 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 'dart:convert';
import 'dart:io';

import 'package:path/path.dart' as path;

import '../asset/asset.dart';
import '../asset/exceptions.dart';
import '../asset/id.dart';
import '../asset/reader.dart';
import '../asset/writer.dart';
import '../package_graph/package_graph.dart';
import 'exceptions.dart';

/// Basic [AssetReader] which uses a [PackageGraph] to look up where to read
/// files from disk.
class FileBasedAssetReader implements AssetReader {
final PackageGraph packageGraph;

FileBasedAssetReader(this.packageGraph);

@override
Future<bool> hasInput(AssetId id) async {
_checkInput(id);
return _fileFor(id, packageGraph).exists();
}

@override
Future<String> readAsString(AssetId id, {Encoding encoding: UTF8}) async {
_checkInput(id);

var file = await _fileFor(id, packageGraph);
if (!await file.exists()) {
throw new AssetNotFoundException(id);
}
return file.readAsString(encoding: encoding);
}

/// Checks that [id] is a valid input, and throws an [InvalidInputException]
/// if its not.
void _checkInput(AssetId id) {
if (id.package != packageGraph.root.name && !id.path.startsWith('lib/')) {
throw new InvalidInputException(id);
}
}
}

/// Basic [AssetWriter] which uses a [PackageGraph] to look up where to write
/// files to disk.
class FileBasedAssetWriter implements AssetWriter {
final PackageGraph packageGraph;

FileBasedAssetWriter(this.packageGraph);

@override
Future writeAsString(Asset asset, {Encoding encoding: UTF8}) async {
if (asset.id.package != packageGraph.root.name) {
throw new InvalidOutputException(asset);
}

var file = _fileFor(asset.id, packageGraph);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this method throw if the file exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The writer doesn't have enough knowledge to know whether that is ok (it is fine to overwrite a generated file from a previous build, just not a previous phase).

This has to be implemented at the BuildStep/Transformer level

await file.create(recursive: true);
await file.writeAsString(asset.stringContents, encoding: encoding);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the answer is no, but should we be flushing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know a ton about when that would be necessary, but I don't think it is? This will eventually be caching these files as well, at which point it really won't matter.

}
}

/// Returns a [File] for [id] given [packageGraph].
File _fileFor(AssetId id, PackageGraph packageGraph) {
var package = packageGraph[id.package];
if (package == null) {
throw new PackageNotFoundException(id.package);
}
return new File(path.join(package.location.toFilePath(), id.path));
}
88 changes: 37 additions & 51 deletions lib/src/generate/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,62 @@
// 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 'dart:convert';
import 'dart:io';

import 'package:path/path.dart' as path;
import 'package:yaml/yaml.dart';

import '../asset/asset.dart';
import '../asset/file_based.dart';
import '../asset/id.dart';
import '../asset/reader.dart';
import '../asset/writer.dart';
import '../builder/builder.dart';
import '../builder/build_step_impl.dart';
import '../package_graph/package_graph.dart';
import 'build_result.dart';
import 'input_set.dart';
import 'phase.dart';

/// Runs all of the [Phases] in [phaseGroups].
Future<BuildResult> build(List<List<Phase>> phaseGroups) async {
try {
///
/// A [packageGraph] may be supplied, otherwise one will be constructed using
/// [PackageGraph.forThisPackage]. The default functionality assumes you are
/// running in the root directory of a package, with both a `pubspec.yaml` and
/// `.packages` file present.
///
/// A [reader] and [writer] may also be supplied, which can read/write assets
/// to arbitrary locations or file systems. By default they will write to the
/// current directory, and will use the `packageGraph` to know where to read
/// files from.
Future<BuildResult> build(List<List<Phase>> phaseGroups,
{PackageGraph packageGraph, AssetReader reader, AssetWriter writer}) async {
packageGraph ??= new PackageGraph.forThisPackage();
reader ??= new FileBasedAssetReader(packageGraph);
writer ??= new FileBasedAssetWriter(packageGraph);
return runZoned(() {
_validatePhases(phaseGroups);
return _runPhases(phaseGroups);
} catch (e, s) {
}, onError: (e, s) {
return new BuildResult(BuildStatus.Failure, BuildType.Full, [],
exception: e, stackTrace: s);
}
}, zoneValues: {
_assetReaderKey: reader,
_assetWriterKey: writer,
_packageGraphKey: packageGraph,
});
}

/// Keys for reading zone local values.
Symbol _assetReaderKey = #buildAssetReader;
Symbol _assetWriterKey = #buildAssetWriter;
Symbol _packageGraphKey = #buildPackageGraph;

/// Getters for zone local values.
AssetReader get _reader => Zone.current[_assetReaderKey];
AssetWriter get _writer => Zone.current[_assetWriterKey];
PackageGraph get _packageGraph => Zone.current[_packageGraphKey];

/// The local package name from your pubspec.
final String _localPackageName = () {
var pubspec = new File('pubspec.yaml');
Expand Down Expand Up @@ -89,18 +118,15 @@ List<AssetId> _assetIdsFor(List<InputSet> inputSets) {

/// Returns all files matching [inputSet].
Set<File> _filesMatching(InputSet inputSet) {
if (inputSet.package != _localPackageName) {
throw new UnimplementedError('Running on packages other than the '
'local package is not yet supported');
}

var files = new Set<File>();
var root = _packageGraph[inputSet.package].location.toFilePath();
for (var glob in inputSet.globs) {
files.addAll(glob.listSync(followLinks: false).where(
files.addAll(glob.listSync(followLinks: false, root: root).where(
(e) => e is File && !_ignoredDirs.contains(path.split(e.path)[1])));
}
return files;
}
const _ignoredDirs = const ['build'];

/// Runs [builder] with [inputs] as inputs.
Stream<Asset> _runBuilder(Builder builder, List<AssetId> inputs) async* {
Expand All @@ -116,43 +142,3 @@ Stream<Asset> _runBuilder(Builder builder, List<AssetId> inputs) async* {
}
}
}

/// Very simple [AssetReader], only works on local package and assumes you are
/// running from the root of the package.
class _SimpleAssetReader implements AssetReader {
const _SimpleAssetReader();

@override
Future<bool> hasInput(AssetId id) async {
assert(id.package == _localPackageName);
return new File(id.path).exists();
}

@override
Future<String> readAsString(AssetId id, {Encoding encoding: UTF8}) async {
assert(id.package == _localPackageName);
return new File(id.path).readAsString(encoding: encoding);
}
}

const AssetReader _reader = const _SimpleAssetReader();

/// Very simple [AssetWriter], only works on local package and assumes you are
/// running from the root of the package.
class _SimpleAssetWriter implements AssetWriter {
final _outputDir;

const _SimpleAssetWriter(this._outputDir);

@override
Future writeAsString(Asset asset, {Encoding encoding: UTF8}) async {
assert(asset.id.package == _localPackageName);
var file = new File(path.join(_outputDir, asset.id.path));
await file.create(recursive: true);
await file.writeAsString(asset.stringContents, encoding: encoding);
}
}

const AssetWriter _writer = const _SimpleAssetWriter('generated');

const _ignoredDirs = const ['generated', 'build', 'packages'];
94 changes: 94 additions & 0 deletions test/asset/file_based_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Copyright (c) 2016, 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.
@TestOn('vm')
import 'dart:io';

import 'package:test/test.dart';

import 'package:build/build.dart';

import '../common/common.dart';

final packageGraph = new PackageGraph.forPath('test/fixtures/basic_pkg');

main() async {

group('FileBasedAssetReader', () {
final reader = new FileBasedAssetReader(packageGraph);

test('can read any application package files', () async {
expect(await reader.readAsString(makeAssetId('basic_pkg|hello.txt')),
'world\n');
expect(await reader.readAsString(makeAssetId('basic_pkg|lib/hello.txt')),
'world\n');
expect(await reader.readAsString(makeAssetId('basic_pkg|web/hello.txt')),
'world\n');
});

test('can only read package dependency files in the lib dir', () async {
expect(await reader.readAsString(makeAssetId('a|lib/a.txt')), 'A\n');
expect(reader.readAsString(makeAssetId('a|web/a.txt')),
throwsA(invalidInputException));
expect(reader.readAsString(makeAssetId('a|a.txt')),
throwsA(invalidInputException));
});

test('can check for existence of any application package files', () async {
expect(await reader.hasInput(makeAssetId('basic_pkg|hello.txt')), isTrue);
expect(await reader.hasInput(makeAssetId('basic_pkg|lib/hello.txt')),
isTrue);
expect(await reader.hasInput(makeAssetId('basic_pkg|web/hello.txt')),
isTrue);

expect(await reader.hasInput(makeAssetId('basic_pkg|a.txt')), isFalse);
expect(
await reader.hasInput(makeAssetId('basic_pkg|lib/a.txt')), isFalse);
});

test('can only check for existence of package dependency files in lib',
() async {
expect(await reader.hasInput(makeAssetId('a|lib/a.txt')), isTrue);
expect(await reader.hasInput(makeAssetId('a|lib/b.txt')), isFalse);
expect(reader.hasInput(makeAssetId('a|web/a.txt')),
throwsA(invalidInputException));
expect(reader.hasInput(makeAssetId('a|a.txt')),
throwsA(invalidInputException));
expect(reader.hasInput(makeAssetId('foo|bar.txt')),
throwsA(invalidInputException));
});

test('throws when attempting to read a non-existent file', () async {
expect(reader.readAsString(makeAssetId('basic_pkg|foo.txt')),
throwsA(assetNotFoundException));
expect(reader.readAsString(makeAssetId('a|lib/b.txt')),
throwsA(assetNotFoundException));
expect(reader.readAsString(makeAssetId('foo|lib/bar.txt')),
throwsA(packageNotFoundException));
});
});

group('FileBasedAssetWriter', () {
final writer = new FileBasedAssetWriter(packageGraph);

test('can output files in the application package', () async {
var asset = makeAsset('basic_pkg|test_file.txt', 'test');
await writer.writeAsString(asset);
var id = asset.id;
var file = new File('test/fixtures/${id.package}/${id.path}');
expect(await file.exists(), isTrue);
expect(await file.readAsString(), 'test');
await file.delete();
});

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If FileBasedAssetWriter can't overwrite files, add a test here for that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n/a

test('can\'t output files in package dependencies', () async {
var asset = makeAsset('a|test.txt');
expect(writer.writeAsString(asset), throwsA(invalidOutputException));
});

test('can\'t output files in arbitrary packages', () async {
var asset = makeAsset('foo|bar.txt');
expect(writer.writeAsString(asset), throwsA(invalidOutputException));
});
});
}
35 changes: 35 additions & 0 deletions test/common/assets.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright (c) 2016, 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 'package:build/build.dart';

import 'in_memory_writer.dart';

int _nextId = 0;
AssetId makeAssetId([String assetIdString]) {
if (assetIdString == null) {
assetIdString = 'a|web/asset_$_nextId.txt';
_nextId++;
}
return new AssetId.parse(assetIdString);
}

Asset makeAsset([String assetIdString, String contents]) {
var id = makeAssetId(assetIdString);
return new Asset(id, contents ?? '$id');
}

Map<AssetId, Asset> makeAssets(Map<String, String> assetsMap) {
var assets = <AssetId, Asset>{};
assetsMap.forEach((idString, content) {
var asset = makeAsset(idString, content);
assets[asset.id] = asset;
});
return assets;
}

void addAssets(Iterable<Asset> assets, InMemoryAssetWriter writer) {
for (var asset in assets) {
writer.assets[asset.id] = asset.stringContents;
}
}
Loading