From 965397cfde1165a2735f517d8071f583786ac6a1 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 7 Feb 2022 11:05:14 -0800 Subject: [PATCH] Fix how Gradle resolves Android plugin (#97823) --- .../gradle/app_plugin_loader.gradle | 1 + packages/flutter_tools/gradle/flutter.gradle | 74 +++++++---- .../android_skip_unsupported_plugin.dart | 120 ++++++++++++++++++ 3 files changed, 168 insertions(+), 27 deletions(-) create mode 100644 packages/flutter_tools/test/integration.shard/android_skip_unsupported_plugin.dart diff --git a/packages/flutter_tools/gradle/app_plugin_loader.gradle b/packages/flutter_tools/gradle/app_plugin_loader.gradle index ed92e8ef7c1d..c5abbf273b7a 100644 --- a/packages/flutter_tools/gradle/app_plugin_loader.gradle +++ b/packages/flutter_tools/gradle/app_plugin_loader.gradle @@ -20,6 +20,7 @@ assert object instanceof Map assert object.plugins instanceof Map assert object.plugins.android instanceof List // Includes the Flutter plugins that support the Android platform. +// This logic must be kept in sync with the logic in flutter.gradle. object.plugins.android.each { androidPlugin -> assert androidPlugin.name instanceof String assert androidPlugin.path instanceof String diff --git a/packages/flutter_tools/gradle/flutter.gradle b/packages/flutter_tools/gradle/flutter.gradle index 744e89c9cffd..3abbd5012fa6 100644 --- a/packages/flutter_tools/gradle/flutter.gradle +++ b/packages/flutter_tools/gradle/flutter.gradle @@ -48,7 +48,7 @@ class FlutterExtension { * Specifies the relative directory to the Flutter project directory. * In an app project, this is ../.. since the app's build.gradle is under android/app. */ - String source + String source = '../..' /** Allows to override the target file. Otherwise, the target is lib/main.dart. */ String target @@ -418,7 +418,7 @@ class FlutterPlugin implements Plugin { /** * Compares semantic versions ignoring labels. - * + * * If the versions are equal (ignoring labels), returns one of the two strings arbitrarily. * * If minor or patch are omitted (non-conformant to semantic versioning), they are considered zero. @@ -459,6 +459,9 @@ class FlutterPlugin implements Plugin { getPluginList().each { plugin -> Project pluginProject = project.rootProject.findProject(plugin.key) + if (pluginProject == null) { + return + } pluginProject.afterEvaluate { int pluginCompileSdkVersion = pluginProject.android.compileSdkVersion.substring(8) as int maxPluginCompileSdkVersion = Math.max(pluginCompileSdkVersion, maxPluginCompileSdkVersion) @@ -476,15 +479,7 @@ class FlutterPlugin implements Plugin { } } } - } - } - - /** - * Returns `true` if the given path contains an `android/build.gradle` file. - */ - private Boolean doesSupportAndroidPlatform(String path) { - File editableAndroidProject = new File(path, 'android' + File.separator + 'build.gradle') - return editableAndroidProject.exists() + } } /** @@ -495,8 +490,7 @@ class FlutterPlugin implements Plugin { private void configurePluginDependencies(Object dependencyObject) { assert dependencyObject.name instanceof String Project pluginProject = project.rootProject.findProject(":${dependencyObject.name}") - if (pluginProject == null || - !doesSupportAndroidPlatform(pluginProject.projectDir.parentFile.path)) { + if (pluginProject == null) { return } assert dependencyObject.dependencies instanceof List @@ -506,8 +500,7 @@ class FlutterPlugin implements Plugin { return } Project dependencyProject = project.rootProject.findProject(":$pluginDependencyName") - if (dependencyProject == null || - !doesSupportAndroidPlatform(dependencyProject.projectDir.parentFile.path)) { + if (dependencyProject == null) { return } // Wait for the Android plugin to load and add the dependency to the plugin project. @@ -519,17 +512,27 @@ class FlutterPlugin implements Plugin { } } + /** Gets the list of plugins that support the Android platform. */ private Properties getPluginList() { - File pluginsFile = new File(project.projectDir.parentFile.parentFile, '.flutter-plugins') - Properties allPlugins = readPropertiesIfExist(pluginsFile) + Map meta = getDependenciesMetadata() Properties androidPlugins = new Properties() - allPlugins.each { name, path -> - if (doesSupportAndroidPlatform(path)) { - androidPlugins.setProperty(name, path) + if (meta == null) { + return androidPlugins + } + assert meta.plugins instanceof Map + assert meta.plugins.android instanceof List + + // This logic must be kept in sync with the logic in app_plugin_loader.gradle. + meta.plugins.android.each { androidPlugin -> + assert androidPlugin.name instanceof String + assert androidPlugin.path instanceof String + // Skip plugins that have no native build (such as a Dart-only implementation + // of a federated plugin). + def needsBuild = androidPlugin.containsKey('native_build') ? androidPlugin['native_build'] : true + if (!needsBuild) { + return } - // TODO(amirh): log an error if this plugin was specified to be an Android - // plugin according to the new schema, and was missing a build.gradle file. - // https://github.com/flutter/flutter/issues/40784 + androidPlugins.setProperty(androidPlugin.name, androidPlugin.path) } return androidPlugins } @@ -557,14 +560,31 @@ class FlutterPlugin implements Plugin { // This means, `plugin-a` depends on `plugin-b` and `plugin-c`. // `plugin-b` depends on `plugin-c`. // `plugin-c` doesn't depend on anything. - File pluginsDependencyFile = new File(project.projectDir.parentFile.parentFile, '.flutter-plugins-dependencies') + Map meta = getDependenciesMetadata() + if (meta == null) { + return [] + } + assert meta.dependencyGraph instanceof List + return meta.dependencyGraph + } + + private Map parsedFlutterPluginsDependencies + + /** + * Parses /.flutter-plugins-dependencies + */ + private Map getDependenciesMetadata() { + if (parsedFlutterPluginsDependencies) { + return parsedFlutterPluginsDependencies + } + File pluginsDependencyFile = new File(getFlutterSourceDirectory(), '.flutter-plugins-dependencies') if (pluginsDependencyFile.exists()) { def object = new JsonSlurper().parseText(pluginsDependencyFile.text) assert object instanceof Map - assert object.dependencyGraph instanceof List - return object.dependencyGraph + parsedFlutterPluginsDependencies = object + return object } - return [] + return null } private static String toCammelCase(List parts) { diff --git a/packages/flutter_tools/test/integration.shard/android_skip_unsupported_plugin.dart b/packages/flutter_tools/test/integration.shard/android_skip_unsupported_plugin.dart new file mode 100644 index 000000000000..c8648e478b2c --- /dev/null +++ b/packages/flutter_tools/test/integration.shard/android_skip_unsupported_plugin.dart @@ -0,0 +1,120 @@ +// 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_testing/file_testing.dart'; +import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:flutter_tools/src/base/io.dart'; +import 'package:flutter_tools/src/cache.dart'; + +import '../src/common.dart'; +import 'test_utils.dart'; + +void main() { + late Directory tempDir; + + setUp(() { + Cache.flutterRoot = getFlutterRoot(); + tempDir = createResolvedTempDirectorySync('flutter_plugin_test.'); + }); + + tearDown(() async { + tryToDelete(tempDir); + }); + + // Regression test for https://github.com/flutter/flutter/issues/97729. + test('skip plugin if it does not support the Android platform', () async { + final String flutterBin = fileSystem.path.join( + getFlutterRoot(), + 'bin', + 'flutter', + ); + + // Create dummy plugin that *only* supports iOS. + processManager.runSync([ + flutterBin, + ...getLocalEngineArguments(), + 'create', + '--template=plugin', + '--platforms=ios', + 'test_plugin', + ], workingDirectory: tempDir.path); + + final Directory pluginAppDir = tempDir.childDirectory('test_plugin'); + + // Create an android directory and a build.gradle file within. + final File pluginGradleFile = pluginAppDir + .childDirectory('android') + .childFile('build.gradle') + ..createSync(recursive: true); + expect(pluginGradleFile, exists); + + pluginGradleFile.writeAsStringSync(r''' +buildscript { + ext.kotlin_version = '1.5.31' + repositories { + google() + mavenCentral() + } + + dependencies { + classpath 'com.android.tools.build:gradle:7.0.0' + classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" + } + + configurations.classpath { + resolutionStrategy.activateDependencyLocking() + } +} + +allprojects { + repositories { + google() + mavenCentral() + } +} + +rootProject.buildDir = '../build' + +subprojects { + project.buildDir = "${rootProject.buildDir}/${project.name}" +} + +subprojects { + project.evaluationDependsOn(':app') +} + +task clean(type: Delete) { + delete rootProject.buildDir +} +'''); + + final Directory pluginExampleAppDir = + pluginAppDir.childDirectory('example'); + + // Add android support to the plugin's example app. + final ProcessResult addAndroidResult = processManager.runSync([ + flutterBin, + ...getLocalEngineArguments(), + 'create', + '--template=app', + '--platforms=android', + '.', + ], workingDirectory: pluginExampleAppDir.path); + expect(addAndroidResult.exitCode, equals(0), + reason: + 'flutter create exited with non 0 code: ${addAndroidResult.stderr}'); + + // Run flutter build apk to build plugin example project. + final ProcessResult buildApkResult = processManager.runSync([ + flutterBin, + ...getLocalEngineArguments(), + 'build', + 'apk', + '--debug', + ], workingDirectory: pluginExampleAppDir.path); + expect(buildApkResult.exitCode, equals(0), + reason: + 'flutter build apk exited with non 0 code: ${buildApkResult.stderr}'); + }); +}