From 4feddffcfa77f6954c0e3a03bdd812193441d1de Mon Sep 17 00:00:00 2001 From: jesswrd Date: Wed, 30 Oct 2024 12:38:55 -0700 Subject: [PATCH] Applied Gradle Plugins Declaratively for `path_provider` (#7822) Updated applying gradle plugins from usage of imperative apply to usage of declarative blocks {} apply. Intending on updating all android example apps under packages. Did one more as a proof of concept before doing more. More information on Flutter Gradle Plugin Apply [here](https://docs.flutter.dev/release/breaking-changes/flutter-gradle-plugin-apply) Partially addresses [#152656](https://github.com/flutter/flutter/issues/152656) ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] page, which explains my responsibilities. - [x] I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use `dart format`.) - [x] I signed the [CLA]. - [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. `[shared_preferences]` - [x] I [linked to at least one issue that this PR fixes] in the description above. - [x] I updated `pubspec.yaml` with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes]. - [x] I updated `CHANGELOG.md` to add a description of the change, [following repository CHANGELOG style], or this PR is [exempt from CHANGELOG changes]. - [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] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md [Tree Hygiene]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md [relevant style guides]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style [CLA]: https://cla.developers.google.com/ [Discord]: https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md [linked to at least one issue that this PR fixes]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview [pub versioning philosophy]: https://dart.dev/tools/pub/versioning [exempt from version changes]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version [following repository CHANGELOG style]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog-style [exempt from CHANGELOG changes]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog [test-exempt]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests --------- Co-authored-by: Reid Baker --- .../example/android/app/build.gradle | 16 +- .../gradle/wrapper/gradle-wrapper.properties | 2 +- .../android/app/src/main/AndroidManifest.xml | 1 + .../example/android/build.gradle | 11 -- .../example/android/settings.gradle | 43 +++-- script/tool/lib/src/gradle_check_command.dart | 56 ++++-- .../tool/test/gradle_check_command_test.dart | 159 +++++++++++++++++- 7 files changed, 226 insertions(+), 62 deletions(-) diff --git a/packages/path_provider/path_provider/example/android/app/build.gradle b/packages/path_provider/path_provider/example/android/app/build.gradle index 3e67e5d7f4ca..cd37f70f199b 100644 --- a/packages/path_provider/path_provider/example/android/app/build.gradle +++ b/packages/path_provider/path_provider/example/android/app/build.gradle @@ -1,3 +1,9 @@ +plugins { + id "com.android.application" + id "org.jetbrains.kotlin.android" + id "dev.flutter.flutter-gradle-plugin" +} + def localProperties = new Properties() def localPropertiesFile = rootProject.file('local.properties') if (localPropertiesFile.exists()) { @@ -6,11 +12,6 @@ if (localPropertiesFile.exists()) { } } -def flutterRoot = localProperties.getProperty('flutter.sdk') -if (flutterRoot == null) { - throw new GradleException("Flutter SDK not found. Define location with flutter.sdk in the local.properties file.") -} - def flutterVersionCode = localProperties.getProperty('flutter.versionCode') if (flutterVersionCode == null) { flutterVersionCode = '1' @@ -21,9 +22,6 @@ if (flutterVersionName == null) { flutterVersionName = '1.0' } -apply plugin: 'com.android.application' -apply from: "$flutterRoot/packages/flutter_tools/gradle/flutter.gradle" - android { namespace 'io.flutter.plugins.pathproviderexample' compileSdk flutter.compileSdkVersion @@ -32,7 +30,7 @@ android { defaultConfig { applicationId "io.flutter.plugins.pathproviderexample" minSdkVersion flutter.minSdkVersion - targetSdkVersion 28 + targetSdkVersion 35 versionCode flutterVersionCode.toInteger() versionName flutterVersionName testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" diff --git a/packages/path_provider/path_provider/example/android/app/gradle/wrapper/gradle-wrapper.properties b/packages/path_provider/path_provider/example/android/app/gradle/wrapper/gradle-wrapper.properties index d951fac2bf31..28f5fcf95755 100644 --- a/packages/path_provider/path_provider/example/android/app/gradle/wrapper/gradle-wrapper.properties +++ b/packages/path_provider/path_provider/example/android/app/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.7-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.8-all.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/packages/path_provider/path_provider/example/android/app/src/main/AndroidManifest.xml b/packages/path_provider/path_provider/example/android/app/src/main/AndroidManifest.xml index df8cee7bc3be..d72897496391 100644 --- a/packages/path_provider/path_provider/example/android/app/src/main/AndroidManifest.xml +++ b/packages/path_provider/path_provider/example/android/app/src/main/AndroidManifest.xml @@ -6,6 +6,7 @@ plugins.load(stream) } -} - -plugins.each { name, path -> - def pluginDirectory = flutterProjectRoot.resolve(path).resolve('android').toFile() - include ":$name" - project(":$name").projectDir = pluginDirectory + repositories { + google() + mavenCentral() + gradlePluginPortal() + } } // See https://github.com/flutter/flutter/blob/master/docs/ecosystem/Plugins-and-Packages-repository-structure.md#gradle-structure for more info. -buildscript { - repositories { - maven { - url "https://plugins.gradle.org/m2/" - } - } - dependencies { - classpath "gradle.plugin.com.google.cloud.artifactregistry:artifactregistry-gradle-plugin:2.2.1" - } +plugins { + id "dev.flutter.flutter-plugin-loader" version "1.0.0" + id "com.android.application" version "8.6.0" apply false + id "org.jetbrains.kotlin.android" version "1.7.10" apply false + id "com.google.cloud.artifactregistry.gradle-plugin" version "2.2.1" } -apply plugin: "com.google.cloud.artifactregistry.gradle-plugin" + +include ":app" diff --git a/script/tool/lib/src/gradle_check_command.dart b/script/tool/lib/src/gradle_check_command.dart index a4657560d3f4..ab6545054c31 100644 --- a/script/tool/lib/src/gradle_check_command.dart +++ b/script/tool/lib/src/gradle_check_command.dart @@ -202,7 +202,6 @@ class GradleCheckCommand extends PackageLoopingCommand { /// configuration that enables artifact hub env variable. @visibleForTesting static String exampleRootSettingsArtifactHubString = ''' -// See $artifactHubDocumentationString for more info. buildscript { repositories { maven { @@ -216,6 +215,18 @@ buildscript { apply plugin: "com.google.cloud.artifactregistry.gradle-plugin" '''; + /// String printed as a valid example of settings.gradle repository + /// configuration that enables artifact hub env variable. + /// GP stands for the gradle plugin method of flutter tooling inclusion. + @visibleForTesting + static String exampleSettingsArtifactHubStringGP = ''' +plugins { + id "dev.flutter.flutter-plugin-loader" version "1.0.0" + // ...other plugins + id "com.google.cloud.artifactregistry.gradle-plugin" version "2.2.1" +} + '''; + /// Validates that [gradleLines] reads and uses a artifiact hub repository /// when ARTIFACT_HUB_REPOSITORY is set. /// @@ -228,6 +239,10 @@ apply plugin: "com.google.cloud.artifactregistry.gradle-plugin" r'classpath.*gradle\.plugin\.com\.google\.cloud\.artifactregistry:artifactregistry-gradle-plugin'); final RegExp artifactRegistryPluginApplyRegex = RegExp( r'apply.*plugin.*com\.google\.cloud\.artifactregistry\.gradle-plugin'); + final RegExp artifactRegistryPluginApplyRegexGP = RegExp( + r'id.*com\.google\.cloud\.artifactregistry\.gradle-plugin.*version.*\b\d+\.\d+\.\d+\b'); + final RegExp artifactRegistryPluginApplyDeclarativeRegex = + RegExp(r'\bpluginManagement\b'); final bool documentationPresent = gradleLines .any((String line) => documentationPresentRegex.hasMatch(line)); @@ -235,17 +250,36 @@ apply plugin: "com.google.cloud.artifactregistry.gradle-plugin" .any((String line) => artifactRegistryDefinitionRegex.hasMatch(line)); final bool artifactRegistryPluginApplied = gradleLines .any((String line) => artifactRegistryPluginApplyRegex.hasMatch(line)); - - if (!(documentationPresent && - artifactRegistryDefined && - artifactRegistryPluginApplied)) { - printError('Failed Artifact Hub validation. Include the following in ' - 'example root settings.gradle:\n$exampleRootSettingsArtifactHubString'); + final bool declarativeArtifactRegistryApplied = gradleLines.any( + (String line) => artifactRegistryPluginApplyRegexGP.hasMatch(line)); + final bool declarativePluginBlockApplied = gradleLines.any((String line) => + artifactRegistryPluginApplyDeclarativeRegex.hasMatch(line)); + + final bool imperativeArtifactRegistryApplied = + artifactRegistryDefined && artifactRegistryPluginApplied; + + final bool validArtifactConfiguration = documentationPresent && + (imperativeArtifactRegistryApplied || + declarativeArtifactRegistryApplied); + + if (!validArtifactConfiguration) { + printError('Failed Artifact Hub validation.'); + if (!documentationPresent) { + printError( + 'The link to the Artifact Hub documentation is missing. Include the following in ' + 'example root settings.gradle:\n// See $artifactHubDocumentationString for more info.'); + } + if (artifactRegistryDefined || + artifactRegistryPluginApplied || + !declarativePluginBlockApplied) { + printError('Include the following in ' + 'example root settings.gradle:\n$exampleRootSettingsArtifactHubString'); + } else if (!declarativeArtifactRegistryApplied) { + printError('Include the following in ' + 'example root settings.gradle:\n$exampleSettingsArtifactHubStringGP'); + } } - - return documentationPresent && - artifactRegistryDefined && - artifactRegistryPluginApplied; + return validArtifactConfiguration; } /// Validates the top-level build.gradle for an example app (e.g., diff --git a/script/tool/test/gradle_check_command_test.dart b/script/tool/test/gradle_check_command_test.dart index 11376e063dc1..457f3f40b02b 100644 --- a/script/tool/test/gradle_check_command_test.dart +++ b/script/tool/test/gradle_check_command_test.dart @@ -174,6 +174,7 @@ ${warningsConfigured ? warningConfig : ''} void writeFakeExampleTopLevelSettingsGradle( RepositoryPackage package, { bool includeArtifactHub = true, + bool includeArtifactDocumentation = true, }) { final File settingsGradle = package .platformDirectory(FlutterPlatform.android) @@ -196,10 +197,58 @@ plugins.each { name, path -> include ":\$name" project(":\$name").projectDir = pluginDirectory } +${includeArtifactDocumentation ? '// See ${GradleCheckCommand.artifactHubDocumentationString} for more info.' : ''} ${includeArtifactHub ? GradleCheckCommand.exampleRootSettingsArtifactHubString : ''} '''); } + /// Writes a fake android/build.gradle file for an example [package] with the + /// given options. + void writeFakeExampleSettingsGradle( + RepositoryPackage package, { + bool includeArtifactHub = true, + bool includeArtifactDocumentation = true, + }) { + final File settingsGradle = package + .platformDirectory(FlutterPlatform.android) + .childFile('settings.gradle'); + settingsGradle.createSync(recursive: true); + + /// String printed as a valid example of settings.gradle repository + /// configuration without the artifact hub env variable. + /// GP stands for the gradle plugin method of flutter tooling inclusion. + const String exampleSettingsWithoutArtifactHubStringGP = ''' +plugins { + id "dev.flutter.flutter-plugin-loader" version "1.0.0" + // ...other plugins +} + '''; + + settingsGradle.writeAsStringSync(''' +pluginManagement { + def flutterSdkPath = { + def properties = new Properties() + file("local.properties").withInputStream { properties.load(it) } + def flutterSdkPath = properties.getProperty("flutter.sdk") + assert flutterSdkPath != null, "flutter.sdk not set in local.properties" + return flutterSdkPath + }() + + includeBuild("\$flutterSdkPath/packages/flutter_tools/gradle") + + repositories { + google() + mavenCentral() + gradlePluginPortal() + } +} + +${includeArtifactDocumentation ? '// See ${GradleCheckCommand.artifactHubDocumentationString} for more info.' : ''} +${includeArtifactHub ? GradleCheckCommand.exampleSettingsArtifactHubStringGP : exampleSettingsWithoutArtifactHubStringGP} +include ":app" +'''); + } + /// Writes a fake android/app/build.gradle file for an example [package] with /// the given options. void writeFakeExampleAppBuildGradle( @@ -258,6 +307,7 @@ dependencies { String? kotlinVersion, bool includeBuildArtifactHub = true, bool includeSettingsArtifactHub = true, + bool includeSettingsDocumentationArtifactHub = true, }) { writeFakeExampleTopLevelBuildGradle( package, @@ -271,6 +321,34 @@ dependencies { writeFakeExampleTopLevelSettingsGradle( package, includeArtifactHub: includeSettingsArtifactHub, + includeArtifactDocumentation: includeSettingsDocumentationArtifactHub, + ); + } + + void writeFakeExampleBuildGradleGP( + RepositoryPackage package, { + required String pluginName, + bool includeNamespace = true, + bool commentNamespace = false, + bool warningsConfigured = true, + String? kotlinVersion, + required bool includeBuildArtifactHub, + required bool includeSettingsArtifactHub, + required bool includeSettingsDocumentationArtifactHub, + }) { + writeFakeExampleTopLevelBuildGradle( + package, + pluginName: pluginName, + warningsConfigured: warningsConfigured, + kotlinVersion: kotlinVersion, + includeArtifactHub: includeBuildArtifactHub, + ); + writeFakeExampleAppBuildGradle(package, + includeNamespace: includeNamespace, commentNamespace: commentNamespace); + writeFakeExampleSettingsGradle( + package, + includeArtifactHub: includeSettingsArtifactHub, + includeArtifactDocumentation: includeSettingsDocumentationArtifactHub, ); } @@ -670,14 +748,14 @@ dependencies { writeFakePluginBuildGradle(package, includeLanguageVersion: true); writeFakeManifest(package); final RepositoryPackage example = package.getExamples().first; - writeFakeExampleBuildGradles( - example, - pluginName: packageName, - // ignore: avoid_redundant_argument_values - includeBuildArtifactHub: true, - // ignore: avoid_redundant_argument_values - includeSettingsArtifactHub: true, - ); + writeFakeExampleBuildGradles(example, + pluginName: packageName, + // ignore: avoid_redundant_argument_values + includeBuildArtifactHub: true, + // ignore: avoid_redundant_argument_values + includeSettingsArtifactHub: true, + // ignore: avoid_redundant_argument_values + includeSettingsDocumentationArtifactHub: true); writeFakeManifest(example, isApp: true); final List output = @@ -789,6 +867,71 @@ dependencies { isNot(contains(GradleCheckCommand.exampleRootGradleArtifactHubString)), ); }); + + test('prints error for declarative method of applying gradle plugins', + () async { + const String packageName = 'a_package'; + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + writeFakePluginBuildGradle(package, includeLanguageVersion: true); + writeFakeManifest(package); + final RepositoryPackage example = package.getExamples().first; + writeFakeExampleBuildGradleGP(example, + pluginName: packageName, + includeBuildArtifactHub: true, + includeSettingsArtifactHub: false, + includeSettingsDocumentationArtifactHub: true); + writeFakeManifest(example, isApp: true); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['gradle-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains(GradleCheckCommand.exampleSettingsArtifactHubStringGP), + ]), + ); + expect( + output, + isNot( + contains(GradleCheckCommand.exampleRootSettingsArtifactHubString)), + ); + }); + + test('error message is printed when documentation link is missing', + () async { + const String packageName = 'a_package'; + final RepositoryPackage package = + createFakePackage('a_package', packagesDir); + writeFakePluginBuildGradle(package, includeLanguageVersion: true); + writeFakeManifest(package); + final RepositoryPackage example = package.getExamples().first; + writeFakeExampleBuildGradleGP(example, + pluginName: packageName, + includeBuildArtifactHub: true, + includeSettingsArtifactHub: true, + includeSettingsDocumentationArtifactHub: false); + writeFakeManifest(example, isApp: true); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['gradle-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains(GradleCheckCommand.artifactHubDocumentationString), + ]), + ); + }); }); group('Kotlin version check', () {