Avoid depending on files from build_system/targets other than from top level entrypoints in flutter_tools. (#142760)

Add a new `BuildTargets` class that provides commonly used build targets. And avoid importing files from `build_system/targets` except from the top level entrypoints or from top level commands.

Also move `scene_importer.dart` and `shader_compiler.dart` into `build_system/tools` because they are not `Target` classes, but wrapper for certain tools.

With this change, we can ignore all files in `build_system/targets` internally and make PR #142709 easier to land internally. See cl/603434066 for the corresponding internal change.

Related to:
https://github.com/flutter/flutter/pull/142709
https://github.com/flutter/flutter/issues/142041

Also note that I have opted to add a new variable in `globals.dart` for `BuildTargets` in this PR, but I know that we are trying to get rid of globals. Several alternatives that I was considering:

1. Add a new field in `BuildSystem` that returns a `BuildTargets` instance. Since `BuildSystem` is already in `globals`, we can access build targets using `globals.buildSystem.buildTargets` without adding a new global variable.
2. Properly inject the `BuildTargetsImpl` instance from the top level `executable.dart` and top level commands.

Let me know if you want me to do one of the above instead. Thanks!
This commit is contained in:
Lau Ching Jun 2024-02-02 10:23:08 -08:00 committed by GitHub
parent b209125d11
commit ac7879e2aa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
30 changed files with 151 additions and 45 deletions

View file

@ -11,6 +11,7 @@ import 'src/base/platform.dart';
import 'src/base/template.dart';
import 'src/base/terminal.dart';
import 'src/base/user_messages.dart';
import 'src/build_system/build_targets.dart';
import 'src/cache.dart';
import 'src/commands/analyze.dart';
import 'src/commands/assemble.dart';
@ -47,6 +48,7 @@ import 'src/devtools_launcher.dart';
import 'src/features.dart';
import 'src/globals.dart' as globals;
// Files in `isolated` are intentionally excluded from google3 tooling.
import 'src/isolated/build_targets.dart';
import 'src/isolated/mustache_template.dart';
import 'src/isolated/resident_web_runner.dart';
import 'src/pre_run_validator.dart';
@ -110,6 +112,7 @@ Future<void> main(List<String> args) async {
logger: globals.logger,
botDetector: globals.botDetector,
),
BuildTargets: () => const BuildTargetsImpl(),
Logger: () {
final LoggerFactory loggerFactory = LoggerFactory(
outputPreferences: globals.outputPreferences,

View file

@ -0,0 +1,58 @@
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import '../base/file_system.dart';
import '../web/compiler_config.dart';
import './build_system.dart';
/// Commonly used build [Target]s.
abstract class BuildTargets {
const BuildTargets();
Target get copyFlutterBundle;
Target get releaseCopyFlutterBundle;
Target get generateLocalizationsTarget;
Target get dartPluginRegistrantTarget;
Target webServiceWorker(FileSystem fileSystem, List<WebCompilerConfig> compileConfigs);
}
/// BuildTargets that return NoOpTarget for every action.
class NoOpBuildTargets extends BuildTargets {
const NoOpBuildTargets();
@override
Target get copyFlutterBundle => const _NoOpTarget();
@override
Target get releaseCopyFlutterBundle => const _NoOpTarget();
@override
Target get generateLocalizationsTarget => const _NoOpTarget();
@override
Target get dartPluginRegistrantTarget => const _NoOpTarget();
@override
Target webServiceWorker(FileSystem fileSystem, List<WebCompilerConfig> compileConfigs) => const _NoOpTarget();
}
/// A [Target] that does nothing.
class _NoOpTarget extends Target {
const _NoOpTarget();
@override
String get name => 'no_op';
@override
List<Source> get inputs => const <Source>[];
@override
List<Source> get outputs => const <Source>[];
@override
List<Target> get dependencies => const <Target>[];
@override
Future<void> build(Environment environment) async {}
}

View file

@ -12,10 +12,10 @@ import '../../convert.dart';
import '../../devfs.dart';
import '../build_system.dart';
import '../depfile.dart';
import '../tools/scene_importer.dart';
import '../tools/shader_compiler.dart';
import 'common.dart';
import 'icon_tree_shaker.dart';
import 'scene_importer.dart';
import 'shader_compiler.dart';
/// A helper function to copy an asset bundle into an [environment]'s output
/// directory.

View file

@ -16,12 +16,12 @@ import '../../globals.dart' as globals show xcode;
import '../build_system.dart';
import '../depfile.dart';
import '../exceptions.dart';
import '../tools/shader_compiler.dart';
import 'assets.dart';
import 'dart_plugin_registrant.dart';
import 'icon_tree_shaker.dart';
import 'localizations.dart';
import 'native_assets.dart';
import 'shader_compiler.dart';
/// Copies the pre-built flutter bundle.
// This is a one-off rule for implementing build bundle in terms of assemble.

View file

@ -20,10 +20,10 @@ import '../../reporting/reporting.dart';
import '../build_system.dart';
import '../depfile.dart';
import '../exceptions.dart';
import '../tools/shader_compiler.dart';
import 'assets.dart';
import 'common.dart';
import 'icon_tree_shaker.dart';
import 'shader_compiler.dart';
/// Supports compiling a dart kernel file to an assembly file.
///

View file

@ -29,15 +29,6 @@ import '../exceptions.dart';
import 'assets.dart';
import 'localizations.dart';
/// Whether the application has web plugins.
const String kHasWebPlugins = 'HasWebPlugins';
/// Base href to set in index.html in flutter build command
const String kBaseHref = 'baseHref';
/// The caching strategy to use for service worker generation.
const String kServiceWorkerStrategy = 'ServiceWorkerStrategy';
@visibleForTesting
List<String> updateDartDefines(List<String> dartDefines, WebRendererMode webRenderer) {
final Set<String> dartDefinesSet = dartDefines.toSet();

View file

@ -92,7 +92,7 @@ class SceneImporter {
/// See [Target.inputs].
static const List<Source> inputs = <Source>[
Source.pattern(
'{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/targets/scene_importer.dart'),
'{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/tools/scene_importer.dart'),
Source.hostArtifact(HostArtifact.scenec),
];

View file

@ -136,7 +136,7 @@ class ShaderCompiler {
///
/// See [Target.inputs].
static const List<Source> inputs = <Source>[
Source.pattern('{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/targets/shader_compiler.dart'),
Source.pattern('{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/tools/shader_compiler.dart'),
Source.hostArtifact(HostArtifact.impellerc),
];

View file

@ -12,9 +12,8 @@ import 'base/logger.dart';
import 'build_info.dart';
import 'build_system/build_system.dart';
import 'build_system/depfile.dart';
import 'build_system/targets/common.dart';
import 'build_system/targets/scene_importer.dart';
import 'build_system/targets/shader_compiler.dart';
import 'build_system/tools/scene_importer.dart';
import 'build_system/tools/shader_compiler.dart';
import 'bundle.dart';
import 'cache.dart';
import 'devfs.dart';
@ -78,8 +77,8 @@ class BundleBuilder {
generateDartPluginRegistry: true,
);
final Target target = buildInfo.mode == BuildMode.debug
? const CopyFlutterBundle()
: const ReleaseCopyFlutterBundle();
? globals.buildTargets.copyFlutterBundle
: globals.buildTargets.releaseCopyFlutterBundle;
final BuildResult result = await buildSystem.build(target, environment);
if (!result.success) {

View file

@ -544,6 +544,7 @@ abstract class CreateBase extends FlutterCommand {
await generateLocalizationsSyntheticPackage(
environment: environment,
buildSystem: globals.buildSystem,
buildTargets: globals.buildTargets,
);
}
final List<SupportedPlatform> platformsForMigrateConfig = <SupportedPlatform>[SupportedPlatform.root];

View file

@ -307,6 +307,7 @@ class PackagesGetCommand extends FlutterCommand {
await generateLocalizationsSyntheticPackage(
environment: environment,
buildSystem: globals.buildSystem,
buildTargets: globals.buildTargets,
);
} else if (rootProject.directory.childFile('l10n.yaml').existsSync()) {
final Environment environment = Environment(

View file

@ -8,11 +8,12 @@ import '../base/common.dart';
import '../base/file_system.dart';
import '../base/utils.dart';
import '../build_system/build_system.dart';
import '../build_system/targets/localizations.dart';
import '../build_system/build_targets.dart';
Future<void> generateLocalizationsSyntheticPackage({
required Environment environment,
required BuildSystem buildSystem,
required BuildTargets buildTargets,
}) async {
final FileSystem fileSystem = environment.fileSystem;
@ -54,7 +55,7 @@ Future<void> generateLocalizationsSyntheticPackage({
}
final BuildResult result = await buildSystem.build(
const GenerateLocalizationsTarget(),
buildTargets.generateLocalizationsTarget,
environment,
);

View file

@ -15,8 +15,8 @@ import 'base/logger.dart';
import 'base/net.dart';
import 'base/os.dart';
import 'build_info.dart';
import 'build_system/targets/scene_importer.dart';
import 'build_system/targets/shader_compiler.dart';
import 'build_system/tools/scene_importer.dart';
import 'build_system/tools/shader_compiler.dart';
import 'compile.dart';
import 'convert.dart' show base64, utf8;
import 'vmservice.dart';

View file

@ -27,6 +27,7 @@ import 'base/terminal.dart';
import 'base/time.dart';
import 'base/user_messages.dart';
import 'build_system/build_system.dart';
import 'build_system/build_targets.dart';
import 'cache.dart';
import 'custom_devices/custom_devices_config.dart';
import 'device.dart';
@ -54,6 +55,7 @@ import 'version.dart';
Artifacts? get artifacts => context.get<Artifacts>();
BuildSystem get buildSystem => context.get<BuildSystem>()!;
BuildTargets get buildTargets => context.get<BuildTargets>()!;
Cache get cache => context.get<Cache>()!;
CocoaPodsValidator? get cocoapodsValidator => context.get<CocoaPodsValidator>();
Config get config => context.get<Config>()!;

View file

@ -0,0 +1,32 @@
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import '../base/file_system.dart';
import '../build_system/build_system.dart';
import '../build_system/build_targets.dart';
import '../build_system/targets/common.dart';
import '../build_system/targets/dart_plugin_registrant.dart';
import '../build_system/targets/localizations.dart';
import '../build_system/targets/web.dart';
import '../web/compiler_config.dart';
class BuildTargetsImpl extends BuildTargets {
const BuildTargetsImpl();
@override
Target get copyFlutterBundle => const CopyFlutterBundle();
@override
Target get releaseCopyFlutterBundle => const ReleaseCopyFlutterBundle();
@override
Target get generateLocalizationsTarget => const GenerateLocalizationsTarget();
@override
Target get dartPluginRegistrantTarget => const DartPluginRegistrantTarget();
@override
Target webServiceWorker(FileSystem fileSystem, List<WebCompilerConfig> compileConfigs) =>
WebServiceWorker(fileSystem, compileConfigs);
}

View file

@ -24,8 +24,8 @@ import '../base/logger.dart';
import '../base/net.dart';
import '../base/platform.dart';
import '../build_info.dart';
import '../build_system/targets/scene_importer.dart';
import '../build_system/targets/shader_compiler.dart';
import '../build_system/tools/scene_importer.dart';
import '../build_system/tools/shader_compiler.dart';
import '../bundle_builder.dart';
import '../cache.dart';
import '../compile.dart';

View file

@ -24,10 +24,8 @@ import 'base/terminal.dart';
import 'base/utils.dart';
import 'build_info.dart';
import 'build_system/build_system.dart';
import 'build_system/targets/dart_plugin_registrant.dart';
import 'build_system/targets/localizations.dart';
import 'build_system/targets/scene_importer.dart';
import 'build_system/targets/shader_compiler.dart';
import 'build_system/tools/scene_importer.dart';
import 'build_system/tools/shader_compiler.dart';
import 'bundle.dart';
import 'cache.dart';
import 'compile.dart';
@ -1260,8 +1258,8 @@ abstract class ResidentRunner extends ResidentHandlers {
);
final CompositeTarget compositeTarget = CompositeTarget(<Target>[
const GenerateLocalizationsTarget(),
const DartPluginRegistrantTarget(),
globals.buildTargets.generateLocalizationsTarget,
globals.buildTargets.dartPluginRegistrantTarget,
]);
_lastBuild = await globals.buildSystem.buildIncremental(

View file

@ -1707,6 +1707,7 @@ Run 'flutter -h' (or 'flutter <command> -h') for available flutter commands and
await generateLocalizationsSyntheticPackage(
environment: environment,
buildSystem: globals.buildSystem,
buildTargets: globals.buildTargets,
);
await pub.get(

View file

@ -13,7 +13,6 @@ import '../base/project_migrator.dart';
import '../base/utils.dart';
import '../build_info.dart';
import '../build_system/build_system.dart';
import '../build_system/targets/web.dart';
import '../cache.dart';
import '../flutter_plugins.dart';
import '../globals.dart' as globals;
@ -28,6 +27,15 @@ import 'migrations/scrub_generated_plugin_registrant.dart';
export 'compiler_config.dart';
/// Whether the application has web plugins.
const String kHasWebPlugins = 'HasWebPlugins';
/// Base href to set in index.html in flutter build command
const String kBaseHref = 'baseHref';
/// The caching strategy to use for service worker generation.
const String kServiceWorkerStrategy = 'ServiceWorkerStrategy';
class WebBuilder {
WebBuilder({
required Logger logger,
@ -81,7 +89,7 @@ class WebBuilder {
final Stopwatch sw = Stopwatch()..start();
try {
final BuildResult result = await _buildSystem.build(
WebServiceWorker(_fileSystem, compilerConfigs),
globals.buildTargets.webServiceWorker(_fileSystem, compilerConfigs),
Environment(
projectDir: _fileSystem.currentDirectory,
outputDir: outputDirectory,

View file

@ -9,7 +9,7 @@ import 'package:file_testing/file_testing.dart';
import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/build_system/tools/shader_compiler.dart';
import 'package:flutter_tools/src/devfs.dart';
import '../../../src/common.dart';

View file

@ -7,7 +7,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/build_system/tools/shader_compiler.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/devfs.dart';
import 'package:flutter_tools/src/device.dart';

View file

@ -9,8 +9,10 @@ import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/build_system/build_system.dart';
import 'package:flutter_tools/src/build_system/build_targets.dart';
import 'package:flutter_tools/src/build_system/targets/localizations.dart';
import 'package:flutter_tools/src/dart/generate_synthetic_packages.dart';
import 'package:flutter_tools/src/isolated/build_targets.dart';
import '../../src/common.dart';
import '../../src/fake_process_manager.dart';
@ -55,6 +57,7 @@ void main() {
() => generateLocalizationsSyntheticPackage(
environment: environment,
buildSystem: buildSystem,
buildTargets: const BuildTargetsImpl(),
),
throwsToolExit(message:
'Generating synthetic localizations package failed with 1 error:'
@ -104,6 +107,7 @@ void main() {
() => generateLocalizationsSyntheticPackage(
environment: environment,
buildSystem: buildSystem,
buildTargets: const BuildTargetsImpl(),
),
throwsToolExit(message:
'Generating synthetic localizations package failed with 1 error:'
@ -151,6 +155,7 @@ void main() {
() => generateLocalizationsSyntheticPackage(
environment: environment,
buildSystem: buildSystem,
buildTargets: const BuildTargetsImpl(),
),
throwsToolExit(message:
'Generating synthetic localizations package failed with 1 error:'
@ -187,6 +192,7 @@ void main() {
await generateLocalizationsSyntheticPackage(
environment: environment,
buildSystem: buildSystem,
buildTargets: const NoOpBuildTargets(),
);
});
@ -220,6 +226,7 @@ void main() {
() => generateLocalizationsSyntheticPackage(
environment: environment,
buildSystem: buildSystem,
buildTargets: const NoOpBuildTargets(),
),
throwsToolExit(message: 'to contain a map, instead was helloWorld'),
);
@ -255,6 +262,7 @@ void main() {
() => generateLocalizationsSyntheticPackage(
environment: environment,
buildSystem: buildSystem,
buildTargets: const NoOpBuildTargets(),
),
throwsToolExit(message: 'to have a bool value, instead was "nonBoolValue"'),
);

View file

@ -17,7 +17,7 @@ import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/os.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/build_system/tools/shader_compiler.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/devfs.dart';
import 'package:flutter_tools/src/vmservice.dart';

View file

@ -10,7 +10,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/build_system/tools/shader_compiler.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/devfs.dart';
import 'package:flutter_tools/src/device.dart';

View file

@ -19,8 +19,8 @@ import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/build_system.dart';
import 'package:flutter_tools/src/build_system/targets/scene_importer.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/build_system/tools/scene_importer.dart';
import 'package:flutter_tools/src/build_system/tools/shader_compiler.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/convert.dart';
import 'package:flutter_tools/src/devfs.dart';

View file

@ -16,8 +16,8 @@ import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/time.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/targets/scene_importer.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/build_system/tools/scene_importer.dart';
import 'package:flutter_tools/src/build_system/tools/shader_compiler.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/devfs.dart';
import 'package:flutter_tools/src/device.dart';

View file

@ -12,7 +12,7 @@ import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/signals.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/build_system/tools/shader_compiler.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/convert.dart';
import 'package:flutter_tools/src/devfs.dart';

View file

@ -12,7 +12,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/build_system/tools/shader_compiler.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/convert.dart';
import 'package:flutter_tools/src/devfs.dart';

View file

@ -5,7 +5,7 @@
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
import 'package:flutter_tools/src/build_system/tools/shader_compiler.dart';
import 'package:flutter_tools/src/globals.dart' as globals;
import '../src/common.dart';

View file

@ -18,6 +18,7 @@ import 'package:flutter_tools/src/base/template.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/base/time.dart';
import 'package:flutter_tools/src/base/version.dart';
import 'package:flutter_tools/src/build_system/build_targets.dart';
import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/context_runner.dart';
import 'package:flutter_tools/src/dart/pub.dart';
@ -28,6 +29,7 @@ import 'package:flutter_tools/src/globals.dart' as globals;
import 'package:flutter_tools/src/ios/plist_parser.dart';
import 'package:flutter_tools/src/ios/simulators.dart';
import 'package:flutter_tools/src/ios/xcodeproj.dart';
import 'package:flutter_tools/src/isolated/build_targets.dart';
import 'package:flutter_tools/src/isolated/mustache_template.dart';
import 'package:flutter_tools/src/persistent_tool_state.dart';
import 'package:flutter_tools/src/project.dart';
@ -122,6 +124,7 @@ void testUsingContext(
Pub: () => ThrowingPub(), // prevent accidentally using pub.
CrashReporter: () => const NoopCrashReporter(),
TemplateRenderer: () => const MustacheTemplateRenderer(),
BuildTargets: () => const BuildTargetsImpl(),
Analytics: () => NoOpAnalytics(),
},
body: () {