Skip to content

Avoid eagerly evaluating unstrippedNativeLibsOverride during the configuration phase #8174

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

Open
OOS93 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Avoid eagerly evaluating unstrippedNativeLibsOverride during the configuration phase #8174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1b94e2
Avoid eagerly evaluating unstrippedNativeLibsOverride during the conf…
OOS93 May 15, 2026
0619553
Cleaning up debugging methods.
OOS93 May 15, 2026
bfdb848
Updating CrashlyticsExtensionTests by adding a new task in the test b…
OOS93 May 20, 2026
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
103 changes: 43 additions & 60 deletions ...est/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsExtensionTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,70 +62,33 @@ class CrashlyticsExtensionTests {
@Test
fun `set unstrippedNativeLibsDir to single path`() {
buildFile.writeText(
"""
import com.google.firebase.crashlytics.buildtools.gradle.CrashlyticsExtension

plugins {
id("com.android.application") version "8.1.4"
id("com.google.gms.google-services") version "4.4.1"
id("com.google.firebase.crashlytics") version "$pluginVersion"
}

android {
compileSdk = 33
namespace = "com.google.firebase.testing.crashlytics"

buildTypes {
debug {
configure<CrashlyticsExtension> {
unstrippedNativeLibsDir = "/some/absolute/string/path"
}
}
}
}
"""
getBuildFileStringTemplate("unstrippedNativeLibsDir = \"/some/absolute/string/path\"")
)

val result = buildGradleRunner(projectDir, "-d", ":tasks", "--configuration-cache")
val result = buildGradleRunner(projectDir, "verifyCrashlyticsPaths", "--configuration-cache")

assertThat(result.output).contains("/some/absolute/string/path")
}

@Test
fun `set unstrippedNativeLibsDir to array of multiple path types`() {
buildFile.writeText(
"""
import com.google.firebase.crashlytics.buildtools.gradle.CrashlyticsExtension

plugins {
id("com.android.application") version "8.1.4"
id("com.google.gms.google-services") version "4.4.1"
id("com.google.firebase.crashlytics") version "$pluginVersion"
}

android {
compileSdk = 33
namespace = "com.google.firebase.testing.crashlytics"

buildTypes {
debug {
configure<CrashlyticsExtension> {
unstrippedNativeLibsDir = arrayOf(
getBuildFileStringTemplate(
"""
unstrippedNativeLibsDir = arrayOf(
"/some/absolute/string/path",
"/another/absolute/string/path",
File("/a/file/object/path"),
project.files("/absolute/project/file/path"),
"relative/path",
project.files("relative/project/file/path"),
)
}
}
}
}
"""
"""
.trimIndent()
)
)

val result = buildGradleRunner(projectDir, "-d", ":tasks", "--configuration-cache")
val result = buildGradleRunner(projectDir, "verifyCrashlyticsPaths", "--configuration-cache")

assertThat(result.output).contains("/some/absolute/string/path")
assertThat(result.output).contains("/another/absolute/string/path")
Expand All @@ -139,8 +102,20 @@ class CrashlyticsExtensionTests {

@Test
fun `set unstrippedNativeLibsDir to invalid type throws`() {
buildFile.writeText(
"""
buildFile.writeText(getBuildFileStringTemplate("unstrippedNativeLibsDir = 42"))

val thrown =
Assertions.assertThrows(UnexpectedBuildFailure::class.java) {
buildGradleRunner(projectDir, "verifyCrashlyticsPaths", "--configuration-cache")
}

assertThat(thrown)
.hasMessageThat()
.contains("Cannot convert the provided notation to a File: 42")
}

private fun getBuildFileStringTemplate(unstrippedNativeLibsDirArg: String): String =
"""
import com.google.firebase.crashlytics.buildtools.gradle.CrashlyticsExtension

plugins {
Expand All @@ -156,21 +131,29 @@ class CrashlyticsExtensionTests {
buildTypes {
debug {
configure<CrashlyticsExtension> {
unstrippedNativeLibsDir = 42
$unstrippedNativeLibsDirArg
}
}
}
}
"""
)

val thrown =
Assertions.assertThrows(UnexpectedBuildFailure::class.java) {
buildGradleRunner(projectDir, "-d", ":tasks", "--configuration-cache")
}

abstract class VerifyPathsTask : DefaultTask() {
@get:InputFiles
abstract val filesToVerify: ConfigurableFileCollection

@TaskAction
fun verify() {
filesToVerify.forEach {
println("VERIFIED_PATH=" + it.absolutePath)
}
}
}

assertThat(thrown)
.hasMessageThat()
.contains("Cannot convert the provided notation to a File: 42")
}
tasks.register<VerifyPathsTask>("verifyCrashlyticsPaths") {
val debugBuildType = android.buildTypes.getByName("debug")
val extension = debugBuildType.extensions.getByName("firebaseCrashlytics") as CrashlyticsExtension

filesToVerify.setFrom(extension.unstrippedNativeLibsDir)
}
"""
}
11 changes: 0 additions & 11 deletions ...n/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsVariantExtension.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,5 @@ constructor(config: VariantExtensionConfig<*>) : VariantExtension, Serializable
)
}
}

