Skip to content

Commit

Permalink
Enables globbing for entry points.
Browse files Browse the repository at this point in the history
We have had a request for entry point abstraction for a while, and this
CL provides a very simple approach that enables the 'test_reflectable'
'pubspec.yaml' to be abbreviated substantially; it might be quite
helpful for others, too, even though it's so simple.

It introduces the ability for the transformer to glob the specified
entry points, i.e., it uses package 'glob' and the `Glob` method
`matches` to test whether a given `AssetId` is an entry point.
Consequently, clients of 'reflectable' (including 'test_reflectable')
can use a simple `entry_points: ["test/*_test.dart"]` rather than a
concrete list with dozens of test file names.

We do not support a seemingly even more convenient model: Dropping the
`entry_points:` specification entirely and relying on the provided
`Asset`s, just testing whether each of them has a `main` (directly or
imported). The problem with that is that we would then need to run
the analyzer during `declareOutputs`, which would completely undo the
response time improvements expected from a lazy transformer. So
programmers still have to take the responsibility to declare which
files are entry points, but they can now use a wildcard for it---they
cannot completely omit this information.

Btw, I think this update is awaited so eagerly that it makes sense to
go directly to a `pub publish` v0.5.3 when it has been landed.

R=sigurdm@google.com

Review URL: https://codereview.chromium.org/1722653002 .
  • Loading branch information
