Skip to content

Commit d272a3a

Browse files
cbrackencbracken
authored
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.
1 parent 67d07a6 commit d272a3a

19 files changed

+155
-43
lines changed

‎.ci.yaml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2637,6 +2637,20 @@ targets:
26372637
- bin/**
26382638
- .ci.yaml
26392639

2640+
- name: Mac flavors_test_macos
2641+
bringup: true
2642+
recipe: devicelab/devicelab_drone
2643+
timeout: 60
2644+
properties:
2645+
dependencies: >-
2646+
[
2647+
{"dependency": "xcode", "version": "14a5294e"},
2648+
{"dependency": "gems", "version": "v3.3.14"}
2649+
]
2650+
tags: >
2651+
["devicelab", "hostonly", "mac"]
2652+
task_name: flavors_test_macos
2653+
26402654
- name: Mac flutter_gallery_macos__compile
26412655
presubmit: false
26422656
recipe: devicelab/devicelab_drone

‎TESTOWNERS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@
218218
/dev/devicelab/bin/tasks/complex_layout_win_desktop__start_up.dart @yaakovschectman @flutter/desktop
219219
/dev/devicelab/bin/tasks/dart_plugin_registry_test.dart @stuartmorgan @flutter/plugin
220220
/dev/devicelab/bin/tasks/entrypoint_dart_registrant.dart @aaclarke @flutter/plugin
221+
/dev/devicelab/bin/tasks/flavors_test_macos.dart @cbracken @flutter/desktop
221222
/dev/devicelab/bin/tasks/flutter_gallery_macos__compile.dart @a-wallen @flutter/desktop
222223
/dev/devicelab/bin/tasks/flutter_gallery_macos__start_up.dart @a-wallen @flutter/desktop
223224
/dev/devicelab/bin/tasks/flutter_gallery_win_desktop__compile.dart @yaakovschectman @flutter/desktop

‎dev/devicelab/bin/tasks/flavors_test_macos.dart

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'package:flutter_devicelab/framework/devices.dart';
6+
import 'package:flutter_devicelab/framework/framework.dart';
7+
import 'package:flutter_devicelab/framework/task_result.dart';
8+
import 'package:flutter_devicelab/framework/utils.dart';
9+
import 'package:flutter_devicelab/tasks/integration_tests.dart';
10+
11+
Future<void> main() async {
12+
deviceOperatingSystem = DeviceOperatingSystem.macos;
13+
await task(() async {
14+
await createFlavorsTest().call();
15+
await createIntegrationTestFlavorsTest().call();
16+
17+
await inDirectory('${flutterDirectory.path}/dev/integration_tests/flavors', () async {
18+
final StringBuffer stderr = StringBuffer();
19+
20+
await evalFlutter(
21+
'install',
22+
canFail: true,
23+
stderr: stderr,
24+
options: <String>[
25+
'--d', 'macos',
26+
'--flavor', 'free'
27+
],
28+
);
29+
30+
final String stderrString = stderr.toString();
31+
if (!stderrString.contains('Host and target are the same. Nothing to install.')) {
32+
print(stderrString);
33+
return TaskResult.failure('Installing a macOS app on macOS should no-op');
34+
}
35+
});
36+
37+
return TaskResult.success(null);
38+
});
39+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,7 @@
651651
"@executable_path/../Frameworks",
652652
);
653653
PRODUCT_BUNDLE_IDENTIFIER = com.yourcompany.flavors.free;
654+
PRODUCT_FLAVOR = free;
654655
PRODUCT_NAME = "$(TARGET_NAME)";
655656
PROVISIONING_PROFILE_SPECIFIER = "";
656657
SWIFT_OPTIMIZATION_LEVEL = "-Onone";
@@ -673,6 +674,7 @@
673674
"@executable_path/../Frameworks",
674675
);
675676
PRODUCT_BUNDLE_IDENTIFIER = com.yourcompany.flavors.free;
677+
PRODUCT_FLAVOR = free;
676678
PRODUCT_NAME = "$(TARGET_NAME)";
677679
PROVISIONING_PROFILE_SPECIFIER = "";
678680
SWIFT_VERSION = 5.0;

‎packages/flutter_tools/bin/macos_assemble.sh

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,32 @@ EchoError() {
1717
echo "$@" 1>&2
1818
}
1919

20+
ParseFlutterBuildMode() {
21+
# Use FLUTTER_BUILD_MODE if it's set, otherwise use the Xcode build configuration name
22+
# This means that if someone wants to use an Xcode build config other than Debug/Profile/Release,
23+
# they _must_ set FLUTTER_BUILD_MODE so we know what type of artifact to build.
24+
local build_mode="$(echo "${FLUTTER_BUILD_MODE:-${CONFIGURATION}}" | tr "[:upper:]" "[:lower:]")"
25+
26+
case "$build_mode" in
27+
*release*) build_mode="release";;
28+
*profile*) build_mode="profile";;
29+
*debug*) build_mode="debug";;
30+
*)
31+
EchoError "========================================================================"
32+
EchoError "ERROR: Unknown FLUTTER_BUILD_MODE: ${build_mode}."
33+
EchoError "Valid values are 'Debug', 'Profile', or 'Release' (case insensitive)."
34+
EchoError "This is controlled by the FLUTTER_BUILD_MODE environment variable."
35+
EchoError "If that is not set, the CONFIGURATION environment variable is used."
36+
EchoError ""
37+
EchoError "You can fix this by either adding an appropriately named build"
38+
EchoError "configuration, or adding an appropriate value for FLUTTER_BUILD_MODE to the"
39+
EchoError ".xcconfig file for the current build configuration (${CONFIGURATION})."
40+
EchoError "========================================================================"
41+
exit -1;;
42+
esac
43+
echo "${build_mode}"
44+
}
45+
2046
BuildApp() {
2147
# Set the working directory to the project root
2248
local project_path="${SOURCE_ROOT}/.."
@@ -28,8 +54,10 @@ BuildApp() {
2854
target_path="${FLUTTER_TARGET}"
2955
fi
3056

31-
# Set the build mode
32-
local build_mode="$(echo "${FLUTTER_BUILD_MODE:-${CONFIGURATION}}" | tr "[:upper:]" "[:lower:]")"
57+
# Use FLUTTER_BUILD_MODE if it's set, otherwise use the Xcode build configuration name
58+
# This means that if someone wants to use an Xcode build config other than Debug/Profile/Release,
59+
# they _must_ set FLUTTER_BUILD_MODE so we know what type of artifact to build.
60+
local build_mode="$(ParseFlutterBuildMode)"
3361

3462
if [[ -n "$LOCAL_ENGINE" ]]; then
3563
if [[ $(echo "$LOCAL_ENGINE" | tr "[:upper:]" "[:lower:]") != *"$build_mode"* ]]; then

‎packages/flutter_tools/lib/src/desktop_device.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ abstract class DesktopDevice extends Device {
123123
}
124124

125125
// Ensure that the executable is locatable.
126-
final BuildMode buildMode = debuggingOptions.buildInfo.mode;
126+
final BuildInfo buildInfo = debuggingOptions.buildInfo;
127127
final bool traceStartup = platformArgs['trace-startup'] as bool? ?? false;
128-
final String? executable = executablePathForDevice(package, buildMode);
128+
final String? executable = executablePathForDevice(package, buildInfo);
129129
if (executable == null) {
130130
_logger.printError('Unable to find executable to run');
131131
return LaunchResult.failed();
@@ -161,7 +161,7 @@ abstract class DesktopDevice extends Device {
161161
try {
162162
final Uri? observatoryUri = await observatoryDiscovery.uri;
163163
if (observatoryUri != null) {
164-
onAttached(package, buildMode, process);
164+
onAttached(package, buildInfo, process);
165165
return LaunchResult.succeeded(observatoryUri: observatoryUri);
166166
}
167167
_logger.printError(
@@ -203,11 +203,11 @@ abstract class DesktopDevice extends Device {
203203

204204
/// Returns the path to the executable to run for [package] on this device for
205205
/// the given [buildMode].
206-
String? executablePathForDevice(ApplicationPackage package, BuildMode buildMode);
206+
String? executablePathForDevice(ApplicationPackage package, BuildInfo buildInfo);
207207

208208
/// Called after a process is attached, allowing any device-specific extra
209209
/// steps to be run.
210-
void onAttached(ApplicationPackage package, BuildMode buildMode, Process process) {}
210+
void onAttached(ApplicationPackage package, BuildInfo buildInfo, Process process) {}
211211

212212
/// Computes a set of environment variables used to pass debugging information
213213
/// to the engine without interfering with application level command line

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ class LinuxDevice extends DesktopDevice {
7070
}
7171

7272
@override
73-
String executablePathForDevice(covariant LinuxApp package, BuildMode buildMode) {
74-
return package.executable(buildMode);
73+
String executablePathForDevice(covariant LinuxApp package, BuildInfo buildInfo) {
74+
return package.executable(buildInfo.mode);
7575
}
7676
}
7777

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,9 @@ abstract class MacOSApp extends ApplicationPackage {
106106
@override
107107
String get displayName => id;
108108

109-
String? applicationBundle(BuildMode buildMode);
109+
String? applicationBundle(BuildInfo buildInfo);
110110

111-
String? executable(BuildMode buildMode);
111+
String? executable(BuildInfo buildInfo);
112112
}
113113

114114
class PrebuiltMacOSApp extends MacOSApp implements PrebuiltApplicationPackage {
@@ -135,10 +135,10 @@ class PrebuiltMacOSApp extends MacOSApp implements PrebuiltApplicationPackage {
135135
String get name => bundleName;
136136

137137
@override
138-
String? applicationBundle(BuildMode buildMode) => uncompressedBundle.path;
138+
String? applicationBundle(BuildInfo buildInfo) => uncompressedBundle.path;
139139

140140
@override
141-
String? executable(BuildMode buildMode) => _executable;
141+
String? executable(BuildInfo buildInfo) => _executable;
142142

143143
/// A [File] or [Directory] pointing to the application bundle.
144144
///
@@ -156,23 +156,30 @@ class BuildableMacOSApp extends MacOSApp {
156156
String get name => 'macOS';
157157

158158
@override
159-
String? applicationBundle(BuildMode buildMode) {
159+
String? applicationBundle(BuildInfo buildInfo) {
160160
final File appBundleNameFile = project.nameFile;
161161
if (!appBundleNameFile.existsSync()) {
162162
globals.printError('Unable to find app name. ${appBundleNameFile.path} does not exist');
163163
return null;
164164
}
165+
165166
return globals.fs.path.join(
166167
getMacOSBuildDirectory(),
167168
'Build',
168169
'Products',
169-
sentenceCase(getNameForBuildMode(buildMode)),
170+
bundleDirectory(buildInfo),
170171
appBundleNameFile.readAsStringSync().trim());
171172
}
172173

174+
String bundleDirectory(BuildInfo buildInfo) {
175+
return sentenceCase(buildInfo.mode.name) + (buildInfo.flavor != null
176+
? ' ${sentenceCase(buildInfo.flavor!)}'
177+
: '');
178+
}
179+
173180
@override
174-
String? executable(BuildMode buildMode) {
175-
final String? directory = applicationBundle(buildMode);
181+
String? executable(BuildInfo buildInfo) {
182+
final String? directory = applicationBundle(buildInfo);
176183
if (directory == null) {
177184
return null;
178185
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ Future<void> buildMacOS({
114114
'xcodebuild',
115115
'-workspace', xcodeWorkspace.path,
116116
'-configuration', configuration,
117-
'-scheme', 'Runner',
117+
'-scheme', scheme,
118118
'-derivedDataPath', flutterBuildDir.absolute.path,
119119
'-destination', 'platform=macOS',
120120
'OBJROOT=${globals.fs.path.join(flutterBuildDir.absolute.path, 'Build', 'Intermediates.noindex')}',

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,17 @@ class MacOSDevice extends DesktopDevice {
7878
}
7979

8080
@override
81-
String? executablePathForDevice(covariant MacOSApp package, BuildMode buildMode) {
82-
return package.executable(buildMode);
81+
String? executablePathForDevice(covariant MacOSApp package, BuildInfo buildInfo) {
82+
return package.executable(buildInfo);
8383
}
8484

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class MacOSDesignedForIPadDevice extends DesktopDevice {
5454
}
5555

5656
@override
57-
String? executablePathForDevice(ApplicationPackage package, BuildMode buildMode) => null;
57+
String? executablePathForDevice(ApplicationPackage package, BuildInfo buildInfo) => null;
5858

5959
@override
6060
Future<LaunchResult> startApp(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ class WindowsDevice extends DesktopDevice {
6161
}
6262

6363
@override
64-
String executablePathForDevice(covariant WindowsApp package, BuildMode buildMode) {
65-
return package.executable(buildMode);
64+
String executablePathForDevice(covariant WindowsApp package, BuildInfo buildInfo) {
65+
return package.executable(buildInfo.mode);
6666
}
6767
}
6868

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,9 @@ class FakeIOSDevice extends Fake implements IOSDevice {
157157
IOSApp app, {
158158
String? userIdentifier,
159159
}) async => true;
160+
161+
@override
162+
String get name => 'iOS';
160163
}
161164

162165
// Unfortunately Device, despite not being immutable, has an `operator ==`.
@@ -177,4 +180,7 @@ class FakeAndroidDevice extends Fake implements AndroidDevice {
177180
AndroidApk app, {
178181
String? userIdentifier,
179182
}) async => true;
183+
184+
@override
185+
String get name => 'Android';
180186
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ void main() {
8686
),
8787
]);
8888
final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager, fileSystem: fileSystem);
89-
final String? executableName = device.executablePathForDevice(FakeApplicationPackage(), BuildMode.debug);
89+
final String? executableName = device.executablePathForDevice(FakeApplicationPackage(), BuildInfo.debug);
9090
fileSystem.file(executableName).writeAsStringSync('\n');
9191
final FakeApplicationPackage package = FakeApplicationPackage();
9292
final LaunchResult result = await device.startApp(
@@ -367,11 +367,11 @@ class FakeDesktopDevice extends DesktopDevice {
367367

368368
// Dummy implementation that just returns the build mode name.
369369
@override
370-
String? executablePathForDevice(ApplicationPackage package, BuildMode buildMode) {
370+
String? executablePathForDevice(ApplicationPackage package, BuildInfo buildInfo) {
371371
if (nullExecutablePathForDevice) {
372372
return null;
373373
}
374-
return getNameForBuildMode(buildMode);
374+
return getNameForBuildMode(buildInfo.mode);
375375
}
376376
}
377377

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,9 @@ void main() {
154154
operatingSystemUtils: FakeOperatingSystemUtils(),
155155
);
156156

157-
expect(device.executablePathForDevice(mockApp, BuildMode.debug), 'debug/executable');
158-
expect(device.executablePathForDevice(mockApp, BuildMode.profile), 'profile/executable');
159-
expect(device.executablePathForDevice(mockApp, BuildMode.release), 'release/executable');
157+
expect(device.executablePathForDevice(mockApp, BuildInfo.debug), 'debug/executable');
158+
expect(device.executablePathForDevice(mockApp, BuildInfo.profile), 'profile/executable');
159+
expect(device.executablePathForDevice(mockApp, BuildInfo.release), 'release/executable');
160160
});
161161
}
162162

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
1010
import 'package:flutter_tools/src/base/logger.dart';
1111
import 'package:flutter_tools/src/base/os.dart';
1212
import 'package:flutter_tools/src/base/utils.dart';
13+
import 'package:flutter_tools/src/build_info.dart';
1314
import 'package:flutter_tools/src/globals.dart' as globals;
1415
import 'package:flutter_tools/src/ios/plist_parser.dart';
1516
import 'package:flutter_tools/src/macos/application_package.dart';
@@ -155,6 +156,20 @@ group('PrebuiltMacOSApp', () {
155156
expect(macosApp.id, 'com.example.placeholder');
156157
expect(macosApp.name, 'macOS');
157158
}, overrides: overrides);
159+
160+
testUsingContext('Chooses the correct directory for application.', () {
161+
final MacOSProject project = FlutterProject.fromDirectory(globals.fs.currentDirectory).macos;
162+
final BuildableMacOSApp macosApp = MacOSApp.fromMacOSProject(project) as BuildableMacOSApp;
163+
164+
const BuildInfo vanillaApp = BuildInfo(BuildMode.debug, null, treeShakeIcons: false);
165+
String? applicationBundle = macosApp.bundleDirectory(vanillaApp);
166+
expect(applicationBundle, 'Debug');
167+
168+
const BuildInfo flavoredApp = BuildInfo(BuildMode.release, 'flavor', treeShakeIcons: false);
169+
applicationBundle = macosApp.bundleDirectory(flavoredApp);
170+
expect(applicationBundle, 'Release Flavor');
171+
172+
}, overrides: overrides);
158173
});
159174
}
160175

0 commit comments

Comments
 (0)