Skip to content

Commit 1816a2c

Browse files
gmackallGray Mackall
authored andcommitted
[reland] Fix regression in NDK version checking (flutter#167011)
Relands flutter#166998. On top of the original PR: 1. fixes unfortunate mistake in commenting the fix 2. adds to the `runIf` cases to better cover when this test should be run in presubmit 3. pins a lower version of `shared_preferences_android`, as we need to use a plugin with `compileSdk 34` for those lower AGP versions. ## 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]. - [ ] 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. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- 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: Gray Mackall <[email protected]>
1 parent 673806f commit 1816a2c

File tree

4 files changed

+44
-6
lines changed

4 files changed

+44
-6
lines changed

.ci.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,10 +1520,12 @@ targets:
15201520
test_timeout_secs: "2700"
15211521
runIf:
15221522
- packages/flutter_tools/templates/**
1523+
- packages/flutter_tools/gradle/**
15231524
- .ci.yaml
15241525
- engine/**
15251526
- DEPS
15261527
- dev/devicelab/bin/tasks/android_java11_dependency_smoke_tests.dart
1528+
- dev/devicelab/lib/framework/dependency_smoke_test_task_definition.dart
15271529

15281530
- name: Linux android_java17_dependency_smoke_tests
15291531
recipe: devicelab/devicelab_drone
@@ -1542,10 +1544,12 @@ targets:
15421544
test_timeout_secs: "2700"
15431545
runIf:
15441546
- packages/flutter_tools/templates/**
1547+
- packages/flutter_tools/gradle/**
15451548
- .ci.yaml
15461549
- engine/**
15471550
- DEPS
15481551
- dev/devicelab/bin/tasks/android_java17_dependency_smoke_tests.dart
1552+
- dev/devicelab/lib/framework/dependency_smoke_test_task_definition.dart
15491553

15501554
- name: Linux tool_tests_commands
15511555
recipe: flutter/flutter_drone

dev/devicelab/bin/tasks/android_java11_dependency_smoke_tests.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,14 @@ List<VersionTuple> versionTuples = <VersionTuple>[
4040
kotlinVersion: '1.7.10',
4141
compileSdkVersion: '34',
4242
),
43+
// minSdk bump required due to a bug in the default version of r8 used by AGP
44+
// 7.4.0. See http://issuetracker.google.com/issues/357553178.
4345
VersionTuple(
4446
agpVersion: '7.4.0',
4547
gradleVersion: '7.5',
4648
kotlinVersion: '1.8.10',
4749
compileSdkVersion: '34',
50+
minSdkVersion: '24',
4851
),
4952
];
5053

dev/devicelab/lib/framework/dependency_smoke_test_task_definition.dart

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ distributionUrl=https\://services.gradle.org/distributions/gradle-GRADLE_REPLACE
6161

6262
const String gradleReplacementString = 'GRADLE_REPLACE_ME';
6363
final RegExp flutterCompileSdkString = RegExp(r'flutter\.compileSdkVersion|flutter\.compileSdk');
64+
final RegExp flutterMinSdkString = RegExp(r'flutter\.minSdkVersion|flutter\.minSdk');
6465

6566
/// A simple class containing a Kotlin, Gradle, and AGP version.
6667
class VersionTuple {
@@ -69,12 +70,14 @@ class VersionTuple {
6970
required this.gradleVersion,
7071
required this.kotlinVersion,
7172
this.compileSdkVersion,
73+
this.minSdkVersion,
7274
});
7375

7476
String agpVersion;
7577
String gradleVersion;
7678
String kotlinVersion;
7779
String? compileSdkVersion;
80+
String? minSdkVersion;
7881

7982
@override
8083
String toString() {
@@ -108,17 +111,25 @@ Future<TaskResult> buildFlutterApkWithSpecifiedDependencyVersions({
108111

109112
final String appPath = '${innerTempDir.absolute.path}/dependency_checker_app';
110113

114+
final File appGradleBuild = getAndroidBuildFile(
115+
localFileSystem.path.join(appPath, 'android', 'app'),
116+
);
111117
if (versions.compileSdkVersion != null) {
112-
final File appGradleBuild = getAndroidBuildFile(
113-
localFileSystem.path.join(appPath, 'android', 'app'),
114-
);
115118
final String appBuildContent = appGradleBuild.readAsStringSync().replaceFirst(
116119
flutterCompileSdkString,
117120
versions.compileSdkVersion!,
118121
);
119122
appGradleBuild.writeAsStringSync(appBuildContent);
120123
}
121124

125+
if (versions.minSdkVersion != null) {
126+
final String appBuildContent = appGradleBuild.readAsStringSync().replaceFirst(
127+
flutterMinSdkString,
128+
versions.minSdkVersion!,
129+
);
130+
appGradleBuild.writeAsStringSync(appBuildContent);
131+
}
132+
122133
// Modify gradle version to passed in version.
123134
final File gradleWrapperProperties = localFileSystem.file(
124135
localFileSystem.path.join(
@@ -144,6 +155,13 @@ Future<TaskResult> buildFlutterApkWithSpecifiedDependencyVersions({
144155
.replaceFirst(kgpReplacementString, versions.kotlinVersion);
145156
await gradleSettingsFile.writeAsString(settingsContent, flush: true);
146157

158+
section('Add a dependency on a plugin');
159+
await flutter(
160+
'pub',
161+
options: <String>['add', 'shared_preferences_android:2.4.7'], // Chosen randomly.
162+
workingDirectory: appPath,
163+
);
164+
147165
// Ensure that gradle files exists from templates.
148166
section(
149167
"Ensure 'flutter build apk' succeeds with Gradle ${versions.gradleVersion}, AGP ${versions.agpVersion}, and Kotlin ${versions.kotlinVersion}",

packages/flutter_tools/gradle/src/main/kotlin/FlutterPluginUtils.kt

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -517,8 +517,16 @@ object FlutterPluginUtils {
517517
getCompileSdkFromProject(project).toIntOrNull() ?: Int.MAX_VALUE
518518

519519
var maxPluginCompileSdkVersion = projectCompileSdkVersion
520-
val projectNdkVersion =
521-
getAndroidExtension(project).ndkVersion
520+
// TODO(gmackall): This should be updated to reflect newer templates.
521+
// The default for AGP 4.1.0 used in old templates.
522+
val ndkVersionIfUnspecified = "21.1.6352462"
523+
524+
// TODO(gmackall): We can remove this elvis when our minimum AGP is >= 8.2.
525+
// This value (ndkVersion) is nullable on AGP versions below that.
526+
// See https://developer.android.com/reference/tools/gradle-api/8.1/com/android/build/api/dsl/CommonExtension#ndkVersion().
527+
@Suppress("USELESS_ELVIS")
528+
val projectNdkVersion: String =
529+
getAndroidExtension(project).ndkVersion ?: ndkVersionIfUnspecified
522530
var maxPluginNdkVersion = projectNdkVersion
523531
var numProcessedPlugins = pluginList.size
524532
val pluginsWithHigherSdkVersion = mutableListOf<PluginVersionPair>()
@@ -543,8 +551,13 @@ object FlutterPluginUtils {
543551
)
544552
)
545553
}
554+
555+
// TODO(gmackall): We can remove this elvis when our minimum AGP is >= 8.2.
556+
// This value (ndkVersion) is nullable on AGP versions below that.
557+
// See https://developer.android.com/reference/tools/gradle-api/8.1/com/android/build/api/dsl/CommonExtension#ndkVersion().
558+
@Suppress("USELESS_ELVIS")
546559
val pluginNdkVersion: String =
547-
getAndroidExtension(pluginProject).ndkVersion
560+
getAndroidExtension(pluginProject).ndkVersion ?: ndkVersionIfUnspecified
548561
maxPluginNdkVersion =
549562
VersionUtils.mostRecentSemanticVersion(
550563
pluginNdkVersion,

0 commit comments

Comments
 (0)