Skip to content

Commit 0a159b3

Browse files
flutteractionsbotflutteractionsbot
authored
[CP-stable]Roll forward: "Initialize default-app-flavor" (#169298) (#169623)
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. ### Issue Link: What is the link to the issue this cherry-pick is addressing? flutter/flutter#169602 ### Changelog Description: Explain this cherry pick in one line that is accessible to most Flutter developers. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples Fixes a bug where `appFlavor` is `null` when being run with `flutter test` or being hot-restarted. ### Impact Description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch) Cannot reliably use `appFlavor` without rebuilding the app from scratch. ### Workaround: Is there a workaround for this issue? Do not use hot restart, do not use `flutter test`. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? Automated test coverage.
1 parent 6374794 commit 0a159b3

File tree

6 files changed

+247
-59
lines changed

6 files changed

+247
-59
lines changed

‎packages/flutter_tools/lib/src/build_system/targets/common.dart‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,14 @@ import 'package:package_config/package_config.dart';
66

77
import '../../artifacts.dart';
88
import '../../base/build.dart';
9-
import '../../base/common.dart';
109
import '../../base/file_system.dart';
1110
import '../../base/io.dart';
1211
import '../../build_info.dart';
1312
import '../../compile.dart';
1413
import '../../dart/package_map.dart';
1514
import '../../devfs.dart';
16-
import '../../globals.dart' as globals show platform, xcode;
15+
import '../../globals.dart' as globals show xcode;
1716
import '../../project.dart';
18-
import '../../runner/flutter_command.dart';
1917
import '../build_system.dart';
2018
import '../depfile.dart';
2119
import '../exceptions.dart';
@@ -310,15 +308,17 @@ class KernelSnapshot extends Target {
310308
if (flavor == null) {
311309
return;
312310
}
313-
if (globals.platform.environment[kAppFlavor] != null) {
314-
throwToolExit('$kAppFlavor is used by the framework and cannot be set in the environment.');
315-
}
316-
if (dartDefines.any((String define) => define.startsWith(kAppFlavor))) {
317-
throwToolExit(
318-
'$kAppFlavor is used by the framework and cannot be '
319-
'set using --${FlutterOptions.kDartDefinesOption} or --${FlutterOptions.kDartDefineFromFileOption}',
320-
);
321-
}
311+
312+
// It is possible there is a flavor already in dartDefines, from another
313+
// part of the build process, but this should take precedence as it happens
314+
// last (xcodebuild execution).
315+
//
316+
// See https://github.com/flutter/flutter/issues/169598.
317+
318+
// If the flavor is already in the dart defines, remove it.
319+
dartDefines.removeWhere((String define) => define.startsWith(kAppFlavor));
320+
321+
// Then, add it to the end.
322322
dartDefines.add('$kAppFlavor=$flavor');
323323
}
324324
}

