Warn about configuring builders with --config (#2968)

Closes #2942

This also cleans up some `dart:io` imports to use the asset reader interface instead, which was a requirement for unit testing this but is a good general cleanup.

Author: jakemac53

build_runner/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 1.10.12-dev
+
+- Add a warning if a `builders` section is found in when parsing an overriden
+  build.yaml file via the `--config` flag.
+
 ## 1.10.11
 
 - Fix handling of `build.yaml` `generateFor` default config for values including

build_runner/lib/src/build_script_generate/build_script_generate.dart

@@ -66,8 +66,8 @@ Future<Iterable<Expression>> _findBuilderApplications() async {
     equals: (a, b) => a.name == b.name,
     hashCode: (n) => n.name.hashCode,
   ).expand((c) => c);
-  final buildConfigOverrides =
-      await findBuildConfigOverrides(packageGraph, null);
+  final buildConfigOverrides = await findBuildConfigOverrides(
+      packageGraph, null, FileBasedAssetReader(packageGraph));
   Future<BuildConfig> _packageBuildConfig(PackageNode package) async {
     if (buildConfigOverrides.containsKey(package.name)) {
       return buildConfigOverrides[package.name];

build_runner/lib/src/daemon/daemon_builder.dart

@@ -191,8 +191,8 @@ class BuildRunnerDaemonBuilder implements DaemonBuilder {
     var logSubscription =
         LogSubscription(environment, verbose: daemonOptions.verbose);
 
-    var overrideBuildConfig =
-        await findBuildConfigOverrides(packageGraph, daemonOptions.configKey);
+    var overrideBuildConfig = await findBuildConfigOverrides(
+        packageGraph, daemonOptions.configKey, daemonEnvironment.reader);
 
     var buildOptions = await BuildOptions.create(
       logSubscription,

build_runner/lib/src/entrypoint/doctor.dart

@@ -51,8 +51,8 @@ class DoctorCommand extends BuildRunnerCommand {
 
   Future<Map<String, BuilderDefinition>> _loadBuilderDefinitions() async {
     final packageGraph = await PackageGraph.forThisPackage();
-    final buildConfigOverrides =
-        await findBuildConfigOverrides(packageGraph, null);
+    final buildConfigOverrides = await findBuildConfigOverrides(
+        packageGraph, null, FileBasedAssetReader(packageGraph));
     Future<BuildConfig> _packageBuildConfig(PackageNode package) async {
       if (buildConfigOverrides.containsKey(package.name)) {
         return buildConfigOverrides[package.name];

build_runner/lib/src/generate/build.dart

@@ -87,8 +87,8 @@ Future<BuildResult> build(List<BuilderApplication> builders,
     deleteFilesByDefault: deleteFilesByDefault,
     packageGraph: packageGraph,
     skipBuildScriptCheck: skipBuildScriptCheck,
-    overrideBuildConfig:
-        await findBuildConfigOverrides(packageGraph, configKey),
+    overrideBuildConfig: await findBuildConfigOverrides(
+        packageGraph, configKey, environment.reader),
     enableLowResourcesMode: enableLowResourcesMode,
     trackPerformance: trackPerformance,
     logPerformanceDir: logPerformanceDir,

build_runner/lib/src/generate/watch_impl.dart

@@ -66,8 +66,8 @@ Future<ServeHandler> watch(
       onLog: onLog ?? stdIOLogListener(assumeTty: assumeTty, verbose: verbose));
   var logSubscription =
       LogSubscription(environment, verbose: verbose, logLevel: logLevel);
-  overrideBuildConfig ??=
-      await findBuildConfigOverrides(packageGraph, configKey);
+  overrideBuildConfig ??= await findBuildConfigOverrides(
+      packageGraph, configKey, environment.reader);
   var options = await BuildOptions.create(
     logSubscription,
     deleteFilesByDefault: deleteFilesByDefault,

build_runner/lib/src/package_graph/build_config_overrides.dart

@@ -3,8 +3,8 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:io';
 
+import 'package:build/build.dart';
 import 'package:build_config/build_config.dart';
 import 'package:build_runner_core/build_runner_core.dart';
 import 'package:glob/glob.dart';
@@ -14,43 +14,49 @@ import 'package:path/path.dart' as p;
 final _log = Logger('BuildConfigOverrides');
 
 Future<Map<String, BuildConfig>> findBuildConfigOverrides(
-    PackageGraph packageGraph, String configKey) async {
+    PackageGraph packageGraph,
+    String configKey,
+    RunnerAssetReader reader) async {
   final configs = <String, BuildConfig>{};
-  final configFiles = Glob('*.build.yaml').list();
-  await for (final file in configFiles) {
-    if (file is File) {
-      final packageName = p.basename(file.path).split('.').first;
-      final packageNode = packageGraph.allPackages[packageName];
-      if (packageNode == null) {
-        _log.warning('A build config override is provided for $packageName but '
-            'that package does not exist. '
-            'Remove the ${p.basename(file.path)} override or add a dependency '
-            'on $packageName.');
-        continue;
-      }
-      final yaml = file.readAsStringSync();
-      final config = BuildConfig.parse(
-        packageName,
-        packageNode.dependencies.map((n) => n.name),
-        yaml,
-        configYamlPath: file.path,
-      );
-      configs[packageName] = config;
+  final configFiles =
+      reader.findAssets(Glob('*.build.yaml'), package: packageGraph.root.name);
+  await for (final id in configFiles) {
+    final packageName = p.basename(id.path).split('.').first;
+    final packageNode = packageGraph.allPackages[packageName];
+    if (packageNode == null) {
+      _log.warning('A build config override is provided for $packageName but '
+          'that package does not exist. '
+          'Remove the ${p.basename(id.path)} override or add a dependency '
+          'on $packageName.');
+      continue;
     }
+    final yaml = await reader.readAsString(id);
+    final config = BuildConfig.parse(
+      packageName,
+      packageNode.dependencies.map((n) => n.name),
+      yaml,
+      configYamlPath: id.path,
+    );
+    configs[packageName] = config;
   }
   if (configKey != null) {
-    final file = File('build.$configKey.yaml');
-    if (!file.existsSync()) {
-      _log.warning('Cannot find build.$configKey.yaml for specified config.');
+    final id = AssetId(packageGraph.root.name, 'build.$configKey.yaml');
+    if (!await reader.canRead(id)) {
+      _log.warning('Cannot find ${id.path} for specified config.');
       throw CannotBuildException();
     }
-    final yaml = file.readAsStringSync();
+    final yaml = await reader.readAsString(id);
     final config = BuildConfig.parse(
       packageGraph.root.name,
       packageGraph.root.dependencies.map((n) => n.name),
       yaml,
-      configYamlPath: file.path,
+      configYamlPath: id.path,
     );
+    if (config.builderDefinitions.isNotEmpty) {
+      _log.warning(
+          'Ignoring `builders` configuration in `build.$configKey.yaml` - '
+          'overriding builder configuration is not supported.');
+    }
     configs[packageGraph.root.name] = config;
   }
   return configs;

build_runner/pubspec.yaml

@@ -1,5 +1,5 @@
 name: build_runner
-version: 1.10.11
+version: 1.10.12-dev
 description: A build system for Dart code generation and modular compilation.
 repository: https://github.com/dart-lang/build/tree/master/build_runner
 

build_runner/test/generate/build_test.dart

@@ -0,0 +1,97 @@
+// Copyright (c) 2021, 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 'package:logging/logging.dart';
+import 'package:path/path.dart' as path;
+import 'package:test/test.dart';
+
+import 'package:build_runner_core/build_runner_core.dart';
+import 'package:build_runner/src/generate/build.dart' as build_impl;
+import 'package:build_test/build_test.dart';
+
+import 'package:_test_common/common.dart';
+import 'package:_test_common/package_graphs.dart';
+
+void main() {
+  // Basic phases/phase groups which get used in many tests
+  final copyABuildApplication = applyToRoot(
+      TestBuilder(buildExtensions: appendExtension('.copy', from: '.txt')));
+  final packageConfigId = makeAssetId('a|.dart_tool/package_config.json');
+  InMemoryRunnerAssetWriter writer;
+
+  setUp(() async {
+    writer = InMemoryRunnerAssetWriter();
+    await writer.writeAsString(makeAssetId('a|.packages'), '''
+# Fake packages file
+a:file://fake/pkg/path
+''');
+    await writer.writeAsString(packageConfigId, jsonEncode(_packageConfig));
+  });
+
+  group('--config', () {
+    test('warns override config defines builders', () async {
+      var logs = <LogRecord>[];
+      final packageGraph = buildPackageGraph({
+        rootPackage('a', path: path.absolute('a')): [],
+      });
+      var result = await _doBuild([
+        copyABuildApplication
+      ], {
+        'a|build.yaml': '',
+        'a|build.cool.yaml': '''
+builders:
+  fake:
+    import: "a.dart"
+    builder_factories: ["myFactory"]
+    build_extensions: {"a": ["b"]}
+'''
+      }, writer,
+          configKey: 'cool',
+          logLevel: Level.WARNING,
+          onLog: logs.add,
+          packageGraph: packageGraph);
+      expect(result.status, BuildStatus.success);
+      expect(
+          logs.first.message,
+          contains('Ignoring `builders` configuration in `build.cool.yaml` - '
+              'overriding builder configuration is not supported.'));
+    });
+  });
+}
+
+Future<BuildResult> _doBuild(List<BuilderApplication> builders,
+    Map<String, String> inputs, InMemoryRunnerAssetWriter writer,
+    {PackageGraph packageGraph,
+    void Function(LogRecord) onLog,
+    Level logLevel,
+    String configKey}) async {
+  onLog ??= (_) {};
+  inputs.forEach((serializedId, contents) {
+    writer.writeAsString(makeAssetId(serializedId), contents);
+  });
+  packageGraph ??=
+      buildPackageGraph({rootPackage('a', path: path.absolute('a')): []});
+  final reader = InMemoryRunnerAssetReader.shareAssetCache(writer.assets,
+      rootPackage: packageGraph.root.name);
+
+  return await build_impl.build(builders,
+      configKey: configKey,
+      deleteFilesByDefault: true,
+      reader: reader,
+      writer: writer,
+      packageGraph: packageGraph,
+      logLevel: logLevel,
+      onLog: onLog,
+      skipBuildScriptCheck: true);
+}
+
+const _packageConfig = {
+  'configVersion': 2,
+  'packages': [
+    {'name': 'a', 'rootUri': 'file://fake/pkg/path', 'packageUri': 'lib/'},
+  ],
+};