Skip to content

Commit 8e8cb92

Browse files
matanlureymatanlurey
authored
Roll forward: "Initialize default-app-flavor" (#169298) (#169602)
Closes flutter/flutter#169598 (which explains the integration test failure). Closes flutter/flutter#169160. Closes flutter/flutter#165803. This is the only diff from 5d013c7: ```diff diff --git a/packages/flutter_tools/lib/src/build_system/targets/common.dart b/packages/flutter_tools/lib/src/build_system/targets/common.dart index 6158321..6773101 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/common.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/common.dart @@ -308,10 +308,18 @@ class KernelSnapshot extends Target { if (flavor == null) { return; } - if (!dartDefines.any((String element) => element.startsWith(kAppFlavor))) { - // If the flavor is not already in the dart defines, add it. - dartDefines.add('$kAppFlavor=$flavor'); - } + + // It is possible there is a flavor already in dartDefines, from another + // part of the build process, but this should take precedence as it happens + // last (xcodebuild execution). + // + // See flutter/flutter#169598. + + // If the flavor is already in the dart defines, remove it. + dartDefines.removeWhere((String define) => define.startsWith(kAppFlavor)); + + // Then, add it to the end. + dartDefines.add('$kAppFlavor=$flavor'); } } ```
1 parent 5e953e7 commit 8e8cb92

6 files changed

Lines changed: 247 additions & 59 deletions

File tree

‎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
@@ -1411,6 +1411,18 @@ abstract class FlutterCommand extends Command<void> {
14111411
final String? cliFlavor = argParser.options.containsKey('flavor') ? stringArg('flavor') : null;
14121412
final String? flavor = cliFlavor ?? defaultFlavor;
14131413

1414+
if (globals.platform.environment[kAppFlavor] != null) {
1415+
throwToolExit('$kAppFlavor is used by the framework and cannot be set in the environment.');
1416+
}
1417+
if (dartDefines.any((String define) => define.startsWith(kAppFlavor))) {
1418+
throwToolExit(
1419+
'$kAppFlavor is used by the framework and cannot be '
1420+
'set using --${FlutterOptions.kDartDefinesOption} or --${FlutterOptions.kDartDefineFromFileOption}',
1421+
);
1422+
}
1423+
if (flavor != null) {
1424+
dartDefines.add('$kAppFlavor=$flavor');
1425+
}
14141426
_addFlutterVersionToDartDefines(globals.flutterVersion, dartDefines);
14151427

14161428
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)