Skip to content

Commit

Permalink
Revert "[macos] add flavor options to commands in the flutter_tool
Browse files Browse the repository at this point in the history
…(#118421)" (#118858)

This reverts commit 73096fd.
  • Loading branch information
jmagman committed Jan 20, 2023
1 parent 73096fd commit 030288d
Show file tree
Hide file tree
Showing 21 changed files with 43 additions and 173 deletions.
14 changes: 0 additions & 14 deletions .ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2668,20 +2668,6 @@ 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
Expand Down
1 change: 0 additions & 1 deletion TESTOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@
/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 @a-wallen @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
Expand Down
39 changes: 0 additions & 39 deletions dev/devicelab/bin/tasks/flavors_test_macos.dart

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,6 @@
"@executable_path/../Frameworks",
);
PRODUCT_BUNDLE_IDENTIFIER = com.yourcompany.flavors.free;
PRODUCT_FLAVOR = free;
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "";
SWIFT_OPTIMIZATION_LEVEL = "-Onone";
Expand All @@ -674,7 +673,6 @@
"@executable_path/../Frameworks",
);
PRODUCT_BUNDLE_IDENTIFIER = com.yourcompany.flavors.free;
PRODUCT_FLAVOR = free;
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "";
SWIFT_VERSION = 5.0;
Expand Down
32 changes: 2 additions & 30 deletions packages/flutter_tools/bin/macos_assemble.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,32 +17,6 @@ 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}/.."
Expand All @@ -54,10 +28,8 @@ BuildApp() {
target_path="${FLUTTER_TARGET}"
fi

# 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="$(ParseFlutterBuildMode)"
# Set the build mode
local build_mode="$(echo "${FLUTTER_BUILD_MODE:-${CONFIGURATION}}" | tr "[:upper:]" "[:lower:]")"

if [[ -n "$LOCAL_ENGINE" ]]; then
if [[ $(echo "$LOCAL_ENGINE" | tr "[:upper:]" "[:lower:]") != *"$build_mode"* ]]; then
Expand Down
5 changes: 0 additions & 5 deletions packages/flutter_tools/lib/src/commands/install.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,6 @@ class InstallCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts
if (device == null) {
throwToolExit('No target device found');
}

if (!device!.supportsInstall) {
throwToolExit('Host and target are the same. Nothing to install.');
}

if (userIdentifier != null && device is! AndroidDevice) {
throwToolExit('--${FlutterOptions.kDeviceUser} is only supported for Android');
}
Expand Down
13 changes: 5 additions & 8 deletions packages/flutter_tools/lib/src/desktop_device.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,6 @@ abstract class DesktopDevice extends Device {
@override
void clearLogs() {}

@override
bool get supportsInstall => false;

@override
Future<LaunchResult> startApp(
ApplicationPackage package, {
Expand All @@ -126,9 +123,9 @@ abstract class DesktopDevice extends Device {
}

// Ensure that the executable is locatable.
final BuildInfo buildInfo = debuggingOptions.buildInfo;
final BuildMode buildMode = debuggingOptions.buildInfo.mode;
final bool traceStartup = platformArgs['trace-startup'] as bool? ?? false;
final String? executable = executablePathForDevice(package, buildInfo);
final String? executable = executablePathForDevice(package, buildMode);
if (executable == null) {
_logger.printError('Unable to find executable to run');
return LaunchResult.failed();
Expand Down Expand Up @@ -164,7 +161,7 @@ abstract class DesktopDevice extends Device {
try {
final Uri? observatoryUri = await observatoryDiscovery.uri;
if (observatoryUri != null) {
onAttached(package, buildInfo, process);
onAttached(package, buildMode, process);
return LaunchResult.succeeded(observatoryUri: observatoryUri);
}
_logger.printError(
Expand Down Expand Up @@ -206,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, BuildInfo buildInfo);
String? executablePathForDevice(ApplicationPackage package, BuildMode buildMode);

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

/// Computes a set of environment variables used to pass debugging information
/// to the engine without interfering with application level command line
Expand Down
4 changes: 0 additions & 4 deletions packages/flutter_tools/lib/src/device.dart
Original file line number Diff line number Diff line change
Expand Up @@ -579,10 +579,6 @@ abstract class Device {
/// Whether the device supports the '--fast-start' development mode.
bool get supportsFastStart => false;

/// Whether this device supports the installation of a flutter app via
/// `flutter install`.
bool get supportsInstall => true;

/// Stop an app package on the current device.
///
/// Specify [userIdentifier] to stop app installed to a profile (Android only).
Expand Down
4 changes: 2 additions & 2 deletions packages/flutter_tools/lib/src/linux/linux_device.dart
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ class LinuxDevice extends DesktopDevice {
}

@override
String executablePathForDevice(covariant LinuxApp package, BuildInfo buildInfo) {
return package.executable(buildInfo.mode);
String executablePathForDevice(covariant LinuxApp package, BuildMode buildMode) {
return package.executable(buildMode);
}
}

Expand Down
23 changes: 8 additions & 15 deletions packages/flutter_tools/lib/src/macos/application_package.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ abstract class MacOSApp extends ApplicationPackage {
@override
String get displayName => id;

String? applicationBundle(BuildInfo buildInfo);
String? applicationBundle(BuildMode buildMode);

String? executable(BuildInfo buildInfo);
String? executable(BuildMode buildMode);
}

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

@override
String? applicationBundle(BuildInfo buildInfo) => uncompressedBundle.path;
String? applicationBundle(BuildMode buildMode) => uncompressedBundle.path;

@override
String? executable(BuildInfo buildInfo) => _executable;
String? executable(BuildMode buildMode) => _executable;

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

@override
String? applicationBundle(BuildInfo buildInfo) {
String? applicationBundle(BuildMode buildMode) {
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',
bundleDirectory(buildInfo),
sentenceCase(getNameForBuildMode(buildMode)),
appBundleNameFile.readAsStringSync().trim());
}

String bundleDirectory(BuildInfo buildInfo) {
return sentenceCase(buildInfo.mode.name) + (buildInfo.flavor != null
? ' ${sentenceCase(buildInfo.flavor!)}'
: '');
}

@override
String? executable(BuildInfo buildInfo) {
final String? directory = applicationBundle(buildInfo);
String? executable(BuildMode buildMode) {
final String? directory = applicationBundle(buildMode);
if (directory == null) {
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/macos/build_macos.dart
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ Future<void> buildMacOS({
'xcodebuild',
'-workspace', xcodeWorkspace.path,
'-configuration', configuration,
'-scheme', scheme,
'-scheme', 'Runner',
'-derivedDataPath', flutterBuildDir.absolute.path,
'-destination', 'platform=macOS',
'OBJROOT=${globals.fs.path.join(flutterBuildDir.absolute.path, 'Build', 'Intermediates.noindex')}',
Expand Down
8 changes: 4 additions & 4 deletions packages/flutter_tools/lib/src/macos/macos_device.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,17 @@ class MacOSDevice extends DesktopDevice {
}

@override
String? executablePathForDevice(covariant MacOSApp package, BuildInfo buildInfo) {
return package.executable(buildInfo);
String? executablePathForDevice(covariant MacOSApp package, BuildMode buildMode) {
return package.executable(buildMode);
}

@override
void onAttached(covariant MacOSApp package, BuildInfo buildInfo, Process process) {
void onAttached(covariant MacOSApp package, BuildMode buildMode, 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(buildInfo);
final String? applicationBundle = package.applicationBundle(buildMode);
if (applicationBundle == null) {
_logger.printError('Failed to foreground app; application bundle not found');
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class MacOSDesignedForIPadDevice extends DesktopDevice {
}

@override
String? executablePathForDevice(ApplicationPackage package, BuildInfo buildInfo) => null;
String? executablePathForDevice(ApplicationPackage package, BuildMode buildMode) => null;

@override
Future<LaunchResult> startApp(
Expand Down
4 changes: 2 additions & 2 deletions packages/flutter_tools/lib/src/windows/windows_device.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ class WindowsDevice extends DesktopDevice {
}

@override
String executablePathForDevice(covariant WindowsApp package, BuildInfo buildInfo) {
return package.executable(buildInfo.mode);
String executablePathForDevice(covariant WindowsApp package, BuildMode buildMode) {
return package.executable(buildMode);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,6 @@ class FakeIOSDevice extends Fake implements IOSDevice {
IOSApp app, {
String? userIdentifier,
}) async => true;

@override
String get name => 'iOS';

@override
bool get supportsInstall => true;
}

// Unfortunately Device, despite not being immutable, has an `operator ==`.
Expand All @@ -183,10 +177,4 @@ class FakeAndroidDevice extends Fake implements AndroidDevice {
AndroidApk app, {
String? userIdentifier,
}) async => true;

@override
String get name => 'Android';

@override
bool get supportsInstall => true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ void main() {
),
]);
final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager, fileSystem: fileSystem);
final String? executableName = device.executablePathForDevice(FakeApplicationPackage(), BuildInfo.debug);
final String? executableName = device.executablePathForDevice(FakeApplicationPackage(), BuildMode.debug);
fileSystem.file(executableName).writeAsStringSync('\n');
final FakeApplicationPackage package = FakeApplicationPackage();
final LaunchResult result = await device.startApp(
Expand Down Expand Up @@ -367,11 +367,11 @@ class FakeDesktopDevice extends DesktopDevice {

// Dummy implementation that just returns the build mode name.
@override
String? executablePathForDevice(ApplicationPackage package, BuildInfo buildInfo) {
String? executablePathForDevice(ApplicationPackage package, BuildMode buildMode) {
if (nullExecutablePathForDevice) {
return null;
}
return buildInfo == null ? 'null' : getNameForBuildMode(buildInfo.mode);
return buildMode == null ? 'null' : getNameForBuildMode(buildMode);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,9 @@ void main() {
operatingSystemUtils: FakeOperatingSystemUtils(),
);

expect(device.executablePathForDevice(mockApp, BuildInfo.debug), 'debug/executable');
expect(device.executablePathForDevice(mockApp, BuildInfo.profile), 'profile/executable');
expect(device.executablePathForDevice(mockApp, BuildInfo.release), 'release/executable');
expect(device.executablePathForDevice(mockApp, BuildMode.debug), 'debug/executable');
expect(device.executablePathForDevice(mockApp, BuildMode.profile), 'profile/executable');
expect(device.executablePathForDevice(mockApp, BuildMode.release), 'release/executable');
});
}

Expand Down
Loading

0 comments on commit 030288d

Please sign in to comment.