Skip to content

Commit

Permalink
[various] Add targetCompatibility to build.gradle (#3825)
Browse files Browse the repository at this point in the history
While current docs about `targetCompatibility` say that it defaults to `sourceCompatibility`, for older toolchains (AGP?) that is apparently not the case, and it's a build error to set `sourceCompatibility` without `targetCompatibility`.

This adds enforcement that it's set to `gradle-check`, and fixes all of the violations.

Fixes flutter/flutter#125482
  • Loading branch information
stuartmorgan authored Apr 26, 2023
1 parent dbab052 commit 5ae8a1d
Show file tree
Hide file tree
Showing 17 changed files with 85 additions and 20 deletions.
4 changes: 4 additions & 0 deletions packages/espresso/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.3.0+3

* Adds `targetCompatibilty` matching `sourceCompatibility` for older toolchains.

## 0.3.0+2

* Adds a namespace for compatibility with AGP 8.0.
Expand Down
1 change: 1 addition & 0 deletions packages/espresso/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ android {

compileOptions {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}

lintOptions {
Expand Down
2 changes: 1 addition & 1 deletion packages/espresso/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Java classes for testing Flutter apps using Espresso.
Allows driving Flutter widgets from a native Espresso test.
repository: https://github.com/flutter/packages/tree/main/packages/espresso
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+espresso%22
version: 0.3.0+2
version: 0.3.0+3

environment:
sdk: ">=2.17.0 <4.0.0"
Expand Down
4 changes: 4 additions & 0 deletions packages/flutter_plugin_android_lifecycle/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.0.12

* Adds `targetCompatibilty` matching `sourceCompatibility` for older toolchains.

## 2.0.11

* Adds a namespace for compatibility with AGP 8.0.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ android {

compileOptions {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}

lintOptions {
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_plugin_android_lifecycle/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: flutter_plugin_android_lifecycle
description: Flutter plugin for accessing an Android Lifecycle within other plugins.
repository: https://github.com/flutter/packages/tree/main/packages/flutter_plugin_android_lifecycle
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+flutter_plugin_android_lifecycle%22
version: 2.0.11
version: 2.0.12

environment:
sdk: ">=2.17.0 <4.0.0"
Expand Down
4 changes: 4 additions & 0 deletions packages/google_sign_in/google_sign_in_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 6.1.13

* Adds `targetCompatibilty` matching `sourceCompatibility` for older toolchains.

## 6.1.12

* Adds a namespace for compatibility with AGP 8.0.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ android {

compileOptions {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}

lintOptions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: google_sign_in_android
description: Android implementation of the google_sign_in plugin.
repository: https://github.com/flutter/packages/tree/main/packages/google_sign_in/google_sign_in_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_sign_in%22
version: 6.1.12
version: 6.1.13

environment:
sdk: ">=2.17.0 <4.0.0"
Expand Down
4 changes: 4 additions & 0 deletions packages/local_auth/local_auth_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.0.26

* Adds `targetCompatibilty` matching `sourceCompatibility` for older toolchains.

## 1.0.25

* Adds a namespace for compatibility with AGP 8.0.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ android {

compileOptions {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}

lintOptions {
Expand Down
2 changes: 1 addition & 1 deletion packages/local_auth/local_auth_android/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: local_auth_android
description: Android implementation of the local_auth plugin.
repository: https://github.com/flutter/packages/tree/main/packages/local_auth/local_auth_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+local_auth%22
version: 1.0.25
version: 1.0.26

environment:
sdk: ">=2.17.0 <4.0.0"
Expand Down
4 changes: 4 additions & 0 deletions packages/url_launcher/url_launcher_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 6.0.30

* Adds `targetCompatibilty` matching `sourceCompatibility` for older toolchains.

## 6.0.29

* Adds a namespace for compatibility with AGP 8.0.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ android {

compileOptions {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}

lintOptions {
Expand Down
2 changes: 1 addition & 1 deletion packages/url_launcher/url_launcher_android/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: url_launcher_android
description: Android implementation of the url_launcher plugin.
repository: https://github.com/flutter/packages/tree/main/packages/url_launcher/url_launcher_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+url_launcher%22
version: 6.0.29
version: 6.0.30

environment:
sdk: ">=2.18.0 <4.0.0"
Expand Down
23 changes: 16 additions & 7 deletions script/tool/lib/src/gradle_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class GradleCheckCommand extends PackageLoopingCommand {
if (!_validateNamespace(package, contents, isExample: false)) {
succeeded = false;
}
if (!_validateSourceCompatibilityVersion(lines)) {
if (!_validateCompatibilityVersions(lines)) {
succeeded = false;
}
if (!_validateGradleDrivenLintConfig(package, lines)) {
Expand Down Expand Up @@ -204,18 +204,27 @@ build.gradle "namespace" must match the "package" attribute in AndroidManifest.x
/// Checks for a source compatibiltiy version, so that it's explicit rather
/// than using whatever the client's local toolchaing defaults to (which can
/// lead to compile errors that show up for clients, but not in CI).
bool _validateSourceCompatibilityVersion(List<String> gradleLines) {
if (!gradleLines.any((String line) =>
line.contains('languageVersion') && !_isCommented(line)) &&
!gradleLines.any((String line) =>
line.contains('sourceCompatibility') && !_isCommented(line))) {
bool _validateCompatibilityVersions(List<String> gradleLines) {
final bool hasLanguageVersion = gradleLines.any((String line) =>
line.contains('languageVersion') && !_isCommented(line));
final bool hasCompabilityVersions = gradleLines.any((String line) =>
line.contains('sourceCompatibility') && !_isCommented(line)) &&
// Newer toolchains default targetCompatibility to the same value as
// sourceCompatibility, but older toolchains require it to be set
// explicitly. The exact version cutoff (and of which piece of the
// toolchain; likely AGP) is unknown; for context see
// https://github.com/flutter/flutter/issues/125482
gradleLines.any((String line) =>
line.contains('targetCompatibility') && !_isCommented(line));
if (!hasLanguageVersion && !hasCompabilityVersions) {
const String errorMessage = '''
build.gradle must set an explicit Java compatibility version.
This can be done either via "sourceCompatibility":
This can be done either via "sourceCompatibility"/"targetCompatibility":
android {
compileOptions {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}
}
Expand Down
47 changes: 39 additions & 8 deletions script/tool/test/gradle_check_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ void main() {
RepositoryPackage package, {
bool includeLanguageVersion = false,
bool includeSourceCompat = false,
bool includeTargetCompat = false,
bool commentSourceLanguage = false,
bool includeNamespace = true,
bool commentNamespace = false,
Expand All @@ -64,11 +65,10 @@ java {
}
''';
final String compileOptionsSection = '''
compileOptions {
${commentSourceLanguage ? '// ' : ''}sourceCompatibility JavaVersion.VERSION_1_8
}
''';
final String sourceCompat =
'${commentSourceLanguage ? '// ' : ''}sourceCompatibility JavaVersion.VERSION_1_8';
final String targetCompat =
'${commentSourceLanguage ? '// ' : ''}targetCompatibility JavaVersion.VERSION_1_8';
final String namespace =
"${commentNamespace ? '// ' : ''}namespace '$_defaultFakeNamespace'";

Expand All @@ -94,7 +94,10 @@ android {
minSdkVersion 30
}
${warningsConfigured ? warningConfig : ''}
${includeSourceCompat ? compileOptionsSection : ''}
compileOptions {
${includeSourceCompat ? sourceCompat : ''}
${includeTargetCompat ? targetCompat : ''}
}
testOptions {
unitTests.includeAndroidResources = true
}
Expand Down Expand Up @@ -280,12 +283,38 @@ dependencies {
);
});

test('passes when sourceCompatibility is specified', () async {
test(
'fails when sourceCompatibility is provided with out targetCompatibility',
() async {
final RepositoryPackage package =
createFakePlugin('a_plugin', packagesDir, examples: <String>[]);
writeFakePluginBuildGradle(package, includeSourceCompat: true);
writeFakeManifest(package);

Error? commandError;
final List<String> output = await runCapturingPrint(
runner, <String>['gradle-check'], errorHandler: (Error e) {
commandError = e;
});

expect(commandError, isA<ToolExit>());
expect(
output,
containsAllInOrder(<Matcher>[
contains(
'build.gradle must set an explicit Java compatibility version.'),
]),
);
});

test('passes when sourceCompatibility and targetCompatibility are specified',
() async {
final RepositoryPackage package =
createFakePlugin('a_plugin', packagesDir, examples: <String>[]);
writeFakePluginBuildGradle(package,
includeSourceCompat: true, includeTargetCompat: true);
writeFakeManifest(package);

final List<String> output =
await runCapturingPrint(runner, <String>['gradle-check']);

Expand Down Expand Up @@ -339,7 +368,9 @@ dependencies {
final RepositoryPackage package =
createFakePlugin('a_plugin', packagesDir, examples: <String>[]);
writeFakePluginBuildGradle(package,
includeSourceCompat: true, commentSourceLanguage: true);
includeSourceCompat: true,
includeTargetCompat: true,
commentSourceLanguage: true);
writeFakeManifest(package);

Error? commandError;
Expand Down

0 comments on commit 5ae8a1d

Please sign in to comment.