From d735c6e5ce195f8af6d9868ee8aa52d9aacde78c Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Mon, 11 Jul 2022 14:49:56 -0700 Subject: [PATCH 1/5] allow customized builder ordering by users --- build_config/CHANGELOG.md | 4 +- build_config/lib/src/build_target.dart | 18 ++- build_config/lib/src/build_target.g.dart | 12 +- build_config/pubspec.yaml | 4 +- build_runner/CHANGELOG.md | 5 + .../build_script_generate.dart | 5 +- .../builder_ordering.dart | 12 +- build_runner/pubspec.yaml | 6 +- .../builder_ordering_test.dart | 107 ++++++++++++++++-- docs/build_yaml_format.md | 1 + docs/faq.md | 12 ++ 11 files changed, 166 insertions(+), 20 deletions(-) diff --git a/build_config/CHANGELOG.md b/build_config/CHANGELOG.md index 5136eb889..0d4038e40 100644 --- a/build_config/CHANGELOG.md +++ b/build_config/CHANGELOG.md @@ -1,6 +1,8 @@ -## 1.0.1-dev +## 1.1.0-dev - Require Dart 2.14 +- Support `runsBefore` global configuration for builders. This allows users to + have some control over builder ordering. ## 1.0.0 diff --git a/build_config/lib/src/build_target.dart b/build_config/lib/src/build_target.dart index b956126bb..5f4efee25 100644 --- a/build_config/lib/src/build_target.dart +++ b/build_config/lib/src/build_target.dart @@ -142,13 +142,29 @@ class GlobalBuilderConfig { /// Overrides for [options] in release mode. final Map 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 runsBefore; + GlobalBuilderConfig({ Map? options, Map? devOptions, Map? releaseOptions, + List? 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); diff --git a/build_config/lib/src/build_target.g.dart b/build_config/lib/src/build_target.g.dart index c1cf9725a..478dba841 100644 --- a/build_config/lib/src/build_target.g.dart +++ b/build_config/lib/src/build_target.g.dart @@ -88,7 +88,12 @@ GlobalBuilderConfig _$GlobalBuilderConfigFromJson(Map json) => $checkedCreate( ($checkedConvert) { $checkKeys( json, - allowedKeys: const ['options', 'dev_options', 'release_options'], + allowedKeys: const [ + 'options', + 'dev_options', + 'release_options', + 'runs_before' + ], ); final val = GlobalBuilderConfig( options: $checkedConvert( @@ -106,11 +111,14 @@ GlobalBuilderConfig _$GlobalBuilderConfigFromJson(Map json) => $checkedCreate( (v) => (v as Map?)?.map( (k, e) => MapEntry(k as String, e), )), + runsBefore: $checkedConvert('runs_before', + (v) => (v as List?)?.map((e) => e as String).toList()), ); return val; }, fieldKeyMap: const { 'devOptions': 'dev_options', - 'releaseOptions': 'release_options' + 'releaseOptions': 'release_options', + 'runsBefore': 'runs_before' }, ); diff --git a/build_config/pubspec.yaml b/build_config/pubspec.yaml index 7b518490d..46179b7a2 100644 --- a/build_config/pubspec.yaml +++ b/build_config/pubspec.yaml @@ -1,5 +1,5 @@ name: build_config -version: 1.0.1-dev +version: 1.1.0-dev description: Support for parsing `build.yaml` configuration. repository: https://github.com/dart-lang/build/tree/master/build_config @@ -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 diff --git a/build_runner/CHANGELOG.md b/build_runner/CHANGELOG.md index 457f5d3d4..85f6ab230 100644 --- a/build_runner/CHANGELOG.md +++ b/build_runner/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.2.0-dev + +- Support global 'runs_before' configuration for builders. This allows users + to specify the ordering of builders in their build. + ## 2.1.11 - Stop reading or requiring `.packages` files. diff --git a/build_runner/lib/src/build_script_generate/build_script_generate.dart b/build_runner/lib/src/build_script_generate/build_script_generate.dart index 60e74e361..40e974d07 100644 --- a/build_runner/lib/src/build_script_generate/build_script_generate.dart +++ b/build_runner/lib/src/build_script_generate/build_script_generate.dart @@ -132,7 +132,10 @@ Future 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) diff --git a/build_runner/lib/src/build_script_generate/builder_ordering.dart b/build_runner/lib/src/build_script_generate/builder_ordering.dart index c6816970e..3c0075b6e 100644 --- a/build_runner/lib/src/build_script_generate/builder_ordering.dart +++ b/build_runner/lib/src/build_script_generate/builder_ordering.dart @@ -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 findBuilderOrder( - Iterable builders) { + Iterable builders, + Map globalBuilderConfigs) { final consistentOrderBuilders = builders.toList() ..sort((a, b) => a.key.compareTo(b.key)); Iterable dependencies(BuilderDefinition parent) => consistentOrderBuilders.where((child) => - _hasInputDependency(parent, child) || _mustRunBefore(parent, child)); + _hasInputDependency(parent, child) || + _mustRunBefore(parent, child, globalBuilderConfigs)); var components = stronglyConnectedComponents( consistentOrderBuilders, dependencies, @@ -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 globalBuilderConfigs) => + child.runsBefore.contains(parent.key) || + (globalBuilderConfigs[child.key]?.runsBefore.contains(parent.key) ?? false); diff --git a/build_runner/pubspec.yaml b/build_runner/pubspec.yaml index 25006c515..76c058184 100644 --- a/build_runner/pubspec.yaml +++ b/build_runner/pubspec.yaml @@ -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 @@ -52,3 +52,7 @@ dev_dependencies: test_process: ^2.0.0 _test_common: path: ../_test_common + +dependency_overrides: + build_config: + path: ../build_config diff --git a/build_runner/test/build_script_generate/builder_ordering_test.dart b/build_runner/test/build_script_generate/builder_ordering_test.dart index db7a8d085..94554424b 100644 --- a/build_runner/test/build_script_generate/builder_ordering_test.dart +++ b/build_runner/test/build_script_generate/builder_ordering_test.dart @@ -3,35 +3,125 @@ // BSD-style license that can be found in the LICENSE file. import 'package:_test_common/build_configs.dart'; +import 'package:build_config/build_config.dart'; import 'package:build_runner/src/build_script_generate/builder_ordering.dart'; import 'package:test/test.dart'; void main() { group('Builder ordering', () { - test('orders builders with `runs_before`', () async { + test('orders builders with "runs_before"', () async { final buildConfigs = parseBuildConfigs({ 'a': { 'builders': { - 'runs_second': { + 'a': { 'builder_factories': ['createBuilder'], 'build_extensions': >{}, 'target': '', 'import': '', }, - 'runs_first': { + 'b': { + 'builder_factories': ['createBuilder'], + 'build_extensions': {}, + 'target': '', + 'import': '', + 'runs_before': [':a'], + }, + 'c': { 'builder_factories': ['createBuilder'], 'build_extensions': {}, 'target': '', 'import': '', - 'runs_before': [':runs_second'], }, } } }); final orderedBuilders = findBuilderOrder( - buildConfigs.values.expand((v) => v.builderDefinitions.values)); + buildConfigs.values.expand((v) => v.builderDefinitions.values), {}); final orderedKeys = orderedBuilders.map((b) => b.key); - expect(orderedKeys, ['a:runs_first', 'a:runs_second']); + expect(orderedKeys, ['a:b', 'a:a', 'a:c']); + }); + + test('orders builders with global "runs_before"', () { + runInBuildConfigZone(() { + final buildConfigs = parseBuildConfigs({ + 'a': { + 'builders': { + 'a': { + 'builder_factories': ['createBuilder'], + 'build_extensions': >{}, + 'target': '', + 'import': '', + }, + 'b': { + 'builder_factories': ['createBuilder'], + 'build_extensions': {}, + 'target': '', + 'import': '', + 'runs_before': [':a'], + }, + 'c': { + 'builder_factories': ['createBuilder'], + 'build_extensions': {}, + 'target': '', + 'import': '', + }, + } + } + }); + final orderedBuilders = findBuilderOrder( + buildConfigs.values.expand((v) => v.builderDefinitions.values), { + 'a:c': GlobalBuilderConfig.fromJson({ + 'runs_before': ['a:a'], + }), + }); + final orderedKeys = orderedBuilders.map((b) => b.key); + expect(orderedKeys, [ + 'a:b', + 'a:c', + 'a:a', + ]); + }, 'a', []); + }); + + test('orders builders with global and local "runs_before"', () { + runInBuildConfigZone(() { + final buildConfigs = parseBuildConfigs({ + 'a': { + 'builders': { + 'a': { + 'builder_factories': ['createBuilder'], + 'build_extensions': >{}, + 'target': '', + 'import': '', + }, + 'b': { + 'builder_factories': ['createBuilder'], + 'build_extensions': {}, + 'target': '', + 'import': '', + }, + 'c': { + 'builder_factories': ['createBuilder'], + 'build_extensions': {}, + 'target': '', + 'import': '', + }, + } + } + }); + final orderedBuilders = findBuilderOrder( + buildConfigs.values.expand((v) => v.builderDefinitions.values), { + 'a:b': GlobalBuilderConfig.fromJson({ + 'runs_before': ['a:a'], + }), + }); + final orderedKeys = orderedBuilders.map((b) => b.key); + expect(orderedKeys, [ + 'a:b', + 'a:a', + 'a:c', + ]); + }, 'a', []); }); test('orders builders with `required_inputs`', () async { @@ -57,7 +147,7 @@ void main() { } }); final orderedBuilders = findBuilderOrder( - buildConfigs.values.expand((v) => v.builderDefinitions.values)); + buildConfigs.values.expand((v) => v.builderDefinitions.values), {}); final orderedKeys = orderedBuilders.map((b) => b.key); expect(orderedKeys, ['a:runs_first', 'a:runs_second']); }); @@ -87,7 +177,8 @@ void main() { }); expect( () => findBuilderOrder( - buildConfigs.values.expand((v) => v.builderDefinitions.values)), + buildConfigs.values.expand((v) => v.builderDefinitions.values), + {}), throwsA(anything)); }); }); diff --git a/docs/build_yaml_format.md b/docs/build_yaml_format.md index eaaf89f2e..6c95a34c8 100644 --- a/docs/build_yaml_format.md +++ b/docs/build_yaml_format.md @@ -85,6 +85,7 @@ key | value | default options | [BuilderOptions](#builderoptions) | none dev_options | [BuilderOptions](#builderoptions) | none release_options | [BuilderOptions](#builderoptions) | none +runs_before | List<[BuilderKey](#builderkey)> | none ## BuilderOptions diff --git a/docs/faq.md b/docs/faq.md index 81572e54b..c8f439c97 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -361,3 +361,15 @@ a schema URL. The configuration should look similar to this: ![Schema configuration for IDEA based IDEs](images/idea_schema_config.png) [YAML extension]: https://marketplace.visualstudio.com/items?itemName=redhat.vscode-yaml + +## How can I adjust builder ordering? + +You can configure certain builders to run before other builders globally, using the +`global_options` configuration in your `build.yaml` file: + +```yaml +global_options: + some_package:some_builder: + runs_before: + - some_other_package:some_other_builder +``` From 7ca9d2c95a6410180e07700c44fa9a278b0d329f Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Mon, 11 Jul 2022 14:57:50 -0700 Subject: [PATCH 2/5] add override on build_runner --- build_config/pubspec.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build_config/pubspec.yaml b/build_config/pubspec.yaml index 46179b7a2..8d3e406fd 100644 --- a/build_config/pubspec.yaml +++ b/build_config/pubspec.yaml @@ -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 From 25c7d8cfeab6a4ac389ff39a68d0e739ecc79102 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Tue, 12 Jul 2022 07:11:19 -0700 Subject: [PATCH 3/5] Update build_runner/CHANGELOG.md Co-authored-by: Nate Bosch --- build_runner/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build_runner/CHANGELOG.md b/build_runner/CHANGELOG.md index 85f6ab230..865f94e99 100644 --- a/build_runner/CHANGELOG.md +++ b/build_runner/CHANGELOG.md @@ -1,7 +1,7 @@ ## 2.2.0-dev - Support global 'runs_before' configuration for builders. This allows users - to specify the ordering of builders in their build. + to influence the ordering of builders in their build. ## 2.1.11 From 02954e0fd128920a6abf6a6a3897bcab72b6a4cc Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 12 Jul 2022 09:33:11 -0700 Subject: [PATCH 4/5] warn about invalid builder keys in global_options --- build_config/lib/src/build_target.dart | 1 + .../build_script_generate.dart | 15 +++ .../build_script_generate_test.dart | 96 ++++++++++++------- 3 files changed, 78 insertions(+), 34 deletions(-) diff --git a/build_config/lib/src/build_target.dart b/build_config/lib/src/build_target.dart index 5f4efee25..29fe9b6f6 100644 --- a/build_config/lib/src/build_target.dart +++ b/build_config/lib/src/build_target.dart @@ -176,5 +176,6 @@ class GlobalBuilderConfig { 'options': options, 'devOptions': devOptions, 'releaseOptions': releaseOptions, + 'runsBefore': runsBefore, }.toString(); } diff --git a/build_runner/lib/src/build_script_generate/build_script_generate.dart b/build_runner/lib/src/build_script_generate/build_script_generate.dart index 40e974d07..4ae48a85c 100644 --- a/build_runner/lib/src/build_script_generate/build_script_generate.dart +++ b/build_runner/lib/src/build_script_generate/build_script_generate.dart @@ -141,6 +141,21 @@ Future findBuildScriptOptions({ .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) diff --git a/build_runner/test/build_script_generate/build_script_generate_test.dart b/build_runner/test/build_script_generate/build_script_generate_test.dart index 7f73ffae8..353d89f76 100644 --- a/build_runner/test/build_script_generate/build_script_generate_test.dart +++ b/build_runner/test/build_script_generate/build_script_generate_test.dart @@ -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', @@ -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.'), + )); }); }); From 76459060a967d02b9982c111616ac0dfcd95985e Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 12 Jul 2022 09:40:07 -0700 Subject: [PATCH 5/5] prep for release --- build_config/CHANGELOG.md | 2 +- build_config/pubspec.yaml | 2 +- build_runner/CHANGELOG.md | 2 +- build_runner/pubspec.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/build_config/CHANGELOG.md b/build_config/CHANGELOG.md index 0d4038e40..299e432b9 100644 --- a/build_config/CHANGELOG.md +++ b/build_config/CHANGELOG.md @@ -1,4 +1,4 @@ -## 1.1.0-dev +## 1.1.0 - Require Dart 2.14 - Support `runsBefore` global configuration for builders. This allows users to diff --git a/build_config/pubspec.yaml b/build_config/pubspec.yaml index 8d3e406fd..400d946c2 100644 --- a/build_config/pubspec.yaml +++ b/build_config/pubspec.yaml @@ -1,5 +1,5 @@ name: build_config -version: 1.1.0-dev +version: 1.1.0 description: Support for parsing `build.yaml` configuration. repository: https://github.com/dart-lang/build/tree/master/build_config diff --git a/build_runner/CHANGELOG.md b/build_runner/CHANGELOG.md index 865f94e99..0bd8ba49b 100644 --- a/build_runner/CHANGELOG.md +++ b/build_runner/CHANGELOG.md @@ -1,4 +1,4 @@ -## 2.2.0-dev +## 2.2.0 - Support global 'runs_before' configuration for builders. This allows users to influence the ordering of builders in their build. diff --git a/build_runner/pubspec.yaml b/build_runner/pubspec.yaml index 76c058184..a9d1b6496 100644 --- a/build_runner/pubspec.yaml +++ b/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