Skip to content

Commit 522eda0

Browse files
camsim99matanlurey
camsim99
and
matanlurey
authored
[Android] Fix integration test to check if dev dependencies are removed from release builds + address no non-dev dependency plugin edge case (#161826)
Makes the following change to fix the case where dev dependencies are stripped in apps that only contain dev dependencies: - Changes the Flutter Gradle plugin to add the Flutter embedding as a direct dependency of a Flutter app if it contains no plugins that would include it transitively (this excludes dev dependencies in release builds) Makes the following changes to correct + improve the integration test that checks if dev dependencies are stripped from release builds: - Fixes the plugin that was supposed to be as dev dependency to actually be a dev dependency - Changes the test structure to check the output of `./gradlew app:dependencies` (see [more details on this call](https://docs.gradle.org/current/userguide/viewing_debugging_dependencies.html)) instead of the classes included in the APK. Checking the APK was leading us astray because it appears obfuscation occurs for release builds. - Adds more sections, comments, etc. Fixes flutter/flutter#161780. ## 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. - [ ] 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 --------- Co-authored-by: Matan Lurey <[email protected]>
1 parent 2c1e4b1 commit 522eda0

File tree

2 files changed

+83
-35
lines changed

2 files changed

+83
-35
lines changed

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

Lines changed: 61 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,72 +8,100 @@ import 'dart:io';
88
import 'package:flutter_devicelab/framework/apk_utils.dart';
99
import 'package:flutter_devicelab/framework/framework.dart';
1010
import 'package:flutter_devicelab/framework/task_result.dart';
11-
import 'package:flutter_devicelab/framework/utils.dart';
11+
import 'package:flutter_devicelab/framework/utils.dart' as utils;
1212
import 'package:path/path.dart' as path;
1313

1414
Future<void> main() async {
1515
await task(() async {
1616
try {
1717
await runProjectTest((FlutterProject flutterProject) async {
18+
utils.section(
19+
'Configure plugins to be marked as dev dependencies in .flutter-plugins-dependencies file',
20+
);
21+
1822
// Enable plugins being marked as dev dependncies in the .flutter-plugins-dependencies file.
19-
await flutter('config', options: <String>['--explicit-package-dependencies']);
23+
await utils.flutter('config', options: <String>['--explicit-package-dependencies']);
2024

2125
// Create dev_dependency plugin to use for test.
2226
final Directory tempDir = Directory.systemTemp.createTempSync(
2327
'android_release_builds_exclude_dev_dependencies_test.',
2428
);
2529
const String devDependencyPluginOrg = 'com.example.dev_dependency_plugin';
30+
31+
utils.section('Create plugin dev_dependency_plugin that supports Android');
32+
2633
await FlutterPluginProject.create(
2734
tempDir,
2835
'dev_dependency_plugin',
2936
options: <String>['--platforms=android', '--org=$devDependencyPluginOrg'],
3037
);
3138

39+
utils.section('Add dev_dependency_plugin as a dev dependency to the Flutter app project');
40+
3241
// Add devDependencyPlugin as dependency of flutterProject.
3342
await flutterProject.addPlugin(
34-
'dev_dependency_plugin',
43+
'dev:dev_dependency_plugin',
3544
options: <String>['--path', path.join(tempDir.path, 'dev_dependency_plugin')],
3645
);
3746

47+
utils.section(
48+
'Verify the app includes/excludes dev_dependency_plugin as dependency in each build mode as expected',
49+
);
3850
final List<String> buildModesToTest = <String>['debug', 'profile', 'release'];
3951
for (final String buildMode in buildModesToTest) {
40-
section('APK does contain methods from dev dependency in $buildMode mode');
52+
final String gradlew = Platform.isWindows ? 'gradlew.bat' : 'gradlew';
53+
final String gradlewExecutable = Platform.isWindows ? '.\\$gradlew' : './$gradlew';
54+
final RegExp regExpToMatchDevDependencyPlugin = RegExp(
55+
r'--- project :dev_dependency_plugin',
56+
);
57+
final RegExp regExpToMatchDevDependencyPluginWithTransitiveDependencies = RegExp(
58+
r'--- project :dev_dependency_plugin\n(\s)*\+--- org.jetbrains.kotlin.*\s\(\*\)\n(\s)*\\---\sio.flutter:flutter_embedding_' +
59+
buildMode,
60+
);
61+
const String stringToMatchFlutterEmbedding = '+--- io.flutter:flutter_embedding_release:';
62+
final bool isTestingReleaseMode = buildMode == 'release';
4163

42-
// Build APK in buildMode and check that devDependencyPlugin is included/excluded in the APK as expected.
43-
await inDirectory(flutterProject.rootPath, () async {
44-
await flutter(
45-
'build',
46-
options: <String>['apk', '--$buildMode', '--target-platform=android-arm'],
47-
);
64+
utils.section('Query the dependencies of the app built with $buildMode');
4865

49-
final File apk = File(
50-
path.join(
51-
flutterProject.rootPath,
52-
'build',
53-
'app',
54-
'outputs',
55-
'flutter-apk',
56-
'app-$buildMode.apk',
57-
),
58-
);
59-
if (!apk.existsSync()) {
60-
throw TaskResult.failure("Expected ${apk.path} to exist, but it doesn't.");
61-
}
66+
final String appDependencies = await utils.eval(gradlewExecutable, <String>[
67+
'app:dependencies',
68+
'--configuration',
69+
'${buildMode}RuntimeClasspath',
70+
], workingDirectory: flutterProject.androidPath);
6271

63-
// We expect the APK to include the devDependencyPlugin except in release mode.
64-
final bool isTestingReleaseMode = buildMode == 'release';
65-
final bool apkIncludesDevDependency = await checkApkContainsMethodsFromLibrary(
66-
apk,
67-
devDependencyPluginOrg,
72+
if (isTestingReleaseMode) {
73+
utils.section(
74+
'Check that the release build includes Flutter embedding as a direct dependency',
6875
);
69-
final bool apkIncludesDevDependencyAsExpected =
70-
isTestingReleaseMode ? !apkIncludesDevDependency : apkIncludesDevDependency;
71-
if (!apkIncludesDevDependencyAsExpected) {
76+
77+
if (!appDependencies.contains(stringToMatchFlutterEmbedding)) {
78+
// We expect dev_dependency_plugin to not be included in the dev dependency, but the Flutter
79+
// embedding should still be a dependency of the app project (regardless of the fact
80+
// that the app does not depend on any plugins that support Android, which would cause the
81+
// Flutter embedding to be included as a transitive dependency).
7282
throw TaskResult.failure(
73-
'Expected to${isTestingReleaseMode ? ' not' : ''} find dev_dependency_plugin in APK built with debug mode but did${isTestingReleaseMode ? '' : ' not'}.',
83+
'Expected to find the Flutter embedding as a dependency of the release app build, but did not.',
7484
);
7585
}
76-
});
86+
}
87+
88+
utils.section(
89+
'Check that the $buildMode build includes/excludes the dev dependency plugin as expected',
90+
);
91+
92+
// Ensure that release builds have no reference to the dev dependency plugin and make sure
93+
// that it is included with expected transitive dependencies for debug, profile builds.
94+
final bool appIncludesDevDependencyAsExpected =
95+
isTestingReleaseMode
96+
? !appDependencies.contains(regExpToMatchDevDependencyPlugin)
97+
: appDependencies.contains(
98+
regExpToMatchDevDependencyPluginWithTransitiveDependencies,
99+
);
100+
if (!appIncludesDevDependencyAsExpected) {
101+
throw TaskResult.failure(
102+
'Expected to${isTestingReleaseMode ? ' not' : ''} find dev_dependency_plugin as a dependency of the app built in $buildMode mode but did${isTestingReleaseMode ? '' : ' not'}.',
103+
);
104+
}
77105
}
78106
});
79107
return TaskResult.success(null);

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -590,11 +590,13 @@ class FlutterPlugin implements Plugin<Project> {
590590
}
591591
// The embedding is set as an API dependency in a Flutter plugin.
592592
// Therefore, don't make the app project depend on the embedding if there are Flutter
593-
// plugins.
593+
// plugin dependencies. In release mode, dev dependencies are stripped, so we do not
594+
// consider those in the check.
594595
// This prevents duplicated classes when using custom build types. That is, a custom build
595596
// type like profile is used, and the plugin and app projects have API dependencies on the
596597
// embedding.
597-
if (!isFlutterAppProject() || getPluginList(project).size() == 0) {
598+
List<Map<String, Object>> pluginsThatIncludeFlutterEmbeddingAsTransitiveDependency = flutterBuildMode == "release" ? getPluginListWithoutDevDependencies(project) : getPluginList(project);
599+
if (!isFlutterAppProject() || pluginsThatIncludeFlutterEmbeddingAsTransitiveDependency.size() == 0) {
598600
addApiDependencies(project, buildType.name,
599601
"io.flutter:flutter_embedding_$flutterBuildMode:$engineVersion")
600602
}
@@ -988,6 +990,24 @@ class FlutterPlugin implements Plugin<Project> {
988990
return pluginList
989991
}
990992

993+
/**
994+
* Gets the list of plugins (as map) that support the Android platform and are dependencies of the
995+
* Android project excluding dev dependencies.
996+
*
997+
* The map value contains either the plugins `name` (String),
998+
* its `path` (String), or its `dependencies` (List<String>).
999+
* See [NativePluginLoader#getPlugins] in packages/flutter_tools/gradle/src/main/groovy/native_plugin_loader.groovy
1000+
*/
1001+
private List<Map<String, Object>> getPluginListWithoutDevDependencies(Project project) {
1002+
List<Map<String, Object>> pluginListWithoutDevDependencies = []
1003+
for (Map<String, Object> plugin in getPluginList(project)) {
1004+
if (!plugin.dev_dependency) {
1005+
pluginListWithoutDevDependencies += plugin
1006+
}
1007+
}
1008+
return pluginListWithoutDevDependencies
1009+
}
1010+
9911011
// TODO(54566, 48918): Remove in favor of [getPluginList] only, see also
9921012
// https://github.com/flutter/flutter/blob/1c90ed8b64d9ed8ce2431afad8bc6e6d9acc4556/packages/flutter_tools/lib/src/flutter_plugins.dart#L212
9931013
/** Gets the plugins dependencies from `.flutter-plugins-dependencies`. */

0 commit comments

Comments
 (0)