Skip to content

Commit

Permalink
Reverts "Expose build mode in environment of asset transformer proces…
Browse files Browse the repository at this point in the history
…ses (#144752)" (#144957)

Reverts: #144752
Initiated by: andrewkolos
Reason for reverting: compilation issue has turned the tree red
Original PR Author: andrewkolos

Reviewed By: {christopherfujino}

This change reverts the following previous change:
In service of #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 #101077 (comment):

> 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…](579912d). The rest of the change is updating call sites with a new argument.
  • Loading branch information
auto-submit[bot] committed Mar 11, 2024
1 parent 83fad74 commit 187ec75
Show file tree
Hide file tree
Showing 13 changed files with 9 additions and 70 deletions.
12 changes: 2 additions & 10 deletions packages/flutter_tools/lib/src/build_system/targets/assets.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ 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';
Expand All @@ -36,7 +35,7 @@ Future<Depfile> copyAssets(
Directory outputDirectory, {
Map<String, DevFSContent> additionalContent = const <String, DevFSContent>{},
required TargetPlatform targetPlatform,
required BuildMode buildMode,
BuildMode? buildMode,
List<File> additionalInputs = const <File>[],
String? flavor,
}) async {
Expand Down Expand Up @@ -102,7 +101,6 @@ Future<Depfile> copyAssets(
processManager: environment.processManager,
fileSystem: environment.fileSystem,
dartBinaryPath: environment.artifacts.getArtifactPath(Artifact.engineDartBinary),
buildMode: buildMode,
);

final Map<String, AssetBundleEntry> assetEntries = <String, AssetBundleEntry>{
Expand Down Expand Up @@ -188,7 +186,7 @@ Future<Depfile> 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') {
if (environment.defines[kDeferredComponents] == 'true' && buildMode != null) {
await Future.wait<void>(assetBundle.deferredComponentsEntries.entries.map<Future<void>>(
(MapEntry<String, Map<String, AssetBundleEntry>> componentEntries) async {
final Directory componentOutputDir =
Expand Down Expand Up @@ -345,11 +343,6 @@ class CopyAssets extends Target {

@override
Future<void> 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');
Expand All @@ -358,7 +351,6 @@ class CopyAssets extends Target {
environment,
output,
targetPlatform: TargetPlatform.android,
buildMode: buildMode,
flavor: environment.defines[kFlavor],
);
environment.depFileService.writeToFile(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,6 @@ abstract class IosAssetBundle extends Target {
environment,
assetDirectory,
targetPlatform: TargetPlatform.ios,
buildMode: buildMode,
additionalInputs: <File>[
flutterProject.ios.infoPlist,
flutterProject.ios.appFrameworkInfoPlist,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ abstract class BundleLinuxAssets extends Target {
environment,
outputDirectory,
targetPlatform: targetPlatform,
buildMode: buildMode,
additionalContent: <String, DevFSContent>{
'version.json': DevFSStringContent(versionInfo),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,6 @@ abstract class MacOSBundleFlutterAssets extends Target {
environment,
assetDirectory,
targetPlatform: TargetPlatform.darwin,
buildMode: buildMode,
flavor: environment.defines[kFlavor],
);
environment.depFileService.writeToFile(
Expand Down
8 changes: 0 additions & 8 deletions packages/flutter_tools/lib/src/build_system/targets/web.dart
Original file line number Diff line number Diff line change
Expand Up @@ -380,21 +380,13 @@ 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ abstract class BundleWindowsAssets extends Target {
environment,
outputDirectory,
targetPlatform: targetPlatform,
buildMode: buildMode,
);
environment.depFileService.writeToFile(
depfile,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ 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';
Expand All @@ -23,18 +22,13 @@ final class AssetTransformer {
required ProcessManager processManager,
required FileSystem fileSystem,
required String dartBinaryPath,
required BuildMode buildMode,
}) : _processManager = processManager,
_fileSystem = fileSystem,
_dartBinaryPath = dartBinaryPath,
_buildMode = buildMode;

static const String buildModeEnvVar = 'FLUTTER_BUILD_MODE';
_dartBinaryPath = dartBinaryPath;

final ProcessManager _processManager;
final FileSystem _fileSystem;
final String _dartBinaryPath;
final BuildMode _buildMode;

/// The [Source] inputs that targets using this should depend on.
///
Expand Down Expand Up @@ -121,9 +115,6 @@ final class AssetTransformer {
final ProcessResult result = await _processManager.run(
command,
workingDirectory: workingDirectory,
environment: <String, String>{
AssetTransformer.buildModeEnvVar: _buildMode.cliName,
}
);
final String stdout = result.stdout as String;
final String stderr = result.stderr as String;
Expand Down
2 changes: 0 additions & 2 deletions packages/flutter_tools/lib/src/devfs.dart
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,6 @@ class DevFS {
required FileSystem fileSystem,
required ProcessManager processManager,
required Artifacts artifacts,
required BuildMode buildMode,
HttpClient? httpClient,
Duration? uploadRetryThrottle,
StopwatchFactory stopwatchFactory = const StopwatchFactory(),
Expand All @@ -466,7 +465,6 @@ class DevFS {
processManager: processManager,
fileSystem: fileSystem,
dartBinaryPath: artifacts.getArtifactPath(Artifact.engineDartBinary),
buildMode: buildMode,
),
fileSystem: fileSystem,
logger: logger,
Expand Down
1 change: 0 additions & 1 deletion packages/flutter_tools/lib/src/resident_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,6 @@ class FlutterDevice {
logger: globals.logger,
processManager: globals.processManager,
artifacts: globals.artifacts!,
buildMode: buildInfo.mode,
);
return devFS!.create();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ 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';

Expand Down Expand Up @@ -54,7 +53,6 @@ void main() {
processManager: processManager,
fileSystem: fileSystem,
dartBinaryPath: artifacts.getArtifactPath(Artifact.engineDartBinary),
buildMode: BuildMode.debug,
);

final AssetTransformationFailure? transformationFailure = await transformer.transformAsset(
Expand Down Expand Up @@ -114,7 +112,6 @@ void main() {
processManager: processManager,
fileSystem: fileSystem,
dartBinaryPath: dartBinaryPath,
buildMode: BuildMode.debug,
);

final AssetTransformationFailure? failure = await transformer.transformAsset(
Expand Down Expand Up @@ -174,7 +171,6 @@ Something went wrong''');
processManager: processManager,
fileSystem: fileSystem,
dartBinaryPath: dartBinaryPath,
buildMode: BuildMode.debug,
);

final AssetTransformationFailure? failure = await transformer.transformAsset(
Expand Down Expand Up @@ -269,7 +265,6 @@ 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(
Expand Down Expand Up @@ -336,18 +331,14 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
onRun: (List<String> args) {
// Do nothing.
},
stderr: 'Transformation failed, but I forgot to exit with a non-zero code.',
environment: const <String, String>{
'FLUTTER_BUILD_MODE': 'debug',
},
stderr: 'Transformation failed, but I forgot to exit with a non-zero code.'
),
]);

final AssetTransformer transformer = AssetTransformer(
processManager: processManager,
fileSystem: fileSystem,
dartBinaryPath: dartBinaryPath,
buildMode: BuildMode.debug,
);

final AssetTransformationFailure? failure = await transformer.transformAsset(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ void main() {
fileSystem: fileSystem,
logger: BufferLogger.test(),
platform: FakePlatform(),
defines: <String, String>{
kBuildMode: BuildMode.debug.cliName,
},
defines: <String, String>{},
);
fileSystem.file(environment.buildDir.childFile('app.dill')).createSync(recursive: true);
fileSystem.file('packages/flutter_tools/lib/src/build_system/targets/assets.dart')
Expand Down Expand Up @@ -180,9 +178,7 @@ flutter:
fileSystem: fileSystem,
logger: logger,
platform: globals.platform,
defines: <String, String>{
kBuildMode: BuildMode.debug.cliName,
},
defines: <String, String>{},
);

await fileSystem.file('.packages').create();
Expand Down Expand Up @@ -266,9 +262,7 @@ flutter:
fileSystem: fileSystem,
logger: logger,
platform: globals.platform,
defines: <String, String>{
kBuildMode: BuildMode.debug.cliName,
},
defines: <String, String>{},
);

await fileSystem.file('.packages').create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ void main() {
outputDir: globals.fs.currentDirectory.childDirectory('bar'),
defines: <String, String>{
kTargetFile: globals.fs.path.join('foo', 'lib', 'main.dart'),
kBuildMode: BuildMode.debug.cliName,
},
artifacts: Artifacts.test(),
processManager: processManager,
Expand Down
17 changes: 2 additions & 15 deletions packages/flutter_tools/test/general.shard/devfs_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ void main() {
httpClient: FakeHttpClient.any(),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
);
expect(() async => devFS.create(), throwsA(isA<DevFSException>()));
});
Expand All @@ -161,7 +160,6 @@ void main() {
httpClient: FakeHttpClient.any(),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
);

expect(await devFS.create(), isNotNull);
Expand Down Expand Up @@ -212,7 +210,6 @@ void main() {
uploadRetryThrottle: Duration.zero,
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
);
await devFS.create();

Expand Down Expand Up @@ -248,7 +245,6 @@ void main() {
httpClient: FakeHttpClient.any(),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
);

await devFS.create();
Expand Down Expand Up @@ -291,7 +287,6 @@ void main() {
httpClient: FakeHttpClient.any(),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
);

await devFS.create();
Expand Down Expand Up @@ -336,7 +331,6 @@ void main() {
httpClient: HttpClient(),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
);

await devFS.create();
Expand Down Expand Up @@ -388,7 +382,6 @@ void main() {
httpClient: FakeHttpClient.any(),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
);

await devFS.create();
Expand Down Expand Up @@ -468,7 +461,6 @@ void main() {
}),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
);

await devFS.create();
Expand Down Expand Up @@ -515,7 +507,6 @@ void main() {
httpClient: FakeHttpClient.any(),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
);

await devFS.create();
Expand Down Expand Up @@ -621,8 +612,7 @@ void main() {
httpClient: FakeHttpClient.any(),
config: Config.test(),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
artifacts: Artifacts.test(),
);

await devFS.create();
Expand Down Expand Up @@ -680,8 +670,7 @@ void main() {
httpClient: FakeHttpClient.any(),
config: Config.test(),
processManager: FakeProcessManager.empty(),
artifacts: Artifacts.test(),
buildMode: BuildMode.debug,
artifacts: Artifacts.test(),
);

await devFS.create();
Expand Down Expand Up @@ -774,7 +763,6 @@ void main() {
config: Config.test(),
processManager: processManager,
artifacts: artifacts,
buildMode: BuildMode.debug,
);

await devFS.create();
Expand Down Expand Up @@ -855,7 +843,6 @@ void main() {
config: Config.test(),
processManager: processManager,
artifacts: artifacts,
buildMode: BuildMode.debug,
);

await devFS.create();
Expand Down

0 comments on commit 187ec75

Please sign in to comment.