From f539bb447167b36302bb2237a7ec6dddb2bd896a Mon Sep 17 00:00:00 2001 From: Gray Mackall <34871572+gmackall@users.noreply.github.com> Date: Mon, 31 Mar 2025 15:35:29 -0700 Subject: [PATCH] Move `.cxx` directory out of `android/app` (#166277) This is a build artifact, so move to `build/app/intermediates/flutter`. See/ fixes https://github.com/flutter/flutter/issues/160372 ## 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]. [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 --- .../bin/tasks/gradle_plugin_fat_apk_test.dart | 21 +++++++++++ .../src/main/kotlin/FlutterPluginUtils.kt | 17 +++++++++ .../src/test/kotlin/FlutterPluginUtilsTest.kt | 36 +++++++++++++++---- 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/dev/devicelab/bin/tasks/gradle_plugin_fat_apk_test.dart b/dev/devicelab/bin/tasks/gradle_plugin_fat_apk_test.dart index 50b3be1c3e8..b0cf6dd7e9c 100644 --- a/dev/devicelab/bin/tasks/gradle_plugin_fat_apk_test.dart +++ b/dev/devicelab/bin/tasks/gradle_plugin_fat_apk_test.dart @@ -152,6 +152,27 @@ Future main() async { throw TaskResult.failure("Shared library doesn't exist"); } } + + section('AGP cxx build artifacts'); + + final String defaultPath = path.join(project.rootPath, 'android', 'app', '.cxx'); + + final String modifiedPath = path.join( + project.rootPath, + 'build', + 'app', + 'intermediates', + 'flutter', + '.cxx', + ); + if (Directory(defaultPath).existsSync()) { + throw TaskResult.failure('Producing unexpected build artifacts in $defaultPath'); + } + if (!Directory(modifiedPath).existsSync()) { + throw TaskResult.failure( + 'Not producing external native build output directory in $modifiedPath', + ); + } }); return TaskResult.success(null); diff --git a/packages/flutter_tools/gradle/src/main/kotlin/FlutterPluginUtils.kt b/packages/flutter_tools/gradle/src/main/kotlin/FlutterPluginUtils.kt index dfef3a72f45..bf59732f686 100644 --- a/packages/flutter_tools/gradle/src/main/kotlin/FlutterPluginUtils.kt +++ b/packages/flutter_tools/gradle/src/main/kotlin/FlutterPluginUtils.kt @@ -610,6 +610,23 @@ object FlutterPluginUtils { "$flutterSdkRootPath/packages/flutter_tools/gradle/src/main/groovy/CMakeLists.txt" ) + // AGP defaults to outputting build artifacts in `android/app/.cxx`. This directory is a + // build artifact, so we move it from that directory to within Flutter's build directory + // to avoid polluting source directories with build artifacts. + // + // AGP explicitely recommends not setting the buildStagingDirectory to be within a build + // directory in + // https://developer.android.com/reference/tools/gradle-api/8.3/null/com/android/build/api/dsl/Cmake#buildStagingDirectory(kotlin.Any), + // but as we are not actually building anything (and are instead only tricking AGP into + // downloading the NDK), it is acceptable for the buildStagingDirectory to be removed + // and rebuilt when running clean builds. + gradleProjectAndroidExtension.externalNativeBuild.cmake.buildStagingDirectory( + gradleProject.layout.buildDirectory + .dir("${FlutterPluginConstants.INTERMEDIATES_DIR}/flutter/.cxx") + .get() + .asFile.path + ) + // CMake will print warnings when you try to build an empty project. // These arguments silence the warnings - our project is intentionally // empty. diff --git a/packages/flutter_tools/gradle/src/test/kotlin/FlutterPluginUtilsTest.kt b/packages/flutter_tools/gradle/src/test/kotlin/FlutterPluginUtilsTest.kt index d12b9080d6a..38c655a3c89 100644 --- a/packages/flutter_tools/gradle/src/test/kotlin/FlutterPluginUtilsTest.kt +++ b/packages/flutter_tools/gradle/src/test/kotlin/FlutterPluginUtilsTest.kt @@ -22,6 +22,8 @@ import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.UnknownTaskException import org.gradle.api.artifacts.dsl.DependencyHandler +import org.gradle.api.file.Directory +import org.gradle.api.file.DirectoryProperty import org.gradle.api.logging.Logger import org.gradle.api.tasks.TaskContainer import org.gradle.api.tasks.TaskProvider @@ -845,6 +847,8 @@ class FlutterPluginUtilsTest { val project = mockk() val mockCmakeOptions = mockk() val mockDefaultConfig = mockk() + val mockDirectoryProperty = mockk() + val mockDirectory = mockk() every { project.extensions .findByType(BaseExtension::class.java)!! @@ -852,17 +856,24 @@ class FlutterPluginUtilsTest { } returns mockCmakeOptions every { project.extensions.findByType(BaseExtension::class.java)!!.defaultConfig } returns mockDefaultConfig + val basePath = "/base/path" + val fakeBuildPath = "/randomapp/build/app/" every { mockCmakeOptions.path } returns null every { mockCmakeOptions.path(any()) } returns Unit every { mockDefaultConfig.externalNativeBuild.cmake.arguments(any(), any()) } returns Unit + every { mockCmakeOptions.buildStagingDirectory(any()) } returns Unit + every { project.layout.buildDirectory } returns mockDirectoryProperty + every { mockDirectoryProperty.dir(any()) } returns mockDirectoryProperty + every { mockDirectoryProperty.get() } returns mockDirectory + every { mockDirectory.asFile.path } returns fakeBuildPath - val basePath = "/base/path" FlutterPluginUtils.forceNdkDownload(project, basePath) verify(exactly = 1) { mockCmakeOptions.path } verify(exactly = 1) { mockCmakeOptions.path("$basePath/packages/flutter_tools/gradle/src/main/groovy/CMakeLists.txt") } + verify(exactly = 1) { mockCmakeOptions.buildStagingDirectory(any()) } verify(exactly = 1) { mockDefaultConfig.externalNativeBuild.cmake.arguments( "-Wno-dev", @@ -1276,11 +1287,18 @@ class FlutterPluginUtilsTest { every { tasks } returns mockk { val registerTaskNameSlot = slot() - every { register(capture(registerTaskNameSlot), capture(registerTaskSlot)) } answers registerAnswer@{ + every { + register( + capture(registerTaskNameSlot), + capture(registerTaskSlot) + ) + } answers registerAnswer@{ val mockRegisterTask = mockk { every { name } returns registerTaskNameSlot.captured - every { description = capture(descriptionSlot) } returns Unit + every { + description = capture(descriptionSlot) + } returns Unit every { dependsOn(any()) } returns mockk() val doLastActionSlot = slot>() every { doLast(capture(doLastActionSlot)) } answers doLastAnswer@{ @@ -1302,7 +1320,8 @@ class FlutterPluginUtilsTest { } variants.forEach { variant -> - val testOutputs: DomainObjectCollection = mockk>() + val testOutputs: DomainObjectCollection = + mockk>() val baseVariantSlot = slot>() val baseVariantOutput = mockk() // Create a real file in a temp directory. @@ -1313,7 +1332,9 @@ class FlutterPluginUtilsTest { manifest.writeText(manifestText) val mockProcessResourcesProvider = mockk>() val mockProcessResources = mockk() - every { mockProcessResourcesProvider.hint(ProcessAndroidResources::class).get() } returns mockProcessResources + every { + mockProcessResourcesProvider.hint(ProcessAndroidResources::class).get() + } returns mockProcessResources every { baseVariantOutput.processResourcesProvider } returns mockProcessResourcesProvider // Fallback processing. every { mockProcessResources.manifestFile } returns manifest @@ -1336,7 +1357,10 @@ class FlutterPluginUtilsTest { assert(descriptionSlot.captured.contains("stores app links settings for the given build variant")) assertEquals(variants.size, registerTaskList.size) for (i in 0 until variants.size) { - assertEquals("output${FlutterPluginUtils.capitalize(variants[i].name)}AppLinkSettings", registerTaskList[i].name) + assertEquals( + "output${FlutterPluginUtils.capitalize(variants[i].name)}AppLinkSettings", + registerTaskList[i].name + ) verify(exactly = 1) { registerTaskList[i].dependsOn(any()) } } // Output assertions are minimal which ensures code is running but is not exhaustive testing.