Sitelet https://github.com/flutter/flutter/pull/166277/files
Skip to content

Move .cxx directory out of android/app #166277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 7 commits into from
Mar 31, 2025
Merged

Move .cxx directory out of android/app #166277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
376b08d
move the dir
mackall-work Mar 31, 2025
8cea0ad
write test
mackall-work Mar 31, 2025
b0a85f7
format
mackall-work Mar 31, 2025
f9ae11d
change message to provide the path, and to be more specific about wha…
mackall-work Mar 31, 2025
e58e062
format
mackall-work Mar 31, 2025
81082b8
fix mocking for forcendkdownload test
mackall-work Mar 31, 2025
7d78480
expand comment
mackall-work Mar 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions dev/devicelab/bin/tasks/gradle_plugin_fat_apk_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,27 @@ Future<void> 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);
Expand Down
17 changes: 17 additions & 0 deletions packages/flutter_tools/gradle/src/main/kotlin/FlutterPluginUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment on lines +623 to +625

@gnprice gnprice Mar 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable. Over at #160372 (comment) on the issue, you wrote:

Interestingly it looks like the AGP maintainers sort of disagree, and explicitly say not to to put the external native build output directory in your projects temporary build directory. I think it probably isn't a problem for us, though, as we aren't actually building anything anyways (we are just tricking AGP into downloading the NDK).

Did they say that anywhere that can be linked to? I'm curious also if they added anything about their thinking on why putting it in the temporary build directory is undesirable.

@gmackall gmackall Mar 31, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, should have included the link but it is in
https://developer.android.com/reference/tools/gradle-api/8.3/null/com/android/build/api/dsl/Cmake#buildStagingDirectory(kotlin.Any)

If you specify a path that's a subdirectory of your project's temporary build/ directory, you get a build error. That's because files in this directory do not persist through clean builds. So, you should either keep using the default <project_dir>//.cxx/ directory or specify a path outside the temporary build directory.

@gnprice gnprice Mar 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. If I'm reading that right, it's saying that that's a constraint which gets enforced — if you specify a directory it doesn't like by this criterion, then that's a build error. So if builds are nevertheless working with this change, then I guess there's no error to worry about.

And it sounds like the motivation is that they want these files to survive a gradle clean so that you don't sit there rebuilding them for a long time. As you said, that doesn't matter because we're not actually building anything.

@gmackall gmackall Mar 31, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I should have been clearer, I was

  1. extrapolating the note in the AGP docs based on assumed motiviation from (you shouldn't use the specific build/ subdirectory that AGP knows about) to (you shouldn't use any build/ subdirectory)
  2. resolving that extrapolated concern by saying we aren't building anything anyways.

Will update the comment to be clearer

.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.
Expand Down
36 changes: 30 additions & 6 deletions packages/flutter_tools/gradle/src/test/kotlin/FlutterPluginUtilsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -845,24 +847,33 @@ class FlutterPluginUtilsTest {
val project = mockk<Project>()
val mockCmakeOptions = mockk<CmakeOptions>()
val mockDefaultConfig = mockk<DefaultConfig>()
val mockDirectoryProperty = mockk<DirectoryProperty>()
val mockDirectory = mockk<Directory>()
every {
project.extensions
.findByType(BaseExtension::class.java)!!
.externalNativeBuild.cmake
} 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<String>()) } 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",
Expand Down Expand Up @@ -1276,11 +1287,18 @@ class FlutterPluginUtilsTest {
every { tasks } returns
mockk<TaskContainer> {
val registerTaskNameSlot = slot<String>()
every { register(capture(registerTaskNameSlot), capture(registerTaskSlot)) } answers registerAnswer@{
every {
register(
capture(registerTaskNameSlot),
capture(registerTaskSlot)
)
} answers registerAnswer@{
val mockRegisterTask =
mockk<Task> {
every { name } returns registerTaskNameSlot.captured
every { description = capture(descriptionSlot) } returns Unit
every {
description = capture(descriptionSlot)
} returns Unit
every { dependsOn(any<ProcessAndroidResources>()) } returns mockk()
val doLastActionSlot = slot<Action<Task>>()
every { doLast(capture(doLastActionSlot)) } answers doLastAnswer@{
Expand All @@ -1302,7 +1320,8 @@ class FlutterPluginUtilsTest {
}

variants.forEach { variant ->
val testOutputs: DomainObjectCollection<BaseVariantOutput> = mockk<DomainObjectCollection<BaseVariantOutput>>()
val testOutputs: DomainObjectCollection<BaseVariantOutput> =
mockk<DomainObjectCollection<BaseVariantOutput>>()
val baseVariantSlot = slot<Action<BaseVariantOutput>>()
val baseVariantOutput = mockk<BaseVariantOutput>()
// Create a real file in a temp directory.
Expand All @@ -1313,7 +1332,9 @@ class FlutterPluginUtilsTest {
manifest.writeText(manifestText)
val mockProcessResourcesProvider = mockk<TaskProvider<ProcessAndroidResources>>()
val mockProcessResources = mockk<ProcessAndroidResources>()
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
Expand All @@ -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<ProcessAndroidResources>()) }
}
// Output assertions are minimal which ensures code is running but is not exhaustive testing.
Expand Down