Reland: [macos] add flavor options to tool commands (#119564)

* Reland: [macos] add flavor options to tool commands

Adds --flavor option to flutter run and flutter build. Running against
preexisting devicelab flavor tests for feature parity between macOS,
iOS, and Android.

This relands #118421 by alex-wallen which was reverted in #118858 due to
the following test failures:

The bail-out with "Host and target are the same. Nothing to install."
added in `packages/flutter_tools/lib/src/commands/install.dart`
triggered failures in the following tests, which unconditionally attempt
to install the built app, which is unsupported on desktop since the
host and target are the same:

* https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8791495589540422465/+/u/run_flutter_view_macos__start_up/test_stdout
* https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8791496218824259121/+/u/run_complex_layout_win_desktop__start_up/test_stdout
* https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8791496218165602641/+/u/run_flutter_gallery_win_desktop__start_up/test_stdout

Fixes #64088

* Partial revert: eliminate install check on desktop

The original flavour support patch included a check that triggered a
failure when flutter install is run on desktop OSes. This was
intentional, since the host and target devices are the same and
installation is unnecessary to launch the app on currently-supported
desktop OSes.

Note that Windows UWP apps *do* require installation to run, and we used
to have an install command for those apps, though UWP is no longer
supported.

Since that part of the change was orthogonal to flavour support itself,
I'm reverting that component of the change and we can deal with it
separately if so desired.

.ci.yaml

@@ -2637,6 +2637,20 @@ targets:
       - bin/**
       - .ci.yaml
 
+  - name: Mac flavors_test_macos
+    bringup: true
+    recipe: devicelab/devicelab_drone
+    timeout: 60
+    properties:
+      dependencies: >-
+        [
+          {"dependency": "xcode", "version": "14a5294e"},
+          {"dependency": "gems", "version": "v3.3.14"}
+        ]
+      tags: >
+        ["devicelab", "hostonly", "mac"]
+      task_name: flavors_test_macos
+
   - name: Mac flutter_gallery_macos__compile
     presubmit: false
     recipe: devicelab/devicelab_drone

TESTOWNERS

@@ -218,6 +218,7 @@
 /dev/devicelab/bin/tasks/complex_layout_win_desktop__start_up.dart @yaakovschectman @flutter/desktop
 /dev/devicelab/bin/tasks/dart_plugin_registry_test.dart @stuartmorgan @flutter/plugin
 /dev/devicelab/bin/tasks/entrypoint_dart_registrant.dart @aaclarke @flutter/plugin
+/dev/devicelab/bin/tasks/flavors_test_macos.dart @cbracken @flutter/desktop
 /dev/devicelab/bin/tasks/flutter_gallery_macos__compile.dart @a-wallen @flutter/desktop
 /dev/devicelab/bin/tasks/flutter_gallery_macos__start_up.dart @a-wallen @flutter/desktop
 /dev/devicelab/bin/tasks/flutter_gallery_win_desktop__compile.dart @yaakovschectman @flutter/desktop

dev/devicelab/bin/tasks/flavors_test_macos.dart

@@ -0,0 +1,39 @@
+// 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.
+
+import 'package:flutter_devicelab/framework/devices.dart';
+import 'package:flutter_devicelab/framework/framework.dart';
+import 'package:flutter_devicelab/framework/task_result.dart';
+import 'package:flutter_devicelab/framework/utils.dart';
+import 'package:flutter_devicelab/tasks/integration_tests.dart';
+
+Future<void> main() async {
+  deviceOperatingSystem = DeviceOperatingSystem.macos;
+  await task(() async {
+    await createFlavorsTest().call();
+    await createIntegrationTestFlavorsTest().call();
+
+    await inDirectory('${flutterDirectory.path}/dev/integration_tests/flavors', () async {
+      final StringBuffer stderr = StringBuffer();
+
+      await evalFlutter(
+        'install',
+        canFail: true,
+        stderr: stderr,
+        options: <String>[
+          '--d', 'macos',
+          '--flavor', 'free'
+        ],
+      );
+
+      final String stderrString = stderr.toString();
+      if (!stderrString.contains('Host and target are the same. Nothing to install.')) {
+        print(stderrString);
+        return TaskResult.failure('Installing a macOS app on macOS should no-op');
+      }
+    });
+
+    return TaskResult.success(null);
+  });
+}

dev/integration_tests/flavors/macos/Runner.xcodeproj/project.pbxproj

@@ -651,6 +651,7 @@
 					"@executable_path/../Frameworks",
 				);
 				PRODUCT_BUNDLE_IDENTIFIER = com.yourcompany.flavors.free;
+				PRODUCT_FLAVOR = free;
 				PRODUCT_NAME = "$(TARGET_NAME)";
 				PROVISIONING_PROFILE_SPECIFIER = "";
 				SWIFT_OPTIMIZATION_LEVEL = "-Onone";
@@ -673,6 +674,7 @@
 					"@executable_path/../Frameworks",
 				);
 				PRODUCT_BUNDLE_IDENTIFIER = com.yourcompany.flavors.free;
+				PRODUCT_FLAVOR = free;
 				PRODUCT_NAME = "$(TARGET_NAME)";
 				PROVISIONING_PROFILE_SPECIFIER = "";
 				SWIFT_VERSION = 5.0;

packages/flutter_tools/bin/macos_assemble.sh

@@ -17,6 +17,32 @@ EchoError() {
   echo "$@" 1>&2
 }
 
+ParseFlutterBuildMode() {
+  # Use FLUTTER_BUILD_MODE if it's set, otherwise use the Xcode build configuration name
+  # This means that if someone wants to use an Xcode build config other than Debug/Profile/Release,
+  # they _must_ set FLUTTER_BUILD_MODE so we know what type of artifact to build.
+  local build_mode="$(echo "${FLUTTER_BUILD_MODE:-${CONFIGURATION}}" | tr "[:upper:]" "[:lower:]")"
+
+  case "$build_mode" in
+    *release*) build_mode="release";;
+    *profile*) build_mode="profile";;
+    *debug*) build_mode="debug";;
+    *)
+      EchoError "========================================================================"
+      EchoError "ERROR: Unknown FLUTTER_BUILD_MODE: ${build_mode}."
+      EchoError "Valid values are 'Debug', 'Profile', or 'Release' (case insensitive)."
+      EchoError "This is controlled by the FLUTTER_BUILD_MODE environment variable."
+      EchoError "If that is not set, the CONFIGURATION environment variable is used."
+      EchoError ""
+      EchoError "You can fix this by either adding an appropriately named build"
+      EchoError "configuration, or adding an appropriate value for FLUTTER_BUILD_MODE to the"
+      EchoError ".xcconfig file for the current build configuration (${CONFIGURATION})."
+      EchoError "========================================================================"
+      exit -1;;
+  esac
+  echo "${build_mode}"
+}
+
 BuildApp() {
   # Set the working directory to the project root
   local project_path="${SOURCE_ROOT}/.."
@@ -28,8 +54,10 @@ BuildApp() {
       target_path="${FLUTTER_TARGET}"
   fi
 
-  # Set the build mode
+  # Use FLUTTER_BUILD_MODE if it's set, otherwise use the Xcode build configuration name
-  local build_mode="$(echo "${FLUTTER_BUILD_MODE:-${CONFIGURATION}}" | tr "[:upper:]" "[:lower:]")"
+  # This means that if someone wants to use an Xcode build config other than Debug/Profile/Release,
+  # they _must_ set FLUTTER_BUILD_MODE so we know what type of artifact to build.
+  local build_mode="$(ParseFlutterBuildMode)"
 
   if [[ -n "$LOCAL_ENGINE" ]]; then
     if [[ $(echo "$LOCAL_ENGINE" | tr "[:upper:]" "[:lower:]") != *"$build_mode"* ]]; then

packages/flutter_tools/lib/src/desktop_device.dart

@@ -123,9 +123,9 @@ abstract class DesktopDevice extends Device {
     }
 
     // Ensure that the executable is locatable.
-    final BuildMode buildMode = debuggingOptions.buildInfo.mode;
+    final BuildInfo buildInfo = debuggingOptions.buildInfo;
     final bool traceStartup = platformArgs['trace-startup'] as bool? ?? false;
-    final String? executable = executablePathForDevice(package, buildMode);
+    final String? executable = executablePathForDevice(package, buildInfo);
     if (executable == null) {
       _logger.printError('Unable to find executable to run');
       return LaunchResult.failed();
@@ -161,7 +161,7 @@ abstract class DesktopDevice extends Device {
     try {
       final Uri? observatoryUri = await observatoryDiscovery.uri;
       if (observatoryUri != null) {
-        onAttached(package, buildMode, process);
+        onAttached(package, buildInfo, process);
         return LaunchResult.succeeded(observatoryUri: observatoryUri);
       }
       _logger.printError(
@@ -203,11 +203,11 @@ abstract class DesktopDevice extends Device {
 
   /// Returns the path to the executable to run for [package] on this device for
   /// the given [buildMode].
-  String? executablePathForDevice(ApplicationPackage package, BuildMode buildMode);
+  String? executablePathForDevice(ApplicationPackage package, BuildInfo buildInfo);
 
   /// Called after a process is attached, allowing any device-specific extra
   /// steps to be run.
-  void onAttached(ApplicationPackage package, BuildMode buildMode, Process process) {}
+  void onAttached(ApplicationPackage package, BuildInfo buildInfo, Process process) {}
 
   /// Computes a set of environment variables used to pass debugging information
   /// to the engine without interfering with application level command line

packages/flutter_tools/lib/src/linux/linux_device.dart

@@ -70,8 +70,8 @@ class LinuxDevice extends DesktopDevice {
   }
 
   @override
-  String executablePathForDevice(covariant LinuxApp package, BuildMode buildMode) {
+  String executablePathForDevice(covariant LinuxApp package, BuildInfo buildInfo) {
-    return package.executable(buildMode);
+    return package.executable(buildInfo.mode);
   }
 }
 

packages/flutter_tools/lib/src/macos/application_package.dart

@@ -106,9 +106,9 @@ abstract class MacOSApp extends ApplicationPackage {
   @override
   String get displayName => id;
 
-  String? applicationBundle(BuildMode buildMode);
+  String? applicationBundle(BuildInfo buildInfo);
 
-  String? executable(BuildMode buildMode);
+  String? executable(BuildInfo buildInfo);
 }
 
 class PrebuiltMacOSApp extends MacOSApp implements PrebuiltApplicationPackage {
@@ -135,10 +135,10 @@ class PrebuiltMacOSApp extends MacOSApp implements PrebuiltApplicationPackage {
   String get name => bundleName;
 
   @override
-  String? applicationBundle(BuildMode buildMode) => uncompressedBundle.path;
+  String? applicationBundle(BuildInfo buildInfo) => uncompressedBundle.path;
 
   @override
-  String? executable(BuildMode buildMode) => _executable;
+  String? executable(BuildInfo buildInfo) => _executable;
 
   /// A [File] or [Directory] pointing to the application bundle.
   ///
@@ -156,23 +156,30 @@ class BuildableMacOSApp extends MacOSApp {
   String get name => 'macOS';
 
   @override
-  String? applicationBundle(BuildMode buildMode) {
+  String? applicationBundle(BuildInfo buildInfo) {
     final File appBundleNameFile = project.nameFile;
     if (!appBundleNameFile.existsSync()) {
       globals.printError('Unable to find app name. ${appBundleNameFile.path} does not exist');
       return null;
     }
+
     return globals.fs.path.join(
         getMacOSBuildDirectory(),
         'Build',
         'Products',
-        sentenceCase(getNameForBuildMode(buildMode)),
+        bundleDirectory(buildInfo),
         appBundleNameFile.readAsStringSync().trim());
   }
 
+  String bundleDirectory(BuildInfo buildInfo) {
+    return sentenceCase(buildInfo.mode.name) + (buildInfo.flavor != null
+      ? ' ${sentenceCase(buildInfo.flavor!)}'
+      : '');
+  }
+
   @override
-  String? executable(BuildMode buildMode) {
+  String? executable(BuildInfo buildInfo) {
-    final String? directory = applicationBundle(buildMode);
+    final String? directory = applicationBundle(buildInfo);
     if (directory == null) {
       return null;
     }

packages/flutter_tools/lib/src/macos/build_macos.dart

@@ -114,7 +114,7 @@ Future<void> buildMacOS({
       'xcodebuild',
       '-workspace', xcodeWorkspace.path,
       '-configuration', configuration,
-      '-scheme', 'Runner',
+      '-scheme', scheme,
       '-derivedDataPath', flutterBuildDir.absolute.path,
       '-destination', 'platform=macOS',
       'OBJROOT=${globals.fs.path.join(flutterBuildDir.absolute.path, 'Build', 'Intermediates.noindex')}',

packages/flutter_tools/lib/src/macos/macos_device.dart

@@ -78,17 +78,17 @@ class MacOSDevice extends DesktopDevice {
   }
 
   @override
-  String? executablePathForDevice(covariant MacOSApp package, BuildMode buildMode) {
+  String? executablePathForDevice(covariant MacOSApp package, BuildInfo buildInfo) {
-    return package.executable(buildMode);
+    return package.executable(buildInfo);
   }
 
   @override
-  void onAttached(covariant MacOSApp package, BuildMode buildMode, Process process) {
+  void onAttached(covariant MacOSApp package, BuildInfo buildInfo, Process process) {
     // Bring app to foreground. Ideally this would be done post-launch rather
     // than post-attach, since this won't run for release builds, but there's
     // no general-purpose way of knowing when a process is far enough along in
     // the launch process for 'open' to foreground it.
-    final String? applicationBundle = package.applicationBundle(buildMode);
+    final String? applicationBundle = package.applicationBundle(buildInfo);
     if (applicationBundle == null) {
       _logger.printError('Failed to foreground app; application bundle not found');
       return;

packages/flutter_tools/lib/src/macos/macos_ipad_device.dart

@@ -54,7 +54,7 @@ class MacOSDesignedForIPadDevice extends DesktopDevice {
   }
 
   @override
-  String? executablePathForDevice(ApplicationPackage package, BuildMode buildMode) => null;
+  String? executablePathForDevice(ApplicationPackage package, BuildInfo buildInfo) => null;
 
   @override
   Future<LaunchResult> startApp(

packages/flutter_tools/lib/src/windows/windows_device.dart

@@ -61,8 +61,8 @@ class WindowsDevice extends DesktopDevice {
   }
 
   @override
-  String executablePathForDevice(covariant WindowsApp package, BuildMode buildMode) {
+  String executablePathForDevice(covariant WindowsApp package, BuildInfo buildInfo) {
-    return package.executable(buildMode);
+    return package.executable(buildInfo.mode);
   }
 }
 

packages/flutter_tools/test/commands.shard/hermetic/install_test.dart

@@ -157,6 +157,9 @@ class FakeIOSDevice extends Fake implements IOSDevice {
     IOSApp app, {
     String? userIdentifier,
   }) async => true;
+
+  @override
+  String get name => 'iOS';
 }
 
 // Unfortunately Device, despite not being immutable, has an `operator ==`.
@@ -177,4 +180,7 @@ class FakeAndroidDevice extends Fake implements AndroidDevice {
     AndroidApk app, {
     String? userIdentifier,
   }) async => true;
+
+  @override
+  String get name => 'Android';
 }

packages/flutter_tools/test/general.shard/desktop_device_test.dart

@@ -86,7 +86,7 @@ void main() {
         ),
       ]);
       final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager, fileSystem: fileSystem);
-      final String? executableName = device.executablePathForDevice(FakeApplicationPackage(), BuildMode.debug);
+      final String? executableName = device.executablePathForDevice(FakeApplicationPackage(), BuildInfo.debug);
       fileSystem.file(executableName).writeAsStringSync('\n');
       final FakeApplicationPackage package = FakeApplicationPackage();
       final LaunchResult result = await device.startApp(
@@ -367,11 +367,11 @@ class FakeDesktopDevice extends DesktopDevice {
 
   // Dummy implementation that just returns the build mode name.
   @override
-  String? executablePathForDevice(ApplicationPackage package, BuildMode buildMode) {
+  String? executablePathForDevice(ApplicationPackage package, BuildInfo buildInfo) {
     if (nullExecutablePathForDevice) {
       return null;
     }
-    return getNameForBuildMode(buildMode);
+    return getNameForBuildMode(buildInfo.mode);
   }
 }
 

packages/flutter_tools/test/general.shard/linux/linux_device_test.dart

@@ -154,9 +154,9 @@ void main() {
       operatingSystemUtils: FakeOperatingSystemUtils(),
     );
 
-    expect(device.executablePathForDevice(mockApp, BuildMode.debug), 'debug/executable');
+    expect(device.executablePathForDevice(mockApp, BuildInfo.debug), 'debug/executable');
-    expect(device.executablePathForDevice(mockApp, BuildMode.profile), 'profile/executable');
+    expect(device.executablePathForDevice(mockApp, BuildInfo.profile), 'profile/executable');
-    expect(device.executablePathForDevice(mockApp, BuildMode.release), 'release/executable');
+    expect(device.executablePathForDevice(mockApp, BuildInfo.release), 'release/executable');
   });
 }
 

packages/flutter_tools/test/general.shard/macos/application_package_test.dart

@@ -10,6 +10,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/base/os.dart';
 import 'package:flutter_tools/src/base/utils.dart';
+import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/globals.dart' as globals;
 import 'package:flutter_tools/src/ios/plist_parser.dart';
 import 'package:flutter_tools/src/macos/application_package.dart';
@@ -155,6 +156,20 @@ group('PrebuiltMacOSApp', () {
       expect(macosApp.id, 'com.example.placeholder');
       expect(macosApp.name, 'macOS');
     }, overrides: overrides);
+
+    testUsingContext('Chooses the correct directory for application.', () {
+      final MacOSProject project = FlutterProject.fromDirectory(globals.fs.currentDirectory).macos;
+      final BuildableMacOSApp macosApp = MacOSApp.fromMacOSProject(project) as BuildableMacOSApp;
+
+      const BuildInfo vanillaApp = BuildInfo(BuildMode.debug, null, treeShakeIcons: false);
+      String? applicationBundle = macosApp.bundleDirectory(vanillaApp);
+      expect(applicationBundle, 'Debug');
+
+      const BuildInfo flavoredApp = BuildInfo(BuildMode.release, 'flavor', treeShakeIcons: false);
+      applicationBundle = macosApp.bundleDirectory(flavoredApp);
+      expect(applicationBundle, 'Release Flavor');
+
+    }, overrides: overrides);
   });
 }
 

packages/flutter_tools/test/general.shard/macos/macos_device_test.dart

@@ -235,9 +235,9 @@ void main() {
     const String profilePath = 'profile/executable';
     const String releasePath = 'release/executable';
 
-    expect(device.executablePathForDevice(package, BuildMode.debug), debugPath);
+    expect(device.executablePathForDevice(package, BuildInfo.debug), debugPath);
-    expect(device.executablePathForDevice(package, BuildMode.profile), profilePath);
+    expect(device.executablePathForDevice(package, BuildInfo.profile), profilePath);
-    expect(device.executablePathForDevice(package, BuildMode.release), releasePath);
+    expect(device.executablePathForDevice(package, BuildInfo.release), releasePath);
   });
 }
 
@@ -251,13 +251,13 @@ FlutterProject setUpFlutterProject(Directory directory) {
 
 class FakeMacOSApp extends Fake implements MacOSApp {
   @override
-  String executable(BuildMode buildMode) {
+  String executable(BuildInfo buildInfo) {
-    switch (buildMode) {
+    switch (buildInfo) {
-      case BuildMode.debug:
+      case BuildInfo.debug:
         return 'debug/executable';
-      case BuildMode.profile:
+      case BuildInfo.profile:
         return 'profile/executable';
-      case BuildMode.release:
+      case BuildInfo.release:
         return 'release/executable';
       default:
         throw StateError('');

packages/flutter_tools/test/general.shard/macos/macos_ipad_device_test.dart

@@ -142,7 +142,7 @@ void main() {
       throwsA(isA<UnimplementedError>()),
     );
     await expectLater(() => device.buildForDevice(buildInfo: BuildInfo.debug), throwsA(isA<UnimplementedError>()));
-    expect(device.executablePathForDevice(FakeIOSApp(), BuildMode.debug), null);
+    expect(device.executablePathForDevice(FakeIOSApp(), BuildInfo.debug), null);
   });
 }
 

packages/flutter_tools/test/general.shard/windows/windows_device_test.dart

@@ -101,9 +101,9 @@ void main() {
     final WindowsDevice windowsDevice = setUpWindowsDevice();
     final FakeWindowsApp fakeApp = FakeWindowsApp();
 
-    expect(windowsDevice.executablePathForDevice(fakeApp, BuildMode.debug), 'debug/executable');
+    expect(windowsDevice.executablePathForDevice(fakeApp, BuildInfo.debug), 'debug/executable');
-    expect(windowsDevice.executablePathForDevice(fakeApp, BuildMode.profile), 'profile/executable');
+    expect(windowsDevice.executablePathForDevice(fakeApp, BuildInfo.profile), 'profile/executable');
-    expect(windowsDevice.executablePathForDevice(fakeApp, BuildMode.release), 'release/executable');
+    expect(windowsDevice.executablePathForDevice(fakeApp, BuildInfo.release), 'release/executable');
   });
 }