Skip to content

Avoid eagerly evaluating unstrippedNativeLibsOverride during the configuration phase #8205

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 #8205

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
03906a5
Getting rid of debug logs.
OOS93 May 22, 2026
de12d81
Avoid eager evaluation for mergeNativeLibs path, preventing unexpecte…
OOS93 May 22, 2026
3b52534
Simplifying overridden warning logic and message.
OOS93 May 23, 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
127 changes: 67 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,46 @@ 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,
"generateCrashlyticsSymbolFileDebug",
"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"),
"build/intermediates/merged_native_libs/debug/out"
)
}
}
}
}
"""
"""
.trimIndent()
)
)

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

assertThat(result.output).contains("/some/absolute/string/path")
assertThat(result.output).contains("/another/absolute/string/path")
Expand All @@ -135,12 +111,35 @@ class CrashlyticsExtensionTests {
// Verify the relative paths were made absolute by checking the prepended slash
assertThat(result.output).contains("/relative/path")
assertThat(result.output).contains("/relative/project/file/path")

// Verify warning is only present when manual overrides points to mergeDebugNativeLibs output.
assertThat(result.output)
.contains(
"""
The unstrippedNativeLibsDir is manually overridden.
This is unnecessary, it is safe to remove from the unstrippedNativeLibsDir
override in the CrashlyticsExtension configuration block.
"""
.trimIndent()
)
}

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

val thrown =
Assertions.assertThrows(UnexpectedBuildFailure::class.java) {
buildGradleRunner(projectDir, "generateCrashlyticsSymbolFileDebug", "--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 +155,29 @@ class CrashlyticsExtensionTests {
buildTypes {
debug {
configure<CrashlyticsExtension> {
unstrippedNativeLibsDir = 42
nativeSymbolUploadEnabled = true
$unstrippedNativeLibsDirArg
}
}
}
}

// Custom test task to print the configured paths at realization phase.
abstract class VerifyPathsTask : DefaultTask() {
@get:InputFiles
abstract val filesToVerify: ConfigurableFileCollection
@TaskAction
fun verify() {
filesToVerify.forEach {
println("VERIFIED_PATH=" + it.absolutePath)
}
}
}
tasks.register<VerifyPathsTask>("verifyCrashlyticsPaths") {
val debugBuildType = android.buildTypes.getByName("debug")
val extension = debugBuildType.extensions.getByName("firebaseCrashlytics") as CrashlyticsExtension

filesToVerify.setFrom(extension.unstrippedNativeLibsDir)
}
"""
)

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

assertThat(thrown)
.hasMessageThat()
.contains("Cannot convert the provided notation to a File: 42")
}
}
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}")
}
}
63 changes: 26 additions & 37 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 @@ -31,7 +31,6 @@ import com.google.firebase.crashlytics.buildtools.ndk.internal.csym.NdkCSymGener
import java.io.File
import org.gradle.api.DefaultTask
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
Expand Down Expand Up @@ -99,34 +98,35 @@ abstract class GenerateSymbolFileTask : DefaultTask() {
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."
)
// Extract default provider outside the lambda to avoid closure capture,
// thus preventing possible Gradle serialization issues.
val defaultMergedNativeLibs = variant.artifacts.get(SingleArtifact.MERGED_NATIVE_LIBS)

// Setting provider as a lazy mechanism which will be invoked at execution phase.
this.unstrippedNativeLibsDirs.setFrom(
project.provider {
if (unstrippedNativeLibsOverride.isEmpty) {
defaultMergedNativeLibs
} else {
unstrippedNativeLibsOverride
}
}
)

// Check if overridden path contains a reference for mergeNativeLibs path.
val logOverrideWarn =
unstrippedNativeLibsOverride.any { it.path.contains("/intermediates/merged_native_libs/") }

if (logOverrideWarn) {
logger.warn(
"""
The unstrippedNativeLibsDir is manually overridden.
This is unnecessary, it is safe to remove from the unstrippedNativeLibsDir
override in the CrashlyticsExtension configuration block.
"""
.trimIndent()
)
}
unstrippedNativeLibsDirs.setFrom(unstrippedNativeLibsOverride)
}

/** Sets and validates the symbol generator type. */
Expand Down Expand Up @@ -188,17 +188,6 @@ abstract class GenerateSymbolFileTask : DefaultTask() {
)

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