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 735c380b10f..67731019a05 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/common.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/common.dart @@ -6,16 +6,14 @@ import 'package:package_config/package_config.dart'; import '../../artifacts.dart'; import '../../base/build.dart'; -import '../../base/common.dart'; import '../../base/file_system.dart'; import '../../base/io.dart'; import '../../build_info.dart'; import '../../compile.dart'; import '../../dart/package_map.dart'; import '../../devfs.dart'; -import '../../globals.dart' as globals show platform, xcode; +import '../../globals.dart' as globals show xcode; import '../../project.dart'; -import '../../runner/flutter_command.dart'; import '../build_system.dart'; import '../depfile.dart'; import '../exceptions.dart'; @@ -310,15 +308,17 @@ class KernelSnapshot extends Target { if (flavor == null) { return; } - if (globals.platform.environment[kAppFlavor] != null) { - throwToolExit('$kAppFlavor is used by the framework and cannot be set in the environment.'); - } - if (dartDefines.any((String define) => define.startsWith(kAppFlavor))) { - throwToolExit( - '$kAppFlavor is used by the framework and cannot be ' - 'set using --${FlutterOptions.kDartDefinesOption} or --${FlutterOptions.kDartDefineFromFileOption}', - ); - } + + // 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 https://github.com/flutter/flutter/issues/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'); } } diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 00f754f8591..38ebd9ceb12 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -1411,6 +1411,18 @@ abstract class FlutterCommand extends Command { final String? cliFlavor = argParser.options.containsKey('flavor') ? stringArg('flavor') : null; final String? flavor = cliFlavor ?? defaultFlavor; + if (globals.platform.environment[kAppFlavor] != null) { + throwToolExit('$kAppFlavor is used by the framework and cannot be set in the environment.'); + } + if (dartDefines.any((String define) => define.startsWith(kAppFlavor))) { + throwToolExit( + '$kAppFlavor is used by the framework and cannot be ' + 'set using --${FlutterOptions.kDartDefinesOption} or --${FlutterOptions.kDartDefineFromFileOption}', + ); + } + if (flavor != null) { + dartDefines.add('$kAppFlavor=$flavor'); + } _addFlutterVersionToDartDefines(globals.flutterVersion, dartDefines); return BuildInfo( diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/common_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/common_test.dart index 2e2ad77efe7..1652a6f518b 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/common_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/common_test.dart @@ -13,6 +13,7 @@ import 'package:flutter_tools/src/build_system/exceptions.dart'; import 'package:flutter_tools/src/build_system/targets/common.dart'; import 'package:flutter_tools/src/build_system/targets/ios.dart'; import 'package:flutter_tools/src/compile.dart'; +import 'package:flutter_tools/src/convert.dart'; import 'package:flutter_tools/src/ios/xcodeproj.dart'; import 'package:test/fake.dart'; @@ -439,57 +440,6 @@ void main() { }, ); - testUsingContext( - "tool exits when $kAppFlavor is already set in user's environment", - () async { - fileSystem.file('.dart_tool/package_config.json') - ..createSync(recursive: true) - ..writeAsStringSync('{"configVersion": 2, "packages":[]}'); - final Future buildResult = const KernelSnapshot().build( - androidEnvironment - ..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.android) - ..defines[kBuildMode] = BuildMode.debug.cliName - ..defines[kFlavor] = 'strawberry' - ..defines[kTrackWidgetCreation] = 'false', - ); - - expect( - buildResult, - throwsToolExit( - message: '$kAppFlavor is used by the framework and cannot be set in the environment.', - ), - ); - }, - overrides: { - Platform: () => FakePlatform(environment: {kAppFlavor: 'I was already set'}), - }, - ); - - testUsingContext( - 'tool exits when $kAppFlavor is set in --dart-define or --dart-define-from-file', - () async { - fileSystem.file('.dart_tool/package_config.json') - ..createSync(recursive: true) - ..writeAsStringSync('{"configVersion": 2, "packages":[]}'); - final Future buildResult = const KernelSnapshot().build( - androidEnvironment - ..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.android) - ..defines[kBuildMode] = BuildMode.debug.cliName - ..defines[kFlavor] = 'strawberry' - ..defines[kDartDefines] = encodeDartDefines([kAppFlavor, 'strawberry']) - ..defines[kTrackWidgetCreation] = 'false', - ); - - expect( - buildResult, - throwsToolExit( - message: - '$kAppFlavor is used by the framework and cannot be set using --dart-define or --dart-define-from-file', - ), - ); - }, - ); - testUsingContext( 'KernelSnapshot sets flavor in dartDefines from Xcode build configuration if ios app', () async { @@ -605,6 +555,63 @@ void main() { }, ); + testUsingContext( + 'KernelSnapshot does not add kAppFlavor twice to Dart defines', + () async { + fileSystem.file('.dart_tool/package_config.json') + ..createSync(recursive: true) + ..writeAsStringSync('{"configVersion": 2, "packages":[]}'); + final String build = iosEnvironment.buildDir.path; + final String flutterPatchedSdkPath = artifacts.getArtifactPath( + Artifact.flutterPatchedSdkPath, + platform: TargetPlatform.darwin, + mode: BuildMode.debug, + ); + processManager.addCommands([ + FakeCommand( + command: [ + artifacts.getArtifactPath(Artifact.engineDartAotRuntime), + artifacts.getArtifactPath(Artifact.frontendServerSnapshotForEngineDartSdk), + '--sdk-root', + '$flutterPatchedSdkPath/', + '--target=flutter', + '--no-print-incremental-dependencies', + '-D$kAppFlavor=strawberry', + ...buildModeOptions(BuildMode.debug, []), + '--packages', + '/.dart_tool/package_config.json', + '--output-dill', + '$build/app.dill', + '--depfile', + '$build/kernel_snapshot_program.d', + '--incremental', + '--initialize-from-dill', + '$build/app.dill', + '--verbosity=error', + 'file:///lib/main.dart', + ], + stdout: 'result $kBoundaryKey\n$kBoundaryKey\n$kBoundaryKey $build/app.dill 0\n', + ), + ]); + + await const KernelSnapshot().build( + iosEnvironment + ..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.darwin) + ..defines[kBuildMode] = BuildMode.debug.cliName + ..defines[kDartDefines] = base64Encode(utf8.encode('FLUTTER_APP_FLAVOR=vanilla')) + ..defines[kFlavor] = 'strawberry' + ..defines[kTrackWidgetCreation] = 'false', + ); + + expect(processManager, hasNoRemainingExpectations); + }, + overrides: { + Platform: () => macPlatform, + FileSystem: () => fileSystem, + ProcessManager: () => processManager, + }, + ); + testWithoutContext('KernelSnapshot does use track widget creation on debug builds', () async { fileSystem.file('.dart_tool/package_config.json') ..createSync(recursive: true) diff --git a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart index 00b603d354f..fbda6491f99 100644 --- a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart @@ -1270,6 +1270,98 @@ flutter: ); }); + testUsingContext( + "tool exits when $kAppFlavor is already set in user's environemnt", + () async { + final CommandRunner runner = createTestCommandRunner( + _TestRunCommandThatOnlyValidates(), + ); + expect( + runner.run(['run', '--no-pub', '--no-hot']), + throwsToolExit( + message: '$kAppFlavor is used by the framework and cannot be set in the environment.', + ), + ); + }, + overrides: { + DeviceManager: + () => FakeDeviceManager()..attachedDevices = [FakeDevice('name', 'id')], + FileSystem: () { + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + fileSystem.file('lib/main.dart').createSync(recursive: true); + fileSystem.file('pubspec.yaml').createSync(); + return fileSystem; + }, + ProcessManager: FakeProcessManager.empty, + Platform: () => FakePlatform()..environment = {kAppFlavor: 'AlreadySet'}, + }, + ); + + testUsingContext( + 'tool exits when $kAppFlavor is set in --dart-define', + () async { + final CommandRunner runner = createTestCommandRunner( + _TestRunCommandThatOnlyValidates(), + ); + expect( + runner.run([ + 'run', + '--dart-define=$kAppFlavor=AlreadySet', + '--no-pub', + '--no-hot', + ]), + throwsToolExit( + message: '$kAppFlavor is used by the framework and cannot be set using --dart-define', + ), + ); + }, + overrides: { + DeviceManager: + () => FakeDeviceManager()..attachedDevices = [FakeDevice('name', 'id')], + FileSystem: () { + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + fileSystem.file('lib/main.dart').createSync(recursive: true); + fileSystem.file('pubspec.yaml').createSync(); + return fileSystem; + }, + ProcessManager: FakeProcessManager.empty, + }, + ); + + testUsingContext( + 'tool exits when $kAppFlavor is set in --dart-define-from-file', + () async { + final CommandRunner runner = createTestCommandRunner( + _TestRunCommandThatOnlyValidates(), + ); + expect( + runner.run([ + 'run', + '--dart-define-from-file=config.json', + '--no-pub', + '--no-hot', + ]), + throwsToolExit( + message: '$kAppFlavor is used by the framework and cannot be set using --dart-define', + ), + ); + }, + overrides: { + DeviceManager: + () => FakeDeviceManager()..attachedDevices = [FakeDevice('name', 'id')], + FileSystem: () { + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + fileSystem.file('lib/main.dart').createSync(recursive: true); + fileSystem.file('pubspec.yaml').createSync(); + fileSystem.file('config.json') + ..createSync() + ..writeAsStringSync('{"$kAppFlavor": "AlreadySet"}'); + return fileSystem; + }, + ProcessManager: FakeProcessManager.empty, + }, + ); + group('Flutter version', () { for (final String dartDefine in FlutterCommand.flutterVersionDartDefines) { testUsingContext( diff --git a/packages/flutter_tools/test/integration.shard/default_flavor_test.dart b/packages/flutter_tools/test/integration.shard/default_flavor_test.dart new file mode 100644 index 00000000000..f84008a05d9 --- /dev/null +++ b/packages/flutter_tools/test/integration.shard/default_flavor_test.dart @@ -0,0 +1,74 @@ +// 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. + +@Tags(['flutter-test-driver']) +library; + +import 'package:flutter_tools/src/base/file_system.dart'; + +import '../src/common.dart'; +import 'test_data/project.dart'; +import 'test_driver.dart'; +import 'test_utils.dart'; + +void main() { + final Project project = _DefaultFlavorProject(); + late Directory tempDir; + late FlutterTestTestDriver flutter; + + setUp(() async { + tempDir = createResolvedTempDirectorySync('default_flavor_test.'); + await project.setUpIn(tempDir); + flutter = FlutterTestTestDriver(tempDir); + }); + + tearDown(() async { + tryToDelete(tempDir); + }); + + testWithoutContext('Reads "default-flavor" in "flutter test"', () async { + await flutter.test(); + + // Without an assertion, this test always passes. + final int? exitCode = await flutter.done; + expect(exitCode, 0, reason: 'flutter test failed with exit code $exitCode'); + }); +} + +final class _DefaultFlavorProject extends Project { + @override + final String main = r''' + // Irrelevant to this test. + void main() {} + '''; + + @override + final String pubspec = r''' + name: test + environment: + sdk: ^3.7.0-0 + + flutter: + default-flavor: dev + + dependencies: + flutter: + sdk: flutter + dev_dependencies: + flutter_test: + sdk: flutter + '''; + + @override + final String test = r''' + import 'package:flutter/services.dart'; + import 'package:flutter_test/flutter_test.dart'; + + void main() { + test('receives default-flavor with flutter test', () async { + expect(appFlavor, 'dev'); + }); + } + '''; +} diff --git a/packages/flutter_tools/test/integration.shard/test_driver.dart b/packages/flutter_tools/test/integration.shard/test_driver.dart index ee9e9d953d7..5671f310718 100644 --- a/packages/flutter_tools/test/integration.shard/test_driver.dart +++ b/packages/flutter_tools/test/integration.shard/test_driver.dart @@ -140,7 +140,10 @@ abstract final class FlutterTestDriver { _stderr.stream.listen((String message) => _debugPrint(message, topic: '<=stderr=')); } - Future get done async => _process?.exitCode; + /// Completes when process exits with the given exit code. + /// + /// If the process has never been started, complets with `null`. + Future get done async => _process?.exitCode; Future connectToVmService({bool pauseOnExceptions = false}) async { _vmService = await vmServiceConnectUri('$_vmServiceWsUri');