‎packages/flutter_tools/lib/src/runner/flutter_command.dart‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,6 +1438,18 @@ abstract class FlutterCommand extends Command<void> {
14381438
final String? cliFlavor = argParser.options.containsKey('flavor') ? stringArg('flavor') : null;
14391439
final String? flavor = cliFlavor ?? defaultFlavor;
14401440

1441+
if (globals.platform.environment[kAppFlavor] != null) {
1442+
throwToolExit('$kAppFlavor is used by the framework and cannot be set in the environment.');
1443+
}
1444+
if (dartDefines.any((String define) => define.startsWith(kAppFlavor))) {
1445+
throwToolExit(
1446+
'$kAppFlavor is used by the framework and cannot be '
1447+
'set using --${FlutterOptions.kDartDefinesOption} or --${FlutterOptions.kDartDefineFromFileOption}',
1448+
);
1449+
}
1450+
if (flavor != null) {
1451+
dartDefines.add('$kAppFlavor=$flavor');
1452+
}
14411453
_addFlutterVersionToDartDefines(globals.flutterVersion, dartDefines);
14421454

14431455
return BuildInfo(

‎packages/flutter_tools/test/general.shard/build_system/targets/common_test.dart‎

Lines changed: 53 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:flutter_tools/src/build_system/exceptions.dart';
1313
import 'package:flutter_tools/src/build_system/targets/common.dart';
1414
import 'package:flutter_tools/src/build_system/targets/ios.dart';
1515
import 'package:flutter_tools/src/compile.dart';
16+
import 'package:flutter_tools/src/convert.dart';
1617
import 'package:flutter_tools/src/ios/xcodeproj.dart';
1718
import 'package:test/fake.dart';
1819

@@ -440,69 +441,76 @@ void main() {
440441
);
441442

442443
testUsingContext(
443-
"tool exits when $kAppFlavor is already set in user's environment",
444+
'KernelSnapshot sets flavor in dartDefines from Xcode build configuration if ios app',
444445
() async {
445446
fileSystem.file('.dart_tool/package_config.json')
446447
..createSync(recursive: true)
447448
..writeAsStringSync('{"configVersion": 2, "packages":[]}');
448-
final Future<void> buildResult = const KernelSnapshot().build(
449-
androidEnvironment
450-
..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.android)
451-
..defines[kBuildMode] = BuildMode.debug.cliName
452-
..defines[kFlavor] = 'strawberry'
453-
..defines[kTrackWidgetCreation] = 'false',
449+
final String build = iosEnvironment.buildDir.path;
450+
final String flutterPatchedSdkPath = artifacts.getArtifactPath(
451+
Artifact.flutterPatchedSdkPath,
452+
platform: TargetPlatform.ios,
453+
mode: BuildMode.debug,
454454
);
455-
456-
expect(
457-
buildResult,
458-
throwsToolExit(
459-
message: '$kAppFlavor is used by the framework and cannot be set in the environment.',
455+
fileSystem.directory('/ios/Runner.xcodeproj').createSync(recursive: true);
456+
processManager.addCommands(<FakeCommand>[
457+
FakeCommand(
458+
command: <String>[
459+
artifacts.getArtifactPath(Artifact.engineDartAotRuntime),
460+
artifacts.getArtifactPath(Artifact.frontendServerSnapshotForEngineDartSdk),
461+
'--sdk-root',
462+
'$flutterPatchedSdkPath/',
463+
'--target=flutter',
464+
'--no-print-incremental-dependencies',
465+
'-D$kAppFlavor=chocolate',
466+
...buildModeOptions(BuildMode.debug, <String>[]),
467+
'--no-link-platform',
468+
'--packages',
469+
'/.dart_tool/package_config.json',
470+
'--output-dill',
471+
'$build/app.dill',
472+
'--depfile',
473+
'$build/kernel_snapshot_program.d',
474+
'--incremental',
475+
'--initialize-from-dill',
476+
'$build/app.dill',
477+
'--verbosity=error',
478+
'file:///lib/main.dart',
479+
],
480+
stdout: 'result $kBoundaryKey\n$kBoundaryKey\n$kBoundaryKey $build/app.dill 0\n',
460481
),
461-
);
462-
},
463-
overrides: <Type, Generator>{
464-
Platform: () => FakePlatform(environment: <String, String>{kAppFlavor: 'I was already set'}),
465-
},
466-
);
482+
]);
467483

468-
testUsingContext(
469-
'tool exits when $kAppFlavor is set in --dart-define or --dart-define-from-file',
470-
() async {
471-
fileSystem.file('.dart_tool/package_config.json')
472-
..createSync(recursive: true)
473-
..writeAsStringSync('{"configVersion": 2, "packages":[]}');
474-
final Future<void> buildResult = const KernelSnapshot().build(
475-
androidEnvironment
476-
..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.android)
484+
await const KernelSnapshot().build(
485+
iosEnvironment
486+
..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.ios)
477487
..defines[kBuildMode] = BuildMode.debug.cliName
478488
..defines[kFlavor] = 'strawberry'
479-
..defines[kDartDefines] = encodeDartDefines(<String>[kAppFlavor, 'strawberry'])
489+
..defines[kXcodeConfiguration] = 'Debug-chocolate'
480490
..defines[kTrackWidgetCreation] = 'false',
481491
);
482492

483-
expect(
484-
buildResult,
485-
throwsToolExit(
486-
message:
487-
'$kAppFlavor is used by the framework and cannot be set using --dart-define or --dart-define-from-file',
488-
),
489-
);
493+
expect(processManager, hasNoRemainingExpectations);
494+
},
495+
overrides: <Type, Generator>{
496+
XcodeProjectInterpreter:
497+
() => FakeXcodeProjectInterpreter(schemes: <String>['Runner', 'chocolate']),
490498
},
491499
);
492500

493501
testUsingContext(
494-
'KernelSnapshot sets flavor in dartDefines from Xcode build configuration if ios app',
502+
'KernelSnapshot sets flavor in dartDefines from Xcode build configuration if macos app',
495503
() async {
496504
fileSystem.file('.dart_tool/package_config.json')
497505
..createSync(recursive: true)
498506
..writeAsStringSync('{"configVersion": 2, "packages":[]}');
499507
final String build = iosEnvironment.buildDir.path;
500508
final String flutterPatchedSdkPath = artifacts.getArtifactPath(
501509
Artifact.flutterPatchedSdkPath,
502-
platform: TargetPlatform.ios,
510+
platform: TargetPlatform.darwin,
503511
mode: BuildMode.debug,
504512
);
505-
fileSystem.directory('/ios/Runner.xcodeproj').createSync(recursive: true);
513+
fileSystem.directory('/macos/Runner.xcodeproj').createSync(recursive: true);
506514
processManager.addCommands(<FakeCommand>[
507515
FakeCommand(
508516
command: <String>[
@@ -514,7 +522,6 @@ void main() {
514522
'--no-print-incremental-dependencies',
515523
'-D$kAppFlavor=chocolate',
516524
...buildModeOptions(BuildMode.debug, <String>[]),
517-
'--no-link-platform',
518525
'--packages',
519526
'/.dart_tool/package_config.json',
520527
'--output-dill',
@@ -533,7 +540,7 @@ void main() {
533540

534541
await const KernelSnapshot().build(
535542
iosEnvironment
536-
..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.ios)
543+
..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.darwin)
537544
..defines[kBuildMode] = BuildMode.debug.cliName
538545
..defines[kFlavor] = 'strawberry'
539546
..defines[kXcodeConfiguration] = 'Debug-chocolate'
@@ -549,7 +556,7 @@ void main() {
549556
);
550557

551558
testUsingContext(
552-
'KernelSnapshot sets flavor in dartDefines from Xcode build configuration if macos app',
559+
'KernelSnapshot does not add kAppFlavor twice to Dart defines',
553560
() async {
554561
fileSystem.file('.dart_tool/package_config.json')
555562
..createSync(recursive: true)
@@ -560,7 +567,6 @@ void main() {
560567
platform: TargetPlatform.darwin,
561568
mode: BuildMode.debug,
562569
);
563-
fileSystem.directory('/macos/Runner.xcodeproj').createSync(recursive: true);
564570
processManager.addCommands(<FakeCommand>[
565571
FakeCommand(
566572
command: <String>[
@@ -570,7 +576,7 @@ void main() {
570576
'$flutterPatchedSdkPath/',
571577
'--target=flutter',
572578
'--no-print-incremental-dependencies',
573-
'-D$kAppFlavor=chocolate',
579+
'-D$kAppFlavor=strawberry',
574580
...buildModeOptions(BuildMode.debug, <String>[]),
575581
'--packages',
576582
'/.dart_tool/package_config.json',
@@ -592,16 +598,17 @@ void main() {
592598
iosEnvironment
593599
..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.darwin)
594600
..defines[kBuildMode] = BuildMode.debug.cliName
601+
..defines[kDartDefines] = base64Encode(utf8.encode('FLUTTER_APP_FLAVOR=vanilla'))
595602
..defines[kFlavor] = 'strawberry'
596-
..defines[kXcodeConfiguration] = 'Debug-chocolate'
597603
..defines[kTrackWidgetCreation] = 'false',
598604
);
599605

600606
expect(processManager, hasNoRemainingExpectations);
601607
},
602608
overrides: <Type, Generator>{
603-
XcodeProjectInterpreter:
604-
() => FakeXcodeProjectInterpreter(schemes: <String>['Runner', 'chocolate']),
609+
Platform: () => macPlatform,
610+
FileSystem: () => fileSystem,
611+
ProcessManager: () => processManager,
605612
},
606613
);
607614

‎packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,6 +1270,98 @@ flutter:
12701270
);
12711271
});
12721272

1273+
testUsingContext(
1274+
"tool exits when $kAppFlavor is already set in user's environemnt",
1275+
() async {
1276+
final CommandRunner<void> runner = createTestCommandRunner(
1277+
_TestRunCommandThatOnlyValidates(),
1278+
);
1279+
expect(
1280+
runner.run(<String>['run', '--no-pub', '--no-hot']),
1281+
throwsToolExit(
1282+
message: '$kAppFlavor is used by the framework and cannot be set in the environment.',
1283+
),
1284+
);
1285+
},
1286+
overrides: <Type, Generator>{
1287+
DeviceManager:
1288+
() => FakeDeviceManager()..attachedDevices = <Device>[FakeDevice('name', 'id')],
1289+
FileSystem: () {
1290+
final MemoryFileSystem fileSystem = MemoryFileSystem.test();
1291+
fileSystem.file('lib/main.dart').createSync(recursive: true);
1292+
fileSystem.file('pubspec.yaml').createSync();
1293+
return fileSystem;
1294+
},
1295+
ProcessManager: FakeProcessManager.empty,
1296+
Platform: () => FakePlatform()..environment = <String, String>{kAppFlavor: 'AlreadySet'},
1297+
},
1298+
);
1299+
1300+
testUsingContext(
1301+
'tool exits when $kAppFlavor is set in --dart-define',
1302+
() async {
1303+
final CommandRunner<void> runner = createTestCommandRunner(
1304+
_TestRunCommandThatOnlyValidates(),
1305+
);
1306+
expect(
1307+
runner.run(<String>[
1308+
'run',
1309+
'--dart-define=$kAppFlavor=AlreadySet',
1310+
'--no-pub',
1311+
'--no-hot',
1312+
]),
1313+
throwsToolExit(
1314+
message: '$kAppFlavor is used by the framework and cannot be set using --dart-define',
1315+
),
1316+
);
1317+
},
1318+
overrides: <Type, Generator>{
1319+
DeviceManager:
1320+
() => FakeDeviceManager()..attachedDevices = <Device>[FakeDevice('name', 'id')],
1321+
FileSystem: () {
1322+
final MemoryFileSystem fileSystem = MemoryFileSystem.test();
1323+
fileSystem.file('lib/main.dart').createSync(recursive: true);
1324+
fileSystem.file('pubspec.yaml').createSync();
1325+
return fileSystem;
1326+
},
1327+
ProcessManager: FakeProcessManager.empty,
1328+
},
1329+
);
1330+
1331+
testUsingContext(
1332+
'tool exits when $kAppFlavor is set in --dart-define-from-file',
1333+
() async {
1334+
final CommandRunner<void> runner = createTestCommandRunner(
1335+
_TestRunCommandThatOnlyValidates(),
1336+
);
1337+
expect(
1338+
runner.run(<String>[
1339+
'run',
1340+
'--dart-define-from-file=config.json',
1341+
'--no-pub',
1342+
'--no-hot',
1343+
]),
1344+
throwsToolExit(
1345+
message: '$kAppFlavor is used by the framework and cannot be set using --dart-define',
1346+
),
1347+
);
1348+
},
1349+
overrides: <Type, Generator>{
1350+
DeviceManager:
1351+
() => FakeDeviceManager()..attachedDevices = <Device>[FakeDevice('name', 'id')],
1352+
FileSystem: () {
1353+
final MemoryFileSystem fileSystem = MemoryFileSystem.test();
1354+
fileSystem.file('lib/main.dart').createSync(recursive: true);
1355+
fileSystem.file('pubspec.yaml').createSync();
1356+
fileSystem.file('config.json')
1357+
..createSync()
1358+
..writeAsStringSync('{"$kAppFlavor": "AlreadySet"}');
1359+
return fileSystem;
1360+
},
1361+
ProcessManager: FakeProcessManager.empty,
1362+
},
1363+
);
1364+
12731365
group('Flutter version', () {
12741366
for (final String dartDefine in FlutterCommand.flutterVersionDartDefines) {
12751367
testUsingContext(

0 commit comments

Comments
 (0)