Skip to content

Commit adca65f

Browse files
authored
[Android] remove obsolete gradle api in FGP (flutter#172085)
fixes flutter#170791 This PR replaces the deprecated (and removed in 9.0) Gradle fileMode API in favor of filePermissions. This change finally unlocks builds with gradle 9 (confirmed in smoke tests, 9.0.0-rc.2 is the latest prerelease version to the date). **Testing strategy** There's an attempt to add a Kotlin unit test that confirms that `filePermissions` were called during plugin application. I failed to find a more precise way to check if the new code works as intended (such as testing that permissions were really changed or something like that), but on the other hand, it should not be required since it will transform the test in a way that it will start to test Gradle APIs and their behavior, and I believe it's a bit out of scope: Gradle APIs are tested in Gradle tests :) Anyway, this new test is a bit cumbersome because it's required to mock all the behaviours related to variants configuration and capturing calls – If it is not desired im not hesitant to remove it. <!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> ## 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. - [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
1 parent 40cbea2 commit adca65f

File tree

3 files changed

+188
-5
lines changed

3 files changed

+188
-5
lines changed

dev/devicelab/bin/tasks/android_java17_dependency_smoke_tests.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ List<VersionTuple> versionTuples = <VersionTuple>[
2121
// Template
2222
VersionTuple(agpVersion: '8.9.1', gradleVersion: '8.12', kotlinVersion: '2.1.0'),
2323
// Max known
24-
VersionTuple(agpVersion: '8.10.0', gradleVersion: '8.12', kotlinVersion: '2.2.0'),
24+
VersionTuple(agpVersion: '8.10.0', gradleVersion: '9.0.0-rc-2', kotlinVersion: '2.2.0'),
2525
/* Others */
2626
VersionTuple(agpVersion: '8.4.0', gradleVersion: '8.6', kotlinVersion: '1.8.22'),
2727
VersionTuple(agpVersion: '8.6.0', gradleVersion: '8.7', kotlinVersion: '1.8.22'),

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -755,10 +755,12 @@ class FlutterPlugin : Plugin<Project> {
755755
) {
756756
dependsOn(compileTask)
757757
with(compileTask.assets)
758-
// TODO(gmackall): Replace with filePermissions.user.read/write = true once
759-
// minimum supported Gradle version is 8.3.
760-
@Suppress("DEPRECATION")
761-
fileMode = 420 // corresponds to unix 0644 in base 8
758+
filePermissions {
759+
user {
760+
read = true
761+
write = true
762+
}
763+
}
762764
if (isUsedAsSubproject) {
763765
// TODO(gmackall): above is always false, can delete
764766
dependsOn(packageAssets)

packages/flutter_tools/gradle/src/test/kotlin/FlutterPluginTest.kt

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,25 @@ import com.android.build.api.dsl.ApplicationDefaultConfig
44
import com.android.build.api.dsl.ApplicationExtension
55
import com.android.build.gradle.AbstractAppExtension
66
import com.android.build.gradle.BaseExtension
7+
import com.android.build.gradle.api.AndroidSourceDirectorySet
8+
import com.android.build.gradle.internal.core.InternalBaseVariant
9+
import com.android.build.gradle.tasks.MergeSourceSetFolders
10+
import com.android.build.gradle.tasks.ProcessAndroidResources
11+
import com.flutter.gradle.tasks.FlutterTask
712
import io.mockk.every
813
import io.mockk.mockk
914
import io.mockk.mockkObject
15+
import io.mockk.slot
1016
import io.mockk.verify
17+
import org.gradle.api.Action
1118
import org.gradle.api.Project
19+
import org.gradle.api.Task
1220
import org.gradle.api.file.Directory
21+
import org.gradle.api.tasks.Copy
22+
import org.gradle.api.tasks.TaskContainer
23+
import org.gradle.api.tasks.TaskProvider
1324
import org.jetbrains.kotlin.gradle.plugin.extraProperties
25+
import org.junit.jupiter.api.Assertions.fail
1426
import org.junit.jupiter.api.io.TempDir
1527
import java.nio.file.Path
1628
import kotlin.io.path.writeText
@@ -55,6 +67,8 @@ class FlutterPluginTest {
5567
val mockDirectory = mockk<Directory>(relaxed = true)
5668
every { project.layout.buildDirectory.get() } returns mockDirectory
5769
val mockAndroidSourceSet = mockk<com.android.build.gradle.api.AndroidSourceSet>(relaxed = true)
70+
val mockAndroidSourceDirectorySet = mockk<AndroidSourceDirectorySet>(relaxed = true)
71+
every { mockAndroidSourceSet.jniLibs.srcDir(any()) } returns mockAndroidSourceDirectorySet
5872
every { mockAbstractAppExtension.sourceSets.getByName("main") } returns mockAndroidSourceSet
5973
// mock return of NativePluginLoaderReflectionBridge.getPlugins
6074
mockkObject(NativePluginLoaderReflectionBridge)
@@ -71,6 +85,173 @@ class FlutterPluginTest {
7185
verify { project.tasks.register("printBuildVariants", any()) }
7286
}
7387

88+
@Test
89+
fun `copyFlutterAssets task sets filePermissions correctly`(
90+
@TempDir tempDir: Path
91+
) {
92+
val projectDir = tempDir.resolve("project-dir").resolve("android").resolve("app")
93+
projectDir.toFile().mkdirs()
94+
val settingsFile = projectDir.parent.resolve("settings.gradle")
95+
settingsFile.writeText("empty for now")
96+
val fakeFlutterSdkDir = tempDir.resolve("fake-flutter-sdk")
97+
fakeFlutterSdkDir.toFile().mkdirs()
98+
val fakeCacheDir = fakeFlutterSdkDir.resolve("bin").resolve("cache")
99+
fakeCacheDir.toFile().mkdirs()
100+
val fakeEngineStampFile = fakeCacheDir.resolve("engine.stamp")
101+
fakeEngineStampFile.writeText(FAKE_ENGINE_STAMP)
102+
val fakeEngineRealmFile = fakeCacheDir.resolve("engine.realm")
103+
fakeEngineRealmFile.writeText(FAKE_ENGINE_REALM)
104+
val project = mockk<Project>(relaxed = true)
105+
val mockAbstractAppExtension = mockk<AbstractAppExtension>(relaxed = true)
106+
every { project.extensions.findByType(AbstractAppExtension::class.java) } returns mockAbstractAppExtension
107+
every { project.extensions.getByType(AbstractAppExtension::class.java) } returns mockAbstractAppExtension
108+
every { project.extensions.findByName("android") } returns mockAbstractAppExtension
109+
every { project.projectDir } returns projectDir.toFile()
110+
every { project.findProperty("flutter.sdk") } returns fakeFlutterSdkDir.toString()
111+
every { project.file(fakeFlutterSdkDir.toString()) } returns fakeFlutterSdkDir.toFile()
112+
val flutterExtension = FlutterExtension()
113+
every { project.extensions.create("flutter", any<Class<*>>()) } returns flutterExtension
114+
every { project.extensions.findByType(FlutterExtension::class.java) } returns flutterExtension
115+
val mockBaseExtension = mockk<BaseExtension>(relaxed = true)
116+
every { project.extensions.findByType(BaseExtension::class.java) } returns mockBaseExtension
117+
val mockApplicationExtension = mockk<ApplicationExtension>(relaxed = true)
118+
every { project.extensions.findByType(ApplicationExtension::class.java) } returns mockApplicationExtension
119+
val mockApplicationDefaultConfig = mockk<ApplicationDefaultConfig>(relaxed = true)
120+
every { mockApplicationExtension.defaultConfig } returns mockApplicationDefaultConfig
121+
every { project.rootProject } returns project
122+
every { project.state.failure } returns null
123+
val mockDirectory = mockk<Directory>(relaxed = true)
124+
every { project.layout.buildDirectory.get() } returns mockDirectory
125+
val mockAndroidSourceSet = mockk<com.android.build.gradle.api.AndroidSourceSet>(relaxed = true)
126+
val mockAndroidSourceDirectorySet = mockk<AndroidSourceDirectorySet>(relaxed = true)
127+
every { mockAndroidSourceSet.jniLibs.srcDir(any()) } returns mockAndroidSourceDirectorySet
128+
every { mockAbstractAppExtension.sourceSets.getByName("main") } returns mockAndroidSourceSet
129+
// mock return of NativePluginLoaderReflectionBridge.getPlugins
130+
mockkObject(NativePluginLoaderReflectionBridge)
131+
every { NativePluginLoaderReflectionBridge.getPlugins(any(), any()) } returns
132+
listOf()
133+
// mock method calls that are invoked by the args to NativePluginLoaderReflectionBridge
134+
every { project.extraProperties } returns mockk()
135+
every { project.file(flutterExtension.source!!) } returns mockk()
136+
// Set up the task container and our task capture
137+
val taskContainer = mockk<TaskContainer>(relaxed = true)
138+
every { project.tasks } returns taskContainer
139+
val copyTaskActionCaptor = slot<Action<Copy>>()
140+
val copyTask = mockk<Copy>(relaxed = true)
141+
val mockVariant = mockk<com.android.build.gradle.api.ApplicationVariant>(relaxed = true)
142+
every { mockVariant.name } returns "debug"
143+
every { mockVariant.buildType.name } returns "debug"
144+
every { mockVariant.flavorName } returns ""
145+
val mergedFlavor = mockk<InternalBaseVariant.MergedFlavor>(relaxed = true)
146+
every { mockVariant.mergedFlavor } returns mergedFlavor
147+
val apiLevel = mockk<com.android.builder.model.ApiVersion>(relaxed = true)
148+
every { apiLevel.apiLevel } returns 21
149+
every { mergedFlavor.minSdkVersion } returns apiLevel
150+
val variantOutput = mockk<com.android.build.gradle.api.BaseVariantOutput>(relaxed = true)
151+
val outputsIterator = mockk<MutableIterator<com.android.build.gradle.api.BaseVariantOutput>>()
152+
every { outputsIterator.hasNext() } returns true andThen false
153+
every { outputsIterator.next() } returns variantOutput
154+
val variantOutputCollection = mockk<org.gradle.api.DomainObjectCollection<com.android.build.gradle.api.BaseVariantOutput>>()
155+
every { variantOutputCollection.iterator() } returns outputsIterator
156+
every { mockVariant.outputs } returns variantOutputCollection
157+
val processResourcesProvider = mockk<TaskProvider<ProcessAndroidResources>>(relaxed = true)
158+
every { processResourcesProvider.hint(ProcessAndroidResources::class).get() } returns mockk<ProcessAndroidResources>(relaxed = true)
159+
every { variantOutput.processResourcesProvider } returns processResourcesProvider
160+
val assembleTask = mockk<Task>(relaxed = true)
161+
val assembleTaskProvider = mockk<TaskProvider<Task>>(relaxed = true)
162+
every { assembleTaskProvider.get() } returns assembleTask
163+
every { mockVariant.assembleProvider } returns assembleTaskProvider
164+
val variants = listOf(mockVariant)
165+
val variantsIterator = mockk<MutableIterator<com.android.build.gradle.api.ApplicationVariant>>()
166+
every { variantsIterator.hasNext() } returns true andThen false
167+
every { variantsIterator.next() } returns mockVariant
168+
val variantCollection = mockk<org.gradle.api.DomainObjectSet<com.android.build.gradle.api.ApplicationVariant>>()
169+
every { mockAbstractAppExtension.applicationVariants } returns variantCollection
170+
every { variantCollection.iterator() } returns variantsIterator
171+
every {
172+
variantCollection.configureEach(any<Action<com.android.build.gradle.api.ApplicationVariant>>())
173+
} answers {
174+
variants.forEach { firstArg<Action<com.android.build.gradle.api.ApplicationVariant>>().execute(it) }
175+
}
176+
every { mockVariant.mergeAssetsProvider.hint(MergeSourceSetFolders::class).get() } returns
177+
mockk<MergeSourceSetFolders>(relaxed = true)
178+
val flutterTask = mockk<FlutterTask>(relaxed = true)
179+
val copySpec = mockk<org.gradle.api.file.CopySpec>(relaxed = true)
180+
every {
181+
(flutterTask).assets
182+
} returns copySpec
183+
val flutterTaskProvider = mockk<TaskProvider<FlutterTask>>(relaxed = true)
184+
every {
185+
flutterTaskProvider.hint(FlutterTask::class).get()
186+
} returns flutterTask
187+
every {
188+
taskContainer.register(
189+
match { it.contains("compileFlutterBuild") },
190+
any<Class<FlutterTask>>(),
191+
any()
192+
)
193+
} answers {
194+
flutterTaskProvider
195+
}
196+
// Actual task that should be captured to test if permissions have been set
197+
val mockCopyTaskProvider = mockk<TaskProvider<Copy>>(relaxed = true)
198+
every { mockCopyTaskProvider.hint(Copy::class).get() } returns copyTask
199+
every {
200+
taskContainer.register(
201+
match { it.startsWith("copyFlutterAssets") },
202+
eq(Copy::class.java),
203+
capture(copyTaskActionCaptor)
204+
)
205+
} answers {
206+
mockCopyTaskProvider
207+
}
208+
val mockJarTaskProvider = mockk<TaskProvider<org.gradle.api.tasks.bundling.Jar>>(relaxed = true)
209+
every { mockJarTaskProvider.hint(org.gradle.api.tasks.bundling.Jar::class).get() } returns
210+
mockk<org.gradle.api.tasks.bundling.Jar>(relaxed = true)
211+
every {
212+
taskContainer.register(
213+
match { it.contains("packJniLibs") },
214+
eq(org.gradle.api.tasks.bundling.Jar::class.java),
215+
any()
216+
)
217+
} answers {
218+
mockJarTaskProvider
219+
}
220+
val mockTaskProvider = mockk<TaskProvider<Task>>(relaxed = true)
221+
every { mockTaskProvider.hint(Task::class).get() } returns mockk<Task>(relaxed = true)
222+
every {
223+
taskContainer.named(any<String>())
224+
} returns mockTaskProvider
225+
val flutterPlugin = FlutterPlugin()
226+
flutterPlugin.apply(project)
227+
228+
copyTaskActionCaptor.captured.execute(copyTask)
229+
val filePermissionsActionCaptor = slot<Action<org.gradle.api.file.ConfigurableFilePermissions>>()
230+
verify {
231+
copyTask.filePermissions(capture(filePermissionsActionCaptor))
232+
}
233+
if (filePermissionsActionCaptor.isCaptured) {
234+
val mockFilePermissionSet = mockk<org.gradle.api.file.ConfigurableFilePermissions>(relaxed = true)
235+
filePermissionsActionCaptor.captured.execute(mockFilePermissionSet)
236+
val userPermissionsActionCaptor = slot<Action<org.gradle.api.file.ConfigurableUserClassFilePermissions>>()
237+
verify {
238+
mockFilePermissionSet.user(capture(userPermissionsActionCaptor))
239+
}
240+
if (userPermissionsActionCaptor.isCaptured) {
241+
val mockUserPermission = mockk<org.gradle.api.file.ConfigurableUserClassFilePermissions>(relaxed = true)
242+
userPermissionsActionCaptor.captured.execute(mockUserPermission)
243+
verify {
244+
mockUserPermission.read = true
245+
mockUserPermission.write = true
246+
}
247+
} else {
248+
fail("User permissions configuration action was not captured")
249+
}
250+
} else {
251+
fail("FilePermissions configuration action was not captured")
252+
}
253+
}
254+
74255
companion object {
75256
const val FAKE_ENGINE_STAMP = "901b0f1afe77c3555abee7b86a26aaa37f131379"
76257
const val FAKE_ENGINE_REALM = "made_up_realm"

0 commit comments

Comments
 (0)