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..fc7d64474dd 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,33 @@ 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 not present when manual overrides points to mergeDebugNativeLibs output but + // is set up using an array. + // The warning is expected to be shown as a notification for users who in the past make use of a + // widely known workaround for flavors and native merged libraries handling. + // There is no real use case for passing this path in an array, but just to prove the solution + // only aims for single provided overrides this check is added. + assertThat(result.output) + .doesNotContain("it is safe to remove from the unstrippedNativeLibsDir") } @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 +153,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/CrashlyticsVariantExtension.kt b/firebase-crashlytics-gradle/src/main/kotlin/com/google/firebase/crashlytics/buildtools/gradle/CrashlyticsVariantExtension.kt index 9b46574b5f4..2d03f2ebe54 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 @@ -47,7 +47,22 @@ constructor(config: VariantExtensionConfig<*>) : VariantExtension, Serializable // Only set the properties for non-null values. crashlyticsExtension.mappingFileUploadEnabled?.let(mappingFileUploadEnabled::set) crashlyticsExtension.nativeSymbolUploadEnabled?.let(nativeSymbolUploadEnabled::set) - crashlyticsExtension.unstrippedNativeLibsDir?.let { unstrippedNativeLibsOverride.from(it) } + crashlyticsExtension.unstrippedNativeLibsDir?.let { nativeLibsOverride -> + if ( + nativeLibsOverride is String && + nativeLibsOverride.contains("/intermediates/merged_native_libs/") + ) { + 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() + ) + } + unstrippedNativeLibsOverride.from(nativeLibsOverride) + } crashlyticsExtension.symbolGeneratorType?.let(symbolGeneratorType::set) crashlyticsExtension.breakpadBinary?.let(breakpadBinary::set) @@ -59,16 +74,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..1a4978d0783 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 @@ -87,46 +86,31 @@ 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. + * Validate the [unstrippedNativeLibsOverride] is not empty at execution phase, with a fallback + * natively pointing out to [SingleArtifact.MERGED_NATIVE_LIBS] directory. * * 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( - project: Project, + private fun setUnstrippedNativeLibsDirs( 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 } } - } - unstrippedNativeLibsDirs.setFrom(unstrippedNativeLibsOverride) + ) } /** Sets and validates the symbol generator type. */ @@ -181,24 +165,12 @@ abstract class GenerateSymbolFileTask : DefaultTask() { this.breakpadExtractionDir.set(buildDir(project, variant, "dump_syms")) this.symbolFileOutputDir.set(buildDir(project, variant, "nativeSymbols")) - validateUnstrippedNativeLibsDirs( - project, + setUnstrippedNativeLibsDirs( 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}") - } }