Skip to content

Commit f9f42be

Browse files
camsim99camsim99
authored
[Android] Removes dev dependency plugins from release builds (#158026)
Removes plugins that are `dev_dependency`s from being dependencies of release builds of Flutter Android app projects. Fixes flutter/flutter#157949. ## 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. <!-- Links --> [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
1 parent e1e4ee9 commit f9f42be

File tree

8 files changed

+171
-31
lines changed

8 files changed

+171
-31
lines changed

‎.ci.yaml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,6 +1077,25 @@ targets:
10771077
["devicelab", "hostonly", "linux"]
10781078
task_name: linux_desktop_impeller
10791079

1080+
- name: Linux android_release_builds_exclude_dev_dependencies_test
1081+
recipe: devicelab/devicelab_drone
1082+
timeout: 60
1083+
bringup: true
1084+
properties:
1085+
dependencies: >-
1086+
[
1087+
{"dependency": "android_sdk", "version": "version:35v1"},
1088+
{"dependency": "chrome_and_driver", "version": "version:125.0.6422.141"},
1089+
{"dependency": "open_jdk", "version": "version:17"}
1090+
]
1091+
tags: >
1092+
["devicelab", "hostonly", "linux"]
1093+
task_name: android_release_builds_exclude_dev_dependencies_test
1094+
runIf:
1095+
- dev/**
1096+
- bin/**
1097+
- .ci.yaml
1098+
10801099
- name: Linux run_release_test_linux
10811100
recipe: devicelab/devicelab_drone
10821101
timeout: 60

‎TESTOWNERS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@
230230

231231
## Host only DeviceLab tests
232232
/dev/devicelab/bin/tasks/animated_complex_opacity_perf_macos__e2e_summary.dart @cbracken @flutter/desktop
233+
/dev/devicelab/bin/tasks/android_release_builds_exclude_dev_dependencies_test.dart @camsim99 @flutter/android
233234
/dev/devicelab/bin/tasks/basic_material_app_macos__compile.dart @cbracken @flutter/desktop
234235
/dev/devicelab/bin/tasks/build_aar_module_test.dart @andrewkolos @flutter/tool
235236
/dev/devicelab/bin/tasks/build_ios_framework_module_test.dart @jmagman @flutter/tool

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
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 'dart:async';
6+
import 'dart:io';
7+
8+
import 'package:flutter_devicelab/framework/apk_utils.dart';
9+
import 'package:flutter_devicelab/framework/framework.dart';
10+
import 'package:flutter_devicelab/framework/task_result.dart';
11+
import 'package:flutter_devicelab/framework/utils.dart';
12+
import 'package:path/path.dart' as path;
13+
14+
Future<void> main() async {
15+
await task(() async {
16+
try {
17+
await runProjectTest((FlutterProject flutterProject) async {
18+
// Create dev_dependency plugin to use for test.
19+
final Directory tempDir = Directory.systemTemp.createTempSync('android_release_builds_exclude_dev_dependencies_test.');
20+
const String devDependencyPluginOrg = 'com.example.dev_dependency_plugin';
21+
await FlutterPluginProject.create(tempDir, 'dev_dependency_plugin', options: <String>['--platforms=android', '--org=$devDependencyPluginOrg']);
22+
23+
// Add devDependencyPlugin as dependency of flutterProject.
24+
await flutterProject.addPlugin('dev_dependency_plugin', options: <String>['--path', path.join(tempDir.path, 'dev_dependency_plugin')]);
25+
26+
final List<String> buildModesToTest = <String>['debug', 'profile', 'release'];
27+
for (final String buildMode in buildModesToTest) {
28+
section('APK does contain methods from dev dependency in $buildMode mode');
29+
30+
// Build APK in buildMode and check that devDependencyPlugin is included/excluded in the APK as expected.
31+
await inDirectory(flutterProject.rootPath, () async {
32+
await flutter('build', options: <String>[
33+
'apk',
34+
'--$buildMode',
35+
'--target-platform=android-arm',
36+
]);
37+
38+
final File apk = File(path.join(flutterProject.rootPath, 'build', 'app', 'outputs', 'flutter-apk', 'app-debug.apk'));
39+
if (!apk.existsSync()) {
40+
throw TaskResult.failure("Expected ${apk.path} to exist, but it doesn't.");
41+
}
42+
43+
// We expect the APK to include the devDependencyPlugin except in release mode.
44+
final bool isTestingReleaseMode = buildMode == 'release';
45+
final bool apkIncludesDevDependency = await checkApkContainsMethodsFromLibrary(apk, devDependencyPluginOrg);
46+
final bool apkIncludesDevDependencyAsExpected = isTestingReleaseMode ? !apkIncludesDevDependency : apkIncludesDevDependency;
47+
if (!apkIncludesDevDependencyAsExpected) {
48+
return TaskResult.failure('Expected to${isTestingReleaseMode ? ' not' : ''} find dev_dependency_plugin in APK built with debug mode but did${isTestingReleaseMode ? '' : ' not'}.');
49+
}
50+
});
51+
}
52+
});
53+
return TaskResult.success(null);
54+
} on TaskResult catch (taskResult) {
55+
return taskResult;
56+
} catch (e) {
57+
return TaskResult.failure(e.toString());
58+
}
59+
});
60+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ Future<void> main() async {
1616
await runProjectTest((FlutterProject flutterProject) async {
1717
section('APK contains plugin classes');
1818
await flutterProject.setMinSdkVersion(20);
19-
flutterProject.addPlugin('google_maps_flutter', value: '^2.2.1');
19+
await flutterProject.addPlugin('google_maps_flutter:^2.2.1');
2020

2121
await inDirectory(flutterProject.rootPath, () async {
2222
await flutter('build', options: <String>[

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,7 @@ Future<void> main() async {
210210
});
211211

212212
section('Configure');
213-
project.addPlugin('plugin_under_test',
214-
value: '${Platform.lineTerminator} path: ${pluginDir.path}');
213+
await project.addPlugin('plugin_under_test', options: <String>['--path', pluginDir.path]);
215214
await project.addCustomBuildType('local', initWith: 'debug');
216215
await project.getPackages();
217216

@@ -229,7 +228,7 @@ Future<void> main() async {
229228
section('gradlew assembleLocal (plugin with custom build type)');
230229
await project.addCustomBuildType('local', initWith: 'debug');
231230
section('Add plugin');
232-
project.addPlugin('path_provider');
231+
await project.addPlugin('path_provider');
233232
await project.getPackages();
234233

235234
await project.runGradleTask('assembleLocal');

‎dev/devicelab/lib/framework/apk_utils.dart‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,17 @@ class ApkExtractor {
192192
_extracted = true;
193193
}
194194

195+
/// Returns true if APK contains classes from library with given [libraryName].
196+
Future<bool> containsLibrary(String libraryName) async {
197+
await _extractDex();
198+
for (final String className in _classes) {
199+
if (className.startsWith(libraryName)) {
200+
return true;
201+
}
202+
}
203+
return false;
204+
}
205+
195206
/// Returns true if the APK contains a given class.
196207
Future<bool> containsClass(String className) async {
197208
await _extractDex();
@@ -218,6 +229,14 @@ Future<String> getAndroidManifest(String apk) async {
218229
);
219230
}
220231

232+
/// Checks that the [apk] includes any classes from a particularly library with
233+
/// given [libraryName] in the [apk] and returns true if so, false otherwise.
234+
Future<bool> checkApkContainsMethodsFromLibrary(File apk, String libraryName) async {
235+
final ApkExtractor extractor = ApkExtractor(apk);
236+
final bool apkContainsMethodsFromLibrary = await extractor.containsLibrary(libraryName);
237+
return apkContainsMethodsFromLibrary;
238+
}
239+
221240
/// Checks that the classes are contained in the APK, throws otherwise.
222241
Future<void> checkApkContainsClasses(File apk, List<String> classes) async {
223242
final ApkExtractor extractor = ApkExtractor(apk);
@@ -272,16 +291,17 @@ android {
272291
}
273292

274293
/// Adds a plugin to the pubspec.
275-
/// In pubspec, each dependency is expressed as key, value pair joined by a colon `:`.
276-
/// such as `plugin_a`:`^0.0.1` or `plugin_a`:`\npath: /some/path`.
277-
void addPlugin(String plugin, { String value = '' }) {
278-
final File pubspec = File(path.join(rootPath, 'pubspec.yaml'));
279-
String content = pubspec.readAsStringSync();
280-
content = content.replaceFirst(
281-
'${Platform.lineTerminator}dependencies:${Platform.lineTerminator}',
282-
'${Platform.lineTerminator}dependencies:${Platform.lineTerminator} $plugin: $value${Platform.lineTerminator}',
283-
);
284-
pubspec.writeAsStringSync(content, flush: true);
294+
///
295+
/// If a particular version of the [plugin] is desired, it should be included
296+
/// in the name as it would be in the `flutter pub add` command, e.g.
297+
/// `google_maps_flutter:^2.2.1`.
298+
///
299+
/// Include all other desired options for running `flutter pub add` to
300+
/// [options], e.g. `<String>['--path', 'path/to/plugin']`.
301+
Future<void> addPlugin(String plugin, {List<String> options = const <String>[]}) async {
302+
await inDirectory(Directory(rootPath), () async {
303+
await flutter('pub', options: <String>['add', plugin, ...options]);
304+
});
285305
}
286306

287307
Future<void> setMinSdkVersion(int sdkVersion) async {
@@ -367,9 +387,9 @@ class FlutterPluginProject {
367387
final Directory parent;
368388
final String name;
369389

370-
static Future<FlutterPluginProject> create(Directory directory, String name) async {
390+
static Future<FlutterPluginProject> create(Directory directory, String name, {List<String> options = const <String>['--platforms=ios,android']}) async {
371391
await inDirectory(directory, () async {
372-
await flutter('create', options: <String>['--template=plugin', '--platforms=ios,android', name]);
392+
await flutter('create', options: <String>['--template=plugin', ...options, name]);
373393
});
374394
return FlutterPluginProject(directory, name);
375395
}

‎packages/flutter_tools/gradle/src/main/groovy/flutter.groovy‎

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -769,10 +769,18 @@ class FlutterPlugin implements Plugin<Project> {
769769
// Apply the "flutter" Gradle extension to plugins so that they can use it's vended
770770
// compile/target/min sdk values.
771771
pluginProject.extensions.create("flutter", FlutterExtension)
772+
772773
// Add plugin dependency to the app project.
773-
project.dependencies {
774-
api(pluginProject)
774+
project.android.buildTypes.each { buildType ->
775+
String flutterBuildMode = buildModeFor(buildType)
776+
if (flutterBuildMode != "release" || !pluginObject.dev_dependency) {
777+
// Only add dependency on dev dependencies in non-release builds.
778+
project.dependencies {
779+
api(pluginProject)
780+
}
781+
}
775782
}
783+
776784
Closure addEmbeddingDependencyToPlugin = { buildType ->
777785
String flutterBuildMode = buildModeFor(buildType)
778786
// In AGP 3.5, the embedding must be added as an API implementation,
@@ -784,6 +792,12 @@ class FlutterPlugin implements Plugin<Project> {
784792
if (!pluginProject.hasProperty("android")) {
785793
return
786794
}
795+
if (flutterBuildMode == "release" && pluginObject.dev_dependency) {
796+
// This plugin is a dev dependency and will not be included in
797+
// the release build, so no need to add the embedding
798+
// dependency to it.
799+
return
800+
}
787801
// Copy build types from the app to the plugin.
788802
// This allows to build apps with plugins and custom build types or flavors.
789803
pluginProject.android.buildTypes {
@@ -944,20 +958,29 @@ class FlutterPlugin implements Plugin<Project> {
944958
if (pluginProject == null) {
945959
return
946960
}
947-
def dependencies = pluginObject.dependencies
948-
assert(dependencies instanceof List<String>)
949-
dependencies.each { pluginDependencyName ->
950-
if (pluginDependencyName.empty) {
951-
return
952-
}
953-
Project dependencyProject = project.rootProject.findProject(":$pluginDependencyName")
954-
if (dependencyProject == null) {
961+
962+
project.android.buildTypes.each { buildType ->
963+
String flutterBuildMode = buildModeFor(buildType)
964+
if (flutterBuildMode == "release" && pluginObject.dev_dependency) {
965+
// This plugin is a dev dependency will not be included in the
966+
// release build, so no need to add its dependencies.
955967
return
956968
}
957-
// Wait for the Android plugin to load and add the dependency to the plugin project.
958-
pluginProject.afterEvaluate {
959-
pluginProject.dependencies {
960-
implementation(dependencyProject)
969+
def dependencies = pluginObject.dependencies
970+
assert(dependencies instanceof List<String>)
971+
dependencies.each { pluginDependencyName ->
972+
if (pluginDependencyName.empty) {
973+
return
974+
}
975+
Project dependencyProject = project.rootProject.findProject(":$pluginDependencyName")
976+
if (dependencyProject == null) {
977+
return
978+
}
979+
// Wait for the Android plugin to load and add the dependency to the plugin project.
980+
pluginProject.afterEvaluate {
981+
pluginProject.dependencies {
982+
implementation(dependencyProject)
983+
}
961984
}
962985
}
963986
}

‎packages/flutter_tools/gradle/src/main/groovy/native_plugin_loader.groovy‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ class NativePluginLoader {
1919
* "path": "/path/to/plugin-a",
2020
* "dependencies": ["plugin-b", "plugin-c"],
2121
* "native_build": true
22+
* "dev_dependency": false
2223
* }
2324
*
24-
* Therefore the map value can either be a `String`, a `List<String>` or a `boolean`.
25+
* Therefore the map value can either be a `String`, a `List<String>` or a `Boolean`.
2526
*/
2627
List<Map<String, Object>> getPlugins(File flutterSourceDirectory) {
2728
List<Map<String, Object>> nativePlugins = []
@@ -40,6 +41,7 @@ class NativePluginLoader {
4041
assert(androidPlugin.name instanceof String)
4142
assert(androidPlugin.path instanceof String)
4243
assert(androidPlugin.dependencies instanceof List<String>)
44+
assert(androidPlugin.dev_dependency instanceof Boolean)
4345
// Skip plugins that have no native build (such as a Dart-only implementation
4446
// of a federated plugin).
4547
def needsBuild = androidPlugin.containsKey(nativeBuildKey) ? androidPlugin[nativeBuildKey] : true
@@ -66,18 +68,28 @@ class NativePluginLoader {
6668
// "path": "/path/to/plugin-a",
6769
// "dependencies": ["plugin-b", "plugin-c"],
6870
// "native_build": true
71+
// "dev_dependency": false
6972
// },
7073
// {
7174
// "name": "plugin-b",
7275
// "path": "/path/to/plugin-b",
7376
// "dependencies": ["plugin-c"],
7477
// "native_build": true
78+
// "dev_dependency": false
7579
// },
7680
// {
7781
// "name": "plugin-c",
7882
// "path": "/path/to/plugin-c",
7983
// "dependencies": [],
8084
// "native_build": true
85+
// "dev_dependency": false
86+
// },
87+
// {
88+
// "name": "plugin-d",
89+
// "path": "/path/to/plugin-d",
90+
// "dependencies": [],
91+
// "native_build": true
92+
// "dev_dependency": true
8193
// },
8294
// ],
8395
// },
@@ -93,12 +105,18 @@ class NativePluginLoader {
93105
// {
94106
// "name": "plugin-c",
95107
// "dependencies": []
108+
// },
109+
// {
110+
// "name": "plugin-d",
111+
// "dependencies": []
96112
// }
97113
// ]
98114
// }
99115
// This means, `plugin-a` depends on `plugin-b` and `plugin-c`.
100116
// `plugin-b` depends on `plugin-c`.
101117
// `plugin-c` doesn't depend on anything.
118+
// `plugin-d` also doesn't depend on anything, but it is a dev
119+
// dependency to the Flutter project, so it is marked as such.
102120
if (parsedFlutterPluginsDependencies) {
103121
return parsedFlutterPluginsDependencies
104122
}

0 commit comments

Comments
 (0)