printDebugProperties(config)
}

private fun printDebugProperties(config: VariantExtensionConfig<*>) {
logger.debug("CrashlyticsVariantExtension for variant: ${config.variant.name}")
logger.debug(" mappingFileUploadEnabled: ${mappingFileUploadEnabled.orNull}")
logger.debug(" nativeSymbolUploadEnabled: ${nativeSymbolUploadEnabled.orNull}")
logger.debug(" unstrippedNativeLibsOverride: ${unstrippedNativeLibsOverride.asPath}")
logger.debug(" symbolGeneratorType: ${symbolGeneratorType.orNull}")
logger.debug(" breakpadBinary: ${breakpadBinary.orNull?.asFile?.path}")
}
}
99 changes: 50 additions & 49 deletions .../kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ import com.google.firebase.crashlytics.buildtools.ndk.internal.breakpad.Breakpad
import com.google.firebase.crashlytics.buildtools.ndk.internal.csym.NdkCSymGenerator
import java.io.File
import org.gradle.api.DefaultTask
import org.gradle.api.GradleException
import org.gradle.api.Project
import org.gradle.api.UnknownTaskException
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.file.RegularFileProperty
import org.gradle.api.provider.Property
import org.gradle.api.tasks.CacheableTask
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFile
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.Internal
Expand All @@ -53,6 +54,7 @@ import org.gradle.kotlin.dsl.register
@Suppress("UnstableApiUsage") // SingleArtifact.MERGED_NATIVE_LIBS
@CacheableTask
abstract class GenerateSymbolFileTask : DefaultTask() {
@get:Input abstract val variantName: Property<String>
@get:[InputFiles PathSensitive(PathSensitivity.RELATIVE) SkipWhenEmpty]
abstract val unstrippedNativeLibsDirs: ConfigurableFileCollection
@get:[InputFile PathSensitive(PathSensitivity.NONE) Optional]
Expand All @@ -69,6 +71,8 @@ abstract class GenerateSymbolFileTask : DefaultTask() {

@TaskAction
fun generateSymbolFiles() {
validatesUnstrippedNativeLibsDirs()

val generator: NativeSymbolGenerator =
when (symbolGeneratorType.get() as SymbolGeneratorType) {
SymbolGeneratorType.BREAKPAD -> BreakpadSymbolGenerator(resolveBreakpadBinary())
Expand All @@ -84,49 +88,56 @@ abstract class GenerateSymbolFileTask : DefaultTask() {
}
}

/**
* Sets and validates the unstripped native libs directories.
*
* Validate the [unstrippedNativeLibsOverride] does not contain a directory manually set to the
* output of the [SingleArtifact.MERGED_NATIVE_LIBS] without include the dependency.
*
* This happens because for a while the plugin did not properly handle product flavors, so
* customers would manually configure this to the output of the merged native libs task in a way
* that didn't include the task dependencies.
*/
private fun validateUnstrippedNativeLibsDirs(
/** Sets a provider for the unstripped native libs directories. */
private fun setUnstrippedNativeLibsDirs(
project: Project,
variant: Variant,
unstrippedNativeLibsOverride: ConfigurableFileCollection,
) {
if (unstrippedNativeLibsOverride.isEmpty) {
unstrippedNativeLibsDirs.setFrom(variant.artifacts.get(SingleArtifact.MERGED_NATIVE_LIBS))
return
}

val mergedNativeLibsOutput = "build/intermediates/merged_native_libs/${variant.name}/out"
if (unstrippedNativeLibsOverride.any { it.path.contains(mergedNativeLibsOutput) }) {
val mergedNativeLibsTask = "merge${variant.name.capitalized()}NativeLibs"

val dependencies =
unstrippedNativeLibsOverride.buildDependencies
.getDependencies(null)
.mapNotNull { it?.name }
.toSet()

if (!dependencies.contains(mergedNativeLibsTask)) {
try {
dependsOn(project.tasks.getByPath(mergedNativeLibsTask))
} catch (ex: UnknownTaskException) {
logger.warn(
"The unstrippedNativeLibsDir manually overridden to output of $mergedNativeLibsTask " +
"task. This is not necessary, it is safe to remove $mergedNativeLibsOutput from " +
"the unstrippedNativeLibsDir override."
)
// Setting provider as a lazy mechanism which will be invoked at execution phase.
this.unstrippedNativeLibsDirs.setFrom(
project.provider {
if (unstrippedNativeLibsOverride.isEmpty) {
variant.artifacts.get(SingleArtifact.MERGED_NATIVE_LIBS)
} else {
unstrippedNativeLibsOverride
}
}
)
}

/**
* Validate the [unstrippedNativeLibsOverride] does not contain a directory manually set to the
* output of the [SingleArtifact.MERGED_NATIVE_LIBS] without include the dependency.
*/
private fun validatesUnstrippedNativeLibsDirs() {
val currentVariant = variantName.get()
val mergedNativeLibsOutput = "build/intermediates/merged_native_libs/$currentVariant/out"

// Check to validate if merged dest is needed.
val reliesOnMergedLibs =
unstrippedNativeLibsDirs.any { it.path.contains(mergedNativeLibsOutput) }

// If mergedNativeLibsOutput is empty means that Gradle did not execute the merging task due to
// the lack of dependsOn.
val isMergedDestValid =
unstrippedNativeLibsDirs.files.any { file ->
file.exists() && (file.isFile || (file.isDirectory && file.list()?.isNotEmpty() == true))
}

if (reliesOnMergedLibs && !isMergedDestValid) {
throw GradleException(
"""
Crashlytics Error: Missing Task Dependency.
The files in 'unstrippedNativeLibsDir' come from a Gradle generated directory ($mergedNativeLibsOutput),
but this task does not depend on the producer task.

Fix this by adding an explicit 'dependsOn' to the merge task in your build script,
or let Crashlytics handle it automatically by not overriding this property.
"""
.trimIndent()
)
}
unstrippedNativeLibsDirs.setFrom(unstrippedNativeLibsOverride)
}

/** Sets and validates the symbol generator type. */
Expand Down Expand Up @@ -180,25 +191,15 @@ abstract class GenerateSymbolFileTask : DefaultTask() {
}
this.breakpadExtractionDir.set(buildDir(project, variant, "dump_syms"))
this.symbolFileOutputDir.set(buildDir(project, variant, "nativeSymbols"))
this.variantName.set(variant.name)

validateUnstrippedNativeLibsDirs(
setUnstrippedNativeLibsDirs(
project,
variant,
crashlyticsExtension.unstrippedNativeLibsOverride,
)

validateSymbolGeneratorType(project, crashlyticsExtension.symbolGeneratorType)

printDebugProperties()
}
}

private fun printDebugProperties() {
logger.debug("GenerateSymbolFileTask:")
logger.debug(" unstrippedNativeLibsDirs: ${unstrippedNativeLibsDirs.asPath}")
logger.debug(" breakpadBinary: ${breakpadBinary.orNull?.asFile?.path}")
logger.debug(" symbolGeneratorType: ${symbolGeneratorType.orNull?.toString()}")
logger.debug(" breakpadExtractionDir: ${breakpadExtractionDir.orNull?.asFile?.path}")
logger.debug(" symbolFileOutputDir: ${symbolFileOutputDir.orNull?.asFile?.path}")
}
}
Loading