From 03906a50172f76d41dffd45456ef3413ffd31d70 Mon Sep 17 00:00:00 2001 From: Omar Orozco Date: Thu, 21 May 2026 20:06:57 -0600 Subject: [PATCH 1/3] Getting rid of debug logs. --- .../buildtools/gradle/CrashlyticsVariantExtension.kt | 11 ----------- .../buildtools/gradle/tasks/GenerateSymbolFileTask.kt | 11 ----------- 2 files changed, 22 deletions(-) diff --git a/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsVariantExtension.kt b/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsVariantExtension.kt index 9b46574b5f4..b0a7a747ae3 100644 --- a/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsVariantExtension.kt +++ b/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsVariantExtension.kt @@ -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}") } } diff --git a/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt b/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt index f78eed97c7e..9dcaea0ad85 100644 --- a/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt +++ b/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt @@ -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}") - } } From de12d81402e9127f742e1855bad6004307bddef9 Mon Sep 17 00:00:00 2001 From: Omar Orozco Date: Fri, 22 May 2026 11:51:06 -0600 Subject: [PATCH 2/3] Avoid eager evaluation for mergeNativeLibs path, preventing unexpected behaviors at Gradle Configuration Phase. Current approach check (via a lazy provider) at Execution Phase which source of truth to use. --- .../gradle/CrashlyticsExtensionTests.kt | 122 +++++++++--------- .../gradle/tasks/GenerateSymbolFileTask.kt | 57 ++++---- 2 files changed, 93 insertions(+), 86 deletions(-) diff --git a/firebase-crashlytics-gradle/src/functionalTest/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsExtensionTests.kt b/firebase-crashlytics-gradle/src/functionalTest/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsExtensionTests.kt index 3e6d46327f9..84d31c4f0ba 100644 --- a/firebase-crashlytics-gradle/src/functionalTest/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsExtensionTests.kt +++ b/firebase-crashlytics-gradle/src/functionalTest/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsExtensionTests.kt @@ -62,31 +62,16 @@ 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 { - 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") } @@ -94,38 +79,29 @@ class CrashlyticsExtensionTests { @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 { - 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") @@ -135,12 +111,30 @@ 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( + "This is unnecessary, it is safe to remove (build/intermediates/merged_native_libs/debug/out)" + ) } @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 { @@ -156,21 +150,29 @@ class CrashlyticsExtensionTests { buildTypes { debug { configure { - 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("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") - } } diff --git a/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt b/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt index 9dcaea0ad85..941f4d25dff 100644 --- a/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt +++ b/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt @@ -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 @@ -99,34 +98,40 @@ 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 } } + ) + + if (!unstrippedNativeLibsOverride.isEmpty) { + val currentVariant = variant.name + + // Resolve the build directory name dynamically to support custom build directories. + val buildDirName = project.layout.buildDirectory.get().asFile.name + val mergedNativeLibsOutput = + "$buildDirName/intermediates/merged_native_libs/$currentVariant/out" + + if (unstrippedNativeLibsOverride.any { it.path.contains(mergedNativeLibsOutput) }) { + logger.warn( + """ + The unstrippedNativeLibsDir is manually overridden to output of ($mergedNativeLibsOutput). + This is unnecessary, it is safe to remove ($mergedNativeLibsOutput) from the unstrippedNativeLibsDir override + in the CrashlyticsExtension configuration block. + """ + .trimIndent() + ) + } } - unstrippedNativeLibsDirs.setFrom(unstrippedNativeLibsOverride) } /** Sets and validates the symbol generator type. */ From 3b525346244ffe347811b35f5dd7ab5542f1e178 Mon Sep 17 00:00:00 2001 From: Omar Orozco Date: Fri, 22 May 2026 18:51:10 -0600 Subject: [PATCH 3/3] Simplifying overridden warning logic and message. --- .../gradle/CrashlyticsExtensionTests.kt | 7 ++++- .../gradle/tasks/GenerateSymbolFileTask.kt | 29 ++++++++----------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/firebase-crashlytics-gradle/src/functionalTest/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsExtensionTests.kt b/firebase-crashlytics-gradle/src/functionalTest/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsExtensionTests.kt index 84d31c4f0ba..b826503e97d 100644 --- a/firebase-crashlytics-gradle/src/functionalTest/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsExtensionTests.kt +++ b/firebase-crashlytics-gradle/src/functionalTest/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsExtensionTests.kt @@ -115,7 +115,12 @@ class CrashlyticsExtensionTests { // Verify warning is only present when manual overrides points to mergeDebugNativeLibs output. assertThat(result.output) .contains( - "This is unnecessary, it is safe to remove (build/intermediates/merged_native_libs/debug/out)" + """ + The unstrippedNativeLibsDir is manually overridden. + This is unnecessary, it is safe to remove from the unstrippedNativeLibsDir + override in the CrashlyticsExtension configuration block. + """ + .trimIndent() ) } diff --git a/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt b/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt index 941f4d25dff..5318acaff32 100644 --- a/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt +++ b/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/tasks/GenerateSymbolFileTask.kt @@ -113,24 +113,19 @@ abstract class GenerateSymbolFileTask : DefaultTask() { } ) - if (!unstrippedNativeLibsOverride.isEmpty) { - val currentVariant = variant.name - - // Resolve the build directory name dynamically to support custom build directories. - val buildDirName = project.layout.buildDirectory.get().asFile.name - val mergedNativeLibsOutput = - "$buildDirName/intermediates/merged_native_libs/$currentVariant/out" - - if (unstrippedNativeLibsOverride.any { it.path.contains(mergedNativeLibsOutput) }) { - logger.warn( - """ - The unstrippedNativeLibsDir is manually overridden to output of ($mergedNativeLibsOutput). - This is unnecessary, it is safe to remove ($mergedNativeLibsOutput) from the unstrippedNativeLibsDir override - in the CrashlyticsExtension configuration block. + // 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() - ) - } + .trimIndent() + ) } }