eernstg committed Feb 24, 2016
1 parent 66e303f commit 8936f98
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 93 deletions.
5 changes: 4 additions & 1 deletion coverage_reflectable/bin/unexecuted_lines
Expand Up @@ -8,7 +8,10 @@ BUILD_DIR=../../test_reflectable
TIMESTAMP=$(date +"%Y%m%d-%H%M")
OUTPUT_FILE="/tmp/unexecuted-lines-$TIMESTAMP.txt"
VM_PORT=43979
DART_VM_OPTIONS="--enable-vm-service:$VM_PORT -Dreflectable.pause.at.exit=true"
TEST_COUNT=$(ls ../../test_reflectable/test/*_test.dart | wc -l)
D_OPTION1="-Dreflectable.pause.at.exit=true"
D_OPTION2="-Dreflectable.pause.at.exit.count=$TEST_COUNT"
DART_VM_OPTIONS="--enable-vm-service:$VM_PORT $D_OPTION1 $D_OPTION2"
export DART_VM_OPTIONS
( cd $BUILD_DIR; pub build --mode=debug test ) & sleep 1

Expand Down
36 changes: 13 additions & 23 deletions reflectable/lib/src/transformer_implementation.dart
Expand Up @@ -13,6 +13,7 @@ import 'package:analyzer/src/generated/utilities_dart.dart';
import 'package:barback/barback.dart';
import 'package:code_transformers/resolver.dart';
import 'package:dart_style/dart_style.dart';
import 'package:glob/glob.dart';
import 'package:path/path.dart' as path;
import 'element_capability.dart' as ec;
import 'encoding_constants.dart' as constants;
Expand Down Expand Up @@ -2869,19 +2870,11 @@ class _ImportCollector {
// debugging which is concerned with the generated code (but that would
// ideally be an infrequent occurrence).

/// Keeps track of all entry points we have seen. Used to determine when the
/// Keeps track of the number of entry points seen. Used to determine when the
/// transformation job is complete during `pub build..` or stand-alone
/// transformation, such that it is time to give control to the debugger.
/// Only in use when `const bool.fromEnvironment("reflectable.pause.at.exit")`.
Set<String> _allEntryPoints = new Set<String>();

/// Keeps track of all entry points we have completed processing (either by
/// transforming it successfully, or by concluding that it should not be
/// transformed). Used to determine when the transformation job is complete
/// during `pub build..` or stand-alone transformation, such that it is time
/// to give control to the debugger.
/// Only in use when `const bool.fromEnvironment("reflectable.pause.at.exit")`.
Set<String> _processedEntryPoints = new Set<String>();
int _processedEntryPointCount = 0;

class TransformerImplementation {
TransformLogger _logger;
Expand Down Expand Up @@ -3750,21 +3743,17 @@ _initializeReflectable() {

Asset asset = await transform.primaryInput;
assert(asset != null); // Every transform has a primary asset.
String currentEntryPoint; // Null means not found.

if (const bool.fromEnvironment("reflectable.pause.at.exit")) {
_allEntryPoints.addAll(entryPoints);
}
AssetId currentEntryPoint; // Null means not found.

for (String entryPoint in entryPoints) {
if (asset.id.path.endsWith(entryPoint)) {
currentEntryPoint = entryPoint;
Glob glob = new Glob(entryPoint);
if (glob.matches(asset.id.path)) {
currentEntryPoint = asset.id;
break;
}
}
// If the given [asset] is not an entry point: skip.
if (currentEntryPoint == null) return;
assert(asset.id.path.endsWith(currentEntryPoint));

// The [_resolver] provides all the static information.
_resolver = await _resolvers.get(transform);
Expand All @@ -3777,7 +3766,7 @@ _initializeReflectable() {
_logger.info("Ignoring entry point $currentEntryPoint that does not "
"include the library 'package:reflectable/reflectable.dart'");
if (const bool.fromEnvironment("reflectable.pause.at.exit")) {
_processedEntryPoints.add(currentEntryPoint);
_processedEntryPointCount++;
}
return;
}
Expand All @@ -3792,7 +3781,7 @@ _initializeReflectable() {
if (world == null) {
// Errors have already been reported during `_computeWorld`.
if (const bool.fromEnvironment("reflectable.pause.at.exit")) {
_processedEntryPoints.add(currentEntryPoint);
_processedEntryPointCount++;
}
return;
}
Expand All @@ -3808,7 +3797,7 @@ _initializeReflectable() {
_logger.info("Entry point: $currentEntryPoint has no member "
"called `main`. Skipping.");
if (const bool.fromEnvironment("reflectable.pause.at.exit")) {
_processedEntryPoints.add(currentEntryPoint);
_processedEntryPointCount++;
}
return;
}
Expand All @@ -3820,12 +3809,13 @@ _initializeReflectable() {
_generateNewEntryPoint(world, asset.id, originalEntryPointFilename);
transform.addOutput(new Asset.fromString(asset.id, newEntryPoint));
if (const bool.fromEnvironment("reflectable.pause.at.exit")) {
_processedEntryPoints.add(currentEntryPoint);
_processedEntryPointCount++;
}
_resolver.release();

if (const bool.fromEnvironment("reflectable.pause.at.exit")) {
if (_processedEntryPoints.containsAll(_allEntryPoints)) {
if (_processedEntryPointCount ==
const int.fromEnvironment("reflectable.pause.at.exit.count")) {
print("Transformation complete, pausing at exit.");
developer.debugger();
}
Expand Down
4 changes: 3 additions & 1 deletion reflectable/lib/transformer.dart
Expand Up @@ -7,6 +7,7 @@ library reflectable.transformer;
import 'dart:async';
import 'dart:io';
import 'package:barback/barback.dart';
import 'package:glob/glob.dart';
import 'package:logging/logging.dart';
import 'src/transformer_implementation.dart' as implementation;

Expand Down Expand Up @@ -143,7 +144,8 @@ class ReflectableTransformer extends Transformer
declareOutputs(DeclaringTransform transform) async {
AssetId id = await transform.primaryId;
_entryPoints.forEach((String entryPoint) {
if (id.path.endsWith(entryPoint)) {
Glob glob = new Glob(entryPoint);
if (glob.matches(id.path)) {
transform.declareOutput(id);
AssetId dataId = id.changeExtension("_reflectable_original_main.dart");
transform.declareOutput(dataId);
Expand Down
1 change: 1 addition & 0 deletions reflectable/pubspec.yaml
Expand Up @@ -14,6 +14,7 @@ dependencies:
barback: ^0.15.0
code_transformers: ^0.4.0
dart_style: ^0.2.0
glob: ^1.1.0
logging: ^0.11.0
dev_dependencies:
unittest: ^0.11.0
Expand Down
69 changes: 1 addition & 68 deletions test_reflectable/pubspec.yaml
Expand Up @@ -12,74 +12,7 @@ dev_dependencies:
unittest: ^0.11.0
transformers:
- reflectable:
$include: ["test/*.dart"]
entry_points:
- test/metadata_test.dart
- test/import_reflectable.dart
- test/use_annotation.dart
- test/reflect_test.dart
- test/export_test.dart
- test/use_prefix_test.dart
- test/three_files_test.dart
- test/invoke_test.dart
- test/member_capability_test.dart
- test/annotated_classes_test.dart
- test/reflect_type_test.dart
- test/declarations_test.dart
- test/new_instance_test.dart
- test/proxy_test.dart
- test/polymer_basic_needs_test.dart
- test/serialize_test.dart
- test/new_instance_default_values_test.dart
- test/capabilities_test.dart
- test/basic_test.dart
- test/no_such_capability_test.dart
- test/invoke_capabilities_test.dart
- test/global_quantify_test.dart
- test/name_clash_test.dart
- test/metadata_name_clash_test.dart
- test/invoker_test.dart
- test/invoker_operator_test.dart
- test/type_relations_test.dart
- test/exported_main_test.dart
- test/libraries_test.dart
- test/parameter_mirrors_test.dart
- test/parameter_test.dart
- test/field_test.dart
- test/implicit_getter_setter_test.dart
- test/mixin_test.dart
- test/mixin_application_static_member_test.dart
- test/mixin_application_static_invoke_test.dart
- test/default_values_test.dart
- test/superinterfaces_test.dart
- test/subtype_quantify_test.dart
- test/expanding_generics_test.dart
- test/mixin_static_const_test.dart
- test/metadata_subtype_test.dart
- test/static_members_test.dart
- test/original_prefix_test.dart
- test/reflected_type_test.dart
- test/reflectors_test.dart
- test/meta_reflector_test.dart
- test/meta_reflectors_test.dart
- test/corresponding_setter_test.dart
- test/private_class_test.dart
- test/multi_field_test.dart
- test/class_property_test.dart
- test/type_variable_test.dart
- test/dynamic_reflected_type_test.dart
- test/delegate_test.dart
- test/generic_mixin_test.dart
- test/subtype_test.dart
- test/no_such_method_test.dart
- test/inherited_variable_test.dart
- test/new_instance_optional_arguments_test.dart
- test/new_instance_native_test.dart
- test/new_instance_html_test.dart
- test/mixin_application_test.dart
- test/unused_reflector_test.dart
- test/function_type_annotation_test.dart
- test/prefixed_reflector_test.dart
entry_points: ["test/*_test.dart"]
formatted: true
- $dart2js:
$include: []

0 comments on commit 8936f98

Please sign in to comment.