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..15ff8efa541 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,10 @@ 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, "verifyCrashlyticsPaths", "--configuration-cache") assertThat(result.output).contains("/some/absolute/string/path") } @@ -94,23 +73,9 @@ 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"), @@ -118,14 +83,12 @@ class CrashlyticsExtensionTests { "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") @@ -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 { @@ -156,21 +131,29 @@ class CrashlyticsExtensionTests { buildTypes { debug { configure { - 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("verifyCrashlyticsPaths") { + val debugBuildType = android.buildTypes.getByName("debug") + val extension = debugBuildType.extensions.getByName("firebaseCrashlytics") as CrashlyticsExtension + + filesToVerify.setFrom(extension.unstrippedNativeLibsDir) + } + """ } 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..2ed8485c316 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 @@ -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 @@ -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 @get:[InputFiles PathSensitive(PathSensitivity.RELATIVE) SkipWhenEmpty] abstract val unstrippedNativeLibsDirs: ConfigurableFileCollection @get:[InputFile PathSensitive(PathSensitivity.NONE) Optional] @@ -69,6 +71,8 @@ abstract class GenerateSymbolFileTask : DefaultTask() { @TaskAction fun generateSymbolFiles() { + validatesUnstrippedNativeLibsDirs() + val generator: NativeSymbolGenerator = when (symbolGeneratorType.get() as SymbolGeneratorType) { SymbolGeneratorType.BREAKPAD -> BreakpadSymbolGenerator(resolveBreakpadBinary()) @@ -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. */ @@ -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}") - } }