From fffbf663ff34ea292b7e7160a16b106d963202ff Mon Sep 17 00:00:00 2001 From: Camille Simon <43054281+camsim99@users.noreply.github.com> Date: Thu, 30 Jan 2025 11:51:07 -0600 Subject: [PATCH] Removes dev dependencies from generated plugin registrant for non-Android platforms (#161828) Removes dev dependencies from the generated plugin registrants for all platforms since they will be removed from release builds (this was already done for Android, mostly in https://github.com/flutter/flutter/pull/161343). Fixes https://github.com/flutter/flutter/issues/161348. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../lib/src/flutter_plugins.dart | 28 +-- .../lib/src/platform_plugins.dart | 2 +- packages/flutter_tools/lib/src/project.dart | 12 + .../test/general.shard/plugins_test.dart | 231 ++++++++++++++++++ .../general.shard/windows/plugins_test.dart | 58 ----- .../flutter_tools/test/src/fake_pub_deps.dart | 3 +- 6 files changed, 255 insertions(+), 79 deletions(-) delete mode 100644 packages/flutter_tools/test/general.shard/windows/plugins_test.dart diff --git a/packages/flutter_tools/lib/src/flutter_plugins.dart b/packages/flutter_tools/lib/src/flutter_plugins.dart index a73e9dd991..bb896919c0 100644 --- a/packages/flutter_tools/lib/src/flutter_plugins.dart +++ b/packages/flutter_tools/lib/src/flutter_plugins.dart @@ -363,20 +363,11 @@ List> _extractPlatformMaps(Iterable plugins, String ]; } -Future _writeAndroidPluginRegistrant( - FlutterProject project, - List plugins, { - required bool releaseMode, -}) async { - Iterable methodChannelPlugins = _filterMethodChannelPlugins( +Future _writeAndroidPluginRegistrant(FlutterProject project, List plugins) async { + final List methodChannelPlugins = _filterMethodChannelPlugins( plugins, AndroidPlugin.kConfigKey, ); - // TODO(camsim99): Remove dev dependencies from release builds for all platforms. See https://github.com/flutter/flutter/issues/161348. - if (releaseMode) { - methodChannelPlugins = methodChannelPlugins.where((Plugin p) => !p.isDevDependency); - } - final List> androidPlugins = _extractPlatformMaps( methodChannelPlugins, AndroidPlugin.kConfigKey, @@ -1204,7 +1195,7 @@ Future injectBuildTimePluginFilesForWebPlatform( /// Injects plugins found in `pubspec.yaml` into the platform-specific projects. /// -/// The injected files are required by the flutter app as soon as possible, so +/// The injected files are required by the Flutter app as soon as possible, so /// it can be built. /// /// Files written by this method end up in platform-specific locations that are @@ -1225,18 +1216,19 @@ Future injectPlugins( DarwinDependencyManagement? darwinDependencyManagement, bool? releaseMode, }) async { - final List plugins = await findPlugins(project); + List plugins = await findPlugins(project); + + if (releaseMode ?? false) { + plugins = plugins.where((Plugin p) => !p.isDevDependency).toList(); + } + final Map> pluginsByPlatform = _resolvePluginImplementations( plugins, pluginResolutionType: _PluginResolutionType.nativeOrDart, ); if (androidPlatform) { - await _writeAndroidPluginRegistrant( - project, - pluginsByPlatform[AndroidPlugin.kConfigKey]!, - releaseMode: releaseMode ?? false, - ); + await _writeAndroidPluginRegistrant(project, pluginsByPlatform[AndroidPlugin.kConfigKey]!); } if (iosPlatform) { await _writeIOSPluginRegistrant(project, pluginsByPlatform[IOSPlugin.kConfigKey]!); diff --git a/packages/flutter_tools/lib/src/platform_plugins.dart b/packages/flutter_tools/lib/src/platform_plugins.dart index dfcd6b2424..ff19f04e76 100644 --- a/packages/flutter_tools/lib/src/platform_plugins.dart +++ b/packages/flutter_tools/lib/src/platform_plugins.dart @@ -244,7 +244,7 @@ class AndroidPlugin extends PluginPlatform implements NativeOrDartPlugin { /// The [name] of the plugin is required. Additionally, either: /// - [defaultPackage], or /// - an implementation consisting of: -/// - the [pluginClass] (with optional [classPrefix]) that will be the entry +/// - the [classPrefix] (with optional [pluginClass]) that will be the entry /// point to the plugin's native code, and/or /// - the [dartPluginClass] with optional [dartFileName] that will be /// the entry point for the plugin's Dart code diff --git a/packages/flutter_tools/lib/src/project.dart b/packages/flutter_tools/lib/src/project.dart index b00e070af2..6e6e1a735f 100644 --- a/packages/flutter_tools/lib/src/project.dart +++ b/packages/flutter_tools/lib/src/project.dart @@ -637,6 +637,18 @@ class AndroidProject extends FlutterProjectPlatform { return hostAppGradleRoot.childFile('AndroidManifest.xml'); } + File get generatedPluginRegistrantFile { + return hostAppGradleRoot + .childDirectory('app') + .childDirectory('src') + .childDirectory('main') + .childDirectory('java') + .childDirectory('io') + .childDirectory('flutter') + .childDirectory('plugins') + .childFile('GeneratedPluginRegistrant.java'); + } + File get gradleAppOutV1File => gradleAppOutV1Directory.childFile('app-debug.apk'); Directory get gradleAppOutV1Directory { diff --git a/packages/flutter_tools/test/general.shard/plugins_test.dart b/packages/flutter_tools/test/general.shard/plugins_test.dart index 826d758dd5..4a49019486 100644 --- a/packages/flutter_tools/test/general.shard/plugins_test.dart +++ b/packages/flutter_tools/test/general.shard/plugins_test.dart @@ -2510,6 +2510,226 @@ The Flutter Preview device does not support the following plugins from your pubs returnsNormally, ); }); + + group('injectPlugins in release mode', () { + const String testPluginName = 'test_plugin'; + + // Fake pub to override dev dependencies of flutterProject. + final Pub fakePubWithTestPluginDevDependency = FakePubWithPrimedDeps( + devDependencies: {testPluginName}, + ); + + testUsingContext( + 'excludes dev dependencies from Android plugin registrant', + () async { + final Directory pluginDir = createPlugin( + name: testPluginName, + platforms: const { + 'android': _PluginPlatformInfo(pluginClass: 'Foo', androidPackage: 'bar.foo'), + }, + ); + + // injectPlugins will fail if main native class not found in expected spot, so add + // it first. + pluginDir + .childDirectory('android') + .childDirectory('src') + .childDirectory('main') + .childDirectory('java') + .childDirectory('bar') + .childDirectory('foo') + .childFile('Foo.java') + ..createSync(recursive: true) + ..writeAsStringSync('import io.flutter.embedding.engine.plugins.FlutterPlugin;'); + + // Test non-release mode. + await injectPlugins(flutterProject, androidPlatform: true, releaseMode: false); + final File generatedPluginRegistrant = + flutterProject.android.generatedPluginRegistrantFile; + expect(generatedPluginRegistrant, exists); + expect(generatedPluginRegistrant.readAsStringSync(), contains('bar.foo.Foo')); + + // Test release mode. + await injectPlugins(flutterProject, androidPlatform: true, releaseMode: true); + expect(generatedPluginRegistrant, exists); + expect(generatedPluginRegistrant.readAsStringSync(), isNot(contains('bar.foo.Foo'))); + }, + overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.any(), + FeatureFlags: enableExplicitPackageDependencies, + Pub: () => fakePubWithTestPluginDevDependency, + }, + ); + + testUsingContext( + 'excludes dev dependencies from iOS plugin registrant', + () async { + createPlugin( + name: testPluginName, + platforms: const { + 'ios': _PluginPlatformInfo(pluginClass: 'Foo'), + }, + ); + + final FakeDarwinDependencyManagement dependencyManagement = + FakeDarwinDependencyManagement(); + const String devDepImport = '#import <$testPluginName/Foo.h>'; + + // Test non-release mode. + await injectPlugins( + flutterProject, + iosPlatform: true, + darwinDependencyManagement: dependencyManagement, + releaseMode: false, + ); + final File generatedPluginRegistrantImpl = + flutterProject.ios.pluginRegistrantImplementation; + expect(generatedPluginRegistrantImpl, exists); + expect(generatedPluginRegistrantImpl.readAsStringSync(), contains(devDepImport)); + + // Test release mode. + await injectPlugins( + flutterProject, + iosPlatform: true, + darwinDependencyManagement: dependencyManagement, + releaseMode: true, + ); + expect(generatedPluginRegistrantImpl, exists); + expect(generatedPluginRegistrantImpl.readAsStringSync(), isNot(contains(devDepImport))); + }, + overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.any(), + FeatureFlags: enableExplicitPackageDependencies, + Pub: () => fakePubWithTestPluginDevDependency, + }, + ); + + testUsingContext( + 'excludes dev dependencies from Linux plugin registrant', + () async { + createPlugin( + name: testPluginName, + platforms: const { + 'linux': _PluginPlatformInfo(pluginClass: 'Foo'), + }, + ); + + const String expectedDevDepImport = '#include <$testPluginName/foo.h>'; + + // Test non-release mode. + await injectPlugins(flutterProject, linuxPlatform: true, releaseMode: false); + final File generatedPluginRegistrant = flutterProject.linux.managedDirectory.childFile( + 'generated_plugin_registrant.cc', + ); + expect(generatedPluginRegistrant, exists); + expect(generatedPluginRegistrant.readAsStringSync(), contains(expectedDevDepImport)); + + // Test release mode. + await injectPlugins(flutterProject, linuxPlatform: true, releaseMode: true); + expect(generatedPluginRegistrant, exists); + expect( + generatedPluginRegistrant.readAsStringSync(), + isNot(contains(expectedDevDepImport)), + ); + }, + overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.any(), + FeatureFlags: enableExplicitPackageDependencies, + Pub: () => fakePubWithTestPluginDevDependency, + }, + ); + + testUsingContext( + 'excludes dev dependencies from MacOS plugin registrant', + () async { + createPlugin( + name: testPluginName, + platforms: const { + 'macos': _PluginPlatformInfo(pluginClass: 'Foo'), + }, + ); + final FakeDarwinDependencyManagement dependencyManagement = + FakeDarwinDependencyManagement(); + const String expectedDevDepRegistration = 'Foo.register'; + + // Test non-release mode. + await injectPlugins( + flutterProject, + macOSPlatform: true, + darwinDependencyManagement: dependencyManagement, + releaseMode: false, + ); + final File generatedPluginRegistrant = flutterProject.macos.managedDirectory.childFile( + 'GeneratedPluginRegistrant.swift', + ); + expect(generatedPluginRegistrant, exists); + expect( + generatedPluginRegistrant.readAsStringSync(), + contains(expectedDevDepRegistration), + ); + + // Test release mode. + await injectPlugins( + flutterProject, + macOSPlatform: true, + darwinDependencyManagement: dependencyManagement, + releaseMode: true, + ); + expect(generatedPluginRegistrant, exists); + expect( + generatedPluginRegistrant.readAsStringSync(), + isNot(contains(expectedDevDepRegistration)), + ); + }, + overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.any(), + FeatureFlags: enableExplicitPackageDependencies, + Pub: () => fakePubWithTestPluginDevDependency, + }, + ); + + testUsingContext( + 'excludes dev dependencies from Windows plugin registrant', + () async { + createPlugin( + name: testPluginName, + platforms: const { + 'windows': _PluginPlatformInfo(pluginClass: 'Foo'), + }, + ); + + const String expectedDevDepRegistration = '#include <$testPluginName/foo.h>'; + + // Test non-release mode. + await injectPlugins(flutterProject, windowsPlatform: true, releaseMode: false); + final File generatedPluginRegistrantImpl = flutterProject.windows.managedDirectory + .childFile('generated_plugin_registrant.cc'); + expect(generatedPluginRegistrantImpl, exists); + expect( + generatedPluginRegistrantImpl.readAsStringSync(), + contains(expectedDevDepRegistration), + ); + + // Test release mode. + await injectPlugins(flutterProject, windowsPlatform: true, releaseMode: true); + expect(generatedPluginRegistrantImpl, exists); + expect( + generatedPluginRegistrantImpl.readAsStringSync(), + isNot(contains(expectedDevDepRegistration)), + ); + }, + overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.any(), + FeatureFlags: enableExplicitPackageDependencies, + Pub: () => fakePubWithTestPluginDevDependency, + }, + ); + }); }); testUsingContext( @@ -2718,6 +2938,17 @@ class FakeAndroidProject extends Fake implements AndroidProject { AndroidEmbeddingVersionResult computeEmbeddingVersion() { return AndroidEmbeddingVersionResult(embeddingVersion, 'reasons for version'); } + + @override + File get generatedPluginRegistrantFile => hostAppGradleRoot + .childDirectory('app') + .childDirectory('src') + .childDirectory('main') + .childDirectory('java') + .childDirectory('io') + .childDirectory('flutter') + .childDirectory('plugins') + .childFile('GeneratedPluginRegistrant.java'); } class FakeWebProject extends Fake implements WebProject { diff --git a/packages/flutter_tools/test/general.shard/windows/plugins_test.dart b/packages/flutter_tools/test/general.shard/windows/plugins_test.dart deleted file mode 100644 index 670d00513c..0000000000 --- a/packages/flutter_tools/test/general.shard/windows/plugins_test.dart +++ /dev/null @@ -1,58 +0,0 @@ -// 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:file/file.dart'; -import 'package:file/memory.dart'; -import 'package:file_testing/file_testing.dart'; -import 'package:flutter_tools/src/base/template.dart'; -import 'package:flutter_tools/src/flutter_plugins.dart'; -import 'package:flutter_tools/src/isolated/mustache_template.dart'; -import 'package:flutter_tools/src/platform_plugins.dart'; -import 'package:flutter_tools/src/plugins.dart'; -import 'package:flutter_tools/src/project.dart'; - -import '../../src/common.dart'; - -const TemplateRenderer renderer = MustacheTemplateRenderer(); - -void main() { - testWithoutContext('Win32 injects Win32 plugins', () async { - final FileSystem fileSystem = MemoryFileSystem.test(); - setUpProject(fileSystem); - final FlutterProject flutterProject = FlutterProject.fromDirectoryTest( - fileSystem.currentDirectory, - ); - - await writeWindowsPluginFiles(flutterProject, [ - Plugin( - name: 'test', - path: 'foo', - defaultPackagePlatforms: const {}, - pluginDartClassPlatforms: const {}, - platforms: const { - WindowsPlugin.kConfigKey: WindowsPlugin( - name: 'test', - pluginClass: 'Foo', - variants: {PluginPlatformVariant.win32}, - ), - }, - dependencies: [], - isDirectDependency: true, - isDevDependency: false, - ), - ], renderer); - - final Directory managed = flutterProject.windows.managedDirectory; - expect(flutterProject.windows.generatedPluginCmakeFile, exists); - expect(managed.childFile('generated_plugin_registrant.h'), exists); - expect( - managed.childFile('generated_plugin_registrant.cc').readAsStringSync(), - contains('#include '), - ); - }); -} - -void setUpProject(FileSystem fileSystem) { - fileSystem.file('pubspec.yaml').createSync(); -} diff --git a/packages/flutter_tools/test/src/fake_pub_deps.dart b/packages/flutter_tools/test/src/fake_pub_deps.dart index c56aa6e19c..53dc61dcbf 100644 --- a/packages/flutter_tools/test/src/fake_pub_deps.dart +++ b/packages/flutter_tools/test/src/fake_pub_deps.dart @@ -27,8 +27,7 @@ final class FakePubWithPrimedDeps implements Pub { 'name': rootPackageName, 'kind': 'root', 'dependencies': [...dependencies.keys, ...devDependencies]..sort(), - 'directDependencies': [...?dependencies[rootPackageName], ...devDependencies] - ..sort(), + 'directDependencies': [...?dependencies[rootPackageName]]..sort(), 'devDependencies': [...devDependencies], }, ];