Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow iOS and macOS plugins to share darwin directory #115337

Merged
merged 5 commits into from Jan 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions .ci.yaml
Expand Up @@ -2944,6 +2944,25 @@ targets:
- bin/**
- .ci.yaml

- name: Mac plugin_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: plugin_test_macos
runIf:
- dev/**
- packages/flutter_tools/**
- bin/**
- .ci.yaml

- name: Mac_x64 tool_host_cross_arch_tests
recipe: flutter/flutter_drone
timeout: 60
Expand Down
1 change: 1 addition & 0 deletions TESTOWNERS
Expand Up @@ -248,6 +248,7 @@
/dev/devicelab/bin/tasks/platform_view_win_desktop__start_up.dart @yaakovschectman @flutter/desktop
/dev/devicelab/bin/tasks/plugin_lint_mac.dart @stuartmorgan @flutter/plugin
/dev/devicelab/bin/tasks/plugin_test_ios.dart @jmagman @flutter/ios
/dev/devicelab/bin/tasks/plugin_test_macos.dart @jmagman @flutter/desktop
/dev/devicelab/bin/tasks/plugin_test.dart @stuartmorgan @flutter/plugin
/dev/devicelab/bin/tasks/run_debug_test_android.dart @zanderso @flutter/tool
/dev/devicelab/bin/tasks/run_debug_test_macos.dart @cbracken @flutter/tool
Expand Down
5 changes: 2 additions & 3 deletions dev/devicelab/bin/tasks/plugin_test_ios.dart
Expand Up @@ -9,12 +9,11 @@ Future<void> main() async {
await task(combine(<TaskFunction>[
PluginTest('ios', <String>['-i', 'objc', '--platforms=ios']).call,
PluginTest('ios', <String>['-i', 'swift', '--platforms=ios']).call,
PluginTest('macos', <String>['--platforms=macos']).call,
// Test that Dart-only plugins are supported.
PluginTest('ios', <String>['--platforms=ios'], dartOnlyPlugin: true).call,
PluginTest('macos', <String>['--platforms=macos'], dartOnlyPlugin: true).call,
// Test that shared darwin directories are supported.
PluginTest('ios', <String>['--platforms=ios,macos'], sharedDarwinSource: true).call,
// Test that FFI plugins are supported.
PluginTest('ios', <String>['--platforms=ios'], template: 'plugin_ffi').call,
PluginTest('macos', <String>['--platforms=macos'], template: 'plugin_ffi').call,
]));
}
18 changes: 18 additions & 0 deletions dev/devicelab/bin/tasks/plugin_test_macos.dart
@@ -0,0 +1,18 @@
// 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/framework.dart';
import 'package:flutter_devicelab/tasks/plugin_tests.dart';

Future<void> main() async {
await task(combine(<TaskFunction>[
PluginTest('macos', <String>['--platforms=macos']).call,
// Test that Dart-only plugins are supported.
PluginTest('macos', <String>['--platforms=macos'], dartOnlyPlugin: true).call,
// Test that shared darwin directories are supported.
PluginTest('macos', <String>['--platforms=ios,macos'], sharedDarwinSource: true).call,
// Test that FFI plugins are supported.
PluginTest('macos', <String>['--platforms=macos'], template: 'plugin_ffi').call,
]));
}
82 changes: 82 additions & 0 deletions dev/devicelab/lib/tasks/plugin_tests.dart
Expand Up @@ -33,6 +33,7 @@ class PluginTest {
this.pluginCreateEnvironment,
this.appCreateEnvironment,
this.dartOnlyPlugin = false,
this.sharedDarwinSource = false,
this.template = 'plugin',
});

Expand All @@ -41,6 +42,7 @@ class PluginTest {
final Map<String, String>? pluginCreateEnvironment;
final Map<String, String>? appCreateEnvironment;
final bool dartOnlyPlugin;
final bool sharedDarwinSource;
final String template;

Future<TaskResult> call() async {
Expand All @@ -58,6 +60,9 @@ class PluginTest {
if (dartOnlyPlugin) {
await plugin.convertDefaultPluginToDartPlugin();
}
if (sharedDarwinSource) {
await plugin.convertDefaultPluginToSharedDarwinPlugin();
}
section('Test plugin');
if (runFlutterTest) {
await plugin.runFlutterTest();
Expand Down Expand Up @@ -159,6 +164,83 @@ class $dartPluginClass {
}
}

/// Converts an iOS/macOS plugin created from the standard template to a shared
/// darwin directory plugin.
Future<void> convertDefaultPluginToSharedDarwinPlugin() async {
// Convert the metadata.
final File pubspec = pubspecFile;
String pubspecContent = await pubspec.readAsString();
const String originalIOSKey = '\n ios:\n';
const String originalMacOSKey = '\n macos:\n';
if (!pubspecContent.contains(originalIOSKey) || !pubspecContent.contains(originalMacOSKey)) {
print(pubspecContent);
throw TaskResult.failure('Missing expected darwin platform plugin keys');
}
pubspecContent = pubspecContent.replaceAll(
originalIOSKey,
'$originalIOSKey sharedDarwinSource: true\n'
);
pubspecContent = pubspecContent.replaceAll(
originalMacOSKey,
'$originalMacOSKey sharedDarwinSource: true\n'
);
await pubspec.writeAsString(pubspecContent, flush: true);

// Copy ios to darwin, and delete macos.
final Directory iosDir = Directory(path.join(rootPath, 'ios'));
final Directory darwinDir = Directory(path.join(rootPath, 'darwin'));
recursiveCopy(iosDir, darwinDir);

await iosDir.delete(recursive: true);
await Directory(path.join(rootPath, 'macos')).delete(recursive: true);

final File podspec = File(path.join(darwinDir.path, '$name.podspec'));
String podspecContent = await podspec.readAsString();
if (!podspecContent.contains('s.platform =')) {
print(podspecContent);
throw TaskResult.failure('Missing expected podspec platform');
}

// Remove "s.platform = :ios" to work on all platforms, including macOS.
podspecContent = podspecContent.replaceFirst(RegExp(r'.*s\.platform.*'), '');
podspecContent = podspecContent.replaceFirst("s.dependency 'Flutter'", "s.ios.dependency 'Flutter'\ns.osx.dependency 'FlutterMacOS'");

await podspec.writeAsString(podspecContent, flush: true);

// Make PlugintestPlugin.swift compile on iOS and macOS with target conditionals.
final String pluginClass = '${name[0].toUpperCase()}${name.substring(1)}Plugin';
print('pluginClass: $pluginClass');
final File pluginRegister = File(path.join(darwinDir.path, 'Classes', '$pluginClass.swift'));
final String pluginRegisterContent = '''
#if os(macOS)
import FlutterMacOS
#elseif os(iOS)
import Flutter
#endif

public class $pluginClass: NSObject, FlutterPlugin {
public static func register(with registrar: FlutterPluginRegistrar) {
#if os(macOS)
let channel = FlutterMethodChannel(name: "$name", binaryMessenger: registrar.messenger)
#elseif os(iOS)
let channel = FlutterMethodChannel(name: "$name", binaryMessenger: registrar.messenger())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really want to see if we can fix this; it bit me recently too. For Obj-C the change is transparent, but it's unclear to me if there's a way to change it without breaking Swift plugins. (Not for this PR, just venting.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

➕ I was surprised at the differences.

#endif
let instance = $pluginClass()
registrar.addMethodCallDelegate(instance, channel: channel)
}

public func handle(_ call: FlutterMethodCall, result: @escaping FlutterResult) {
#if os(macOS)
result("macOS " + ProcessInfo.processInfo.operatingSystemVersionString)
#elseif os(iOS)
result("iOS " + UIDevice.current.systemVersion)
#endif
}
}
''';
await pluginRegister.writeAsString(pluginRegisterContent, flush: true);
}

Future<void> runFlutterTest() async {
await inDirectory(Directory(rootPath), () async {
await flutter('test');
Expand Down
9 changes: 7 additions & 2 deletions packages/flutter_tools/bin/podhelper.rb
Expand Up @@ -266,14 +266,19 @@ def flutter_install_plugin_pods(application_path = nil, relative_symlink_dir, pl
plugin_name = plugin_hash['name']
plugin_path = plugin_hash['path']
has_native_build = plugin_hash.fetch('native_build', true)

# iOS and macOS code can be shared in "darwin" directory, otherwise
# respectively in "ios" or "macos" directories.
shared_darwin_source = plugin_hash.fetch('shared_darwin_source', false)
platform_directory = shared_darwin_source ? 'darwin' : platform
next unless plugin_name && plugin_path && has_native_build
symlink = File.join(symlink_plugins_dir, plugin_name)
File.symlink(plugin_path, symlink)

# Keep pod path relative so it can be checked into Podfile.lock.
relative = flutter_relative_path_from_podfile(symlink)

pod plugin_name, path: File.join(relative, platform)
pod plugin_name, path: File.join(relative, platform_directory)
end
end

Expand All @@ -288,7 +293,7 @@ def flutter_parse_plugins_file(file, platform)

# dependencies_hash.dig('plugins', 'ios') not available until Ruby 2.3
return [] unless dependencies_hash.has_key?('plugins')
return [] unless dependencies_hash['plugins'].has_key?('ios')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How has this been working?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I guess there aren't many plugins that support macOS but not iOS? I was not pleased when I spotted this though (my fault totally).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path_provider currently doesn't share implementations though, so the macOS implementation package is macOS-only, and it works. 🤔

return [] unless dependencies_hash['plugins'].has_key?(platform)
dependencies_hash['plugins'][platform] || []
end

Expand Down
3 changes: 3 additions & 0 deletions packages/flutter_tools/lib/src/flutter_plugins.dart
Expand Up @@ -104,6 +104,7 @@ const String _kFlutterPluginsNameKey = 'name';
const String _kFlutterPluginsPathKey = 'path';
const String _kFlutterPluginsDependenciesKey = 'dependencies';
const String _kFlutterPluginsHasNativeBuildKey = 'native_build';
const String _kFlutterPluginsSharedDarwinSource = 'shared_darwin_source';

/// Filters [plugins] to those supported by [platformKey].
List<Map<String, Object>> _filterPluginsByPlatform(List<Plugin> plugins, String platformKey) {
Expand All @@ -119,6 +120,8 @@ List<Map<String, Object>> _filterPluginsByPlatform(List<Plugin> plugins, String
pluginInfo.add(<String, Object>{
_kFlutterPluginsNameKey: plugin.name,
_kFlutterPluginsPathKey: globals.fsUtils.escapePath(plugin.path),
if (platformPlugin is DarwinPlugin && (platformPlugin as DarwinPlugin).sharedDarwinSource)
_kFlutterPluginsSharedDarwinSource: (platformPlugin as DarwinPlugin).sharedDarwinSource,
if (platformPlugin is NativeOrDartPlugin)
_kFlutterPluginsHasNativeBuildKey: (platformPlugin as NativeOrDartPlugin).hasMethodChannel() || (platformPlugin as NativeOrDartPlugin).hasFfi(),
_kFlutterPluginsDependenciesKey: <String>[...plugin.dependencies.where(pluginNames.contains)],
Expand Down
37 changes: 33 additions & 4 deletions packages/flutter_tools/lib/src/platform_plugins.dart
Expand Up @@ -19,6 +19,10 @@ const String kFfiPlugin = 'ffiPlugin';
// Constant for 'defaultPackage' key in plugin maps.
const String kDefaultPackage = 'default_package';

/// Constant for 'sharedDarwinSource' key in plugin maps.
/// Can be set for iOS and macOS plugins.
const String kSharedDarwinSource = 'sharedDarwinSource';

/// Constant for 'supportedVariants' key in plugin maps.
const String kSupportedVariants = 'supportedVariants';

Expand Down Expand Up @@ -52,6 +56,11 @@ abstract class NativeOrDartPlugin {
bool hasMethodChannel();
}

abstract class DarwinPlugin {
/// Indicates the iOS and macOS native code is shareable the subdirectory "darwin",
bool get sharedDarwinSource;
}

/// Contains parameters to template an Android plugin.
///
/// The [name] of the plugin is required. Additionally, either:
Expand Down Expand Up @@ -227,15 +236,17 @@ class AndroidPlugin extends PluginPlatform implements NativeOrDartPlugin {
/// - the [dartPluginClass] that will be the entry point for the plugin's
/// Dart code
/// is required.
class IOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
class IOSPlugin extends PluginPlatform implements NativeOrDartPlugin, DarwinPlugin {
const IOSPlugin({
required this.name,
required this.classPrefix,
this.pluginClass,
this.dartPluginClass,
bool? ffiPlugin,
this.defaultPackage,
}) : ffiPlugin = ffiPlugin ?? false;
bool? sharedDarwinSource,
}) : ffiPlugin = ffiPlugin ?? false,
sharedDarwinSource = sharedDarwinSource ?? false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need to be able to explicitly pass sharedDarwinSource: null for some reason? If not we should be able to make it this.sharedDarwinSource = false above and not need this I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes it easier to do:

  sharedDarwinSource: yaml[kSharedDarwinSource] as bool?,

You're right the property should be nonnull though even when the constructor allows null.


factory IOSPlugin.fromYaml(String name, YamlMap yaml) {
assert(validate(yaml)); // TODO(zanderso): https://github.com/flutter/flutter/issues/67241
Expand All @@ -246,6 +257,7 @@ class IOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
dartPluginClass: yaml[kDartPluginClass] as String?,
ffiPlugin: yaml[kFfiPlugin] as bool?,
defaultPackage: yaml[kDefaultPackage] as String?,
sharedDarwinSource: yaml[kSharedDarwinSource] as bool?,
);
}

Expand All @@ -256,6 +268,7 @@ class IOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
return yaml[kPluginClass] is String ||
yaml[kDartPluginClass] is String ||
yaml[kFfiPlugin] == true ||
yaml[kSharedDarwinSource] == true ||
yaml[kDefaultPackage] is String;
}

Expand All @@ -271,6 +284,11 @@ class IOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
final bool ffiPlugin;
final String? defaultPackage;

/// Indicates the iOS native code is shareable with macOS in
/// the subdirectory "darwin", otherwise in the subdirectory "ios".
@override
final bool sharedDarwinSource;

@override
bool hasMethodChannel() => pluginClass != null;

Expand All @@ -288,6 +306,7 @@ class IOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
if (pluginClass != null) 'class': pluginClass,
if (dartPluginClass != null) kDartPluginClass : dartPluginClass,
if (ffiPlugin) kFfiPlugin: true,
if (sharedDarwinSource) kSharedDarwinSource: true,
if (defaultPackage != null) kDefaultPackage : defaultPackage,
};
}
Expand All @@ -298,14 +317,16 @@ class IOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
/// The [name] of the plugin is required. Either [dartPluginClass] or
/// [pluginClass] or [ffiPlugin] are required.
/// [pluginClass] will be the entry point to the plugin's native code.
class MacOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
class MacOSPlugin extends PluginPlatform implements NativeOrDartPlugin, DarwinPlugin {
const MacOSPlugin({
required this.name,
this.pluginClass,
this.dartPluginClass,
bool? ffiPlugin,
this.defaultPackage,
}) : ffiPlugin = ffiPlugin ?? false;
bool? sharedDarwinSource,
}) : ffiPlugin = ffiPlugin ?? false,
sharedDarwinSource = sharedDarwinSource ?? false;

factory MacOSPlugin.fromYaml(String name, YamlMap yaml) {
assert(validate(yaml));
Expand All @@ -320,6 +341,7 @@ class MacOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
dartPluginClass: yaml[kDartPluginClass] as String?,
ffiPlugin: yaml[kFfiPlugin] as bool?,
defaultPackage: yaml[kDefaultPackage] as String?,
sharedDarwinSource: yaml[kSharedDarwinSource] as bool?,
);
}

Expand All @@ -330,6 +352,7 @@ class MacOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
return yaml[kPluginClass] is String ||
yaml[kDartPluginClass] is String ||
yaml[kFfiPlugin] == true ||
yaml[kSharedDarwinSource] == true ||
yaml[kDefaultPackage] is String;
}

Expand All @@ -341,6 +364,11 @@ class MacOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
final bool ffiPlugin;
final String? defaultPackage;

/// Indicates the macOS native code is shareable with iOS in
/// the subdirectory "darwin", otherwise in the subdirectory "macos".
@override
final bool sharedDarwinSource;

@override
bool hasMethodChannel() => pluginClass != null;

Expand All @@ -357,6 +385,7 @@ class MacOSPlugin extends PluginPlatform implements NativeOrDartPlugin {
if (pluginClass != null) 'class': pluginClass,
if (dartPluginClass != null) kDartPluginClass: dartPluginClass,
if (ffiPlugin) kFfiPlugin: true,
if (sharedDarwinSource) kSharedDarwinSource: true,
if (defaultPackage != null) kDefaultPackage: defaultPackage,
};
}
Expand Down