Roll forward: "Initialize default-app-flavor" (#169298) (#169602)

Closes https://github.com/flutter/flutter/issues/169598 (which explains the integration test failure). Closes https://github.com/flutter/flutter/issues/169160. Closes https://github.com/flutter/flutter/issues/165803. This is the only diff from 5d013c73ba: ```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 61583210e47..67731019a05 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 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'); } } ```
2025-06-03 00:51:18 +00:00 · 2025-05-28 13:15:01 -07:00 · 2025-05-28 13:15:01 -07:00 · 8e8cb92553
commit 8e8cb92553
parent 5e953e76c1
6 changed files with 252 additions and 64 deletions
--- 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');
  }
 }
--- 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<void> {
    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(
--- 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<void> 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: <Type, Generator>{
-      Platform: () => FakePlatform(environment: <String, String>{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<void> buildResult = const KernelSnapshot().build(
-        androidEnvironment
-          ..defines[kTargetPlatform] = getNameForTargetPlatform(TargetPlatform.android)
-          ..defines[kBuildMode] = BuildMode.debug.cliName
-          ..defines[kFlavor] = 'strawberry'
-          ..defines[kDartDefines] = encodeDartDefines(<String>[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>[
+        FakeCommand(
+          command: <String>[
+            artifacts.getArtifactPath(Artifact.engineDartAotRuntime),
+            artifacts.getArtifactPath(Artifact.frontendServerSnapshotForEngineDartSdk),
+            '--sdk-root',
+            '$flutterPatchedSdkPath/',
+            '--target=flutter',
+            '--no-print-incremental-dependencies',
+            '-D$kAppFlavor=strawberry',
+            ...buildModeOptions(BuildMode.debug, <String>[]),
+            '--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: <Type, Generator>{
+      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)
--- 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<void> runner = createTestCommandRunner(
+          _TestRunCommandThatOnlyValidates(),
+        );
+        expect(
+          runner.run(<String>['run', '--no-pub', '--no-hot']),
+          throwsToolExit(
+            message: '$kAppFlavor is used by the framework and cannot be set in the environment.',
+          ),
+        );
+      },
+      overrides: <Type, Generator>{
+        DeviceManager:
+            () => FakeDeviceManager()..attachedDevices = <Device>[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 = <String, String>{kAppFlavor: 'AlreadySet'},
+      },
+    );
+
+    testUsingContext(
+      'tool exits when $kAppFlavor is set in --dart-define',
+      () async {
+        final CommandRunner<void> runner = createTestCommandRunner(
+          _TestRunCommandThatOnlyValidates(),
+        );
+        expect(
+          runner.run(<String>[
+            '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: <Type, Generator>{
+        DeviceManager:
+            () => FakeDeviceManager()..attachedDevices = <Device>[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<void> runner = createTestCommandRunner(
+          _TestRunCommandThatOnlyValidates(),
+        );
+        expect(
+          runner.run(<String>[
+            '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: <Type, Generator>{
+        DeviceManager:
+            () => FakeDeviceManager()..attachedDevices = <Device>[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(
--- a/packages/flutter_tools/test/integration.shard/default_flavor_test.dart
+++ 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(<String>['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');
+      });
+    }
+  ''';
+}
--- 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<void> 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<int?> get done async => _process?.exitCode;

  Future<void> connectToVmService({bool pauseOnExceptions = false}) async {
    _vmService = await vmServiceConnectUri('$_vmServiceWsUri');