Skip to content
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

allow customized builder ordering by users #3334

Merged
merged 5 commits into from Jul 12, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion build_config/CHANGELOG.md
@@ -1,6 +1,8 @@
## 1.0.1-dev
## 1.1.0

- Require Dart 2.14
- Support `runsBefore` global configuration for builders. This allows users to
have some control over builder ordering.

## 1.0.0

Expand Down
19 changes: 18 additions & 1 deletion build_config/lib/src/build_target.dart
Expand Up @@ -142,13 +142,29 @@ class GlobalBuilderConfig {
/// Overrides for [options] in release mode.
final Map<String, dynamic> releaseOptions;

/// Builder keys in `$package:$builder` format which should only be run after
/// this Builder.
///
/// This allows the user to configure the ordering of particular builders as
/// necessary, but it is a global option.
///
/// This config is merged with the `runsBefore` configuration of the builder
/// definition, and does not overwrite it.
final List<String> runsBefore;

GlobalBuilderConfig({
Map<String, dynamic>? options,
Map<String, dynamic>? devOptions,
Map<String, dynamic>? releaseOptions,
List<String>? runsBefore,
}) : options = options ?? const {},
devOptions = devOptions ?? const {},
releaseOptions = releaseOptions ?? const {};
releaseOptions = releaseOptions ?? const {},
runsBefore = runsBefore
?.map((builder) =>
normalizeBuilderKeyUsage(builder, currentPackage))
.toList() ??
const [];

factory GlobalBuilderConfig.fromJson(Map json) {
ArgumentError.checkNotNull(json);
Expand All @@ -160,5 +176,6 @@ class GlobalBuilderConfig {
'options': options,
'devOptions': devOptions,
'releaseOptions': releaseOptions,
'runsBefore': runsBefore,
}.toString();
}
12 changes: 10 additions & 2 deletions build_config/lib/src/build_target.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions build_config/pubspec.yaml
@@ -1,5 +1,5 @@
name: build_config
version: 1.0.1-dev
version: 1.1.0
description: Support for parsing `build.yaml` configuration.
repository: https://github.com/dart-lang/build/tree/master/build_config

Expand All @@ -8,7 +8,7 @@ environment:

dependencies:
checked_yaml: ^2.0.0
json_annotation: ^4.3.0
json_annotation: ^4.5.0
path: ^1.8.0
pubspec_parse: ^1.0.0
yaml: ^3.0.0
Expand All @@ -19,3 +19,7 @@ dev_dependencies:
json_serializable: ^6.0.0
term_glyph: ^1.2.0
test: ^1.16.0

dependency_overrides:
build_runner:
path: ../build_runner
5 changes: 5 additions & 0 deletions build_runner/CHANGELOG.md
@@ -1,3 +1,8 @@
## 2.2.0

- Support global 'runs_before' configuration for builders. This allows users
to influence the ordering of builders in their build.

## 2.1.11

- Stop reading or requiring `.packages` files.
Expand Down
Expand Up @@ -132,12 +132,30 @@ Future<BuildScriptInfo> findBuildScriptOptions({
.expand((c) => c.builderDefinitions.values)
.where(_isValidDefinition);

final orderedBuilders = findBuilderOrder(builderDefinitions).toList();
final rootBuildConfig = orderedConfigs.last;
final orderedBuilders =
findBuilderOrder(builderDefinitions, rootBuildConfig.globalOptions)
.toList();

final postProcessBuilderDefinitions = orderedConfigs
.expand((c) => c.postProcessBuilderDefinitions.values)
.where(_isValidDefinition);

// Validate the builder keys in the global builder config, these should always
// refer to actual builders.
var allBuilderKeys = {for (var definition in orderedBuilders) definition.key};
for (var globalBuilderConfig in rootBuildConfig.globalOptions.entries) {
void _checkBuilderKey(String builderKey) {
if (allBuilderKeys.contains(builderKey)) return;
_log.warning(
'Invalid builder key `$builderKey` found in global_options config of '
'build.yaml. This configuration will have no effect.');
}

_checkBuilderKey(globalBuilderConfig.key);
globalBuilderConfig.value.runsBefore.forEach(_checkBuilderKey);
}

final applications = [
for (var builder in orderedBuilders) _applyBuilder(builder),
for (var builder in postProcessBuilderDefinitions)
Expand Down
12 changes: 8 additions & 4 deletions build_runner/lib/src/build_script_generate/builder_ordering.dart
Expand Up @@ -12,12 +12,14 @@ import 'package:graphs/graphs.dart';
/// Builders will be ordered such that their `required_inputs` and `runs_before`
/// constraints are met, but the rest of the ordering is arbitrary.
Iterable<BuilderDefinition> findBuilderOrder(
Iterable<BuilderDefinition> builders) {
Iterable<BuilderDefinition> builders,
Map<String, GlobalBuilderConfig> globalBuilderConfigs) {
final consistentOrderBuilders = builders.toList()
..sort((a, b) => a.key.compareTo(b.key));
Iterable<BuilderDefinition> dependencies(BuilderDefinition parent) =>
consistentOrderBuilders.where((child) =>
_hasInputDependency(parent, child) || _mustRunBefore(parent, child));
_hasInputDependency(parent, child) ||
_mustRunBefore(parent, child, globalBuilderConfigs));
var components = stronglyConnectedComponents<BuilderDefinition>(
consistentOrderBuilders,
dependencies,
Expand All @@ -41,5 +43,7 @@ bool _hasInputDependency(BuilderDefinition parent, BuilderDefinition child) {
}

/// Whether [child] specifies that it wants to run before [parent].
bool _mustRunBefore(BuilderDefinition parent, BuilderDefinition child) =>
child.runsBefore.contains(parent.key);
bool _mustRunBefore(BuilderDefinition parent, BuilderDefinition child,
Map<String, GlobalBuilderConfig?> globalBuilderConfigs) =>
child.runsBefore.contains(parent.key) ||
(globalBuilderConfigs[child.key]?.runsBefore.contains(parent.key) ?? false);
8 changes: 6 additions & 2 deletions build_runner/pubspec.yaml
@@ -1,5 +1,5 @@
name: build_runner
version: 2.1.11
version: 2.2.0
description: A build system for Dart code generation and modular compilation.
repository: https://github.com/dart-lang/build/tree/master/build_runner

Expand All @@ -11,7 +11,7 @@ dependencies:
async: ^2.5.0
analyzer: ">=1.4.0 <5.0.0"
build: ">=2.1.0 <2.4.0"
build_config: ">=1.0.0 <1.1.0"
build_config: ">=1.1.0 <1.2.0"
build_daemon: ^3.1.0
build_resolvers: ^2.0.0
build_runner_core: ^7.2.0
Expand Down Expand Up @@ -52,3 +52,7 @@ dev_dependencies:
test_process: ^2.0.0
_test_common:
path: ../_test_common

dependency_overrides:
build_config:
path: ../build_config
Expand Up @@ -10,8 +10,8 @@ import 'package:test/test.dart';
import 'package:test_descriptor/test_descriptor.dart' as d;

void main() {
group('Builder imports', () {
setUp(() async {
group('validation', () {
setUpAll(() async {
await d.dir('a', [
await pubspec('a', currentIsolateDependencies: [
'build',
Expand All @@ -26,66 +26,94 @@ void main() {
await runPub('a', 'get');
});

test('warn about deprecated ../ style imports', () async {
await d.dir('a', [
d.file('build.yaml', '''
group('of builder imports', () {
test('warn about deprecated ../ style imports', () async {
await d.dir('a', [
d.file('build.yaml', '''
builders:
fake:
import: "../../../tool/builder.dart"
builder_factories: ["myFactory"]
build_extensions: {"foo": ["bar"]}
'''),
]).create();
]).create();

var result = await runPub('a', 'run', args: ['build_runner', 'build']);
expect(result.stderr, isEmpty);
expect(result.stdout,
contains('The `../` import syntax in build.yaml is now deprecated'));
});
var result = await runPub('a', 'run', args: ['build_runner', 'build']);
expect(result.stderr, isEmpty);
expect(
result.stdout,
contains(
'The `../` import syntax in build.yaml is now deprecated'));
});

test('support package relative imports', () async {
await d.dir('a', [
d.file('build.yaml', '''
test('support package relative imports', () async {
await d.dir('a', [
d.file('build.yaml', '''
builders:
fake:
import: "tool/builder.dart"
builder_factories: ["myFactory"]
build_extensions: {"foo": ["bar"]}
'''),
]).create();
]).create();

var result = await runPub('a', 'run', args: ['build_runner', 'build']);
expect(result.stderr, isEmpty);
expect(
result.stdout,
isNot(contains(
'The `../` import syntax in build.yaml is now deprecated')));
var result = await runPub('a', 'run', args: ['build_runner', 'build']);
expect(result.stderr, isEmpty);
expect(
result.stdout,
isNot(contains(
'The `../` import syntax in build.yaml is now deprecated')));

await d.dir('a', [
d.dir('.dart_tool', [
d.dir('build', [
d.dir('entrypoint', [
d.file(
'build.dart', contains("import '../../../tool/builder.dart'"))
await d.dir('a', [
d.dir('.dart_tool', [
d.dir('build', [
d.dir('entrypoint', [
d.file('build.dart',
contains("import '../../../tool/builder.dart'"))
])
])
])
])
]).validate();
});
]).validate();
});

test('warns for builder config that leaves unparseable Dart', () async {
await d.dir('a', [
d.file('build.yaml', '''
test('warns for builder config that leaves unparseable Dart', () async {
await d.dir('a', [
d.file('build.yaml', '''
builders:
fake:
import: "tool/builder.dart"
builder_factories: ["not an identifier"]
build_extensions: {"foo": ["bar"]}
''')
]).create();
var result = await runPub('a', 'run', args: ['build_runner', 'build']);
expect(result.stderr, isEmpty);
expect(result.stdout, contains('could not be parsed'));
});
});

test('checks builder keys in global_options', () async {
await d.dir('a', [
d.file('build.yaml', '''
global_options:
a:a:
runs_before:
- b:b
'''),
]).create();

var result = await runPub('a', 'run', args: ['build_runner', 'build']);
expect(result.stderr, isEmpty);
expect(result.stdout, contains('could not be parsed'));
expect(
result.stdout,
allOf(
contains(
'Invalid builder key `a:a` found in global_options config of '
'build.yaml. This configuration will have no effect.'),
contains(
'Invalid builder key `b:b` found in global_options config of '
'build.yaml. This configuration will have no effect.'),
));
});
});

Expand Down