From 83fad745358d923a9e9a22434a1fe719ba32699c Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Mon, 11 Mar 2024 13:39:31 -0700 Subject: [PATCH] Expose build mode in environment of asset transformer processes (#144752) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In service of https://github.com/flutter/flutter/issues/143348 When invoking a package to transform an asset, we set `FLUTTER_BUILD_MODE` to the CLI name of the build mode being used. Inspired by https://github.com/flutter/flutter/issues/101077#issuecomment-1890379501: > Do transformers know whether they get executed in debug or release mode? I kinda imagine that being useful. Ex: There's a transformer that optimizes the file size of images. Depending on the amount and size of the images, that could take a significant amount of time. Therefore, I might want to only execute it in release builds. Note for the reviewer: the interesting part of this change can be found in the commit [set environment variable to build mode when running asset transformer…](https://github.com/flutter/flutter/pull/144752/commits/579912d470b6f6eb036fd6b23a128fa3942f6282). The rest of the change is updating call sites with a new argument. --- .../lib/src/build_system/targets/assets.dart | 12 ++++++++++-- .../lib/src/build_system/targets/ios.dart | 1 + .../lib/src/build_system/targets/linux.dart | 1 + .../lib/src/build_system/targets/macos.dart | 1 + .../lib/src/build_system/targets/web.dart | 8 ++++++++ .../lib/src/build_system/targets/windows.dart | 1 + .../build_system/tools/asset_transformer.dart | 11 ++++++++++- packages/flutter_tools/lib/src/devfs.dart | 2 ++ .../flutter_tools/lib/src/resident_runner.dart | 1 + .../targets/asset_transformer_test.dart | 11 ++++++++++- .../build_system/targets/assets_test.dart | 12 +++++++++--- .../build_system/targets/web_test.dart | 1 + .../test/general.shard/devfs_test.dart | 17 +++++++++++++++-- 13 files changed, 70 insertions(+), 9 deletions(-) diff --git a/packages/flutter_tools/lib/src/build_system/targets/assets.dart b/packages/flutter_tools/lib/src/build_system/targets/assets.dart index 795cbf77da79..77a5fc003a26 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/assets.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/assets.dart @@ -15,6 +15,7 @@ import '../../devfs.dart'; import '../../flutter_manifest.dart'; import '../build_system.dart'; import '../depfile.dart'; +import '../exceptions.dart'; import '../tools/asset_transformer.dart'; import '../tools/scene_importer.dart'; import '../tools/shader_compiler.dart'; @@ -35,7 +36,7 @@ Future copyAssets( Directory outputDirectory, { Map additionalContent = const {}, required TargetPlatform targetPlatform, - BuildMode? buildMode, + required BuildMode buildMode, List additionalInputs = const [], String? flavor, }) async { @@ -101,6 +102,7 @@ Future copyAssets( processManager: environment.processManager, fileSystem: environment.fileSystem, dartBinaryPath: environment.artifacts.getArtifactPath(Artifact.engineDartBinary), + buildMode: buildMode, ); final Map assetEntries = { @@ -186,7 +188,7 @@ Future copyAssets( // Copy deferred components assets only for release or profile builds. // The assets are included in assetBundle.entries as a normal asset when // building as debug. - if (environment.defines[kDeferredComponents] == 'true' && buildMode != null) { + if (environment.defines[kDeferredComponents] == 'true') { await Future.wait(assetBundle.deferredComponentsEntries.entries.map>( (MapEntry> componentEntries) async { final Directory componentOutputDir = @@ -343,6 +345,11 @@ class CopyAssets extends Target { @override Future build(Environment environment) async { + final String? buildModeEnvironment = environment.defines[kBuildMode]; + if (buildModeEnvironment == null) { + throw MissingDefineException(kBuildMode, name); + } + final BuildMode buildMode = BuildMode.fromCliName(buildModeEnvironment); final Directory output = environment .buildDir .childDirectory('flutter_assets'); @@ -351,6 +358,7 @@ class CopyAssets extends Target { environment, output, targetPlatform: TargetPlatform.android, + buildMode: buildMode, flavor: environment.defines[kFlavor], ); environment.depFileService.writeToFile( diff --git a/packages/flutter_tools/lib/src/build_system/targets/ios.dart b/packages/flutter_tools/lib/src/build_system/targets/ios.dart index 9ca6e543a206..1fd586e063f3 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/ios.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/ios.dart @@ -503,6 +503,7 @@ abstract class IosAssetBundle extends Target { environment, assetDirectory, targetPlatform: TargetPlatform.ios, + buildMode: buildMode, additionalInputs: [ flutterProject.ios.infoPlist, flutterProject.ios.appFrameworkInfoPlist, diff --git a/packages/flutter_tools/lib/src/build_system/targets/linux.dart b/packages/flutter_tools/lib/src/build_system/targets/linux.dart index bc446c5fa652..6f761f27b95f 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/linux.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/linux.dart @@ -137,6 +137,7 @@ abstract class BundleLinuxAssets extends Target { environment, outputDirectory, targetPlatform: targetPlatform, + buildMode: buildMode, additionalContent: { 'version.json': DevFSStringContent(versionInfo), }, diff --git a/packages/flutter_tools/lib/src/build_system/targets/macos.dart b/packages/flutter_tools/lib/src/build_system/targets/macos.dart index 9996664a0a1e..2e018fe84ccc 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/macos.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/macos.dart @@ -438,6 +438,7 @@ abstract class MacOSBundleFlutterAssets extends Target { environment, assetDirectory, targetPlatform: TargetPlatform.darwin, + buildMode: buildMode, flavor: environment.defines[kFlavor], ); environment.depFileService.writeToFile( diff --git a/packages/flutter_tools/lib/src/build_system/targets/web.dart b/packages/flutter_tools/lib/src/build_system/targets/web.dart index d3423e73808d..d8372c8c340d 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/web.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/web.dart @@ -380,13 +380,21 @@ class WebReleaseBundle extends Target { } } + final String? buildModeEnvironment = environment.defines[kBuildMode]; + if (buildModeEnvironment == null) { + throw MissingDefineException(kBuildMode, name); + } + final BuildMode buildMode = BuildMode.fromCliName(buildModeEnvironment); + createVersionFile(environment, environment.defines); final Directory outputDirectory = environment.outputDir.childDirectory('assets'); outputDirectory.createSync(recursive: true); + final Depfile depfile = await copyAssets( environment, environment.outputDir.childDirectory('assets'), targetPlatform: TargetPlatform.web_javascript, + buildMode: buildMode, ); final DepfileService depfileService = environment.depFileService; depfileService.writeToFile( diff --git a/packages/flutter_tools/lib/src/build_system/targets/windows.dart b/packages/flutter_tools/lib/src/build_system/targets/windows.dart index ada0afe5b556..2aadcf12b179 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/windows.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/windows.dart @@ -142,6 +142,7 @@ abstract class BundleWindowsAssets extends Target { environment, outputDirectory, targetPlatform: targetPlatform, + buildMode: buildMode, ); environment.depFileService.writeToFile( depfile, diff --git a/packages/flutter_tools/lib/src/build_system/tools/asset_transformer.dart b/packages/flutter_tools/lib/src/build_system/tools/asset_transformer.dart index 8e352d2186e9..43ffd0d59a67 100644 --- a/packages/flutter_tools/lib/src/build_system/tools/asset_transformer.dart +++ b/packages/flutter_tools/lib/src/build_system/tools/asset_transformer.dart @@ -12,6 +12,7 @@ import '../../base/error_handling_io.dart'; import '../../base/file_system.dart'; import '../../base/io.dart'; import '../../base/logger.dart'; +import '../../build_info.dart'; import '../../devfs.dart'; import '../../flutter_manifest.dart'; import '../build_system.dart'; @@ -22,13 +23,18 @@ final class AssetTransformer { required ProcessManager processManager, required FileSystem fileSystem, required String dartBinaryPath, + required BuildMode buildMode, }) : _processManager = processManager, _fileSystem = fileSystem, - _dartBinaryPath = dartBinaryPath; + _dartBinaryPath = dartBinaryPath, + _buildMode = buildMode; + + static const String buildModeEnvVar = 'FLUTTER_BUILD_MODE'; final ProcessManager _processManager; final FileSystem _fileSystem; final String _dartBinaryPath; + final BuildMode _buildMode; /// The [Source] inputs that targets using this should depend on. /// @@ -115,6 +121,9 @@ final class AssetTransformer { final ProcessResult result = await _processManager.run( command, workingDirectory: workingDirectory, + environment: { + AssetTransformer.buildModeEnvVar: _buildMode.cliName, + } ); final String stdout = result.stdout as String; final String stderr = result.stderr as String; diff --git a/packages/flutter_tools/lib/src/devfs.dart b/packages/flutter_tools/lib/src/devfs.dart index 9db81a4df819..4027001eee3b 100644 --- a/packages/flutter_tools/lib/src/devfs.dart +++ b/packages/flutter_tools/lib/src/devfs.dart @@ -442,6 +442,7 @@ class DevFS { required FileSystem fileSystem, required ProcessManager processManager, required Artifacts artifacts, + required BuildMode buildMode, HttpClient? httpClient, Duration? uploadRetryThrottle, StopwatchFactory stopwatchFactory = const StopwatchFactory(), @@ -465,6 +466,7 @@ class DevFS { processManager: processManager, fileSystem: fileSystem, dartBinaryPath: artifacts.getArtifactPath(Artifact.engineDartBinary), + buildMode: buildMode, ), fileSystem: fileSystem, logger: logger, diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index 1e73e423259c..147c3ae676b9 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -387,6 +387,7 @@ class FlutterDevice { logger: globals.logger, processManager: globals.processManager, artifacts: globals.artifacts!, + buildMode: buildInfo.mode, ); return devFS!.create(); } diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/asset_transformer_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/asset_transformer_test.dart index 02aea1e3f062..9e017573b0a3 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/asset_transformer_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/asset_transformer_test.dart @@ -8,6 +8,7 @@ import 'package:file_testing/file_testing.dart'; 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_info.dart'; import 'package:flutter_tools/src/build_system/tools/asset_transformer.dart'; import 'package:flutter_tools/src/flutter_manifest.dart'; @@ -53,6 +54,7 @@ void main() { processManager: processManager, fileSystem: fileSystem, dartBinaryPath: artifacts.getArtifactPath(Artifact.engineDartBinary), + buildMode: BuildMode.debug, ); final AssetTransformationFailure? transformationFailure = await transformer.transformAsset( @@ -112,6 +114,7 @@ void main() { processManager: processManager, fileSystem: fileSystem, dartBinaryPath: dartBinaryPath, + buildMode: BuildMode.debug, ); final AssetTransformationFailure? failure = await transformer.transformAsset( @@ -171,6 +174,7 @@ Something went wrong'''); processManager: processManager, fileSystem: fileSystem, dartBinaryPath: dartBinaryPath, + buildMode: BuildMode.debug, ); final AssetTransformationFailure? failure = await transformer.transformAsset( @@ -265,6 +269,7 @@ Transformation failed, but I forgot to exit with a non-zero code.''' processManager: processManager, fileSystem: fileSystem, dartBinaryPath: dartBinaryPath, + buildMode: BuildMode.debug, ); final AssetTransformationFailure? failure = await transformer.transformAsset( @@ -331,7 +336,10 @@ Transformation failed, but I forgot to exit with a non-zero code.''' onRun: (List args) { // Do nothing. }, - stderr: 'Transformation failed, but I forgot to exit with a non-zero code.' + stderr: 'Transformation failed, but I forgot to exit with a non-zero code.', + environment: const { + 'FLUTTER_BUILD_MODE': 'debug', + }, ), ]); @@ -339,6 +347,7 @@ Transformation failed, but I forgot to exit with a non-zero code.''' processManager: processManager, fileSystem: fileSystem, dartBinaryPath: dartBinaryPath, + buildMode: BuildMode.debug, ); final AssetTransformationFailure? failure = await transformer.transformAsset( diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/assets_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/assets_test.dart index cd9fc31799c6..6d33eeec2972 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/assets_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/assets_test.dart @@ -38,7 +38,9 @@ void main() { fileSystem: fileSystem, logger: BufferLogger.test(), platform: FakePlatform(), - defines: {}, + defines: { + kBuildMode: BuildMode.debug.cliName, + }, ); fileSystem.file(environment.buildDir.childFile('app.dill')).createSync(recursive: true); fileSystem.file('packages/flutter_tools/lib/src/build_system/targets/assets.dart') @@ -178,7 +180,9 @@ flutter: fileSystem: fileSystem, logger: logger, platform: globals.platform, - defines: {}, + defines: { + kBuildMode: BuildMode.debug.cliName, + }, ); await fileSystem.file('.packages').create(); @@ -262,7 +266,9 @@ flutter: fileSystem: fileSystem, logger: logger, platform: globals.platform, - defines: {}, + defines: { + kBuildMode: BuildMode.debug.cliName, + }, ); await fileSystem.file('.packages').create(); diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart index 46c4e1cc7bcb..e5eef5cd8a04 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart @@ -68,6 +68,7 @@ void main() { outputDir: globals.fs.currentDirectory.childDirectory('bar'), defines: { kTargetFile: globals.fs.path.join('foo', 'lib', 'main.dart'), + kBuildMode: BuildMode.debug.cliName, }, artifacts: Artifacts.test(), processManager: processManager, diff --git a/packages/flutter_tools/test/general.shard/devfs_test.dart b/packages/flutter_tools/test/general.shard/devfs_test.dart index 2cd54da3ea72..98b9078fac42 100644 --- a/packages/flutter_tools/test/general.shard/devfs_test.dart +++ b/packages/flutter_tools/test/general.shard/devfs_test.dart @@ -135,6 +135,7 @@ void main() { httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.empty(), artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); expect(() async => devFS.create(), throwsA(isA())); }); @@ -160,6 +161,7 @@ void main() { httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.empty(), artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); expect(await devFS.create(), isNotNull); @@ -210,6 +212,7 @@ void main() { uploadRetryThrottle: Duration.zero, processManager: FakeProcessManager.empty(), artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); await devFS.create(); @@ -245,6 +248,7 @@ void main() { httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.empty(), artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); await devFS.create(); @@ -287,6 +291,7 @@ void main() { httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.empty(), artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); await devFS.create(); @@ -331,6 +336,7 @@ void main() { httpClient: HttpClient(), processManager: FakeProcessManager.empty(), artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); await devFS.create(); @@ -382,6 +388,7 @@ void main() { httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.empty(), artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); await devFS.create(); @@ -461,6 +468,7 @@ void main() { }), processManager: FakeProcessManager.empty(), artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); await devFS.create(); @@ -507,6 +515,7 @@ void main() { httpClient: FakeHttpClient.any(), processManager: FakeProcessManager.empty(), artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); await devFS.create(); @@ -612,7 +621,8 @@ void main() { httpClient: FakeHttpClient.any(), config: Config.test(), processManager: FakeProcessManager.empty(), - artifacts: Artifacts.test(), + artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); await devFS.create(); @@ -670,7 +680,8 @@ void main() { httpClient: FakeHttpClient.any(), config: Config.test(), processManager: FakeProcessManager.empty(), - artifacts: Artifacts.test(), + artifacts: Artifacts.test(), + buildMode: BuildMode.debug, ); await devFS.create(); @@ -763,6 +774,7 @@ void main() { config: Config.test(), processManager: processManager, artifacts: artifacts, + buildMode: BuildMode.debug, ); await devFS.create(); @@ -843,6 +855,7 @@ void main() { config: Config.test(), processManager: processManager, artifacts: artifacts, + buildMode: BuildMode.debug, ); await devFS.create();